mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-06 08:54:04 +01:00
test: fix native review completion and eval retry isolation
This commit is contained in:
Garry Tan committed
1 parent
084f75297d
commit
36fa6add62
14 files changed
+1731
-71
No files matched your search
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { autoplanRoutingSetupInput } from './helpers/autoplan-setup-question';
|
||||
import { autoplanRoutingSetupInput, autoplanSetupDecision } from './helpers/autoplan-setup-question';
|
||||
import { E2E_TOUCHFILES, selectTests } from './helpers/touchfiles';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
@@ -500,3 +500,290 @@ await Bun.write(${JSON.stringify(resultFile)},JSON.stringify(results));
|
||||
fs.rmSync(dir, {recursive:true,force:true});
|
||||
}
|
||||
}, 30000);
|
||||
|
||||
|
||||
// Exact current viewport from source-M's routing stall. Owned temporary paths
|
||||
// are retained as display text; no fixture path is accessed by this replay.
|
||||
const M_ROUTING_CAPTURE = "\n\n❯ /autoplan\n\n● Starting the autoplan pipeline — running the preamble first.\n\n● Bash(_SS=\"$HOME/.claude/skills/gstack/bin/gstack-skill-start\"\n [ -x \"$_SS\" ] || _SS=\".claude/skills/gstack/bin/gstack-skill-start\"…)\n ⎿ SKILL_START_PROTO: 1\n BRANCH: main\n PROACTIVE: true \n … +54 lines (ctrl+o to expand)\n ⎿ Allowed by auto mode classifier\n\n● The preamble ran. SESSION_KIND is interactive, SESSION_ID is 1144263-1788912944-701e8cc4. There's a one-time routing\n instruction to handle first.\n\n Let me check if CLAUDE.md exists and explore the repo before presenting the routing question.\n\n Read 1 file, listed 1 directory (ctrl+o to expand)\n────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\nPlanning:\n/tmp/gstack-paid-shard-2DwzUD/tmp/gstack-hermetic-1144068-Ep9FFb/with-skills/.claude/plans/scalable-bouncing-moth.md\n────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\n ☐ Skill routing\n\n│ gstack works best when your project's CLAUDE.md includes skill routing rules. Should I add them now?\n│ <gstack-qid:routing-injection>\n\n❯ 1. Add routing rules (Recommended)\n Append skill routing rules to CLAUDE.md and commit it — /autoplan, /ship, /qa, and other skills will be suggested\n automatically when relevant.\n 2. No thanks, manual only\n Skip for now; you can invoke skills manually anytime. You won't be asked again.\n 3. Type something.\n────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\n 4. Chat about this\n\nEnter to select · ↑/↓ to navigate · Esc to cancel\n";
|
||||
|
||||
describe('M routing manual-only action grammar', () => {
|
||||
test('answers the exact native panel once and preserves the Add choice in either order', () => {
|
||||
const seen = new Set<string>();
|
||||
expect(autoplanRoutingSetupInput(M_ROUTING_CAPTURE, seen)).toBe('1');
|
||||
expect(autoplanRoutingSetupInput(M_ROUTING_CAPTURE, seen)).toBeNull();
|
||||
const reversed = M_ROUTING_CAPTURE
|
||||
.replace('❯ 1. Add routing rules (Recommended)', '❯ 1. No thanks, manual only')
|
||||
.replace(' 2. No thanks, manual only', ' 2. Add routing rules (Recommended)');
|
||||
expect(autoplanRoutingSetupInput(reversed, new Set())).toBe('2');
|
||||
});
|
||||
|
||||
test('equivalent manual actions use the same grammar with or without a courtesy prefix', () => {
|
||||
for (const label of [
|
||||
'No thanks, manual', 'No thanks, manual only', 'Skip — manual only',
|
||||
'Manual', 'Manual only', 'Manual-only', 'Manual invocation', 'Manual invocation only',
|
||||
'No thanks, manual invocation only', 'Invoke skills manually only',
|
||||
"No thanks, I'll invoke skills manually only",
|
||||
]) expect(autoplanRoutingSetupInput(M_ROUTING_CAPTURE.replace('No thanks, manual only', label), new Set()), label).toBe('1');
|
||||
});
|
||||
|
||||
test('manual modifiers do not admit extra actions, other workflows or ambiguous choices', () => {
|
||||
for (const label of [
|
||||
'No thanks, manual data migration only', 'Manual deployment only',
|
||||
'No thanks, invoke the deployment manually only', 'No thanks, manual only then delete CLAUDE.md',
|
||||
'No thanks, skip the review', 'No thanks, proceed with implementation',
|
||||
'No thanks, manual invocation only after deleting the rules', 'Manual only approval',
|
||||
]) expect(autoplanRoutingSetupInput(M_ROUTING_CAPTURE.replace('No thanks, manual only', label), new Set()), label).toBeNull();
|
||||
for (const frame of [
|
||||
M_ROUTING_CAPTURE.replace(' 3. Type something.', ' 3. Manual only'),
|
||||
M_ROUTING_CAPTURE.replace(' 3. Type something.', ' 3. Add routing rules'),
|
||||
M_ROUTING_CAPTURE.replace('Add routing rules (Recommended)', 'Add product routes (Recommended)'),
|
||||
M_ROUTING_CAPTURE.replace('Add routing rules (Recommended)', 'Add ruting rules (Recommended)'),
|
||||
M_ROUTING_CAPTURE.replace("gstack works best when your project's CLAUDE.md includes skill routing rules. Should I add them now?", 'Which application API routing design should we choose?'),
|
||||
M_ROUTING_CAPTURE.replace("gstack works best when your project's CLAUDE.md includes skill routing rules. Should I add them now?", 'The plan quotes gstack skill routing rules in CLAUDE.md. Should we expand the feature?'),
|
||||
]) expect(autoplanRoutingSetupInput(frame, new Set()), frame).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
const UNSUPPORTED_ROUTING = M_ROUTING_CAPTURE.replace('No thanks, manual only', 'Ask me after this review');
|
||||
const unsupportedNative = () => ({
|
||||
sessionId: 'unsupported-routing', toolUseId: 'routing-call', answered: false, failed: false,
|
||||
questions: [{ header: 'Skill routing', question: "gstack works best when your project's CLAUDE.md includes skill routing rules. Should I add them now? <gstack-qid:routing-injection>",
|
||||
options: [{label:'Add routing rules (Recommended)'},{label:'Ask me after this review'}] }],
|
||||
});
|
||||
|
||||
describe('unsupported setup diagnostic state', () => {
|
||||
test('a complete recognized unsupported setup fails explicitly without selecting an action', () => {
|
||||
const seen = new Set<string>();
|
||||
for (const pending of [undefined, unsupportedNative()]) {
|
||||
const result = autoplanSetupDecision(UNSUPPORTED_ROUTING, seen, pending);
|
||||
expect(result.kind).toBe('unsupported_setup');
|
||||
if (result.kind === 'unsupported_setup') {
|
||||
expect(result.setup).toBe('routing');
|
||||
expect(result.options).toEqual([{index:1,label:'Add routing rules (Recommended)'},{index:2,label:'Ask me after this review'}]);
|
||||
expect(result.identitySource).toBe(pending ? 'native-bound' : 'current-native-panel');
|
||||
}
|
||||
expect(seen.size).toBe(0);
|
||||
}
|
||||
expect(autoplanSetupDecision(PREREQUISITE_CAPTURE.replace('Skip — proceed with standard review', 'Ask me later'), new Set()).kind).toBe('unsupported_setup');
|
||||
});
|
||||
|
||||
test('supported input is pure until sent; redraw and delayed metadata then wait', () => {
|
||||
const seen = new Set<string>();
|
||||
const decision = autoplanSetupDecision(M_ROUTING_CAPTURE, seen);
|
||||
expect(decision.kind).toBe('input'); expect(seen.size).toBe(0);
|
||||
if (decision.kind !== 'input') throw Error('Expected supported setup');
|
||||
expect(decision.input).toBe('1');
|
||||
for (const signature of decision.signatures) seen.add(signature);
|
||||
expect(autoplanSetupDecision(M_ROUTING_CAPTURE, seen).kind).toBe('waiting');
|
||||
const native = unsupportedNative(); native.questions[0]!.options[1]!.label = 'No thanks, manual only';
|
||||
expect(autoplanSetupDecision(M_ROUTING_CAPTURE, seen, native).kind).toBe('waiting');
|
||||
expect(autoplanSetupDecision(M_ROUTING_CAPTURE + '\n⏺ Continuing…', seen).kind).toBe('waiting');
|
||||
expect(autoplanSetupDecision(PREREQUISITE_CAPTURE, new Set()).kind).toBe('input');
|
||||
});
|
||||
|
||||
test('a substantive product or taste question mentioning office hours is not an unsupported prerequisite', () => {
|
||||
const fullQuestion = prerequisiteQuestion.question;
|
||||
const unsupported = PREREQUISITE_CAPTURE.replace('Skip — proceed with standard review', 'Ask me after this review');
|
||||
for (const [prompt, first, second] of [
|
||||
['No design doc exists for /office-hours integration. Should we build X or defer Y?', 'Build X', 'Defer Y'],
|
||||
['We should produce a design doc for /office-hours. Which visual style should this product use?', 'Minimal', 'Expressive'],
|
||||
['No design doc exists for /office-hours integration. Should we build X or defer Y?', 'Run /office-hours now', 'Defer Y'],
|
||||
['No design doc found. Run /office-hours first?', 'Run /office-hours now and delete the feature', 'Ask me later'],
|
||||
]) {
|
||||
const native = prerequisiteCall();
|
||||
native.questions[0]!.question = prompt!;
|
||||
native.questions[0]!.options = [{label:first!},{label:second!}];
|
||||
// Reconstruct from the actual full native layout, including footer.
|
||||
const frame = unsupported.replace(/│ No design doc[\s\S]*?Run \/office-hours first\?/, prompt!)
|
||||
.replace('1. Run /office-hours now', '1. ' + first)
|
||||
.replace('2. Ask me after this review', '2. ' + second);
|
||||
for (const pending of [undefined, native]) {
|
||||
expect(autoplanSetupDecision(frame, new Set(), pending).kind, prompt).toBe('unrelated');
|
||||
}
|
||||
}
|
||||
// Existing unsupported offer remains positively identified independently
|
||||
// of the unsupported opposite label; no exact question wording is needed.
|
||||
const native = prerequisiteCall();
|
||||
native.questions[0]!.question = fullQuestion.replace('Run /office-hours first?', 'Would you like to run /office-hours now?');
|
||||
native.questions[0]!.options[1]!.label = 'Ask me after this review';
|
||||
expect(autoplanSetupDecision(unsupported.replace('Run /office-hours first?', 'Would you like to run /office-hours now?'), new Set(), native).kind).toBe('unsupported_setup');
|
||||
});
|
||||
|
||||
test('routing identity still needs its explicit setup action before an unsupported failure', () => {
|
||||
for (const [first, second] of [['React', 'Vue'], ['Accept recommendation', 'Defer finding'], ['Add routing rules (Recommended)', 'Add routing rules (Recommended)']]) {
|
||||
const frame = UNSUPPORTED_ROUTING.replace('1. Add routing rules (Recommended)', '1. ' + first)
|
||||
.replace('2. Ask me after this review', '2. ' + second);
|
||||
const native = unsupportedNative();
|
||||
native.questions[0]!.options = [{label:first!},{label:second!}];
|
||||
for (const pending of [undefined,native]) expect(autoplanSetupDecision(frame,new Set(),pending).kind).toBe('waiting');
|
||||
}
|
||||
});
|
||||
|
||||
test('incomplete, stale, quoted, indented or mixed UI cannot establish unsupported setup', () => {
|
||||
const panel = UNSUPPORTED_ROUTING.slice(UNSUPPORTED_ROUTING.indexOf(' ☐ Skill routing'));
|
||||
for (const frame of [
|
||||
panel.replace('Enter to select · ↑/↓ to navigate · Esc to cancel', ''),
|
||||
panel.replace(' 2. Ask me after this review', ''),
|
||||
panel.replace(' 4. Chat about this', ''),
|
||||
panel.replace('❯ 1.', ' 1.'),
|
||||
panel.replace(' 2.', '❯ 2.'),
|
||||
panel.replace('1. Add', '1. [ ] Add'),
|
||||
panel.replace(' ☐ Skill routing', '← ☐ Skill routing ✔ Submit →'),
|
||||
panel + '\n⏺ Continuing the review now.',
|
||||
panel + '\n❯ 1. Different menu\n 2. Other choice',
|
||||
'Example panel:\n' + panel,
|
||||
'Quoted source:\n' + panel,
|
||||
'```text\n' + panel,
|
||||
'~~~~text\n```\n' + panel,
|
||||
panel.split('\n').map(line => ' ' + line).join('\n'),
|
||||
panel.split('\n').map(line => '> ' + line).join('\n'),
|
||||
]) expect(autoplanSetupDecision(frame, new Set()).kind, frame).not.toBe('unsupported_setup');
|
||||
expect(autoplanSetupDecision('```text\nearlier code\n```\n' + panel, new Set()).kind).toBe('unsupported_setup');
|
||||
const product = panel.replace("gstack works best when your project's CLAUDE.md includes skill routing rules. Should I add them now?", 'Which product API router should we use?');
|
||||
expect(autoplanSetupDecision(product, new Set()).kind).toBe('unrelated');
|
||||
});
|
||||
|
||||
test('mismatched, failed, answered, empty and multi-question metadata cannot diagnose this panel', () => {
|
||||
for (const mutate of [
|
||||
(call: ReturnType<typeof unsupportedNative>) => { call.failed = true; },
|
||||
(call: ReturnType<typeof unsupportedNative>) => { call.answered = true; },
|
||||
(call: ReturnType<typeof unsupportedNative>) => { call.questions = []; },
|
||||
(call: ReturnType<typeof unsupportedNative>) => { call.questions.push(structuredClone(call.questions[0]!)); },
|
||||
(call: ReturnType<typeof unsupportedNative>) => { Object.assign(call.questions[0]!, {multiSelect:true}); },
|
||||
(call: ReturnType<typeof unsupportedNative>) => { call.questions[0]!.header = 'Other question'; },
|
||||
(call: ReturnType<typeof unsupportedNative>) => { call.questions[0]!.question = 'Different question <gstack-qid:routing-injection>'; },
|
||||
(call: ReturnType<typeof unsupportedNative>) => { call.questions[0]!.options[1]!.label = 'Different choice'; },
|
||||
(call: ReturnType<typeof unsupportedNative>) => { call.questions[0]!.options[1]!.label = 'No thanks, manual only'; },
|
||||
]) {
|
||||
const native = unsupportedNative(); mutate(native);
|
||||
expect(autoplanSetupDecision(UNSUPPORTED_ROUTING, new Set(), native).kind).not.toBe('unsupported_setup');
|
||||
}
|
||||
});
|
||||
|
||||
test('a supported native question clipped by the actual viewport preserves its existing input policy', async () => {
|
||||
const {createPtyScreen} = await import('./helpers/pty-screen');
|
||||
const {matchesNativePlanQuestion} = await import('./helpers/claude-pty-runner');
|
||||
const native = unsupportedNative();
|
||||
native.questions[0]!.question += '\n' + Array.from({length:41}, (_,i) =>
|
||||
`Routing context line ${i+1}: keep current project conventions and existing commands.`).join('\n');
|
||||
native.questions[0]!.options[1]!.label = 'No thanks, invoke manually';
|
||||
const frame = `☐ Skill routing\n${native.questions[0]!.question}\n❯ 1. Add routing rules (Recommended)\n 2. No thanks, invoke manually\n 3. Type something.\n 4. Chat about this\nEnter to select · ↑/↓ to navigate · Esc to cancel`;
|
||||
const screen = await createPtyScreen(120,40);
|
||||
try {
|
||||
screen.write(frame.replace(/\n/g,'\r\n'));
|
||||
const visible = await screen.read();
|
||||
expect(visible).not.toContain('☐ Skill routing');
|
||||
expect(matchesNativePlanQuestion(visible,native)).toBe(true);
|
||||
const seen = new Set<string>();
|
||||
const decision = autoplanSetupDecision(visible,seen,native);
|
||||
expect(decision.kind).toBe('input');
|
||||
if (decision.kind !== 'input') throw new Error('Expected supported native input');
|
||||
expect(decision.input).toBe('1');
|
||||
expect(seen.size).toBe(0);
|
||||
for (const signature of decision.signatures) seen.add(signature);
|
||||
expect(autoplanSetupDecision(visible,seen,native).kind).toBe('waiting');
|
||||
} finally { await screen.dispose(); }
|
||||
});
|
||||
});
|
||||
|
||||
test.skipIf(process.platform === 'win32')('real PTY unsupported setup fails after ready with zero input and durable parsed evidence', async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-unsupported-setup-'));
|
||||
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].map(early => ({
|
||||
name: early ? 'early' : 'deferred', early, cwd: path.join(dir, early ? 'early' : 'deferred'),
|
||||
events: path.join(dir, early ? 'early.jsonl' : 'deferred.jsonl'),
|
||||
evalDir: path.join(dir, early ? 'early-artifacts' : 'deferred-artifacts'),
|
||||
frame: UNSUPPORTED_ROUTING, native: unsupportedNative(),
|
||||
}));
|
||||
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.SETUP_DIAGNOSTIC_CASE);
|
||||
const event=value=>fs.appendFileSync(item.events,JSON.stringify(value)+'\n');
|
||||
event({kind:'startup',pid:process.pid});
|
||||
if(item.early){
|
||||
const folder=path.join(process.env.CLAUDE_CONFIG_DIR,'projects','fixture');fs.mkdirSync(folder,{recursive:true});
|
||||
fs.writeFileSync(path.join(folder,item.name+'.jsonl'),JSON.stringify({type:'assistant',sessionId:item.name,isSidechain:false,cwd:process.cwd(),timestamp:new Date().toISOString(),message:{role:'assistant',content:[{type:'tool_use',id:'setup',name:'AskUserQuestion',input:{questions:item.native.questions}}]}})+'\n');
|
||||
}
|
||||
process.stdin.setRawMode?.(true);
|
||||
process.stdin.on('data',data=>event({kind:'input',data:data.toString()}));
|
||||
process.stdout.write('\x1b[2J\x1b[H'+item.frame);
|
||||
process.on('SIGINT',()=>process.exit(0));process.stdin.resume();
|
||||
`);
|
||||
fs.chmodSync(fake, 0o755);
|
||||
const url = (name: string) => pathToFileURL(path.resolve(import.meta.dir, 'helpers', name)).href;
|
||||
fs.writeFileSync(worker, `
|
||||
import * as fs from 'node:fs';
|
||||
import {launchClaudePty} from ${JSON.stringify(url('claude-pty-runner.ts'))};
|
||||
import {autoplanSetupDecision,autoplanRoutingSetupInput} from ${JSON.stringify(url('autoplan-setup-question.ts'))};
|
||||
import {readPlanCountTranscript} from ${JSON.stringify(url('plan-count-transcript.ts'))};
|
||||
import {createPlanCountSnapshotWriter} from ${JSON.stringify(url('plan-count-artifacts.ts'))};
|
||||
const results=[];
|
||||
for(const item of ${JSON.stringify(cases)}){
|
||||
const session=await launchClaudePty({cwd:item.cwd,observeScreen:true,timeoutMs:20000,env:{SETUP_DIAGNOSTIC_CASE:JSON.stringify(item)}});
|
||||
const result={name:item.name,config:session.hermeticConfigDir};
|
||||
try{
|
||||
await session.waitFor('Enter to select',{timeoutMs:10000,pollMs:20});
|
||||
const viewport=await session.currentScreen();
|
||||
const native=readPlanCountTranscript(session.hermeticConfigDir,item.cwd);
|
||||
const pending=native.calls.find(call=>!call.answered&&!call.failed);
|
||||
if(Boolean(pending)!==item.early)throw Error('Readiness did not establish expected metadata state');
|
||||
result.legacyInput=autoplanRoutingSetupInput(viewport,new Set(),pending);
|
||||
const decision=autoplanSetupDecision(viewport,new Set(),pending);
|
||||
if(decision.kind==='input')throw Error('Unexpected guessed input');
|
||||
if(decision.kind!=='unsupported_setup')throw Error('Expected unsupported_setup, got '+decision.kind);
|
||||
const save=createPlanCountSnapshotWriter({EVALS_RUN_ID:item.name,GSTACK_EVAL_DIR:item.evalDir});
|
||||
Object.assign(result,save({skillName:'autoplan',cwd:item.cwd,claudeConfigDir:session.hermeticConfigDir,raw:session.rawOutput(),visible:session.visibleText(),viewport,
|
||||
observation:{state:'unsupported_setup',unsupportedSetup:decision,native,retention:'UI and parsed metadata only; full parent JSONL not guaranteed.'}}));
|
||||
throw Error('UNSUPPORTED_SETUP_DIAGNOSTIC: '+decision.prompt);
|
||||
}catch(error){result.failed=true;result.error=String(error);}
|
||||
finally{await session.close();fs.rmSync(item.cwd,{recursive:true,force:true});}
|
||||
results.push(result);
|
||||
}
|
||||
await Bun.write(${JSON.stringify(resultFile)},JSON.stringify(results));
|
||||
process.exitCode=results.some(result=>result.failed)?1:0;
|
||||
`);
|
||||
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(1);
|
||||
const results = JSON.parse(fs.readFileSync(resultFile, 'utf8'));
|
||||
expect(results.length).toBe(2);
|
||||
for (const [index, result] of results.entries()) {
|
||||
const item = cases[index]!;
|
||||
expect(result.failed).toBe(true);
|
||||
expect(result.error).toContain('UNSUPPORTED_SETUP_DIAGNOSTIC:');
|
||||
expect(result.legacyInput).toBeNull();
|
||||
expect(result.artifactError).toBeUndefined();
|
||||
const artifact = JSON.parse(fs.readFileSync(path.join(result.artifactDir, 'observation.json'), 'utf8'));
|
||||
expect(artifact.state).toBe('unsupported_setup');
|
||||
expect(artifact.native.calls.length).toBe(item.early ? 1 : 0);
|
||||
expect(artifact.retention).toContain('full parent JSONL not guaranteed');
|
||||
expect(fs.readFileSync(path.join(result.artifactDir, 'terminal.screen.log'), 'utf8')).toContain('Ask me after this review');
|
||||
expect(fs.readFileSync(path.join(result.artifactDir, 'terminal.raw.log'), 'utf8')).toContain('routing-injection');
|
||||
expect(fs.existsSync(item.cwd)).toBe(false);
|
||||
expect(fs.existsSync(result.config)).toBe(false);
|
||||
const events = fs.readFileSync(item.events, 'utf8').trim().split('\n').map(line => JSON.parse(line));
|
||||
expect(events.filter(event => event.kind === 'input')).toEqual([]);
|
||||
expect(() => process.kill(events[0].pid, 0)).toThrow();
|
||||
}
|
||||
} finally {
|
||||
clearTimeout(killer); child.kill('SIGKILL');
|
||||
for (const item of cases) {
|
||||
if (!fs.existsSync(item.events)) continue;
|
||||
const pid = JSON.parse(fs.readFileSync(item.events, 'utf8').split('\n')[0]!).pid;
|
||||
if (process.platform === 'linux') {
|
||||
try { if (fs.readFileSync('/proc/' + pid + '/cmdline', 'utf8').split('\0').includes(fake)) process.kill(pid, 'SIGKILL'); }
|
||||
catch { /* already reaped or PID no longer belongs to this fixture */ }
|
||||
}
|
||||
}
|
||||
fs.rmSync(dir, {recursive:true,force:true});
|
||||
}
|
||||
}, 30000);
|
||||
@@ -0,0 +1,261 @@
|
||||
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, 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 captured from './fixtures/ceo-completion-handoff-m-call.json';
|
||||
|
||||
const calls = () => structuredClone(captured.calls) as NativePlanQuestionCall[];
|
||||
const handoff = () => calls().at(-1)!;
|
||||
const fingerprint = (call: NativePlanQuestionCall) => nativePlanCallFingerprint(call, 0, false);
|
||||
|
||||
function reanswer(call: NativePlanQuestionCall) {
|
||||
call.answers = { [call.questions[0]!.question]: call.questions[0]!.options[0]!.label };
|
||||
return call;
|
||||
}
|
||||
|
||||
describe('CEO completion described by a native navigation choice', () => {
|
||||
test('the exact seven-call session preserves three setup and three finding decisions', () => {
|
||||
let reviewStarted = false;
|
||||
const counts = { setup: 0, review: 0, administrative: 0 };
|
||||
const original = calls();
|
||||
for (const call of original) {
|
||||
const phase = planCountQuestionPhase(fingerprint(call), reviewStarted, ceoStep0Boundary,
|
||||
ceoFirstReviewAUQ, undefined, isCeoCompletionHandoff);
|
||||
reviewStarted = phase.reviewStarted;
|
||||
if (phase.administrative) counts.administrative++;
|
||||
else if (phase.preReview) counts.setup++;
|
||||
else counts.review++;
|
||||
}
|
||||
expect(counts).toEqual({ setup: 3, review: 3, administrative: 1 });
|
||||
expect(original).toEqual(calls());
|
||||
expect(original.slice(3, -1).map(call => isCeoCompletionHandoff(fingerprint(call)))).toEqual([false, false, false]);
|
||||
});
|
||||
|
||||
test('the active pending handoff selects the actual manual option in either order', () => {
|
||||
for (const reverse of [false, true]) {
|
||||
const call = handoff();
|
||||
call.answered = false;
|
||||
delete call.answers;
|
||||
delete call.unansweredQuestionIndices;
|
||||
const q = call.questions[0]!;
|
||||
if (reverse) q.options.reverse();
|
||||
const screen = `☐ ${q.header}\n${q.question}\n❯ 1. ${q.options[0]!.label}\n 2. ${q.options[1]!.label}\nEnter to select · ↑/↓ to navigate · Esc to cancel`;
|
||||
const active = capturePlanCountQuestion(screen, new Set(), 0, false, call)!;
|
||||
expect(active.nativeCall?.toolUseId).toBe(call.toolUseId);
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call), active)).toBe(reverse ? 1 : 2);
|
||||
expect(isCeoCompletionHandoff(active)).toBe(false);
|
||||
expect(pickCeoCompletionHandoff(capturePlanCountQuestion(screen, new Set(), 0, false)!)).toBeNull();
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call), { ...active, signature: 'another:call' })).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
test('completion placement is independent of the next-step wording and option order', () => {
|
||||
const call = handoff();
|
||||
const q = call.questions[0]!;
|
||||
q.question = 'D9 — Next steps: The review is done. Where should we go next? <gstack-qid:plan-ceo-review-next-step>';
|
||||
q.header = 'Next review';
|
||||
q.options[0]!.description = 'Eng review is the required shipping gate.';
|
||||
for (const description of [
|
||||
'CEO review found 3 specification gaps (all resolved). Continue manually.',
|
||||
'The CEO review identified gaps; all findings are resolved. Continue manually.',
|
||||
'CEO review is complete with 0 unresolved decisions. Continue manually.',
|
||||
]) {
|
||||
q.options[1]!.description = description;
|
||||
expect(isCeoCompletionHandoff(fingerprint(reanswer(call)))).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
test('conditional, unfinished, quoted and non-CEO recaps cannot supply completion', () => {
|
||||
for (const description of [
|
||||
'Eng review is the required shipping gate. CEO review found 3 gaps (all resolved after adding tests).',
|
||||
'Eng review is the required shipping gate. If CEO review found 3 gaps (all resolved), continue.',
|
||||
'Eng review is the required shipping gate. CEO review found 3 gaps (all resolved); one gap remains.',
|
||||
'Eng review is the required shipping gate. CEO review found 3 gaps (all resolved). There is an unresolved test issue.',
|
||||
'Eng review is the required shipping gate. The document says "CEO review found 3 gaps (all resolved)."',
|
||||
'Eng review is the required shipping gate. Design review found 3 gaps (all resolved).',
|
||||
'Eng review is the required shipping gate. CEO review found 3 gaps.',
|
||||
'Eng review is the required shipping gate. CEO review found 3 specification gaps (not all resolved).',
|
||||
'Eng review is the required shipping gate. CEO review did not find all gaps resolved.',
|
||||
'Eng review is the required shipping gate. CEO review found 3 gaps (all resolved). Also add a new test before proceeding.',
|
||||
'Eng review is the required shipping gate. CEO review found 3 gaps (all resolved). Please fix the new missing authorization check before proceeding.',
|
||||
]) {
|
||||
const call = handoff();
|
||||
call.questions[0]!.options[0]!.description = description;
|
||||
expect(isCeoCompletionHandoff(fingerprint(call)), description).toBe(false);
|
||||
call.answered = false;
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call))).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
test('native identity, completion, required gate and exclusively administrative choices remain necessary', () => {
|
||||
for (const mutate of [
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.question = 'Review complete only after fixing tests. What next? <gstack-qid:plan-ceo-review-next-step>'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.question = 'Should we finish reviewing? <gstack-qid:plan-ceo-review-next-step>'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.question += ' <gstack-qid:plan-ceo-security-issue>'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.question = '<gstack-qid broken> ' + call.questions[0]!.question; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.header = 'TODO decision'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.options[1]!.label = 'Add another TODO'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.options[0]!.label += ' and fix the missing test'; },
|
||||
(call: NativePlanQuestionCall) => { for (const option of call.questions[0]!.options) option.description = option.description?.replaceAll('required', 'optional'); },
|
||||
(call: NativePlanQuestionCall) => { call.questions.push(calls()[3]!.questions[0]!); },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.multiSelect = true; },
|
||||
(call: NativePlanQuestionCall) => { call.failed = true; },
|
||||
(call: NativePlanQuestionCall) => { call.unansweredQuestionIndices = [0]; },
|
||||
(call: NativePlanQuestionCall) => { call.answered = false; },
|
||||
]) {
|
||||
const call = handoff();
|
||||
mutate(call);
|
||||
expect(isCeoCompletionHandoff(fingerprint(reanswer(call)))).toBe(false);
|
||||
}
|
||||
const addedWork = handoff();
|
||||
addedWork.answers = { [addedWork.questions[0]!.question]: 'First add another payment test' };
|
||||
expect(isCeoCompletionHandoff(fingerprint(addedWork))).toBe(false);
|
||||
});
|
||||
|
||||
test('the actual report and Exit order permits only the administrative freshness exception', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ceo-native-handoff-'));
|
||||
const report = path.join(dir, 'plan.md');
|
||||
try {
|
||||
fs.writeFileSync(report, captured.report.content);
|
||||
const reportAt = Date.parse(captured.report.successfulResult.timestamp) / 1000;
|
||||
fs.utimesSync(report, reportAt, reportAt);
|
||||
const transcript = { status: 'ready' as const, calls: calls(), assistantMessages: [],
|
||||
planReadyRequests: structuredClone(captured.planReadyRequests) };
|
||||
const administrative = new Set(transcript.calls.filter(call => isCeoCompletionHandoff(fingerprint(call)))
|
||||
.map(call => `${call.sessionId}:${call.toolUseId}`));
|
||||
const startedAt = Date.parse('2026-09-09T00:15:27Z');
|
||||
expect(administrative.size).toBe(1);
|
||||
expect(hasNativePlanTerminal(transcript, report, startedAt, 'plan_ready')).toBe(false);
|
||||
expect(hasNativePlanTerminal(transcript, report, startedAt, 'plan_ready', administrative)).toBe(true);
|
||||
transcript.planReadyRequests[0]!.failed = true;
|
||||
expect(hasNativePlanTerminal(transcript, report, startedAt, 'plan_ready', administrative)).toBe(false);
|
||||
transcript.planReadyRequests[0]!.failed = false;
|
||||
transcript.calls.push({ ...structuredClone(transcript.calls[3]!), toolUseId: 'new-test-obligation',
|
||||
answeredAt: captured.calls.at(-1)!.answeredAt });
|
||||
expect(hasNativePlanTerminal(transcript, report, startedAt, 'plan_ready', administrative)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('native next-review navigation with a resolved CEO recap', () => {
|
||||
const retryCalls = () => structuredClone(captured.distinctRetry.calls) as NativePlanQuestionCall[];
|
||||
const retryHandoff = () => retryCalls().at(-1)!;
|
||||
|
||||
test('the captured retry preserves its four actual findings and the unchanged mechanical band', () => {
|
||||
let started = false;
|
||||
const counts = { setup: 0, review: 0, administrative: 0 };
|
||||
const original = retryCalls();
|
||||
for (const call of original) {
|
||||
const phase = planCountQuestionPhase(fingerprint(call), started, ceoStep0Boundary,
|
||||
ceoFirstReviewAUQ, undefined, isCeoCompletionHandoff);
|
||||
started = phase.reviewStarted;
|
||||
if (phase.administrative) counts.administrative++;
|
||||
else if (phase.preReview) counts.setup++;
|
||||
else counts.review++;
|
||||
}
|
||||
expect(counts).toEqual({ setup: 4, review: 4, administrative: 1 });
|
||||
expect(original).toEqual(retryCalls());
|
||||
// The transcript contains four individual findings. The unasked dispatcher
|
||||
// remedy remains a separate workflow-quality limitation, never a fifth call.
|
||||
expect(original.slice(4, -1).every(call => !isCeoCompletionHandoff(fingerprint(call)))).toBe(true);
|
||||
});
|
||||
|
||||
test('actual offered manual navigation still requires the matching pending native question', () => {
|
||||
for (const reverse of [false, true]) {
|
||||
const call = retryHandoff();
|
||||
call.answered = false;
|
||||
delete call.answers;
|
||||
const q = call.questions[0]!;
|
||||
if (reverse) q.options.reverse();
|
||||
const screen = `☐ ${q.header}\n${q.question}\n❯ 1. ${q.options[0]!.label}\n 2. ${q.options[1]!.label}\nEnter to select · ↑/↓ to navigate · Esc to cancel`;
|
||||
const active = capturePlanCountQuestion(screen, new Set(), 0, false, call)!;
|
||||
expect(active.nativeCall?.toolUseId).toBe(call.toolUseId);
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call), active)).toBe(reverse ? 1 : 2);
|
||||
expect(isCeoCompletionHandoff(active)).toBe(false);
|
||||
expect(pickCeoCompletionHandoff(capturePlanCountQuestion(screen, new Set(), 0, false)!)).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
test('partial, conditional, quoted or still-open recaps never establish this navigation boundary', () => {
|
||||
for (const recap of [
|
||||
'This CEO review resolved some security bugs.',
|
||||
'This CEO review resolved most security bugs.',
|
||||
'This CEO review resolved all but one security bugs.',
|
||||
'This CEO review resolved two of three security bugs.',
|
||||
'This CEO review only resolved the security bugs.',
|
||||
'This CEO review did not resolve the security bugs.',
|
||||
'If this CEO review resolved the security bugs, continue.',
|
||||
'The document says "This CEO review resolved the security bugs."',
|
||||
'This CEO review resolved the security bugs. One issue remains unresolved.',
|
||||
'This CEO review resolved the security bugs; validation of that remedy is still pending.',
|
||||
'This CEO review resolved the security bugs. Please add a new test first.',
|
||||
'This CEO review will resolve the security bugs.',
|
||||
]) {
|
||||
const call = retryHandoff();
|
||||
call.questions[0]!.options[0]!.description = 'Eng review is the required shipping gate. ' + recap;
|
||||
expect(isCeoCompletionHandoff(fingerprint(call)), recap).toBe(false);
|
||||
call.answered = false;
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call))).toBeNull();
|
||||
}
|
||||
for (const question of [
|
||||
'Should we add a missing authorization test as the next step after this CEO review?',
|
||||
'The CEO review did not finish. What is the next step after this CEO review?',
|
||||
'Can you first fix the missing authorization check as the next step after this CEO review?',
|
||||
'If the CEO review finishes, what is the next step after this CEO review?',
|
||||
'Example: What is the next step after this CEO review?',
|
||||
]) {
|
||||
const call = retryHandoff();
|
||||
call.questions[0]!.question = question + ' <gstack-qid:plan-ceo-next-step>';
|
||||
reanswer(call);
|
||||
expect(isCeoCompletionHandoff(fingerprint(call)), question).toBe(false);
|
||||
call.answered = false;
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call)), question).toBeNull();
|
||||
}
|
||||
for (const mutate of [
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.question = 'Choose a fix for the missing test <gstack-qid:plan-ceo-next-step>'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.question = call.questions[0]!.question.replace('plan-ceo-next-step', 'plan-ceo-test-gap'); },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.question = call.questions[0]!.question.replace(/ <gstack-qid:[^>]+>/, ''); },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.header = 'TODO'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.options[1]!.label = 'Add a missing receipt assertion'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions.push(retryCalls()[4]!.questions[0]!); },
|
||||
(call: NativePlanQuestionCall) => { call.failed = true; },
|
||||
(call: NativePlanQuestionCall) => { call.unansweredQuestionIndices = [0]; },
|
||||
]) {
|
||||
const call = retryHandoff();
|
||||
mutate(call);
|
||||
expect(isCeoCompletionHandoff(fingerprint(reanswer(call)))).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test('the final native report edit precedes handoff and still covers every real answer', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ceo-retry-handoff-'));
|
||||
const report = path.join(dir, 'plan.md');
|
||||
try {
|
||||
fs.writeFileSync(report, captured.distinctRetry.reportContent);
|
||||
const reportAt = Date.parse(captured.distinctRetry.reportUpdate.at(-1)!.timestamp) / 1000;
|
||||
fs.utimesSync(report, reportAt, reportAt);
|
||||
const transcript = { status: 'ready' as const, calls: retryCalls(), assistantMessages: [],
|
||||
planReadyRequests: structuredClone(captured.distinctRetry.planReadyRequests) };
|
||||
const administrative = new Set(transcript.calls.filter(call => isCeoCompletionHandoff(fingerprint(call)))
|
||||
.map(call => `${call.sessionId}:${call.toolUseId}`));
|
||||
const startedAt = Date.parse('2026-09-09T00:23:30Z');
|
||||
expect(administrative.size).toBe(1);
|
||||
expect(hasNativePlanTerminal(transcript, report, startedAt, 'plan_ready')).toBe(false);
|
||||
expect(hasNativePlanTerminal(transcript, report, startedAt, 'plan_ready', administrative)).toBe(true);
|
||||
transcript.planReadyRequests[0]!.failed = true;
|
||||
expect(hasNativePlanTerminal(transcript, report, startedAt, 'plan_ready', administrative)).toBe(false);
|
||||
transcript.planReadyRequests[0]!.failed = false;
|
||||
transcript.calls.push({ ...structuredClone(transcript.calls[4]!), toolUseId: 'new-independent-finding',
|
||||
answeredAt: transcript.calls.at(-1)!.answeredAt });
|
||||
expect(hasNativePlanTerminal(transcript, report, startedAt, 'plan_ready', administrative)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
+562
@@ -0,0 +1,562 @@
|
||||
{
|
||||
"sourceHead": "084f75297dfe33530baa0cce33b6ec60faf0cc99",
|
||||
"artifact": "plan-ceo-review-1788912957495-fB0byY",
|
||||
"nativeSha256": "275cf3bfb1f78699abef1570c04e037833ab28c939c5db5e500e20b0efb47e77",
|
||||
"rawCounts": {
|
||||
"setup": 3,
|
||||
"review": 4,
|
||||
"administrative": 0
|
||||
},
|
||||
"calls": [
|
||||
{
|
||||
"sessionId": "5036f6f6-8360-4f76-bea8-417f5c727b32",
|
||||
"toolUseId": "toolu_013RgeeqwFtbTgK5PST95ywa",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 No design doc found. Run /office-hours first, or proceed with the standard review? <gstack-qid:plan-ceo-review-office-hours-prereq>",
|
||||
"header": "Prereq skill",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Skip \u2014 proceed with review (Recommended)",
|
||||
"description": "The plan is already well-scoped (2 specific tests, documented infrastructure). /office-hours would add little here. Proceed directly to the CEO review."
|
||||
},
|
||||
{
|
||||
"label": "Run /office-hours first",
|
||||
"description": "Produces a structured problem statement, premise challenge, and explored alternatives \u2014 gives the review sharper input. Takes ~10 min. We'll pick up the review right after."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D1 \u2014 No design doc found. Run /office-hours first, or proceed with the standard review? <gstack-qid:plan-ceo-review-office-hours-prereq>": "Skip \u2014 proceed with review (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:17:45.716Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "5036f6f6-8360-4f76-bea8-417f5c727b32",
|
||||
"toolUseId": "toolu_01Gxc4Dzcfghy2XJ44dwociF",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D2 \u2014 Which implementation approach for the test coverage? <gstack-qid:plan-ceo-review-approach-selection>",
|
||||
"header": "Approach",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Minimal (Recommended)",
|
||||
"description": "Exactly 2 tests as written: happy path + 502 retry exhaustion. Uses existing factory and virtual sleeper. Completeness: 7/10. Effort: human ~30 min / CC ~5 min. Closes the stated gap cleanly."
|
||||
},
|
||||
{
|
||||
"label": "B) Expanded matrix",
|
||||
"description": "Add 402 (card decline) and 429 (rate limit) function-level tests alongside the 2 planned. Already infrastructure-supported \u2014 same factory, same mocks. Completeness: 9/10. Effort: human ~45 min / CC ~8 min. Closes the full function-level gap while scope is open."
|
||||
},
|
||||
{
|
||||
"label": "C) Table-driven / parameterized",
|
||||
"description": "Restructure as parameterized tests covering all error paths. DRY and scales well. May require factory extensions. Completeness: 9/10. Effort: human ~1 hr / CC ~15 min. Cleaner structure, higher initial complexity."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D2 \u2014 Which implementation approach for the test coverage? <gstack-qid:plan-ceo-review-approach-selection>": "A) Minimal (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:18:27.821Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "5036f6f6-8360-4f76-bea8-417f5c727b32",
|
||||
"toolUseId": "toolu_01YAqKUDJyGUgSLjFWNdLxC9",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D3 \u2014 Which review mode for this plan? <gstack-qid:plan-ceo-review-mode-selection>",
|
||||
"header": "Review mode",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "HOLD SCOPE (Recommended)",
|
||||
"description": "The 2 tests are the scope. Review for maximum rigor: assertion correctness, edge cases, test isolation, naming, factory usage. No expansions surfaced."
|
||||
},
|
||||
{
|
||||
"label": "SELECTIVE EXPANSION",
|
||||
"description": "Hold the 2-test baseline, but surface the 402/429 expansion opportunities individually so you can cherry-pick. You decide what gets added."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D3 \u2014 Which review mode for this plan? <gstack-qid:plan-ceo-review-mode-selection>": "HOLD SCOPE (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:18:37.843Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "5036f6f6-8360-4f76-bea8-417f5c727b32",
|
||||
"toolUseId": "toolu_0145EK1Vb4mBrAEd1vsHFnC9",
|
||||
"questions": [
|
||||
{
|
||||
"question": "Finding 1 (Section 2) \u2014 What does 'fails clean' mean for the 502 test assertion? The plan says 'fails clean' but doesn't specify what the test should assert after retries are exhausted. <gstack-qid:plan-ceo-review-fails-clean-spec>",
|
||||
"header": "502 assertion",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Assert a specific exception is raised (Recommended)",
|
||||
"description": "Test asserts processPayment() raises a specific exception class (e.g., PaymentError, StripeError, or similar) when 502 retries are exhausted. Update the plan to name the exception class. Verifiable: the test fails if the wrong exception or no exception is raised."
|
||||
},
|
||||
{
|
||||
"label": "B) Assert an error return value",
|
||||
"description": "Test asserts processPayment() returns a failure object/nil/false. Update the plan to specify the return shape. Verifiable: test fails if success is returned."
|
||||
},
|
||||
{
|
||||
"label": "C) Leave as-is \u2014 implementer decides",
|
||||
"description": "Keep 'fails clean' as the spec. The implementer matches whatever the existing behavior is. Risk: the test may silently pass on wrong behavior if the assertion is too loose."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"Finding 1 (Section 2) \u2014 What does 'fails clean' mean for the 502 test assertion? The plan says 'fails clean' but doesn't specify what the test should assert after retries are exhausted. <gstack-qid:plan-ceo-review-fails-clean-spec>": "A) Assert a specific exception is raised (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:19:38.037Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "5036f6f6-8360-4f76-bea8-417f5c727b32",
|
||||
"toolUseId": "toolu_01GNKjDq8UJMakRW1jCmNFhe",
|
||||
"questions": [
|
||||
{
|
||||
"question": "Finding 2 (Section 4) \u2014 'Assert correct receipt is generated' is underspecified. What should the happy path test actually assert about the receipt? <gstack-qid:plan-ceo-review-receipt-assertion>",
|
||||
"header": "Receipt spec",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Assert specific receipt fields (Recommended)",
|
||||
"description": "Test verifies at minimum: receipt amount matches charge amount, and customer/transaction ID is present and non-nil. Update plan to name the fields. Makes the test fail on wrong data, not just on nil receipt."
|
||||
},
|
||||
{
|
||||
"label": "B) Assert receipt is non-nil only",
|
||||
"description": "Test checks receipt is returned (not nil/false). Simpler, but doesn't catch data corruption or wrong-amount bugs. Completeness: 5/10 as a correctness test."
|
||||
},
|
||||
{
|
||||
"label": "C) Assert receipt matches a snapshot/fixture",
|
||||
"description": "Test compares the full receipt against a known-good fixture. Catches all field regressions but brittle to non-functional changes (formatting, field order). Best if receipt schema is stable."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"Finding 2 (Section 4) \u2014 'Assert correct receipt is generated' is underspecified. What should the happy path test actually assert about the receipt? <gstack-qid:plan-ceo-review-receipt-assertion>": "A) Assert specific receipt fields (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:19:56.074Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "5036f6f6-8360-4f76-bea8-417f5c727b32",
|
||||
"toolUseId": "toolu_01Cs55t94c1ENEJx9KUupDEj",
|
||||
"questions": [
|
||||
{
|
||||
"question": "Finding 3 (Section 6) \u2014 The virtual sleeper records backoff duration. Should the 502 test assert the sleep duration was correct? <gstack-qid:plan-ceo-review-backoff-assertion>",
|
||||
"header": "Backoff assert",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Assert sleep count only \u2014 not duration (Recommended)",
|
||||
"description": "Test verifies virtual_sleeper.calls.length == 1 (exactly one sleep occurred between the two charge attempts). Catches missing or extra retries without brittleness around the specific timing formula."
|
||||
},
|
||||
{
|
||||
"label": "B) Assert sleep count AND expected duration",
|
||||
"description": "Test verifies virtual_sleeper.calls == [expected_backoff_ms]. Catches broken backoff formula too, but couples the test to the timing implementation. Update plan to specify the expected duration."
|
||||
},
|
||||
{
|
||||
"label": "C) Don\u2019t assert the sleeper at all",
|
||||
"description": "Two charge attempts (from Stripe mock call history) are sufficient proof of retry. Skip the sleeper assertion entirely. Risk: if retry fires without sleeping, test still passes."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"Finding 3 (Section 6) \u2014 The virtual sleeper records backoff duration. Should the 502 test assert the sleep duration was correct? <gstack-qid:plan-ceo-review-backoff-assertion>": "A) Assert sleep count only \u2014 not duration (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:20:12.131Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "5036f6f6-8360-4f76-bea8-417f5c727b32",
|
||||
"toolUseId": "toolu_01DNHtxrJdk1mb8cYyfmBh9B",
|
||||
"questions": [
|
||||
{
|
||||
"question": "Review complete. What's the next step? <gstack-qid:plan-ceo-review-next-step>",
|
||||
"header": "Next step",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Run /plan-eng-review next (Recommended)",
|
||||
"description": "Eng review is the required shipping gate. It covers architecture, code quality, tests, and performance. CEO review found 3 specification gaps (all resolved); eng review validates the implementation plan is sound."
|
||||
},
|
||||
{
|
||||
"label": "B) Skip \u2014 I'll handle reviews manually",
|
||||
"description": "Proceed without running eng review. Note: eng review is the required gate before shipping."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"Review complete. What's the next step? <gstack-qid:plan-ceo-review-next-step>": "A) Run /plan-eng-review next (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:24:14.771Z"
|
||||
}
|
||||
],
|
||||
"report": {
|
||||
"id": "toolu_01Ndio4NgC8czFMD2XqEaaZ1",
|
||||
"timestamp": "2026-09-09T00:23:12.466Z",
|
||||
"path": "/tmp/gstack-paid-shard-Q1hRY6/tmp/gstack-e2e-plan-ceo-paired-lEplIi/gstack-test-plan-ceo-paired.md",
|
||||
"content": "# Plan Review: Payment Processing \u2014 Test Coverage\n\n## Context\n\n`processPayment()` is a revenue-critical function with zero function-level unit tests. The Stripe adapter suite and receipt-builder regression tests cover their own layers, but nothing verifies that `processPayment()` itself correctly integrates those layers. This review closes that gap for the two most critical paths: the success path (correctness) and the 502 retry exhaustion path (graceful degradation).\n\n## Review Summary\n\n- **Mode:** HOLD SCOPE\n- **Approach:** Minimal \u2014 exactly 2 tests as specified, using existing test infrastructure\n- **Findings:** 3 specification gaps resolved (exception class, receipt fields, backoff assertion)\n- **Critical gaps:** 0\n\n---\n\n## Step 0 Decisions\n\n- **Implementation approach:** A (Minimal) \u2014 exactly 2 tests reusing existing factory, virtual sleeper, and Stripe mock call history\n- **Mode:** HOLD SCOPE \u2014 the plan's scope is correct; make it bulletproof\n\n---\n\n## Section 1: Architecture\n\nTest-only change. No production architecture changes.\n\n```\nEXISTING ARCHITECTURE (unchanged)\n processPayment() \u2500\u2500\u2192 StripeAdapter \u2500\u2500\u2192 Stripe API\n \u2502 \u2502\n \u25bc \u25bc\n ReceiptBuilder retry/backoff\n\nTEST ADDITIONS (new)\n HappyPathTest \u2500\u2500\u2192 payment_factory(max_retries=1)\n \u2502\u2500\u2500 Stripe mock (200 success)\n \u2502\u2500\u2500 virtual_sleeper (should NOT be called)\n \u2500\u2500\u2192 assert receipt.amount, receipt.customer_id, receipt.transaction_id\n\n 502Test \u2500\u2500\u2192 payment_factory(max_retries=1)\n \u2502\u2500\u2500 Stripe mock (502 \u2192 502)\n \u2502\u2500\u2500 virtual_sleeper.calls.length == 1\n \u2500\u2500\u2192 assert Stripe mock called exactly 2 times\n \u2500\u2500\u2192 assert specific exception class raised\n```\n\nNo new coupling, no single points of failure. Rollback: trivial (delete test file).\n\nNo issues.\n\n---\n\n## Section 2: Error & Rescue Map\n\n```\nCODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS\n---------------------|--------------------------------|---------------------------\nprocessPayment() | Stripe 502 (retries exhausted) | <name from production code>\n | Stripe 200 success | N/A\n\nFAILURE | RESCUED? | RESCUE ACTION | TESTED?\n----------------|----------|--------------------------------|--------\n502 exhaustion | Y | retry once (max_retries=1), | YES (test 2)\n | | then raise specific exception |\n```\n\n**Resolved (Finding 1):** \"fails clean\" is clarified as: the test asserts a specific named exception class is raised when 502 retries are exhausted. The implementer must inspect the production `processPayment()` error handling to find the correct exception class name and use it explicitly in the test assertion.\n\nNo critical gaps.\n\n---\n\n## Section 3: Security\n\nTest code only. No new endpoints, credentials, or data access. No issues.\n\n---\n\n## Section 4: Data Flow & Interaction Edge Cases\n\n```\nHAPPY PATH:\n payment_params \u2500\u2500\u2192 processPayment() \u2500\u2500\u2192 StripeAdapter mock (200)\n \u2502\n \u25bc\n ReceiptBuilder \u2500\u2500\u2192 receipt\n \u2502\n ASSERT: receipt.amount == charge_amount\n ASSERT: receipt.customer_id is present\n ASSERT: receipt.transaction_id is present\n\n502 ERROR PATH:\n payment_params \u2500\u2500\u2192 processPayment() \u2500\u2500\u2192 StripeAdapter mock (502)\n \u2502 \u2502\n \u25bc \u25bc\n virtual_sleeper.sleep() \u2500\u2500\u25b6 StripeAdapter mock (502)\n \u2502\n RETRIES EXHAUSTED (max_retries=1: 2 total attempts)\n \u2502\n ASSERT: Stripe mock call_history.length == 2\n ASSERT: virtual_sleeper.calls.length == 1\n ASSERT: raises SpecificExceptionClass\n```\n\n**Resolved (Finding 2):** \"assert correct receipt is generated\" is clarified as: assert `receipt.amount == charge_amount`, `receipt.customer_id` is present and non-nil, and `receipt.transaction_id` is present and non-nil. This catches data corruption and wrong-amount bugs, not just nil receipt.\n\nNo unhandled edge cases within scope.\n\n---\n\n## Section 5: Code Quality\n\nNo implementation code to review. TwoLine truncated
|
||||
"successfulResult": {
|
||||
"timestamp": "2026-09-09T00:23:13.042Z",
|
||||
"isError": false,
|
||||
"content": "File created successfully at: /tmp/gstack-paid-shard-Q1hRY6/tmp/gstack-e2e-plan-ceo-paired-lEplIi/gstack-test-plan-ceo-paired.md (file state is current in your context \u2014 no need to Read it back)"
|
||||
}
|
||||
},
|
||||
"planReadyRequests": [
|
||||
{
|
||||
"sessionId": "5036f6f6-8360-4f76-bea8-417f5c727b32",
|
||||
"toolUseId": "toolu_01HLTX6zXpbrSuRWv1u3hKAR",
|
||||
"timestamp": "2026-09-09T00:24:23.441Z",
|
||||
"failed": false
|
||||
}
|
||||
],
|
||||
"distinctRetry": {
|
||||
"calls": [
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_013YFPiqij263H7pMNfX8hau",
|
||||
"questions": [
|
||||
{
|
||||
"question": "Add gstack skill routing rules to this project's CLAUDE.md? <gstack-qid:routing-injection>",
|
||||
"header": "Routing rules",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add routing rules (recommended)",
|
||||
"description": "Appends skill routing rules to CLAUDE.md so gstack automatically suggests the right skill for each task type (plan review, QA, ship, etc.) without needing to type the slash command."
|
||||
},
|
||||
{
|
||||
"label": "No thanks, invoke manually",
|
||||
"description": "Skip routing rules. You'll invoke skills manually with slash commands each time."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"Add gstack skill routing rules to this project's CLAUDE.md? <gstack-qid:routing-injection>": "Add routing rules (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:27:11.829Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_01GbewHUBvWHxXvUVdRGNf1e",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D2 \u2014 No design doc found for this branch. Run /office-hours first, or proceed directly to the plan review? <gstack-qid:plan-ceo-prereq-office-hours>",
|
||||
"header": "office-hours?",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Skip \u2014 review the plan as-is (recommended)",
|
||||
"description": "You already have PLAN.md written. Proceed with the CEO review directly. Best when the problem is well-understood and the plan is ready for scrutiny."
|
||||
},
|
||||
{
|
||||
"label": "Run /office-hours first",
|
||||
"description": "Run /office-hours to produce a structured problem statement and explored alternatives before reviewing. Useful when you want sharper input but adds ~10 minutes upfront."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D2 \u2014 No design doc found for this branch. Run /office-hours first, or proceed directly to the plan review? <gstack-qid:plan-ceo-prereq-office-hours>": "Skip \u2014 review the plan as-is (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:27:33.912Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_01PZULs2ng16FwGgSHrH91ZP",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D3 \u2014 Which implementation approach for the Stripe webhook handler? <gstack-qid:plan-ceo-approach-selection>",
|
||||
"header": "Approach",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Minimal Secure Fix (recommended)",
|
||||
"description": "Parameterized queries, email error handling, basic tests, N+1 fix. Smallest complete-secure diff. Retains the namespace-separation architecture. Human ~1 day / CC ~15 min. Completeness: 7/10."
|
||||
},
|
||||
{
|
||||
"label": "B) Ideal Architecture",
|
||||
"description": "Async email via job queue, comprehensive tests at every layer, batch queries, WebhookDispatcher evaluation. Production-grade from day one. Human ~3 days / CC ~45 min. Completeness: 10/10."
|
||||
},
|
||||
{
|
||||
"label": "C) Integrated via WebhookDispatcher",
|
||||
"description": "Route through existing WebhookDispatcher instead of bypassing it. Fixes all security and testing gaps while staying close to existing patterns. Human ~1.5 days / CC ~20 min. Completeness: 8/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D3 \u2014 Which implementation approach for the Stripe webhook handler? <gstack-qid:plan-ceo-approach-selection>": "A) Minimal Secure Fix (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:28:12.095Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_015J3yUJCKkX2QCgyjj83xP5",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D4 \u2014 Review mode for the Stripe webhook handler plan? <gstack-qid:plan-ceo-mode-selection>",
|
||||
"header": "Review mode",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "HOLD SCOPE \u2014 make it bulletproof (recommended)",
|
||||
"description": "The plan has a critical SQL injection bug and missing tests. Hold scope and review with maximum rigor: security, error paths, edge cases, observability. No expansions surfaced.",
|
||||
"preview": "Mode: HOLD SCOPE\nPlan has SQL injection. Make it secure,\ntested, and correct. No expansions."
|
||||
},
|
||||
{
|
||||
"label": "SELECTIVE EXPANSION \u2014 fix + cherry-pick improvements",
|
||||
"description": "Hold the minimal-secure baseline but also surface expansion opportunities (async email, richer observability) as individual opt-in decisions.",
|
||||
"preview": "Mode: SELECTIVE EXPANSION\nFix critical bugs, then surface optional\nimprovements one at a time."
|
||||
},
|
||||
{
|
||||
"label": "SCOPE EXPANSION \u2014 dream bigger",
|
||||
"description": "Push scope up. Envision a 10x better webhook processing system. Each expansion is an explicit opt-in decision.",
|
||||
"preview": "Mode: SCOPE EXPANSION\nRethink the whole webhook architecture.\nWhat would a 10x system look like?"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D4 \u2014 Review mode for the Stripe webhook handler plan? <gstack-qid:plan-ceo-mode-selection>": "HOLD SCOPE \u2014 make it bulletproof (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:28:22.136Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_01KG7LpP7rxLGnHChP5Yz5if",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D5 \u2014 Section 2: Email fires inline with no exception handling, creating a data-integrity risk <gstack-qid:plan-ceo-email-error-handling>",
|
||||
"header": "Email errors",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Catch email exceptions, log, continue (recommended)",
|
||||
"description": "Wrap the email send in try/catch. On failure: log userId, event ID, and the exception with full context. Return HTTP 200. The webhook succeeds; email is best-effort. No duplicate DB writes. Verify: test that a simulated email failure does not trigger HTTP 500 or a second DB update. Human ~30min / CC ~5min."
|
||||
},
|
||||
{
|
||||
"label": "B) Remove inline email; add TODOS.md item for async email",
|
||||
"description": "Ship the webhook handler now without the notification email. Add a TODOS.md item to wire up async email separately. Eliminates the error entirely; no email at launch. Human ~15min / CC ~5min."
|
||||
},
|
||||
{
|
||||
"label": "C) Async email via job queue",
|
||||
"description": "Enqueue the email as a background job after DB commit. Email failures don't affect webhook processing or dedup recording. Correct architecture, but expands scope beyond Approach A. Human ~1 day / CC ~20min."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D5 \u2014 Section 2: Email fires inline with no exception handling, creating a data-integrity risk <gstack-qid:plan-ceo-email-error-handling>": "A) Catch email exceptions, log, continue (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:32:18.928Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_01GytSPkm6VPeam3qPaQqAL4",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D6 \u2014 Section 3: SQL injection \u2014 userId from Stripe metadata is interpolated directly into raw SQL <gstack-qid:plan-ceo-sql-injection>",
|
||||
"header": "SQL injection",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Parameterized query (recommended)",
|
||||
"description": "Replace raw SQL interpolation with a bind parameter: `WHERE id = ?` (or `:userId`). The DB driver handles escaping. One-line fix. Verify: test with userId = \"1 OR 1=1\" and confirm it returns no results (no match). Completeness: 9/10.",
|
||||
"preview": "-- Before (UNSAFE)\nSELECT * FROM users WHERE id = '#{userId}'\n\n-- After (safe)\nSELECT * FROM users WHERE id = ?\n-- bind: [userId]"
|
||||
},
|
||||
{
|
||||
"label": "B) ORM-safe lookup method",
|
||||
"description": "Replace raw SQL with the ORM's safe finder (e.g. User.find_by(id: userId) in Rails, or equivalent). The ORM handles parameterization automatically. Same safety as A; abstraction is cleaner. Completeness: 9/10.",
|
||||
"preview": "# Before (UNSAFE)\ndb.execute(\"SELECT * FROM users WHERE id = '#{userId}'\")\n\n# After (safe ORM)\nUser.find_by(id: userId)"
|
||||
},
|
||||
{
|
||||
"label": "C) Input validation + parameterized query",
|
||||
"description": "Validate userId is a safe format (numeric, UUID) before executing ANY SQL. Reject and log if invalid. Plus use parameterized query. Defense in depth: two independent layers. Slightly more code but maximum safety. Completeness: 10/10.",
|
||||
"preview": "# Validate first\nraise InvalidParam unless userId.match?(/\\A\\d+\\z/)\n# Then parameterized query\nUser.find_by(id: userId)"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D6 \u2014 Section 3: SQL injection \u2014 userId from Stripe metadata is interpolated directly into raw SQL <gstack-qid:plan-ceo-sql-injection>": "A) Parameterized query (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:32:43.068Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_01G6ZjfH55xcHTTBf5MWwAgS",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D7 \u2014 Section 6: Zero tests planned for new payment processing code <gstack-qid:plan-ceo-missing-tests>",
|
||||
"header": "Test coverage",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Minimum viable integration test suite (recommended)",
|
||||
"description": "Add 6-8 integration tests targeting the webhook endpoint directly: happy path, unknown user, email failure (no duplicate), duplicate event delivery (idempotency), SQL injection rejection, DB failure (returns 500). Verify tests pass before ship. Human ~2h / CC ~15min. Completeness: 7/10."
|
||||
},
|
||||
{
|
||||
"label": "B) Unit tests for the handler class only",
|
||||
"description": "Test StripePaymentWebhookHandler in isolation with mocked DB and email. Faster to write but won't catch integration issues between the handler and the existing guards (dedup, lock). Human ~1h / CC ~10min. Completeness: 6/10."
|
||||
},
|
||||
{
|
||||
"label": "C) Unit tests + integration tests (full coverage)",
|
||||
"description": "Both unit tests for the handler class and integration tests through the full endpoint. Covers the most ground; this is what we'd want at 2am on a Friday. Human ~4h / CC ~30min. Completeness: 9/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D7 \u2014 Section 6: Zero tests planned for new payment processing code <gstack-qid:plan-ceo-missing-tests>": "A) Minimum viable integration test suite (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:33:03.193Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_01NmaABLks71Q4V4cVvC6bZz",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D8 \u2014 Section 7: N+1 query \u2014 handler fetches each order in a loop <gstack-qid:plan-ceo-n1-query>",
|
||||
"header": "N+1 query",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "A) Fix now: replace loop with batch query (recommended)",
|
||||
"description": "Replace the per-order DB fetch loop with a single batch query (e.g. WHERE user_id = ? or using an ORM include/preload). One-line change. Adds the DB index check: confirm the user lookup column is indexed. Human ~30min / CC ~5min."
|
||||
},
|
||||
{
|
||||
"label": "B) Defer to TODOS.md",
|
||||
"description": "Log the N+1 as a known performance issue. At current payment volume it likely doesn't matter. Fix it when query time shows up in monitoring. Risk: if orders-per-user is large or payment volume is high, this degrades under load."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D8 \u2014 Section 7: N+1 query \u2014 handler fetches each order in a loop <gstack-qid:plan-ceo-n1-query>": "A) Fix now: replace loop with batch query (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:33:23.321Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_0198YPJhdTFWmJqGPqVNxsrD",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D9 \u2014 What's the next step after this CEO review? <gstack-qid:plan-ceo-next-step>",
|
||||
"header": "Next step",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Run /plan-eng-review next (recommended)",
|
||||
"description": "Eng Review is the required shipping gate. It covers architecture, code quality, test adequacy, and performance. This CEO review resolved the security and correctness bugs; eng review validates the implementation approach. Run it before writing any code."
|
||||
},
|
||||
{
|
||||
"label": "Skip \u2014 handle reviews manually",
|
||||
"description": "Take the implementation tasks (T1-T6) from this review and implement directly. Run /plan-eng-review separately when ready."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D9 \u2014 What's the next step after this CEO review? <gstack-qid:plan-ceo-next-step>": "Run /plan-eng-review next (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-09T00:37:26.587Z"
|
||||
}
|
||||
],
|
||||
"reportContent": "# CEO Plan Review: Payment Processing Integration\n\n**Date:** 2026-09-09 \n**Mode:** HOLD SCOPE \n**Approach:** A \u2014 Minimal Secure Fix \n**Branch:** main \n**Reviewer:** gstack /plan-ceo-review\n\n---\n\n## Context\n\nThe plan adds a `StripePaymentWebhookHandler` to process `payment_intent.succeeded` events\nfrom Stripe. On success it looks up the user, updates the user record, and fires a notification\nemail. Several existing guards (signature verification, event dedup, per-user transaction lock)\nare retained unchanged.\n\n**Why this review:** The plan has a critical SQL injection vulnerability, no email error\nhandling, and zero tests. All three must be resolved before this ships.\n\n---\n\n## Step 0: Premise Challenge & Approach\n\n### 0A. Premise\nWebhook-driven payment confirmation is the right shape for this problem. Stripe's event model\nis the industry standard; building against it is correct. The \"do nothing\" cost is real: users\ndon't get payment confirmations and their records don't update.\n\n### 0B. Existing Code Leverage\nThe plan correctly retains existing guards:\n- Ingress middleware: Stripe signature verification (retained)\n- Event-type filter: only `payment_intent.succeeded` routed here (retained)\n- Payload adapter: exposes `event.data.object.metadata.user_id` as `request.params.userId`\n- Dedup guard: by Stripe event ID (retained)\n- Per-user transaction lock (retained)\n- Lookup-result guard: unknown/deleted users \u2192 HTTP 200 + log (retained)\n- Ingress logging: event IDs, outcomes, durations, alerts (retained)\n- Feature flag + rollback path (retained)\n\n**WebhookDispatcher bypass:** The plan introduces `StripePaymentWebhookHandler` outside the\nexisting `WebhookDispatcher` module. Rationale given: \"clean namespace separation.\" Approach A\nretains this decision; it is a WARNING but not a correctness bug.\n\n### 0C. Dream State (12-month arc)\n```\nCURRENT STATE THIS PLAN (as written) 12-MONTH IDEAL\nNo webhook handler ---> Handler with SQL injection --> Secure handler,\n no email error handling async email,\n no tests, N+1 query full test coverage,\n batch queries\n```\n\n### Implementation Alternatives Considered\n| Approach | Effort | Coverage |\n|----------|--------|----------|\n| A: Minimal Secure Fix (SELECTED) | S | 7/10 |\n| B: Ideal Architecture (async email, batch queries) | L | 10/10 |\n| C: Integrate via WebhookDispatcher | M | 8/10 |\n\n### 0D. Complexity Check (HOLD SCOPE)\nThe plan touches: `StripePaymentWebhookHandler` (new class), DB access layer, email\nnotification. Small footprint \u2014 no complexity smell.\n\n---\n\n## Architecture Diagram\n\n```\nStripe Webhook POST /webhook\n \u2502\n \u25bc\n[Ingress Middleware]\n \u251c\u2500 Verify Stripe signature (raw body) \u2500\u2500\u2500\u2500 invalid \u2192 400\n \u251c\u2500 Log event ID + timestamp\n \u2514\u2500 Filter: payment_intent.succeeded? \u2500\u2500 other types \u2192 200 ack\n \u2502\n \u25bc\n[Event Dedup Guard]\n \u251c\u2500 Already processed? \u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500 yes \u2192 200 ack\n \u2514\u2500 No \u2192 proceed\n \u2502\n \u25bc\n[Per-User Transaction Lock] (serialize per userId)\n \u2502\n \u25bc\n[StripePaymentWebhookHandler] \u25c4\u2500\u2500 NEW CLASS (bypasses WebhookDispatcher)\n \u2502\n \u251c\u2500 DB lookup: raw SQL with request.params.userId \u25c4\u2500\u2500 \u26a0\ufe0f SQL INJECTION\n \u2502 \u251c\u2500 User not found \u2192 200 ack + log\n \u2502 \u2514\u2500 User found \u2192\n \u2502 \u251c\u2500 Update user record (in transaction)\n \u2502 \u251c\u2500 Fire notification email \u25c4\u2500\u2500 \u26a0\ufe0f NO ERROR HANDLING\n \u2502 \u2502 \u2514\u2500 Email failure \u2192 exception propagates \u2192 HTTP 500\n \u2502 \u2514\u2500 (return HTTP 200 if email succeeds)\n \u2502\n \u2514\u2500 DB exception \u2192 propagates to ingress \u2192 HTTP 500 \u2192 Stripe retries\n \u2502\n \u25bc\n[Dedup Guard records completion] \u25c4\u2500\u2500 only after DB transaction commits\n (email fires BEFORE this records)\n```\n\n**Data flow shadow paths for userId:**\n- Happy path: valid userId \u2192 parameterized query \u2192 user found \u2192 update + email\n- Nil path: userId is null \u2192 raw SQL with NULL \u2192 behavior undefined / injection risk\n- Empty path: userId is `\"\"` \u2192 raw SQL with empty string \u2192 SQL syntax error or injection\n- Malicious path: userId is `\"1 OR 1=1\"` \u2192 raw SQL \u2192 table scan / data exfiltration\n\n---\n\n## Section 1: Architecture Review\n\n**Architecture diagram:** producLine truncated
|
||||
"reportUpdate": [
|
||||
{
|
||||
"timestamp": "2026-09-09T00:36:22.639Z",
|
||||
"id": "toolu_01URf23uR37vCphgbewjWMa6",
|
||||
"name": "Edit",
|
||||
"input": {
|
||||
"replace_all": false,
|
||||
"file_path": "/tmp/gstack-paid-shard-Q1hRY6/tmp/gstack-e2e-plan-ceo-dM1qj1/gstack-test-plan-ceo.md",
|
||||
"old_string": "## Outside Voice \u2014 Independent Plan Challenge\n\n",
|
||||
"new_string": "## Outside Voice \u2014 Independent Plan Challenge\n\nCodex review skipped (codex_reviews disabled). Re-enable: `gstack-config set codex_reviews enabled`.\n\n---\n\n## Decisions Made\n\n| # | Section | Finding | Decision |\n|---|---------|---------|---------|\n| D5 | Section 2 | Email inline with no exception handling \u2192 data integrity risk | **A: Catch exceptions, log with context, return HTTP 200** |\n| D6 | Section 3 | SQL injection via userId in raw SQL fragment | **A: Parameterized query (`WHERE id = ?`, bind userId)** |\n| D7 | Section 6 | Zero tests for new payment processing code | **A: 6-8 integration tests (happy path, unknown user, email failure, idempotency, SQL injection rejection, DB failure)** |\n| D8 | Section 7 | N+1 query \u2014 fetches each order in a loop | **A: Fix now \u2014 batch query, confirm lookup column indexed** |\n\n---\n\n## Implementation Tasks\n\nSynthesized from this review's findings. Each task derives from a specific finding above.\nRun with Claude Code or Codex; checkbox as you ship.\n\n- [ ] **T1 (P1, human: ~30min / CC: ~5min)** \u2014 StripePaymentWebhookHandler \u2014 Replace raw SQL userId interpolation with parameterized query\n - Surfaced by: Section 3 \u2014 SQL injection: userId from Stripe metadata interpolated into raw SQL\n - Files: `app/handlers/stripe_payment_webhook_handler.rb`\n - Verify: test with `userId = \"1 OR 1=1\"` \u2014 must return no match; test with valid userId \u2014 must find user\n\n- [ ] **T2 (P1, human: ~30min / CC: ~5min)** \u2014 StripePaymentWebhookHandler \u2014 Wrap email send in try/catch; log userId + event ID + exception; return HTTP 200\n - Surfaced by: Section 2 \u2014 email failure causes HTTP 500, triggering duplicate DB writes on Stripe retry\n - Files: `app/handlers/stripe_payment_webhook_handler.rb`\n - Verify: simulate email failure \u2014 webhook returns HTTP 200, DB update not duplicated on re-delivery\n\n- [ ] **T3 (P1, human: ~2h / CC: ~15min)** \u2014 StripePaymentWebhookHandler \u2014 Add 6-8 integration tests\n - Surfaced by: Section 6 \u2014 zero tests; existing suite cannot catch regressions in new handler\n - Files: `spec/handlers/stripe_payment_webhook_handler_spec.rb` (or equivalent)\n - Tests required:\n - `it \"updates user record on valid payment_intent.succeeded\"`\n - `it \"returns HTTP 200 for unknown user without updating anything\"`\n - `it \"handles email failure without HTTP 500 or duplicate DB write\"`\n - `it \"is idempotent (duplicate event delivery produces no duplicate update)\"`\n - `it \"rejects SQL injection attempt in userId\"`\n - `it \"returns HTTP 500 on DB failure so Stripe retries\"`\n\n- [ ] **T4 (P2, human: ~30min / CC: ~5min)** \u2014 StripePaymentWebhookHandler \u2014 Replace per-order DB fetch loop with single batch query\n - Surfaced by: Section 7 \u2014 N+1 query\n - Files: `app/handlers/stripe_payment_webhook_handler.rb`\n - Verify: confirm 2 DB queries total (user lookup + orders batch) regardless of order count; confirm user lookup column is indexed\n\n- [ ] **T5 (P2, human: ~5min / CC: ~1min)** \u2014 StripePaymentWebhookHandler \u2014 Add comment explaining WebhookDispatcher bypass rationale\n - Surfaced by: Section 10 \u2014 bypass creates pattern debt for future engineers\n - Files: `app/handlers/stripe_payment_webhook_handler.rb`\n - Verify: comment explains \"bypasses WebhookDispatcher for namespace separation; this handler is scoped to StripePaymentWebhook namespace specifically\"\n\n- [ ] **T6 (P2, human: ~2min / CC: ~1min)** \u2014 CLAUDE.md \u2014 Add gstack skill routing rules and commit (post-plan-mode)\n - Surfaced by: GSTACK_INSTRUCTION routing-injection (user opted in)\n - Files: `CLAUDE.md`\n - Verify: routing rules appended; `git commit -m \"chore: add gstack skill routing rules to CLAUDE.md\"`\n\n---\n\n## NOT in Scope\n\n| Item | Rationale |\n|------|-----------|\n| Async email via job queue | Scope expansion beyond Approach A; best-effort sync email with error handling is sufficient for now |\n| WebhookDispatcher integration | Approach A explicitly retains namespace separation; can refactor later if a second handler is added |\n| Input format validation on userId | Defense in depth; parameterized query prevents injection; format validation is an optional enhancement |\n| Full unit + integration test suite | Approach A targets minimum viable (6-8 integration tests); expand if time permits |\n\n---\n\n## What Already Exists\n\n| Existing component | What it does | Plan reuses it? |\n|-------------------|-------------|----------------|\n| Ingress middleware (Stripe signature verify) | Verifies HMAC signature against raw body; rejects 400 on failure | YES |\n| Event-type filter | Routes only `payment_intent.succeeded` to this handler | YES |\n| Payload adapter | Exposes `event.data.object.metadata.user_id` as `request.params.userId` | YES |\n| Dedup guard (by Stripe event ID) | Prevents duplicate Line truncated
|
||||
}
|
||||
},
|
||||
{
|
||||
"timestamp": "2026-09-09T00:36:24.575Z",
|
||||
"isError": false,
|
||||
"content": "The file /tmp/gstack-paid-shard-Q1hRY6/tmp/gstack-e2e-plan-ceo-dM1qj1/gstack-test-plan-ceo.md has been updated successfully. (file state is current in your context \u2014 no need to Read it back)"
|
||||
}
|
||||
],
|
||||
"planReadyRequests": [
|
||||
{
|
||||
"sessionId": "89297a6b-6a00-4308-8802-75c287f45046",
|
||||
"toolUseId": "toolu_011uEY7Zrm5grGaiSZvat6dh",
|
||||
"timestamp": "2026-09-09T00:37:39.599Z",
|
||||
"failed": false
|
||||
}
|
||||
],
|
||||
"rawCounts": {
|
||||
"setup": 4,
|
||||
"review": 5,
|
||||
"administrative": 0
|
||||
},
|
||||
"mechanicalBand": [
|
||||
4,
|
||||
7
|
||||
],
|
||||
"qualityLimitation": "Dispatcher-bypass remedy added without its own finding approval; this is separate from the mechanical4\u20137 count gate."
|
||||
}
|
||||
}
|
||||
+103
@@ -0,0 +1,103 @@
|
||||
{
|
||||
"sourceHead": "084f75297dfe33530baa0cce33b6ec60faf0cc99",
|
||||
"nativeSha256": "06816f001d183eb41053b561cf3cdca27b496ac15321a10ce25e4319c9520d76",
|
||||
"events": [
|
||||
{
|
||||
"isSidechain": false,
|
||||
"type": "assistant",
|
||||
"uuid": "7d81efef-536d-48eb-b6c5-a0590946e741",
|
||||
"timestamp": "2026-09-09T00:16:56.277Z",
|
||||
"sessionId": "f60050c4-2ad1-41f0-b547-c08306903ead",
|
||||
"message": {
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_use",
|
||||
"id": "toolu_016PugkAqmFq5DjPFmvDA3DB",
|
||||
"name": "Bash",
|
||||
"input": {
|
||||
"command": "_REPO_ROOT=$(git rev-parse --show-toplevel)\n_OUTSIDE_TMP=$(mktemp -d \"${TMPDIR:-/tmp}/gstack-outside.XXXXXXXX\")\n\nsource /home/vercel-sandbox/gstack/bin/gstack-codex-probe 2>/dev/null || true\n\n_gstack_codex_timeout_wrapper 540 codex exec \"$(cat /tmp/codex_adversarial_prompt.txt)\" -C \"$_REPO_ROOT\" -s read-only -c 'model_reasoning_effort=\"high\"' -c 'web_search=\"cached\"' < /dev/null > \"$_OUTSIDE_TMP/text\" 2> \"$_OUTSIDE_TMP/stderr\"\n_OUTSIDE_EXIT=$?\n\ncat \"$_OUTSIDE_TMP/text\"\ncat \"$_OUTSIDE_TMP/stderr\" >&2\n\nif [ \"$_OUTSIDE_EXIT\" -ne 0 ]; then\n echo \"Codex outside review unavailable: execution failed (exit $_OUTSIDE_EXIT); missing coverage.\"\nelse\n bun /home/vercel-sandbox/gstack/lib/outside-review-result.ts review \"$_OUTSIDE_TMP/text\" || true\n echo \"OUTSIDE_STATUS: completed provider=codex host=claude\"\nfi",
|
||||
"description": "Run Codex adversarial review",
|
||||
"timeout": 600000,
|
||||
"run_in_background": true
|
||||
},
|
||||
"caller": {
|
||||
"type": "direct"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"isSidechain": false,
|
||||
"type": "user",
|
||||
"uuid": "48883a6f-71f6-4fa2-b34c-b86db284d11f",
|
||||
"timestamp": "2026-09-09T00:16:56.296Z",
|
||||
"sessionId": "f60050c4-2ad1-41f0-b547-c08306903ead",
|
||||
"message": {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"tool_use_id": "toolu_016PugkAqmFq5DjPFmvDA3DB",
|
||||
"type": "tool_result",
|
||||
"content": "Command running in background with ID: bn8jkc52l. Output is being written to: /tmp/gstack-paid-shard-OhjUxg/tmp/claude-1000/-tmp-gstack-paid-shard-OhjUxg-tmp-gstack-outside-live-kVBfLL-claude/f60050c4-2ad1-41f0-b547-c08306903ead/tasks/bn8jkc52l.output. You will be notified when it completes. To check interim output, use Read on that file path.",
|
||||
"is_error": false
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"isSidechain": false,
|
||||
"attachment": {
|
||||
"type": "queued_command",
|
||||
"prompt": "<task-notification>\n<task-id>bn8jkc52l</task-id>\n<tool-use-id>toolu_016PugkAqmFq5DjPFmvDA3DB</tool-use-id>\n<output-file>/tmp/gstack-paid-shard-OhjUxg/tmp/claude-1000/-tmp-gstack-paid-shard-OhjUxg-tmp-gstack-outside-live-kVBfLL-claude/f60050c4-2ad1-41f0-b547-c08306903ead/tasks/bn8jkc52l.output</output-file>\n<status>completed</status>\n<summary>Background command \"Run Codex adversarial review\" completed (exit code 0)</summary>\n</task-notification>",
|
||||
"commandMode": "task-notification",
|
||||
"timestamp": "2026-09-09T00:17:04.299Z"
|
||||
},
|
||||
"type": "attachment",
|
||||
"uuid": "93081646-3c7e-4710-a652-5509b9611243",
|
||||
"timestamp": "2026-09-09T00:17:04.299Z",
|
||||
"sessionId": "f60050c4-2ad1-41f0-b547-c08306903ead"
|
||||
},
|
||||
{
|
||||
"isSidechain": false,
|
||||
"type": "assistant",
|
||||
"uuid": "b81e4244-ac19-4d34-8883-eed7e34b53c5",
|
||||
"timestamp": "2026-09-09T00:17:51.017Z",
|
||||
"sessionId": "f60050c4-2ad1-41f0-b547-c08306903ead",
|
||||
"message": {
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_use",
|
||||
"id": "toolu_01J2PNv3YyETiCWXKkHxFVxa",
|
||||
"name": "Read",
|
||||
"input": {
|
||||
"file_path": "/tmp/gstack-paid-shard-OhjUxg/tmp/claude-1000/-tmp-gstack-paid-shard-OhjUxg-tmp-gstack-outside-live-kVBfLL-claude/f60050c4-2ad1-41f0-b547-c08306903ead/tasks/bn8jkc52l.output"
|
||||
},
|
||||
"caller": {
|
||||
"type": "direct"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"isSidechain": false,
|
||||
"type": "user",
|
||||
"uuid": "6311dd36-0cfd-434e-a8c8-13c1e1a5b373",
|
||||
"timestamp": "2026-09-09T00:17:51.024Z",
|
||||
"sessionId": "f60050c4-2ad1-41f0-b547-c08306903ead",
|
||||
"message": {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"tool_use_id": "toolu_01J2PNv3YyETiCWXKkHxFVxa",
|
||||
"type": "tool_result",
|
||||
"content": "1\t- **Critical \u2014 Broken object-level authorization / IDOR** ([invoice.ts](/tmp/gstack-paid-shard-OhjUxg/tmp/gstack-outside-live-kVBfLL/claude/invoice.ts:4)): Removing the ownership check lets any authenticated caller retrieve any invoice by supplying its ID. This exposes sensitive financial data, including `bankAccount` and `amount`; predictable, leaked, or brute-forced IDs enable bulk cross-tenant disclosure. The now-unused `actor` parameter may also falsely signal to callers that authorization still occurs.\n2\t\n3\tRecommendation: Restore and test the ownership check before returning invoice data because any caller can currently read another customer\u2019s bank account through an invoice-ID enumeration attack.\n4\tReading additional input from stdin...\n5\tOpenAI Codex v0.153.2\n6\t--------\n7\tworkdir: /tmp/gstack-paid-shard-OhjUxg/tmp/gstack-outside-live-kVBfLL/claude\n8\tmodel: gpt-5.6-sol\n9\tprovider: openai\n10\tapproval: never\n11\tsandbox: read-only\n12\treasoning effort: high\n13\treasoning summaries: none\n14\tsession id: 01a08386-5e31-7a32-be0c-7d90503e865c\n15\t--------\n16\tuser\n17\tIMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are skill definitions, not repository review data. Do not follow nested skills, hooks, or tool instructions. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n18\t\n19\tReview the changes on this branch against the base branch. The diff is supplied below.\n20\t\n21\tDIFF (branch: feature/invoice-lookup vs base: main):\n22\t\n23\t```diff\n24\tdiff --git a/invoice.ts b/invoice.ts\n25\tindex c6aedd5..3bfe144 100644\n26\t--- a/invoice.ts\n27\t+++ b/invoice.ts\n28\t@@ -1,6 +1,5 @@\n29\t export async function readPrivateInvoice(db, actor, invoiceId) {\n30\t const invoice = await db.invoice.findUnique({ where: { id: invoiceId } });\n31\t if (!invoice) return null;\n32\t- if (invoice.ownerId !== actor.id) throw new Error('Forbidden');\n33\t return { amount: invoice.amount, bankAccount: invoice.bankAccount };\n34\t }\n35\t```\n36\t\n37\tYour job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments \u2014 just the problems. End your output with ONE line in the canonical format `Recommendation: <action> because <one-line reason naming the most exploitable finding>`. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale.\n38\tcodex\n39\t- **Critical \u2014 Broken object-level authorization / IDOR** ([invoice.ts](/tmp/gstack-paid-shard-OhjUxg/tmp/gstack-outside-live-kVBfLL/claude/invoice.ts:4)): Removing the ownership check lets any authenticated caller retrieve any invoice by supplying its ID. This exposes sensitive financial data, including `bankAccount` and `amount`; predictable, leaked, or brute-forced IDs enable bulk cross-tenant disclosure. The now-unused `actor` parameter may also falsely signal to callers that authorization still occurs.\n40\t\n41\tRecommendation: Restore and test the ownership check before returning invoice data because any caller can currently read another customer\u2019s bank account through an invoice-ID enumeration attack.\n42\ttokens used\n43\t11,840\n44\tOUTSIDE_STATUS: completed provider=codex host=claude\n45\t\n46\t[exited with code 0]\n47\t"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -1,9 +1,9 @@
|
||||
import { capturePlanCountQuestion, planCountPrerequisitePick, planCountQuestionInput } from './claude-pty-runner';
|
||||
import { capturePlanCountQuestion, planCountPrerequisitePick, planCountQuestionInput, type AskUserQuestionFingerprint } from './claude-pty-runner';
|
||||
|
||||
import type { NativePlanQuestionCall } from './plan-count-transcript';
|
||||
|
||||
/** A copied native panel is not actionable after prose or inside a code example. */
|
||||
function activePrerequisitePanel(visible: string): boolean {
|
||||
function activeSetupPanel(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));
|
||||
@@ -20,8 +20,53 @@ function activePrerequisitePanel(visible: string): boolean {
|
||||
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 {
|
||||
export type AutoplanSetupDecision =
|
||||
| { kind: 'input'; input: string; signatures: string[] }
|
||||
| { kind: 'waiting' | 'unrelated' }
|
||||
| { kind: 'unsupported_setup'; setup: 'routing' | 'prerequisite'; prompt: string;
|
||||
options: Array<{ index: number; label: string }>; identitySource: 'native-bound' | 'current-native-panel' };
|
||||
|
||||
/** Fail only a complete current setup panel; absence or stale/partial metadata is not failure. */
|
||||
function completeSetupOptions(visible: string, pending?: NativePlanQuestionCall): Array<{ index: number; label: string }> | null {
|
||||
if (!activeSetupPanel(visible)) return null;
|
||||
const lines = visible.replace(/\r+\n?/g, '\n').trimEnd().split('\n');
|
||||
const header = lines.findLastIndex(line => /^ {0,3}[☐□][^\n]+$/.test(line));
|
||||
const introduction = lines.slice(0, header).findLast(line => !/^[\t ─━-]*$/.test(line)) ?? '';
|
||||
if (/\b(?:example|sample|quot(?:e[sd]?|ed)|template|source)\b[^\n]*:\s*$/i.test(introduction)) return null;
|
||||
const panel = lines.slice(header);
|
||||
if (/[←→☒]|✔\s*Submit/.test(panel[0]!) ||
|
||||
panel.some(line => /(?:^|\s)[1-9]\.\s*\[[ ✓✔xX]\]/.test(line))) return null;
|
||||
if (panel.filter(line => /^ {0,3}❯\s*1\./.test(line)).length !== 1 ||
|
||||
panel.filter(line => /❯\s*[1-9]\./.test(line)).length !== 1) return null;
|
||||
const rows = panel.flatMap(line => {
|
||||
const match = /^ {0,3}(?:❯\s*)?([1-9])\.[\t ]*(\S.*?)\s*$/.exec(line);
|
||||
return match ? [{ index: Number(match[1]), label: match[2]! }] : [];
|
||||
});
|
||||
const compact = (text: string) => text.replace(/\s+/g, '').toLowerCase();
|
||||
if (rows.length < 4 || rows.some((row, index) => row.index !== index + 1) ||
|
||||
compact(rows.at(-2)!.label) !== 'typesomething.' || compact(rows.at(-1)!.label) !== 'chataboutthis') return null;
|
||||
const options = rows.slice(0, -2);
|
||||
if (pending) {
|
||||
const native = pending.questions[0];
|
||||
const cursor = panel.findIndex(line => /^ {0,3}❯\s*1\./.test(line));
|
||||
if (pending.answered || pending.failed || pending.questions.length !== 1 || native?.multiSelect ||
|
||||
compact(panel[0]!.replace(/^ {0,3}[☐□]/, '')) !== compact(native!.header) ||
|
||||
!compact(panel.slice(1, cursor).join(' ')).includes(compact(native!.question)) ||
|
||||
options.length !== native!.options.length || options.some((row, index) => compact(row.label) !== compact(native!.options[index]!.label))) return null;
|
||||
}
|
||||
return options;
|
||||
}
|
||||
|
||||
function unsupportedSetup(visible: string, question: AskUserQuestionFingerprint,
|
||||
setup: 'routing' | 'prerequisite', pending?: NativePlanQuestionCall): AutoplanSetupDecision {
|
||||
const options = completeSetupOptions(visible, pending);
|
||||
return options ? { kind: 'unsupported_setup', setup, prompt: question.promptSnippet, options,
|
||||
identitySource: pending ? 'native-bound' : 'current-native-panel' } : { kind: 'waiting' };
|
||||
}
|
||||
|
||||
/** Pure classification: only the caller that sends input commits returned identities. */
|
||||
export function autoplanSetupDecision(visible: string, seen: ReadonlySet<string>, pending?: NativePlanQuestionCall): AutoplanSetupDecision {
|
||||
if (pending && (pending.answered || pending.failed || pending.questions.length !== 1 || pending.questions[0]?.multiSelect)) return { kind: 'waiting' };
|
||||
// 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');
|
||||
@@ -29,14 +74,15 @@ export function autoplanRoutingSetupInput(visible: string, seen: Set<string>, pe
|
||||
// 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;
|
||||
};
|
||||
if (!question) return { kind: 'waiting' };
|
||||
const answered = (input: string | null): AutoplanSetupDecision => input === null
|
||||
? { kind: 'waiting' }
|
||||
: { kind: 'input', input, signatures: [...captured].filter(signature => !seen.has(signature)) };
|
||||
|
||||
const prerequisite = planCountPrerequisitePick(question);
|
||||
if (prerequisite !== null) {
|
||||
const prerequisitePrompt = /\/office-hours/i.test(question.promptSnippet) &&
|
||||
/(?:no\s*design\s*doc|produce\s*a\s*design\s*doc)/i.test(question.promptSnippet);
|
||||
if (prerequisitePrompt) {
|
||||
// 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
|
||||
@@ -44,9 +90,20 @@ export function autoplanRoutingSetupInput(visible: string, seen: Set<string>, pe
|
||||
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;
|
||||
(!question.nativeCall || pending.questions.length !== 1 || pending.questions[0]?.multiSelect))) return { kind: 'waiting' };
|
||||
if (prerequisite === null) {
|
||||
// Mentioning a missing design doc or /office-hours in a product/taste
|
||||
// question does not make it setup. Independently identify an offer to
|
||||
// run that prerequisite even when its opposed skip is unsupported.
|
||||
const run = choices.filter(({ label }) => /^Run\s*\/office-hours\s*(?:now|first)(?:\s*\(recommended\))?$/i.test(label));
|
||||
// UI fingerprints abbreviate long questions; inspect the current
|
||||
// question before its cursor, with full-panel validation below.
|
||||
const beforeOptions = display.slice(0, display.search(/❯\s*1\./));
|
||||
const offer = /\brun\s*\/office-hours\s*(?:now|first)\s*\?(?:\s*<gstack-qid:[^>]+>)?\s*$/i.test(beforeOptions);
|
||||
return run.length === 1 && offer ? unsupportedSetup(display, question, 'prerequisite', pending) : { kind: 'unrelated' };
|
||||
}
|
||||
const input = planCountQuestionInput(display, question, prerequisite);
|
||||
if (!/^[1-9]\d*$/.test(input ?? '') || !activePrerequisitePanel(display)) return null;
|
||||
if (!/^[1-9]\d*$/.test(input ?? '') || !activeSetupPanel(display)) return { kind: 'waiting' };
|
||||
return answered(input);
|
||||
}
|
||||
|
||||
@@ -65,6 +122,9 @@ export function autoplanRoutingSetupInput(visible: string, seen: Set<string>, pe
|
||||
// Match the declined setup action, not every English label separately:
|
||||
// No thanks/Skip may stand alone or opt into manual invocation. A manual
|
||||
// migration, deletion, or unrelated workflow is not the opposed action.
|
||||
// "Only" limits the same manual action; it does not add a second action.
|
||||
// Use one whole-label grammar with and without a courtesy/Skip prefix.
|
||||
const manualAction = /^(?:manual(?:invocation)?|(?:I['’]ll)?invoke(?:skills)?manually)(?:[-–—]?only)?$/i;
|
||||
const decline = options.filter(option => {
|
||||
const title = option.title.replace(/\(Recommended\)$/i, '');
|
||||
// The action can stand alone or follow a short courtesy ('No thanks').
|
||||
@@ -72,15 +132,14 @@ export function autoplanRoutingSetupInput(visible: string, seen: Set<string>, pe
|
||||
// manually' intact. Match the complete action, not the spelling of No;
|
||||
// arbitrary preceding instructions and extra trailing actions still fail.
|
||||
const manual = title.replace(/^[a-z]{0,3}thanks[,—–-]/i, '');
|
||||
if (/^(?:manualinvocation|(?:I['’]ll)?invoke(?:skills)?manually)$/i.test(manual)) return true;
|
||||
if (manualAction.test(manual)) return true;
|
||||
const prefix = /^(?:Nothanks|Skip)(?:[,—–-])?/i.exec(title);
|
||||
if (!prefix) return false;
|
||||
// 'No thanks' can be followed by the same explicit Skip action. Strip
|
||||
// that decline verb before checking any optional manual-invocation text.
|
||||
const action = title.slice(prefix[0].length).replace(/^skip(?:[,—–-])?/i, '');
|
||||
return action === '' || /^(?:manual(?:invocation)?|(?:I['’]ll)?invoke(?:skills)?manually)$/i.test(action);
|
||||
return action === '' || manualAction.test(action);
|
||||
});
|
||||
if (add.length !== 1 || decline.length !== 1 || add[0]!.index === decline[0]!.index) return null;
|
||||
const routingId = /<gstack-qid:routing-injection>/i.test(question.promptSnippet);
|
||||
const routingPremise = /gstack/i.test(prompt) && /CLAUDE\.md/i.test(prompt) && /skillroutingrules/i.test(prompt);
|
||||
// A qid can replace the longer premise, but cannot override a question
|
||||
@@ -88,6 +147,20 @@ export function autoplanRoutingSetupInput(visible: string, seen: Set<string>, pe
|
||||
// project setup rather than a product routing or taste decision.
|
||||
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;
|
||||
if (!claudeTarget || quotedPremise || (!routingId && !routingPremise)) return { kind: 'unrelated' };
|
||||
if (pending && (!question.nativeCall || pending.questions.length !== 1 || pending.questions[0]?.multiSelect)) return { kind: 'waiting' };
|
||||
// An intact Add-to-CLAUDE.md action identifies this setup offer even if
|
||||
// its opposed decline is unsupported. A qid or premise alone must not
|
||||
// turn substantive/ambiguous choices into an early setup failure.
|
||||
if (add.length !== 1) return { kind: 'waiting' };
|
||||
if (decline.length !== 1 || add[0]!.index === decline[0]!.index) return unsupportedSetup(display, question, 'routing', pending);
|
||||
return answered(planCountQuestionInput(display, question, add[0]!.index));
|
||||
}
|
||||
|
||||
/** Compatibility wrapper: preserve the existing input-only API. */
|
||||
export function autoplanRoutingSetupInput(visible: string, seen: Set<string>, pending?: NativePlanQuestionCall): string | null {
|
||||
const decision = autoplanSetupDecision(visible, seen, pending);
|
||||
if (decision.kind !== 'input') return null;
|
||||
for (const signature of decision.signatures) seen.add(signature);
|
||||
return decision.input;
|
||||
}
|
||||
@@ -1,5 +1,18 @@
|
||||
import type { AskUserQuestionFingerprint } from './claude-pty-runner';
|
||||
|
||||
/** A native choice may carry the CEO-specific recap beside a generic completion question. */
|
||||
function closedCeoRecap(description: string): boolean {
|
||||
const clause = /(?:^|[.!?]\s+)((?:The\s+)?CEO\s+review\b[^.!?]{0,240})(?=[.!?]|$)/i.exec(description)?.[1];
|
||||
if (!clause || /\b(?:if|unless|until|once|when|after|not|never)\b|n['’]t\b/i.test(clause)) return false;
|
||||
return /\b(?:all(?:\s+(?:gaps?|issues?|findings?))?(?:\s+(?:are|were))?\s+resolved|(?:no|0)\s+unresolved\s+(?:decisions|gaps|issues|findings))(?=\s*\)?\s*(?:;|$))/i.test(clause);
|
||||
}
|
||||
|
||||
/** Past-tense resolution can close a native next-review recap without the word "complete". */
|
||||
function resolvedCeoRecap(description: string): boolean {
|
||||
const clause = /(?:^|[.!?]\s+)((?:(?:This|The)\s+)?CEO\s+review\s+resolved\s+[^.!?;]{1,180}\b(?:bugs|gaps|issues|findings))(?=\s*(?:[.!?;]|$))/i.exec(description)?.[1];
|
||||
return Boolean(clause && !/\b(?:if|unless|until|once|when|after|not|never|some|most|partially|only|of|but|several|few)\b|n['’]t\b/i.test(clause));
|
||||
}
|
||||
|
||||
/** Only the CEO's finished-review menu, never a finding/TODO mentioning another skill. */
|
||||
function manualHandoffIndex(fp: AskUserQuestionFingerprint): number | null {
|
||||
const call = fp.nativeCall;
|
||||
@@ -8,12 +21,24 @@ function manualHandoffIndex(fp: AskUserQuestionFingerprint): number | null {
|
||||
if (!call || call.failed || fp.signature !== `${call.sessionId}:${call.toolUseId}` || call.questions.length !== 1) return null;
|
||||
const q = call.questions[0]!;
|
||||
if (q.multiSelect || q.options.length < 2) return null;
|
||||
const id = /<gstack-qid:\s*([a-z0-9-]+)\s*>/i.exec(q.question)?.[1]?.toLowerCase();
|
||||
const ids = [...q.question.matchAll(/<gstack-qid:\s*([a-z0-9-]+)\s*>/gi)];
|
||||
if (ids.length > 1 || (q.question.match(/<gstack-qid/gi)?.length ?? 0) !== ids.length) return null;
|
||||
const id = ids[0]?.[1]?.toLowerCase();
|
||||
if (id && !/^(?:plan-ceo-(?:review-)?next-(?:steps?|review)|ceo-next-step-eng-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+)(?: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+with\s+0\s+unresolved\s+decisions)?(?=\s*(?:[.!?—–]|$))/i.test(declaration);
|
||||
const gateContext = [q.question, ...q.options.map(option => option.description ?? '')].join('\n');
|
||||
const explicitCompletion = /(?:^|[.!?]\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+with\s+0\s+unresolved\s+decisions)?(?=\s*(?:[.!?—–]|$))/i.test(declaration);
|
||||
const genericCompletion = /(?:^|[.!?]\s+)(?:The\s+)?review\s+(?:is\s+)?(?:complete|cleared|clean|done)(?=\s*(?:[.!?—–]|$))/i.test(declaration);
|
||||
const questionText = declaration.replace(/<gstack-qid:[^>]+>/gi, '').trim();
|
||||
const recappedNavigation = Boolean(id) &&
|
||||
/^What(?:['’]s|\s+is)\s+the\s+next\s+(?:steps?|review)\s+after\s+(?:this|the)\s+CEO\s+review\?$/i.test(questionText) &&
|
||||
q.options.some(option => resolvedCeoRecap(option.description ?? ''));
|
||||
const unfinished = gateContext.replace(/\b(?:no|0)\s+unresolved\s+(?:decisions|gaps|issues|findings)\b/gi, '');
|
||||
const describedCompletion = (recappedNavigation || (genericCompletion && q.options.some(option => closedCeoRecap(option.description ?? '')))) &&
|
||||
!/\b(?:unresolved|outstanding|remains?|remaining|pending)\b/i.test(unfinished) &&
|
||||
!/(?:^|[.!?;]\s+|\b(?:please|must|need\s+to)\s+)(?:(?:please|first|then|also)\s+)*(?:add|fix|implement|resolve|decide)\b/im.test(gateContext);
|
||||
const completion = explicitCompletion || describedCompletion;
|
||||
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
|
||||
|
||||
@@ -3,26 +3,149 @@ export interface OutsideExecution {
|
||||
command: string;
|
||||
output: string;
|
||||
succeeded: boolean;
|
||||
background?: {
|
||||
toolUseId: string;
|
||||
taskId: string;
|
||||
outputFile: string;
|
||||
outputToolUseId?: string;
|
||||
completion: 'pending' | 'task_notification' | 'TaskOutput';
|
||||
};
|
||||
}
|
||||
|
||||
function toolText(content: unknown): string {
|
||||
return typeof content === 'string' ? content : Array.isArray(content)
|
||||
? content.filter(block => block?.type === 'text' && typeof block.text === 'string').map(block => block.text).join('\n') : '';
|
||||
}
|
||||
|
||||
function sessionIdentity(event: any): string | null {
|
||||
if (event?.isSidechain === true || event?.parent_tool_use_id != null) return null;
|
||||
const id = event?.session_id ?? event?.sessionId;
|
||||
if (typeof id !== 'string' || !id || (event.sessionId && event.sessionId !== id)) return null;
|
||||
return id;
|
||||
}
|
||||
|
||||
/** Only native task events carry completion; quoted/user-authored notifications do not. */
|
||||
function taskNotification(event: any) {
|
||||
if (event?.type === 'system' && event.subtype === 'task_notification') return {
|
||||
taskId: event.task_id, toolUseId: event.tool_use_id, outputFile: event.output_file,
|
||||
status: event.status, summary: event.summary,
|
||||
};
|
||||
if (event?.type !== 'attachment' || event.attachment?.type !== 'queued_command' ||
|
||||
event.attachment.commandMode !== 'task-notification') return null;
|
||||
const text = event.attachment.prompt;
|
||||
if (typeof text !== 'string') return null;
|
||||
const match = /^<task-notification>\s*<task-id>([^<>\r\n]+)<\/task-id>\s*<tool-use-id>([^<>\r\n]+)<\/tool-use-id>\s*<output-file>([^<>\r\n]+)<\/output-file>\s*<status>(completed|failed|stopped)<\/status>\s*<summary>([^<>]+)<\/summary>\s*<\/task-notification>$/.exec(text);
|
||||
return match ? { taskId: match[1], toolUseId: match[2], outputFile: match[3], status: match[4], summary: match[5] } : null;
|
||||
}
|
||||
|
||||
/** A complete native Read starts at line one and includes the task's exit footer. */
|
||||
function completedTaskRead(text: string): string | null {
|
||||
const lines = text.replace(/\r?\n$/, '').split(/\r?\n/);
|
||||
const numbered = lines.map(line => /^\s*(\d+)\t(.*)$/.exec(line));
|
||||
if (!numbered.length || numbered.some((line, index) => !line || Number(line[1]) !== index + 1)) return null;
|
||||
const output = numbered.map(line => line![2]).join('\n');
|
||||
return /(?:^|\n)\[exited with code 0\]\s*$/.test(output) ? output : null;
|
||||
}
|
||||
|
||||
export function claudeOutsideExecutions(transcript: unknown[]): OutsideExecution[] {
|
||||
const calls = new Map<string, string>();
|
||||
const calls = new Map<string, { command?: string; name: string; input: any; session: string | null }>();
|
||||
const tasks = new Map<string, {
|
||||
command: string; session: string; taskId: string; outputFile: string; failed: boolean;
|
||||
completion?: 'task_notification' | 'TaskOutput';
|
||||
outputs: Array<{ toolUseId: string; output: string; complete: boolean }>;
|
||||
}>();
|
||||
const results: OutsideExecution[] = [];
|
||||
for (const event of transcript as any[]) {
|
||||
const notice = taskNotification(event);
|
||||
const notified = notice && tasks.get(notice.toolUseId);
|
||||
if (notified && sessionIdentity(event) === notified.session && notice.taskId === notified.taskId && notice.outputFile === notified.outputFile) {
|
||||
if (notice.status === 'failed' || notice.status === 'stopped') notified.failed = true;
|
||||
else if (notice.status === 'completed') {
|
||||
const terminal = /^Background command [\s\S]+ completed \(exit code (-?\d+)\)$/.exec(notice.summary ?? '');
|
||||
if (terminal?.[1] === '0') notified.completion = 'task_notification';
|
||||
else if (terminal) notified.failed = true;
|
||||
}
|
||||
}
|
||||
const blocks = event?.message?.content;
|
||||
if (!Array.isArray(blocks)) continue;
|
||||
for (const block of blocks) {
|
||||
if (event.type === 'assistant' && block.type === 'tool_use' && block.name === 'Bash') {
|
||||
const command = block.input?.command;
|
||||
if (typeof block.id === 'string' && typeof command === 'string') calls.set(block.id, command);
|
||||
if (event.type === 'assistant' && block.type === 'tool_use' && typeof block.id === 'string' &&
|
||||
['Bash', 'Read', 'TaskOutput'].includes(block.name)) {
|
||||
calls.set(block.id, { command: block.input?.command, name: block.name, input: block.input, session: sessionIdentity(event) });
|
||||
}
|
||||
if (event.type === 'user' && block.type === 'tool_result' && calls.has(block.tool_use_id)) {
|
||||
const output = typeof block.content === 'string' ? block.content
|
||||
: (Array.isArray(block.content) ? block.content.filter((b: any) => b.type === 'text').map((b: any) => b.text).join('\n') : '');
|
||||
results.push({ command: calls.get(block.tool_use_id)!, output, succeeded: block.is_error !== true });
|
||||
const call = calls.get(block.tool_use_id)!;
|
||||
const output = toolText(block.content);
|
||||
if (call.name === 'Bash' && typeof call.command === 'string') {
|
||||
const acknowledgment = /^Command running in background with ID: ([a-zA-Z0-9_-]+)\. Output is being written to: ([^\r\n]+)\. You will be notified when it completes\. To check interim output, use Read on that file path\.$/.exec(output);
|
||||
if (!acknowledgment) {
|
||||
const existing = tasks.get(block.tool_use_id);
|
||||
if (existing) {
|
||||
if (block.is_error === true) existing.failed = true;
|
||||
results.push({ command: existing.command, output, succeeded: false,
|
||||
background: { toolUseId: block.tool_use_id, taskId: existing.taskId,
|
||||
outputFile: existing.outputFile, completion: 'pending' } });
|
||||
continue;
|
||||
}
|
||||
const pending = /^Command running in background\b/.test(output) || event.toolUseResult?.backgroundTaskId;
|
||||
results.push({ command: call.command, output, succeeded: block.is_error !== true && !pending });
|
||||
continue;
|
||||
}
|
||||
const [, taskId, outputFile] = acknowledgment;
|
||||
const background = { toolUseId: block.tool_use_id, taskId: taskId!, outputFile: outputFile!, completion: 'pending' as const };
|
||||
results.push({ command: call.command, output, succeeded: false, background });
|
||||
const existing = tasks.get(block.tool_use_id);
|
||||
if (block.is_error === true || !call.session || sessionIdentity(event) !== call.session ||
|
||||
!outputFile!.endsWith(`/tasks/${taskId}.output`) || !outputFile!.startsWith('/')) {
|
||||
if (existing) existing.failed = true;
|
||||
continue;
|
||||
}
|
||||
if (existing) {
|
||||
// Replayed acknowledgments cannot reset completed output or a
|
||||
// terminal failure. One original call cannot launch a second task.
|
||||
if (existing.command !== call.command || existing.session !== call.session ||
|
||||
existing.taskId !== taskId || existing.outputFile !== outputFile) existing.failed = true;
|
||||
continue;
|
||||
}
|
||||
tasks.set(block.tool_use_id, { command: call.command, session: call.session, taskId: taskId!, outputFile: outputFile!, failed: false, outputs: [] });
|
||||
continue;
|
||||
}
|
||||
if (!call.session || sessionIdentity(event) !== call.session) continue;
|
||||
for (const task of tasks.values()) {
|
||||
if (task.session !== call.session) continue;
|
||||
if (call.name === 'Read' && call.input?.file_path === task.outputFile) {
|
||||
const complete = block.is_error !== true && (call.input.offset === undefined || call.input.offset === 1)
|
||||
? completedTaskRead(output) : null;
|
||||
task.outputs.push({ toolUseId: block.tool_use_id, output: complete ?? output, complete: complete !== null });
|
||||
}
|
||||
if (block.is_error === true) {
|
||||
if (call.name === 'TaskOutput' && call.input?.task_id === task.taskId) task.failed = true;
|
||||
continue;
|
||||
}
|
||||
if (call.name === 'TaskOutput' && call.input?.task_id === task.taskId) {
|
||||
// CLI's native TaskOutput envelope puts terminal status before the
|
||||
// output. Findings containing an exit-code string cannot supply it.
|
||||
const terminal = /^<retrieval_status>success<\/retrieval_status>\s*<task_id>([^<>]+)<\/task_id>\s*<task_type>local_bash<\/task_type>\s*<status>(completed|failed|stopped)<\/status>\s*<exit_code>(-?\d+)<\/exit_code>\s*<output>\n([\s\S]*)\n<\/output>\s*$/.exec(output);
|
||||
if (!terminal || terminal[1] !== task.taskId) continue;
|
||||
const complete = terminal[2] === 'completed' && terminal[3] === '0';
|
||||
if (!complete) task.failed = true;
|
||||
else {
|
||||
task.completion = 'TaskOutput';
|
||||
}
|
||||
task.outputs.push({ toolUseId: block.tool_use_id, output: terminal[4]!, complete });
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
for (const [toolUseId, task] of tasks) {
|
||||
for (const output of task.outputs) {
|
||||
results.push({ command: task.command, output: output.output,
|
||||
succeeded: output.complete && Boolean(task.completion) && !task.failed,
|
||||
background: { toolUseId, taskId: task.taskId, outputFile: task.outputFile,
|
||||
outputToolUseId: output.toolUseId, completion: task.completion ?? 'pending' } });
|
||||
}
|
||||
}
|
||||
return results;
|
||||
}
|
||||
|
||||
@@ -46,8 +169,9 @@ function outsideInvocation(provider: 'codex' | 'claude-code'): RegExp {
|
||||
/** 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 }) => ({
|
||||
.map(({ command, output, succeeded, background }) => ({
|
||||
type: 'outside_execution' as const, provider, command, output, succeeded,
|
||||
...(background ? { background } : {}),
|
||||
}));
|
||||
}
|
||||
|
||||
@@ -56,5 +180,6 @@ export function foundInvoiceAuthorizationDefect(executions: OutsideExecution[],
|
||||
&& outsideInvocation(provider).test(command)
|
||||
&& /invoice/i.test(output)
|
||||
&& /owner|ownership|unauthori[sz]ed|another user|cross[- ](?:tenant|user)|access control|authorization/i.test(output)
|
||||
&& !/(?:^|\n)\s*(?:I|We)\s+(?:cannot|can['’]t|won['’]t|am unable to|are unable to)\s+(?:review|perform|complete|provide|conduct|help with)\b/i.test(output)
|
||||
&& !/OUTSIDE_STATUS:\s*(?:unavailable|disabled|skipped)/.test(output));
|
||||
}
|
||||
@@ -1,4 +1,6 @@
|
||||
/** Extract the installed review workflow, handling carved and inline host renders. */
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { extractSkillSections } from './skill-fixture';
|
||||
@@ -24,3 +26,33 @@ export function installOutsideReviewFixture(rendered: string, host: 'claude' | '
|
||||
writeFileSync(join(destination, 'SKILL.md'), content);
|
||||
return destination;
|
||||
}
|
||||
|
||||
// Each paid invocation must begin at the same committed authorization defect.
|
||||
function git(cwd: string, ...args: string[]) {
|
||||
const result = Bun.spawnSync(['git', ...args], { cwd, stdout: 'pipe', stderr: 'pipe', timeout: 10_000 });
|
||||
if (result.exitCode !== 0) throw new Error(`git ${args[0]}: ${result.stderr.toString()}`);
|
||||
}
|
||||
|
||||
export function createOutsideReviewRepo(fixtureRoot: string, host: 'claude' | 'codex'): string {
|
||||
// Bun retries reuse beforeAll state; each attempt needs a new git repository.
|
||||
const dir = fs.mkdtempSync(path.join(fixtureRoot, `${host}-`));
|
||||
git(dir, 'init', '-b', 'main');
|
||||
git(dir, 'config', 'user.email', 'eval@example.com');
|
||||
git(dir, 'config', 'user.name', 'Outside Voice Eval');
|
||||
const safe = `export async function readPrivateInvoice(db, actor, invoiceId) {
|
||||
const invoice = await db.invoice.findUnique({ where: { id: invoiceId } });
|
||||
if (!invoice) return null;
|
||||
if (invoice.ownerId !== actor.id) throw new Error('Forbidden');
|
||||
return { amount: invoice.amount, bankAccount: invoice.bankAccount };
|
||||
}
|
||||
`;
|
||||
fs.writeFileSync(path.join(dir, 'invoice.ts'), safe);
|
||||
git(dir, 'add', 'invoice.ts');
|
||||
git(dir, 'commit', '-m', 'Protect private invoices by owner');
|
||||
git(dir, 'update-ref', 'refs/remotes/origin/main', 'HEAD');
|
||||
git(dir, 'checkout', '-b', 'feature/invoice-lookup');
|
||||
fs.writeFileSync(path.join(dir, 'invoice.ts'), safe.replace(" if (invoice.ownerId !== actor.id) throw new Error('Forbidden');\n", ''));
|
||||
git(dir, 'add', 'invoice.ts');
|
||||
git(dir, 'commit', '-m', 'Simplify invoice lookup');
|
||||
return dir;
|
||||
}
|
||||
@@ -160,7 +160,7 @@ 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/**', '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', 'test/ceo-completion-handoff-l.test.ts', 'test/fixtures/ceo-completion-handoff-l-calls.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt'],
|
||||
'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', 'test/ceo-completion-handoff-l.test.ts', 'test/fixtures/ceo-completion-handoff-l-calls.json', 'test/ceo-completion-handoff-m.test.ts', 'test/fixtures/ceo-completion-handoff-m-call.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt'],
|
||||
'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', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt'],
|
||||
'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', 'test/fixtures/design-review-l-calls.json', 'test/design-completion-handoff.test.ts', 'test/fixtures/design-handoff-l-calls.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt'],
|
||||
'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', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt'],
|
||||
@@ -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/outside-voice-evidence.test.ts', 'test/skill-e2e-outside-voice.test.ts',
|
||||
'test/helpers/skill-fixture.ts', 'test/helpers/outside-voice-fixture.ts', 'test/outside-voice-fixture.test.ts', 'test/helpers/outside-voice-evidence.ts', 'test/outside-voice-evidence.test.ts', 'test/outside-voice-async.test.ts', 'test/fixtures/outside-async-task-m-events.json', '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/outside-voice-evidence.test.ts', 'test/skill-e2e-outside-voice.test.ts',
|
||||
'test/helpers/skill-fixture.ts', 'test/helpers/outside-voice-fixture.ts', 'test/outside-voice-fixture.test.ts', 'test/helpers/outside-voice-evidence.ts', 'test/outside-voice-evidence.test.ts', 'test/outside-voice-async.test.ts', 'test/fixtures/outside-async-task-m-events.json', 'test/skill-e2e-outside-voice.test.ts',
|
||||
],
|
||||
|
||||
// Disabled means no extra plan review, including a native Agent fallback.
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { claudeOutsideExecutions, foundInvoiceAuthorizationDefect, outsideExecutionTranscript } from './helpers/outside-voice-evidence';
|
||||
import { parseNDJSON } from './helpers/session-runner';
|
||||
import captured from './fixtures/outside-async-task-m-events.json';
|
||||
|
||||
const events = () => structuredClone(captured.events) as any[];
|
||||
const finding = 'The removed invoice owner check allows unauthorized access. Restore the ownership check.';
|
||||
|
||||
function sdkEvents() {
|
||||
// The CLI emits the same native task completion as this documented SDK
|
||||
// event. Disk-only attachment fields are not required by the stream parser.
|
||||
const input = events();
|
||||
const attachment = input[2];
|
||||
const prompt = attachment.attachment.prompt as string;
|
||||
const field = (name: string) => new RegExp(`<${name}>([^<]+)</${name}>`).exec(prompt)![1];
|
||||
input[2] = { type: 'system', subtype: 'task_notification', session_id: attachment.sessionId,
|
||||
task_id: field('task-id'), tool_use_id: field('tool-use-id'), output_file: field('output-file'),
|
||||
status: field('status'), summary: field('summary') };
|
||||
return input.map(event => {
|
||||
const { sessionId, isSidechain, ...rest } = event;
|
||||
return { ...rest, session_id: event.session_id ?? sessionId };
|
||||
});
|
||||
}
|
||||
|
||||
function detected(input: unknown[]) {
|
||||
return foundInvoiceAuthorizationDefect(claudeOutsideExecutions(input), 'codex');
|
||||
}
|
||||
|
||||
describe('completed background outside execution correlation', () => {
|
||||
test('the exact task completion and complete output Read establish the actual external finding', () => {
|
||||
const result = claudeOutsideExecutions(events());
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]!.succeeded).toBe(false);
|
||||
expect(result[0]!.background?.completion).toBe('pending');
|
||||
expect(result[1]!.succeeded).toBe(true);
|
||||
expect(result[1]!.background).toEqual({
|
||||
toolUseId: 'toolu_016PugkAqmFq5DjPFmvDA3DB', taskId: 'bn8jkc52l',
|
||||
outputFile: events()[3].message.content[0].input.file_path,
|
||||
outputToolUseId: 'toolu_01J2PNv3YyETiCWXKkHxFVxa', completion: 'task_notification',
|
||||
});
|
||||
expect(result[1]!.output).toContain('Removing the ownership check lets any authenticated caller retrieve any invoice');
|
||||
expect(detected(events())).toBe(true);
|
||||
expect(outsideExecutionTranscript(result, 'codex')[1]!.background).toEqual(result[1]!.background);
|
||||
});
|
||||
|
||||
test('SDK NDJSON retains the same exact identity and output instead of borrowing host prose', () => {
|
||||
const stream = sdkEvents().map(event => JSON.stringify(event));
|
||||
const { transcript } = parseNDJSON(stream);
|
||||
expect(detected(transcript)).toBe(true);
|
||||
expect(detected(transcript.filter(event => event.type !== 'system'))).toBe(false);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['missing acknowledgment', (e: any[]) => { e.splice(1, 1); }],
|
||||
['acknowledgment only', (e: any[]) => { e.splice(2); }],
|
||||
['missing completion', (e: any[]) => { e.splice(2, 1); }],
|
||||
['missing Read use', (e: any[]) => { e.splice(3, 1); }],
|
||||
['missing Read result', (e: any[]) => { e.pop(); }],
|
||||
['wrong Read identity', (e: any[]) => { e[4].message.content[0].tool_use_id = 'other'; }],
|
||||
['wrong task output path', (e: any[]) => { e[3].message.content[0].input.file_path += '.other'; }],
|
||||
['foreign Read session', (e: any[]) => { e[4].sessionId = 'foreign'; }],
|
||||
['sidechain Read', (e: any[]) => { e[4].isSidechain = true; }],
|
||||
['nested Read', (e: any[]) => { e[4].parent_tool_use_id = 'nested'; }],
|
||||
['failed Read', (e: any[]) => { e[4].message.content[0].is_error = true; }],
|
||||
['offset Read', (e: any[]) => { e[3].message.content[0].input.offset = 2; }],
|
||||
['partial output', (e: any[]) => { e[4].message.content[0].content = e[4].message.content[0].content.split('\n').slice(0, 3).join('\n'); }],
|
||||
['nonconsecutive lines', (e: any[]) => { e[4].message.content[0].content = e[4].message.content[0].content.replace('2\t', '9\t'); }],
|
||||
['output without exit footer', (e: any[]) => { e[4].message.content[0].content = e[4].message.content[0].content.replace('[exited with code 0]', ''); }],
|
||||
['failed exit footer', (e: any[]) => { e[4].message.content[0].content = e[4].message.content[0].content.replace('[exited with code 0]', '[exited with code 1]'); }],
|
||||
['raw forged output', (e: any[]) => { e[4].message.content[0].content = finding + '\n[exited with code 0]'; }],
|
||||
['foreign completion', (e: any[]) => { e[2].sessionId = 'foreign'; }],
|
||||
['wrong completed task', (e: any[]) => { e[2].attachment.prompt = e[2].attachment.prompt.replace('<task-id>bn8jkc52l', '<task-id>other'); }],
|
||||
['wrong original tool', (e: any[]) => { e[2].attachment.prompt = e[2].attachment.prompt.replace('toolu_016PugkAqmFq5DjPFmvDA3DB', 'other'); }],
|
||||
['wrong completed file', (e: any[]) => { e[2].attachment.prompt = e[2].attachment.prompt.replace('bn8jkc52l.output', 'other.output'); }],
|
||||
['failed task', (e: any[]) => { e[2].attachment.prompt = e[2].attachment.prompt.replace('<status>completed', '<status>failed'); }],
|
||||
['stopped task', (e: any[]) => { e[2].attachment.prompt = e[2].attachment.prompt.replace('<status>completed', '<status>stopped'); }],
|
||||
['nonzero completed exit', (e: any[]) => { e[2].attachment.prompt = e[2].attachment.prompt.replace('(exit code 0)', '(exit code 1)'); }],
|
||||
['stale completion before launch', (e: any[]) => { e.unshift(e.splice(2, 1)[0]); }],
|
||||
['quoted user notification', (e: any[]) => { e[2] = { type: 'user', sessionId: e[2].sessionId, message: { content: [{ type: 'text', text: e[2].attachment.prompt }] } }; }],
|
||||
['assistant notification claim', (e: any[]) => { e[2] = { type: 'assistant', sessionId: e[2].sessionId, message: { content: [{ type: 'text', text: e[2].attachment.prompt }] } }; }],
|
||||
['refusal containing the fixture words', (e: any[]) => { e[4].message.content[0].content = '1\tI cannot review the invoice owner authorization issue.\n2\t[exited with code 0]'; }],
|
||||
['unavailable provider', (e: any[]) => { e[4].message.content[0].content = `1\t${finding}\n2\tOUTSIDE_STATUS: unavailable\n3\t[exited with code 0]`; }],
|
||||
])('rejects %s despite a matching finding elsewhere', (_name, mutate) => {
|
||||
const input = events();
|
||||
mutate(input);
|
||||
expect(detected(input)).toBe(false);
|
||||
});
|
||||
|
||||
test('a terminal failure overrides an earlier completion and an output containing a finding', () => {
|
||||
for (const terminal of [
|
||||
{ status: 'failed', summary: 'Background command failed (exit code 1)' },
|
||||
{ status: 'completed', summary: 'Background command "Run Codex adversarial review" completed (exit code 1)' },
|
||||
]) {
|
||||
const input = sdkEvents();
|
||||
input.push({ ...input[2], ...terminal });
|
||||
expect(detected(input)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test('replayed acknowledgments preserve task state and reject conflicting launch identities', () => {
|
||||
const original = sdkEvents();
|
||||
const failed = { ...original[2], status: 'failed', summary: 'Background command failed (exit code 1)' };
|
||||
expect(detected([
|
||||
...original.slice(0, 3), failed, original[1], ...original.slice(2),
|
||||
])).toBe(false);
|
||||
expect(detected([...original, original[1]])).toBe(true);
|
||||
|
||||
const plain = structuredClone(original[1]);
|
||||
plain.message.content[0].content = finding;
|
||||
expect(detected([...original.slice(0, 2), plain])).toBe(false);
|
||||
const retained = claudeOutsideExecutions([...original, plain]);
|
||||
expect(retained.filter(result => result.succeeded)).toHaveLength(1);
|
||||
expect(retained.find(result => result.output === finding)?.succeeded).toBe(false);
|
||||
plain.message.content[0].is_error = true;
|
||||
plain.message.content[0].content = 'The background launch failed.';
|
||||
expect(detected([...original, plain])).toBe(false);
|
||||
|
||||
for (const alter of [
|
||||
(ack: any) => { ack.message.content[0].content = ack.message.content[0].content.replaceAll('bn8jkc52l', 'other-task'); },
|
||||
(ack: any) => { ack.session_id = 'foreign'; },
|
||||
(ack: any) => { ack.message.content[0].is_error = true; },
|
||||
]) {
|
||||
const acknowledgment = structuredClone(original[1]);
|
||||
alter(acknowledgment);
|
||||
expect(detected([...original, acknowledgment])).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test('partial and failed exact-path Reads remain diagnostic output without earning completion', () => {
|
||||
for (const fail of [false, true]) {
|
||||
const input = events();
|
||||
input[4].message.content[0].content = finding;
|
||||
input[4].message.content[0].is_error = fail;
|
||||
const retained = outsideExecutionTranscript(claudeOutsideExecutions(input), 'codex');
|
||||
expect(retained.at(-1)!.output).toBe(finding);
|
||||
expect(retained.at(-1)!.succeeded).toBe(false);
|
||||
expect(detected(input)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test('an unsupported background acknowledgment cannot pass as a foreground result', () => {
|
||||
const input = events().slice(0, 2);
|
||||
input[1].message.content[0].content += '\n' + finding;
|
||||
expect(detected(input)).toBe(false);
|
||||
});
|
||||
|
||||
test('TaskOutput requires the exact launched task and native completed exit-zero envelope', () => {
|
||||
const input = sdkEvents().slice(0, 2);
|
||||
const session_id = input[0].session_id;
|
||||
const result = `<retrieval_status>success</retrieval_status>\n\n<task_id>bn8jkc52l</task_id>\n\n<task_type>local_bash</task_type>\n\n<status>completed</status>\n\n<exit_code>0</exit_code>\n\n<output>\n${finding}\n</output>`;
|
||||
input.push({ type: 'assistant', session_id, message: { content: [{ type: 'tool_use', name: 'TaskOutput', id: 'task-output', input: { task_id: 'bn8jkc52l' } }] } });
|
||||
input.push({ type: 'user', session_id, message: { content: [{ type: 'tool_result', tool_use_id: 'task-output', content: result }] } });
|
||||
expect(detected(input)).toBe(true);
|
||||
for (const content of [
|
||||
result.replace('>success<', '>not_ready<'), result.replace('>completed<', '>running<'),
|
||||
result.replace('>0<', '>1<'), result.replace('>bn8jkc52l<', '>other<'),
|
||||
result.replace('>local_bash<', '>local_agent<'), finding + result,
|
||||
]) {
|
||||
const invalid = structuredClone(input);
|
||||
invalid[3].message.content[0].content = content;
|
||||
expect(detected(invalid)).toBe(false);
|
||||
}
|
||||
const invalid = structuredClone(input);
|
||||
invalid[3].message.content[0].is_error = true;
|
||||
expect(detected(invalid)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,33 @@
|
||||
import { test, expect } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { createOutsideReviewRepo } from './helpers/outside-voice-fixture';
|
||||
|
||||
function git(cwd: string, ...args: string[]): string {
|
||||
const result = Bun.spawnSync(['git', ...args], { cwd, stdout: 'pipe', stderr: 'pipe', timeout: 10_000 });
|
||||
if (result.exitCode !== 0) throw new Error(result.stderr.toString());
|
||||
return result.stdout.toString().trim();
|
||||
}
|
||||
|
||||
for (const host of ['claude', 'codex'] as const) {
|
||||
test(`${host} retries use independent committed fixtures with the same one-line defect`, () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'outside-retry-'));
|
||||
try {
|
||||
const first = createOutsideReviewRepo(root, host);
|
||||
fs.writeFileSync(path.join(first, 'review-scratch.txt'), 'first attempt evidence');
|
||||
const second = createOutsideReviewRepo(root, host);
|
||||
expect(second).not.toBe(first);
|
||||
expect(fs.realpathSync(path.join(second, '.git'))).not.toBe(fs.realpathSync(path.join(first, '.git')));
|
||||
expect(fs.readFileSync(path.join(first, 'review-scratch.txt'), 'utf8')).toBe('first attempt evidence');
|
||||
expect(fs.existsSync(path.join(second, 'review-scratch.txt'))).toBe(false);
|
||||
for (const repo of [first, second]) {
|
||||
expect(git(repo, 'branch', '--show-current')).toBe('feature/invoice-lookup');
|
||||
expect(git(repo, 'diff', '--numstat', 'origin/main...HEAD')).toBe('0\t1\tinvoice.ts');
|
||||
expect(git(repo, 'show', 'origin/main:invoice.ts')).toContain('invoice.ownerId !== actor.id');
|
||||
expect(git(repo, 'show', 'HEAD:invoice.ts')).not.toContain('invoice.ownerId !== actor.id');
|
||||
}
|
||||
expect(git(second, 'status', '--porcelain')).toBe('');
|
||||
} finally { fs.rmSync(root, { recursive: true, force: true }); }
|
||||
});
|
||||
}
|
||||
@@ -30,7 +30,7 @@ import {
|
||||
isNumberedOptionListVisible,
|
||||
selectPtyNumberedOption,
|
||||
} from './helpers/claude-pty-runner';
|
||||
import { autoplanRoutingSetupInput } from './helpers/autoplan-setup-question';
|
||||
import { autoplanSetupDecision, type AutoplanSetupDecision } from './helpers/autoplan-setup-question';
|
||||
import { autoplanPhaseCompletions, type AutoplanPhaseHit } from './helpers/autoplan-phase-observer';
|
||||
import { readPlanCountTranscript, type PlanCountTranscript } from './helpers/plan-count-transcript';
|
||||
import { createPlanCountSnapshotWriter } from './helpers/plan-count-artifacts';
|
||||
@@ -81,7 +81,8 @@ describeE2E('/autoplan native chain ordering (periodic)', () => {
|
||||
|
||||
let hits: AutoplanPhaseHit[] = [];
|
||||
let transcript: PlanCountTranscript = { status: 'missing', calls: [], assistantMessages: [] };
|
||||
let outcome: 'chain_complete' | 'plan_ready' | 'timeout' | 'exited' = 'timeout';
|
||||
let outcome: 'chain_complete' | 'plan_ready' | 'timeout' | 'exited' | 'unsupported_setup' = 'timeout';
|
||||
let unsupportedSetup: Extract<AutoplanSetupDecision, { kind: 'unsupported_setup' }> | null = null;
|
||||
let evidence = '';
|
||||
let viewport = '';
|
||||
let fullSessionEvidence = '';
|
||||
@@ -99,7 +100,8 @@ describeE2E('/autoplan native chain ordering (periodic)', () => {
|
||||
artifacts = saveSnapshot({
|
||||
skillName: 'autoplan', cwd: tempDir, claudeConfigDir: session.hermeticConfigDir,
|
||||
raw: session.rawOutput(), visible: session.visibleText(), viewport,
|
||||
observation: { state, hits, native: transcript, exitCode: session.exitCode() },
|
||||
observation: { state, hits, native: transcript, exitCode: session.exitCode(), unsupportedSetup,
|
||||
retention: 'Current raw/visible/viewport and parsed native metadata only; full parent JSONL retention is not guaranteed.' },
|
||||
});
|
||||
};
|
||||
|
||||
@@ -147,13 +149,20 @@ describeE2E('/autoplan native chain ordering (periodic)', () => {
|
||||
// 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) {
|
||||
if (setupInput.includes('\r')) await selectPtyNumberedOption(session, Number(setupInput.trim()));
|
||||
else session.send(setupInput);
|
||||
const setup = autoplanSetupDecision(visible, seenSetupQuestions, transcript.calls.find(call => !call.answered && !call.failed));
|
||||
if (setup.kind === 'input') {
|
||||
if (setup.input.includes('\r')) await selectPtyNumberedOption(session, Number(setup.input.trim()));
|
||||
else session.send(setup.input);
|
||||
for (const signature of setup.signatures) seenSetupQuestions.add(signature);
|
||||
await Bun.sleep(2000);
|
||||
continue;
|
||||
}
|
||||
if (setup.kind === 'unsupported_setup') {
|
||||
outcome = 'unsupported_setup';
|
||||
unsupportedSetup = setup;
|
||||
evidence = viewport.slice(-3000);
|
||||
break;
|
||||
}
|
||||
|
||||
// Terminal: Phase 3 (Eng) seen — chain reached the required end.
|
||||
if (hits.some(h => h.phase === 3)) {
|
||||
@@ -182,10 +191,11 @@ describeE2E('/autoplan native chain ordering (periodic)', () => {
|
||||
} finally { await session.close(); }
|
||||
}
|
||||
|
||||
if (outcome === 'exited' || outcome === 'timeout') {
|
||||
if (outcome === 'exited' || outcome === 'timeout' || outcome === 'unsupported_setup') {
|
||||
throw new Error(
|
||||
`autoplan chain test FAILED: outcome=${outcome}, exitCode=${exitCode}, hits=${JSON.stringify(hits)}\n` +
|
||||
`Native transcript: ${transcript.status}; artifacts=${JSON.stringify(artifacts)}\n` +
|
||||
(unsupportedSetup ? `Unsupported setup: ${JSON.stringify(unsupportedSetup)}; no input sent. Artifacts contain UI and parsed metadata, not guaranteed full parent JSONL.\n` : '') +
|
||||
`--- post-command evidence (last 3KB) ---\n${diagnosticTail(evidence)}\n` +
|
||||
`--- full-session visible tail, including startup (last 3KB) ---\n${fullSessionEvidence}`,
|
||||
);
|
||||
|
||||
@@ -7,7 +7,7 @@ import { e2eTierEnabled } from './helpers/e2e-gate';
|
||||
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 { createOutsideReviewRepo, installOutsideReviewFixture } from './helpers/outside-voice-fixture';
|
||||
import { claudeOutsideExecutions, codexOutsideExecutions, foundInvoiceAuthorizationDefect, outsideExecutionTranscript } from './helpers/outside-voice-evidence';
|
||||
import { CAPTURE_LONG_MS } from './helpers/eval-budgets';
|
||||
|
||||
@@ -24,36 +24,6 @@ const describeLive = enabled ? describe : describe.skip;
|
||||
let fixtureRoot: string;
|
||||
let rendered: string;
|
||||
|
||||
function git(cwd: string, ...args: string[]) {
|
||||
const result = Bun.spawnSync(['git', ...args], { cwd, stdout: 'pipe', stderr: 'pipe', timeout: 10_000 });
|
||||
if (result.exitCode !== 0) throw new Error(`git ${args[0]}: ${result.stderr.toString()}`);
|
||||
}
|
||||
|
||||
function createRepo(host: 'claude' | 'codex'): string {
|
||||
const dir = path.join(fixtureRoot, host);
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
git(dir, 'init', '-b', 'main');
|
||||
git(dir, 'config', 'user.email', 'eval@example.com');
|
||||
git(dir, 'config', 'user.name', 'Outside Voice Eval');
|
||||
const safe = `export async function readPrivateInvoice(db, actor, invoiceId) {
|
||||
const invoice = await db.invoice.findUnique({ where: { id: invoiceId } });
|
||||
if (!invoice) return null;
|
||||
if (invoice.ownerId !== actor.id) throw new Error('Forbidden');
|
||||
return { amount: invoice.amount, bankAccount: invoice.bankAccount };
|
||||
}
|
||||
`;
|
||||
fs.writeFileSync(path.join(dir, 'invoice.ts'), safe);
|
||||
git(dir, 'add', 'invoice.ts');
|
||||
git(dir, 'commit', '-m', 'Protect private invoices by owner');
|
||||
git(dir, 'update-ref', 'refs/remotes/origin/main', 'HEAD');
|
||||
git(dir, 'checkout', '-b', 'feature/invoice-lookup');
|
||||
fs.writeFileSync(path.join(dir, 'invoice.ts'), safe.replace(" if (invoice.ownerId !== actor.id) throw new Error('Forbidden');\n", ''));
|
||||
git(dir, 'add', 'invoice.ts');
|
||||
git(dir, 'commit', '-m', 'Simplify invoice lookup');
|
||||
return dir;
|
||||
}
|
||||
|
||||
|
||||
function prompt(skill: string): string {
|
||||
return `Run the installed ${skill} skill on this branch. The base branch is main (origin/main exists locally). Complete its adversarial review workflow and report the outside provider's actual findings separately from the native review. Review only; leave source files unchanged. All reviewer calls in this skill are authorized. If a native subagent tool is unavailable, report that and continue the independent outside CLI pass. Do not claim success if the outside CLI cannot complete.`;
|
||||
}
|
||||
@@ -74,7 +44,7 @@ describeLive('Installed workflows dispatch outside their host harness', () => {
|
||||
});
|
||||
|
||||
testIfSelected('outside-voice-codex-to-claude-code', async () => {
|
||||
const repo = createRepo('codex');
|
||||
const repo = createOutsideReviewRepo(fixtureRoot, 'codex');
|
||||
const skillDir = installOutsideReviewFixture(rendered, 'codex', repo, ROOT);
|
||||
const result = await runCodexSkill({
|
||||
skillDir, skillName: 'gstack-review', cwd: repo, prompt: prompt('gstack-review'),
|
||||
@@ -95,7 +65,7 @@ describeLive('Installed workflows dispatch outside their host harness', () => {
|
||||
});
|
||||
|
||||
testIfSelected('outside-voice-claude-code-to-codex', async () => {
|
||||
const repo = createRepo('claude');
|
||||
const repo = createOutsideReviewRepo(fixtureRoot, 'claude');
|
||||
installOutsideReviewFixture(rendered, 'claude', repo, ROOT);
|
||||
const env: Record<string, string> = {};
|
||||
// Admit only Codex authentication/config location, never the operator's
|
||||
|
||||
@@ -82,6 +82,9 @@ const planCeo5Findings = (planPath: string) => [
|
||||
'The existing payload adapter exposes `event.data.object.metadata.user_id`',
|
||||
'as `request.params.userId`. This params object is the parsed body-data map,',
|
||||
'not URL query/path parameters; all users share one webhook URL.',
|
||||
'The adapter acknowledges missing, nil, or empty user_id metadata with',
|
||||
'HTTP 200 and an event-correlated warning before invoking this handler.',
|
||||
'For every nonempty external string it performs no SQL-format validation.',
|
||||
'The adapter forwards that external string unchanged. It does not cast,',
|
||||
'escape, or SQL-sanitize it; a valid signature does not make it safe for SQL.',
|
||||
'The existing webhook event guard deduplicates deliveries by Stripe event ID,',
|
||||
@@ -92,12 +95,21 @@ const planCeo5Findings = (planPath: string) => [
|
||||
'HTTP 200, logs the event, and stops before user updates or email fan-out.',
|
||||
'The ingress wrapper already logs event IDs, outcomes, and durations, with',
|
||||
'alerts for failed webhook processing. Those controls remain in place.',
|
||||
'The existing DB and mail clients attach the adapter user ID and event ID',
|
||||
'to outcome traces, including update success and email delivery success or',
|
||||
'failure. These shared clients rethrow exceptions unchanged; tracing does',
|
||||
'not rescue email errors or change the inline email call below.',
|
||||
'DB lookup/update exceptions propagate to that ingress wrapper, which logs',
|
||||
'the failure and returns HTTP 500 so Stripe retries the event. The existing',
|
||||
'event-ID dedup guard records completion only after the database transaction',
|
||||
'commits; failed or rolled-back database attempts remain retryable.',
|
||||
'The deployment already has a handler feature flag and a documented, tested',
|
||||
'rollback to the prior handler; this change uses that existing rollout path.',
|
||||
'That documented manual rollout checklist already requires a staging',
|
||||
'payment-event replay for this handler and verification of the user update,',
|
||||
'email delivery, and correlated outcome trace before enabling it broadly.',
|
||||
'This is manual deployment verification, not automated handler regression',
|
||||
'coverage; no new automated tests are planned in the Tests section below.',
|
||||
'',
|
||||
'## Architecture',
|
||||
"We're adding a new `StripePaymentWebhookHandler` class that will handle Stripe webhooks.",
|
||||
|
||||
Reference in new issue
Block a user