fix(cli): preserve caller context so responses use the agent's wire format - #197
Open
tupe12334 wants to merge 1 commit into
Open
fix(cli): preserve caller context so responses use the agent's wire format#197tupe12334 wants to merge 1 commit into
tupe12334 wants to merge 1 commit into
Conversation
…ormat
`run_app` read stdin into a buffer and called `polyhook::parse::parse_event`
directly. polyhook records the detected caller and event type in thread-locals
during `read_from`, and `polyhook::respond` reads them back to choose the wire
format — so parsing the bytes directly left that context at its default
(`caller = Unknown`, `event = None`) and every response was serialized in the
legacy Claude Code shape.
Two consequences: Claude Code `PreToolUse` blocks used the top-level
`decision: "block"` field, which terminates the entire agent session instead of
denying the single tool call, and non-Claude-Code agents (Cline, Windsurf, Amp,
Gemini CLI, ...) received a response they cannot parse, so the gate failed open.
Read through `polyhook::read_from` instead. The approve path is unchanged in
practice — polyhook 0.1.11 emits `{}` for an approve either way.
The existing `run_app` tests only asserted on the `HookResponse` enum variant
and never serialized, leaving the caller-format layer untested; add two tests
that assert on the serialized JSON for Claude Code and Cline.
Fixes #196
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #196.
Problem
run_appbuffered stdin and calledpolyhook::parse::parse_eventdirectly. polyhook records the detected caller and event type in thread-locals duringread_from, andpolyhook::respondreads them back to pick the wire format. Parsing the bytes directly skipped that, leavingcaller = Unknown, event = None, soserialize_response_with_eventalways fell through toserialize_claude_code(resp, None).Two consequences:
PreToolUseblocks terminated the whole session. The legacy top-leveldecision: "block"aborts the agent session, rather thanhookSpecificOutput.permissionDecision: "deny"which denies just that tool call. That also made steplock's own "Then retry your original command" instruction impossible to follow.Change
core/src/bin/main.rs— read viapolyhook::read_from(&mut reader)instead ofread_to_end+parse::parse_event. Theparseimport moves into the test module, where it is still used.Verification
Built from this branch and replayed the same three payloads from the issue:
PreToolUse{"decision":"block",...}{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"..."}}beforeToolUse{"decision":"block",...}{"approved":false,"reason":"..."}pre_tool{"decision":"block",...}{"allow":false,"reason":"..."}The approve path is unchanged in practice — polyhook 0.1.11 emits
{}for an approve with or without event context — so this PR changes only block responses.Full end-to-end gate behavior re-verified after the change and still correct: blocks on the current item, repeats the same item until acknowledged, advances only on
ack.shwith state persisted tostate.json, approves once the checklist reaches[*], resets per session, and branching flows present both options and follow whichever is acknowledged (including rejecting an ambiguous or invalidack.shtarget).Tests
The existing
run_apptests asserted only on theHookResponseenum variant and never serialized, which is why the entire caller-format layer was untested. Added two tests that assert on the serialized JSON:block_uses_claude_code_pre_tool_use_deny_not_session_blockblock_is_serialized_in_the_calling_agents_format(Cline)Both fail on
mainwith the exact defect visible in the failure output, and pass with the fix.Checks run locally
cargo test --all-features— 148 passed, 0 failedcargo clippy --all-targets --all-features— cleancargo fmt --check— cleanOpened automatically by the moadim routine "Nightly lib dogfood → issue + PR (polyhook, block-no-verify, steplock)".