mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-06 17:04:27 +01:00
fix: recognize completed reviews without losing findings
This commit is contained in:
Garry Tan committed
1 parent
15356c8894
commit
02e6d4db57
17 files changed
+1235
-45
No files matched your search
@@ -182,10 +182,13 @@ For each item in the diagram:
|
||||
* What is the edge case test? (nil, empty, boundary values, concurrent access)
|
||||
|
||||
For each behavior, name its observable assertion and a wrong result it must reject.
|
||||
Helper coverage alone does not prove the caller's path. Deferring assertion syntax
|
||||
does not waive a missing behavioral check. Raise each independent missing check
|
||||
under the per-issue rule below; do not invent one when equivalent caller coverage
|
||||
exists or the user explicitly accepted that particular risk.
|
||||
Keep user-required behaviors and assertions mandatory unless the user explicitly
|
||||
approves changing them. Never fill a missing behavioral assertion silently in the
|
||||
final report: obtain its own AskUserQuestion decision first. Scope/approach approval
|
||||
is not approval of individual assertion gaps. Helper coverage alone does not prove
|
||||
the caller's path; deferring syntax does not waive a missing check. Raise each
|
||||
independent missing check separately; do not invent one where equivalent caller
|
||||
coverage exists or the user explicitly accepted that particular risk.
|
||||
|
||||
Test ambition check (all modes): For each new feature, answer:
|
||||
* What's the test that would make you confident shipping at 2am on a Friday?
|
||||
|
||||
@@ -180,10 +180,13 @@ For each item in the diagram:
|
||||
* What is the edge case test? (nil, empty, boundary values, concurrent access)
|
||||
|
||||
For each behavior, name its observable assertion and a wrong result it must reject.
|
||||
Helper coverage alone does not prove the caller's path. Deferring assertion syntax
|
||||
does not waive a missing behavioral check. Raise each independent missing check
|
||||
under the per-issue rule below; do not invent one when equivalent caller coverage
|
||||
exists or the user explicitly accepted that particular risk.
|
||||
Keep user-required behaviors and assertions mandatory unless the user explicitly
|
||||
approves changing them. Never fill a missing behavioral assertion silently in the
|
||||
final report: obtain its own AskUserQuestion decision first. Scope/approach approval
|
||||
is not approval of individual assertion gaps. Helper coverage alone does not prove
|
||||
the caller's path; deferring syntax does not waive a missing check. Raise each
|
||||
independent missing check separately; do not invent one where equivalent caller
|
||||
coverage exists or the user explicitly accepted that particular risk.
|
||||
|
||||
Test ambition check (all modes): For each new feature, answer:
|
||||
* What's the test that would make you confident shipping at 2am on a Friday?
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { autoplanRoutingSetupInput } from './helpers/autoplan-setup-question';
|
||||
import { E2E_TOUCHFILES, selectTests } from './helpers/touchfiles';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
|
||||
// Sanitized terminal frame from the 2026-09-08 autoplan timeout. The qid is
|
||||
// visibly incomplete; the prompt body and explicit choices remain intact.
|
||||
@@ -317,3 +321,182 @@ describe('autoplan routing action survives courtesy repaint', () => {
|
||||
]) expect(autoplanRoutingSetupInput(frame, new Set()), frame).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
const PREREQUISITE_CAPTURE = " ☐ Design doc\n\n│ No design doc found for this branch. /office-hours produces a structured problem statement, premise challenge, and\n│ explored alternatives — it gives this review much sharper input to work with. Takes about 10 minutes. The design doc\n│ is per-feature, not per-product — it captures the thinking behind this specific change. Run /office-hours first?\n\n❯ 1. Run /office-hours now\n Runs /office-hours to produce a design doc first, then picks up the full autoplan review right after. (~10 min)\n 2. Skip — proceed with standard review\n Skips /office-hours and runs the autoplan review pipeline now using the existing plan file as input.\n 3. Type something.\n────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\n 4. Chat about this\n\nEnter to select · ↑/↓ to navigate · Esc to cancel\n";
|
||||
const prerequisiteQuestion = {
|
||||
header: 'Design doc',
|
||||
question: "No design doc found for this branch. /office-hours produces a structured problem statement, premise challenge, and explored alternatives — it gives this review much sharper input to work with. Takes about 10 minutes. The design doc is per-feature, not per-product — it captures the thinking behind this specific change. Run /office-hours first?",
|
||||
options: [{ label: 'Run /office-hours now' }, { label: 'Skip — proceed with standard review' }],
|
||||
};
|
||||
const prerequisiteCall = () => ({
|
||||
sessionId: 'prerequisite-session', toolUseId: 'prerequisite-call',
|
||||
answered: false, failed: false, questions: [structuredClone(prerequisiteQuestion)],
|
||||
});
|
||||
function prerequisiteMenu(reverse = false) {
|
||||
if (!reverse) return PREREQUISITE_CAPTURE;
|
||||
return PREREQUISITE_CAPTURE
|
||||
.replace('1. Run /office-hours now', '1. Skip — proceed with standard review')
|
||||
.replace('2. Skip — proceed with standard review', '2. Run /office-hours now');
|
||||
}
|
||||
|
||||
describe('autoplan optional design-doc prerequisite', () => {
|
||||
test('the exact K native screen declines the optional prerequisite by label', () => {
|
||||
for (const reverse of [false, true]) {
|
||||
const frame = prerequisiteMenu(reverse);
|
||||
expect(autoplanRoutingSetupInput(frame, new Set())).toBe(reverse ? '1' : '2');
|
||||
const native = prerequisiteCall(); if (reverse) native.questions[0]!.options.reverse();
|
||||
expect(autoplanRoutingSetupInput(frame, new Set(), native)).toBe(reverse ? '1' : '2');
|
||||
}
|
||||
});
|
||||
|
||||
test('quoted panels and menus followed by new output are not active input', () => {
|
||||
for (const frame of [
|
||||
'Example panel:\n```text\n' + PREREQUISITE_CAPTURE + '\n```\n',
|
||||
'Example panel:\n~~~text\n' + PREREQUISITE_CAPTURE,
|
||||
'Example panel:\n' + PREREQUISITE_CAPTURE,
|
||||
PREREQUISITE_CAPTURE.split('\n').map(line => ' ' + line).join('\n'),
|
||||
'The document quotes this panel:\n────────────────────\n' + PREREQUISITE_CAPTURE,
|
||||
PREREQUISITE_CAPTURE + '\n⏺ Continuing the review without office hours.\n',
|
||||
PREREQUISITE_CAPTURE + '\n❯ 1. A new menu\n 2. Another choice\n',
|
||||
]) for (const native of [undefined, prerequisiteCall()]) {
|
||||
expect(autoplanRoutingSetupInput(frame, new Set(), native)).toBeNull();
|
||||
}
|
||||
expect(autoplanRoutingSetupInput('```text\nearlier real code\n```\n────────────────────\n' + PREREQUISITE_CAPTURE, new Set())).toBe('2');
|
||||
});
|
||||
|
||||
test('late native identity does not re-answer the retained menu', () => {
|
||||
const seen = new Set<string>();
|
||||
expect(autoplanRoutingSetupInput(PREREQUISITE_CAPTURE, seen)).toBe('2');
|
||||
expect(autoplanRoutingSetupInput(PREREQUISITE_CAPTURE, seen, prerequisiteCall())).toBeNull();
|
||||
expect(autoplanRoutingSetupInput(PREREQUISITE_CAPTURE, seen)).toBeNull();
|
||||
});
|
||||
|
||||
test('unrelated, failed, mixed and checkbox native calls do not borrow the setup menu', () => {
|
||||
for (const mutate of [
|
||||
(call: ReturnType<typeof prerequisiteCall>) => { call.questions[0]!.question = 'Should we change the dashboard design?'; },
|
||||
(call: ReturnType<typeof prerequisiteCall>) => { call.failed = true; },
|
||||
(call: ReturnType<typeof prerequisiteCall>) => { call.answered = true; },
|
||||
(call: ReturnType<typeof prerequisiteCall>) => { call.questions.push({ header:'Finding', question:'Fix missing auth?', options:[{label:'Fix it'},{label:'Defer'}] }); },
|
||||
(call: ReturnType<typeof prerequisiteCall>) => { Object.assign(call.questions[0]!, {multiSelect:true}); },
|
||||
]) {
|
||||
const native = prerequisiteCall(); mutate(native);
|
||||
const seen = new Set<string>();
|
||||
expect(autoplanRoutingSetupInput(PREREQUISITE_CAPTURE, seen, native)).toBeNull();
|
||||
// Waiting for correct metadata must not mark an unanswered UI as sent.
|
||||
expect(autoplanRoutingSetupInput(PREREQUISITE_CAPTURE, seen, prerequisiteCall())).toBe('2');
|
||||
}
|
||||
});
|
||||
|
||||
test('arbitrary skip, outside offers, mixed actions and prose examples remain unanswered', () => {
|
||||
for (const frame of [
|
||||
PREREQUISITE_CAPTURE.replace('Skip — proceed with standard review', 'Skip this security check'),
|
||||
PREREQUISITE_CAPTURE.replaceAll('/office-hours', '/codex'),
|
||||
PREREQUISITE_CAPTURE.replace('3. Type something.', '3. Fix the missing authorization check'),
|
||||
PREREQUISITE_CAPTURE.replace('No design doc found for this branch.', 'A dashboard design issue was found.'),
|
||||
PREREQUISITE_CAPTURE.replace(' ☐ Design doc', 'Example choices:').replace('Enter to select · ↑/↓ to navigate · Esc to cancel', ''),
|
||||
]) expect(autoplanRoutingSetupInput(frame, new Set())).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
test.skipIf(process.platform === 'win32')('real PTY prerequisite answer survives early and deferred native records without a second key', async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-autoplan-prereq-'));
|
||||
const fake = path.join(dir, 'fake-claude');
|
||||
const worker = path.join(dir, 'worker.ts');
|
||||
const resultFile = path.join(dir, 'result.json');
|
||||
const cases = [false, true].flatMap(early => [false, true].map(reverse => {
|
||||
const name = `${early ? 'early' : 'deferred'}-${reverse ? 'reversed' : 'original'}`;
|
||||
const q = structuredClone(prerequisiteQuestion); if (reverse) q.options.reverse();
|
||||
return { name, early, cwd: path.join(dir, name), record: path.join(dir, name + '.jsonl'),
|
||||
question: q, frame: prerequisiteMenu(reverse), expected: reverse ? '1' : '2' };
|
||||
}));
|
||||
for (const item of cases) fs.mkdirSync(item.cwd);
|
||||
fs.writeFileSync(fake, `#!${process.execPath}\n` + String.raw`
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
const item = JSON.parse(process.env.PREREQUISITE_REPLAY);
|
||||
const record = event => fs.appendFileSync(item.record, JSON.stringify(event) + '\n');
|
||||
record({type:'startup',pid:process.pid});
|
||||
const folder = path.join(process.env.CLAUDE_CONFIG_DIR, 'projects', 'fixture');
|
||||
fs.mkdirSync(folder, {recursive:true});
|
||||
const transcript = path.join(folder, item.name + '.jsonl');
|
||||
let logged = false;
|
||||
function writeCall() {
|
||||
if (logged) return; logged = true;
|
||||
fs.appendFileSync(transcript, JSON.stringify({type:'assistant',sessionId:item.name,isSidechain:false,cwd:process.cwd(),timestamp:new Date().toISOString(),
|
||||
message:{role:'assistant',content:[{type:'tool_use',id:'prerequisite',name:'AskUserQuestion',input:{questions:[item.question]}}]}})+'\n');
|
||||
}
|
||||
if (item.early) writeCall();
|
||||
process.stdin.setRawMode?.(true);
|
||||
let answered = false;
|
||||
process.stdin.on('data', data => {
|
||||
record({type:'input',data:data.toString()});
|
||||
for (const key of data.toString()) if (/^[12]$/.test(key) && !answered) {
|
||||
answered = true; writeCall();
|
||||
const label = item.question.options[Number(key)-1].label;
|
||||
fs.appendFileSync(transcript, JSON.stringify({type:'user',sessionId:item.name,isSidechain:false,cwd:process.cwd(),timestamp:new Date().toISOString(),
|
||||
toolUseResult:{answers:{[item.question.question]:label}},
|
||||
message:{role:'user',content:[{type:'tool_result',tool_use_id:'prerequisite',content:'answered'}]}})+'\n');
|
||||
process.stdout.write('\x1b[2J\x1b[H'+item.frame+'\nSETUP_ANSWERED\n');
|
||||
}
|
||||
});
|
||||
process.stdout.write('\x1b[2J\x1b[H'+item.frame);
|
||||
process.on('SIGINT', () => process.exit(0));
|
||||
process.stdin.resume();
|
||||
`);
|
||||
fs.chmodSync(fake, 0o755);
|
||||
const moduleUrl = (name: string) => pathToFileURL(path.resolve(import.meta.dir, 'helpers', name)).href;
|
||||
fs.writeFileSync(worker, `
|
||||
import {launchClaudePty} from ${JSON.stringify(moduleUrl('claude-pty-runner.ts'))};
|
||||
import {autoplanRoutingSetupInput} from ${JSON.stringify(moduleUrl('autoplan-setup-question.ts'))};
|
||||
import {readPlanCountTranscript} from ${JSON.stringify(moduleUrl('plan-count-transcript.ts'))};
|
||||
const results = await Promise.all(${JSON.stringify(cases)}.map(async item => {
|
||||
const session = await launchClaudePty({cwd:item.cwd,observeScreen:true,timeoutMs:20000,env:{PREREQUISITE_REPLAY:JSON.stringify(item)}});
|
||||
try {
|
||||
await session.waitFor('Enter to select', {timeoutMs:10000,pollMs:20});
|
||||
const screen = await session.currentScreen();
|
||||
const before = readPlanCountTranscript(session.hermeticConfigDir,item.cwd);
|
||||
const pending = before.calls.find(call => !call.answered && !call.failed);
|
||||
if (Boolean(pending) !== item.early) throw Error('Wrong initial native persistence state');
|
||||
const seen = new Set();
|
||||
const input = autoplanRoutingSetupInput(screen,seen,pending);
|
||||
if (input !== item.expected) throw Error('Expected skip input '+item.expected+', got '+JSON.stringify(input));
|
||||
session.send(input);
|
||||
await session.waitFor('SETUP_ANSWERED', {timeoutMs:10000,pollMs:20});
|
||||
const after = readPlanCountTranscript(session.hermeticConfigDir,item.cwd);
|
||||
const call = after.calls[0];
|
||||
if (after.calls.length !== 1 || !call.answered) throw Error('Native answer was not persisted');
|
||||
const retained = await session.currentScreen();
|
||||
return {name:item.name,input,answer:call.answers[item.question.question],
|
||||
redraw:autoplanRoutingSetupInput(retained,seen),
|
||||
delayedIdentity:autoplanRoutingSetupInput(screen,seen,{...call,answered:false})};
|
||||
} finally {await session.close();}
|
||||
}));
|
||||
await Bun.write(${JSON.stringify(resultFile)},JSON.stringify(results));
|
||||
`);
|
||||
const child = Bun.spawn([process.execPath, worker], {
|
||||
env: { ...process.env, BROWSE_TERMINAL_BINARY: fake, EVALS_HERMETIC: '1' },
|
||||
stdout: 'pipe', stderr: 'pipe',
|
||||
});
|
||||
const killer = setTimeout(() => child.kill('SIGKILL'), 25000);
|
||||
try {
|
||||
const [exit, stdout, stderr] = await Promise.all([child.exited, new Response(child.stdout).text(), new Response(child.stderr).text()]);
|
||||
expect(exit, stdout + stderr).toBe(0);
|
||||
expect(JSON.parse(fs.readFileSync(resultFile, 'utf8'))).toEqual(cases.map(item => ({
|
||||
name:item.name,input:item.expected,answer:'Skip — proceed with standard review',redraw:null,delayedIdentity:null,
|
||||
})));
|
||||
for (const item of cases) {
|
||||
const events = fs.readFileSync(item.record, 'utf8').trim().split('\n').map(line => JSON.parse(line));
|
||||
expect(events.filter(event => event.type === 'input').map(event => event.data).join('')).toBe(item.expected);
|
||||
expect(() => process.kill(events[0].pid, 0)).toThrow();
|
||||
}
|
||||
} finally {
|
||||
clearTimeout(killer); child.kill('SIGKILL');
|
||||
for (const item of cases) {
|
||||
if (!fs.existsSync(item.record)) continue;
|
||||
const first = JSON.parse(fs.readFileSync(item.record, 'utf8').split('\n')[0]!);
|
||||
try { process.kill(first.pid, 'SIGKILL'); } catch { /* already reaped */ }
|
||||
}
|
||||
fs.rmSync(dir, {recursive:true,force:true});
|
||||
}
|
||||
}, 30000);
|
||||
@@ -2,11 +2,12 @@ import { describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { capturePlanCountQuestion, ceoStep0Boundary, hasNativePlanTerminal, nativePlanCallFingerprint, planCountQuestionPhase } from './helpers/claude-pty-runner';
|
||||
import { capturePlanCountQuestion, ceoFirstReviewAUQ, ceoStep0Boundary, hasNativePlanTerminal, nativePlanCallFingerprint, planCountQuestionPhase } from './helpers/claude-pty-runner';
|
||||
import { isCeoCompletionHandoff, pickCeoCompletionHandoff } from './helpers/ceo-completion-handoff';
|
||||
import type { NativePlanQuestionCall } from './helpers/plan-count-transcript';
|
||||
import captures from './fixtures/ceo-completion-handoff-calls.json';
|
||||
import currentHandoffs from './fixtures/ceo-completion-handoff-j-calls.json';
|
||||
import kHandoffs from './fixtures/ceo-completion-handoff-k-calls.json';
|
||||
|
||||
type CapturedCall = typeof captures.cases[number]['calls'][number];
|
||||
function nativeCall(record: CapturedCall, sessionId = 'native-capture'): NativePlanQuestionCall {
|
||||
@@ -20,7 +21,7 @@ function nativeCall(record: CapturedCall, sessionId = 'native-capture'): NativeP
|
||||
const handoff = () => nativeCall(captures.cases[0]!.calls.at(-1)!);
|
||||
const fingerprint = (call: NativePlanQuestionCall) => nativePlanCallFingerprint(call, 0, false);
|
||||
|
||||
function replay(calls: NativePlanQuestionCall[], reviewStarted = true) {
|
||||
function replay(calls: NativePlanQuestionCall[], reviewStarted = true, firstReview = (_fp: ReturnType<typeof fingerprint>) => true) {
|
||||
const counts = { step0Count: 0, reviewCount: 0, administrativeCount: 0 };
|
||||
const classifications = [];
|
||||
for (const call of calls) {
|
||||
@@ -28,7 +29,7 @@ function replay(calls: NativePlanQuestionCall[], reviewStarted = true) {
|
||||
const phase = planCountQuestionPhase(fp, reviewStarted, ceoStep0Boundary,
|
||||
// A completion summary can mention defects; even a broad positive
|
||||
// first-finding predicate must not promote a handoff into coverage.
|
||||
() => true, undefined, isCeoCompletionHandoff);
|
||||
firstReview, undefined, isCeoCompletionHandoff);
|
||||
if (phase.administrative) counts.administrativeCount++;
|
||||
else if (phase.preReview) counts.step0Count++;
|
||||
else counts.reviewCount++;
|
||||
@@ -437,3 +438,89 @@ describe('captured CEO next-step prefixes and immediate review menus', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe('native CEO completed handoffs with deferred implementation', () => {
|
||||
test('captured full sessions keep all substantive questions and classify only the final handoff', () => {
|
||||
for (const scenario of kHandoffs.cases) {
|
||||
const calls = structuredClone(scenario.calls) as NativePlanQuestionCall[];
|
||||
const original = structuredClone(calls);
|
||||
const result = replay(calls, false, ceoFirstReviewAUQ);
|
||||
expect(result).toMatchObject({ step0Count: scenario.expectedSetupCount,
|
||||
reviewCount: scenario.expectedReviewCount, administrativeCount: 1 });
|
||||
expect(result.classifications.slice(0, -1).every(p => !p.administrative)).toBe(true);
|
||||
expect(calls).toEqual(original);
|
||||
}
|
||||
});
|
||||
|
||||
test('active native handoffs choose manual in either order, never implementation or another review', () => {
|
||||
for (const scenario of kHandoffs.cases) for (const reverse of [false, true]) {
|
||||
const call = structuredClone(scenario.calls.at(-1)!) as NativePlanQuestionCall;
|
||||
call.answered = false; delete call.answers; delete call.unansweredQuestionIndices;
|
||||
const q = call.questions[0]!;
|
||||
if (reverse) q.options.reverse();
|
||||
const options = q.options.map((option, i) => `${i === 0 ? '❯' : ' '} ${i + 1}. ${option.label}`).join('\n');
|
||||
const screen = `☐ ${q.header}\n${q.question}\n${options}\nEnter to select · ↑/↓ to navigate · Esc to cancel`;
|
||||
const bound = capturePlanCountQuestion(screen, new Set(), 0, false, call)!;
|
||||
expect(bound.nativeCall?.toolUseId).toBe(call.toolUseId);
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call), bound)).toBe(q.options.findIndex(o => /handle.*manually/i.test(o.label)) + 1);
|
||||
expect(isCeoCompletionHandoff(bound)).toBe(false);
|
||||
const uiOnly = capturePlanCountQuestion(screen, new Set(), 0, false)!;
|
||||
expect(pickCeoCompletionHandoff(uiOnly)).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
test('conditional declarations and new implementation obligations remain substantive', () => {
|
||||
for (const scenario of kHandoffs.cases) for (const question of [
|
||||
'ELI10: If the CEO review is done and the plan is cleared, choose the next step.',
|
||||
'ELI10: The CEO review is done only after resolving the test gap.',
|
||||
'ELI10: The CEO review is done and the plan is cleared after you add retry tests.',
|
||||
'ELI10: The CEO review is not done and the plan is not cleared.',
|
||||
]) {
|
||||
const call = structuredClone(scenario.calls.at(-1)!) as NativePlanQuestionCall;
|
||||
call.questions[0]!.question = question + ' The required shipping gate is an Eng Review.';
|
||||
call.answers = { [call.questions[0]!.question]: call.questions[0]!.options[0]!.label };
|
||||
expect(isCeoCompletionHandoff(fingerprint(call))).toBe(false);
|
||||
}
|
||||
for (const option of [
|
||||
{ label: 'Implement now, eng review later', description: 'Add the missing receipt test, then implement.' },
|
||||
{ label: 'Implement now, eng review later', description: 'Implement the approved tasks and add a new receipt assertion before the next review.' },
|
||||
{ label: 'Implement now, eng review later', description: 'The plan has no approved tasks; decide the missing error contract during implementation.' },
|
||||
{ label: 'Implement new retry behavior now, eng review later', description: 'The plan already has approved tasks.' },
|
||||
{ label: 'Add another TODO before implementing', description: 'Use the approved plan.' },
|
||||
]) {
|
||||
const call = structuredClone(kHandoffs.cases[0]!.calls.at(-1)!) as NativePlanQuestionCall;
|
||||
call.questions[0]!.options[1] = option;
|
||||
expect(isCeoCompletionHandoff(fingerprint(call))).toBe(false);
|
||||
call.answered = false;
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call))).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
test('a real native approval after the completed report still requires all substantive answers in that report', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ceo-k-handoff-'));
|
||||
const file = path.join(dir, 'plan.md');
|
||||
try {
|
||||
for (const scenario of kHandoffs.cases) {
|
||||
fs.writeFileSync(file, '# Plan\n\n## GSTACK REVIEW REPORT\n\n' +
|
||||
'| Review | Runs | Status | Findings |\n|---|---|---|---|\n| CEO | 1 | COMPLETE | 4 |\n\n' +
|
||||
'VERDICT: CEO CLEARED\n\nNO UNRESOLVED DECISIONS\n');
|
||||
fs.utimesSync(file, scenario.reportAtMs / 1000, scenario.reportAtMs / 1000);
|
||||
const calls = structuredClone(scenario.calls) as NativePlanQuestionCall[];
|
||||
const transcript = { status: 'ready' as const, calls, assistantMessages: [],
|
||||
planReadyRequests: structuredClone(scenario.planReadyRequests) };
|
||||
const admin = new Set(calls.filter(c => isCeoCompletionHandoff(fingerprint(c))).map(c => `${c.sessionId}:${c.toolUseId}`));
|
||||
const startedAt = Date.parse('2026-09-08T22:17:54Z');
|
||||
expect(admin.size).toBe(1);
|
||||
expect(hasNativePlanTerminal(transcript, file, startedAt, 'plan_ready')).toBe(false);
|
||||
expect(hasNativePlanTerminal(transcript, file, startedAt, 'plan_ready', admin)).toBe(true);
|
||||
transcript.planReadyRequests[0]!.failed = true;
|
||||
expect(hasNativePlanTerminal(transcript, file, startedAt, 'plan_ready', admin)).toBe(false);
|
||||
transcript.planReadyRequests[0]!.failed = false;
|
||||
calls.splice(-1, 0, { ...structuredClone(calls[2]!), toolUseId: 'new-substantive-answer',
|
||||
answeredAt: new Date(scenario.reportAtMs + 1000).toISOString() });
|
||||
expect(hasNativePlanTerminal(transcript, file, startedAt, 'plan_ready', admin)).toBe(false);
|
||||
}
|
||||
} finally { fs.rmSync(dir, { recursive: true, force: true }); }
|
||||
});
|
||||
});
|
||||
+422
@@ -0,0 +1,422 @@
|
||||
{
|
||||
"provenance": "Native source-K diagnostic calls; historical outcomes are unchanged by replay.",
|
||||
"cases": [
|
||||
{
|
||||
"name": "plan-ceo-review-1788905904650-1fnva5",
|
||||
"calls": [
|
||||
{
|
||||
"sessionId": "4cd0a135-d63c-488a-ae90-e0f2f4128675",
|
||||
"toolUseId": "toolu_01K5chLBCqTL9PaEi8H3AZhK",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Which implementation approach for the payment webhook handler? <gstack-qid:plan-ceo-review-approach>",
|
||||
"header": "Approach",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "B: Correct implementation (Recommended)",
|
||||
"description": "Parameterized queries (fixes SQL injection), try/catch on email with structured logging, batch order fetch (kills N+1), full unit + integration tests. Effort: human ~4\u20136h / CC ~20min. Completeness: 9/10.",
|
||||
"preview": "// Approach B shape\nconst user = await db.query(\n 'SELECT * FROM users WHERE id = $1',\n [userId] // parameterized \u2014 injection impossible\n);\nconst orders = await db.query(\n 'SELECT * FROM orders WHERE user_id = $1',\n [userId] // single batch, not N+1 loop\n);\nawait db.updateUser(userId, { status: 'paid' });\ntry {\n await email.send(user.email, 'Payment confirmed');\n} catch (err) {\n logger.error({ err, userId, eventId }, 'email send failed');\n // webhook still returns 200; email failure is logged, not fatal\n}"
|
||||
},
|
||||
{
|
||||
"label": "A: Minimal patch",
|
||||
"description": "Fix only the SQL injection (parameterized query) and add a try/catch on email. Keep N+1 loop and add 2 smoke tests. Faster but leaves performance debt. Effort: human ~2h / CC ~10min. Completeness: 6/10.",
|
||||
"preview": "// Approach A shape \u2014 fix injection, add catch, skip N+1\nconst user = await db.query(\n 'SELECT * FROM users WHERE id = $1',\n [userId] // parameterized\n);\nfor (const orderId of orderIds) {\n const order = await db.query( // N+1 still here\n 'SELECT * FROM orders WHERE id = $1', [orderId]\n );\n}\ntry {\n await email.send(user.email, 'Payment confirmed');\n} catch (err) {\n logger.warn('email failed'); // logged but thin\n}"
|
||||
},
|
||||
{
|
||||
"label": "C: WebhookDispatcher integration",
|
||||
"description": "Route through the existing WebhookDispatcher module instead of bypassing it. Includes all of B's security and test fixes. Keeps one webhook-handling surface. Effort: human ~1d / CC ~30min. Completeness: 10/10.",
|
||||
"preview": "// Approach C shape \u2014 extend existing dispatcher\n// In WebhookDispatcher:\nregister('payment_intent.succeeded', StripePaymentHandler);\n\n// StripePaymentHandler reuses dispatcher's\n// routing, logging, and error contract \u2014\n// plus all Approach B fixes."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D1 \u2014 Which implementation approach for the payment webhook handler? <gstack-qid:plan-ceo-review-approach>": "B: Correct implementation (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:26:07.550Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "4cd0a135-d63c-488a-ae90-e0f2f4128675",
|
||||
"toolUseId": "toolu_017YoVavTgiZtN5Lb1LLJuMY",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D2 \u2014 Which review mode for this payment webhook plan? <gstack-qid:plan-ceo-review-mode>",
|
||||
"header": "Review mode",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "HOLD SCOPE (Recommended)",
|
||||
"description": "The plan has 2 critical gaps (SQL injection, email failures). Hold scope exactly as stated; make it bulletproof. No expansions surfaced. Best for a plan with known critical defects to fix first.",
|
||||
"preview": "HOLD SCOPE:\n- Fix SQL injection (parameterized query)\n- Fix email error handling (try/catch + logging)\n- Fix N+1 (batch query)\n- Add full test suite\n- Document WebhookDispatcher bypass rationale\nNo scope additions."
|
||||
},
|
||||
{
|
||||
"label": "SELECTIVE EXPANSION",
|
||||
"description": "Hold the current scope as baseline, but also surface expansion opportunities (async email via job queue, WebhookDispatcher integration, payment audit log) as individual cherry-picks. Effort adds ~30\u201360min CC per cherry-pick.",
|
||||
"preview": "SELECTIVE EXPANSION:\n- All HOLD SCOPE fixes (required)\n- OPTIONAL cherry-picks:\n \u2022 Async email via job queue (resilient)\n \u2022 WebhookDispatcher integration (clean arch)\n \u2022 Payment audit log (compliance)\n \u2022 Payment success metric/dashboard (observability)"
|
||||
},
|
||||
{
|
||||
"label": "SCOPE REDUCTION",
|
||||
"description": "Strip to the absolute minimum: just the user record update on payment success, no email, no order fetch. Defer everything else. Useful if timeline is critical and email can ship later.",
|
||||
"preview": "SCOPE REDUCTION:\n- User record update only on payment success\n- No email (deferred to follow-up PR)\n- No order fetch loop (deferred)\n- Parameterized query (still required)\n- Minimal smoke test"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D2 \u2014 Which review mode for this payment webhook plan? <gstack-qid:plan-ceo-review-mode>": "HOLD SCOPE (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:26:23.611Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "4cd0a135-d63c-488a-ae90-e0f2f4128675",
|
||||
"toolUseId": "toolu_01LPiLGS6RKcERL9KmQ6dXpH",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D3 \u2014 SQL injection in user lookup: the plan uses request.params.userId directly in a raw SQL fragment. The plan itself notes this string is not sanitized. How should the plan be fixed? <gstack-qid:plan-ceo-review-sql-injection>",
|
||||
"header": "SQL injection",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "3A: Parameterized query (Recommended)",
|
||||
"description": "Replace the raw SQL fragment with a parameterized query binding userId to a $1 placeholder. Eliminates SQL injection entirely. The plan's 'existing contracts retained' section already acknowledges the string is unsafe for SQL \u2014 the fix is a one-liner. Verify: unit test with userId = '1 OR 1=1' asserts the handler rejects/escapes it, not executes it. Completeness: 10/10.",
|
||||
"preview": "// BEFORE (SQL injection):\ndb.query(`SELECT * FROM users WHERE id = ${userId}`);\n\n// AFTER (parameterized \u2014 injection impossible):\ndb.query('SELECT * FROM users WHERE id = $1', [userId]);\n\n// Test:\ntest('rejects injected userId', async () => {\n const result = await handler({ userId: '1 OR 1=1' });\n expect(result.status).not.toBe(200); // or assert no SQL exec\n});"
|
||||
},
|
||||
{
|
||||
"label": "3B: Input validation before SQL",
|
||||
"description": "Validate userId is a positive integer before the DB call; reject the webhook with HTTP 400 if invalid. Adds defense-in-depth but is not sufficient alone (validation can be bypassed; parameterized queries cannot). Use alongside 3A, not instead of it. Completeness: 7/10 if standalone.",
|
||||
"preview": "// Input validation (good, but not a substitute for parameterized queries)\nconst numericId = parseInt(userId, 10);\nif (isNaN(numericId) || numericId <= 0) {\n logger.warn({ userId }, 'invalid userId in webhook');\n return { status: 400 };\n}\n// Then STILL use parameterized query below"
|
||||
},
|
||||
{
|
||||
"label": "3C: Do nothing \u2014 accept the risk",
|
||||
"description": "Leave the raw SQL as-is. The Stripe signature verification reduces (but does not eliminate) the attack surface. Not recommended: any actor who can craft a Stripe payment intent with a malicious metadata.user_id can trigger this, including test-mode API keys or compromised merchant accounts. Completeness: 3/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D3 \u2014 SQL injection in user lookup: the plan uses request.params.userId directly in a raw SQL fragment. The plan itself notes this string is not sanitized. How should the plan be fixed? <gstack-qid:plan-ceo-review-sql-injection>": "3A: Parameterized query (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:26:41.687Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "4cd0a135-d63c-488a-ae90-e0f2f4128675",
|
||||
"toolUseId": "toolu_01VaDTCokqQ8ioX2rGHPGqwm",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D4 \u2014 The plan has zero tests for a payment-critical handler. How should this be addressed? <gstack-qid:plan-ceo-review-tests>",
|
||||
"header": "Tests",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "4A: Full test suite (Recommended)",
|
||||
"description": "Add unit tests for the handler (happy path, missing user, injected userId, email failure) AND one integration test that replays a real Stripe payload through the full stack and asserts user record updated + email logged. With CC this takes ~15 minutes to implement. This is the test you'd need to ship at 2am on a Friday. Completeness: 10/10.",
|
||||
"preview": "// Unit: happy path\ntest('updates user record on payment success', ...);\n// Unit: SQL injection guard\ntest('rejects injected userId', ...);\n// Unit: missing user\ntest('returns 200 and logs when user not found', ...);\n// Unit: email failure is non-fatal\ntest('returns 200 even when email send throws', ...);\n// Integration: full Stripe payload replay\ntest('processes payment_intent.succeeded end-to-end', ...);"
|
||||
},
|
||||
{
|
||||
"label": "4B: Smoke tests only",
|
||||
"description": "Add 2 smoke tests: one happy-path integration test, one for the SQL injection guard. Skips edge cases (email failure, missing user, N+1 regression). Faster but leaves gaps. Completeness: 6/10."
|
||||
},
|
||||
{
|
||||
"label": "4C: Defer to existing integration suite",
|
||||
"description": "Keep the plan as-is: rely on the existing integration suite to catch regressions. The plan states this explicitly. Risk: the existing suite likely doesn\u2019t know about this new handler\u2019s specific failure modes (SQL injection, email failure cascade, N+1). Not recommended. Completeness: 3/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D4 \u2014 The plan has zero tests for a payment-critical handler. How should this be addressed? <gstack-qid:plan-ceo-review-tests>": "4A: Full test suite (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:26:55.745Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "4cd0a135-d63c-488a-ae90-e0f2f4128675",
|
||||
"toolUseId": "toolu_01APtZYudKo1F7n3o9cvZxxx",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D5 \u2014 Email send fires inline with no error handling. A provider timeout holds the transaction lock and returns 500 to Stripe (triggering retries). How should this be fixed? <gstack-qid:plan-ceo-review-email-error>",
|
||||
"header": "Email errors",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "5A: Try/catch with structured logging (Recommended)",
|
||||
"description": "Wrap the email call in try/catch. On failure: log structured error (userId, eventId, error class), and return 200 \u2014 the payment succeeded; the email is non-fatal. Add an explicit timeout to the email call (e.g., 5s) so provider slowness doesn\u2019t hold the transaction lock. Verify: unit test asserts handler returns 200 when email throws. Completeness: 9/10.",
|
||||
"preview": "try {\n await email.sendWithTimeout(\n user.email,\n 'Payment confirmed',\n { timeoutMs: 5000 } // explicit timeout\n );\n} catch (err) {\n logger.error(\n { err, userId, eventId, errorClass: err.constructor.name },\n 'notification email failed \u2014 payment succeeded'\n );\n // return 200 below; payment is confirmed, email is non-fatal\n}"
|
||||
},
|
||||
{
|
||||
"label": "5B: Async email via job queue",
|
||||
"description": "Enqueue an email job instead of sending inline. The webhook returns 200 immediately after the DB update; email delivery is decoupled and retried independently. Fully resilient but adds a job queue dependency (may already exist). Completeness: 10/10, but larger scope change."
|
||||
},
|
||||
{
|
||||
"label": "5C: Leave inline, no error handling",
|
||||
"description": "Keep email send inline with no catch. Accept that email provider failures will cause 500s and Stripe retries. The dedup guard should prevent duplicate DB updates on retry (if transaction scope is correct). Not recommended: silent email failures are never acceptable in a payment flow. Completeness: 3/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D5 \u2014 Email send fires inline with no error handling. A provider timeout holds the transaction lock and returns 500 to Stripe (triggering retries). How should this be fixed? <gstack-qid:plan-ceo-review-email-error>": "5A: Try/catch with structured logging (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:27:09.805Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "4cd0a135-d63c-488a-ae90-e0f2f4128675",
|
||||
"toolUseId": "toolu_012zc42MrWewJHLB6yMzzbcx",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D6 \u2014 The plan fetches orders in a per-order loop (N+1 queries). Under concurrent webhook volume this exhausts DB connections. How should this be fixed? <gstack-qid:plan-ceo-review-n1-query>",
|
||||
"header": "N+1 queries",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "6A: Single batch query (Recommended)",
|
||||
"description": "Replace the per-order loop with one parameterized batch query: SELECT * FROM orders WHERE user_id = $1. Reduces O(N) queries to O(1). Verify: confirm an index exists on orders.user_id; if not, add one in the same migration. Assert in tests that only 3 DB queries fire per webhook (lookup, batch orders, update). Completeness: 10/10.",
|
||||
"preview": "// BEFORE (N+1):\nfor (const orderId of orderIds) {\n const order = await db.query(\n 'SELECT * FROM orders WHERE id = $1', [orderId]\n );\n}\n\n// AFTER (1 batch query):\nconst orders = await db.query(\n 'SELECT * FROM orders WHERE user_id = $1',\n [userId] // parameterized\n);\n// + confirm index: CREATE INDEX IF NOT EXISTS\n// idx_orders_user_id ON orders(user_id);"
|
||||
},
|
||||
{
|
||||
"label": "6B: Keep the loop, add a DB index",
|
||||
"description": "Keep the per-order fetch loop but ensure an index exists on orders.user_id. Reduces full-table scans but doesn\u2019t eliminate the N+1 round-trips. Still O(N) queries. Completeness: 6/10."
|
||||
},
|
||||
{
|
||||
"label": "6C: Accept as-is, defer optimization",
|
||||
"description": "Leave the N+1 loop and note it as a known performance issue for later. Acceptable only if order counts per user are guaranteed to be very low (e.g., \u22645) AND webhook volume is low. The plan gives no such guarantee. Completeness: 3/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D6 \u2014 The plan fetches orders in a per-order loop (N+1 queries). Under concurrent webhook volume this exhausts DB connections. How should this be fixed? <gstack-qid:plan-ceo-review-n1-query>": "6A: Single batch query (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:27:21.854Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "4cd0a135-d63c-488a-ae90-e0f2f4128675",
|
||||
"toolUseId": "toolu_015BU81bzFBuuDhD1uQu3pN2",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D7 \u2014 CEO review complete. Eng review is the required shipping gate \u2014 what next? <gstack-qid:plan-ceo-review-next-step>",
|
||||
"header": "Next step",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Run /plan-eng-review (Recommended)",
|
||||
"description": "This CEO review found 2 critical gaps (SQL injection, N+1) that directly affect architecture, security, and performance. Eng review goes deeper on implementation correctness, test coverage, and DB safety. Required gate before shipping.",
|
||||
"preview": "Findings from this CEO review that eng review will verify:\n- T1: Parameterized query (SQL injection fix)\n- T2: Full test suite (5 cases)\n- T3: Email try/catch with timeout\n- T4: Batch orders query + index\n- T5: WebhookDispatcher doc/resolve\n\nEng review gates: architecture, security,\ntests, performance, observability."
|
||||
},
|
||||
{
|
||||
"label": "Implement now, eng review later",
|
||||
"description": "Proceed to implementation with the 5 approved tasks (T1-T5). Run /plan-eng-review before the PR is merged. Acceptable if implementation is expected to be fast with CC.",
|
||||
"preview": "Implement in order:\n1. T1: Parameterized query (blocks everything)\n2. T3: Email try/catch\n3. T4: Batch orders query + index\n4. T2: Full test suite\n5. T5: WebhookDispatcher doc\nThen run /plan-eng-review on the diff."
|
||||
},
|
||||
{
|
||||
"label": "Skip \u2014 handle manually",
|
||||
"description": "No further automated reviews. The 5 tasks are documented; the team will implement and review manually."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D7 \u2014 CEO review complete. Eng review is the required shipping gate \u2014 what next? <gstack-qid:plan-ceo-review-next-step>": "Run /plan-eng-review (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:30:12.250Z"
|
||||
}
|
||||
],
|
||||
"expectedSetupCount": 2,
|
||||
"expectedReviewCount": 4,
|
||||
"reportAtMs": 1788906562505.1587,
|
||||
"planReadyRequests": [
|
||||
{
|
||||
"sessionId": "4cd0a135-d63c-488a-ae90-e0f2f4128675",
|
||||
"toolUseId": "toolu_01S23dJSitXZPX79TyzcZCBV",
|
||||
"timestamp": "2026-09-08T22:30:26.736Z",
|
||||
"failed": false
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "plan-ceo-review-1788905904654-k2LGUy",
|
||||
"calls": [
|
||||
{
|
||||
"sessionId": "eb433057-4f1e-447c-8393-6484749cc3d1",
|
||||
"toolUseId": "toolu_014ZdrCijtgGzSGYMbZaFRCo",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Which implementation approach for processPayment() test coverage?\n\nELI10: The plan specifies exactly 2 unit tests: happy path (Stripe succeeds \u2192 receipt generated) and 502-exhaustion (Stripe 502 \u2192 retry once \u2192 clean failure). A third natural scenario exists \u2014 retry-then-success (Stripe 502 \u2192 retry \u2192 Stripe succeeds \u2192 correct receipt) \u2014 which the adapter suite already models but processPayment() has no unit test for. The question is how much test coverage to write while you're in the code.\n\nStakes if we pick wrong: Too narrow leaves the retry-then-success orchestration untested, risking silent regressions on the recovery path. Too wide duplicates adapter-suite coverage without adding new signal.\n\nRecommendation: B because it closes the one genuinely missing orchestration scenario for ~10 extra lines, without duplicating adapter-boundary tests.\n\nCompleteness: A=7/10, B=9/10, C=10/10 (C has diminishing returns due to 402/429 adapter overlap)\n\n<gstack-qid:plan-ceo-review-approach>",
|
||||
"header": "Approach",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "B) Standard \u2014 3 tests (Recommended)",
|
||||
"description": "Add the 2 specified tests plus a third: retry-then-success (502 \u2192 retry \u2192 charge succeeds \u2192 correct receipt). Uses the same infrastructure the adapter suite already uses for this scenario. Effort: human ~40 min / CC ~3 min. Completeness: 9/10."
|
||||
},
|
||||
{
|
||||
"label": "A) Minimal \u2014 exactly 2 tests as specified",
|
||||
"description": "Happy path + 502-exhaustion only. Smallest possible diff, zero scope creep, uses existing factory/sleeper. The retry-then-success path remains untested at processPayment() level. Effort: human ~30 min / CC ~2 min. Completeness: 7/10."
|
||||
},
|
||||
{
|
||||
"label": "C) Full \u2014 5 tests (all Stripe outcomes)",
|
||||
"description": "Happy path, 502-exhaust, retry-then-success, 402 decline, 429 rate-limit \u2014 all tested at the processPayment() level. Maximum confidence; eliminates all coverage questions permanently. Risk: 402/429 may duplicate adapter-suite tests without new signal. Effort: human ~75 min / CC ~5 min. Completeness: 10/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D1 \u2014 Which implementation approach for processPayment() test coverage?\n\nELI10: The plan specifies exactly 2 unit tests: happy path (Stripe succeeds \u2192 receipt generated) and 502-exhaustion (Stripe 502 \u2192 retry once \u2192 clean failure). A third natural scenario exists \u2014 retry-then-success (Stripe 502 \u2192 retry \u2192 Stripe succeeds \u2192 correct receipt) \u2014 which the adapter suite already models but processPayment() has no unit test for. The question is how much test coverage to write while you're in the code.\n\nStakes if we pick wrong: Too narrow leaves the retry-then-success orchestration untested, risking silent regressions on the recovery path. Too wide duplicates adapter-suite coverage without adding new signal.\n\nRecommendation: B because it closes the one genuinely missing orchestration scenario for ~10 extra lines, without duplicating adapter-boundary tests.\n\nCompleteness: A=7/10, B=9/10, C=10/10 (C has diminishing returns due to 402/429 adapter overlap)\n\n<gstack-qid:plan-ceo-review-approach>": "B) Standard \u2014 3 tests (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:22:14.665Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "eb433057-4f1e-447c-8393-6484749cc3d1",
|
||||
"toolUseId": "toolu_0178W4ZyWc5QA9ioQqRfiAcN",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D2 \u2014 Which review mode for this test coverage plan?\n\nELI10: Approach B is locked in (3 tests: happy path, 502-exhaust, retry-then-success). The review mode decides HOW rigorously I examine that scope. HOLD SCOPE means I focus entirely on making those 3 tests bulletproof \u2014 catching every ambiguity about what to assert, how to name tests, what 'fails clean' means, edge cases in the retry path. SELECTIVE EXPANSION means I do that rigorous review AND surface cherry-pick opportunities like additional structural improvements or assertion depth upgrades.\n\nStakes if we pick wrong: Too expansive turns a simple 3-test addition into a bigger refactor. Too narrow misses critical specification gaps (what exactly should be asserted in each test).\n\nRecommendation: HOLD SCOPE because the plan is already well-scoped at 3 tests; the highest value is making those tests precise \u2014 specifying exact assertions, defining 'fails clean', confirming backoff is asserted and not just recorded.\n\nNote: options differ in kind, not coverage \u2014 no completeness score.\n\n<gstack-qid:plan-ceo-review-mode>",
|
||||
"header": "Review Mode",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "HOLD SCOPE (Recommended)",
|
||||
"description": "Take the 3-test scope as final. Review with maximum rigor: catch every assertion gap, specify what 'fails clean' means, validate the backoff assertion, define the receipt fields under test. Make each test bulletproof before implementation starts."
|
||||
},
|
||||
{
|
||||
"label": "SELECTIVE EXPANSION",
|
||||
"description": "Hold the 3-test baseline AND surface cherry-pick opportunities: e.g., test naming conventions, shared setup helpers, assertion libraries, or a 4th edge case. Each expansion is individually opt-in. More expansive but still user-controlled."
|
||||
},
|
||||
{
|
||||
"label": "SCOPE REDUCTION",
|
||||
"description": "Pull back to the original 2 tests (drop retry-then-success). Strictest possible minimum. Only if you feel approach B overshot."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D2 \u2014 Which review mode for this test coverage plan?\n\nELI10: Approach B is locked in (3 tests: happy path, 502-exhaust, retry-then-success). The review mode decides HOW rigorously I examine that scope. HOLD SCOPE means I focus entirely on making those 3 tests bulletproof \u2014 catching every ambiguity about what to assert, how to name tests, what 'fails clean' means, edge cases in the retry path. SELECTIVE EXPANSION means I do that rigorous review AND surface cherry-pick opportunities like additional structural improvements or assertion depth upgrades.\n\nStakes if we pick wrong: Too expansive turns a simple 3-test addition into a bigger refactor. Too narrow misses critical specification gaps (what exactly should be asserted in each test).\n\nRecommendation: HOLD SCOPE because the plan is already well-scoped at 3 tests; the highest value is making those tests precise \u2014 specifying exact assertions, defining 'fails clean', confirming backoff is asserted and not just recorded.\n\nNote: options differ in kind, not coverage \u2014 no completeness score.\n\n<gstack-qid:plan-ceo-review-mode>": "HOLD SCOPE (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:22:44.754Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "eb433057-4f1e-447c-8393-6484749cc3d1",
|
||||
"toolUseId": "toolu_018Whc7SViMfHtat7rdpPLFu",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D3 \u2014 Section 2: Should the plan specify the error contract for \"fails clean\"?\n\nELI10: The 502-exhaustion test needs to assert something specific when processPayment() gives up after one retry. The plan says it should \"fail clean\" but doesn't say what that means in code: does processPayment() raise a named exception (e.g. PaymentGatewayError), return nil, or return an error result object? Without this, the implementer might write an assertion that passes trivially \u2014 for instance, asserting that the return value is nil when the function actually raises, meaning the rescue path is never exercised.\n\nStakes if we pick wrong: An underspecified assertion could let a broken retry path pass the test silently.\n\nRecommendation: A \u2014 add a one-line note to the plan naming the error contract (\"processPayment() raises X\" or \"returns nil\"), so the test asserts the right thing. This is one sentence of spec, not implementation work.\n\nNote: options differ in kind, not coverage \u2014 no completeness score.\n\n<gstack-qid:plan-ceo-review-fails-clean>",
|
||||
"header": "Error contract",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Add spec: name the error contract (Recommended)",
|
||||
"description": "Add a brief note to the plan specifying what processPayment() does on retry exhaustion (raises SpecificError / returns nil / returns error object). Implementer then asserts the correct thing. One sentence."
|
||||
},
|
||||
{
|
||||
"label": "B) Leave as-is: implementer reads the source",
|
||||
"description": "The production code already has a defined behavior. The implementer will look at processPayment() and write the correct assertion without the plan needing to spell it out."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D3 \u2014 Section 2: Should the plan specify the error contract for \"fails clean\"?\n\nELI10: The 502-exhaustion test needs to assert something specific when processPayment() gives up after one retry. The plan says it should \"fail clean\" but doesn't say what that means in code: does processPayment() raise a named exception (e.g. PaymentGatewayError), return nil, or return an error result object? Without this, the implementer might write an assertion that passes trivially \u2014 for instance, asserting that the return value is nil when the function actually raises, meaning the rescue path is never exercised.\n\nStakes if we pick wrong: An underspecified assertion could let a broken retry path pass the test silently.\n\nRecommendation: A \u2014 add a one-line note to the plan naming the error contract (\"processPayment() raises X\" or \"returns nil\"), so the test asserts the right thing. This is one sentence of spec, not implementation work.\n\nNote: options differ in kind, not coverage \u2014 no completeness score.\n\n<gstack-qid:plan-ceo-review-fails-clean>": "A) Add spec: name the error contract (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:25:13.296Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "eb433057-4f1e-447c-8393-6484749cc3d1",
|
||||
"toolUseId": "toolu_017ijdwbrLBaXiy7rsoKC3vm",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D4 \u2014 Section 6: Should the plan explicitly require asserting Stripe call count for retry tests?\n\nELI10: The 502-exhaustion test and the retry-then-success test both involve a retry. The plan says the test should \"assert retry-with-backoff fires once\" \u2014 but without explicitly asserting the Stripe call count (expect 2 calls via mock call history), a broken implementation that skips the retry entirely would still pass if it produces the right final outcome. The factory already exposes the mock call history, so this assertion costs one line per test.\n\nStakes if we pick wrong: Skip it \u2014 a future refactor that accidentally removes the retry logic could pass both tests if it happens to produce the same final failure/receipt. Add it \u2014 the tests are more brittle (they now break if retry count changes) but correctly verify the retry behavior.\n\nRecommendation: A \u2014 the plan explicitly states \"retry-with-backoff fires once,\" which implies the retry count IS the behavior under test. Asserting it makes that intent machine-verifiable. One line per test, already free via factory.\n\nNote: options differ in kind, not coverage \u2014 no completeness score.\n\n<gstack-qid:plan-ceo-review-retry-count>",
|
||||
"header": "Retry assertion",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Add to plan: assert Stripe call count in both retry tests (Recommended)",
|
||||
"description": "The 502-exhaustion and retry-then-success tests should each assert the exact Stripe call count (2) via the mock call history. One line per test. Verifies the retry actually executed, not just the final outcome."
|
||||
},
|
||||
{
|
||||
"label": "B) Leave as-is: assert outcome only",
|
||||
"description": "Tests assert the final state (failure/receipt) only. Retry behavior is tested at the adapter suite level. Simpler tests, but a retry regression could pass silently."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D4 \u2014 Section 6: Should the plan explicitly require asserting Stripe call count for retry tests?\n\nELI10: The 502-exhaustion test and the retry-then-success test both involve a retry. The plan says the test should \"assert retry-with-backoff fires once\" \u2014 but without explicitly asserting the Stripe call count (expect 2 calls via mock call history), a broken implementation that skips the retry entirely would still pass if it produces the right final outcome. The factory already exposes the mock call history, so this assertion costs one line per test.\n\nStakes if we pick wrong: Skip it \u2014 a future refactor that accidentally removes the retry logic could pass both tests if it happens to produce the same final failure/receipt. Add it \u2014 the tests are more brittle (they now break if retry count changes) but correctly verify the retry behavior.\n\nRecommendation: A \u2014 the plan explicitly states \"retry-with-backoff fires once,\" which implies the retry count IS the behavior under test. Asserting it makes that intent machine-verifiable. One line per test, already free via factory.\n\nNote: options differ in kind, not coverage \u2014 no completeness score.\n\n<gstack-qid:plan-ceo-review-retry-count>": "A) Add to plan: assert Stripe call count in both retry tests (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:25:55.466Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "eb433057-4f1e-447c-8393-6484749cc3d1",
|
||||
"toolUseId": "toolu_015gSYhTd9gkZe3N4gSMFAmx",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D5 \u2014 Next step: which review runs after this CEO review?\n\nELI10: The CEO review is done and the plan is cleared. The required shipping gate is an Eng Review (/plan-eng-review), which checks architecture, test structure, and deployment safety at a deeper technical level. No eng review has been run yet for this plan. No UI scope was detected, so a design review is not applicable.\n\nNote: options differ in kind, not coverage \u2014 no completeness score.\n\n<gstack-qid:plan-ceo-review-next-step>",
|
||||
"header": "Next review",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Run /plan-eng-review next (Recommended)",
|
||||
"description": "Required shipping gate. Covers architecture, test coverage gaps, performance, and deployment safety at a technical depth beyond this CEO review. Not run yet."
|
||||
},
|
||||
{
|
||||
"label": "Skip \u2014 I'll handle reviews manually",
|
||||
"description": "Proceed without running /plan-eng-review now. Note: eng review is required before shipping; it can be run separately at any time."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D5 \u2014 Next step: which review runs after this CEO review?\n\nELI10: The CEO review is done and the plan is cleared. The required shipping gate is an Eng Review (/plan-eng-review), which checks architecture, test structure, and deployment safety at a deeper technical level. No eng review has been run yet for this plan. No UI scope was detected, so a design review is not applicable.\n\nNote: options differ in kind, not coverage \u2014 no completeness score.\n\n<gstack-qid:plan-ceo-review-next-step>": "Run /plan-eng-review next (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T22:30:38.373Z"
|
||||
}
|
||||
],
|
||||
"expectedSetupCount": 2,
|
||||
"expectedReviewCount": 2,
|
||||
"reportAtMs": 1788906596661.16,
|
||||
"planReadyRequests": [
|
||||
{
|
||||
"sessionId": "eb433057-4f1e-447c-8393-6484749cc3d1",
|
||||
"toolUseId": "toolu_01RKhgxR5f4pxachRK972M3t",
|
||||
"timestamp": "2026-09-08T22:30:44.782Z",
|
||||
"failed": false
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -1,11 +1,54 @@
|
||||
import { capturePlanCountQuestion, planCountQuestionInput } from './claude-pty-runner';
|
||||
import { capturePlanCountQuestion, planCountPrerequisitePick, planCountQuestionInput } from './claude-pty-runner';
|
||||
|
||||
import type { NativePlanQuestionCall } from './plan-count-transcript';
|
||||
|
||||
/** Answer only gstack's routing setup prompt in the isolated autoplan fixture. */
|
||||
/** A copied native panel is not actionable after prose or inside a code example. */
|
||||
function activePrerequisitePanel(visible: string): boolean {
|
||||
const lines = visible.replace(/\r+\n?/g, '\n').trimEnd().split('\n');
|
||||
if (!/^Enter[\t ]+to[\t ]+select[\t ]*·[\t ]*↑\/↓[\t ]+to[\t ]+navigate[\t ]*·[\t ]*Esc[\t ]+to[\t ]+cancel$/i.test(lines.at(-1)?.trim() ?? '')) return false;
|
||||
const header = lines.findLastIndex(line => /^ {0,3}[☐□][^\n]+$/.test(line));
|
||||
if (header < 0) return false;
|
||||
let fence: { char: string; length: number } | undefined;
|
||||
for (const line of lines.slice(0, header)) {
|
||||
const match = /^ {0,3}(`{3,}|~{3,})(.*)$/.exec(line);
|
||||
if (!match) continue;
|
||||
if (!fence) fence = { char: match[1]![0]!, length: match[1]!.length };
|
||||
else if (match[1]![0] === fence.char && match[1]!.length >= fence.length && !match[2]!.trim()) fence = undefined;
|
||||
}
|
||||
if (fence) return false;
|
||||
const introduction = lines.slice(0, header).findLast(line => !/^[\t ─━-]*$/.test(line)) ?? '';
|
||||
return !/\b(?:example|sample|quot(?:e[sd]?|ed)|template|source)\b.*\b(?:panel|menu|choices?|prompt|question|below|following)\b/i.test(introduction);
|
||||
}
|
||||
|
||||
/** Handle routing and the optional design-doc prerequisite in this isolated fixture. */
|
||||
export function autoplanRoutingSetupInput(visible: string, seen: Set<string>, pending?: NativePlanQuestionCall): string | null {
|
||||
const question = capturePlanCountQuestion(visible, seen, 0, true, pending);
|
||||
// Box borders are terminal decoration, not part of an untagged native
|
||||
// question's wrapped text. Keep its full content for identity matching.
|
||||
const display = visible.replace(/(^|[\r\n])[\t ]*[│┃][\t ]?/g, '$1');
|
||||
// A rejected/mismatched menu has received no input. Keep its identities
|
||||
// available when the actual native call arrives after the visible prompt.
|
||||
const captured = new Set(seen);
|
||||
const question = capturePlanCountQuestion(display, captured, 0, true, pending);
|
||||
if (!question) return null;
|
||||
const answered = (input: string | null) => {
|
||||
if (input !== null) for (const signature of captured) seen.add(signature);
|
||||
return input;
|
||||
};
|
||||
|
||||
const prerequisite = planCountPrerequisitePick(question);
|
||||
if (prerequisite !== null) {
|
||||
// The canonical offer has two opposed choices. A known native call must
|
||||
// match this one-question panel; no mixed packet or substantive choice
|
||||
// may borrow its skip. Before JSONL flushes, require the complete native
|
||||
// single-select UI and the existing prerequisite premise/action guard.
|
||||
const choices = question.options.filter(option =>
|
||||
!/^(?:Type something\.|Chat about this)$/.test(option.label));
|
||||
if (choices.length !== 2 || (pending &&
|
||||
(!question.nativeCall || pending.questions.length !== 1 || pending.questions[0]?.multiSelect))) return null;
|
||||
const input = planCountQuestionInput(display, question, prerequisite);
|
||||
if (!/^[1-9]\d*$/.test(input ?? '') || !activePrerequisitePanel(display)) return null;
|
||||
return answered(input);
|
||||
}
|
||||
|
||||
// The model rephrases the setup question's closing sentence. Its routing
|
||||
// identity/premise and two opposed setup actions establish what is being
|
||||
@@ -46,5 +89,5 @@ export function autoplanRoutingSetupInput(visible: string, seen: Set<string>, pe
|
||||
const claudeTarget = /CLAUDE\.md/i.test(prompt);
|
||||
const quotedPremise = /\b(?:plan|spec|document)\s+(?:quotes?|cites?|references?)\b/i.test(primary);
|
||||
if (!claudeTarget || quotedPremise || (!routingId && !routingPremise)) return null;
|
||||
return planCountQuestionInput(visible, question, add[0]!.index);
|
||||
return answered(planCountQuestionInput(display, question, add[0]!.index));
|
||||
}
|
||||
@@ -12,19 +12,23 @@ function manualHandoffIndex(fp: AskUserQuestionFingerprint): number | null {
|
||||
if (id && !/^plan-ceo-(?:review-)?next-(?:steps?|review)$/.test(id)) return null;
|
||||
const declaration = q.question.replace(/^D\s*\d+\s*[—–:-]\s*/i, '')
|
||||
.replace(/^next\s+(?:review|steps?)\s*:\s*/i, '');
|
||||
const completion = /(?:^|[.!?]\s+)(?:The\s+)?CEO\s+review\s+(?:is\s+)?(?:complete|cleared|clean)(?=\s*(?:[.!?—–]|$))/i.test(declaration);
|
||||
const completion = /(?:^|[.!?]\s+)(?:ELI10:\s*)?(?:The\s+)?CEO\s+review\s+(?:is\s+)?(?:complete|cleared|clean|done(?:\s+and\s+the\s+plan\s+is\s+cleared)?)(?=\s*(?:[.!?—–]|$))/i.test(declaration);
|
||||
const gateContext = [q.question, ...q.options.map(option => option.description ?? '')].join('\n');
|
||||
const requiredEng = /(?:\bEng(?:ineering)?\s+review|\/plan-eng-review)\b[^.!?]{0,180}\brequired(?:\s+shipping)?\s+gate\b/i.test(gateContext);
|
||||
const requiredEng = /(?:\bEng(?:ineering)?\s+review|\/plan-eng-review)\b[^.!?]{0,180}\brequired(?:\s+shipping)?\s+gate\b/i.test(gateContext) ||
|
||||
/\brequired(?:\s+shipping)?\s+gate\s+is\s+(?:an?\s+)?(?:Eng(?:ineering)?\s+review|\/plan-eng-review)\b/i.test(gateContext);
|
||||
// A qid names the menu; it cannot replace its completed-review declaration
|
||||
// or authorize another fix. The named gate can be explained in a choice.
|
||||
if (!/^next\s+(?:review|steps?)$/i.test(q.header.trim()) || !completion || !requiredEng) return null;
|
||||
|
||||
const labels = q.options.map(o => o.label.trim().replace(/^[A-Z][).]\s*/i, '').replace(/\s*\(recommended\)\s*$/i, '').trim());
|
||||
const runs = labels.map(label => /^Run\s+\/plan-(?:eng|design)-review\s+(?:next|now)(?:\s*\(required gate\))?$/i.test(label));
|
||||
const manual = labels.map(label => /^(?:Skip|Done)\s*[—–-]\s*(?:I['’]ll\s+)?handle\s+reviews\s+manually$/i.test(label));
|
||||
// Every choice must be administrative. Adding a fix/TODO/build option
|
||||
// makes the whole call substantive, even if it also offers a next review.
|
||||
if (!runs.some(Boolean) || manual.filter(Boolean).length !== 1 || !labels.every((_, i) => runs[i] || manual[i])) return null;
|
||||
const runs = labels.map(label => /^Run\s+\/plan-(?:eng|design)-review(?:\s+(?:next|now))?(?:\s*\(required gate\))?$/i.test(label));
|
||||
const manual = labels.map(label => /^(?:Skip|Done)\s*[—–-]\s*(?:I['’]ll\s+)?handle\s+(?:reviews\s+)?manually$/i.test(label));
|
||||
// Deferring the next review until after already-approved implementation is
|
||||
// navigation too. A new fix/TODO/task choice remains substantive. The picker
|
||||
// always selects manual, never this implementation route.
|
||||
const deferred = labels.map((label, i) => /^Implement\s+now,\s+eng\s+review\s+later$/i.test(label) &&
|
||||
/^Proceed to implementation with (?:the )?(?:\d+ )?(?:already )?approved (?:tasks|plan|changes)(?: \([A-Z0-9–-]+\))?\. Run \/plan-eng-review before (?:the PR is merged|shipping)\.(?: Acceptable if implementation is expected to be fast with CC\.)?$/i.test(q.options[i]!.description?.trim() ?? ''));
|
||||
if (!runs.some(Boolean) || manual.filter(Boolean).length !== 1 || !labels.every((_, i) => runs[i] || manual[i] || deferred[i])) return null;
|
||||
return manual.findIndex(Boolean) + 1;
|
||||
}
|
||||
|
||||
|
||||
@@ -31,6 +31,7 @@ import { withHermeticSkillRuntime } from './hermetic-skill-runtime';
|
||||
import { createPlanCountFixture } from './plan-count-fixture';
|
||||
import { createPlanCountSnapshotWriter } from './plan-count-artifacts';
|
||||
import { readPlanCountTranscript, unresolvedPlanQuestionCalls, type NativePlanQuestionCall, type PlanCountTranscript } from './plan-count-transcript';
|
||||
import { createPendingExitRecorder, withPendingExit } from './plan-count-pending-exit';
|
||||
import { trustDialogInput } from './pty-trust-dialog';
|
||||
import { createPtyScreen } from './pty-screen';
|
||||
|
||||
@@ -98,6 +99,8 @@ export interface ClaudePtyOptions {
|
||||
rows?: number;
|
||||
/** Opt in only when input targeting needs the actual VT viewport. */
|
||||
observeScreen?: boolean;
|
||||
/** Count-only pending identity; the hook never approves or changes native tools. */
|
||||
observePlanReady?: boolean;
|
||||
/** Working directory. Default: process.cwd(). The repo cwd has the gstack
|
||||
* skill registry and trusted-folder cookie, so most tests want this. */
|
||||
cwd?: string;
|
||||
@@ -154,6 +157,8 @@ export interface ClaudePtySession {
|
||||
* the dir name ends in `/.claude` by contract).
|
||||
*/
|
||||
hermeticConfigDir: string | null;
|
||||
/** Owned pre-tool identity record, removed by close(). */
|
||||
pendingPlanReadyFile?: string;
|
||||
/**
|
||||
* Send SIGINT, then SIGKILL after 1s. Always safe to call multiple times.
|
||||
* Awaits process exit before resolving.
|
||||
@@ -1984,7 +1989,13 @@ export async function launchClaudePty(
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
let proc: any;
|
||||
try { proc = (Bun as any).spawn([claudePath, ...args], {
|
||||
let pendingExit: ReturnType<typeof createPendingExitRecorder> | undefined;
|
||||
try {
|
||||
if (opts.observePlanReady && hermetic && childEnv.CLAUDE_CONFIG_DIR) {
|
||||
pendingExit = createPendingExitRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR);
|
||||
args.push('--settings', pendingExit.settings);
|
||||
}
|
||||
proc = (Bun as any).spawn([claudePath, ...args], {
|
||||
terminal: {
|
||||
cols,
|
||||
rows,
|
||||
@@ -1996,7 +2007,7 @@ export async function launchClaudePty(
|
||||
},
|
||||
cwd,
|
||||
env: childEnv,
|
||||
}); } catch (error) { await disposeScreen(); throw error; }
|
||||
}); } catch (error) { pendingExit?.dispose(); await disposeScreen(); throw error; }
|
||||
|
||||
// Track exit so waitForAny can fail fast if claude crashes.
|
||||
let exitedPromise: Promise<void> = Promise.resolve();
|
||||
@@ -2129,7 +2140,7 @@ export async function launchClaudePty(
|
||||
clearTimeout(trustWatcherStop);
|
||||
clearInterval(trustWatcher);
|
||||
for (const timer of trustInputTimers) clearTimeout(timer);
|
||||
if (exited) { await disposeScreen(); return; }
|
||||
if (exited) { pendingExit?.dispose(); await disposeScreen(); return; }
|
||||
try {
|
||||
proc.kill?.('SIGINT');
|
||||
} catch {
|
||||
@@ -2145,6 +2156,7 @@ export async function launchClaudePty(
|
||||
}
|
||||
await Promise.race([exitedPromise, Bun.sleep(1000)]);
|
||||
}
|
||||
pendingExit?.dispose();
|
||||
await disposeScreen();
|
||||
}
|
||||
|
||||
@@ -2167,6 +2179,7 @@ export async function launchClaudePty(
|
||||
exited: () => exited,
|
||||
exitCode: () => exitCodeCaptured,
|
||||
hermeticConfigDir: hermetic ? childEnv.CLAUDE_CONFIG_DIR ?? null : null,
|
||||
pendingPlanReadyFile: pendingExit?.file,
|
||||
close,
|
||||
};
|
||||
}
|
||||
@@ -2717,6 +2730,7 @@ export async function runPlanSkillCounting(opts: {
|
||||
model: opts.model,
|
||||
seedSkills: true,
|
||||
observeScreen: true,
|
||||
observePlanReady: true,
|
||||
});
|
||||
} catch (error) {
|
||||
fixture.cleanup();
|
||||
@@ -2784,6 +2798,8 @@ export async function runPlanSkillCounting(opts: {
|
||||
transcript = session.hermeticConfigDir
|
||||
? readPlanCountTranscript(session.hermeticConfigDir, fixture.cwd)
|
||||
: { status: 'error', calls: [], assistantMessages: [], error: 'Claude count session has no isolated transcript directory' };
|
||||
transcript = withPendingExit(transcript, session.pendingPlanReadyFile, fixture.cwd,
|
||||
session.hermeticConfigDir, startedAt, visible);
|
||||
if (transcript.status === 'error') {
|
||||
return snapshot('transcript_unavailable', transcript.error!, visible);
|
||||
}
|
||||
|
||||
@@ -38,11 +38,22 @@ export function codexOutsideExecutions(lines: string[]): OutsideExecution[] {
|
||||
});
|
||||
}
|
||||
|
||||
export function foundInvoiceAuthorizationDefect(executions: OutsideExecution[], provider: 'codex' | 'claude-code'): boolean {
|
||||
const invocation = provider === 'codex' ? /\bcodex\s+(?:exec|review)\b/
|
||||
function outsideInvocation(provider: 'codex' | 'claude-code'): RegExp {
|
||||
return provider === 'codex' ? /\bcodex\s+(?:exec|review)\b/
|
||||
: /\bgstack-claude-code(?:['"])?\s+--/;
|
||||
}
|
||||
|
||||
/** Preserve provider results, including failures, without unrelated shell/config events. */
|
||||
export function outsideExecutionTranscript(executions: OutsideExecution[], provider: 'codex' | 'claude-code') {
|
||||
return executions.filter(({ command }) => outsideInvocation(provider).test(command))
|
||||
.map(({ command, output, succeeded }) => ({
|
||||
type: 'outside_execution' as const, provider, command, output, succeeded,
|
||||
}));
|
||||
}
|
||||
|
||||
export function foundInvoiceAuthorizationDefect(executions: OutsideExecution[], provider: 'codex' | 'claude-code'): boolean {
|
||||
return executions.some(({ command, output, succeeded }) => succeeded
|
||||
&& invocation.test(command)
|
||||
&& outsideInvocation(provider).test(command)
|
||||
&& /invoice/i.test(output)
|
||||
&& /owner|ownership|unauthori[sz]ed|another user|cross[- ](?:tenant|user)|access control|authorization/i.test(output)
|
||||
&& !/OUTSIDE_STATUS:\s*(?:unavailable|disabled|skipped)/.test(output));
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
/** Read-only pending ExitPlanMode identity for counting evals. */
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import type { PlanCountTranscript } from './plan-count-transcript';
|
||||
|
||||
const MAX_RECORD_BYTES = 4096;
|
||||
const identifier = (value: unknown): value is string =>
|
||||
typeof value === 'string' && /^[A-Za-z0-9_-]{1,160}$/.test(value);
|
||||
const shellQuote = (value: string) => `'${(process.platform === 'win32' ? value.replaceAll('\\', '/') : value).replaceAll("'", "'\\''")}'`;
|
||||
|
||||
export function createPendingExitRecorder(cwd: string, configDir: string) {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-pending-exit-'));
|
||||
const file = path.join(dir, 'pending.json');
|
||||
const command = [process.execPath, import.meta.path, '--record', file, cwd, configDir].map(shellQuote).join(' ');
|
||||
const settings = JSON.stringify({ hooks: { PreToolUse: [{ matcher: '^ExitPlanMode$',
|
||||
hooks: [{ type: 'command', command, timeout: 5 }],
|
||||
}] } });
|
||||
return { file, settings, dispose: () => fs.rmSync(dir, { recursive: true, force: true }) };
|
||||
}
|
||||
|
||||
function scopedTranscript(file: unknown, configDir: string, session: string): boolean {
|
||||
if (typeof file !== 'string' || !path.isAbsolute(file)) return false;
|
||||
const relative = path.relative(path.join(configDir, 'projects'), file).split(path.sep);
|
||||
return relative.length === 2 && relative[0] !== '..' && relative[0] !== '.' &&
|
||||
relative[1] === `${session}.jsonl`;
|
||||
}
|
||||
|
||||
/** No stdout, permission decision, updated input, or model-visible context. */
|
||||
export function recordPendingExit(input: string, file: string, cwd: string, configDir: string): void {
|
||||
try {
|
||||
if (Buffer.byteLength(input) > 64 * 1024) throw new Error('hook input too large');
|
||||
const event = JSON.parse(input);
|
||||
// Subagents and other fixtures must never supply this main session's gate.
|
||||
if (event?.agent_id !== undefined || event?.cwd !== cwd) return;
|
||||
fs.rmSync(file, { force: true });
|
||||
if (event.hook_event_name !== 'PreToolUse' || event.tool_name !== 'ExitPlanMode' ||
|
||||
!identifier(event.session_id) || !identifier(event.tool_use_id) ||
|
||||
!scopedTranscript(event.transcript_path, configDir, event.session_id)) return;
|
||||
const record = { sessionId: event.session_id, toolUseId: event.tool_use_id,
|
||||
cwd, transcriptPath: event.transcript_path, timestamp: new Date().toISOString() };
|
||||
const temporary = `${file}.tmp`;
|
||||
fs.writeFileSync(temporary, JSON.stringify(record) + '\n', { mode: 0o600 });
|
||||
fs.renameSync(temporary, file);
|
||||
} catch {
|
||||
// A failed recorder cannot turn an earlier gate into current evidence.
|
||||
try { fs.rmSync(file, { force: true }); } catch { /* no evidence */ }
|
||||
}
|
||||
}
|
||||
|
||||
/** Exact current native gate; ordinary prose or a still-streaming menu is insufficient. */
|
||||
export function isCurrentPlanApprovalScreen(screen: string): boolean {
|
||||
const gate = /(?:^|\n) {0,3}─{5,}[ \t]*\n {0,3}Claude has written up a plan and is ready to execute\. Would you like to proceed\?[ \t]*\n\s*❯[ \t]*1\.[ \t]*Yes, and use auto mode[ \t]*\n[ \t]*2\.[ \t]*Yes, manually approve edits[ \t]*\n[ \t]*3\.[ \t]*Tell Claude what to change[ \t]*(?:\n[ \t]*shift\+tab to approve with this feedback)?\s*$/i.exec(screen) ??
|
||||
/(?:^|\n) {0,3}Exit plan mode\?[ \t]*\n {0,3}Claude wants to exit plan mode[ \t]*\n\s*❯[ \t]*1\.[ \t]*Yes, and switch to default \(ask each time\) for this session[ \t]*\n[ \t]*2\.[ \t]*No\s*$/i.exec(screen);
|
||||
if (!gate) return false;
|
||||
const before = screen.slice(0, gate.index);
|
||||
if (/\b(?:example|sample|template|quote)\b.*[:：]\s*$/i.test(before.trimEnd().split('\n').at(-1) ?? '')) return false;
|
||||
let fence: { marker: string; length: number } | undefined;
|
||||
for (const line of before.split('\n')) {
|
||||
const delimiter = /^ {0,3}(`{3,}|~{3,})(.*)$/.exec(line);
|
||||
if (!delimiter) continue;
|
||||
if (!fence) fence = { marker: delimiter[1]![0]!, length: delimiter[1]!.length };
|
||||
else if (delimiter[1]![0] === fence.marker && delimiter[1]!.length >= fence.length && !delimiter[2]!.trim()) fence = undefined;
|
||||
}
|
||||
return fence === undefined;
|
||||
}
|
||||
|
||||
/** Adds pending identity only; successful AUQ coverage still comes from JSONL results. */
|
||||
export function withPendingExit(
|
||||
transcript: PlanCountTranscript, file: string | undefined, cwd: string,
|
||||
configDir: string | null, startedAt: number, screen: string,
|
||||
): PlanCountTranscript {
|
||||
if (!file || !configDir || transcript.status !== 'ready' || !transcript.calls.length ||
|
||||
!isCurrentPlanApprovalScreen(screen)) return transcript;
|
||||
try {
|
||||
const stat = fs.lstatSync(file);
|
||||
if (!stat.isFile() || stat.size > MAX_RECORD_BYTES) return transcript;
|
||||
const record = JSON.parse(fs.readFileSync(file, 'utf8'));
|
||||
const time = Date.parse(record.timestamp);
|
||||
const sessions = new Set([...transcript.calls.map(call => call.sessionId),
|
||||
...transcript.assistantMessages.map(message => message.sessionId)]);
|
||||
if (record.cwd !== cwd || !identifier(record.sessionId) || !identifier(record.toolUseId) ||
|
||||
sessions.size !== 1 || !sessions.has(record.sessionId) ||
|
||||
!scopedTranscript(record.transcriptPath, configDir, record.sessionId) ||
|
||||
!Number.isFinite(time) || time < startedAt || time > Date.now()) return transcript;
|
||||
const requests = transcript.planReadyRequests ?? [];
|
||||
// A flushed native record, including a failed result, always wins.
|
||||
if (requests.some(request => request.sessionId === record.sessionId && request.toolUseId === record.toolUseId)) return transcript;
|
||||
return { ...transcript, planReadyRequests: [...requests, {
|
||||
sessionId: record.sessionId, toolUseId: record.toolUseId,
|
||||
timestamp: record.timestamp, failed: false, source: 'pre_tool_use',
|
||||
}] };
|
||||
} catch { return transcript; }
|
||||
}
|
||||
|
||||
if (import.meta.main && process.argv[2] === '--record') {
|
||||
try {
|
||||
const [file, cwd, configDir] = process.argv.slice(3);
|
||||
if (file && cwd && configDir) recordPendingExit(await Bun.stdin.text(), file, cwd, configDir);
|
||||
} catch { /* observation never changes the native permission outcome */ }
|
||||
}
|
||||
@@ -26,7 +26,7 @@ export interface PlanCountTranscript {
|
||||
calls: NativePlanQuestionCall[];
|
||||
assistantMessages: Array<{ sessionId: string; text: string; timestamp: string }>;
|
||||
/** Actual native plan-mode approval requests; pending is the UI gate, never an AUQ. */
|
||||
planReadyRequests?: Array<{ sessionId: string; toolUseId: string; timestamp: string; failed: boolean }>;
|
||||
planReadyRequests?: Array<{ sessionId: string; toolUseId: string; timestamp: string; failed: boolean; source?: 'pre_tool_use' }>;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
|
||||
@@ -148,7 +148,7 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
|
||||
'tpa-absent-darwin': ['scripts/resolvers/third-party-actions.ts', 'ship/SKILL.md.tmpl', 'ship/sections/apple-release.md.tmpl', 'scripts/gen-skill-docs.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-third-party-actions.test.ts'],
|
||||
'tpa-apple-ban': ['scripts/resolvers/third-party-actions.ts', 'ship/SKILL.md.tmpl', 'ship/sections/apple-release.md.tmpl', 'scripts/gen-skill-docs.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-third-party-actions.test.ts'],
|
||||
'ship-section-loading': ['ship/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/skill-e2e-ship-section-loading.test.ts'],
|
||||
'plan-ceo-section-loading': ['plan-ceo-review/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/helpers/ceo-section-loading-fixture.ts', 'test/ceo-section-loading-fixture.test.ts', 'test/skill-e2e-plan-ceo-review-section-loading.test.ts'],
|
||||
'plan-ceo-section-loading': ['plan-ceo-review/**', 'test/skill-ceo-section-ordering.test.ts', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/helpers/ceo-section-loading-fixture.ts', 'test/ceo-section-loading-fixture.test.ts', 'test/skill-e2e-plan-ceo-review-section-loading.test.ts'],
|
||||
// Data-driven behavioral guard for the 'plan'/'prompt' carves (eng, design,
|
||||
// devex, office-hours + future PR2 carves). One file iterating CARVE_GUARDS;
|
||||
// the selector sets GSTACK_CARVE_SKILL=<name> to scope cost to the changed
|
||||
@@ -160,10 +160,10 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
|
||||
// Each test drives its skill end-to-end; touchfiles include preamble +
|
||||
// completion-status resolvers because they affect question cadence and
|
||||
// terminal output (the regression surface this test catches).
|
||||
'plan-ceo-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/ceo-completion-handoff.ts', 'test/ceo-completion-handoff.test.ts', 'test/fixtures/ceo-completion-handoff-calls.json', 'test/fixtures/ceo-completion-handoff-j-calls.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-finding-count.test.ts'],
|
||||
'plan-eng-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-eng-finding-count.test.ts'],
|
||||
'plan-design-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-design-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/design-count-review.ts', 'test/design-count-review.test.ts', 'test/fixtures/design-review-j-calls.json', 'test/helpers/design-count-outside.ts', 'test/design-count-outside.test.ts', 'test/skill-e2e-plan-design-finding-count.test.ts'],
|
||||
'plan-devex-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-devex-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts'],
|
||||
'plan-ceo-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-ceo-review/**', 'test/skill-ceo-section-ordering.test.ts', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/ceo-completion-handoff.ts', 'test/ceo-completion-handoff.test.ts', 'test/fixtures/ceo-completion-handoff-calls.json', 'test/fixtures/ceo-completion-handoff-j-calls.json', 'test/fixtures/ceo-completion-handoff-k-calls.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-finding-count.test.ts'],
|
||||
'plan-eng-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-eng-finding-count.test.ts'],
|
||||
'plan-design-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-design-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/design-count-review.ts', 'test/design-count-review.test.ts', 'test/fixtures/design-review-j-calls.json', 'test/helpers/design-count-outside.ts', 'test/design-count-outside.test.ts', 'test/skill-e2e-plan-design-finding-count.test.ts'],
|
||||
'plan-devex-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-devex-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts'],
|
||||
|
||||
// Gate-tier reviewCount-floor counterparts. Catch the May 2026 transcript
|
||||
// bug (model wrote a plan-mode plan and ExitPlanMode'd without firing any
|
||||
@@ -180,8 +180,8 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
|
||||
// a model fires one AUQ then batches the rest into a "## Decisions to
|
||||
// confirm" plan write. runPlanSkillFloorCheck cannot detect that shape
|
||||
// (it exits on first AUQ); runPlanSkillCounting can.
|
||||
'plan-eng-multi-finding-batching': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts'],
|
||||
'plan-ceo-split-overflow': ['bin/gstack-config', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts'],
|
||||
'plan-eng-multi-finding-batching': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts'],
|
||||
'plan-ceo-split-overflow': ['bin/gstack-config', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts'],
|
||||
'brain-privacy-gate': ['bin/gstack-skill-start', 'bin/gstack-skill-end', 'scripts/resolvers/preamble/generate-brain-sync-block.ts', 'scripts/resolvers/preamble.ts', 'bin/gstack-brain-sync', 'bin/gstack-artifacts-init', 'bin/gstack-config', 'test/helpers/agent-sdk-runner.ts', 'test/skill-e2e-brain-privacy-gate.test.ts'],
|
||||
|
||||
// /setup-gbrain Path 4 (Remote MCP) — happy + bad-token end-to-end via
|
||||
@@ -281,14 +281,14 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
|
||||
'scripts/gen-skill-docs.ts', 'scripts/resolvers/index.ts', 'scripts/resolvers/outside-voice.ts',
|
||||
'scripts/resolvers/constants.ts', 'scripts/resolvers/review.ts',
|
||||
'bin/gstack-claude-code', 'lib/claude-code.ts', 'lib/claude-code-windows-job.ts', 'lib/claude-bin.ts', 'lib/outside-review-result.ts', 'test/helpers/codex-session-runner.ts',
|
||||
'test/helpers/skill-fixture.ts', 'test/helpers/outside-voice-fixture.ts', 'test/helpers/outside-voice-evidence.ts', 'test/skill-e2e-outside-voice.test.ts',
|
||||
'test/helpers/skill-fixture.ts', 'test/helpers/outside-voice-fixture.ts', 'test/helpers/outside-voice-evidence.ts', 'test/outside-voice-evidence.test.ts', 'test/skill-e2e-outside-voice.test.ts',
|
||||
],
|
||||
'outside-voice-claude-code-to-codex': [
|
||||
'review/**', 'codex/**', 'hosts/claude.ts', 'hosts/define-host.ts',
|
||||
'scripts/gen-skill-docs.ts', 'scripts/resolvers/index.ts', 'scripts/resolvers/outside-voice.ts',
|
||||
'scripts/resolvers/constants.ts', 'scripts/resolvers/review.ts',
|
||||
'bin/gstack-codex-probe', 'lib/outside-review-result.ts', 'test/helpers/session-runner.ts',
|
||||
'test/helpers/skill-fixture.ts', 'test/helpers/outside-voice-fixture.ts', 'test/helpers/outside-voice-evidence.ts', 'test/skill-e2e-outside-voice.test.ts',
|
||||
'test/helpers/skill-fixture.ts', 'test/helpers/outside-voice-fixture.ts', 'test/helpers/outside-voice-evidence.ts', 'test/outside-voice-evidence.test.ts', 'test/skill-e2e-outside-voice.test.ts',
|
||||
],
|
||||
|
||||
// Disabled means no extra plan review, including a native Agent fallback.
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { claudeOutsideExecutions, codexOutsideExecutions, foundInvoiceAuthorizationDefect } from './helpers/outside-voice-evidence';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { EvalCollector } from './helpers/eval-store';
|
||||
import { claudeOutsideExecutions, codexOutsideExecutions, foundInvoiceAuthorizationDefect, outsideExecutionTranscript } from './helpers/outside-voice-evidence';
|
||||
|
||||
const finding = '[P1] invoice.ts removed the owner check, allowing unauthorized access to private invoices.';
|
||||
|
||||
@@ -28,3 +32,47 @@ describe('cross-harness live eval evidence', () => {
|
||||
]) expect(foundInvoiceAuthorizationDefect(codexOutsideExecutions([JSON.stringify(invalid)]), 'claude-code')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('outside execution artifact retention', () => {
|
||||
for (const provider of ['codex', 'claude-code'] as const) {
|
||||
test(`${provider} persists full successful and failed results before cleanup`, () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'outside-evidence-'));
|
||||
try {
|
||||
const command = provider === 'codex' ? 'codex exec --json -'
|
||||
: '"/runtime/bin/gstack-claude-code" --cwd /repo --access none --timeout-ms 1000';
|
||||
const output = 'Review detail\n'.repeat(300) + finding;
|
||||
const failedOutput = 'OUTSIDE_STATUS: unavailable\nprovider exited before final response';
|
||||
const executions = [
|
||||
{ command: 'cat /owned/auth.json /owned/config.toml', output: 'UNRELATED_CONFIG_CONTENT', succeeded: true },
|
||||
{ command, output: failedOutput, succeeded: false },
|
||||
{ command, output, succeeded: true, unrelated: 'UNRELATED_EVENT_FIELD' },
|
||||
];
|
||||
const transcript = outsideExecutionTranscript(executions, provider);
|
||||
const collector = new EvalCollector('e2e', dir);
|
||||
collector.addTest({ name: `outside-${provider}`, suite: 'outside-voice', tier: 'e2e',
|
||||
passed: true, duration_ms: 1, cost_usd: 0, transcript });
|
||||
|
||||
// addTest writes immediately, before disposable host homes/repositories disappear.
|
||||
const stored = fs.readFileSync(path.join(dir, '_partial-e2e.json'), 'utf8');
|
||||
const entries = JSON.parse(stored).tests[0].transcript;
|
||||
expect(entries).toEqual([
|
||||
{ type: 'outside_execution', provider, command, output: failedOutput, succeeded: false },
|
||||
{ type: 'outside_execution', provider, command, output, succeeded: true },
|
||||
]);
|
||||
expect(entries[1].output.length).toBeGreaterThan(2000);
|
||||
expect(stored).not.toContain('UNRELATED_CONFIG_CONTENT');
|
||||
expect(stored).not.toContain('UNRELATED_EVENT_FIELD');
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
test('unrelated providers and host prose never become retained outside executions', () => {
|
||||
const unrelated = [{ command: 'codex exec --json -', output: finding, succeeded: true }];
|
||||
expect(outsideExecutionTranscript(unrelated, 'claude-code')).toEqual([]);
|
||||
expect(outsideExecutionTranscript(claudeOutsideExecutions([
|
||||
{ type: 'assistant', message: { content: [{ type: 'text', text: `codex exec: ${finding}` }] } },
|
||||
]), 'codex')).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,250 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import { createPendingExitRecorder, isCurrentPlanApprovalScreen, recordPendingExit, withPendingExit } from './helpers/plan-count-pending-exit';
|
||||
import { hasNativePlanTerminal } from './helpers/claude-pty-runner';
|
||||
import type { PlanCountTranscript } from './helpers/plan-count-transcript';
|
||||
|
||||
const GATE = '────────────────────────────────────────────────────────\n' +
|
||||
'Claude has written up a plan and is ready to execute. Would you like to proceed?\n\n' +
|
||||
'❯ 1. Yes, and use auto mode\n 2. Yes, manually approve edits\n' +
|
||||
' 3. Tell Claude what to change\n shift+tab to approve with this feedback\n';
|
||||
const COMPACT_GATE = 'Exit plan mode?\nClaude wants to exit plan mode\n' +
|
||||
'❯ 1. Yes, and switch to default (ask each time) for this session\n 2. No\n';
|
||||
const REPORT = '# Reviewed plan\n\n## GSTACK REVIEW REPORT\n\n' +
|
||||
'| Review | Status | Findings |\n|---|---|---|\n| Design Review | clean | resolved |\n\n' +
|
||||
'VERDICT: Design review complete\n\nNO UNRESOLVED DECISIONS\n';
|
||||
|
||||
function fixture() {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'pending-exit-test-'));
|
||||
const config = path.join(dir, '.claude');
|
||||
const startedAt = Date.now() - 10_000;
|
||||
const report = path.join(dir, 'report.md');
|
||||
fs.writeFileSync(report, REPORT);
|
||||
const transcript: PlanCountTranscript = { status: 'ready', assistantMessages: [], calls: [{
|
||||
sessionId: 'main-session', toolUseId: 'finding', answered: true, failed: false,
|
||||
questions: [{ header: 'Finding', question: 'Fix this gap?', options: [{ label: 'Fix' }, { label: 'Keep' }] }],
|
||||
answers: { 'Fix this gap?': 'Fix' }, unansweredQuestionIndices: [],
|
||||
answeredAt: new Date(startedAt + 1000).toISOString(),
|
||||
}] };
|
||||
const recorder = createPendingExitRecorder(dir, config);
|
||||
const event = { hook_event_name: 'PreToolUse', tool_name: 'ExitPlanMode',
|
||||
session_id: 'main-session', tool_use_id: 'native-exit', cwd: dir,
|
||||
transcript_path: path.join(config, 'projects', 'owned-project', 'main-session.jsonl'),
|
||||
tool_input: { ignored: 'DO_NOT_PERSIST_INPUT_OR_CONFIG' } };
|
||||
const observe = (screen = GATE, value = transcript) => withPendingExit(value, recorder.file, dir, config, startedAt, screen);
|
||||
return { dir, config, report, startedAt, transcript, recorder, event, observe,
|
||||
cleanup() { recorder.dispose(); fs.rmSync(dir, { recursive: true, force: true }); } };
|
||||
}
|
||||
|
||||
describe('pending native ExitPlanMode identity', () => {
|
||||
test('the captured gate can complete only with real pending identity and a fresh report', () => {
|
||||
const f = fixture();
|
||||
try {
|
||||
expect(hasNativePlanTerminal(f.observe(), f.report, f.startedAt, 'plan_ready')).toBe(false);
|
||||
recordPendingExit(JSON.stringify(f.event), f.recorder.file, f.dir, f.config);
|
||||
const observed = f.observe();
|
||||
expect(observed.calls).toBe(f.transcript.calls);
|
||||
expect(observed.planReadyRequests?.[0]?.source).toBe('pre_tool_use');
|
||||
expect(hasNativePlanTerminal(observed, f.report, f.startedAt, 'plan_ready')).toBe(true);
|
||||
expect(fs.readFileSync(f.recorder.file, 'utf8')).not.toContain('DO_NOT_PERSIST');
|
||||
expect(hasNativePlanTerminal(f.observe('The report is ready to execute.'), f.report, f.startedAt, 'plan_ready')).toBe(false);
|
||||
} finally { f.cleanup(); }
|
||||
});
|
||||
|
||||
test('missing, malformed, partial, foreign, stale and future records add no identity', () => {
|
||||
const f = fixture();
|
||||
try {
|
||||
recordPendingExit(JSON.stringify(f.event), f.recorder.file, f.dir, f.config);
|
||||
const valid = JSON.parse(fs.readFileSync(f.recorder.file, 'utf8'));
|
||||
for (const input of ['{', '{}', 'x'.repeat(4097), ...[
|
||||
{ ...valid, sessionId: 'foreign' }, { ...valid, cwd: path.join(f.dir, 'other') },
|
||||
{ ...valid, transcriptPath: '/outside/main-session.jsonl' },
|
||||
{ ...valid, transcriptPath: path.join(f.config, 'projects', 'owned', 'foreign.jsonl') },
|
||||
{ ...valid, timestamp: new Date(f.startedAt - 1).toISOString() },
|
||||
{ ...valid, timestamp: new Date(Date.now() + 60_000).toISOString() },
|
||||
].map(value => JSON.stringify(value))]) {
|
||||
fs.writeFileSync(f.recorder.file, input);
|
||||
expect(f.observe().planReadyRequests, input).toBeUndefined();
|
||||
}
|
||||
} finally { f.cleanup(); }
|
||||
});
|
||||
|
||||
test('foreign, sidechain and non-Exit hook payloads cannot create records', () => {
|
||||
const f = fixture();
|
||||
try {
|
||||
for (const change of [
|
||||
{ agent_id: 'subagent' }, { cwd: '/different-fixture' }, { tool_name: 'AskUserQuestion' },
|
||||
{ hook_event_name: 'PostToolUse' }, { session_id: '../escape' }, { tool_use_id: '' },
|
||||
]) {
|
||||
recordPendingExit(JSON.stringify({ ...f.event, ...change }), f.recorder.file, f.dir, f.config);
|
||||
expect(fs.existsSync(f.recorder.file)).toBe(false);
|
||||
}
|
||||
recordPendingExit(JSON.stringify(f.event), f.recorder.file, f.dir, f.config);
|
||||
recordPendingExit('{broken', f.recorder.file, f.dir, f.config);
|
||||
expect(fs.existsSync(f.recorder.file)).toBe(false);
|
||||
} finally { f.cleanup(); }
|
||||
});
|
||||
|
||||
test('pending identity cannot bypass missing coverage, failed results, or stale reports', () => {
|
||||
const f = fixture();
|
||||
try {
|
||||
recordPendingExit(JSON.stringify(f.event), f.recorder.file, f.dir, f.config);
|
||||
for (const change of [
|
||||
(t: any) => { t.status = 'missing'; }, (t: any) => { t.status = 'error'; },
|
||||
(t: any) => { t.calls = []; }, (t: any) => { t.calls[0].answered = false; },
|
||||
(t: any) => { t.calls[0].failed = true; },
|
||||
(t: any) => { t.calls[0].answeredAt = new Date(Date.now() + 1000).toISOString(); },
|
||||
(t: any) => { t.assistantMessages.push({ sessionId: 'other', timestamp: new Date().toISOString(), text: 'Done' }); },
|
||||
(t: any) => { t.planReadyRequests = [{ sessionId: 'main-session', toolUseId: 'native-exit', timestamp: new Date().toISOString(), failed: true }]; },
|
||||
]) {
|
||||
const value = structuredClone(f.transcript); change(value);
|
||||
expect(hasNativePlanTerminal(f.observe(GATE, value), f.report, f.startedAt, 'plan_ready')).toBe(false);
|
||||
}
|
||||
const observed = f.observe();
|
||||
fs.utimesSync(f.report, new Date(f.startedAt - 1), new Date(f.startedAt - 1));
|
||||
expect(hasNativePlanTerminal(observed, f.report, f.startedAt, 'plan_ready')).toBe(false);
|
||||
fs.writeFileSync(f.report, '## GSTACK REVIEW REPORT\n');
|
||||
expect(hasNativePlanTerminal(observed, f.report, f.startedAt, 'plan_ready')).toBe(false);
|
||||
fs.rmSync(f.report);
|
||||
expect(hasNativePlanTerminal(observed, f.report, f.startedAt, 'plan_ready')).toBe(false);
|
||||
} finally { f.cleanup(); }
|
||||
});
|
||||
|
||||
test('quoted, incomplete, busy and superseded screens never supply the active gate', () => {
|
||||
expect(isCurrentPlanApprovalScreen(GATE)).toBe(true);
|
||||
expect(isCurrentPlanApprovalScreen(COMPACT_GATE)).toBe(true);
|
||||
expect(isCurrentPlanApprovalScreen('```ts\nconst done = true;\n```\n' + GATE)).toBe(true);
|
||||
expect(isCurrentPlanApprovalScreen('> An earlier quoted review note.\n' + GATE)).toBe(true);
|
||||
for (const screen of [
|
||||
'> ' + GATE.replaceAll('\n', '\n> '), '```text\n' + GATE + '```',
|
||||
'```text\n' + GATE, 'Example approval menu:\n' + GATE,
|
||||
...[GATE, COMPACT_GATE].map(screen => screen.split('\n').map(line => ' ' + line).join('\n')),
|
||||
GATE.replace(' 3. Tell Claude what to change', ''),
|
||||
GATE + '\nWorking…', GATE + '\n☐ Finding\n❯ 1. Fix\n 2. Keep\nEnter to select',
|
||||
'Claude has written up a plan and is ready to execute. Would you like to proceed?',
|
||||
]) expect(isCurrentPlanApprovalScreen(screen), screen).toBe(false);
|
||||
});
|
||||
|
||||
test('the executable observer is silent on success and malformed input', async () => {
|
||||
const f = fixture();
|
||||
try {
|
||||
for (const input of [JSON.stringify(f.event), '{broken']) {
|
||||
const child = Bun.spawn([process.execPath, path.join(import.meta.dir, 'helpers/plan-count-pending-exit.ts'),
|
||||
'--record', f.recorder.file, f.dir, f.config], { stdin: new Blob([input]), stdout: 'pipe', stderr: 'pipe' });
|
||||
const [exit, stdout, stderr] = await Promise.all([child.exited,
|
||||
new Response(child.stdout).text(), new Response(child.stderr).text()]);
|
||||
expect([exit, stdout, stderr]).toEqual([0, '', '']);
|
||||
}
|
||||
expect(fs.existsSync(f.recorder.file)).toBe(false);
|
||||
} finally { f.cleanup(); }
|
||||
});
|
||||
});
|
||||
|
||||
(process.platform === 'win32' ? test.skip : test)('real fake CLI executes the opt-in hook before the gate and cleans up on close', async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'pending-exit-pty-'));
|
||||
const fake = path.join(dir, 'fake-claude');
|
||||
const worker = path.join(dir, 'worker.ts');
|
||||
const result = path.join(dir, 'result.json');
|
||||
const helper = (name: string) => pathToFileURL(path.join(import.meta.dir, 'helpers', name)).href;
|
||||
fs.writeFileSync(fake, `#!${process.execPath}
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
const cwd = process.cwd();
|
||||
fs.writeFileSync(path.join(cwd,'pid'),String(process.pid));
|
||||
const config = process.env.CLAUDE_CONFIG_DIR;
|
||||
const session = 'fake-main';
|
||||
const native = path.join(config, 'projects', path.basename(cwd), session + '.jsonl');
|
||||
fs.mkdirSync(path.dirname(native), {recursive:true});
|
||||
const question = {header:'Finding',question:'Fix the gap?',options:[{label:'Fix'},{label:'Keep'}]};
|
||||
const common = {sessionId:session,cwd,isSidechain:false,timestamp:new Date(Date.now()-100).toISOString()};
|
||||
fs.writeFileSync(native, JSON.stringify({...common,type:'assistant',message:{role:'assistant',content:[{type:'tool_use',name:'AskUserQuestion',id:'q1',input:{questions:[question]}}]}})+'\\n'+
|
||||
JSON.stringify({...common,type:'user',message:{role:'user',content:[{type:'tool_result',tool_use_id:'q1',content:'User answered'}]},toolUseResult:{answers:{'Fix the gap?':'Fix'}}})+'\\n');
|
||||
fs.writeFileSync(path.join(cwd,'report.md'), ${JSON.stringify(REPORT)});
|
||||
const index = process.argv.indexOf('--settings');
|
||||
if (index >= 0) {
|
||||
const hook = JSON.parse(process.argv[index+1]).hooks.PreToolUse[0];
|
||||
if (hook.matcher !== '^ExitPlanMode$') throw Error('Wrong hook scope');
|
||||
const payload = {hook_event_name:'PreToolUse',tool_name:'ExitPlanMode',session_id:session,tool_use_id:'exit1',cwd,transcript_path:native};
|
||||
const child = Bun.spawn(['bash','-c',hook.hooks[0].command],{stdin:new Blob([JSON.stringify(payload)]),stdout:'pipe',stderr:'pipe'});
|
||||
const [exit,out,err] = await Promise.all([child.exited,new Response(child.stdout).text(),new Response(child.stderr).text()]);
|
||||
if (exit || out || err) throw Error('Observer changed native response');
|
||||
}
|
||||
if (process.env.PENDING_EXIT_CRASH === '1') process.exit(19);
|
||||
process.stdout.write(${JSON.stringify(GATE)}.replaceAll('\\n','\\r\\n'));
|
||||
process.stdin.on('data', data => fs.appendFileSync(path.join(cwd,'unexpected-input'),data));
|
||||
process.stdin.resume();
|
||||
`);
|
||||
fs.chmodSync(fake, 0o755);
|
||||
fs.writeFileSync(worker, `
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import * as os from 'node:os';
|
||||
import {launchClaudePty,hasNativePlanTerminal} from ${JSON.stringify(helper('claude-pty-runner.ts'))};
|
||||
import {readPlanCountTranscript} from ${JSON.stringify(helper('plan-count-transcript.ts'))};
|
||||
import {withPendingExit} from ${JSON.stringify(helper('plan-count-pending-exit.ts'))};
|
||||
const results=[];
|
||||
for (const enabled of [false,true]) {
|
||||
const cwd=path.join(${JSON.stringify(dir)},String(enabled)); fs.mkdirSync(cwd);
|
||||
const startedAt=Date.now()-1000;
|
||||
const session=await launchClaudePty({cwd,observeScreen:true,observePlanReady:enabled,timeoutMs:10000});
|
||||
const owned=session.pendingPlanReadyFile;
|
||||
try {
|
||||
await session.waitFor('shift+tab to approve',{timeoutMs:5000,pollMs:20});
|
||||
const native=readPlanCountTranscript(session.hermeticConfigDir,cwd);
|
||||
const observed=withPendingExit(native,owned,cwd,session.hermeticConfigDir,startedAt,await session.currentScreen());
|
||||
results.push({enabled,hook:owned!==undefined,nativeCalls:native.calls.length,nativeReady:native.planReadyRequests?.length??0,
|
||||
ready:hasNativePlanTerminal(observed,path.join(cwd,'report.md'),startedAt,'plan_ready'),input:fs.existsSync(path.join(cwd,'unexpected-input'))});
|
||||
} finally {await session.close();}
|
||||
if (owned && fs.existsSync(path.dirname(owned))) throw Error('Hook directory leaked');
|
||||
}
|
||||
const crashCwd=path.join(${JSON.stringify(dir)},'crash'); fs.mkdirSync(crashCwd);
|
||||
const crashed=await launchClaudePty({cwd:crashCwd,observeScreen:true,observePlanReady:true,
|
||||
timeoutMs:10000,env:{PENDING_EXIT_CRASH:'1'}});
|
||||
const crashRecord=crashed.pendingPlanReadyFile;
|
||||
while (!crashed.exited()) await Bun.sleep(20);
|
||||
if (crashed.exitCode()!==19 || !fs.existsSync(crashRecord)) throw Error('Crash control did not execute hook');
|
||||
await crashed.close();
|
||||
if (fs.existsSync(path.dirname(crashRecord))) throw Error('Crashed child leaked hook directory');
|
||||
const spawn=Bun.spawn;
|
||||
Bun.spawn=()=>{throw Error('simulated spawn failure');};
|
||||
let rejected=false;
|
||||
try {await launchClaudePty({cwd:crashCwd,observeScreen:true,observePlanReady:true});}
|
||||
catch (error) {rejected=String(error).includes('simulated spawn failure');}
|
||||
finally {Bun.spawn=spawn;}
|
||||
if (!rejected || fs.readdirSync(os.tmpdir()).some(name=>name.startsWith('gstack-pending-exit-'))) throw Error('Startup failure leaked recorder');
|
||||
fs.writeFileSync(${JSON.stringify(result)},JSON.stringify(results));
|
||||
`);
|
||||
const workerTemp = path.join(dir, 'owned-tmp');
|
||||
fs.mkdirSync(workerTemp);
|
||||
const child = Bun.spawn([process.execPath, worker], {
|
||||
env: { ...process.env, TMPDIR: workerTemp, BROWSE_TERMINAL_BINARY: fake, EVALS_HERMETIC: '1' }, stdout: 'pipe', stderr: 'pipe',
|
||||
});
|
||||
const timeout = setTimeout(() => child.kill('SIGKILL'), 20_000);
|
||||
try {
|
||||
const [exit, stdout, stderr] = await Promise.all([child.exited,
|
||||
new Response(child.stdout).text(), new Response(child.stderr).text()]);
|
||||
expect(exit, stdout + stderr).toBe(0);
|
||||
expect(JSON.parse(fs.readFileSync(result, 'utf8'))).toEqual([
|
||||
{ enabled:false,hook:false,nativeCalls:1,nativeReady:0,ready:false,input:false },
|
||||
{ enabled:true,hook:true,nativeCalls:1,nativeReady:0,ready:true,input:false },
|
||||
]);
|
||||
} finally {
|
||||
clearTimeout(timeout);
|
||||
child.kill('SIGKILL');
|
||||
// The outer watchdog may stop the worker before its session finally runs.
|
||||
for (const name of ['false', 'true', 'crash']) {
|
||||
const pidFile = path.join(dir, name, 'pid');
|
||||
if (!fs.existsSync(pidFile)) continue;
|
||||
const pid = Number(fs.readFileSync(pidFile, 'utf8'));
|
||||
if (!Number.isInteger(pid) || pid <= 0) continue;
|
||||
try {
|
||||
if (process.platform === 'linux' && !fs.readFileSync(`/proc/${pid}/cmdline`, 'utf8').includes(fake)) continue;
|
||||
process.kill(pid, 'SIGKILL');
|
||||
} catch { /* the owned fake child already exited */ }
|
||||
}
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
}, 25_000);
|
||||
@@ -76,6 +76,21 @@ describe('plan-ceo-review carve — static ordering', () => {
|
||||
expect(gate).toBeGreaterThan(stop);
|
||||
});
|
||||
|
||||
test('the loaded test-review section preserves mandatory behaviors and individual assertion decisions', () => {
|
||||
const template = fs.readFileSync(`${SECTION}.tmpl`, 'utf-8');
|
||||
for (const document of [template, section]) {
|
||||
const testReview = document.split('### Section 6: Test Review')[1]?.split('### Section 7:')[0];
|
||||
expect(testReview).toBeDefined();
|
||||
const instructions = testReview!.replace(/\s+/g, ' ');
|
||||
expect(instructions).toContain('Keep user-required behaviors and assertions mandatory unless the user explicitly approves changing them.');
|
||||
expect(instructions).toContain('Never fill a missing behavioral assertion silently in the final report: obtain its own AskUserQuestion decision first.');
|
||||
expect(instructions).toContain('Scope/approach approval is not approval of individual assertion gaps.');
|
||||
expect(instructions).toContain("Helper coverage alone does not prove the caller's path");
|
||||
expect(instructions).toContain('equivalent caller coverage exists or the user explicitly accepted that particular risk.');
|
||||
expect(instructions).toContain('AskUserQuestion once per issue. Do NOT batch.');
|
||||
}
|
||||
});
|
||||
|
||||
test('the section is generated, not hand-edited', () => {
|
||||
expect(section.slice(0, 120)).toContain('AUTO-GENERATED');
|
||||
});
|
||||
|
||||
@@ -142,9 +142,9 @@ describeE2E('/autoplan native chain ordering (periodic)', () => {
|
||||
}
|
||||
}
|
||||
|
||||
// This new repository also asks once to add gstack routing to
|
||||
// CLAUDE.md. Answer only that recognized setup prompt; review and
|
||||
// taste decisions remain autoplan's responsibility. The helper
|
||||
// This new repository offers routing and an optional design-doc
|
||||
// prerequisite. Keep the supplied plan and continue its full
|
||||
// review; taste decisions remain autoplan's responsibility. The helper
|
||||
// deduplicates the complete question before returning an input.
|
||||
const setupInput = autoplanRoutingSetupInput(visible, seenSetupQuestions, transcript.calls.find(call => !call.answered && !call.failed));
|
||||
if (setupInput !== null) {
|
||||
|
||||
@@ -8,7 +8,7 @@ import { runSkillTest } from './helpers/session-runner';
|
||||
import { runCodexSkill } from './helpers/codex-session-runner';
|
||||
import { EvalCollector } from './helpers/eval-store';
|
||||
import { installOutsideReviewFixture } from './helpers/outside-voice-fixture';
|
||||
import { claudeOutsideExecutions, codexOutsideExecutions, foundInvoiceAuthorizationDefect } from './helpers/outside-voice-evidence';
|
||||
import { claudeOutsideExecutions, codexOutsideExecutions, foundInvoiceAuthorizationDefect, outsideExecutionTranscript } from './helpers/outside-voice-evidence';
|
||||
import { CAPTURE_LONG_MS } from './helpers/eval-budgets';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
@@ -82,10 +82,12 @@ describeLive('Installed workflows dispatch outside their host harness', () => {
|
||||
// disposable fixture, and the prompt keeps repository source unchanged.
|
||||
sandbox: 'danger-full-access', timeoutMs: CAPTURE_LONG_MS,
|
||||
});
|
||||
const dispatched = foundInvoiceAuthorizationDefect(codexOutsideExecutions(result.rawLines), 'claude-code');
|
||||
const executions = codexOutsideExecutions(result.rawLines);
|
||||
const dispatched = foundInvoiceAuthorizationDefect(executions, 'claude-code');
|
||||
const passed = result.exitCode === 0 && dispatched;
|
||||
collector?.addTest({ name: 'outside-voice-codex-to-claude-code', suite: 'outside-voice', tier: 'e2e', passed,
|
||||
duration_ms: result.durationMs, cost_usd: 0, output: result.output.slice(0, 2000),
|
||||
transcript: outsideExecutionTranscript(executions, 'claude-code'),
|
||||
turns_used: result.toolCalls.length, exit_reason: result.exitCode === 0 ? 'success' : `exit_${result.exitCode}` });
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(dispatched).toBe(true);
|
||||
@@ -104,10 +106,12 @@ describeLive('Installed workflows dispatch outside their host harness', () => {
|
||||
allowedTools: ['Bash', 'Read', 'Write', 'Grep', 'Glob', 'Agent', 'Skill'], env,
|
||||
testName: 'outside-voice-claude-code-to-codex',
|
||||
});
|
||||
const dispatched = foundInvoiceAuthorizationDefect(claudeOutsideExecutions(result.transcript), 'codex');
|
||||
const executions = claudeOutsideExecutions(result.transcript);
|
||||
const dispatched = foundInvoiceAuthorizationDefect(executions, 'codex');
|
||||
const passed = result.exitReason === 'success' && dispatched;
|
||||
collector?.addTest({ name: 'outside-voice-claude-code-to-codex', suite: 'outside-voice', tier: 'e2e', passed,
|
||||
duration_ms: result.duration, cost_usd: result.costEstimate.estimatedCost, output: result.output.slice(0, 2000),
|
||||
transcript: outsideExecutionTranscript(executions, 'codex'),
|
||||
turns_used: result.costEstimate.turnsUsed, exit_reason: result.exitReason });
|
||||
expect(result.exitReason).toBe('success');
|
||||
expect(dispatched).toBe(true);
|
||||
|
||||
Reference in new issue
Block a user