Repository navigation
Conversation
Both clients answered a hook_callback by flattening HookOutput into a
map: hookSpecificOutput became top-level snake_case keys
(permission_decision, permission_decision_reason, updated_input) and
"continue" was always present, null when the hook left it unset.
The CLI validates the response against the hook JSON output format, the
same one a command hook prints: control fields at the top level and
hookSpecificOutput nested with camelCase keys. Against CLI 2.1.291 the
null "continue" fails that validation ("Invalid input: expected
boolean, received null"), the CLI logs "Error in hook callback" and
carries on as if the hook had not answered. A PreToolUse deny therefore
let the tool run, and updatedInput and additionalContext were dropped
the same way. Had "continue" been set, the snake_case keys would still
have been ignored as unknown.
HookOutput already carries the CLI's property names and NON_NULL
inclusion, so it is now sent as is, through one
HookRegistry.handleCallback used by both clients. The async client's
copy had also never forwarded updated_input. An unknown callback ID is
answered with an explicit error instead of a NullPointerException.
HookCallbackWireTest drives each client through a hook_callback round
trip against a stub CLI. HookDecisionIT (live) proves against the real
CLI that a deny stops a Bash command, that updatedInput replaces it,
and that UserPromptSubmit additionalContext reaches the model; all
three failed before this change.
DefaultClaudeAsyncClient.sendControlRequest wrote
{"type":"control","request_id":...,"subtype":...} with the payload
flattened beside the envelope. The CLI reads only
{"type":"control_request","request_id":...,"request":{...}}, the shape
the sync client sends, and drops anything else without a reply.
So the async client's initialize never registered its hooks: against
CLI 2.1.291 a PreToolUse hook on Bash was never called, and the command
ran. interrupt, set_permission_mode and set_model went out in the same
envelope and were dropped the same way.
The envelope now matches the sync client. The request stays
fire-and-forget, but it is registered in pendingResponses, so the CLI's
reply is matched instead of being logged as a response to an unknown
request, and an error reply is logged as a warning.
HookCallbackWireTest checks for both clients that hooks are registered
with a control_request initialize. HookDecisionIT (live) gains the
async deny case, which failed before this change because the hook was
never called. ClaudeAsyncClientIT still passes live.
The async client sent the user prompt right after initialize without waiting for the reply, so the CLI could start the turn before the hooks were registered, and a refused initialize was only logged. It now waits for the reply, as the sync client does, and a refusal fails the connect. The CLI rejects a hookSpecificOutput whose hookEventName is not the event it asked for and then ignores the whole output, deny included. A missing name is now filled in from the hook's registration; a name for another event is logged and sent as is.
Author
|
Pushed a follow-up commit: the async client now awaits the Two things I left out of scope, since they predate this PR — happy to follow up separately:
|
HookRegistry.handleCallback looks the hook up once and runs it through a shared execute(), and fills in a missing hookEventName with new withers on HookOutput and HookSpecificOutput instead of positional constructor copies. The async client registers a Sinks.One per control request instead of a cached Mono.create with a throwaway subscriber, logs a refusal where the reply arrives, and fails pending requests on close, so a connect waiting for initialize is released instead of waiting out the client timeout. The new tests share their setup through helpers, and the stub-CLI wire test checks initialize and the deny from one session per client.
…etModel The async client sent these requests without waiting for the reply, so a refusal was only logged and the client recorded the new mode or model anyway. They now complete once the CLI accepts the request and fail with a ClaudeSDKException when it refuses or does not reply in time, as the sync client does. Python SDK @23bb015: differs: the reply timeout is the client timeout, not Python's 60 s, because the sync client already uses it; awaiting the reply and failing on a refusal matches.
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
1 participant
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.
Summary
A PreToolUse deny returned through
HookOutput.hookSpecificOutputdid not stop the tool. Both clients flattened the hook output into snake_case keys and always sent"continue", null when the hook left it unset. CLI 2.1.291 rejects that payload (ZodError … "path": ["continue"], "message": "Invalid input: expected boolean, received null"), logsError in hook callback, and runs the tool anyway.updatedInputandadditionalContextwere dropped the same way.HookOutputalready carries the CLI's property names andNON_NULLinclusion, so it is now sent as is. Both clients go through one method,HookRegistry.handleCallback.The async client had a second bug that made its hooks dead before any output was sent.
sendControlRequestused an envelope the CLI does not read, soinitializenever registered the hooks.interrupt,set_permission_modeandset_modelwere dropped in the same way:Each fix is in its own commit.
Evidence
HookDecisionIT(@Tag("live"), CLI 2.1.291,bypassPermissions, so only the hook can stop the tool):touch <marker>is_errorwith the deny reasontouch <marker>preToolUseModifyrewrites the commandUserPromptSubmitadditionalContextwith a codewordNONEDeterministic tests, no CLI calls:
HookCallbackWireTest: a stub CLI drives both clients throughinitializeand ahook_callbackand records the client's stdin. Against the old clients it failed withbut was: {"continue":null,"permission_decision":"deny",…}.HookRegistryTest: checks the response payload for deny, allow, modify,additionalContext, the top-level fields, and an unknown callback ID../mvnw clean verifypasses.ClaudeAsyncClientITalso still passes live.Merge Danger
Door: two-way
Blast Radius: hooks, async control requests
interrupt(),setPermissionMode()andsetModel()now actually reach the CLI, and theirMonocompletes only once the CLI accepts the request. A refusal or a missing reply now fails it with aClaudeSDKException, as in the sync client, and leaves the current mode or model unchanged.NullPointerExceptioninside the client.com.fasterxml.jackson.databind, so they would need the same migration as the rest of the suite.