Repository navigation
fix: read tool_response instead of tool_output in PostToolUse hook - #561
Conversation
Claude Code's PostToolUse payload sends the field as `tool_response`, not `tool_output`. The hook was reading `data.tool_output` which is always undefined, so `cleanOutput` was undefined, the observe request contained no `tool_output` value, and mem::compress consistently failed its XML schema validation (requires narrative >= 10 chars + facts >= 1). Fix: read `data.tool_response` with `data.tool_output` as a fallback so older integrations that emit the legacy field name keep working. Fixes rohitg00#539
|
@faraz152 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughBoth the TypeScript hook and JavaScript script now use ChangesTool Response Compatibility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/post-tool-use.ts (1)
35-36: ⚡ Quick winRemove the explanatory WHAT comment and rely on clear code instead.
The fallback expression is already self-explanatory; please drop the inline explanatory comment to align with repository style rules.
As per coding guidelines, "Avoid code comments explaining WHAT — use clear naming instead".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/post-tool-use.ts` around lines 35 - 36, Remove the explanatory inline comment and rely on the clear fallback expression itself: delete the comment line that describes "Claude Code's PostToolUse payload uses `tool_response`; fall back to `tool_output`…" and keep the existing fallback expression that references `tool_response` and `tool_output` unchanged so the code reads cleanly without a WHAT-style comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/hooks/post-tool-use.ts`:
- Around line 35-36: Remove the explanatory inline comment and rely on the clear
fallback expression itself: delete the comment line that describes "Claude
Code's PostToolUse payload uses `tool_response`; fall back to `tool_output`…"
and keep the existing fallback expression that references `tool_response` and
`tool_output` unchanged so the code reads cleanly without a WHAT-style comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 035c737b-679f-494d-a29e-af86185049f8
📒 Files selected for processing (2)
plugin/scripts/post-tool-use.mjssrc/hooks/post-tool-use.ts
|
Thanks @faraz152 — clean root-cause writeup with a captured payload, exactly the kind of repro that makes review easy. Merged. |
…uard (#588) Three concerns surfaced when auditing PRs merged since v0.9.21: 1) Graph parser regex from #494 was correct for self-closing tags ONLY when they're the only entity in the document. With a self-closing entity followed by an explicit-close entity, the greedy `[^>]*` ate the trailing `/` of the self-close, the alternation fell through to the explicit-close branch, and `[\s\S]*?` ran ahead to the *next* `</entity>` — merging two entity declarations into one match and silently dropping a node. Switch to lazy `[^>]*?` so the self-closing alternation gets first chance. Test from #494 (which was failing on main but I didn't notice) now passes. 2) #386 shipped `${AGENTMEMORY_URL}` / `${AGENTMEMORY_SECRET}` placeholders in plugin/.mcp.json. Claude Code and Cursor expand those at MCP launch time; some hosts pass the literal string through. A truthy literal `"${AGENTMEMORY_URL}"` defeats the existing `|| DEFAULT_URL` fallback and would have the shim POST to `${AGENTMEMORY_URL}/agentmemory/...` (DNS failure). Add a defensive guard in src/mcp/rest-proxy.ts that strips any value of the form `${...}` so the fallback engages. Mirror in src/mcp/standalone.ts's mode-announce log line. 3) CHANGELOG entries for all PRs landed since v0.9.21 (#321, #325, #386, #454, #494, #526, #542, #560, #561, #562, #564, #567) plus the regex + env-guard hardening here. Validation: - npm test → 98 files, 1091 tests pass - npm pack → 142 files, fresh install clean, iii-sdk@0.11.2 pinned, plugin/ shipped with hooks/scripts/skills/opencode/ - New test file covers 8 placeholder cases (unset, empty, `${VAR}` literal, mismatched braces, real values with $, unclosed `${`, no-braces `$VAR`).
…ohitg00#561) * fix: read tool_response instead of tool_output in PostToolUse hook Claude Code's PostToolUse payload sends the field as `tool_response`, not `tool_output`. The hook was reading `data.tool_output` which is always undefined, so `cleanOutput` was undefined, the observe request contained no `tool_output` value, and mem::compress consistently failed its XML schema validation (requires narrative >= 10 chars + facts >= 1). Fix: read `data.tool_response` with `data.tool_output` as a fallback so older integrations that emit the legacy field name keep working. Fixes rohitg00#539 * style: remove explanatory comment per repo guidelines
…uard (rohitg00#588) Three concerns surfaced when auditing PRs merged since v0.9.21: 1) Graph parser regex from rohitg00#494 was correct for self-closing tags ONLY when they're the only entity in the document. With a self-closing entity followed by an explicit-close entity, the greedy `[^>]*` ate the trailing `/` of the self-close, the alternation fell through to the explicit-close branch, and `[\s\S]*?` ran ahead to the *next* `</entity>` — merging two entity declarations into one match and silently dropping a node. Switch to lazy `[^>]*?` so the self-closing alternation gets first chance. Test from rohitg00#494 (which was failing on main but I didn't notice) now passes. 2) rohitg00#386 shipped `${AGENTMEMORY_URL}` / `${AGENTMEMORY_SECRET}` placeholders in plugin/.mcp.json. Claude Code and Cursor expand those at MCP launch time; some hosts pass the literal string through. A truthy literal `"${AGENTMEMORY_URL}"` defeats the existing `|| DEFAULT_URL` fallback and would have the shim POST to `${AGENTMEMORY_URL}/agentmemory/...` (DNS failure). Add a defensive guard in src/mcp/rest-proxy.ts that strips any value of the form `${...}` so the fallback engages. Mirror in src/mcp/standalone.ts's mode-announce log line. 3) CHANGELOG entries for all PRs landed since v0.9.21 (rohitg00#321, rohitg00#325, rohitg00#386, rohitg00#454, rohitg00#494, rohitg00#526, rohitg00#542, rohitg00#560, rohitg00#561, rohitg00#562, rohitg00#564, rohitg00#567) plus the regex + env-guard hardening here. Validation: - npm test → 98 files, 1091 tests pass - npm pack → 142 files, fresh install clean, iii-sdk@0.11.2 pinned, plugin/ shipped with hooks/scripts/skills/opencode/ - New test file covers 8 placeholder cases (unset, empty, `${VAR}` literal, mismatched braces, real values with $, unclosed `${`, no-braces `$VAR`).
Root cause
src/hooks/post-tool-use.ts(and its compiled counterpartplugin/scripts/post-tool-use.mjs) read the tool output fromdata.tool_output:But Claude Code's PostToolUse hook payload uses
tool_response, nottool_output. This meanscleanOutputis alwaysundefined, the observe request reaches the server with no output value, andmem::compressfails schema validation on every real tool call —narrativeandfactsare empty so the XML schema check fails.Captured real PostToolUse payload from Claude Code (from #539):
{ "hook_event_name": "PostToolUse", "tool_name": "Bash", "tool_input": { "command": "echo hi" }, "tool_response": { "stdout": "hi\n", "stderr": "", "interrupted": false } }Fix
Read
data.tool_responsewithdata.tool_outputas a fallback so any older integration still using the legacy field name keeps working:Same one-liner applied to the compiled
plugin/scripts/post-tool-use.mjs.Impact
Without this fix
mem::compresssilently fails on ~100% of real Claude Code tool calls, degradingmemory_recalloutput and starving the consolidation pipeline of compressed observations.Fixes #539
Summary by CodeRabbit