fix(hooks): question-preference-hook must not emit permissionDecision 'defer' (merges #2165 + #1924)

[Akagilnc]

Picking up the handoff from #2165. @time-attack asked there on 2026-07-15:

Please rewrite this version to include the accurate documentation and strict output coverage from PR 1924. Its implementation is otherwise the best current foundation.

Neither PR has moved since, so this branch does that merge. Credit belongs to @dpnascimento (#2165) and @jacksondc (#1924); every commit here carries both as co-authors. If either of them would rather carry it forward, I am happy to close this in favour of theirs.

The bug

hosts/claude/hooks/question-preference-hook.ts emitted permissionDecision: "defer" on its no-opinion path. defer is a real Claude Code value meaning "pause this tool call and hand control back", and it is honored in print/non-interactive mode. So in any non-interactive session, every AskUserQuestion the hook did not auto-decide got parked and never executed. On my machine one such call was replayed several times across resumes, and no tool_result was ever written for it.

What changed

• defer() becomes passThrough(): silent exit 0, or hookSpecificOutput.additionalContext alone when plan-tune has context to inject. No permissionDecision on that path.
• The deny paths (never-ask auto-decide, Conductor prose redirect) are untouched.
• Spike doc corrections, going a little beyond Fix AskUserQuestion hook pass-through #1924: it restores the deny > ask > allow precedence fact that Fix AskUserQuestion hook pass-through #1924 deleted outright, moves "No output" out of the permissionDecision values: list where it is not a value, and rewrites the Auto-decide example from the never-implemented allow + updatedInput to the deny + reason shape the hook actually emits.
• Tests: expectNoDecision (from Fix AskUserQuestion hook pass-through #1924) now covers about ten pass-through cases, plus two cases Fix AskUserQuestion hook pass-through #1924 lacked. One is mcp__conductor__AskUserQuestion with no preference, which is the exact shape the bug reports fire on. The other asserts hookEventName === 'PreToolUse' on the additionalContext-only output.

Scope

Fixes the root cause behind #2006, #2035, #2292 and #2310.

#2207 is not fixed here. That one is the Conductor auto-install of the hook, owned by #2208. An early commit body on this branch listed #2207 among the root-cause issues; that wording was too broad, and this PR body is authoritative on claim surface.

Verification

bun test test/question-preference-hook.test.ts test/memory-cache-injection.test.ts
  28 pass / 0 fail / 67 expect()      (upstream/main: 27 pass / 52 expect)

bun run test
  2 failures: spec-template-sync, session-runner observability
  Both also fail on untouched upstream/main. Zero new failures.

bun run slop:diff
  no new findings in 5 changed files

Two things I could not verify

The cathedral e2e assertion never executed locally. test/skill-e2e-plan-tune-cathedral.test.ts is EVALS=1-gated. Forcing it with EVALS_ALL=1 EVALS=1 gives 0 pass / 5 fail, because the fixture scaffold does not copy lib/, so the hook dies with Cannot find module '../../../lib/is-conductor' at expect(res.status).toBe(0), before the pass-through assertion runs. That fixture gap predates this branch and is not fixed here. Gate coverage for this behaviour is the two unit test files above.

The Conductor deny reason carries a premise I could not confirm. In Conductor the hook still denies with a prose redirect whose reason cites the MCP variant being flaky. That premise was tied to the same defer path this PR removes, and I have not re-proven it under current Claude Code. The behaviour is deliberately unchanged, since it is out of scope, but that wording is now a historical claim this PR does not confirm.

I can rebase or reshape this however suits the review.

[trunk-io]

Merging to main in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

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

[Akagilnc]

Twelve days open, no review yet. Not trying to jump the queue — just putting the confirmations that have come in since in one place, since they all land on the same change.

Independent confirmations since 2026-07-22

• question-preference-hook: permissionDecision 'defer' hangs AskUserQuestion on Claude Code >= 2.1.21x #2320 (manhit96, Jul 22) — same bug; closed as a duplicate after applying the same change as this PR (silent exit 0 pass-through, additionalContext only). Verified end-to-end on Claude Code 2.1.217 / VSCode extension.
• question-preference-hook emits invalid permissionDecision "defer" → aborts every AskUserQuestion call #2310 (devty, Jul 26) — corrects the mechanism several of the reports state. defer is not rejected as invalid; Claude Code honors it and parks the tool. The hook exits 0 and is logged as a success, hook_deferred_tool follows, and no tool_result is ever produced — which is also why auq-error-fallback-hook never helps: it's a PostToolUse hook, and the tool never completes.
• question-preference-hook emits invalid permissionDecision "defer" → aborts every AskUserQuestion call #2310 (topcoder1, Jul 31) — blast radius: Claude Code desktop 2.1.217–2.1.219, gstack v1.60.1.0, macOS — 57/57 AskUserQuestion calls wedged over 8 days, none ever answered. Same fix, re-applied at SessionStart because upgrades rematerialize the file.

Current state

The same bug now has five open reports — #2006, #2035, #2292, #2310, #2340 — plus #2320 closed as a duplicate. Three open PRs: #1924, #2165, and this one. main is unchanged since a325940 (Jul 15); hosts/claude/hooks/question-preference-hook.ts:98 still emits permissionDecision: 'defer'.

Impact: the no-preference path runs on every unmarked question, so anyone with no plan-tune preferences loses AskUserQuestion entirely — silently, while the hook still reports success.

What this PR is

#1924 (@jacksondc) and #2165 (@dpnascimento) carried forward on current main, both authors credited as co-authors on the commits. The behavior change is small: pass-through becomes a silent exit 0, and permissionDecision is reserved for the documented allow / deny / ask values. Deny and auto-answer paths are unchanged. The rest of the diff is regression tests and a doc update.

Happy to rebase, split, or reshape it however you want. Leaving this here so the later confirmations are easy to find — and so I'm not just watching more duplicates arrive for something a few of us have already checked.

[garrytan]

Closing: this was absorbed or superseded on main by v1.67.0.0 or earlier (see the v1.67 tracker-wave receipts in PR #2604). Thank you!