CalvinBackup/gstack
1
0
Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-09-10 23:19:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(hooks): passThrough() two-branch contract — never emit permissionDecision:'defer' (#2035, #2006)

Browse Source 
Every AskUserQuestion died with "Tool result missing due to internal error"
on current Claude Code builds (Desktop 1.14271.0, CC 2.1.177). Root cause:
the question-preference-hook emitted permissionDecision:'defer' on every
pass-through path. 'defer' is a real PreToolUse value, but since CC v2.1.89
its semantics are "pause this tool call for external resumption" (headless
resume) — never "abstain". Interactive sessions have nothing to resume the
paused call, so the tool orphaned. Pre-2.1.89 builds ignored the unknown
value, which is why the hook worked when it shipped and broke later.

The fix is the two-branch pass-through contract:
- no context -> exit 0 with EXACTLY empty stdout
- memory nuggets present -> hookSpecificOutput with hookEventName +
  additionalContext ONLY (the documented shape; plan-tune Layer 8 memory
  injection ships through this branch and keeps working)

defer() is renamed passThrough() so the function says what it does, and
docs/spikes/claude-code-hook-mutation.md's protocol contract (cited by the
hook header) is corrected in the same commit — it taught '"defer" — let
permission flow continue' and was the reintroduction vector.

Test contract rewritten in the same commit (13 assertions across 3 files,
verified fail-first against the unfixed hook): pass-through paths assert
exact-empty stdout (a garbage/partial write cannot slip past an
optional-chained parse), the nugget path asserts permissionDecision is
ABSENT while additionalContext survives, and a new tripwire asserts no
non-deny path ever puts the string "permissionDecision" on stdout. The
deny (auto-decide) and Conductor prose-redirect paths are unchanged.

Deployment: no migration needed — settings.json points at the absolute
bash shim which execs the .ts live; /gstack-upgrade delivers the fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-07-09 19:08:49 -07:00
co-authored by Claude Fable 5
parent c4efc2a4f7
commit 3d97863b14
5 changed files with 137 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/memory-cache-injection.test.ts
+14 -9
View File
@@ -2,7 +2,7 @@
* Layer 8 memory cache + injection (plan-tune cathedral T12).
*
* Verifies the PreToolUse hook reads ~/.gstack/free-text-memory.json and
* surfaces matching nuggets via additionalContext on the hook response.
* surfaces matching nuggets via additionalContext-only output (#2035: never a permissionDecision).
* Cache: per-session memory-cache.json populated on first read, sub-1ms
* thereafter (D13 perf).
*/
@@ -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 the 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 on pass-through', () => {
writeMemory([
{
nugget: 'User prefers verbose explanations with tradeoffs',
@@ -91,7 +91,10 @@ describe('memory injection', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
// #2035: nugget delivery is additionalContext-ONLY — a permissionDecision
// here (any value) would orphan the tool call on CC >= 2.1.89.
expect('permissionDecision' in (r.parsed?.hookSpecificOutput ?? {})).toBe(false);
expect(r.parsed?.hookSpecificOutput?.hookEventName).toBe('PreToolUse');
expect(r.parsed?.hookSpecificOutput?.additionalContext).toContain('verbose explanations');
});
@@ -115,8 +118,9 @@ describe('memory injection', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined();
// No nugget → pure pass-through: exit 0 with EXACTLY empty stdout.
expect(r.status).toBe(0);
expect(r.stdout).toBe('');
});
test('caps to 3 most-recent nuggets when many match', () => {
@@ -219,7 +223,8 @@ describe('per-session memory cache', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined();
// No nugget → pure pass-through: exit 0 with EXACTLY empty stdout.
expect(r.status).toBe(0);
expect(r.stdout).toBe('');
});
});
test/question-preference-hook.test.ts
+70 -29
View File
@@ -3,15 +3,18 @@
*
* 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 → pass-through (D18 marker gate)
* - never-ask + one-way → pass-through (safety override)
* - never-ask + ambiguous recommendation → pass-through (D2 refuse-on-ambiguous)
* - always-ask → pass-through
* - no preference → pass-through
* - 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 → pass-through (crash safety)
*
* Pass-through contract (#2035/#2006): exit 0 + EXACTLY empty stdout, or
* additionalContext-only hookSpecificOutput — never a permissionDecision.
* - auto-decided event logged via gstack-question-log (PostToolUse won't fire)
* - auto-decided marker written to ~/.gstack/sessions/<id>/.auto-decided-<tool_use_id>
*/
@@ -97,6 +100,20 @@ function runHook(stdin: object, cwd?: string, extraEnv?: Record<string, string>)
};
}
/**
* #2035/#2006 contract: pass-through (abstain) is exit 0 with EXACTLY empty
* stdout — never a permissionDecision. 'defer' is a real PreToolUse value,
* but its semantics are pause-for-external-resumption (CC v2.1.89+), so
* emitting it orphans the tool call in interactive sessions. Exact-empty
* (not trim) is deliberate: whitespace on stdout is still hook output, and a
* garbage/partial write must fail this assertion rather than slip past an
* optional-chained parse.
*/
function expectPassThrough(r: { status: number; stdout: string }): void {
expect(r.status).toBe(0);
expect(r.stdout).toBe('');
}
function autoDecidedEvents(): Array<Record<string, unknown>> {
const f = path.join(stateRoot, 'projects', cwdSlug, 'question-log.jsonl');
if (!fs.existsSync(f)) return [];
@@ -113,8 +130,8 @@ function autoDecidedEvents(): Array<Record<string, unknown>> {
// Defer paths
// ----------------------------------------------------------------------
describe('defers (no enforcement)', () => {
test('no preference set → defer', () => {
describe('passes through (no enforcement)', () => {
test('no preference set → pass-through (empty stdout, no permissionDecision)', () => {
const r = runHook({
session_id: 's1',
tool_name: 'AskUserQuestion',
@@ -125,11 +142,10 @@ describe('defers (no enforcement)', () => {
],
},
});
expect(r.status).toBe(0);
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expectPassThrough(r);
});
test('marker missing → defer (D18)', () => {
test('marker missing → pass-through (D18)', () => {
writeProjectPref('test-q', 'never-ask');
const r = runHook({
session_id: 's2',
@@ -141,10 +157,10 @@ describe('defers (no enforcement)', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expectPassThrough(r);
});
test('always-ask preference → defer', () => {
test('always-ask preference → pass-through', () => {
writeProjectPref('test-q', 'always-ask');
const r = runHook({
session_id: 's3',
@@ -156,10 +172,10 @@ describe('defers (no enforcement)', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expectPassThrough(r);
});
test('empty stdin → defer (crash safety)', () => {
test('empty stdin → pass-through (crash safety)', () => {
const env: Record<string, string> = {};
for (const [k, v] of Object.entries(process.env)) {
if (v !== undefined) env[k] = v;
@@ -167,14 +183,39 @@ describe('defers (no enforcement)', () => {
env.GSTACK_STATE_ROOT = stateRoot;
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(res.stdout).toBe('');
});
test('non-AUQ tool_name → defer (defensive)', () => {
test('non-AUQ tool_name → pass-through (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');
expectPassThrough(r);
});
// #2035 tripwire: no non-deny/non-allow path may EVER put the string
// "permissionDecision" on stdout. Emitting one on a pass-through path (any
// value — 'defer' included) hands the platform a decision where the hook
// has none, and 'defer' specifically pauses the call for a resumption that
// never comes in interactive sessions.
test('pass-through stdout never contains "permissionDecision" (#2035)', () => {
const paths = [
runHook({
session_id: 's-trip-1',
tool_name: 'AskUserQuestion',
tool_use_id: 'tu-trip-1',
tool_input: {
questions: [
{ question: '<gstack-qid:test-q> Approve?', options: ['A) Yes (recommended)', 'B) No'] },
],
},
}),
runHook({ session_id: 's-trip-2', tool_name: 'Bash', tool_use_id: 'tu-trip-2', tool_input: {} }),
runHook({ session_id: 's-trip-3', tool_name: 'AskUserQuestion', tool_use_id: 'tu-trip-3', tool_input: { questions: [] } }),
];
for (const r of paths) {
expect(r.status).toBe(0);
expect(r.stdout).not.toContain('"permissionDecision"');
}
});
});
@@ -204,7 +245,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 → pass-through even with never-ask (safety override)', () => {
writeProjectPref('ship-test-failure-triage', 'never-ask');
const r = runHook({
session_id: 's6',
@@ -219,10 +260,10 @@ describe('enforces never-ask preferences', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expectPassThrough(r);
});
test('ambiguous recommendation (two labels) → defer (D2 refuse-on-ambiguous)', () => {
test('ambiguous recommendation (two labels) → pass-through (D2 refuse-on-ambiguous)', () => {
writeProjectPref('ship-pre-landing-review-fix', 'never-ask');
const r = runHook({
session_id: 's7',
@@ -237,10 +278,10 @@ describe('enforces never-ask preferences', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expectPassThrough(r);
});
test('no recommendation marker AND no prose match → defer', () => {
test('no recommendation marker AND no prose match → pass-through', () => {
writeProjectPref('ship-pre-landing-review-fix', 'never-ask');
const r = runHook({
session_id: 's8',
@@ -255,7 +296,7 @@ describe('enforces never-ask preferences', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expectPassThrough(r);
});
});
@@ -301,7 +342,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 → pass-through (project wins)', () => {
writeProjectPref('ship-pre-landing-review-fix', 'always-ask');
writeGlobalPref('ship-pre-landing-review-fix', 'never-ask');
const r = runHook({
@@ -317,7 +358,7 @@ describe('precedence: project wins over global (D8)', () => {
],
},
});
expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer');
expectPassThrough(r);
});
});
@@ -437,13 +478,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 pass-through (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');
expectPassThrough(r);
});
});
test/skill-e2e-plan-tune-cathedral.test.ts
+4 -1
View File
@@ -296,7 +296,10 @@ 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');
// #2035: memory-nugget delivery is additionalContext-ONLY. Emitting a
// permissionDecision here (the old 'defer') pauses the tool call for a
// resumption that never comes in interactive sessions.
expect('permissionDecision' in (parsed.hookSpecificOutput ?? {})).toBe(false);
expect(parsed.hookSpecificOutput?.additionalContext).toContain('verbose explanations');
});
});