v1.60.2.0 fix(hooks): AskUserQuestion is cancelled in every non-interactive session - #2425
Open
jiqin822 wants to merge 4 commits into
Open
v1.60.2.0 fix(hooks): AskUserQuestion is cancelled in every non-interactive session#2425jiqin822 wants to merge 4 commits into
jiqin822 wants to merge 4 commits into
Conversation
…sessions
The question-preference hook returned `permissionDecision: 'defer'` on all six
of its no-opinion paths. In Claude Code `defer` is not a no-op: for a solo tool
call in a non-interactive (print-mode) session it emits `hook_deferred_tool` and
returns WITHOUT executing the tool. Since the hook's matcher is scoped to
`(AskUserQuestion|mcp__.*__AskUserQuestion)` and an AskUserQuestion call is
almost always solo, every question in every headless or spawned session died as
"Tool execution was interrupted" — no error, no log line, 100% reproducible.
Interactive sessions only warn-and-ignore the decision, which is why this was
invisible to anyone testing in a live terminal.
The correct way to express "no opinion" is to omit `permissionDecision`
entirely; Claude Code branches on its presence, so an absent field falls through
to the normal permission flow in both modes. `additionalContext` is delivered on
a separate event and still works without it. `deny` is untouched, so auto-decide
and the Conductor prose redirect keep working.
The 14 assertions that pinned `toBe('defer')` encoded the bug as the spec, which
is why a fully green `bun test` never caught it. They now assert the field is
absent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…Use hooks
Two static greps (comment-stripped so the prose explaining the ban does not trip
it) plus behavioral assertions over three stdin shapes:
1. no hook under hosts/claude/hooks/ may emit a live `permissionDecision: 'defer'`
2. no test may re-assert `toBe('defer')` — the original bug survived a green
suite precisely because 14 assertions across three files encoded it as the spec
The behavioral cases assert the key is ABSENT rather than merely undefined; a
serialized `"permissionDecision": null` would still reach Claude Code's validator.
Env is hermetic (temp GSTACK_STATE_ROOT, CONDUCTOR_* stripped) for the same
reason question-preference-hook.test.ts does it: ambient Conductor markers turn
every pass-through into the [conductor] prose deny, and a stored never-ask
preference would produce a deny, either of which fails the test for a reason
unrelated to the invariant.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on hooks
Mechanical rename, no behavior change. The helper that means "this hook has no
opinion, let the normal permission flow proceed" was named `defer()`, which
collides head-on with Claude Code's `permissionDecision: 'defer'` — a wire value
that means the opposite ("cancel this tool call and park it for resume").
That collision is what produced the bug fixed in 003ce5d: the obvious
implementation of a function named `defer()` is to emit `'defer'`. The static
tripwire blocks the literal string, but the name kept the footgun loaded for the
next contributor. Comments that used "defer" to mean "let the user answer" are
reworded to "pass through" for the same reason; the one comment that quotes the
wire value deliberately keeps it.
Applied to both hooks so the vocabulary is consistent. auq-error-fallback-hook
already emitted no decision and is unaffected behaviorally.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CLAUDE.md gains a "PreToolUse hooks: never emit permissionDecision: 'defer'" section with the per-mode behavior table (interactive warns and ignores; non-interactive + solo never executes the tool; non-interactive + batched warns and ignores), why the AskUserQuestion matcher makes the solo path the common case, and a pointer to the tripwire. PATCH bump per the repo's scale guide: bug fix plus one test file, no new user-facing capability. package.json is synced because gen-skill-docs pins it against VERSION. Doc sweep: AGENTS.md and ARCHITECTURE.md reviewed, neither needs a change. Neither documents hook permission contracts. Their only AskUserQuestion mentions are the /plan-tune skill one-liner and the preamble's question FORMAT, both untouched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
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.
The bug
Every
AskUserQuestioncall fails withTool execution was interruptedin any non-interactive session:claude -p, spawned skill agents, scheduled tasks, headless runs. No error, no log line, nothing pointing at a cause. 100% reproducible.The cause is our own PreToolUse hook.
hosts/claude/hooks/question-preference-hook.tsreturnedpermissionDecision: 'defer'on all six of its no-opinion paths, meaning "no opinion, let the tool run."In Claude Code,
defermeans the opposite: "cancel this tool call and park it for a later resume."Why it only bit headless sessions
From the
case "defer"branch in the Claude Code 2.1.219 runtime:hook_deferred_tool, returns without executing the toolOur matcher is scoped to
(AskUserQuestion|mcp__.*__AskUserQuestion), and an AskUserQuestion call is almost always solo in its batch. So the middle row was the common case for every headless user, and interactive developers never saw it.The fix
Omit
permissionDecisionentirely. Claude Code branches onif (hookSpecificOutput.permissionDecision), so an absent field falls through to the normal permission flow in both modes.additionalContextis delivered on a separate event and still works without a decision field.denyis untouched, so auto-decide and the Conductor prose redirect keep working.Note this is presence-sensitive, not value-sensitive: a serialized
"permissionDecision": nullwould still reach the validator. The tests assert the key is absent, not merelyundefined.Why a green test suite missed it
14 assertions across three test files asserted
toBe('defer'). The tests encoded the bug as the specification, so fixing the hook looked like breaking the tests. Those assertions now check that the field is absent.Commits
fix(hooks):the behavior fix, plus the 14 assertion updates.test(hooks):new tripwiretest/auq-no-defer-decision.test.ts. Two static greps (comment-stripped so the prose explaining the ban does not trip them) fail CI if any hook underhosts/claude/hooks/reintroduces a livepermissionDecision: 'defer', or if any test re-assertstoBe('defer'). Plus behavioral assertions over three stdin shapes.refactor(hooks):renamedefer()topassThrough()in both AskUserQuestion hooks. Mechanical, no behavior change. The old name is the root cause: the obvious implementation of a function calleddefer()is to emit'defer'. The static grep blocks the string, but the name kept the footgun loaded for the next contributor. Drop this commit if you would rather keep the name.docs:CLAUDE.md section, VERSION and package.json to v1.60.2.0, CHANGELOG entry.Test isolation note
The new tripwire builds a hermetic env (temp
GSTACK_STATE_ROOT,CONDUCTOR_*stripped) the same waytest/question-preference-hook.test.tsdoes. Without it, ambient Conductor markers turn every pass-through into the[conductor]prose deny, and a storednever-askpreference produces a deny. Either would fail the test for a reason unrelated to the invariant. Verified by running the suite withCONDUCTOR_WORKSPACE_PATHandCONDUCTOR_PORTinjected.Verification
bun test test/auq-no-defer-decision.test.ts: 5 pass. Also passes withCONDUCTOR_*injected.permissionDecision: 'defer'fails it, restoring the fix passes.auq-no-defer-decision,question-preference-hook,memory-cache-injection,auq-error-fallback-hook): 45 pass, 0 fail.ship-version-sync,gstack-version-bump,gstack-next-version,gen-skill-docs): 459 pass, 0 fail.bun teston this branch:EXIT=0, 8 failures.AskUserQuestionworks and returns the selection.Pre-existing failures, with receipts
Per the repo's E2E blame protocol, I did not assert "pre-existing" without proving it. A clean detached worktree at
origin/mainproduces the identical 8 failures, one for one:All are environment-dependent (missing
gbrainbinary, stripped auth env, live Codex CLI).diffbetween the branch failure list and theorigin/mainfailure list is empty.Doc sweep
All three agent doc files reviewed, root and nested. Only three exist, all at repo root.
CLAUDE.md: updated. New "PreToolUse hooks: never emitpermissionDecision: 'defer'" section with the per-mode table and a pointer to the tripwire.AGENTS.md: no change needed. Skill catalog, build commands, conventions. Documents no hook permission contract. Its onlyAskUserQuestionmention is the/plan-tuneskill one-liner, which describes question sensitivity tuning and is unaffected.ARCHITECTURE.md: no change needed. Covers the browse daemon, tunnel/token security model, refs, logging, SKILL template system, command dispatch. Documents no Claude Code hook behavior. ItsAskUserQuestionmentions are the preamble's question format, untouched here.Version
PATCH (
1.60.1.0to1.60.2.0) per the scale guide: bug fix plus one test file, no new user-facing capability.🤖 Generated with Claude Code