fix(conversation-memory): correct PostToolUse hook input parsing - #487
Conversation
Claude Code hooks pass tool_input and tool_response fields, not result. This fix updates observe-cli.ts to correctly parse the hook input format. Changes: - observe-cli.ts: Use tool_input + tool_response instead of result field - inject-cli.ts: Handle empty stdin gracefully for SessionStart hook - observe-cli.test.ts: Update tests to match new input format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughHandle empty stdin for SessionStart CLI; reshape PostToolUse input to separate Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/conversation-memory/src/cli/observe-cli.ts`:
- Around line 64-69: The merge currently spreads toolResponse into mergedData
and can overwrite toolInput fields (see mergedData, toolInput, toolResponse), so
change the merge to preserve all input fields and namespace any object response
under a single result key (e.g., mergedData = { ...(toolInputObject), result:
toolResponseObject } and for primitives keep result: toolResponse); update the
logic used by compressToolData/compressBash to continue reading input fields
unchanged while the full response is available under result.
🧹 Nitpick comments (1)
plugins/conversation-memory/src/cli/observe-cli.test.ts (1)
396-401: Good edge-case coverage for missingtool_inputand nulltool_response.Consider adding a test that exercises the actual merge logic in
handleObserve— particularly the case wheretool_inputandtool_responseshare overlapping keys — to guard against silent data loss at the integration boundary.Also applies to: 410-415
| const mergedData = { | ||
| ...((toolInput && typeof toolInput === 'object') ? toolInput : {}), | ||
| ...(typeof toolResponse === 'object' && toolResponse !== null ? toolResponse : {}), | ||
| // Include primitive responses as 'result' field | ||
| ...(typeof toolResponse !== 'object' ? { result: toolResponse } : {}), | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for compressToolData function definition
rg "function compressToolData|const compressToolData" -A 15 plugins/conversation-memory/src/Repository: baleen37/claude-plugins
Length of output: 1320
🏁 Script executed:
#!/bin/bash
# Also search for where mergedData is used after being created
rg "mergedData" plugins/conversation-memory/src/cli/observe-cli.ts -B 5 -A 10Repository: baleen37/claude-plugins
Length of output: 830
🏁 Script executed:
#!/bin/bash
# Check the full observe-cli.ts file around the lines in question
cat -n plugins/conversation-memory/src/cli/observe-cli.ts | sed -n '55,85p'Repository: baleen37/claude-plugins
Length of output: 1310
🏁 Script executed:
#!/bin/bash
# Search for test files that show typical tool_input and tool_response structures
find plugins/conversation-memory -name "*.test.ts" -o -name "*.spec.ts" | head -5Repository: baleen37/claude-plugins
Length of output: 332
🏁 Script executed:
#!/bin/bash
# Check the observe-cli test file to see test cases and typical tool structures
cat -n plugins/conversation-memory/src/cli/observe-cli.test.ts | head -150Repository: baleen37/claude-plugins
Length of output: 6146
🏁 Script executed:
#!/bin/bash
# Look for specific compression functions to understand expected data shape
rg "function compressRead|function compressEdit|function compressWrite|function compressBash" -A 10 plugins/conversation-memory/src/core/compress.tsRepository: baleen37/claude-plugins
Length of output: 1318
🏁 Script executed:
#!/bin/bash
# Check how handlePostToolUse uses mergedData
rg "handlePostToolUse" -A 15 plugins/conversation-memory/src/cli/observe-cli.tsRepository: baleen37/claude-plugins
Length of output: 850
Merge logic silently overwrites tool_input fields when tool_response contains overlapping keys.
When tool_response (object) contains keys that also exist in tool_input (e.g., both have exitCode or file_path), the spread on line 66 overwrites the input values. This causes compressToolData to read corrupted or response data instead of input arguments that define what was executed.
Example: A Bash tool with input { command: "ls" } and response { exitCode: 0, stderr: "..." } would merge correctly. But if the response accidentally includes { exitCode: 1, ... }, the merge would lose the true input exitCode, and compressBash would compress misleading data.
Consider namespacing to preserve both:
Proposed fix
- const mergedData = {
- ...((toolInput && typeof toolInput === 'object') ? toolInput : {}),
- ...(typeof toolResponse === 'object' && toolResponse !== null ? toolResponse : {}),
- // Include primitive responses as 'result' field
- ...(typeof toolResponse !== 'object' ? { result: toolResponse } : {}),
- };
+ const mergedData = {
+ ...((toolInput && typeof toolInput === 'object') ? toolInput : {}),
+ result: toolResponse,
+ };This preserves all input arguments (file_path, command, etc.) and places the complete response under a single result key. Compression functions already read only specific input fields and will work unchanged with the extra result key.
🤖 Prompt for AI Agents
In `@plugins/conversation-memory/src/cli/observe-cli.ts` around lines 64 - 69, The
merge currently spreads toolResponse into mergedData and can overwrite toolInput
fields (see mergedData, toolInput, toolResponse), so change the merge to
preserve all input fields and namespace any object response under a single
result key (e.g., mergedData = { ...(toolInputObject), result:
toolResponseObject } and for primitives keep result: toolResponse); update the
logic used by compressToolData/compressBash to continue reading input fields
unchanged while the full response is available under result.
…d LLM calls
Implements token bucket algorithm for rate limiting:
- Embedding generation: 5 req/sec, burst 10 (default)
- LLM API calls: 2 req/sec, burst 4 (default)
Features:
- Token bucket with configurable refill rate and burst capacity
- Non-blocking tryAcquire for immediate feedback
- Singleton instances for embedding and LLM rate limiting
- Configurable via ~/.config/conversation-memory/config.json
Configuration example:
{
"ratelimit": {
"embedding": { "requestsPerSecond": 5, "burstSize": 10 },
"llm": { "requestsPerSecond": 2, "burstSize": 4 }
}
}
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
## [5.21.1](v5.21.0...v5.21.1) (2026-02-12) ### Bug Fixes * **conversation-memory:** correct PostToolUse hook input parsing ([#487](#487)) ([32ca130](32ca130))
## [5.21.1](baleen37/bstack@v5.21.0...v5.21.1) (2026-02-12) ### Bug Fixes * **conversation-memory:** correct PostToolUse hook input parsing ([#487](baleen37/bstack#487)) ([32ca130](baleen37/bstack@32ca130))
1 participant
Summary
Fixes PostToolUse hook input parsing in conversation-memory plugin.
Root Cause: Claude Code hooks pass
tool_inputandtool_responsefields, but the code was expecting aresultfield.Changes
PostToolUseInputinterface to usetool_inputandtool_responseinstead ofresult. Merged both fields for compression to ensure all relevant data is captured.Test Plan
npm testin conversation-memory plugin)node dist/cli.mjs injectnow works without stdinnode dist/cli.mjs observecorrectly parses hook input🤖 Generated with Claude Code
Summary by CodeRabbit