mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-08 09:48:20 +01:00
fix: the CEO review gate waits for Claude Code's transcript flush (GST-1701) (#4102)
* test: failing-first cases for the CEO review gate reading a transcript before Claude Code flushes it (GST-1701) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: the CEO review gate waits for the transcript flush through #4100's shared settle helper (GST-1701) 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>
This commit is contained in:
1 parent
ca5dbfc7e0
commit
6c3d0ec536
6 files changed
+67
-40
No files matched your search
@@ -0,0 +1 @@
|
||||
**The /plan-ceo-review gate no longer judges a transcript Claude Code hasn't finished writing (GST-1701).** Claude Code can fire Stop about 200 ms before it flushes the turn's last transcript lines. The CEO review gate read the transcript once, so a finished review written outside the repository (a plan under `~/.claude/plans`) could be held because its Write wasn't on disk yet, and a run whose load hadn't landed could stop without being checked. The gate now waits, like the /autoplan gate, until the transcript holds the stop's final message (at most 2 seconds or half its deadline). Both gates share one helper, `settledTranscript` in `lib/skill-gates.ts`.
|
||||
@@ -69,6 +69,8 @@ Once a session loads /plan-ceo-review (the Skill tool, a typed `/plan-ceo-review
|
||||
|
||||
Bash commands are never parsed: a review written with a heredoc or `tee` is found by the repository and ceo-plans scans.
|
||||
|
||||
Like the /autoplan gate, the hook first re-reads the transcript until it holds the stop's `last_assistant_message`, for at most 2 seconds or half its deadline (`settledTranscript` in `lib/skill-gates.ts`). Claude Code can fire Stop before it writes the turn's last lines, so a single read could miss the Write of a review kept outside the repository (a plan under `~/.claude/plans`) and hold a finished run, or miss the load itself and let an unfinished one through.
|
||||
|
||||
**The block.** Its reason starts with the `blocked via=stop` marker and says exactly what to write and where: a `## Findings` section ranking the top risks as a numbered list, in `GSTACK_CEO_REVIEW_OUTPUT`, else the last markdown file the run wrote, else the last plan file it read (never a file in a `skills/` directory, judged from the repository root so a project that itself sits under a `skills/` folder still counts, such as the review's own methodology sections; such a file never releases the gate either), plus each file checked and why it fell short. It never names the off switch.
|
||||
|
||||
**The cap.** A run is blocked at most 3 times. The next final stop without ranked risks goes through with an `exhausted` marker and `exhausted_at` in the record. `stop_hook_active` (Claude Code sets it once a Stop hook has blocked the turn) does not skip the check; the cap ends the repeats. After `released` or `exhausted` the gate does nothing for the rest of that run; a new load starts a new run.
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* SWITCH_DEFAULT). Registered through one shim: autoplan-stop-gate-stop (Stop).
|
||||
*
|
||||
* Stop: payload transcript_path (+ last_assistant_message), re-read until it
|
||||
* holds that final message (at most 2 s; Stop can beat the CLI's flush)
|
||||
* holds that final message (lib/skill-gates.ts settledTranscript)
|
||||
* -> the last /autoplan load (none: nothing)
|
||||
* -> a final turn? (lib/skill-contract.ts isFinalTurn; a question to a
|
||||
* human in an interactive session is never blocked)
|
||||
@@ -25,45 +25,20 @@
|
||||
*/
|
||||
import { sessionKind } from './spawn-bin';
|
||||
import { SKILL, checkRun, missingStep } from '../../../lib/autoplan-stop-gate';
|
||||
import { StateError, UnknownShape, isFinalTurn, parseTranscript, readTranscriptFile, sameSkill, skillLoads, turnEnd, withFinalMessage } from '../../../lib/skill-contract';
|
||||
import { isFinalTurn, sameSkill, skillLoads, turnEnd } from '../../../lib/skill-contract';
|
||||
import { readSkillSwitch } from '../../../lib/skill-route';
|
||||
import {
|
||||
GateError, blockWith, hookRecord, note, pruneSessionDirs, readGateRecord, recordPath, remainingMs, runGateHook, sessionDir, systemMessage,
|
||||
blockWith, hookRecord, note, pruneSessionDirs, readGateRecord, recordPath, remainingMs, runGateHook, sessionDir, settledTranscript, systemMessage,
|
||||
withSessionLock, writeGateRecord, type GatePayload, type HookOutput,
|
||||
} from '../../../lib/skill-gates';
|
||||
|
||||
const HOOK = 'autoplan-gate';
|
||||
const MAX_BLOCKS = 3;
|
||||
const SESSION_KIND_TIMEOUT_MS = 1000;
|
||||
const SETTLE_MS = 2000;
|
||||
const SETTLE_POLL_MS = 50;
|
||||
|
||||
/**
|
||||
* The transcript once it holds the turn's final message. Claude Code can run
|
||||
* Stop before it flushes the turn's last lines (the smoke run saw the scope
|
||||
* call and a phase publication land about 200 ms after Stop fired), so a
|
||||
* stale read would block a finished run. Re-read until the final text is
|
||||
* there, for at most SETTLE_MS or half the deadline, then judge what exists.
|
||||
*/
|
||||
function settledTranscript(p: GatePayload) {
|
||||
const until = Date.now() + Math.min(SETTLE_MS, remainingMs() / 2);
|
||||
for (;;) {
|
||||
const t = parseTranscript(readTranscriptFile(p.transcriptPath));
|
||||
const settled = withFinalMessage(t, p.raw.last_assistant_message);
|
||||
if (settled === t || Date.now() >= until) return settled;
|
||||
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, SETTLE_POLL_MS);
|
||||
}
|
||||
}
|
||||
|
||||
function handle(p: GatePayload): HookOutput | undefined {
|
||||
if (p.event && p.event !== 'Stop') return undefined;
|
||||
let t;
|
||||
try {
|
||||
t = settledTranscript(p);
|
||||
} catch (e) {
|
||||
if (e instanceof StateError || e instanceof UnknownShape) throw new GateError('GATE_TRANSCRIPT', e.message);
|
||||
throw e;
|
||||
}
|
||||
const { t } = settledTranscript(p.transcriptPath, p.raw.last_assistant_message);
|
||||
const load = skillLoads(t).filter(l => sameSkill(l.skill, SKILL)).pop();
|
||||
if (!load) return undefined;
|
||||
if (!isFinalTurn(turnEnd(t), () => sessionKind(Math.min(SESSION_KIND_TIMEOUT_MS, remainingMs())))) return undefined;
|
||||
|
||||
@@ -8,6 +8,8 @@
|
||||
* Stop: payload transcript_path (+ last_assistant_message)
|
||||
* SubagentStop: payload agent_transcript_path, sidechain records included,
|
||||
* so only a subagent that itself loaded /plan-ceo-review is gated
|
||||
* -> re-read until it holds that final message (lib/skill-gates.ts
|
||||
* settledTranscript; Stop can beat the CLI's flush)
|
||||
* -> the last /plan-ceo-review load (none: nothing)
|
||||
* -> Stop only: a final turn? (lib/skill-contract.ts isFinalTurn; a
|
||||
* question to a human in an interactive session is never blocked)
|
||||
@@ -26,10 +28,10 @@
|
||||
import * as path from 'path';
|
||||
import { runBin, sessionKind } from './spawn-bin';
|
||||
import { SKILL, checkOutput } from '../../../lib/ceo-review-gate';
|
||||
import { StateError, UnknownShape, isFinalTurn, parseTranscript, readTranscriptFile, sameSkill, skillLoads, turnEnd, withFinalMessage } from '../../../lib/skill-contract';
|
||||
import { isFinalTurn, sameSkill, skillLoads, turnEnd } from '../../../lib/skill-contract';
|
||||
import { readSkillSwitch } from '../../../lib/skill-route';
|
||||
import {
|
||||
GateError, blockWith, hookRecord, note, pruneSessionDirs, readGateRecord, recordPath, remainingMs, runGateHook, sessionDir, systemMessage,
|
||||
GateError, blockWith, hookRecord, note, pruneSessionDirs, readGateRecord, recordPath, remainingMs, runGateHook, sessionDir, settledTranscript, systemMessage,
|
||||
withSessionLock, writeGateRecord, type GatePayload, type HookOutput,
|
||||
} from '../../../lib/skill-gates';
|
||||
|
||||
@@ -49,15 +51,7 @@ function handle(p: GatePayload): HookOutput | undefined {
|
||||
const sub = p.event === 'SubagentStop';
|
||||
const file = sub ? p.raw.agent_transcript_path : p.transcriptPath;
|
||||
if (typeof file !== 'string' || !file) return undefined;
|
||||
let raw: string;
|
||||
let t;
|
||||
try {
|
||||
raw = readTranscriptFile(file);
|
||||
t = withFinalMessage(parseTranscript(raw, { includeSidechain: sub }), p.raw.last_assistant_message);
|
||||
} catch (e) {
|
||||
if (e instanceof StateError || e instanceof UnknownShape) throw new GateError('GATE_TRANSCRIPT', e.message);
|
||||
throw e;
|
||||
}
|
||||
const { raw, t } = settledTranscript(file, p.raw.last_assistant_message, { includeSidechain: sub });
|
||||
const load = skillLoads(t).filter(l => sameSkill(l.skill, SKILL)).pop();
|
||||
if (!load) return undefined;
|
||||
if (!sub && !isFinalTurn(turnEnd(t), () => sessionKind(Math.min(SESSION_KIND_TIMEOUT_MS, remainingMs())))) return undefined;
|
||||
|
||||
@@ -16,6 +16,7 @@ import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { atomicWriteSync } from './fs-atomic';
|
||||
import { resolveStateRoot } from './state-root';
|
||||
import { StateError, UnknownShape, parseTranscript, readTranscriptFile, withFinalMessage, type Transcript } from './skill-contract';
|
||||
import { appendHookTrace, readStdinBounded, type TraceHook } from './skill-route';
|
||||
|
||||
export type GateHook = 'investigate-gate' | 'ceo-review-gate' | 'autoplan-gate';
|
||||
@@ -59,6 +60,34 @@ export class GateError extends Error {
|
||||
constructor(readonly code: string, why: string) { super(why); }
|
||||
}
|
||||
|
||||
const SETTLE_MS = 2000;
|
||||
const SETTLE_POLL_MS = 50;
|
||||
|
||||
/**
|
||||
* A stop's transcript once it holds the payload's final message (raw text and
|
||||
* 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.
|
||||
*/
|
||||
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);
|
||||
}
|
||||
} catch (e) {
|
||||
if (e instanceof StateError || e instanceof UnknownShape) throw new GateError('GATE_TRANSCRIPT', e.message);
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
export const cleanSessionId = (sid: string) => sid.replace(/[^A-Za-z0-9_-]/g, '').slice(0, 64) || 'unknown';
|
||||
const markerValue = (v: string | number) => String(v).replace(/\s+/g, '_').slice(0, 240);
|
||||
|
||||
|
||||
@@ -12,6 +12,7 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { bunIsolationArgs } from '../lib/bun-isolation';
|
||||
import { rankedRisksGap } from '../lib/ceo-review-gate';
|
||||
import { ROOT, hookEnv, result, runHook, say, skillsDir, tmpDir, tool, user } from './helpers/skill-contract-fixture';
|
||||
|
||||
@@ -219,6 +220,31 @@ describe('final turns only', () => {
|
||||
expect(stop([...LOADED], { last_assistant_message: 'Which mode do you want: expansion or hold?' }).stdout).toBe('');
|
||||
});
|
||||
|
||||
test('a Stop that fires before the CLI flushes the turn waits for the final message: a review written outside the repo is released, not held', async () => {
|
||||
const plan = put(path.join(work, 'plans', 'checkout.md'), RANKED);
|
||||
const file = transcript(LOADED);
|
||||
const payload = { session_id: 'sess-1', transcript_path: file, cwd: repo, hook_event_name: 'Stop', stop_hook_active: false, last_assistant_message: 'Review complete.' };
|
||||
const proc = Bun.spawn([process.execPath, ...bunIsolationArgs(), HOOK], { stdin: new Blob([JSON.stringify(payload)]), env: baseEnv(), stdout: 'pipe', timeout: 20_000 });
|
||||
await Bun.sleep(300);
|
||||
fs.appendFileSync(file, `${[...wrote(plan), DONE].join('\n')}\n`);
|
||||
const out = json(await new Response(proc.stdout).text());
|
||||
expect(await proc.exited).toBe(0);
|
||||
expect(out.decision).toBeUndefined();
|
||||
expect(out.systemMessage).toMatch(/^GSTACK_GATE ceo-review-gate released session=sess-1 skill=plan-ceo-review file=checkout.md blocked=0$/m);
|
||||
});
|
||||
|
||||
test('a Stop that fires before the CLI flushes the load itself still judges the run once the final message lands', async () => {
|
||||
const file = transcript([user('review my plan')]);
|
||||
const payload = { session_id: 'sess-1', transcript_path: file, cwd: repo, hook_event_name: 'Stop', stop_hook_active: false, last_assistant_message: 'Review complete.' };
|
||||
const proc = Bun.spawn([process.execPath, ...bunIsolationArgs(), HOOK], { stdin: new Blob([JSON.stringify(payload)]), env: baseEnv(), stdout: 'pipe', timeout: 20_000 });
|
||||
await Bun.sleep(300);
|
||||
fs.appendFileSync(file, `${[...LOADED.slice(1), DONE].join('\n')}\n`);
|
||||
const out = json(await new Response(proc.stdout).text());
|
||||
expect(await proc.exited).toBe(0);
|
||||
expect(out?.decision).toBe('block');
|
||||
expect(out.reason.split('\n')[0]).toBe('GSTACK_GATE ceo-review-gate blocked session=sess-1 skill=plan-ceo-review via=stop files=none n=1');
|
||||
});
|
||||
|
||||
test('no /plan-ceo-review load, or the switch off: no output and no log', () => {
|
||||
expect(stop([user('hi'), DONE]).stdout).toBe('');
|
||||
expect(stop([...LOADED, DONE], {}, { GSTACK_CEO_REVIEW_GATE: 'off' }).stdout).toBe('');
|
||||
|
||||
Reference in new issue
Block a user