From 487506bd38f73cfc33454b8bcb1ad25b74994220 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 6 Nov 2026 17:54:08 -0800 Subject: [PATCH] fix: the skill contract Stop hook waits for Claude Code's transcript flush (GST-1703) (#4104) * test: failing-first cases for the skill contract Stop hook reading a transcript before Claude Code flushes it (GST-1703) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: the skill contract Stop hook waits for the transcript flush through the shared settle loop (GST-1703) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .../capy-gst-1703-contract-hook-settle.md | 1 + docs/skill-routing-and-contracts.md | 2 +- .../claude/hooks/skill-contract-stop-hook.ts | 11 +++++--- lib/skill-gates.ts | 27 +++++++++++-------- test/skill-contract-stop-hook.test.ts | 27 +++++++++++++++++++ 5 files changed, 52 insertions(+), 16 deletions(-) create mode 100644 changelog.d/capy-gst-1703-contract-hook-settle.md diff --git a/changelog.d/capy-gst-1703-contract-hook-settle.md b/changelog.d/capy-gst-1703-contract-hook-settle.md new file mode 100644 index 000000000..88f733f7f --- /dev/null +++ b/changelog.d/capy-gst-1703-contract-hook-settle.md @@ -0,0 +1 @@ +**The skill contract Stop hook no longer judges a transcript Claude Code hasn't finished writing (GST-1703).** Claude Code can fire Stop about 200 ms before it flushes the turn's last transcript lines. The contract hook read the transcript once, so a finished /investigate run could be blocked for a missing repro because its final test rerun wasn't on disk yet, and a run whose skill load hadn't landed could stop unchecked. The hook now re-reads the transcript, up to 2 seconds, until it holds the stop's final message. It uses the same loop as the /autoplan and /plan-ceo-review gates (`readSettledTranscript` in `lib/skill-gates.ts`). diff --git a/docs/skill-routing-and-contracts.md b/docs/skill-routing-and-contracts.md index f8457da3b..e6ddc4c6c 100644 --- a/docs/skill-routing-and-contracts.md +++ b/docs/skill-routing-and-contracts.md @@ -109,7 +109,7 @@ The hook checks the most recently loaded skill that declares a contract, and onl - the session is headless or unattended (`gstack-session-kind`), or - the turn does not end on a question to the user (a last paragraph ending in "?", an AskUserQuestion call, or `NEEDS_CONTEXT`). -So an interactive session that pauses to ask a question is never blocked. When Claude Code says a Stop hook already continued this turn (`stop_hook_active`), the hook never blocks: after its own block it records the result (`met_after_block` or `miss`). On a final turn it also allows the stop while Agent dispatches are still running (no result yet, or launched in the background with no completion notice naming that agent), because their outputs may still arrive, and records the run as `deferred`. Once the same agents have been running for 10 minutes since the stop that first saw them, they count as stalled and the contract is checked at the next stop. Because Claude Code can fire Stop before the turn's last message reaches the transcript file, the hook reads the payload's `last_assistant_message` as the turn's final text. +So an interactive session that pauses to ask a question is never blocked. When Claude Code says a Stop hook already continued this turn (`stop_hook_active`), the hook never blocks: after its own block it records the result (`met_after_block` or `miss`). On a final turn it also allows the stop while Agent dispatches are still running (no result yet, or launched in the background with no completion notice naming that agent), because their outputs may still arrive, and records the run as `deferred`. Once the same agents have been running for 10 minutes since the stop that first saw them, they count as stalled and the contract is checked at the next stop. Because Claude Code can fire Stop before the turn's last lines reach the transcript file (about 200 ms late in a smoke run), the hook re-reads the file until it holds the payload's `last_assistant_message`, for at most 2 seconds (`readSettledTranscript` in `lib/skill-gates.ts`, the loop the skill gates share). If the message never shows up, it counts that message as the turn's final text. A single read could miss the test rerun after the last edit and block a finished run, or miss the skill load and let an unfinished one through. It blocks at most once per skill run. If the model stops again with the contract still unmet, the stop is allowed and the run is recorded as a miss. diff --git a/hosts/claude/hooks/skill-contract-stop-hook.ts b/hosts/claude/hooks/skill-contract-stop-hook.ts index a8b680a1a..709a4f8e0 100755 --- a/hosts/claude/hooks/skill-contract-stop-hook.ts +++ b/hosts/claude/hooks/skill-contract-stop-hook.ts @@ -5,7 +5,9 @@ * skill_contracts is on (env GSTACK_SKILL_CONTRACTS, then config * skill_contracts, then SWITCH_DEFAULT). * - * stdin -> switch on? -> transcript_path -> parse (+ last_assistant_message) + * stdin -> switch on? -> transcript_path -> parse (+ last_assistant_message), + * re-read until it holds that final message (lib/skill-gates.ts + * readSettledTranscript; Stop can beat the CLI's flush) * -> the most recently loaded skill that declares a contract (none: allow) * -> final turn? (a STATUS line, a headless or unattended session, or no * pending user question; otherwise allow) @@ -31,10 +33,11 @@ import { binPath, sessionKind } from './spawn-bin'; import { appendHookTrace, installedSkillsDir, readSkillSwitch, readStdinBounded } from '../../../lib/skill-route'; import { - AGENT_STALL_MS, StateError, UnknownShape, blockMessage, bumpCanary, checkContract, isFinalTurn, lookupContract, nextOutcome, parseTranscript, - readRecord, readTranscriptFile, recordPath, runKey, sameSkill, skillLoads, stateErrorLine, withFinalMessage, writeRecord, + AGENT_STALL_MS, StateError, UnknownShape, blockMessage, bumpCanary, checkContract, isFinalTurn, lookupContract, nextOutcome, + readRecord, recordPath, runKey, sameSkill, skillLoads, stateErrorLine, writeRecord, type Contract, type Transcript, } from '../../../lib/skill-contract'; +import { readSettledTranscript } from '../../../lib/skill-gates'; export const DEADLINE_MS = 5000; const STDIN_CAP_BYTES = 1024 * 1024; @@ -89,7 +92,7 @@ async function main(): Promise { if (!transcriptPath || !sessionId) return unknownShape('', 'payload has no transcript_path or session_id'); let transcript: Transcript; try { - transcript = withFinalMessage(parseTranscript(readTranscriptFile(transcriptPath)), payload.last_assistant_message); + transcript = readSettledTranscript(transcriptPath, payload.last_assistant_message).t; } catch (e) { if (e instanceof UnknownShape) return unknownShape(transcriptPath, e.message); throw e; diff --git a/lib/skill-gates.ts b/lib/skill-gates.ts index 5736c7d60..43e0e5939 100644 --- a/lib/skill-gates.ts +++ b/lib/skill-gates.ts @@ -68,20 +68,25 @@ const SETTLE_POLL_MS = 50; * parsed). Claude Code can run Stop before it flushes the turn's last lines (a * smoke run saw a scope call and a phase publication land about 200 ms after * Stop fired), so a stale read can hold a finished run or miss a run's load. - * Re-read until the final text is there, for at most SETTLE_MS or half the - * deadline, then judge what exists. A transcript the parser cannot read is - * GATE_TRANSCRIPT. + * Re-read until the final text is there, for at most maxWaitMs (SETTLE_MS by + * default), then judge what exists. Read and parse errors (StateError, + * UnknownShape) pass through for the caller to classify. */ +export function readSettledTranscript(file: string, finalMessage: unknown, opts: { includeSidechain?: boolean; maxWaitMs?: number } = {}): { raw: string; t: Transcript } { + const until = Date.now() + (opts.maxWaitMs ?? SETTLE_MS); + for (;;) { + const raw = readTranscriptFile(file); + const t = parseTranscript(raw, opts); + const settled = withFinalMessage(t, finalMessage); + if (settled === t || Date.now() >= until) return { raw, t: settled }; + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, SETTLE_POLL_MS); + } +} + +/** readSettledTranscript for a gate hook: waits at most half its deadline, and a transcript it cannot read is GATE_TRANSCRIPT. */ export function settledTranscript(file: string, finalMessage: unknown, opts: { includeSidechain?: boolean } = {}): { raw: string; t: Transcript } { - const until = Date.now() + Math.min(SETTLE_MS, remainingMs() / 2); try { - for (;;) { - const raw = readTranscriptFile(file); - const t = parseTranscript(raw, opts); - const settled = withFinalMessage(t, finalMessage); - if (settled === t || Date.now() >= until) return { raw, t: settled }; - Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, SETTLE_POLL_MS); - } + return readSettledTranscript(file, finalMessage, { ...opts, maxWaitMs: Math.min(SETTLE_MS, remainingMs() / 2) }); } catch (e) { if (e instanceof StateError || e instanceof UnknownShape) throw new GateError('GATE_TRANSCRIPT', e.message); throw e; diff --git a/test/skill-contract-stop-hook.test.ts b/test/skill-contract-stop-hook.test.ts index 4259268ec..570324ebc 100644 --- a/test/skill-contract-stop-hook.test.ts +++ b/test/skill-contract-stop-hook.test.ts @@ -7,6 +7,7 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { bunIsolationArgs } from '../lib/bun-isolation'; import { AGENT_STALL_MS, STATE_ERROR_DOC, stateErrorLine } from '../lib/skill-contract'; import { SWITCH_DEFAULT } from '../lib/skill-route'; import { @@ -108,6 +109,32 @@ describe('block once, with the pinned text', () => { expect((Object.values(record().runs)[0] as any).outcome).toBe('met'); }); + test('a Stop that fires before the CLI flushes the turn waits for the final message: a run that reran its check is met, not blocked', async () => { + const file = transcript(J1_TRANSCRIPT.slice(0, -1)); + const proc = Bun.spawn([process.execPath, ...bunIsolationArgs(), CONTRACT_HOOK], { + stdin: new Blob([payload(file, { last_assistant_message: DEBUG_REPORT })]), env: hookEnv(state, skills, { GSTACK_SKILL_CONTRACTS: 'on' }), stdout: 'pipe', timeout: 20_000, + }); + await Bun.sleep(300); + fs.appendFileSync(file, `${[tool('b9', 'Bash', { command: 'bun test test/auth.test.ts' }), result('b9', 'pass'), say(DEBUG_REPORT)].join('\n')}\n`); + const out = await new Response(proc.stdout).text(); + expect(await proc.exited).toBe(0); + expect(out).toBe(''); + expect((Object.values(record().runs)[0] as any).outcome).toBe('met'); + }); + + test('a Stop that fires before the CLI flushes the load itself still checks the run once the final message lands', async () => { + const file = transcript(J1_TRANSCRIPT.slice(0, 1)); + const proc = Bun.spawn([process.execPath, ...bunIsolationArgs(), CONTRACT_HOOK], { + stdin: new Blob([payload(file, { last_assistant_message: 'Fixed: the null check was missing in src/auth.ts.' })]), env: hookEnv(state, skills, { GSTACK_SKILL_CONTRACTS: 'on' }), stdout: 'pipe', timeout: 20_000, + }); + await Bun.sleep(300); + fs.appendFileSync(file, `${J1_TRANSCRIPT.slice(1).join('\n')}\n`); + const out = await new Response(proc.stdout).text(); + expect(await proc.exited).toBe(0); + expect(JSON.parse(out || '{}').decision).toBe('block'); + expect((Object.values(record().runs)[0] as any).outcome).toBe('blocked'); + }); + test('an /investigate run that ends with Status: BLOCKED in its DEBUG REPORT is met, not blocked', () => { const file = transcript([...J1_TRANSCRIPT.slice(0, -1), say(DEBUG_REPORT.replace('Status: DONE', 'Status: BLOCKED'))]); expect(stop(file)).toEqual({ code: 0, stdout: '', stderr: '' });