From 003ce5dd320cdbe431afaea30666900188174476 Mon Sep 17 00:00:00 2001 From: bjwang Date: Sat, 1 Aug 2026 03:24:07 -0700 Subject: [PATCH 1/4] fix(hooks): stop cancelling every AskUserQuestion in non-interactive sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../claude/hooks/question-preference-hook.ts | 9 ++++++++- test/memory-cache-injection.test.ts | 6 +++--- test/question-preference-hook.test.ts | 20 +++++++++---------- test/skill-e2e-plan-tune-cathedral.test.ts | 2 +- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/hosts/claude/hooks/question-preference-hook.ts b/hosts/claude/hooks/question-preference-hook.ts index 12cbd5ea28..b5ab8968c1 100644 --- a/hosts/claude/hooks/question-preference-hook.ts +++ b/hosts/claude/hooks/question-preference-hook.ts @@ -92,10 +92,17 @@ function readStdin(): Promise { }); } +// No-op pass-through: emit NO `permissionDecision` at all. +// +// Do NOT send `permissionDecision: 'defer'` here. Claude Code's `defer` is not a +// no-op: in a NON-INTERACTIVE (print-mode) session with a solo tool call it emits +// `hook_deferred_tool` and returns WITHOUT ever executing the tool, so every +// AskUserQuestion dies as "Tool execution was interrupted". In interactive mode it +// is merely warned-and-ignored, which is why this only bites headless/spawned runs. +// Omitting the field falls through to the normal permission flow in both modes. function defer(additionalContext?: string): void { const out: Record = { hookEventName: 'PreToolUse', - permissionDecision: 'defer', }; if (additionalContext) out.additionalContext = additionalContext; process.stdout.write(JSON.stringify({ hookSpecificOutput: out })); diff --git a/test/memory-cache-injection.test.ts b/test/memory-cache-injection.test.ts index 3ab6a2144a..4f8a3b68a7 100644 --- a/test/memory-cache-injection.test.ts +++ b/test/memory-cache-injection.test.ts @@ -91,7 +91,7 @@ describe('memory injection', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); expect(r.parsed?.hookSpecificOutput?.additionalContext).toContain('verbose explanations'); }); @@ -115,7 +115,7 @@ describe('memory injection', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined(); }); @@ -219,7 +219,7 @@ describe('per-session memory cache', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined(); }); }); diff --git a/test/question-preference-hook.test.ts b/test/question-preference-hook.test.ts index 39de02f4e8..c104f44940 100644 --- a/test/question-preference-hook.test.ts +++ b/test/question-preference-hook.test.ts @@ -126,7 +126,7 @@ describe('defers (no enforcement)', () => { }, }); expect(r.status).toBe(0); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); test('marker missing → defer (D18)', () => { @@ -141,7 +141,7 @@ describe('defers (no enforcement)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); test('always-ask preference → defer', () => { @@ -156,7 +156,7 @@ describe('defers (no enforcement)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); test('empty stdin → defer (crash safety)', () => { @@ -168,13 +168,13 @@ describe('defers (no enforcement)', () => { const res = spawnSync(HOOK, [], { env, input: '', encoding: 'utf-8' }); expect(res.status).toBe(0); const parsed = JSON.parse(res.stdout || '{}'); - expect(parsed.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(parsed.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); test('non-AUQ tool_name → defer (defensive)', () => { writeProjectPref('test-q', 'never-ask'); const r = runHook({ session_id: 's4', tool_name: 'Bash', tool_use_id: 'tu-4', tool_input: {} }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); }); @@ -219,7 +219,7 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); test('ambiguous recommendation (two labels) → defer (D2 refuse-on-ambiguous)', () => { @@ -237,7 +237,7 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); test('no recommendation marker AND no prose match → defer', () => { @@ -255,7 +255,7 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); }); @@ -317,7 +317,7 @@ describe('precedence: project wins over global (D8)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); }); @@ -443,7 +443,7 @@ describe('Conductor prose redirect', () => { undefined, CONDUCTOR, ); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); }); diff --git a/test/skill-e2e-plan-tune-cathedral.test.ts b/test/skill-e2e-plan-tune-cathedral.test.ts index f9c006914e..e07eb70fa1 100644 --- a/test/skill-e2e-plan-tune-cathedral.test.ts +++ b/test/skill-e2e-plan-tune-cathedral.test.ts @@ -296,7 +296,7 @@ describeIfSelected('PlanTune cathedral E2E: annotation', ['plan-tune-annotation' }); expect(res.status).toBe(0); const parsed = JSON.parse(res.stdout || '{}'); - expect(parsed.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(parsed.hookSpecificOutput?.permissionDecision).toBeUndefined(); expect(parsed.hookSpecificOutput?.additionalContext).toContain('verbose explanations'); }); }); From 28b12229cb805016d598cfb2ef2ecb9a2851ca76 Mon Sep 17 00:00:00 2001 From: bjwang Date: Sat, 1 Aug 2026 03:24:14 -0700 Subject: [PATCH 2/4] test(hooks): add tripwire banning permissionDecision=defer in PreToolUse hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- test/auq-no-defer-decision.test.ts | 157 +++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 test/auq-no-defer-decision.test.ts diff --git a/test/auq-no-defer-decision.test.ts b/test/auq-no-defer-decision.test.ts new file mode 100644 index 0000000000..b32d5abad1 --- /dev/null +++ b/test/auq-no-defer-decision.test.ts @@ -0,0 +1,157 @@ +/** + * Tripwire: no PreToolUse hook may emit `permissionDecision: 'defer'`. + * + * WHY THIS EXISTS + * --------------- + * `defer` reads like a harmless "I have no opinion, let the tool run" no-op. It is + * not. In Claude Code, a PreToolUse hook returning `defer`: + * + * - in an INTERACTIVE session → is warned about and ignored (tool runs). Harmless. + * - in a NON-INTERACTIVE session (print mode / headless / spawned agent), when the + * tool call is solo in its batch → emits `hook_deferred_tool` and returns WITHOUT + * EXECUTING THE TOOL. + * + * Because our matcher is scoped to `(AskUserQuestion|mcp__.*__AskUserQuestion)` and an + * AskUserQuestion call is almost always solo, the second path meant EVERY question in + * every headless/spawned session died as "Tool execution was interrupted" — with no + * error, no log line, and no hint that a hook was responsible. It reproduced 100% of + * the time and was invisible to `bun test`, because the unit tests asserted the broken + * value rather than the observable behavior. + * + * The correct way to express "no opinion" is to OMIT `permissionDecision` entirely. + * Claude Code branches on `if (hookSpecificOutput.permissionDecision)`, 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 a decision field. + * + * `deny` remains valid and is untouched — the auto-decide and Conductor prose paths + * still rely on it. + */ + +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { spawnSync } from 'child_process'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const HOOK_DIR = path.join(ROOT, 'hosts', 'claude', 'hooks'); +const TEST_DIR = path.join(ROOT, 'test'); +const HOOK = path.join(HOOK_DIR, 'question-preference-hook'); + +/** Strip line and block comments so prose explaining the ban doesn't trip the ban. */ +function stripComments(src: string): string { + return src.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, ''); +} + +describe('PreToolUse hooks never emit permissionDecision=defer', () => { + test('no hook source contains a live defer decision', () => { + const offenders: string[] = []; + for (const file of fs.readdirSync(HOOK_DIR)) { + if (!file.endsWith('.ts')) continue; + const code = stripComments( + fs.readFileSync(path.join(HOOK_DIR, file), 'utf-8'), + ); + if (/permissionDecision\s*:\s*['"`]defer['"`]/.test(code)) { + offenders.push(file); + } + } + expect(offenders).toEqual([]); + }); + + // The original bug survived a fully green suite because 14 assertions across + // three test files asserted `toBe('defer')`. The tests encoded the bug as the + // spec, so fixing the hook looked like breaking the tests. Ban the assertion. + test('no test re-encodes the defer decision as expected behavior', () => { + const offenders: string[] = []; + for (const file of fs.readdirSync(TEST_DIR)) { + if (!file.endsWith('.test.ts') || file === path.basename(import.meta.file)) continue; + const code = stripComments(fs.readFileSync(path.join(TEST_DIR, file), 'utf-8')); + if (/permissionDecision\s*\)?\s*\.toBe\(\s*['"`]defer['"`]\s*\)/.test(code)) { + offenders.push(file); + } + } + expect(offenders).toEqual([]); + }); +}); + +describe('question-preference-hook pass-through emits no decision', () => { + let stateRoot: string; + let fixtureCwd: string; + + beforeEach(() => { + stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-nodefer-')); + fixtureCwd = path.join(stateRoot, 'fixture-slug'); + fs.mkdirSync(path.join(stateRoot, 'projects', 'fixture-slug'), { recursive: true }); + fs.mkdirSync(fixtureCwd, { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(stateRoot, { recursive: true, force: true }); + }); + + function run(stdin: string): Record { + // Hermetic env, mirroring test/question-preference-hook.test.ts. Without + // this the hook reads the operator's real preference files (a stored + // never-ask could produce a deny) and, inside Conductor, the ambient + // CONDUCTOR_* markers flip every pass-through into the [conductor] prose + // deny. Either would fail this test for a reason unrelated to the invariant. + const env: Record = {}; + for (const [k, v] of Object.entries(process.env)) { + if (v !== undefined) env[k] = v; + } + env.GSTACK_STATE_ROOT = stateRoot; + delete env.GSTACK_HOME; + delete env.CONDUCTOR_WORKSPACE_PATH; + delete env.CONDUCTOR_PORT; + env.GSTACK_QUESTION_LOG_NO_DERIVE = '1'; + + // Empty stdin must stay byte-empty; the crash-safety path is what's under test. + const input = stdin + ? JSON.stringify({ ...JSON.parse(stdin), cwd: fixtureCwd }) + : stdin; + + const res = spawnSync(HOOK, [], { + env, + input, + encoding: 'utf-8', + cwd: ROOT, + timeout: 15000, + }); + expect(res.status).toBe(0); + return JSON.parse(res.stdout); + } + + const auq = JSON.stringify({ + hook_event_name: 'PreToolUse', + tool_name: 'AskUserQuestion', + tool_input: { + questions: [ + { + question: 'Which approach?', + header: 'Approach', + multiSelect: false, + options: [ + { label: 'A', description: 'first' }, + { label: 'B', description: 'second' }, + ], + }, + ], + }, + }); + + test.each([ + ['plain AskUserQuestion with no stored preference', auq], + ['empty stdin (crash safety)', ''], + [ + 'non-AUQ tool (defensive)', + JSON.stringify({ hook_event_name: 'PreToolUse', tool_name: 'Bash', tool_input: {} }), + ], + ])('%s → hookSpecificOutput carries no permissionDecision', (_label, stdin) => { + const out = run(stdin); + expect(out.hookSpecificOutput.hookEventName).toBe('PreToolUse'); + expect(out.hookSpecificOutput.permissionDecision).toBeUndefined(); + // The key must be ABSENT, not present-and-undefined — a serialized + // `"permissionDecision": null` would still hit Claude Code's validator. + expect('permissionDecision' in out.hookSpecificOutput).toBe(false); + }); +}); From d53787881355d073e48349571d5491f01c8fb8f9 Mon Sep 17 00:00:00 2001 From: bjwang Date: Sat, 1 Aug 2026 03:24:52 -0700 Subject: [PATCH 3/4] refactor(hooks): rename defer() to passThrough() in the AskUserQuestion hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 003ce5dd: 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 --- hosts/claude/hooks/auq-error-fallback-hook.ts | 12 ++++----- .../claude/hooks/question-preference-hook.ts | 26 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/hosts/claude/hooks/auq-error-fallback-hook.ts b/hosts/claude/hooks/auq-error-fallback-hook.ts index 502c9d4866..e920c15cb5 100755 --- a/hosts/claude/hooks/auq-error-fallback-hook.ts +++ b/hosts/claude/hooks/auq-error-fallback-hook.ts @@ -72,7 +72,7 @@ function readStdin(): Promise { } /** No-op output — let the tool result stand untouched. */ -function defer(): void { +function passThrough(): void { process.stdout.write( JSON.stringify({ hookSpecificOutput: { hookEventName: 'PostToolUse' } }), ); @@ -172,22 +172,22 @@ export function directiveFor(kind: 'spawned' | 'headless' | 'interactive'): stri async function main(): Promise { const raw = await readStdin(); - if (!raw.trim()) return defer(); + if (!raw.trim()) return passThrough(); let stdin: HookStdin; try { stdin = JSON.parse(raw); } catch (e) { logHookError(`stdin parse failed: ${(e as Error).message}`); - return defer(); + return passThrough(); } const toolName = stdin.tool_name || ''; if (toolName !== 'AskUserQuestion' && !/^mcp__.+__AskUserQuestion$/.test(toolName)) { - return defer(); + return passThrough(); } - if (!isErrorResponse(stdin.tool_response)) return defer(); + if (!isErrorResponse(stdin.tool_response)) return passThrough(); inject(directiveFor(sessionKind(stdin.cwd))); } @@ -197,6 +197,6 @@ async function main(): Promise { if (import.meta.main) { main().catch((e) => { logHookError(`main crash: ${(e as Error).message}`); - defer(); + passThrough(); }); } diff --git a/hosts/claude/hooks/question-preference-hook.ts b/hosts/claude/hooks/question-preference-hook.ts index b5ab8968c1..c1c1091d05 100644 --- a/hosts/claude/hooks/question-preference-hook.ts +++ b/hosts/claude/hooks/question-preference-hook.ts @@ -12,11 +12,11 @@ * 2. Look up door_type from scripts/question-registry.ts (default two-way). * 3. Read preferences with precedence: project-local > global (D8). * 4. Apply: - * never-ask + one-way → defer (safety override; one-way always asks). + * never-ask + one-way → pass through (safety override; one-way always asks). * never-ask + two-way + marker → deny with auto-decided recommendation * in reason. Mark tool_use_id so PostToolUse logs as 'auto-decided'. * ask-only-for-one-way + two-way + marker → same as never-ask. - * always-ask, or no preference → defer. + * always-ask, or no preference → pass through. * * Why deny+reason instead of allow+updatedInput: * AskUserQuestion's `updatedInput` shape for "pre-resolve this question" @@ -31,7 +31,7 @@ * - First: (recommended) label suffix on an option. * - Fall back: "Recommendation: X" prose match against option labels. * - Refuse to auto-decide if ambiguous (multiple labels OR no parseable - * recommendation): defer instead of silent-wrong. + * recommendation): pass through instead of silent-wrong. * * Always exits 0. Hook errors land in ~/.gstack/hook-errors.log. * See docs/spikes/claude-code-hook-mutation.md for the protocol contract. @@ -100,7 +100,7 @@ function readStdin(): Promise { // AskUserQuestion dies as "Tool execution was interrupted". In interactive mode it // is merely warned-and-ignored, which is why this only bites headless/spawned runs. // Omitting the field falls through to the normal permission flow in both modes. -function defer(additionalContext?: string): void { +function passThrough(additionalContext?: string): void { const out: Record = { hookEventName: 'PreToolUse', }; @@ -354,7 +354,7 @@ function logAutoDecided( async function main(): Promise { const raw = await readStdin(); if (!raw.trim()) { - defer(); + passThrough(); return; } let stdin: HookStdin; @@ -362,7 +362,7 @@ async function main(): Promise { stdin = JSON.parse(raw); } catch (e) { logHookError(`stdin parse failed: ${(e as Error).message}`); - defer(); + passThrough(); return; } @@ -371,26 +371,26 @@ async function main(): Promise { toolName !== 'AskUserQuestion' && !toolName.match(/^mcp__.+__AskUserQuestion$/) ) { - defer(); + passThrough(); return; } const questions = stdin.tool_input?.questions || []; if (questions.length === 0) { - defer(); + passThrough(); return; } // For multi-question AUQ, enforcement is all-or-nothing per call: // we deny only if ALL questions have marker + never-ask + safe door type. - // Mixed cases pass through (defer) so the user still gets to answer. + // Mixed cases pass through so the user still gets to answer. const registry = loadRegistry(); const slug = slugFromCwd(stdin.cwd); const memoryNuggets = loadMemoryNuggets(stdin.session_id); // Compute Layer 8 memory context inline: any nuggets matching the // signal_keys of the questions in this AUQ get surfaced as additionalContext. - // This applies whether we defer OR deny — gives the agent + user the + // This applies whether we pass through OR deny — gives the agent + user the // relevant prior context either way. const contextNuggets: string[] = []; for (const q of questions) { @@ -409,7 +409,7 @@ async function main(): Promise { : undefined; // Determine whether EVERY question is eligible for never-ask auto-decide. - // We deliberately do NOT early-return defer on the first ineligible question: + // We deliberately do NOT early-return pass-through on the first ineligible question: // a Conductor session still needs the [conductor] prose deny as a fallback, // so we compute eligibility, then branch. memoryContext is preserved on every // non-enforcing exit. (All-or-nothing per-call semantics are unchanged: any @@ -478,10 +478,10 @@ async function main(): Promise { return; } - defer(memoryContext); + passThrough(memoryContext); } main().catch((e) => { logHookError(`main crash: ${(e as Error).message}`); - defer(); + passThrough(); }); From 98d2ac03af77ce0756c1b60ba226631c74aa3b54 Mon Sep 17 00:00:00 2001 From: bjwang Date: Sat, 1 Aug 2026 03:26:43 -0700 Subject: [PATCH 4/4] docs: document the PreToolUse defer ban; bump to v1.60.2.0 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 --- CHANGELOG.md | 38 ++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 37 +++++++++++++++++++++++++++++++++++++ VERSION | 2 +- package.json | 2 +- 4 files changed, 77 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 351e7cabde..6074249033 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,43 @@ # Changelog +## [1.60.2.0] - 2026-08-01 + +## **Questions work again in headless and spawned sessions.** +## **One hook field was cancelling every AskUserQuestion call.** + +If you ran gstack in a non-interactive session, a spawned agent, a `claude -p` run, a scheduled task, anything that is not a live terminal, every AskUserQuestion died with `Tool execution was interrupted`. No error, no log line, nothing pointing at a cause. The culprit was our own PreToolUse hook. `question-preference-hook` returned `permissionDecision: 'defer'` to mean "no opinion, let the tool run". In Claude Code, `defer` means "cancel this tool call and park it for a later resume". Interactive sessions warn and ignore it, which is why terminal users never saw this and headless users saw nothing else. + +### The numbers that matter + +Source: the `case "defer"` branch in the Claude Code 2.1.219 runtime, plus the new regression test (reproducible: `bun test test/auq-no-defer-decision.test.ts`). + +| Metric | Before | After | Δ | +|--------|--------|-------|---| +| AskUserQuestion success, non-interactive session | 0% | 100% | restored | +| AskUserQuestion success, interactive session | 100% | 100% | unchanged | +| Hook call sites returning the cancelling value | 6 | 0 | -6 | +| Tripwire tests guarding the invariant | 0 | 5 | +5 | + +The failure rate was 0%, not "flaky". Every question, every run, as long as the session was not a live terminal. If a spawned agent ever went quiet on you partway through a task and you assumed it had crashed, this is what happened to it. + +### What this means for you + +Any workflow that asks you something now works headless: spawned skill agents, scheduled runs, `claude -p` pipelines, and Conductor sessions. If you had worked around this by avoiding questions in automation, you can stop. Upgrade with `/gstack-upgrade`, then re-run whatever you gave up on. + +### Itemized changes + +#### Fixed + +- `hosts/claude/hooks/question-preference-hook.ts`: the no-opinion helper now omits `permissionDecision` entirely instead of sending `'defer'`. That covers all 6 call sites: empty stdin, unparseable stdin, non-AUQ tool name, zero questions, the main pass-through (where no stored preference, always-ask, missing recommendation marker, one-way door, and ambiguous recommendation all land), and the `main()` crash handler. `additionalContext` still rides along, since it is delivered on a separate event. The `deny` path used by auto-decide and the Conductor prose fallback is untouched. + +#### For contributors + +- `test/auq-no-defer-decision.test.ts`: new tripwire. Two static greps (comment-stripped, so prose explaining the ban does not trip them) fail CI if any hook under `hosts/claude/hooks/` reintroduces a live `permissionDecision: 'defer'`, or if any test re-asserts `toBe('defer')`. Behavioral assertions check the key is absent rather than present-and-undefined, since a serialized `null` would still reach the validator. +- 14 assertions changed from `toBe('defer')` to `toBeUndefined()` across `test/question-preference-hook.test.ts` (10), `test/memory-cache-injection.test.ts` (3), and `test/skill-e2e-plan-tune-cathedral.test.ts` (1). These previously encoded the broken behavior as expected behavior. +- `package.json` version synced to `1.60.2.0` (`gen-skill-docs` pins it against `VERSION`). +- `defer()` is renamed to `passThrough()` in `question-preference-hook.ts` and `auq-error-fallback-hook.ts`, with the comments that used "defer" to mean "let the user answer" reworded to match. The old name is what made emitting the cancelling wire value look like the obvious implementation. No behavior change in either hook. +- `CLAUDE.md`: new "PreToolUse hooks: never emit `permissionDecision: 'defer'`" section with the per-mode behavior table and a pointer to the tripwire. + ## [1.60.1.0] - 2026-07-09 ## **The /autoplan dual-voice eval is back on the board, catching real regressions.** diff --git a/CLAUDE.md b/CLAUDE.md index 9848449020..5df9d43ee6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -196,6 +196,43 @@ structures. Instead: This applies to test commands, eval commands, deploy commands, and any other project-specific behavior. The project owns its config; gstack reads it. +## PreToolUse hooks: never emit `permissionDecision: 'defer'` + +A PreToolUse hook that has no opinion MUST omit `permissionDecision` entirely. +Emitting `'defer'` is not a no-op. In Claude Code it means "cancel this tool call +and park it for a later resume": + +| Session | Solo tool call | Result | +|---------|----------------|--------| +| interactive | any | warned and ignored, tool runs | +| non-interactive (print/headless/spawned) | yes | `hook_deferred_tool`, tool **never executes** | +| non-interactive | no (batched) | warned and ignored, tool runs | + +Because our matcher is scoped to `(AskUserQuestion|mcp__.*__AskUserQuestion)` and an +AskUserQuestion call is almost always solo, a `defer` return meant every question in +every headless or spawned session died as `Tool execution was interrupted`, with no +error and no log line pointing at the hook. It reproduced 100% of the time and was +invisible to `bun test`, because the unit tests asserted the broken value instead of +the observable behavior. + +Omitting the field falls through to the normal permission flow in both modes, and +`additionalContext` still arrives (it is delivered on a separate event). `deny` is +unaffected and remains the mechanism for auto-decide and the Conductor prose path. + +The helper for this in both AskUserQuestion hooks is `passThrough()`. It is +deliberately NOT called `defer()` — that name is what made emitting `'defer'` +look like the obvious implementation in the first place. + +`test/auq-no-defer-decision.test.ts` pins the invariant with two static-grep +tripwires (comment-stripped, so prose explaining the ban does not trip them) plus +behavioral assertions that the key is absent, not merely undefined: + +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 fully green + `bun test` because 14 assertions across three test files encoded it as the spec + +Reintroducing either fails CI. + ## Writing SKILL templates SKILL.md.tmpl files are **prompt templates read by Claude**, not bash scripts. diff --git a/VERSION b/VERSION index c4190e0048..762f175545 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.60.1.0 +1.60.2.0 diff --git a/package.json b/package.json index 846438a603..78b98a86f1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.60.1.0", + "version": "1.60.2.0", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module",