From 0164fd2e1b136aa3401caa1760ce8f82f8e37791 Mon Sep 17 00:00:00 2001 From: Akagilnc Date: Wed, 22 Jul 2026 15:15:28 +0900 Subject: [PATCH 1/4] fix(hooks): stop emitting permissionDecision defer from question-preference-hook Silent pass-through (exit 0, optional additionalContext only) instead of explicit permissionDecision: 'defer'. Claude Code treats defer as a real pause/resume decision and honors it in print/non-interactive mode, which swallowed every AskUserQuestion (no tool_result, resume replay loops). Fixes the root cause behind #2006/#2035/#2207/#2292/#2310. Merges PR #2165 (print-mode why comment + foundation) with PR #1924 (passThrough rename, accurate docs, stricter empty-stdout assertions). deny paths (never-ask auto-decide, Conductor prose redirect) unchanged. Co-authored-by: dpnascimento Co-authored-by: jacksondc --- docs/spikes/claude-code-hook-mutation.md | 8 ++- .../claude/hooks/question-preference-hook.ts | 57 +++++++++------- test/memory-cache-injection.test.ts | 14 ++-- test/question-preference-hook.test.ts | 67 ++++++++++--------- test/skill-e2e-plan-tune-cathedral.test.ts | 4 +- 5 files changed, 86 insertions(+), 64 deletions(-) diff --git a/docs/spikes/claude-code-hook-mutation.md b/docs/spikes/claude-code-hook-mutation.md index 9d91e16cd4..8bef9128ad 100644 --- a/docs/spikes/claude-code-hook-mutation.md +++ b/docs/spikes/claude-code-hook-mutation.md @@ -51,7 +51,8 @@ Optional in subagent context: `agent_id`, `agent_type`. - `"deny"` — block (feedback to Claude, NOT a synthetic answer per Codex correction in D-prefixed decisions) - `"ask"` — escalate to user -- `"defer"` — let permission flow continue +- No output — let permission flow continue +- `"defer"` — pause a non-interactive SDK/tool caller so it can resume later **`updatedInput` semantics:** shallow merge of fields present in the returned object onto the original `tool_input`. Only valid with @@ -88,7 +89,7 @@ required for our hook to fire there. accepting. **`permissionDecision` precedence (when multiple hooks decide):** -`deny > ask > allow > defer` — most restrictive wins. +Claude Code treats `defer` as an explicit pause/resume decision, not as the normal pass-through path. Hooks that do not need to decide should exit 0 with no output. ## Implementation hookSpecificOutput examples @@ -107,11 +108,12 @@ required for our hook to fire there. ``` **Pass-through (no preference, or one-way safety override):** +Exit 0 with no stdout. If the hook has context to add, omit `permissionDecision`: ```json { "hookSpecificOutput": { "hookEventName": "PreToolUse", - "permissionDecision": "defer" + "additionalContext": "optional context for Claude" } } ``` diff --git a/hosts/claude/hooks/question-preference-hook.ts b/hosts/claude/hooks/question-preference-hook.ts index 12cbd5ea28..be059638a3 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 → no decision (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 → no decision. * * 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): no decision 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. @@ -92,13 +92,24 @@ function readStdin(): Promise { }); } -function defer(additionalContext?: string): void { - const out: Record = { - hookEventName: 'PreToolUse', - permissionDecision: 'defer', - }; - if (additionalContext) out.additionalContext = additionalContext; - process.stdout.write(JSON.stringify({ hookSpecificOutput: out })); +function passThrough(additionalContext?: string): void { + // "No opinion" must be a SILENT exit 0 (optionally with additionalContext + // only), never an explicit `permissionDecision: 'defer'`. `defer` is a real + // value in Claude Code, but it means "pause this tool call and hand control + // back" and is honored in print/non-interactive mode. Emitting it here made + // every AskUserQuestion get deferred in non-interactive sessions (e.g. the + // desktop app) — the question UI never rendered. Interactive mode merely + // warns and ignores it. + if (additionalContext) { + process.stdout.write( + JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'PreToolUse', + additionalContext, + }, + }), + ); + } process.exit(0); } @@ -347,7 +358,7 @@ function logAutoDecided( async function main(): Promise { const raw = await readStdin(); if (!raw.trim()) { - defer(); + passThrough(); return; } let stdin: HookStdin; @@ -355,7 +366,7 @@ async function main(): Promise { stdin = JSON.parse(raw); } catch (e) { logHookError(`stdin parse failed: ${(e as Error).message}`); - defer(); + passThrough(); return; } @@ -364,26 +375,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) { @@ -402,11 +413,11 @@ 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: - // 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 - // ineligible question makes the whole call not auto-decidable.) + // We deliberately do NOT early-return passThrough 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 ineligible question makes the whole call not auto-decidable.) const autoDecisions: Array<{ id: string; recommended: string }> = []; let fullyAutoDecidable = true; for (const q of questions) { @@ -471,10 +482,10 @@ async function main(): Promise { return; } - defer(memoryContext); + passThrough(memoryContext); } main().catch((e) => { logHookError(`main crash: ${(e as Error).message}`); - defer(); + passThrough(); }); diff --git a/test/memory-cache-injection.test.ts b/test/memory-cache-injection.test.ts index 3ab6a2144a..c2865178f4 100644 --- a/test/memory-cache-injection.test.ts +++ b/test/memory-cache-injection.test.ts @@ -43,9 +43,9 @@ function runHook(stdin: object): { stdout: string; stderr: string; status: numbe env.GSTACK_STATE_ROOT = stateRoot; env.GSTACK_QUESTION_LOG_NO_DERIVE = '1'; delete env.GSTACK_HOME; - // These cases assert the defer-path memoryContext injection. Strip ambient + // These cases assert pass-through memoryContext injection. Strip ambient // Conductor markers so running inside Conductor (CONDUCTOR_WORKSPACE_PATH/PORT - // set) doesn't flip the hook into the [conductor] prose deny instead of defer. + // set) doesn't flip the hook into the [conductor] prose deny instead of pass-through. delete env.CONDUCTOR_WORKSPACE_PATH; delete env.CONDUCTOR_PORT; const res = spawnSync(HOOK, [], { @@ -69,7 +69,7 @@ function runHook(stdin: object): { stdout: string; stderr: string; status: numbe // ---------------------------------------------------------------------- describe('memory injection', () => { - test('injects matching nugget into additionalContext on defer', () => { + test('injects matching nugget into additionalContext while passing through', () => { writeMemory([ { nugget: 'User prefers verbose explanations with tradeoffs', @@ -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,8 @@ describe('memory injection', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.stdout).toBe(''); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined(); }); @@ -219,7 +220,8 @@ describe('per-session memory cache', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(r.stdout).toBe(''); + 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..35a61bc3e4 100644 --- a/test/question-preference-hook.test.ts +++ b/test/question-preference-hook.test.ts @@ -3,15 +3,15 @@ * * Covers: * - never-ask + marker + two-way + clean recommendation → deny+reason - * - never-ask + no marker → defer (D18 marker gate) - * - never-ask + one-way → defer (safety override) - * - never-ask + ambiguous recommendation → defer (D2 refuse-on-ambiguous) - * - always-ask → defer - * - no preference → defer + * - never-ask + no marker → no decision (D18 marker gate) + * - never-ask + one-way → no decision (safety override) + * - never-ask + ambiguous recommendation → no decision (D2 refuse-on-ambiguous) + * - always-ask → no decision + * - no preference → no decision * - project preference wins over global (D8 precedence) * - global preference applies when no project preference set * - mcp__*__AskUserQuestion matcher accepted - * - empty stdin → defer (crash safety) + * - empty stdin → no decision (crash safety) * - auto-decided event logged via gstack-question-log (PostToolUse won't fire) * - auto-decided marker written to ~/.gstack/sessions//.auto-decided- */ @@ -74,7 +74,7 @@ function runHook(stdin: object, cwd?: string, extraEnv?: Record) delete env.GSTACK_HOME; // Strip ambient Conductor markers so these cases characterize NON-Conductor // behavior deterministically — otherwise running the suite inside Conductor - // (CONDUCTOR_WORKSPACE_PATH/PORT set) would flip every defer into the + // (CONDUCTOR_WORKSPACE_PATH/PORT set) would flip every pass-through into the // [conductor] prose deny. The Conductor cases below opt back in explicitly // via extraEnv. delete env.CONDUCTOR_WORKSPACE_PATH; @@ -109,12 +109,18 @@ function autoDecidedEvents(): Array> { .filter((e) => e.source === 'auto-decided'); } +/** Silent pass-through: no stdout and no permissionDecision. */ +function expectNoDecision(r: { stdout: string; parsed: any }): void { + expect(r.stdout).toBe(''); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); +} + // ---------------------------------------------------------------------- -// Defer paths +// Pass-through paths // ---------------------------------------------------------------------- -describe('defers (no enforcement)', () => { - test('no preference set → defer', () => { +describe('passes through (no enforcement)', () => { + test('no preference set → no decision', () => { const r = runHook({ session_id: 's1', tool_name: 'AskUserQuestion', @@ -126,10 +132,10 @@ describe('defers (no enforcement)', () => { }, }); expect(r.status).toBe(0); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectNoDecision(r); }); - test('marker missing → defer (D18)', () => { + test('marker missing → no decision (D18)', () => { writeProjectPref('test-q', 'never-ask'); const r = runHook({ session_id: 's2', @@ -141,10 +147,10 @@ describe('defers (no enforcement)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectNoDecision(r); }); - test('always-ask preference → defer', () => { + test('always-ask preference → no decision', () => { writeProjectPref('test-q', 'always-ask'); const r = runHook({ session_id: 's3', @@ -156,10 +162,10 @@ describe('defers (no enforcement)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectNoDecision(r); }); - test('empty stdin → defer (crash safety)', () => { + test('empty stdin → no decision (crash safety)', () => { const env: Record = {}; for (const [k, v] of Object.entries(process.env)) { if (v !== undefined) env[k] = v; @@ -167,14 +173,15 @@ describe('defers (no enforcement)', () => { env.GSTACK_STATE_ROOT = stateRoot; const res = spawnSync(HOOK, [], { env, input: '', encoding: 'utf-8' }); expect(res.status).toBe(0); + expect(res.stdout || '').toBe(''); const parsed = JSON.parse(res.stdout || '{}'); - expect(parsed.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(parsed.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); - test('non-AUQ tool_name → defer (defensive)', () => { + test('non-AUQ tool_name → no decision (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'); + expectNoDecision(r); }); }); @@ -204,7 +211,7 @@ describe('enforces never-ask preferences', () => { expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('Fix now'); }); - test('one-way door → defer even with never-ask (safety override)', () => { + test('one-way door → no decision even with never-ask (safety override)', () => { writeProjectPref('ship-test-failure-triage', 'never-ask'); const r = runHook({ session_id: 's6', @@ -219,10 +226,10 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectNoDecision(r); }); - test('ambiguous recommendation (two labels) → defer (D2 refuse-on-ambiguous)', () => { + test('ambiguous recommendation (two labels) → no decision (D2 refuse-on-ambiguous)', () => { writeProjectPref('ship-pre-landing-review-fix', 'never-ask'); const r = runHook({ session_id: 's7', @@ -237,10 +244,10 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectNoDecision(r); }); - test('no recommendation marker AND no prose match → defer', () => { + test('no recommendation marker AND no prose match → no decision', () => { writeProjectPref('ship-pre-landing-review-fix', 'never-ask'); const r = runHook({ session_id: 's8', @@ -255,7 +262,7 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectNoDecision(r); }); }); @@ -301,7 +308,7 @@ describe('precedence: project wins over global (D8)', () => { expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); }); - test('project always-ask + global never-ask → defer (project wins)', () => { + test('project always-ask + global never-ask → no decision (project wins)', () => { writeProjectPref('ship-pre-landing-review-fix', 'always-ask'); writeGlobalPref('ship-pre-landing-review-fix', 'never-ask'); const r = runHook({ @@ -317,7 +324,7 @@ describe('precedence: project wins over global (D8)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectNoDecision(r); }); }); @@ -384,7 +391,7 @@ describe('Conductor prose redirect', () => { expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('[conductor]'); }); - test('one-way door → deny with prose directive (NOT defer — destructive must reach human via prose)', () => { + test('one-way door → deny with prose directive (NOT pass-through — destructive must reach human via prose)', () => { const r = runHook({ session_id: 'c3', tool_name: 'AskUserQuestion', @@ -437,13 +444,13 @@ describe('Conductor prose redirect', () => { expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).not.toContain('[conductor]'); }); - test('non-AUQ tool in Conductor → still defer (no redirect on unrelated tools)', () => { + test('non-AUQ tool in Conductor → still no decision (no redirect on unrelated tools)', () => { const r = runHook( { session_id: 'c6', tool_name: 'Bash', tool_use_id: 'tu-c6', tool_input: {} }, undefined, CONDUCTOR, ); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectNoDecision(r); }); }); diff --git a/test/skill-e2e-plan-tune-cathedral.test.ts b/test/skill-e2e-plan-tune-cathedral.test.ts index f9c006914e..f273fbe505 100644 --- a/test/skill-e2e-plan-tune-cathedral.test.ts +++ b/test/skill-e2e-plan-tune-cathedral.test.ts @@ -263,7 +263,7 @@ describeIfSelected('PlanTune cathedral E2E: annotation', ['plan-tune-annotation' cleanupFixture(fixture.workDir); }); - testConcurrentIfSelected('PreToolUse hook surfaces memory nugget on defer', async () => { + testConcurrentIfSelected('PreToolUse hook surfaces memory nugget while passing through', async () => { const hookPath = path.join( fixture.workDir, 'hosts', @@ -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 926d539044e0550496ec47d4b084c9f67185df31 Mon Sep 17 00:00:00 2001 From: Akagilnc Date: Wed, 22 Jul 2026 15:15:28 +0900 Subject: [PATCH 2/4] fix(hooks): tighten AUQ pass-through coverage and restore spike docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 fixer pass against review findings on fix/auq-hook-neutral-passthrough (4c082806). Tests: - empty-stdin case uses expectNoDecision (strict empty stdout; drops dead JSON.parse assertions that were always true when stdout was empty) - add MCP-named AUQ + no preference → expectNoDecision (real failure shape for #2006/#2035/#2292/#2310) Docs (docs/spikes/claude-code-hook-mutation.md): - restore multi-hook precedence fact: deny > ask > allow (most restrictive wins), keep defer-is-not-pass-through clarification - move "no output" out of permissionDecision values list - mark revised date for the pass-through correction Scope note for PR body: this PR addresses silent pass-through for the hook itself (#2006/#2035/#2292/#2310). #2207 Conductor auto-install belongs to #2208 and is not claimed fixed here. Co-authored-by: dpnascimento Co-authored-by: jacksondc --- docs/spikes/claude-code-hook-mutation.md | 11 ++++++++-- test/question-preference-hook.test.ts | 27 +++++++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/docs/spikes/claude-code-hook-mutation.md b/docs/spikes/claude-code-hook-mutation.md index 8bef9128ad..38d5261f96 100644 --- a/docs/spikes/claude-code-hook-mutation.md +++ b/docs/spikes/claude-code-hook-mutation.md @@ -1,6 +1,8 @@ # Spike: Claude Code hook mutation for plan-tune cathedral **Status:** complete (2026-05-27) +**Revised:** 2026-07-22 — pass-through is empty stdout (not `defer`); restored +decision-precedence facts alongside defer semantics. **Surfaces:** D10 (does PreToolUse allow mutating AUQ input?), D19/Codex (matcher must cover MCP variants) **Downstream consumers:** T3, T5, T6, T8 @@ -51,9 +53,11 @@ Optional in subagent context: `agent_id`, `agent_type`. - `"deny"` — block (feedback to Claude, NOT a synthetic answer per Codex correction in D-prefixed decisions) - `"ask"` — escalate to user -- No output — let permission flow continue - `"defer"` — pause a non-interactive SDK/tool caller so it can resume later +No output (exit 0, empty stdout) is not a `permissionDecision` value. It is +the pass-through path that lets the permission flow continue unchanged. + **`updatedInput` semantics:** shallow merge of fields present in the returned object onto the original `tool_input`. Only valid with `permissionDecision: "allow"`. This is what lets us substitute an @@ -89,7 +93,10 @@ required for our hook to fire there. accepting. **`permissionDecision` precedence (when multiple hooks decide):** -Claude Code treats `defer` as an explicit pause/resume decision, not as the normal pass-through path. Hooks that do not need to decide should exit 0 with no output. +`deny > ask > allow` — most restrictive wins. +Claude Code treats `defer` as an explicit pause/resume decision, not as the +normal pass-through path. Hooks that do not need to decide should exit 0 with +no output (empty stdout), not emit `permissionDecision: "defer"`. ## Implementation hookSpecificOutput examples diff --git a/test/question-preference-hook.test.ts b/test/question-preference-hook.test.ts index 35a61bc3e4..3884b949ad 100644 --- a/test/question-preference-hook.test.ts +++ b/test/question-preference-hook.test.ts @@ -173,9 +173,10 @@ describe('passes through (no enforcement)', () => { env.GSTACK_STATE_ROOT = stateRoot; const res = spawnSync(HOOK, [], { env, input: '', encoding: 'utf-8' }); expect(res.status).toBe(0); - expect(res.stdout || '').toBe(''); - const parsed = JSON.parse(res.stdout || '{}'); - expect(parsed.hookSpecificOutput?.permissionDecision).toBeUndefined(); + // Same shape as runHook so expectNoDecision applies (strict empty stdout). + let parsed: any = null; + try { parsed = JSON.parse(res.stdout || '{}'); } catch {} + expectNoDecision({ stdout: res.stdout ?? '', parsed }); }); test('non-AUQ tool_name → no decision (defensive)', () => { @@ -350,6 +351,26 @@ describe('MCP variant', () => { }); expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); }); + + // Real failure shape for #2006/#2035/#2292/#2310: non-interactive MCP AUQ + // with no preference must be silent pass-through (not permissionDecision). + test('mcp__conductor__AskUserQuestion + no preference → no decision', () => { + const r = runHook({ + session_id: 's12-passthrough', + tool_name: 'mcp__conductor__AskUserQuestion', + tool_use_id: 'tu-12-passthrough', + tool_input: { + questions: [ + { + question: ' Need approval?', + options: ['A) Yes (recommended)', 'B) No'], + }, + ], + }, + }); + expect(r.status).toBe(0); + expectNoDecision(r); + }); }); // ---------------------------------------------------------------------- From eb6aa3136c076b8867d2cda7c469658551d13297 Mon Sep 17 00:00:00 2001 From: Akagilnc Date: Wed, 22 Jul 2026 15:15:28 +0900 Subject: [PATCH 3/4] fix(hooks): drop dead empty-stdin parse; align auto-decide docs Round-2 fixer pass on fix/auq-hook-neutral-passthrough. Tests: - empty-stdin crash-safety case asserts empty stdout directly (no JSON.parse of '' that could never fail after expectNoDecision) - memory additionalContext-only path also requires hookEventName PreToolUse (#2310 output shape) Docs (docs/spikes/claude-code-hook-mutation.md): - Auto-decide example matches live hook: deny + permissionDecisionReason (not allow + updatedInput); open-question item updated for deny path Co-authored-by: dpnascimento Co-authored-by: jacksondc --- docs/spikes/claude-code-hook-mutation.md | 23 ++++++++++++----------- test/memory-cache-injection.test.ts | 1 + test/question-preference-hook.test.ts | 6 ++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/spikes/claude-code-hook-mutation.md b/docs/spikes/claude-code-hook-mutation.md index 38d5261f96..898c0bacf2 100644 --- a/docs/spikes/claude-code-hook-mutation.md +++ b/docs/spikes/claude-code-hook-mutation.md @@ -101,15 +101,17 @@ no output (empty stdout), not emit `permissionDecision: "defer"`. ## Implementation hookSpecificOutput examples **Auto-decide (PreToolUse, `never-ask` preference + non-one-way):** + +The live hook uses `deny` + reason (not `allow` + `updatedInput`). See the +hook header: AskUserQuestion's pre-resolve `updatedInput` shape is not +structurally pinned, so deny naming the recommended option is the reliable +path; the model reads the reason and proceeds without re-firing AUQ. ```json { "hookSpecificOutput": { "hookEventName": "PreToolUse", - "permissionDecision": "allow", - "permissionDecisionReason": "plan-tune: never-ask preference on ship-test-failure-triage", - "updatedInput": { - "questions": [{ /* same as input, but with auto-selected answer */ }] - } + "permissionDecision": "deny", + "permissionDecisionReason": "[plan-tune auto-decide] ship-pre-landing-review-fix → A) Fix now (recommended) (your never-ask preference). Proceed with that option without re-prompting. Change with /plan-tune." } } ``` @@ -215,12 +217,11 @@ shells into bun. examples: ship/SKILL.md.tmpl emits options like `"A) Fix now" (recommended)`. -2. **Auto-decided event tagging.** When hook returns `updatedInput`, the - PostToolUse hook will see the resolved input and log a normal event. - Need an extra field on the PostToolUse payload (e.g., - `was_auto_decided: true`) that the hook can set via session state - tracking — write a marker file in `~/.gstack/sessions//.auto-decided-` - from PreToolUse, read it from PostToolUse, delete on read. +2. **Auto-decided event tagging.** Auto-decide is `deny` + reason, so + PostToolUse never fires on that tool call. The PreToolUse hook itself + writes a session marker and logs `source=auto-decided` events before + deny (see `markAutoDecided` / `logAutoDecided` in the hook). No + PostToolUse payload field is required for the current path. 3. **Timeout behavior.** Default hook timeout is 60s but the docs are thin on what happens at timeout. Set explicit `timeout: 5` so the diff --git a/test/memory-cache-injection.test.ts b/test/memory-cache-injection.test.ts index c2865178f4..b81a0cde7e 100644 --- a/test/memory-cache-injection.test.ts +++ b/test/memory-cache-injection.test.ts @@ -91,6 +91,7 @@ describe('memory injection', () => { ], }, }); + expect(r.parsed?.hookSpecificOutput?.hookEventName).toBe('PreToolUse'); expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBeUndefined(); expect(r.parsed?.hookSpecificOutput?.additionalContext).toContain('verbose explanations'); }); diff --git a/test/question-preference-hook.test.ts b/test/question-preference-hook.test.ts index 3884b949ad..892ea24626 100644 --- a/test/question-preference-hook.test.ts +++ b/test/question-preference-hook.test.ts @@ -173,10 +173,8 @@ describe('passes through (no enforcement)', () => { env.GSTACK_STATE_ROOT = stateRoot; const res = spawnSync(HOOK, [], { env, input: '', encoding: 'utf-8' }); expect(res.status).toBe(0); - // Same shape as runHook so expectNoDecision applies (strict empty stdout). - let parsed: any = null; - try { parsed = JSON.parse(res.stdout || '{}'); } catch {} - expectNoDecision({ stdout: res.stdout ?? '', parsed }); + // Crash-safety pass-through: empty stdout only (no permissionDecision). + expect(res.stdout ?? '').toBe(''); }); test('non-AUQ tool_name → no decision (defensive)', () => { From f5968c4e91d14de6fedae6d3b01d7745af4846d4 Mon Sep 17 00:00:00 2001 From: Akagilnc Date: Wed, 22 Jul 2026 15:15:28 +0900 Subject: [PATCH 4/4] fix(hooks): strict empty-stdout assert; qualify updatedInput in spike MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 fixer pass on fix/auq-hook-neutral-passthrough. M1 — empty-stdin crash-safety case now asserts res.stdout directly (no ?? '' / || '' mask). Matches expectNoDecision, #1924 original, and every other empty-stdout assertion in-repo. Sh1 — spike Answer + updatedInput semantics keep the platform fact but qualify that plan-tune lives on deny+reason (Implementation examples), so the headline no longer contradicts the live-hook section. Co-authored-by: dpnascimento Co-authored-by: jacksondc --- docs/spikes/claude-code-hook-mutation.md | 10 ++++++---- test/question-preference-hook.test.ts | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/spikes/claude-code-hook-mutation.md b/docs/spikes/claude-code-hook-mutation.md index 898c0bacf2..18bf9eb951 100644 --- a/docs/spikes/claude-code-hook-mutation.md +++ b/docs/spikes/claude-code-hook-mutation.md @@ -13,8 +13,9 @@ answer via `updatedInput`? If yes, what's the exact protocol? ## Answer -**Yes.** `updatedInput` is the supported mechanism. Source: -https://code.claude.com/docs/en/hooks (confirmed 2026-04 reference). +**Yes.** `updatedInput` is the supported mechanism (platform capability; +this plan-tune hook does not take that path — see Implementation examples). +Source: https://code.claude.com/docs/en/hooks (confirmed 2026-04 reference). ## Hook stdin schema (PreToolUse + PostToolUse) @@ -60,8 +61,9 @@ the pass-through path that lets the permission flow continue unchanged. **`updatedInput` semantics:** shallow merge of fields present in the returned object onto the original `tool_input`. Only valid with -`permissionDecision: "allow"`. This is what lets us substitute an -auto-decided answer for `never-ask` preferences. +`permissionDecision: "allow"`. This is what would let a hook substitute an +auto-decided answer for `never-ask` preferences; the live plan-tune hook +instead uses `deny` + reason (see Implementation examples). ## Matcher schema diff --git a/test/question-preference-hook.test.ts b/test/question-preference-hook.test.ts index 892ea24626..db3ada184a 100644 --- a/test/question-preference-hook.test.ts +++ b/test/question-preference-hook.test.ts @@ -174,7 +174,7 @@ describe('passes through (no enforcement)', () => { const res = spawnSync(HOOK, [], { env, input: '', encoding: 'utf-8' }); expect(res.status).toBe(0); // Crash-safety pass-through: empty stdout only (no permissionDecision). - expect(res.stdout ?? '').toBe(''); + expect(res.stdout).toBe(''); }); test('non-AUQ tool_name → no decision (defensive)', () => {