diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index b366fdd4d..402e2af05 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -748,7 +748,7 @@ bun "" scope "" ### Step 3: Locate review skills; load each at phase entry -At each phase, follow its full-load checkpoint and lazy-section triggers: +Resolve this phase's source to absolute ``; load via its checkpoint: - Phase 1: `~/.claude/skills/gstack/plan-ceo-review/SKILL.md` - Phase 2: `~/.claude/skills/gstack/plan-design-review/SKILL.md` (only if UI scope detected) - Phase 2.5: `~/.claude/skills/gstack/plan-devex-review/SKILL.md` (only if DX scope detected) diff --git a/autoplan/SKILL.md.tmpl b/autoplan/SKILL.md.tmpl index 861f8e24e..7323b3958 100644 --- a/autoplan/SKILL.md.tmpl +++ b/autoplan/SKILL.md.tmpl @@ -230,7 +230,7 @@ bun "" scope "" ### Step 3: Locate review skills; load each at phase entry -At each phase, follow its full-load checkpoint and lazy-section triggers: +Resolve this phase's source to absolute ``; load via its checkpoint: - Phase 1: {{AUTOPLAN_REVIEW_FILE:plan-ceo-review}} - Phase 2: {{AUTOPLAN_REVIEW_FILE:plan-design-review}} (only if UI scope detected) - Phase 2.5: {{AUTOPLAN_REVIEW_FILE:plan-devex-review}} (only if DX scope detected) diff --git a/autoplan/sections/ceo-phase.md b/autoplan/sections/ceo-phase.md index 35f2fd6fa..1f18dd59b 100644 --- a/autoplan/sections/ceo-phase.md +++ b/autoplan/sections/ceo-phase.md @@ -1,6 +1,6 @@ -Before dispatch, fully Read `~/.claude/skills/gstack/plan-ceo-review/SKILL.md` and `sections/review-sections.md` beside it; log successful start/end/total; fill gaps to EOF. Skip-listed: load only. +Before dispatch, fully Read `methodologyPath` from `bun "" methodology ceo "" ""`; log successful start/end/total; fill gaps to EOF. Skip-listed: load only. **Override rules:** - Mode selection: SELECTIVE EXPANSION diff --git a/autoplan/sections/design-phase.md b/autoplan/sections/design-phase.md index 0c1777a34..0e75e99f6 100644 --- a/autoplan/sections/design-phase.md +++ b/autoplan/sections/design-phase.md @@ -1,6 +1,6 @@ -Before dispatch, fully Read `~/.claude/skills/gstack/plan-design-review/SKILL.md` and `sections/review-sections.md` beside it; log successful start/end/total; fill gaps to EOF. Skip-listed: load only. +Before dispatch, fully Read `methodologyPath` from `bun "" methodology design "" ""`; log successful start/end/total; fill gaps to EOF. Skip-listed: load only. **Override rules:** - Focus areas: all relevant dimensions (P1) diff --git a/autoplan/sections/dx-phase.md b/autoplan/sections/dx-phase.md index 779c741ee..1c11c3986 100644 --- a/autoplan/sections/dx-phase.md +++ b/autoplan/sections/dx-phase.md @@ -1,6 +1,6 @@ -Before dispatch, fully Read `~/.claude/skills/gstack/plan-devex-review/SKILL.md` and `sections/review-sections.md` beside it; log successful start/end/total; fill gaps to EOF. Skip-listed: load only. +Before dispatch, fully Read `methodologyPath` from `bun "" methodology dx "" ""`; log successful start/end/total; fill gaps to EOF. Skip-listed: load only. **Override rules:** - Mode selection: DX POLISH diff --git a/autoplan/sections/eng-phase.md b/autoplan/sections/eng-phase.md index b585d82b2..1086265fc 100644 --- a/autoplan/sections/eng-phase.md +++ b/autoplan/sections/eng-phase.md @@ -1,6 +1,6 @@ -Before dispatch, fully Read `~/.claude/skills/gstack/plan-eng-review/SKILL.md` and `sections/review-sections.md` beside it; log successful start/end/total; fill gaps to EOF. Skip-listed: load only. +Before dispatch, fully Read `methodologyPath` from `bun "" methodology eng "" ""`; log successful start/end/total; fill gaps to EOF. Skip-listed: load only. **Override rules:** - Scope challenge: never reduce (P2) diff --git a/bin/gstack-autoplan-snapshot.ts b/bin/gstack-autoplan-snapshot.ts index b4d3d0d50..20eae3e5a 100644 --- a/bin/gstack-autoplan-snapshot.ts +++ b/bin/gstack-autoplan-snapshot.ts @@ -489,6 +489,72 @@ function phaseName(phase: string): string { return phase; } +/** One complete current-phase load target, not evidence that an agent read it. */ +export function prepareMethodology(phase: string, skillFile: string, restorePath: string) { + phaseName(phase); + const skill = `plan-${phase === 'dx' ? 'devex' : phase}-review`; + if (!isAbsolute(skillFile) || basename(skillFile) !== 'SKILL.md') throw new Error('Expected an absolute installed SKILL.md path'); + const restore = realpathSync(restorePath); + if (!statSync(restore).isFile()) throw new Error('Expected the existing restore-point file'); + const readPart = (file: string) => { + const resolved = realpathSync(file); + if (!statSync(resolved).isFile()) throw new Error('Methodology source must be a regular file'); + const bytes = readFileSync(resolved); + const text = bytes.toString('utf8'); + if (!Buffer.from(text).equals(bytes)) throw new Error('Methodology source must be valid UTF-8'); + return { path: file, resolvedPath: resolved, bytes, text }; + }; + const main = readPart(skillFile); + const frontmatter = /^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/.exec(main.text); + if (!frontmatter?.[1]!.split(/\r?\n/).includes(`name: ${skill}`)) { + throw new Error('Methodology skill identity does not match this phase'); + } + const mainProse = referenceProse(main.text).join('\n'); + const indexHeadings = [...mainProse.matchAll(/^## Section index[^\r\n]*$/gm)]; + const parts = [main]; + if (indexHeadings.length) { + if (indexHeadings.length !== 1 || /^## Review Sections\b/m.test(mainProse)) throw new Error('Ambiguous methodology layout'); + const index = mainProse.slice(indexHeadings[0]!.index! + indexHeadings[0]![0].length).split(/\n## /)[0]!; + const sections = [...index.matchAll(/`(sections\/[^`]+)`/g)].map(match => match[1]!); + if (sections.length !== 1 || sections[0] !== 'sections/review-sections.md') throw new Error('Expected the one complete current review section'); + const section = readPart(join(dirname(skillFile), sections[0])); + const sectionProse = referenceProse(section.text).join('\n'); + if ([...sectionProse.matchAll(/^## Review Sections\b/gm)].length !== 1 || + /^## Section index\b/m.test(sectionProse) || /sections\/[\w.-]+\.md/.test(sectionProse)) { + throw new Error('Missing or nested methodology review section'); + } + parts.push(section); + } else if ([...mainProse.matchAll(/^## Review Sections\b/gm)].length !== 1) { + throw new Error('Inline methodology is missing its complete review section'); + } + // Read and validate every source before creating anything. Preserve source + // bytes inside explicit ranges; separators never replace a source newline. + const chunks: Buffer[] = []; + let offset = 0; + const sources = parts.map(part => { + const header = Buffer.from(`\n`); + chunks.push(header, part.bytes, Buffer.from('\n\n')); + const startByte = offset + header.length; + offset = startByte + part.bytes.length + 2; + return { path: part.path, resolvedPath: part.resolvedPath, sha256: sha256(part.text), bytes: part.bytes.length, + startByte, endByte: startByte + part.bytes.length }; + }); + const content = Buffer.concat(chunks); + const directory = mkdtempSync(join(dirname(restore), `autoplan-${phase}-methodology-`)); + try { + const methodologyPath = join(directory, 'methodology.md'); + const manifest = { phase, methodologyPath, sha256: sha256(content.toString('utf8')), bytes: content.length, + lines: content.toString('utf8').split('\n').length, sources, + instruction: 'Read methodologyPath completely before phase snapshot creation or dispatch; log successful ranges through EOF. Apply the existing Autoplan skip list and overrides. This artifact supplies exact methodology, not proof of reading or execution.' }; + writeFileSync(methodologyPath, content, { flag: 'wx', mode: 0o444 }); + writeFileSync(join(directory, 'methodology.json'), JSON.stringify(manifest) + '\n', { flag: 'wx', mode: 0o444 }); + return manifest; + } catch (error) { + rmSync(directory, { recursive: true, force: true }); + throw error; + } +} + export function createSnapshot(phase: string, activePlan: string, restorePath: string) { phaseName(phase); const source = realpathSync(activePlan); @@ -597,6 +663,9 @@ if (import.meta.main) { if (command === 'init') { if (args.length !== 3 || args.some(arg => !arg)) throw new Error('Usage: init SOURCE_PLAN ACTIVE_PLAN RESTORE_PATH'); process.stdout.write(JSON.stringify(initializePlan(args[0]!, args[1]!, args[2]!)) + '\n'); + } else if (command === 'methodology') { + if (args.length !== 3 || args.some(arg => !arg)) throw new Error('Usage: methodology PHASE SKILL_FILE RESTORE_PATH'); + process.stdout.write(JSON.stringify(prepareMethodology(args[0]!, args[1]!, args[2]!)) + '\n'); } else if (command === 'scope') { const [activePlan, ...flags] = args; if (!activePlan || flags.some(flag => !['--developer-tool', '--agent-primary'].includes(flag)) || diff --git a/lib/claude-code.ts b/lib/claude-code.ts index a5ae3c04c..3faec769a 100644 --- a/lib/claude-code.ts +++ b/lib/claude-code.ts @@ -47,6 +47,16 @@ export function claudeCodeArgs(options: Pick" methodology ${phase} "" ""\`` + : entry; } const host = getHostConfig(ctx.host); diff --git a/test/autoplan-phase-order.test.ts b/test/autoplan-phase-order.test.ts index 5ca06001a..88be35420 100644 --- a/test/autoplan-phase-order.test.ts +++ b/test/autoplan-phase-order.test.ts @@ -75,7 +75,7 @@ describe('autoplan phase execution checkpoints', () => { test('loads full review skills at phase entry instead of prefetching future phases', () => { const intake = tmpl.split('### Step 3:')[1]?.split('## Phase 0.5:')[0] ?? ''; - expect(intake).toContain('At each phase, follow its full-load checkpoint'); + expect(intake).toContain("Resolve this phase's source to absolute ``; load via its checkpoint"); expect(intake).toContain('Do not prefetch future phase sections or review skills'); for (const phase of phases) { const section = read(`autoplan/sections/${phase}-phase.md.tmpl`); diff --git a/test/autoplan-review-discovery.test.ts b/test/autoplan-review-discovery.test.ts index b84bb55a2..1ad34157b 100644 --- a/test/autoplan-review-discovery.test.ts +++ b/test/autoplan-review-discovery.test.ts @@ -3,6 +3,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { spawnSync } from 'child_process'; +import { createHash } from 'crypto'; import { ALL_HOST_CONFIGS } from '../hosts'; import { generateAutoplanReviewFile } from '../scripts/resolvers/composition'; import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; @@ -19,6 +20,29 @@ function installFile(source: string, destination: string, mode: 'copy' | 'symlin else fs.symlinkSync(source, destination); } +function methodology(phase: string, entry: string, restore: string) { + return spawnSync(process.execPath, [path.join(ROOT, 'bin/gstack-autoplan-snapshot.ts'), 'methodology', phase, entry, restore], { + cwd: ROOT, encoding: 'utf8', timeout: 10_000, + }); +} + +function assertBundle(result: ReturnType, expected: string[]) { + expect(result.status, result.stderr).toBe(0); + const manifest = JSON.parse(result.stdout); + const bundle = fs.readFileSync(manifest.methodologyPath); + expect(createHash('sha256').update(bundle).digest('hex')).toBe(manifest.sha256); + expect(bundle.length).toBe(manifest.bytes); + expect(manifest.sources.map((part: any) => part.path)).toEqual(expected); + for (const part of manifest.sources) { + const actual = fs.readFileSync(part.path); + expect(bundle.subarray(part.startByte, part.endByte)).toEqual(actual); + expect(createHash('sha256').update(actual).digest('hex')).toBe(part.sha256); + expect(actual.length).toBe(part.bytes); + } + if (process.platform !== 'win32') expect(fs.statSync(manifest.methodologyPath).mode & 0o777).toBe(0o444); + return manifest; +} + beforeAll(() => { owned = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-autoplan-discovery-')); rendered = path.join(owned, 'rendered'); @@ -74,24 +98,38 @@ describe('autoplan reads installed host methodology', () => { ? fs.readFileSync(path.join(generatedRoot, 'autoplan', 'sections', `${phase}-phase.md`), 'utf8') : body; const directive = phaseBody.split('\n').find(line => line.startsWith('Before dispatch, fully Read ') - && (line.includes(`/${review}/SKILL.md`) || line.includes(`/gstack-${review}/SKILL.md`))); + && (line.includes(`methodology ${phase} `) || line.includes(`/${review}/SKILL.md`) || line.includes(`/gstack-${review}/SKILL.md`))); expect(directive).toBeDefined(); expect(phaseBody.indexOf(directive!)).toBeLessThan(phaseBody.indexOf(`create ${phase} `)); // U's CEO loaded this section only after its child finished. Pin the // concrete prerequisite, then resolve the rendered path in real // copy/symlink installations; references alone are not its contents. if (host.name === 'claude') { - expect(directive).toContain('and `sections/review-sections.md` beside it'); - const relative = directive!.match(/and `([^`]+)` beside it/)![1]!; + expect(directive).toContain(`methodology ${phase} `); + expect(directive).toContain('`methodologyPath` from'); + expect(directive).toContain('""'); + const relative = 'sections/review-sections.md'; const sectionSource = path.join(generatedRoot, review, relative); const sectionInstalled = path.resolve(path.dirname(entry), reference, '..', relative); installFile(sectionSource, sectionInstalled, mode); const section = fs.readFileSync(sectionInstalled, 'utf8'); expect(section).toBe(fs.readFileSync(sectionSource, 'utf8')); expect(section).toContain('## Review Sections'); + // W read the whole main file but skipped this section before Agent + // dispatch. A real helper invocation now supplies one complete + // byte-preserving target, instead of asking the model to concatenate. + expect(loaded).not.toContain('## Review Sections'); + const restore = path.join(registry, 'restore.md'); + fs.writeFileSync(restore, 'owned restore\n'); + const skillPath = path.resolve(path.dirname(entry), reference); + assertBundle(methodology(phase, skillPath, restore), [skillPath, sectionInstalled]); } else { expect(directive).not.toContain('sections/review-sections.md'); expect(loaded).toContain('## Review Sections'); + const restore = path.join(registry, 'restore.md'); + fs.writeFileSync(restore, 'owned restore\n'); + const skillPath = path.resolve(path.dirname(entry), reference); + assertBundle(methodology(phase, skillPath, restore), [skillPath]); } } } @@ -110,6 +148,40 @@ describe('autoplan reads installed host methodology', () => { } }); + test('methodology validation fails before publication; repeated valid loads keep prior bytes', () => { + const registry = fs.mkdtempSync(path.join(owned, 'method-errors-')); + const entry = path.join(registry, 'SKILL.md'); + const section = path.join(registry, 'sections/review-sections.md'); + const restore = path.join(registry, 'restore.md'); + fs.mkdirSync(path.dirname(section)); + fs.writeFileSync(restore, 'restore remains exact\n'); + const main = '---\r\nname: plan-ceo-review\r\n---\r\n## Section index\r\n| when | `sections/review-sections.md` |\r\n## Step 0\r\nFull first step 🌱\r\n'; + const deep = '## Review Sections\r\nRequired late verification β\r\n'; + fs.writeFileSync(entry, main); + const reject = () => { + const before = fs.readdirSync(registry).sort(); + const result = methodology('ceo', entry, restore); + expect(result.status).toBe(1); + expect(result.stderr).toContain('gstack-autoplan-snapshot:'); + expect(fs.readdirSync(registry).sort()).toEqual(before); + expect(fs.readFileSync(restore, 'utf8')).toBe('restore remains exact\n'); + }; + reject(); // required section absent + fs.writeFileSync(section, '```md\n## Review Sections\n```\n'); reject(); + fs.writeFileSync(section, deep + 'Read `sections/missing.md`.\n'); reject(); + fs.writeFileSync(section, deep); + fs.writeFileSync(entry, main.replace('name: plan-ceo-review', 'name: plan-eng-review')); reject(); + fs.writeFileSync(entry, main.replace('## Step 0', '| extra | `sections/extra.md` |\r\n## Step 0')); reject(); + fs.writeFileSync(entry, main); + const first = assertBundle(methodology('ceo', entry, restore), [entry, section]); + const original = fs.readFileSync(first.methodologyPath); + const second = assertBundle(methodology('ceo', entry, restore), [entry, section]); + expect(second.methodologyPath).not.toBe(first.methodologyPath); + expect(fs.readFileSync(first.methodologyPath)).toEqual(original); + expect(fs.readFileSync(entry, 'utf8')).toBe(main); + expect(fs.readFileSync(section, 'utf8')).toBe(deep); + }); + test('the new discovery contract selects the affected live autoplan workflows', () => { for (const name of ['autoplan-chain-pty', 'autoplan-dual-voice', 'carve-section-loading']) { expect(E2E_TOUCHFILES[name]).toContain('test/autoplan-review-discovery.test.ts'); diff --git a/test/ceo-completion-handoff.test.ts b/test/ceo-completion-handoff.test.ts index f01d10cd6..768dfaea9 100644 --- a/test/ceo-completion-handoff.test.ts +++ b/test/ceo-completion-handoff.test.ts @@ -12,6 +12,7 @@ import rCalls from './fixtures/ceo-completion-handoff-r-calls.json'; import tHandoff from './fixtures/ceo-completion-handoff-t-call.json'; import uHandoff from './fixtures/ceo-completion-handoff-u-call.json'; import vHandoff from './fixtures/ceo-completion-handoff-v-call.json'; +import wHandoff from './fixtures/ceo-completion-handoff-w-call.json'; type CapturedCall = typeof captures.cases[number]['calls'][number]; function nativeCall(record: CapturedCall, sessionId = 'native-capture'): NativePlanQuestionCall { @@ -25,6 +26,108 @@ function nativeCall(record: CapturedCall, sessionId = 'native-capture'): NativeP const handoff = () => nativeCall(captures.cases[0]!.calls.at(-1)!); const fingerprint = (call: NativePlanQuestionCall) => nativePlanCallFingerprint(call, 0, false); +describe('W unconditional CLEAR recap and required Eng pronoun navigation', () => { + const actual = () => structuredClone(wHandoff.calls.at(-1)!) as NativePlanQuestionCall; + const pending = (call: NativePlanQuestionCall) => { + const copy = structuredClone(call); copy.answered = false; delete copy.answers; delete copy.unansweredQuestionIndices; + return fingerprint(copy); + }; + const answer = (call: NativePlanQuestionCall) => { + call.answers = { [call.questions[0]!.question]: call.questions[0]!.options[0]!.label }; + return call; + }; + test('exact seven calls retain two issue decisions and select the offered manual action', () => { + const calls = structuredClone(wHandoff.calls) as NativePlanQuestionCall[]; + expect(replay(calls, false, ceoFirstReviewAUQ)) + .toMatchObject({ step0Count: 4, reviewCount: 2, administrativeCount: 1 }); + expect(isCeoCompletionHandoff(fingerprint(actual()))).toBe(true); + expect(pickCeoCompletionHandoff(pending(actual()))).toBe(2); + expect(pickCeoCompletionHandoff(fingerprint(actual()))).toBeNull(); + expect(calls).toEqual(wHandoff.calls); + }); + test('case, gap count and pure navigation option order do not change the meaning', () => { + const call = actual(); const q = call.questions[0]!; + q.question = q.question.toLowerCase().replace(' — ', ' - '); + q.options[0]!.description = q.options[0]!.description!.replace('2 assertion gaps', '12 assertion gaps'); + q.options.reverse(); answer(call); + expect(isCeoCompletionHandoff(fingerprint(call))).toBe(true); + expect(pickCeoCompletionHandoff(pending(call))).toBe(1); + }); + test('conditional, negated, quoted or additional question text is not a closed handoff', () => { + const source = actual().questions[0]!.question; + for (const question of [ + source.replace('is CLEAR.', 'is not CLEAR.'), source.replace('is CLEAR.', 'will be CLEAR.'), + source.replace('is CLEAR.', 'is CLEAR after tests pass.'), 'Once ' + source, + source.replace('required shipping gate', 'optional shipping check'), + source.replace('Eng review', 'Design review'), source.replace('run it next?', 'repair its findings next?'), + source + ' Remove the failing test.', source + ' Should we change the error contract?', + '> ' + source, 'Example: ' + source, '`' + source + '`', + source + ' ', + ]) { + const call = actual(); call.questions[0]!.question = question; answer(call); + expect(isCeoCompletionHandoff(fingerprint(call)), question).toBe(false); + expect(pickCeoCompletionHandoff(pending(call)), question).toBeNull(); + } + }); + test('every description sentence must be closed navigation, including unknown action verbs', () => { + for (const extra of [ + 'Delete the authorization test.', 'Grant access to all accounts.', 'Repair the missing assertion.', + 'One gap remains unresolved.', 'The CEO review is CLEAR only if we change the contract.', + 'The CEO review will be CLEAR after another fix.', 'Should we add another test?', + 'Quoted source: CEO review is CLEAR.', + ]) { + for (const index of [0, 1]) { + const call = actual(); call.questions[0]!.options[index]!.description += ' ' + extra; + expect(isCeoCompletionHandoff(fingerprint(call)), extra).toBe(false); + expect(pickCeoCompletionHandoff(pending(call)), extra).toBeNull(); + } + } + for (const description of ['', 'This CEO review held scope and resolved some assertion gaps — eng review verifies the test structure is sound.', + 'This CEO review held scope and resolved 2 assertion gaps after changing the contract — eng review verifies the test structure is sound.']) { + const call = actual(); call.questions[0]!.options[0]!.description = description; + expect(isCeoCompletionHandoff(fingerprint(call))).toBe(false); + expect(pickCeoCompletionHandoff(pending(call))).toBeNull(); + } + }); + test('native identity, complete answers, Eng/manual choices and a single question remain required', () => { + for (const mutate of [ + (c: NativePlanQuestionCall) => { c.failed = true; }, + (c: NativePlanQuestionCall) => { delete c.failed; }, + (c: NativePlanQuestionCall) => { c.answered = false; }, + (c: NativePlanQuestionCall) => { c.unansweredQuestionIndices = [0]; }, + (c: NativePlanQuestionCall) => { delete c.unansweredQuestionIndices; }, + (c: NativePlanQuestionCall) => { c.answers = { [c.questions[0]!.question]: 'Fix another issue' }; }, + (c: NativePlanQuestionCall) => { c.questions.push(structuredClone(c.questions[0]!)); }, + (c: NativePlanQuestionCall) => { c.questions[0]!.multiSelect = true; }, + (c: NativePlanQuestionCall) => { c.questions[0]!.header = 'New finding'; }, + (c: NativePlanQuestionCall) => { c.questions[0]!.options[0]!.label = 'Run /plan-design-review'; answer(c); }, + (c: NativePlanQuestionCall) => { c.questions[0]!.options[1]!.label = 'Fix remaining issues manually'; }, + (c: NativePlanQuestionCall) => { c.questions[0]!.options.push(structuredClone(c.questions[0]!.options[1]!)); }, + ]) { const call = actual(); mutate(call); expect(isCeoCompletionHandoff(fingerprint(call))).toBe(false); } + expect(pickCeoCompletionHandoff({ ...pending(actual()), signature: 'foreign:call' })).toBeNull(); + expect(pickCeoCompletionHandoff({ ...pending(actual()), nativeCall: undefined })).toBeNull(); + }); + test('controlled report time excludes the handoff but still rejects a later real issue answer', () => { + expect(wHandoff.provenance.reportMtimeMs).toBeNull(); // No historical filesystem-time claim. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ceo-w-handoff-')); + try { + const calls = structuredClone(wHandoff.calls) as NativePlanQuestionCall[]; + const issueAt = Date.parse(calls.at(-2)!.answeredAt!); + const navigationAt = Date.parse(calls.at(-1)!.answeredAt!); + const syntheticWritten = Math.floor((issueAt + navigationAt) / 2); + const file = path.join(dir, 'report.md'); fs.writeFileSync(file, wHandoff.reportContent); + fs.utimesSync(file, syntheticWritten / 1000, syntheticWritten / 1000); + const transcript = { status: 'ready' as const, calls, assistantMessages: [], planReadyRequests: wHandoff.planReadyRequests }; + const admin = new Set(calls.filter(c => isCeoCompletionHandoff(fingerprint(c))).map(c => `${c.sessionId}:${c.toolUseId}`)); + const start = Date.parse('2026-09-09T09:28:55Z'); + expect(hasNativePlanTerminal(transcript, file, start, 'plan_ready', new Set())).toBe(false); + expect(hasNativePlanTerminal(transcript, file, start, 'plan_ready', admin)).toBe(true); + calls.at(-2)!.answeredAt = new Date(syntheticWritten + 1000).toISOString(); + expect(hasNativePlanTerminal(transcript, file, start, 'plan_ready', admin)).toBe(false); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } + }); +}); + describe('V closed CEO recap with a resolved-gap count', () => { const actual = () => structuredClone(vHandoff.calls.at(-1)!) as NativePlanQuestionCall; const pending = (call: NativePlanQuestionCall) => { diff --git a/test/claude-code-runner.test.ts b/test/claude-code-runner.test.ts index 22ee367e9..47b41670c 100644 --- a/test/claude-code-runner.test.ts +++ b/test/claude-code-runner.test.ts @@ -85,6 +85,11 @@ describe('Claude Code restricted execution', () => { expect(capture().auth).toBe('fake-test-credential'); const args = capture().args; expect(args[args.indexOf('--tools') + 1]).toBe(''); + const capability = args[args.indexOf('--append-system-prompt') + 1]; + expect(capability).toContain('No tools are available'); + expect(capability).toContain('Do not attempt or simulate tool calls'); + expect(capability).toContain('If essential context is missing, identify it explicitly'); + expect(capability).not.toContain(prompt); expect(args).toContain('--disable-slash-commands'); expect(args).toContain('--strict-mcp-config'); expect(JSON.parse(args[args.indexOf('--mcp-config') + 1])).toEqual({mcpServers:{}}); @@ -106,6 +111,7 @@ describe('Claude Code restricted execution', () => { const args = capture().args; expect(args[args.indexOf('--tools') + 1]).toBe('Read,Grep,Glob'); expect(args[args.indexOf('--allowedTools') + 1]).toBe('Read,Grep,Glob'); + expect(args).not.toContain('--append-system-prompt'); expect(args[args.indexOf('--resume') + 1]).toBe(resume); expect(args).not.toContain('--no-session-persistence'); }); diff --git a/test/design-artifact-question.test.ts b/test/design-artifact-question.test.ts new file mode 100644 index 000000000..6b8dc4baa --- /dev/null +++ b/test/design-artifact-question.test.ts @@ -0,0 +1,114 @@ +import { expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { execFileSync } from 'node:child_process'; +import calls from './fixtures/design-artifacts-w-calls.json'; +import { isDesignArtifactGeneration } from './helpers/design-artifact-question'; +import { nativePlanCallFingerprint, planCountQuestionPhase } from './helpers/claude-pty-runner'; +import type { NativePlanQuestionCall } from './helpers/plan-count-transcript'; + +const fp = (call: any) => nativePlanCallFingerprint(call as NativePlanQuestionCall, 0, false); +const artifacts = [calls[2]!, calls[3]!]; + +test('all eight W calls remain visible: five seeded findings, one shell decision and two artifact approvals', () => { + const phases = calls.map(call => planCountQuestionPhase(fp(call), true, () => false, + undefined, undefined, undefined, isDesignArtifactGeneration)); + expect(phases.map(p => p.administrative ?? 'review')).toEqual([ + 'review', 'review', 'artifact-generation', 'artifact-generation', 'review', 'review', 'review', 'review', + ]); + for (const call of artifacts) { + expect(planCountQuestionPhase(fp(call), false, () => false, () => true, + undefined, undefined, isDesignArtifactGeneration)).toEqual({ + preReview: false, reviewStarted: false, administrative: 'artifact-generation', + }); + } +}); + +test('new decisions, missing coverage, altered artifacts, deferrals and quoted examples remain findings', () => { + for (const original of artifacts) { + for (const mutate of [ + (c: any) => { c.questions[0].options[0].description += ' Also change the Save behavior.'; }, + (c: any) => { c.questions[0].options[0].description += ' Drop the error state.'; }, + (c: any) => { c.questions[0].options[0].description = c.questions[0].options[0].description.replace('No new design decisions', 'Choose new design decisions'); }, + (c: any) => { c.questions[0].options[1].description += ' The failure contract is still missing.'; }, + (c: any) => { c.questions[0].options[0].preview = 'Change the save contract'; }, + (c: any) => { c.answers[c.questions[0].question] = c.questions[0].options[1].label; }, + (c: any) => { const q = c.questions[0]; const a = c.answers[q.question]; q.question = 'Example: ' + q.question; c.answers = { [q.question]: a }; }, + (c: any) => { c.questions.push(calls[7]!.questions[0]); }, + (c: any) => { c.answered = false; }, (c: any) => { c.failed = true; }, + (c: any) => { delete c.failed; }, (c: any) => { delete c.unansweredQuestionIndices; }, + (c: any) => { c.unansweredQuestionIndices = [0]; }, (c: any) => { c.answeredAt = 'invalid'; }, + (c: any) => { c.sessionId = ''; }, + ]) { + const call = structuredClone(original); mutate(call); + expect(isDesignArtifactGeneration(fp(call))).toBe(false); + } + const reordered = structuredClone(original); reordered.questions[0]!.options.reverse(); + expect(isDesignArtifactGeneration(fp(reordered))).toBe(true); + expect(isDesignArtifactGeneration({ ...fp(original), signature: 'foreign' })).toBe(false); + } +}); + +const REPORT = '# Reviewed plan\n\n## GSTACK REVIEW REPORT\n\n| Review | Status | Findings |\n|---|---|---|\n| Design | clean | recorded |\n\nVERDICT: Review complete\n\nNO UNRESOLVED DECISIONS\n'; +const GATE = 'Exit plan mode?\n\nClaude wants to exit plan mode\n❯ 1. Yes, and switch to default (ask each time) for this session\n 2. No\n'; + +test.skipIf(process.platform === 'win32').each(['only-artifacts', 'freshness'] as const)('real fake-PTY artifact %s preserves coverage and fresh-report requirements', async mode => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'design-artifact-free-')); + const fake = path.join(dir, 'fake-claude'), worker = path.join(dir, 'worker.ts'); + const report = path.join(dir, 'report.md'), output = path.join(dir, 'result.json'); + const pidFile = path.join(dir, 'pid.json'), inputs = path.join(dir, 'inputs.jsonl'); + const refreshed = path.join(dir, 'refreshed'); + const selected = mode === 'only-artifacts' ? artifacts : [calls[0]!, artifacts[0]!]; + fs.writeFileSync(fake, `#!${process.execPath}\n` + String.raw` +import * as fs from 'node:fs'; import * as path from 'node:path'; +const stat = process.platform === 'linux' ? fs.readFileSync('/proc/self/stat','utf8') : null; +fs.writeFileSync(process.env.PID_FILE, JSON.stringify({pid:process.pid,start:stat?.slice(stat.lastIndexOf(')')+2).split(' ')[19]})); +let sent=false; process.stdin.setRawMode?.(true); +process.stdin.on('data', data => { + fs.appendFileSync(process.env.INPUT_FILE,JSON.stringify(data.toString())+'\n'); + if(sent)return;sent=true; + const at=Date.now(),sid='artifact-free'; + const project=path.join(process.env.CLAUDE_CONFIG_DIR,'projects','owned');fs.mkdirSync(project,{recursive:true}); + const events=JSON.parse(process.env.CALLS).flatMap((call,i)=>[ + {cwd:process.cwd(),sessionId:sid,isSidechain:false,timestamp:new Date(at-100+i*10).toISOString(),message:{role:'assistant',content:[{type:'tool_use',id:call.toolUseId,name:'AskUserQuestion',input:{questions:call.questions}}]}}, + {cwd:process.cwd(),sessionId:sid,isSidechain:false,timestamp:new Date(at-99+i*10).toISOString(),toolUseResult:{answers:call.answers},message:{role:'user',content:[{type:'tool_result',tool_use_id:call.toolUseId,content:'Your questions have been answered: '+Object.entries(call.answers).map(([q,a])=>JSON.stringify(q)+'='+JSON.stringify(a)).join(', ')+'. You can now continue with these answers in mind.'}]}} + ]); + events.push({cwd:process.cwd(),sessionId:sid,isSidechain:false,timestamp:new Date(at).toISOString(),message:{role:'assistant',content:[{type:'text',text:'Design review complete.'},{type:'tool_use',id:'exit',name:'ExitPlanMode',input:{}}]}}); + fs.writeFileSync(path.join(project,sid+'.jsonl'),events.map(e=>JSON.stringify(e)+'\n').join('')); + fs.writeFileSync(process.env.REPORT_FILE,process.env.REPORT); + if(process.env.MODE==='freshness') { + fs.utimesSync(process.env.REPORT_FILE,(at-95)/1000,(at-95)/1000); + setTimeout(()=>{fs.writeFileSync(process.env.REPORT_FILE,process.env.REPORT);fs.writeFileSync(process.env.REFRESHED,'yes');},5500); + } + process.stdout.write(process.env.GATE); +});process.stdin.resume(); +`); fs.chmodSync(fake,0o755); + const runner = pathToFileURL(path.join(import.meta.dir,'helpers/claude-pty-runner.ts')).href; + const artifactHelper = pathToFileURL(path.join(import.meta.dir,'helpers/design-artifact-question.ts')).href; + const env = {PID_FILE:pidFile,INPUT_FILE:inputs,REPORT_FILE:report,REPORT,GATE,CALLS:JSON.stringify(selected),MODE:mode,REFRESHED:refreshed}; + fs.writeFileSync(worker, `import {runPlanSkillCounting} from ${JSON.stringify(runner)};\nimport {isDesignArtifactGeneration} from ${JSON.stringify(artifactHelper)};\nconst result=await runPlanSkillCounting({skillName:'plan-design-review',slashCommand:'/plan-design-review',followUpPrompt:'# Artifact control',expectedPlanPath:${JSON.stringify(report)},isLastStep0AUQ:()=>false,isFirstReviewAUQ:()=>true,isArtifactGenerationAUQ:isDesignArtifactGeneration,reviewCountCeiling:8,timeoutMs:33000,env:${JSON.stringify(env)}});await Bun.write(${JSON.stringify(output)},JSON.stringify(result));\n`); + const child = Bun.spawn([process.execPath,worker],{env:{...process.env,EVALS_HERMETIC:'1',EVALS_RUN_ID:'',BROWSE_TERMINAL_BINARY:fake},stdout:'pipe',stderr:'pipe'}); + const timer=setTimeout(()=>child.kill('SIGKILL'),35000); + try { + const [code,out,err]=await Promise.all([child.exited,new Response(child.stdout).text(),new Response(child.stderr).text()]); + expect(code,out+err).toBe(0);const result=JSON.parse(fs.readFileSync(output,'utf8')); + expect(result.transcript.calls).toHaveLength(2); + expect(result.administrativeCount).toBe(mode==='only-artifacts'?2:1); + expect(result.reviewCount).toBe(mode==='only-artifacts'?0:1); + expect(result.outcome).toBe(mode==='only-artifacts'?'no_review_questions':'plan_ready'); + if(mode==='freshness') expect(fs.existsSync(refreshed)).toBe(true); + expect(fs.readFileSync(inputs,'utf8').trim().split('\n').map(x=>JSON.parse(x))).toEqual(['/plan-design-review\r']); + } finally { + clearTimeout(timer);child.kill('SIGKILL'); + if(fs.existsSync(pidFile))try { + const p=JSON.parse(fs.readFileSync(pidFile,'utf8'));let owned=false; + if(process.platform==='linux') { + const s=fs.readFileSync(`/proc/${p.pid}/stat`,'utf8');owned=s.slice(s.lastIndexOf(')')+2).split(' ')[19]===p.start && fs.readFileSync(`/proc/${p.pid}/cmdline`,'utf8').split('\0').includes(fake); + } else owned=execFileSync('ps',['-p',String(p.pid),'-o','command='],{encoding:'utf8',timeout:1000}).includes(fake); + if(owned)process.kill(p.pid,'SIGKILL'); + }catch{/* owned fake already gone */} + fs.rmSync(dir,{recursive:true,force:true}); + } +},40000); diff --git a/test/fixtures/ceo-completion-handoff-w-call.json b/test/fixtures/ceo-completion-handoff-w-call.json new file mode 100644 index 000000000..56db4ad01 --- /dev/null +++ b/test/fixtures/ceo-completion-handoff-w-call.json @@ -0,0 +1,252 @@ +{ + "provenance": { + "observation": ".context/ship-source-w-full-paid-20260909-0928/native-observer-closure-v1/attempt-final/plan-ceo-review-1788946756315-4sRoMx/observation.json", + "observationSha256": "946bb1cf1694c28cbb5bec96e7004247933b7f61f5fab39de84460243eeb45ed", + "reportProof": ".context/ship-source-w-full-paid-20260909-0928/ceo-retry-report-versions-v1/proof.json", + "reportProofSha256": "150520848c0d938fbd5b3f635ac4282e4a20efd10a0c3ebdc1b3a4e13261d8c6", + "reportSha256": "811397e1ae81671e0a32f08013f79e436a303d675f7e6c0924fdeff5a59d4c61", + "reportBytes": 21351, + "reportMtimeMs": null, + "rawCounts": { + "step0Count": 4, + "reviewCount": 3, + "administrativeCount": 0 + }, + "rawState": "in_progress", + "limits": "Original report mtime was not retained. Snapshot timestamps are not substituted for it. Unit freshness checks use an explicitly synthetic timestamp; W remains globally timed out." + }, + "calls": [ + { + "sessionId": "2bafbd5b-abe8-4b41-9872-3d8126188b94", + "toolUseId": "toolu_01DouqUfSF7pXo6AT4kdSq6S", + "questions": [ + { + "question": "gstack works best when your project's CLAUDE.md includes skill routing rules \u2014 so the right skill gets invoked automatically. Add them to this project's CLAUDE.md?", + "header": "Routing rules", + "multiSelect": false, + "options": [ + { + "label": "Add routing rules (recommended)", + "description": "Appends a ## Skill routing section to CLAUDE.md. Note: this is a plan-review fixture repo, so you may prefer to skip." + }, + { + "label": "No thanks, manual only", + "description": "Skips the update. You can re-enable with gstack-config set routing_declined true." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "gstack works best when your project's CLAUDE.md includes skill routing rules \u2014 so the right skill gets invoked automatically. Add them to this project's CLAUDE.md?": "Add routing rules (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:40:40.622Z" + }, + { + "sessionId": "2bafbd5b-abe8-4b41-9872-3d8126188b94", + "toolUseId": "toolu_01QM8FC4oqXEg3hKTFTBsTnf", + "questions": [ + { + "question": "No design doc found for this branch. /office-hours produces a structured problem statement, premise challenge, and explored alternatives \u2014 it gives this review much sharper input. Want to run it first?", + "header": "Office hours?", + "multiSelect": false, + "options": [ + { + "label": "Skip \u2014 proceed with standard review (recommended)", + "description": "The plan is concise and well-scoped (two unit tests for processPayment). Standard review is sufficient here." + }, + { + "label": "Run /office-hours first", + "description": "Takes ~10 min to produce a structured design doc, then we pick up the CEO review right after." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "No design doc found for this branch. /office-hours produces a structured problem statement, premise challenge, and explored alternatives \u2014 it gives this review much sharper input. Want to run it first?": "Skip \u2014 proceed with standard review (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:41:02.700Z" + }, + { + "sessionId": "2bafbd5b-abe8-4b41-9872-3d8126188b94", + "toolUseId": "toolu_01TS918a5t4AZH9vHhUUPXDh", + "questions": [ + { + "question": "Which implementation approach should guide this review?", + "header": "Approach", + "multiSelect": false, + "options": [ + { + "label": "A: Minimal \u2014 exactly as specified (recommended)", + "description": "Two unit tests only: happy path + 502. Reuses factory/sleeper/mock. Completeness 7/10. Smallest diff, matches plan's stated 'deliberately separate concerns' framing.", + "preview": "APPROACH A: Minimal viable\n Tests: 2 (happy path + 502 retry)\n Effort: S \u2014 human ~1h / CC ~5 min\n LOC: ~60\n Score: Completeness 7/10\n Reuses: factory, sleeper, Stripe mock" + }, + { + "label": "B: Full processPayment() coverage", + "description": "The 2 specified tests + 402, 429, receipt-builder failure, idempotency under retry. Completeness 9/10. Catches more real bugs but wider diff.", + "preview": "APPROACH B: Full coverage\n Tests: 6-8 (happy + 502 + 402 + 429\n + builder failure + idempotency)\n Effort: M \u2014 human ~3-4h / CC ~15 min\n LOC: ~150-200\n Score: Completeness 9/10" + }, + { + "label": "C: Table-driven / parametric", + "description": "Same breadth as B but expressed as a parametric test structure. Fewer LOC, easy to extend, but mixes correctness + degradation (contradicts plan's 'separate concerns' call).", + "preview": "APPROACH C: Table-driven\n Tests: 1 parametric covering 5+ cases\n Effort: S-M \u2014 human ~2h / CC ~10 min\n LOC: ~100\n Score: Completeness 9/10\n Risk: contradicts 'separate concerns' design" + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "Which implementation approach should guide this review?": "A: Minimal \u2014 exactly as specified (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:42:19.517Z" + }, + { + "sessionId": "2bafbd5b-abe8-4b41-9872-3d8126188b94", + "toolUseId": "toolu_01W2gJwyumADmyKD1cdjoRUh", + "questions": [ + { + "question": "Which review mode should I run? (You chose Approach A: 2 unit tests, minimal scope.)", + "header": "Review mode", + "multiSelect": false, + "options": [ + { + "label": "HOLD SCOPE \u2014 make the 2 tests bulletproof (recommended)", + "description": "Accept the plan's scope. Review with maximum rigor: assertion quality, edge cases within the 2 tests, observability, failure modes, mock setup correctness. No expansions surfaced." + }, + { + "label": "SELECTIVE EXPANSION \u2014 baseline + cherry-pick extras", + "description": "Review the 2 tests with rigor, then surface individual expansion opportunities (e.g. idempotency test, receipt-builder failure) for you to opt in or skip. Neutral recommendations." + }, + { + "label": "SCOPE EXPANSION \u2014 dream bigger", + "description": "Envision the ambitious version: mutation testing, full processPayment() contract, integration test layer. Each proposal presented individually for your approval." + }, + { + "label": "SCOPE REDUCTION \u2014 find the minimum", + "description": "Already minimal (2 tests) \u2014 only choose this if you think even fewer tests would suffice." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "Which review mode should I run? (You chose Approach A: 2 unit tests, minimal scope.)": "HOLD SCOPE \u2014 make the 2 tests bulletproof (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:42:35.584Z" + }, + { + "sessionId": "2bafbd5b-abe8-4b41-9872-3d8126188b94", + "toolUseId": "toolu_015MLgtzrhpX8nh5hMiVPtAG", + "questions": [ + { + "question": "D5 \u2014 What does processPayment() do when the 502 retry is exhausted? The plan says 'fails clean' but that's not a testable assertion. The 502 test must assert something specific.", + "header": "502 error contract", + "multiSelect": false, + "options": [ + { + "label": "Check production code \u2014 add exact exception to plan (recommended)", + "description": "Look at what processPayment() raises when Stripe retries exhaust. Add the specific class (e.g. PaymentGatewayError, Stripe::BadGateway) to the plan so the test asserts it precisely. Completeness: 10/10." + }, + { + "label": "Raises an exception \u2014 document it in the plan", + "description": "The 502 test will assert `raise_error(SpecificErrorClass)`. Add the class name to the plan now. Completeness: 10/10." + }, + { + "label": "Returns a sentinel (nil or Result.failure)", + "description": "The 502 test will assert nil return or Result.failure pattern. Add the expected return shape to the plan now. Completeness: 10/10." + }, + { + "label": "Keep 'fails clean' \u2014 leave assertion to implementer", + "description": "Accepts the ambiguity. Developer figures out the contract from the code. Risk: test may only assert `raise_error` (catches any exception, including bugs). Completeness: 6/10." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D5 \u2014 What does processPayment() do when the 502 retry is exhausted? The plan says 'fails clean' but that's not a testable assertion. The 502 test must assert something specific.": "Check production code \u2014 add exact exception to plan (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:49:03.078Z" + }, + { + "sessionId": "2bafbd5b-abe8-4b41-9872-3d8126188b94", + "toolUseId": "toolu_01WxzEVe9SLn16nwFDYgRHBZ", + "questions": [ + { + "question": "D6 \u2014 What makes a receipt 'correct' in the happy-path test? The plan says 'assert correct receipt is generated' but doesn't say which fields to verify.", + "header": "Receipt assertion", + "multiSelect": false, + "options": [ + { + "label": "Assert receipt fields bound to mock charge data (recommended)", + "description": "Assert receipt.charge_id matches the Stripe mock's returned charge ID, receipt.amount matches, etc. Ties assertions to the factory's mock output \u2014 catches any mis-routing or wrong-charge bugs. Completeness: 10/10." + }, + { + "label": "Assert receipt is non-nil only", + "description": "Simpler but weak \u2014 passes even if receipt has wrong charge_id or wrong amount. Completeness: 5/10." + }, + { + "label": "Leave assertion definition to the implementer", + "description": "Developer decides what 'correct' means from the production code. Risk: test may only assert presence. Completeness: 6/10." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D6 \u2014 What makes a receipt 'correct' in the happy-path test? The plan says 'assert correct receipt is generated' but doesn't say which fields to verify.": "Assert receipt fields bound to mock charge data (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:49:13.122Z" + }, + { + "sessionId": "2bafbd5b-abe8-4b41-9872-3d8126188b94", + "toolUseId": "toolu_01GHHxQzpHdBGoFtGqMRhFGt", + "questions": [ + { + "question": "CEO review is CLEAR. Eng review is the required shipping gate \u2014 run it next?", + "header": "Next review", + "multiSelect": false, + "options": [ + { + "label": "Run /plan-eng-review (recommended)", + "description": "Architecture, code quality, tests, performance review. Required gate before shipping. This CEO review held scope and resolved 2 assertion gaps \u2014 eng review verifies the test structure is sound." + }, + { + "label": "Skip \u2014 I'll handle reviews manually", + "description": "You manage the review pipeline yourself. Note: eng review is required to CLEAR for /ship." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "CEO review is CLEAR. Eng review is the required shipping gate \u2014 run it next?": "Run /plan-eng-review (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:52:10.256Z" + } + ], + "planReadyRequests": [ + { + "sessionId": "2bafbd5b-abe8-4b41-9872-3d8126188b94", + "toolUseId": "toolu_01Smj246aCYcFRF8wQvHwz9a", + "timestamp": "2026-09-09T09:52:16.599Z", + "failed": false + } + ], + "reportContent": "# CEO Review: Payment Processing \u2014 Test Coverage\n\n**Skill:** /plan-ceo-review | **Mode:** HOLD SCOPE | **Approach:** A (minimal, exactly as specified)\n**Branch:** main | **Date:** 2026-09-09\n\n---\n\n## Context\n\n`processPayment()` has zero unit tests. The existing test infrastructure (test factory, Stripe mock, virtual sleeper) is already in place. This plan adds two unit tests that cover the orchestration layer \u2014 not duplicating the adapter-level Stripe suite, but testing how `processPayment()` wires charge + receipt together.\n\n## Step 0 Summary\n\n**Premise:** Valid. `processPayment()` is a payment-critical path with no unit-level safety net.\n**Approach A selected:** 2 unit tests \u2014 happy path (charge \u2192 receipt) and 502 retry path.\n**Mode:** HOLD SCOPE \u2014 make these 2 tests bulletproof. No expansions surfaced.\n\n### 0A. Premise Challenge\nThe problem is real: `processPayment()` has no unit tests. The adapter suite covers Stripe responses in isolation; the receipt-builder failure tests cover one component; neither covers the orchestration. If `processPayment()` mis-routes a receipt after a charge, or silently succeeds on a 502, neither existing suite catches it.\n\nDoing nothing: next payment bug surfaces in production or is caught only if both the adapter and receipt-builder tests happen to cover the exact interaction \u2014 fragile.\n\n### 0B. Existing Code Leverage\nAll infrastructure is in place. The test factory injects:\n- Stripe mock (call history exposed)\n- Virtual sleeper (records backoff without real delays)\n- `max_retries=1` configuration (exhausted 502 = exactly 2 charge attempts)\n\nNo rebuilding needed. The plan correctly reuses all of it.\n\n### 0C. Dream State\n```\nCURRENT STATE THIS PLAN 12-MONTH IDEAL\nprocessPayment() has 0 Add happy-path test + Full processPayment()\nunit tests. Adapter suite Add 502 retry test. contract: happy, 4xx,\ncovers Stripe isolation. Reuse factory/sleeper/ 502, idempotency,\nReceipt-builder tests mock. ~2 tests, ~60 LOC. receipt-builder\nexist separately. failure propagation.\n```\n\n---\n\n## Section 1: Architecture Review\n\n**No issues.** This is a test-only change \u2014 no new production components, no new coupling, no rollback required.\n\nTest architecture:\n```\n [Payment Test Factory]\n \u251c\u2500\u2500 max_retries=1\n \u251c\u2500\u2500 Virtual Sleeper\n \u2514\u2500\u2500 Stripe Mock (call history)\n \u2502\n \u25bc\n processPayment()\n \u2502\n \u250c\u2500\u2500\u2500\u2500\u2500\u2534\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n \u2502 (happy path) (502 path) \u2502\n \u25bc \u25bc \u2502\n Stripe OK Stripe 502 \u2502\n \u2502 \u2502 \u2502\n \u25bc \u25bc \u2502\n Receipt retry (\u00d71) \u2502\n Builder \u2502 \u2502\n \u2502 Stripe 502 \u2502\n \u25bc \u2502 \u2502\n receipt fails \u2502\n clean \u2502\n \u2514\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500-\u2500\u2518\n```\n\nData flows for the 4 shadow paths are exercised within the 2 proposed tests:\n- Happy path: full success chain tested\n- Nil path: not tested (out of scope per Approach A \u2014 input validation is a separate concern)\n- Empty path: not tested (same)\n- Error path: 502 exhaustion tested\n\nNo security architecture changes. No new endpoints. No deployment risk.\n\n---\n\n## Section 2: Error & Rescue Map\n\n**FINDING [S2-1]:** \"fails clean\" is unspecified.\n\nThe 502 test plan says: _\"assert retry-with-backoff fires once, then fails clean.\"_\n\n`fails clean` is not a testable assertion without knowing processPayment()'s error contract:\n- Does it raise a specific exception? (e.g., `Stripe::BadGateway`, `PaymentGatewayError`)\n- Does it return a sentinel value? (nil, `Result.failure(...)`, an error struct)\n- Does the error carry context? (charge attempt count, last error code)\n\n**Why this matters:** A developer writing this test must assert something specific. Without knowing the expected error type/return value, the test can be written trivially and still \"pass\" the plan:\n```\n# Too weak \u2014 this just checks \"something was raised\":\nexpect { processPayment(payment) }.to raise_error\n```\nvs.\n```\n# Correct \u2014 this checks the exact error contract:\nexpect { processPayment(payment) }.to raise_error(PaymentGatewayError, /502/)\n```\n\nError & Rescue Map (incomplete \u2014 gap marked):\n\n```\nMETHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS\n---------------------|----------------------------|------------------\nprocessPayment() | Stripe returns 502 | UNKNOWN \u2190 GAP\n | Retry exhausted (max=1) | UNKNOWN \u2190 GAP\n | Receipt builder fails | (separate test coverage)\n | Stripe returns success | \u2014\n\nEXCEPTION CLASS | RESCUED? | RESCUE ACTION | USER SEES\n---------------------|----------|-------------------------|------------------\nStripe 502 | Y | Retry \u00d71 with backoff | (buffered)\n502 exhausted | ? | fails clean \u2192 ??? | UNKNOWN \u2190 GAP\n```\n\n**RESOLVED (S2-1):** Implementer must check production `processPayment()` to find the specific exception class raised when Stripe retry is exhausted. The 502 test assertion must use that class: `expect { processPayment(payment) }.to raise_error(SpecificErrorClass)` \u2014 not bare `raise_error`. Add the class name to the test and to this plan before merging.\n\n---\n\n## Section 3: Security & Threat Model\n\n**No issues.** Test-only change. No new endpoints, no new params, no new secrets. The test factory uses mocked payment data (not real PII). No audit trail requirements for unit tests.\n\n---\n\n## Section 4: Data Flow & Interaction Edge Cases\n\n**No issues for test-only scope.** No user-facing interactions. The data flow shadow paths are evaluated in Section 6 (test coverage). No async ordering concerns \u2014 the virtual sleeper is synchronous.\n\n---\n\n## Section 5: Code Quality\n\n**No issues at plan level.** The plan correctly calls for reusing all existing helpers (factory, sleeper, mock). The \"deliberately separate concerns\" framing (success = correctness, failure = graceful degradation) is sound \u2014 avoids conflating two different failure modes into one test. Naming should follow this framing: `test_happy_path_generates_correct_receipt` and `test_502_retry_exhaustion_fails_clean`.\n\nOne quality note (not a blocker): since `max_retries=1` means exactly 2 charge attempts on 502 exhaustion, the 502 test should assert Stripe mock was called **exactly twice** (not \"at least once\") to verify the retry count contract precisely.\n\n---\n\n## Section 6: Test Review (PENDING \u2014 partial)\n\n### New codepaths\n\n```\nNEW UX FLOWS: none (test-only)\nNEW DATA FLOWS: none (test-only)\nNEW CODEPATHS: unit test: processPayment() happy path\n unit test: processPayment() 502 retry + exhaustion\nNEW BACKGROUND JOBS: none\nNEW INTEGRATIONS: none (Stripe mock only)\nNEW ERROR PATHS: processPayment() 502 exhaustion observable behavior (GAP: see S2-1)\n```\n\n**Test 1: Happy path**\n- Type: Unit\n- In plan? Yes\n- Happy path test: Stripe mock returns success \u2192 receipt returned \u2192 *assert receipt is \"correct\"*\n- **RESOLVED [S6-1]:** Assert receipt fields bound to mock charge data. The happy-path test must assert `receipt.charge_id == stripe_mock.last_charge.id` (and equivalent for amount, currency, or whatever fields the receipt captures from the charge response). Never assert only `expect(receipt).not_to be_nil` \u2014 that catches nothing.\n- Edge case: Use factory-returned mock charge metadata in assertions \u2014 not hardcoded values \u2014 so the test fails if processPayment() mis-routes the receipt to the wrong charge.\n\n**Test 2: 502 retry path**\n- Type: Unit\n- In plan? Yes\n- The plan specifies 3 sub-assertions:\n 1. Retry-with-backoff fires \u2014 **testable**: virtual sleeper records exactly 1 backoff event\n 2. Exactly 2 charge attempts \u2014 **testable**: Stripe mock call history count == 2\n 3. \"Fails clean\" \u2014 **blocked by S2-1**: requires knowing processPayment()'s error contract\n- Failure path: what wrong result this test should reject:\n - Only 1 Stripe call (no retry fired)\n - 0 backoff records (retry without backoff)\n - A receipt being returned despite 502 (silent success)\n - Exception type wrong or missing\n\n**Test ambition check (HOLD SCOPE):**\n- 2am Friday confidence: requires both tests to assert specific values, not just \"something non-nil\"\n- Hostile QA: would attempt `processPayment()` with a 502 and check whether a receipt leaks through \u2014 this is caught by the 502 test only if we assert no receipt was generated\n- Chaos: virtual sleeper eliminates time-dependency flakiness \u2713\n\n**Flakiness risk:** None \u2014 virtual sleeper, deterministic mock, no external calls. \u2713\n\n**Both gaps resolved.** See S2-1 and S6-1 decisions above.\n\n---\n\n## Section 7: Performance Review\n\n**No issues.** Test-only change. Virtual sleeper eliminates wait time. No DB, no network, no caching concerns.\n\n---\n\n## Section 8: Observability & Debuggability Review\n\n**No issues for HOLD SCOPE.** The virtual sleeper records backoff, and mock call history records charge attempts \u2014 these are the observability hooks the test already uses. Adding production-side log assertions is out of scope for HOLD SCOPE (no evidenced gap in the accepted scope's correctness).\n\n---\n\n## Section 9: Deployment & Rollout Review\n\n**No issues.** Test-only change. No migration, no feature flag, no deployment sequence. Ship by merging \u2014 zero downtime risk.\n\n---\n\n## Section 10: Long-Term Trajectory\n\n**No issues.** These tests reduce technical debt. Reversibility: 5/5 (tests are trivially removable). Path dependency: none. The tests serve as executable documentation of `processPayment()`'s contract \u2014 a future engineer reading them learns what the function guarantees.\n\nOne note: the 12-month ideal (full contract coverage) requires these 2 tests to be internally consistent so future tests extend them rather than duplicate them. The factory-based setup and clear concern separation help here.\n\n---\n\n## Section 11: Design & UX Review\n\n**SKIPPED.** No UI scope detected.\n\n---\n\n## Codex / Outside Voice\n\nCodex review skipped (`codex_reviews` disabled). Re-enable: `gstack-config set codex_reviews enabled`.\n\nOutside voice: disabled \u2014 no independent pass ran.\n\n---\n\n## NOT in scope\n\n- processPayment() behavior under nil/empty payment input (approach A, test-only)\n- Card decline (402) at processPayment() level (adapter-level coverage exists)\n- Rate limit (429) at processPayment() level (adapter-level coverage exists)\n- Receipt-builder failure path in processPayment() tests (own passing regression tests exist)\n- Idempotency under retry (deferred \u2014 not in approach A)\n- Log emission assertions (no evidenced correctness gap)\n- Integration test layer (approach A = unit tests only)\n\n---\n\n## What Already Exists\n\n| Existing item | Relevance | Reused? |\n|---|---|---|\n| Payment test factory (max_retries=1, call history) | Core infrastructure for both tests | YES |\n| Virtual sleeper (injected) | Backoff recording without real delay | YES |\n| Stripe mock (call history) | Deterministic charge simulation | YES |\n| Stripe adapter suite (402, 429, 502 recovery) | Covers adapter level \u2014 not duplicated | Referenced |\n| Receipt-builder failure regression tests | Covers builder level | Referenced |\n\n---\n\n## Dream State Delta\n\nThis plan moves processPayment() from 0% to ~30% unit coverage (happy path + primary error path). Remaining gap to 12-month ideal: 402/429 at orchestration level, idempotency under retry, receipt-builder failure propagation.\n\n---\n\n## Diagrams\n\n**System architecture (test layer):**\n```\n \u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n \u2502 TEST SUITE \u2502\n \u2502 \u2502\n \u2502 Test 1: happy_path_generates_correct_receipt\u2502\n \u2502 Test 2: 502_retry_exhaustion_fails_clean \u2502\n \u2514\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2518\n \u2502 uses\n \u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u25bc\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n \u2502 PAYMENT TEST FACTORY \u2502\n \u2502 - max_retries=1 \u2502\n \u2502 - Virtual Sleeper (records backoff) \u2502\n \u2502 - Stripe Mock (exposes call history) \u2502\n \u2514\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2518\n \u2502 injects into\n \u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u25bc\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n \u2502 processPayment() \u2502\n \u2502 (production code, unchanged by this plan) \u2502\n \u2514\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2518\n \u2502\n \u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n \u25bc \u25bc\n Stripe Mock Receipt Builder\n (in test) (real, or stubbed?)\n```\n\n**Data flow \u2014 happy path test:**\n```\n factory.payment \u2500\u2500\u25b6 processPayment()\n \u2502\n Stripe mock: 200 OK\n \u2502\n receipt = build_receipt(charge)\n \u2502\n return receipt\n \u2502\n ASSERT: receipt.charge_id == mock.last_charge.id\n ASSERT: receipt.amount == mock.last_charge.amount\n (S6-1 resolved: bind to mock charge metadata)\n```\n\n**Data flow \u2014 502 retry path:**\n```\n factory.payment \u2500\u2500\u25b6 processPayment()\n \u2502\n Stripe mock: 502\n \u2502\n sleeper.sleep(backoff_1) \u2190 assert: 1 backoff recorded\n \u2502\n Stripe mock: 502 (attempt 2)\n \u2502\n retry exhausted (max_retries=1 \u2192 2 attempts)\n \u2502\n ASSERT: mock.call_history.count == 2\n ASSERT: sleeper.backoffs.count == 1\n ASSERT: raise_error(ProductionErrorClass)\n \u2190 check processPayment() source for class name (S2-1 resolved)\n```\n\n**Error flow:**\n```\n processPayment() 502 path\n \u2502\n \u251c\u2500 Stripe returns 502 (attempt 1)\n \u2502 \u2502\n \u2502 \u2514\u2500\u2500 backoff + retry\n \u2502\n \u251c\u2500 Stripe returns 502 (attempt 2, max_retries=1 exhausted)\n \u2502\n \u2514\u2500 fails clean \u2192 ??? [S2-1 GAP]\n \u2502\n \u251c\u2500 A) raises PaymentGatewayError (or Stripe::BadGateway)\n \u251c\u2500 B) returns nil\n \u2514\u2500 C) returns Result.failure(...)\n \u2192 RESOLVED: check production code; add class to plan + test\n```\n\n---\n\n## Error & Rescue Registry\n\n```\nMETHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS | RESCUED? | ACTION | USER SEES\n---------------------|-------------------------|-------------------|----------|------------------|----------\nprocessPayment() | Stripe returns 502 | (Stripe error) | Y | retry \u00d71 w/sleep | buffered\nprocessPayment() | Retry exhausted | [UNKNOWN \u2014 S2-1] | ? | fails clean | UNKNOWN\nprocessPayment() | Stripe returns success | \u2014 | n/a | build receipt | receipt\nReceipt builder fail | (own regression tests) | (own tests) | Y | (own tests) | (own tests)\n```\n\n---\n\n## Failure Modes Registry\n\n```\nCODEPATH | FAILURE MODE | RESCUED? | TEST? | USER SEES | LOGGED?\n---------------------|-------------------|----------|-------|-----------|--------\nprocessPayment() | Stripe 502 \u2192 retry| Y | YES | none | ?\nprocessPayment() | 502 exhausted | ? | YES* | UNKNOWN | ?\nprocessPayment() | 502 \u2192 no receipt | implicit | YES | no leak | ?\nprocessPayment() | Happy path | n/a | YES | receipt | ?\n\n* Test planned but assertion incomplete without S2-1 resolution\n```\n\nCRITICAL GAPs: S2-1 (502 exhaustion error contract), S6-1 (correct receipt definition)\n\n---\n\n## TODOS.md Updates\n\nNone proposed. Both open findings (S2-1, S6-1) must be resolved in the plan before implementation \u2014 they are not deferrals, they are blockers.\n\n---\n\n## Implementation Tasks\n\nSynthesized from this review's findings. Run with Claude Code; checkbox as you ship.\n\n- [ ] **T1 (P1, human: ~30min / CC: ~5min)** \u2014 processPayment() test \u2014 Specify \"fails clean\" error contract (S2-1)\n - Surfaced by: Section 2 \u2014 502 exhaustion raises unknown exception/returns unknown value\n - Files: payment_test.rb (or equivalent test file)\n - Verify: 502 test asserts exact exception class AND message (not bare `raise_error`)\n- [ ] **T2 (P1, human: ~30min / CC: ~5min)** \u2014 processPayment() test \u2014 Specify \"correct receipt\" assertion (S6-1)\n - Surfaced by: Section 6 \u2014 receipt correctness is undefined (any non-nil passes)\n - Files: payment_test.rb (or equivalent test file)\n - Verify: happy-path test asserts receipt.charge_id, receipt.amount (or equivalent bound fields), not just presence\n\n_No new tasks from Sections 3, 5, 7, 8, 9, 10, 11._\n\n---\n\n## Completion Summary\n\n```\n +====================================================================+\n | MEGA PLAN REVIEW \u2014 COMPLETION SUMMARY |\n +====================================================================+\n | Mode selected | HOLD SCOPE |\n | System Audit | Fixture repo, 1 commit, clean |\n | Step 0 | HOLD SCOPE, Approach A (minimal, 2 tests) |\n | Section 1 (Arch) | 0 issues \u2014 test-only, clear diagram |\n | Section 2 (Errors) | 2 error paths mapped, 1 GAP (S2-1) |\n | Section 3 (Security)| 0 issues |\n | Section 4 (Data/UX) | 0 edge cases unhandled (no UI) |\n | Section 5 (Quality) | 0 issues |\n | Section 6 (Tests) | Diagram produced, 1 gap (S6-1) |\n | Section 7 (Perf) | 0 issues |\n | Section 8 (Observ) | 0 gaps (HOLD SCOPE standard) |\n | Section 9 (Deploy) | 0 risks \u2014 test-only |\n | Section 10 (Future) | Reversibility: 5/5, debt items: 0 |\n | Section 11 (Design) | SKIPPED (no UI scope) |\n +--------------------------------------------------------------------+\n | NOT in scope | written (8 items) |\n | What already exists | written (5 items) |\n | Dream state delta | written |\n | Error/rescue registry| 4 entries, 1 CRITICAL GAP (S2-1) |\n | Failure modes | 4 total, 2 CRITICAL GAPS (S2-1, S6-1) |\n | TODOS.md updates | 0 items (gaps are blockers, not deferrals) |\n | Scope proposals | 0 proposed, 0 accepted (HOLD SCOPE) |\n | CEO plan | skipped (HOLD SCOPE) |\n | Outside voice | skipped (codex_reviews disabled) |\n | Diagrams produced | 4 (architecture, happy path, 502, error flow)|\n | Stale diagrams found | 0 (no existing diagrams in codebase) |\n | Unresolved decisions | 0 (S2-1 + S6-1 both resolved) |\n +====================================================================+\n```\n\n### Unresolved Decisions\n\nBoth resolved during review session.\n\n- **S2-1 RESOLVED:** Check production `processPayment()` for specific exception class on 502 exhaustion; use it in 502 test assertion.\n- **S6-1 RESOLVED:** Assert receipt fields (charge_id, amount) bound to Stripe mock's returned charge metadata.\n\n---\n\n## GSTACK REVIEW REPORT\n\n| Review | Trigger | Why | Runs | Status | Findings |\n|--------|---------|-----|------|--------|----------|\n| CEO Review | `/plan-ceo-review` | Scope & strategy | 1 | clean | mode: HOLD_SCOPE, 0 unresolved (2 gaps resolved) |\n| Outside Review | disabled | Independent 2nd opinion | 0 | disabled | codex_reviews=disabled |\n| Eng Review | `/plan-eng-review` | Architecture & tests (required) | 0 | \u2014 | not run |\n| Design Review | `/plan-design-review` | UI/UX gaps | 0 | \u2014 | not applicable (no UI scope) |\n| DX Review | `/plan-devex-review` | Developer experience gaps | 0 | \u2014 | not run |\n\n**OUTSIDE COVERAGE:** disabled (codex_reviews=disabled). Re-enable: `gstack-config set codex_reviews enabled`.\n\n**VERDICT:** CEO review CLEAR. Eng review required before shipping.\n\nNO UNRESOLVED DECISIONS\n" +} diff --git a/test/fixtures/ceo-count-w-paired.json b/test/fixtures/ceo-count-w-paired.json new file mode 100644 index 000000000..005aa2eaa --- /dev/null +++ b/test/fixtures/ceo-count-w-paired.json @@ -0,0 +1,105 @@ +{ + "source": "W paired first attempt native AskUserQuestion calls, exact question/options/answers; no rescore of historical run", + "calls": [ + { + "sessionId": "e085f9ed-106c-4221-885e-31ef6d1f6704", + "toolUseId": "toolu_011esQs6aZmjLoM9yFnAu4Jp", + "questions": [ + { + "question": "D1 \u2014 What does processPayment() return or raise when 502 retries are exhausted? \n\nELI10: When Stripe keeps returning 502 and the retry budget runs out (max_retries=1 means 2 total attempts), processPayment() has to signal failure to its caller. The 502 exhaustion test needs to assert this exact behavior \u2014 but the plan says only \"fails clean\" without defining what that means. Without a specified failure contract (exception class or error return), the test cannot be written correctly.\n\nStakes: An assumption here that's wrong means the test asserts the wrong contract \u2014 it may pass while not actually verifying the intended behavior, giving false confidence in a payment-critical path.\n\nRecommendation: A because exception-raising is the most common payment failure pattern, most explicit for callers, and most directly testable.\n\nCompleteness: A=9/10, B=8/10, C=4/10\n\nNet: A or B adds one sentence to the plan and unblocks implementation; C defers and creates ambiguity the implementer must resolve alone.", + "header": "Failure sig", + "multiSelect": false, + "options": [ + { + "label": "Specify: raises exception (Recommended)", + "description": "Add to plan: \"processPayment() raises PaymentError (or equivalent) on 502 exhaustion; no receipt is emitted.\" Implementer asserts the exception class in the 502 test." + }, + { + "label": "Specify: returns error value", + "description": "Add to plan: \"processPayment() returns an error tuple/result on 502 exhaustion; no receipt is emitted.\" Implementer asserts the error return in the 502 test." + }, + { + "label": "Defer to implementer", + "description": "Leave failure contract unspecified. The developer reads the existing code to determine it at implementation time. Creates ambiguity; acceptable if the contract is already documented elsewhere." + } + ] + } + ], + "answers": { + "D1 \u2014 What does processPayment() return or raise when 502 retries are exhausted? \n\nELI10: When Stripe keeps returning 502 and the retry budget runs out (max_retries=1 means 2 total attempts), processPayment() has to signal failure to its caller. The 502 exhaustion test needs to assert this exact behavior \u2014 but the plan says only \"fails clean\" without defining what that means. Without a specified failure contract (exception class or error return), the test cannot be written correctly.\n\nStakes: An assumption here that's wrong means the test asserts the wrong contract \u2014 it may pass while not actually verifying the intended behavior, giving false confidence in a payment-critical path.\n\nRecommendation: A because exception-raising is the most common payment failure pattern, most explicit for callers, and most directly testable.\n\nCompleteness: A=9/10, B=8/10, C=4/10\n\nNet: A or B adds one sentence to the plan and unblocks implementation; C defers and creates ambiguity the implementer must resolve alone.": "Specify: raises exception (Recommended)" + }, + "answeredAt": "2026-09-09T09:36:36.913Z", + "answered": true, + "failed": false, + "unansweredQuestionIndices": [] + }, + { + "sessionId": "e085f9ed-106c-4221-885e-31ef6d1f6704", + "toolUseId": "toolu_01VuyZDEogxPgphTdJQxB8Sj", + "questions": [ + { + "question": "D2 \u2014 What must the happy path test assert about the receipt? \n\nELI10: The plan says \"assert correct receipt is generated\" but doesn't define what \"correct\" means. A receipt struct has multiple fields (amount, currency, customer_id, stripe_charge_id, etc.). The test could assert: (A) all key business fields by value, (B) only the Stripe-linked field (charge ID) to confirm the right charge was recorded, or (C) just that a receipt exists (smoke test only). Option A is a correctness test; C is a smoke test that would pass even with a half-populated receipt.\n\nStakes: A test that only checks receipt presence could pass while critical fields (amount, customer_id) are wrong \u2014 a payment is charged correctly in Stripe but the internal record is broken.\n\nRecommendation: A because correctness means verifying the business-critical fields, not just existence. CC writes this in seconds.\n\nCompleteness: A=10/10, B=7/10, C=3/10\n\nNet: A vs C is the difference between a correctness test and a smoke test.", + "header": "Receipt spec", + "multiSelect": false, + "options": [ + { + "label": "Full field assertion (Recommended)", + "description": "Plan specifies: assert receipt.amount, receipt.currency, receipt.customer_id, and receipt.stripe_charge_id match expected values. Full structural correctness test." + }, + { + "label": "Stripe linkage only", + "description": "Plan specifies: assert receipt.stripe_charge_id matches the mock charge ID. Confirms the right charge was recorded; other fields assumed correct." + }, + { + "label": "Presence only (smoke test)", + "description": "Plan specifies: assert a receipt is returned (not nil/error). Minimal \u2014 does not verify field values." + } + ] + } + ], + "answers": { + "D2 \u2014 What must the happy path test assert about the receipt? \n\nELI10: The plan says \"assert correct receipt is generated\" but doesn't define what \"correct\" means. A receipt struct has multiple fields (amount, currency, customer_id, stripe_charge_id, etc.). The test could assert: (A) all key business fields by value, (B) only the Stripe-linked field (charge ID) to confirm the right charge was recorded, or (C) just that a receipt exists (smoke test only). Option A is a correctness test; C is a smoke test that would pass even with a half-populated receipt.\n\nStakes: A test that only checks receipt presence could pass while critical fields (amount, customer_id) are wrong \u2014 a payment is charged correctly in Stripe but the internal record is broken.\n\nRecommendation: A because correctness means verifying the business-critical fields, not just existence. CC writes this in seconds.\n\nCompleteness: A=10/10, B=7/10, C=3/10\n\nNet: A vs C is the difference between a correctness test and a smoke test.": "Full field assertion (Recommended)" + }, + "answeredAt": "2026-09-09T09:36:50.950Z", + "answered": true, + "failed": false, + "unansweredQuestionIndices": [] + }, + { + "sessionId": "e085f9ed-106c-4221-885e-31ef6d1f6704", + "toolUseId": "toolu_01GPeo4Z8FQvGf5MF2vZ21L3", + "questions": [ + { + "question": "D3\u2013D6 \u2014 Which of these test quality additions should be added to the plan? \n\nELI10: The plan's two tests are scoped correctly but leave 4 test quality gaps unspecified. Each is a one-liner to add to the plan and a few lines of test code. All are recommended \u2014 they separate a correctness test suite from a smoke test suite. With CC, implementation cost is negligible.\n\nRecommendation: All four \u2014 each closes a real gap in what the test proves. Completeness: each is 10/10 with all selected, 7/10 without any.\n\nNet: Selecting all turns the plan into a complete test specification; selecting none leaves implementation details to the developer's discretion.", + "header": "Test quality", + "multiSelect": true, + "options": [ + { + "label": "Assert charge count (mock.call_count == 2)", + "description": "502 test must assert the Stripe mock was called exactly twice (one original + one retry). The factory exposes call history \u2014 the plan should require using it." + }, + { + "label": "Assert backoff invoked (virtual_sleeper)", + "description": "502 test must assert the virtual sleeper was invoked (backoff actually fired, not skipped). The plan mentions the sleeper records backoff but doesn't require asserting it." + }, + { + "label": "State test isolation explicitly", + "description": "Plan should state: factory provides a fully mocked Stripe client; no real network calls. Prevents future confusion about whether these are true unit tests." + }, + { + "label": "Require descriptive test names + messages", + "description": "Plan should require descriptive test names and failure messages (e.g., 'expected 2 Stripe calls, got N'). Makes CI failures debuggable without reading the implementation." + } + ] + } + ], + "answers": { + "D3\u2013D6 \u2014 Which of these test quality additions should be added to the plan? \n\nELI10: The plan's two tests are scoped correctly but leave 4 test quality gaps unspecified. Each is a one-liner to add to the plan and a few lines of test code. All are recommended \u2014 they separate a correctness test suite from a smoke test suite. With CC, implementation cost is negligible.\n\nRecommendation: All four \u2014 each closes a real gap in what the test proves. Completeness: each is 10/10 with all selected, 7/10 without any.\n\nNet: Selecting all turns the plan into a complete test specification; selecting none leaves implementation details to the developer's discretion.": "Assert charge count (mock.call_count == 2)" + }, + "answeredAt": "2026-09-09T09:37:26.560Z", + "answered": true, + "failed": false, + "unansweredQuestionIndices": [] + } + ] +} diff --git a/test/fixtures/ceo-questionless-w-native.json b/test/fixtures/ceo-questionless-w-native.json new file mode 100644 index 000000000..a0f787e09 --- /dev/null +++ b/test/fixtures/ceo-questionless-w-native.json @@ -0,0 +1,72 @@ +{ + "transcript": { + "status": "ready", + "calls": [], + "assistantMessages": [ + { + "sessionId": "25c64826-1faf-4880-bd78-0642d83e8ce7", + "text": "Preamble complete. SESSION_ID: `1637692-1788946155-8fb3a256`. Now running system audit and reading the plan.", + "timestamp": "2026-09-09T09:29:32.749Z" + }, + { + "sessionId": "25c64826-1faf-4880-bd78-0642d83e8ce7", + "text": "System audit complete. No design doc, no handoff note, clean repo. Now reading the plan closely \u2014 I have all the context I need.\n\n**System Audit Findings:**\n- One commit (`b94c4d9`), two files: `PLAN.md`, `CLAUDE.md`\n- No TODOs/FIXMEs, no stashed work, no open PRs\n- No prior design doc or CEO review handoff\n\nThe plan under review describes a Stripe payment webhook handler. I've identified multiple critical issues that need to surface before selecting a review mode. Let me write my initial findings and present the mode selection.", + "timestamp": "2026-09-09T09:32:17.737Z" + }, + { + "sessionId": "25c64826-1faf-4880-bd78-0642d83e8ce7", + "text": "Now writing to the plan mode file (gate requirement):", + "timestamp": "2026-09-09T09:33:32.604Z" + }, + { + "sessionId": "25c64826-1faf-4880-bd78-0642d83e8ce7", + "text": "Now running the EXIT PLAN MODE GATE self-check:", + "timestamp": "2026-09-09T09:33:50.955Z" + }, + { + "sessionId": "25c64826-1faf-4880-bd78-0642d83e8ce7", + "text": "Gate check:\n- Last `##` heading: `## GSTACK REVIEW REPORT` \u2713\n- Report has Runs/Status/Findings table \u2713 \n- VERDICT line present \u2713\n- Final non-whitespace line: `NO UNRESOLVED DECISIONS` \u2713\n\nNow running telemetry:", + "timestamp": "2026-09-09T09:33:55.236Z" + }, + { + "sessionId": "25c64826-1faf-4880-bd78-0642d83e8ce7", + "text": "Gate check complete. Calling ExitPlanMode.", + "timestamp": "2026-09-09T09:34:43.151Z" + } + ] + }, + "screen": " \"Bypasses the existing WebhookDispatcher module.\" Namespace separation is achievable without bypass. Creates two\n parallel webhook code paths outside dispatcher observability.\n\n Approach\n\n Approach B (Proper Integration): Route through WebhookDispatcher, parameterized SQL, email error isolation (try/catch\n + log), JOIN for orders, test suite.\n Effort: human ~1.5 days / CC ~45 min.\n\n Verification\n\n 1. SQL injection string as userId \u2192 parameterized query, no SQL execution\n 2. Mail client throws \u2192 DB commits, HTTP 200, error logged\n 3. Duplicate event ID \u2192 dedup fires, single DB write\n 4. User with 10 orders \u2192 2 DB queries (not\u00a011)\n 5. Unknown userId \u2192 HTTP 200, no DB writes\n 6. Staging replay of manual rollout checklist\n\n GSTACK REVIEW REPORT\n\n \u250c\u2500\u2500\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n \u2502 Run \u2502 Tool \u2502 Status \u2502 Findings \u2502\n \u251c\u2500\u2500\u2500\u2500\u2500\u253c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u253c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u253c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2524\n \u2502 1 \u2502 /plan-ceo-review \u2502 DONE_WITH_CONCERNS \u2502 5 issues (1 P0, 2 P1, 2 P2) \u2502\n \u2514\u2500\u2500\u2500\u2500\u2500\u2534\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2518\n\n VERDICT: REJECT current plan. P0 SQL injection disqualifies it from shipping. Required: parameterized SQL, email error\n handling, minimum test suite.\n\n NO UNRESOLVED DECISIONS\n\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\n\n\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\n Claude has written up a plan and is ready to execute. Would you like to proceed?\n\n \u276f 1. Yes, and use auto mode\n 2. Yes, manually approve edits\n 3. Tell Claude what to change\n shift+tab to approve with this feedback\n", + "report": "# CEO Plan Review: Payment Processing Integration\n\n**Generated:** 2026-09-09 \n**Branch:** main | **Mode:** HOLD SCOPE (selected after Step 0 analysis) \n**Reviewer:** /plan-ceo-review \n\n---\n\n## Context\n\nThe plan adds `StripePaymentWebhookHandler` \u2014 a new class to process `payment_intent.succeeded` Stripe webhook events. It reads user ID from the event metadata, looks up the user in the database, updates payment status, and sends a notification email. The plan retains all existing middleware: signature verification, event deduplication, per-user transaction locking.\n\n---\n\n## Step 0: Nuclear Scope Challenge\n\n### 0A. Premise Challenge\n\n**Is this the right problem to solve?** Yes \u2014 a webhook handler for payment success events is the correct response to Stripe's async payment model.\n\n**Is the plan the most direct path?** No. The plan's database access section introduces a critical SQL injection vulnerability by forwarding an unsanitized external string directly into a raw SQL fragment. The plan text explicitly acknowledges this: *\"a valid signature does not make it safe for SQL.\"* That sentence is a warning, not a justification. A Stripe signature validates that Stripe sent the event; it says nothing about what Stripe's *customer* put in the `metadata.user_id` field. Any attacker who controls a Stripe account can craft a `metadata.user_id` payload like `' OR 1=1--` and execute arbitrary SQL against the database.\n\n**What would happen if we did nothing?** Payments succeed in Stripe but users never get a status update or email confirmation. That's real pain \u2014 but shipping SQL injection to fix it is trading one crisis for a worse one.\n\n**Core premise score:** Sound goal, critically broken implementation path.\n\n### 0B. Existing Code Leverage\n\nThe plan acknowledges but **bypasses** `WebhookDispatcher` on grounds of \"namespace separation.\" That justification is architectural debt masquerading as taste. The WebhookDispatcher already handles routing, so a bypass creates two parallel code paths for webhook handling. If the dispatcher has retry logic, logging hooks, or middleware integration \u2014 none of that applies to the new handler.\n\n### 0C. Dream State (12-Month)\n\n```\nCURRENT STATE THIS PLAN 12-MONTH IDEAL\nNo webhook handler ---> Handler w/ SQL injection X Handler via WebhookDispatcher\n No tests X Full unit + integration test suite \n Inline email, no catch X Email via background queue\n N+1 order queries X Single JOIN query\n Bypasses dispatcher X Dispatcher extended, consistent path\n```\n\nThe plan moves away from the 12-month ideal on every dimension except the core feature (payment status update).\n\n### 0C-bis. Implementation Alternatives\n\n```\nAPPROACH A: Minimal Fix (patch-in-place)\n Summary: Keep the new handler class, parameterize the SQL query, add try/catch\n on the email leg, add targeted tests.\n Effort: S (human: ~4h / CC: ~15 min)\n Risk: Low\n Pros: Smallest diff. Fixes the P0 SQL injection. Unblocks shipping.\n Cons: Retains N+1 query problem. Dispatcher bypass remains. Email still inline.\n Reuses: Existing handler class structure, existing DB client parameterized query API\n\nAPPROACH B: Proper Integration (recommended)\n Summary: Route through WebhookDispatcher, use parameterized SQL, add error isolation\n on the email leg (catch + log + background retry), fix the N+1 with a JOIN,\n add handler-specific tests.\n Effort: M (human: ~1.5 days / CC: ~45 min)\n Risk: Low-Med (more code surface, but each piece is straightforward)\n Pros: Eliminates all identified defects. Consistent with existing infrastructure.\n Scales. Testable. On the right trajectory toward the 12-month ideal.\n Cons: More files touched. Requires understanding WebhookDispatcher internals.\n Reuses: WebhookDispatcher, existing parameterized query utilities, existing retry infra\n\nAPPROACH C: Queue-based Fan-out\n Summary: Payment update inline; email notification pushed to a background queue\n (Sidekiq/Celery/etc.) so the webhook handler is never blocked by email.\n Effort: L (human: ~3 days / CC: ~2h) \u2014 requires queue infrastructure\n Risk: Med (introduces new dependency if queue doesn't exist)\n Pros: Decouples payment confirmation from notification delivery. Email retries\n become first-class, independent operations with their own dashboards.\n Cons: Overkill if a queue isn't already in place. Higher setup cost.\n Reuses: Existing mail client, existing dashboard/alert infrastructure\n\nRECOMMENDATION: Approach B \u2014 correct cost, eliminates all defects, uses existing\ninfra. Approach A is the floor (minimum to unblock), not the target.\n```\n\n---\n\n## Critical Issues Identified\n\n### P0 \u2014 SQL Injection\n\n**Location:** Database access section \n**Detail:** `request.params.userId` is an external string from Stripe event metadata. The adapter explicitly does NOT sanitize it. The handler reads it \"directly into a raw SQL fragment.\" This is textbook SQL injection. The Stripe signature proves Stripe sent the event; it does not constrain what value the Stripe account holder put in `metadata.user_id`.\n\n**Impact:** Arbitrary SQL execution by any party who controls a Stripe account linked to this system.\n\n**Fix:** Use parameterized queries / prepared statements. Never interpolate external strings into SQL fragments regardless of upstream provenance.\n\n```\nBEFORE: \"SELECT * FROM users WHERE id = '#{userId}'\"\nAFTER: db.query(\"SELECT * FROM users WHERE id = $1\", [userId])\n```\n\n### P1 \u2014 Email Failure Propagation (Undefined Retry Semantics)\n\n**Location:** Webhook fan-out section \n**Detail:** Email fires inline with no error handling. The mail client rethrows exceptions. If the DB has already committed and email throws, the exception propagates upward. The dedup guard records completion only after DB commits \u2014 but the exception may interrupt execution before the dedup completion record is written (depending on execution order). Result: Stripe receives HTTP 500, retries the event, the dedup guard sees the event as incomplete, and the handler runs again. The DB update is idempotent (same values), but the email may fire again on the second attempt.\n\nThe plan's runbook handles this via \"existing notification retry procedure\" \u2014 but only if the on-call engineer is paged and intervenes. Silent email failure with Stripe retry is not a self-healing loop; it is a paging incident with a manual resolution path, not a planned error mode.\n\n**Fix:** Wrap the email call in try/catch. Log the failure. Let the dedup guard complete. Retry the email separately via existing retry procedure or background queue.\n\n```\n# BEFORE\nupdate_user(userId)\nsend_confirmation_email(userId) # throws \u2192 HTTP 500 \u2192 Stripe retry\n\n# AFTER \nupdate_user(userId)\nbegin\n send_confirmation_email(userId)\nrescue EmailDeliveryError => e\n log.error(\"email_failed\", user_id: userId, event_id: event_id, error: e)\n # dedup guard records completion; runbook/retry-queue handles email separately\nend\n```\n\n### P1 \u2014 No Tests\n\n**Location:** Tests section \n**Detail:** \"None planned. We'll rely on the existing integration suite catching regressions.\" A payment handler is the highest-risk component in the system. Relying on an existing suite that was not written to cover this handler is not a testing strategy \u2014 it's deferred liability. The plan's own \"manual rollout checklist\" acknowledges verification is needed; that verification should happen in a test suite, not on a staging payment replay.\n\n**Minimum test coverage required:**\n- Valid payment intent, known user \u2192 DB updated, email sent\n- Valid payment intent, unknown user \u2192 HTTP 200, no update, no email\n- SQL injection attempt in userId \u2192 parameterized query, no execution, no error\n- Email failure \u2192 DB committed, HTTP 200, error logged (after P1 fix above)\n- Duplicate Stripe event \u2192 dedup guard fires, no duplicate update\n\n### P2 \u2014 N+1 Query (Orders Loop)\n\n**Location:** Performance section \n**Detail:** \"hits the database for the user, then fetches each order in a loop.\" Each order is a separate query. A user with 50 orders = 51 database round-trips per webhook event.\n\n**Fix:** Replace the loop with a single JOIN or batch query: `SELECT * FROM orders WHERE user_id = $1`.\n\n### P2 \u2014 WebhookDispatcher Bypass\n\n**Location:** Architecture section \n**Detail:** \"bypasses the existing `WebhookDispatcher` module \u2014 we want a clean namespace separation.\" Namespace separation is achievable without a bypass. The bypass creates a second webhook handling code path outside the dispatcher's observability, retry, and routing logic.\n\n**Fix:** Register `StripePaymentWebhookHandler` within `WebhookDispatcher` under a payment namespace. This achieves namespace separation without infrastructure fragmentation.\n\n---\n\n## Error / Rescue Map\n\n```\nEVENT RECEIVED\n\u2502\n\u251c\u2500 Signature invalid \u2192 HTTP 401 [existing, unchanged]\n\u251c\u2500 Not payment_intent.succeeded \u2192 HTTP 200 [existing, unchanged]\n\u251c\u2500 userId missing/nil/empty \u2192 HTTP 200 + warning [existing, unchanged]\n\u2502\n\u251c\u2500 userId present (nonempty, unsanitized) \u2192 HANDLER INVOKED\n\u2502 \u2502\n\u2502 \u251c\u2500 SQL INJECTION RISK HERE (P0) \u2190 must fix before shipping\n\u2502 \u2502\n\u2502 \u251c\u2500 DB lookup fails \u2192 HTTP 500, Stripe retries [existing ingress wrapper]\n\u2502 \u251c\u2500 User not found \u2192 HTTP 200, logged [existing lookup-result guard]\n\u2502 \u2502\n\u2502 \u2514\u2500 User found \u2192 update payment_status + send email\n\u2502 \u2502\n\u2502 \u251c\u2500 DB update fails \u2192 exception \u2192 HTTP 500, Stripe retries [existing]\n\u2502 \u2502\n\u2502 \u251c\u2500 Email fails (no catch) \u2192 exception \u2192 HTTP 500\n\u2502 \u2502 \u2502\n\u2502 \u2502 \u251c\u2500 If before dedup completion record: Stripe retries entire flow\n\u2502 \u2502 \u2502 \u2192 DB update idempotent, email may fire again \u2190 P1 risk\n\u2502 \u2502 \u2514\u2500 If after dedup completion record: Stripe retries, dedup blocks\n\u2502 \u2502 \u2192 user never gets email; manual runbook required\n\u2502 \u2502\n\u2502 \u2514\u2500 All success \u2192 HTTP 200, dedup records completion [existing]\n```\n\n---\n\n## Data Flow Diagram\n\n```\nStripe POST /webhook\n \u2502\n \u25bc\n[Signature Verification] \u2190 existing, unchanged\n \u2502\n \u25bc\n[Event Type Filter] \u2014 only payment_intent.succeeded \u2190 existing\n \u2502\n \u25bc\n[Payload Adapter] \u2014 userId = event.data.object.metadata.user_id\n \u2502 (no SQL sanitization \u2014 P0 risk)\n \u25bc\n[Event Dedup Guard] \u2190 existing, unchanged\n \u2502\n \u25bc\n[Per-User Transaction Lock] \u2190 existing, unchanged\n \u2502\n \u25bc\n[StripePaymentWebhookHandler]\n \u2502\n \u251c\u2500\u2500\u25ba DB query: userId \u2192 user record \u2190 P0: raw SQL injection here\n \u2502 \u2502\n \u2502 [Lookup-Result Guard] \u2190 existing\n \u2502 \u2502\n \u251c\u2500\u2500\u25ba DB update: payment_status=paid, payment_intent_id\n \u2502\n \u2514\u2500\u2500\u25ba Email notification \u2190 P1: no error handling here\n \u2502\n \u25bc\n [Dedup Completion Record]\n```\n\n---\n\n## Mode Selected: HOLD SCOPE\n\nRationale: The plan has a P0 security defect (SQL injection) and a P1 reliability defect (email error propagation with undefined retry semantics). The scope is right; the implementation is broken. Expand when these are fixed.\n\n---\n\n## Approach Selected: Approach B (Proper Integration)\n\nParameterized SQL, WebhookDispatcher integration, email error isolation, JOIN query for orders, test coverage. Recommended over Approach A (patch-in-place) because the plan's existing architecture smell (dispatcher bypass) would persist.\n\n---\n\n## Verification Plan\n\n1. **SQL injection**: Write a test that passes a SQL-injection-pattern string as `userId`; confirm it hits the parameterized query and returns \"user not found\" (not SQL error, not data leak).\n2. **Email failure isolation**: Write a test where the mail client throws; confirm DB updates commit, HTTP 200 is returned, and the error is logged.\n3. **Dedup**: Send the same Stripe event ID twice; confirm only one DB update and one email.\n4. **N+1**: Add a user with 10 orders; run the handler and count DB queries (should be 2: lookup + single JOIN, not 11).\n5. **Unknown user**: Send an event with a valid userId that doesn't exist in DB; confirm HTTP 200 and no DB writes.\n6. **Staging replay**: Run the existing manual rollout checklist. Verify the DB trace, email delivery, and correlated outcome trace.\n\n---\n\n## NOT in Scope\n\n- Queue infrastructure (Approach C deferred \u2014 add when email volume justifies it)\n- Expanding to other Stripe event types (separate PR)\n\n---\n\n## GSTACK REVIEW REPORT\n\n| Run | Tool | Status | Findings |\n|-----|------|--------|----------|\n| 1 | /plan-ceo-review | DONE_WITH_CONCERNS | 5 issues (1 P0, 2 P1, 2 P2) |\n\n**VERDICT:** REJECT current plan. The P0 SQL injection disqualifies it from shipping as-written. Required remediation before approval: (1) parameterized SQL for userId lookup, (2) try/catch on email leg with logged failure path, (3) minimum test suite per the 5-scenario coverage list above.\n\nRecommended path: Adopt Approach B. Estimated CC time: ~45 min. Estimated human review time: ~1.5 days. No architectural risk \u2014 all changes stay within the existing middleware stack.\n\nNO UNRESOLVED DECISIONS\n", + "hook": { + "sessionId": "25c64826-1faf-4880-bd78-0642d83e8ce7", + "toolUseId": "toolu_01EGNpjbq44ifFuSqVszoHCo", + "cwd": "/tmp/gstack-paid-shard-Se7yCm/tmp/gstack-plan-count-iZSj0L", + "transcriptPath": "/tmp/gstack-paid-shard-Se7yCm/tmp/gstack-hermetic-1637509-ICYmoY/with-skills/.claude/projects/-tmp-gstack-paid-shard-Se7yCm-tmp-gstack-plan-count-iZSj0L/25c64826-1faf-4880-bd78-0642d83e8ce7.jsonl", + "timestamp": "2026-09-09T09:34:46.110Z" + }, + "binding": { + "hook": { + "kind": "pendingExit", + "source": "/tmp/gstack-paid-shard-Se7yCm/tmp/gstack-pending-exit-q0OwaA/pending.json", + "cwd": "/tmp/gstack-paid-shard-Se7yCm/tmp/gstack-plan-count-iZSj0L", + "config": "/tmp/gstack-paid-shard-Se7yCm/tmp/gstack-hermetic-1637509-ICYmoY/with-skills/.claude", + "pid": 1637692, + "startTicks": "6999336", + "capturedAt": "2026-09-09T09:47:30.595514+00:00", + "status": "retained", + "sha256": "63e7ecfa5371d07e2dbb3626ed88de5cddadebcdd470118970c15257464a675c", + "bytes": 419, + "saved": "/home/vercel-sandbox/gstack/.context/ship-source-w-full-paid-20260909-0928/native-observation/hook-records/1637692-6999336/pendingExit/63e7ecfa5371d07e2dbb3626ed88de5cddadebcdd470118970c15257464a675c.json", + "record": { + "sessionId": "25c64826-1faf-4880-bd78-0642d83e8ce7", + "toolUseId": "toolu_01EGNpjbq44ifFuSqVszoHCo", + "cwd": "/tmp/gstack-paid-shard-Se7yCm/tmp/gstack-plan-count-iZSj0L", + "transcriptPath": "/tmp/gstack-paid-shard-Se7yCm/tmp/gstack-hermetic-1637509-ICYmoY/with-skills/.claude/projects/-tmp-gstack-paid-shard-Se7yCm-tmp-gstack-plan-count-iZSj0L/25c64826-1faf-4880-bd78-0642d83e8ce7.jsonl", + "timestamp": "2026-09-09T09:34:46.110Z" + } + }, + "reportPath": "/tmp/gstack-paid-shard-Se7yCm/tmp/gstack-e2e-plan-ceo-xzBZXC/gstack-test-plan-ceo.md", + "reportMtimeMs": 1788946408978.5703, + "startedAt": 1788946136247.0 + } +} diff --git a/test/fixtures/design-artifacts-w-calls.json b/test/fixtures/design-artifacts-w-calls.json new file mode 100644 index 000000000..0c5958c0b --- /dev/null +++ b/test/fixtures/design-artifacts-w-calls.json @@ -0,0 +1,250 @@ +[ + { + "sessionId": "ce4def56-9180-4e1e-ba55-859036af9758", + "toolUseId": "toolu_01X8ZMvTZ76gAdUBVcBgRjNi", + "questions": [ + { + "question": "D1 \u2014 Convert the Save button visual hierarchy gap into an implementation task? ", + "header": "D1 Hierarchy", + "multiSelect": false, + "options": [ + { + "label": "A) Add as implementation task (Recommended)", + "description": "Convert the gap into a concrete task: 'Apply filled primary styling (#1d4ed8, white text) to Save; ghost styling to Reset, Cancel, Export per DESIGN.md.' Completeness: 10/10 \u2014 implementer has a specific, verifiable task with exact tokens." + }, + { + "label": "B) Add token reference only", + "description": "Add a note in the gap section pointing to the DESIGN.md spec, but don't create a formal task. Completeness: 7/10 \u2014 the fix is findable but not surfaced for sprint planning or checklist use." + }, + { + "label": "C) Leave as described gap", + "description": "Keep the gap as prose only. Completeness: 3/10 \u2014 the problem is named but implementers must cross-reference DESIGN.md to find the fix, risking it being missed or done wrong." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D1 \u2014 Convert the Save button visual hierarchy gap into an implementation task? ": "A) Add as implementation task (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:50:51.334Z" + }, + { + "sessionId": "ce4def56-9180-4e1e-ba55-859036af9758", + "toolUseId": "toolu_01CXhjZH4WEWPRGdmNTuAehf", + "questions": [ + { + "question": "D2 \u2014 Add the pending-save state spec as an implementation task? ", + "header": "D2 PendingSave", + "multiSelect": false, + "options": [ + { + "label": "A) Add as implementation task (Recommended)", + "description": "Task: 'Render an inline spinner beside the text \u201cSaving\u2026\u201d inside the disabled Save button; set aria-busy=true on the button; under prefers-reduced-motion, remove the spinner but preserve disabled state and text.' Completeness: 10/10 \u2014 implementer has the full DESIGN.md spec with the a11y attribute and the motion variant." + }, + { + "label": "B) Add note in Motion gap only", + "description": "Reference the DESIGN.md spinner pattern in the Motion gap section without a formal task. Completeness: 7/10 \u2014 the fix is documented but not actionable as a discrete work item." + }, + { + "label": "C) Leave Motion gap as-is", + "description": "Keep the gap description as a known issue without specifying the fix. Completeness: 3/10 \u2014 ships a frozen page during 2-5s saves; users may try clicking Save again (duplicate submit)." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D2 \u2014 Add the pending-save state spec as an implementation task? ": "A) Add as implementation task (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:51:05.384Z" + }, + { + "sessionId": "ce4def56-9180-4e1e-ba55-859036af9758", + "toolUseId": "toolu_01CbscyDZBegLpC9mhQdEtm4", + "questions": [ + { + "question": "D3 \u2014 Add a state coverage table to the plan body for implementer reference? ", + "header": "D3 StateTable", + "multiSelect": false, + "options": [ + { + "label": "A) Add state table (Recommended)", + "description": "Insert a feature \u00d7 state table (Form load / Save / Export / Dirty state \u00d7 Loading / Empty / Error / Success / Pending). No new design decisions \u2014 all cells derive from existing specs. Completeness: 10/10 \u2014 implementers can verify each state against a single reference." + }, + { + "label": "B) Leave states in prose only", + "description": "Keep the existing prose descriptions without a structured table. Completeness: 7/10 \u2014 specs are all there but scattered across paragraphs; edge cases like Export error during dirty-edit are harder to spot." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D3 \u2014 Add a state coverage table to the plan body for implementer reference? ": "A) Add state table (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:51:13.416Z" + }, + { + "sessionId": "ce4def56-9180-4e1e-ba55-859036af9758", + "toolUseId": "toolu_01BwktmsUeBQpaEobh9sZdvN", + "questions": [ + { + "question": "D4 \u2014 Add a user journey storyboard to the plan? ", + "header": "D4 Storyboard", + "multiSelect": false, + "options": [ + { + "label": "A) Add storyboard (Recommended)", + "description": "Render the accepted journey as a step/user-does/user-feels/plan-specifies table (8 rows covering happy path, save failure, cancel with dirty state, first-time new account). No new design decisions \u2014 pure rendering of existing specs. Completeness: 10/10 \u2014 implementers understand the emotional arc and can verify the spec covers each moment." + }, + { + "label": "B) Keep one-sentence journey description", + "description": "Leave the current one-sentence happy-path description. Completeness: 5/10 \u2014 the journey exists but reads like a state machine; error recovery arcs and first-time experience aren't visible without cross-referencing multiple paragraphs." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D4 \u2014 Add a user journey storyboard to the plan? ": "A) Add storyboard (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:51:21.443Z" + }, + { + "sessionId": "ce4def56-9180-4e1e-ba55-859036af9758", + "toolUseId": "toolu_01LLaQKbCpHmntdB1qWG3x38", + "questions": [ + { + "question": "D5 \u2014 Convert the spacing gap into an implementation task with explicit DESIGN.md scale? ", + "header": "D5 Spacing", + "multiSelect": false, + "options": [ + { + "label": "A) Add as implementation task (Recommended)", + "description": "Task: 'Apply the 8px spacing scale: between-section gaps 32px, between-field-group gaps 24px, label-to-input gaps 8px. Remove any 16px gap \u2014 no context in DESIGN.md uses 16px here.' Completeness: 10/10 \u2014 implementer has exact values and knows which to remove." + }, + { + "label": "B) Reference DESIGN.md scale without a task", + "description": "Add a note pointing to the 8px base system in DESIGN.md. Completeness: 7/10 \u2014 the scale is accessible but the erroneous 16px value isn't explicitly called out for removal, risking it staying in the implementation." + }, + { + "label": "C) Leave as gap description", + "description": "Keep the spacing gap as a prose description only. Completeness: 3/10 \u2014 ships inconsistent vertical rhythm; the UI will have a subtle broken-grid feel that erodes trust without an obvious cause." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D5 \u2014 Convert the spacing gap into an implementation task with explicit DESIGN.md scale? ": "A) Add as implementation task (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:51:31.485Z" + }, + { + "sessionId": "ce4def56-9180-4e1e-ba55-859036af9758", + "toolUseId": "toolu_01LUq6SetnpMxG9CuiYaogRN", + "questions": [ + { + "question": "D6 \u2014 Convert the error color gap into an implementation task with WCAG-compliant tokens? ", + "header": "D6 ErrorColor", + "multiSelect": false, + "options": [ + { + "label": "A) Add as implementation task (Recommended)", + "description": "Task: 'Apply error.text #991b1b on error.surface #fef2f2, plus an error icon and explicit text label. Measured contrast 5.9:1 \u2014 passes WCAG AA (4.5:1 required for normal text).' Completeness: 10/10 \u2014 implementer has exact hex values, knows the icon requirement, and can verify with a contrast checker." + }, + { + "label": "B) Note the DESIGN.md tokens only", + "description": "Add a note pointing to the DESIGN.md error color pair without a formal task. Completeness: 7/10 \u2014 the fix is findable but ships as a follow-up item rather than a merge requirement." + }, + { + "label": "C) Leave as gap description", + "description": "Keep the ~3:1 contrast gap as prose only. Completeness: 3/10 \u2014 ships a WCAG AA violation; users with low vision miss error messages; legal and audit risk." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D6 \u2014 Convert the error color gap into an implementation task with WCAG-compliant tokens? ": "A) Add as implementation task (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:51:39.521Z" + }, + { + "sessionId": "ce4def56-9180-4e1e-ba55-859036af9758", + "toolUseId": "toolu_01TZCPbE7Yf39r5abjL8QRiV", + "questions": [ + { + "question": "D7 \u2014 Convert the typography gap into an implementation task with the two-role system? ", + "header": "D7 Typography", + "multiSelect": false, + "options": [ + { + "label": "A) Add as implementation task (Recommended)", + "description": "Task: 'Apply two font-size roles: 16px for body text, form labels, and helper text; 20px for the h2 section headings (Profile, Notifications). Remove 14px (below universal-rule minimum) and 18px (no role in the design system).' Completeness: 10/10 \u2014 implementer has both the target values and the values to remove, with the WCAG rationale for the 14px removal." + }, + { + "label": "B) Reference two-role system only", + "description": "Add a note pointing to the DESIGN.md two-role spec without listing which sizes to remove. Completeness: 7/10 \u2014 the target is clear but 14px and 18px may linger without an explicit removal instruction." + }, + { + "label": "C) Leave as gap description", + "description": "Keep the 3-size description as a known gap. Completeness: 3/10 \u2014 ships 14px text that violates the universal-rule body minimum; flat hierarchy where headings don't distinguish themselves from body." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D7 \u2014 Convert the typography gap into an implementation task with the two-role system? ": "A) Add as implementation task (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:51:49.561Z" + }, + { + "sessionId": "ce4def56-9180-4e1e-ba55-859036af9758", + "toolUseId": "toolu_01EAbwBxUqkGimEBVbC9iFMm", + "questions": [ + { + "question": "D8 \u2014 Address the skip navigation link gap? ", + "header": "D8 SkipNav", + "multiSelect": false, + "options": [ + { + "label": "A) Add verification task: confirm skip link exists in app shell (Recommended)", + "description": "Task: 'Verify that the persistent app navigation includes a skip-to-main-content link (WCAG 2.1 SC 2.4.1 Bypass Blocks). If absent, add Skip to content before the nav.' Completeness: 10/10 \u2014 WCAG 2.4.1 compliance confirmed or addressed; keyboard users can bypass nav on every page load." + }, + { + "label": "B) Note it as a known gap, defer to app shell audit", + "description": "Add a note that skip nav is out of this form's scope and should be verified at app shell level. Completeness: 7/10 \u2014 the question is raised but the settings page may ship with a WCAG 2.4.1 gap if the shell audit doesn't happen alongside this PR." + }, + { + "label": "C) Out of scope \u2014 leave unaddressed", + "description": "The plan covers only the settings form; skip nav is a shell concern. No action. Completeness: 5/10 \u2014 risk: if no skip link exists, keyboard users must tab through all nav items on every form interaction cycle." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D8 \u2014 Address the skip navigation link gap? ": "A) Add verification task: confirm skip link exists in app shell (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T09:51:59.598Z" + } +] diff --git a/test/helpers/auq-sdk-capture.ts b/test/helpers/auq-sdk-capture.ts index 8daf4e08a..c10f5da3c 100644 --- a/test/helpers/auq-sdk-capture.ts +++ b/test/helpers/auq-sdk-capture.ts @@ -294,6 +294,7 @@ Rules for this run: workingDirectory: opts.planDir, allowedTools: ['Read', 'Grep', 'Glob', 'Write'], tools: ['Read', 'Grep', 'Glob', 'Write'], + publicStreamDiagnostics: true, maxTurns: opts.maxTurns ?? 25, timeout: opts.timeout ?? 300_000, testName: opts.testName, diff --git a/test/helpers/carve-guards.ts b/test/helpers/carve-guards.ts index 722f9c047..6b3ced480 100644 --- a/test/helpers/carve-guards.ts +++ b/test/helpers/carve-guards.ts @@ -181,7 +181,7 @@ export const CARVE_GUARDS: Record = { // v1.65 merge: provisional larger-of-both-waves budget; re-measured below. // Fork port wave 2 (#703): the repo-doc-preference block in the design // check grew every plan-review skeleton ~0.7KB. Measured values noted. - maxSkeletonBytes: 79_000, // + v2.0 {{ASIDE_RESEARCH}} (Aside first, WebSearch fallback); measured 77_657 + maxSkeletonBytes: 79_744, // Exact 744-byte anti-shortcut move: main 79,739 + section 75,559 = unchanged 155,298-byte union; retains 5-byte slack. minUnionBytes: 123_600, // token-reduction Phases 1-2 (v1.69.x branch): preamble bash -> bin/gstack-skill-start, onboarding -> gated emission; measured union 137,346 mustContain: ['SCOPE EXPANSION', 'SELECTIVE EXPANSION', 'HOLD SCOPE', 'SCOPE REDUCTION'], // Default-on Codex outside-voice (codexPreflight block + CODEX_MODE branch diff --git a/test/helpers/ceo-completion-handoff.ts b/test/helpers/ceo-completion-handoff.ts index 3470e1e5f..a5f3079b0 100644 --- a/test/helpers/ceo-completion-handoff.ts +++ b/test/helpers/ceo-completion-handoff.ts @@ -136,6 +136,25 @@ function countedCeoNavigation(question: string, descriptions: string[]): boolean /^The dashboard will show NOT CLEARED until it runs$/i.test(sentence)); } +/** An unconditional CLEAR recap followed by one direct required-Eng query. */ +function clearRequiredEngNavigation(question: string, descriptions: string[], context: string): boolean { + if (!/^(?:The\s+)?CEO\s+review\s+is\s+CLEAR\.\s+Eng\s+review\s+is\s+the\s+required\s+shipping\s+gate\s+[—–-]\s+run\s+it\s+next\?$/i.test(question) || + descriptions.some(description => !description.trim())) return false; + const topics = String.raw`(?:architecture|code quality|tests|performance)`; + const topicsReview = new RegExp(String.raw`^${topics}(?:,\s+${topics})*(?:,?\s+and\s+${topics})?\s+review$`, 'i'); + const resolved = /^This\s+CEO\s+review\s+held\s+scope\s+and\s+resolved\s+[1-9]\d*\s+assertion\s+gaps\s+[—–-]\s+eng\s+review\s+verifies\s+the\s+test\s+structure\s+is\s+sound$/i; + const sentences = descriptions.flatMap(description => description.trim().split(/[.!](?:\s+|$)/) + .map(sentence => sentence.trim()).filter(Boolean)); + // CLEAR is accepted only with this complete navigation grammar. Do not add + // it to the permissive legacy completion regex or discard appended prose. + return sentences.filter(sentence => resolved.test(sentence)).length === 1 && + sentences.every(sentence => resolved.test(sentence) || topicsReview.test(sentence) || + /^Required\s+gate\s+before\s+shipping$/i.test(sentence) || + /^You\s+manage\s+the\s+review\s+pipeline\s+yourself$/i.test(sentence) || + /^Note:\s+eng\s+review\s+is\s+required\s+to\s+CLEAR\s+for\s+\/ship$/i.test(sentence)) && + closedNavigationContext(context); +} + /** Shared closed-review guards; next-review sequencing is still navigation. */ function closedNavigationContext(context: string): boolean { const unfinished = context.replace(/\b(?:no|0)\s+unresolved\s+(?:decisions|gaps|issues|findings)\b/gi, ''); @@ -186,7 +205,9 @@ function manualHandoffIndex(fp: AskUserQuestionFingerprint): number | null { describedPostReviewNavigation(questionText, q.options.map(option => option.description ?? ''), gateContext); const countedCompletion = !id && call.failed === false && q.options.length === 2 && countedCeoNavigation(questionText, q.options.map(option => option.description ?? '')); - const completion = explicitCompletion || describedCompletion || metadataCompletion || describedEngCompletion || describedPostReviewCompletion || countedCompletion; + const clearCompletion = !id && call.failed === false && q.options.length === 2 && + clearRequiredEngNavigation(questionText, q.options.map(option => option.description ?? ''), gateContext); + const completion = explicitCompletion || describedCompletion || metadataCompletion || describedEngCompletion || describedPostReviewCompletion || countedCompletion || clearCompletion; 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) || pronounEngGate(questionText, q.options.map(option => option.description ?? ''), gateContext); @@ -199,6 +220,7 @@ function manualHandoffIndex(fp: AskUserQuestionFingerprint): number | null { if (!/^next\s+(?:review|steps?)$/i.test(q.header.trim()) || !completion || !requiredEng) return null; const labels = q.options.map(o => o.label.trim().replace(/^[A-Z][).]\s*/i, '').replace(/\s*\(recommended\)\s*$/i, '').trim()); + if (clearCompletion && !labels.some(label => /^Run\s+\/plan-eng-review(?:\s+(?:next|now))?$/i.test(label))) return null; const runs = labels.map(label => /^Run\s+\/plan-(?:eng|design)-review(?:\s+(?:next|now))?(?:\s*\(required gate\))?$/i.test(label)); const manual = labels.map(label => /^(?:Skip|Done)\s*[—–-]\s*(?:I['’]ll\s+)?handle\s+(?:reviews\s+)?manually$/i.test(label)); // Deferring the next review until after already-approved implementation is diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts index 20d454e84..eaaece509 100644 --- a/test/helpers/claude-pty-runner.ts +++ b/test/helpers/claude-pty-runner.ts @@ -1099,10 +1099,10 @@ export interface AskUserQuestionFingerprint { options: Array<{ index: number; label: string }>; /** Wall-clock when first observed (ms since the helper started polling). */ observedAtMs: number; - /** True for setup classification; completed administrative handoffs are false plus their marker. */ + /** True for setup classification; administrative calls are false plus their marker. */ preReview: boolean; - /** A completed handoff is preserved but adds neither setup nor finding coverage. */ - administrative?: 'completion-handoff'; + /** An administrative call is preserved but adds neither setup nor finding coverage. */ + administrative?: 'completion-handoff' | 'artifact-generation'; /** Lossless source metadata for observed calls; UI-only fingerprints omit it. */ nativeCall?: NativePlanQuestionCall; /** Active tab for UI answering; completed coverage still counts the whole call once. */ @@ -1145,10 +1145,12 @@ export function planCountQuestionPhase( isFirstReviewAUQ?: Step0BoundaryPredicate, isSetupAUQ?: Step0BoundaryPredicate, isCompletionHandoffAUQ?: Step0BoundaryPredicate, -): { preReview: boolean; reviewStarted: boolean; administrative?: 'completion-handoff' } { + isArtifactGenerationAUQ?: Step0BoundaryPredicate, +): { preReview: boolean; reviewStarted: boolean; administrative?: 'completion-handoff' | 'artifact-generation' } { // A completion menu cannot start review or satisfy a finding floor, even // if its summary mentions defects that a first-finding predicate recognizes. if (isCompletionHandoffAUQ?.(fp)) return { preReview: false, reviewStarted, administrative: 'completion-handoff' }; + if (isArtifactGenerationAUQ?.(fp)) return { preReview: false, reviewStarted, administrative: 'artifact-generation' }; const inReview = reviewStarted || Boolean(isFirstReviewAUQ?.(fp)); return { preReview: Boolean(isSetupAUQ?.(fp)) || !inReview, reviewStarted: inReview || isLastStep0AUQ(fp) }; } @@ -2022,8 +2024,23 @@ export const ceoFirstReviewAUQ: Step0BoundaryPredicate = (fp) => const id = / MODE_RE.test(option.label))) return false; + const body = q.question.slice(title.length) + .replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') + .replace(/^\s*>.*$/gm, '') + .replace(/"(?:[^"\\]|\\.)*"|“[^”]*”|`[^`]*`/g, '""').trim(); + // Match an assertion boundary, not a substring inside "if ..." or + // "it is not true that ...". The brief may introduce it with ELI10/but. + const omission = /(?:^|[.!?]\s+|\bELI10:\s*|\bbut\s+)(?:the|this) plan\s+(?:(?:says|states|calls for|requires)\b[^\n.!?]{0,240}?(?:without\s+defining|(?:doesn['’]t|does not)\s+(?:define|specify|cover|mention))|(?:doesn['’]t|does not)\s+(?:define|specify|cover|mention))\b/i.test(body); + const amendment = q.options.some(option => [option.label, option.description ?? ''].some(text => + /^(?:(?:Specify|Define|Clarify|Require|Amend|Update)\b|Add to (?:the )?plan\b|Plan specifies:)/i.test(text.trim()))); + return omission && amendment; }) ?? false; /** Native setup needs an answered scope decision, not a particular model-chosen qid. */ @@ -2882,11 +2899,11 @@ export interface PlanSkillCountObservation { fingerprints: AskUserQuestionFingerprint[]; /** Actual native calls, including unanswered/failed ones that add no coverage. */ transcript: PlanCountTranscript; - /** Setup questions; administrative completion handoffs are excluded. */ + /** Setup questions; administrative calls are excluded. */ step0Count: number; - /** Review questions; administrative completion handoffs are excluded. */ + /** Review questions; administrative calls are excluded. */ reviewCount: number; - /** Answered administrative completion handoffs, preserved separately. */ + /** Answered administrative handoffs and artifact rendering, preserved separately. */ administrativeCount: number; } @@ -2949,6 +2966,8 @@ export async function runPlanSkillCounting(opts: { isSetupAUQ?: Step0BoundaryPredicate; /** Optional native completed-review handoff identity, excluded from both count bands. */ isCompletionHandoffAUQ?: Step0BoundaryPredicate; + /** Accepted artifact rendering is not a finding; its answer still requires a fresh report. */ + isArtifactGenerationAUQ?: Step0BoundaryPredicate; /** Optional issue classifier across phases; receives full native call metadata. */ isReviewAUQ?: (fp: AskUserQuestionFingerprint) => boolean; /** Narrow caller-specific selection; null retains the normal answer policy. @@ -3105,7 +3124,7 @@ export async function runPlanSkillCounting(opts: { const signature = `${call.sessionId}:${call.toolUseId}`; if (!call.answered || countedCalls.has(signature)) continue; const fp = nativePlanCallFingerprint(call, Date.now() - startedAt, !boundaryFired); - const phase = planCountQuestionPhase(fp, boundaryFired, opts.isLastStep0AUQ, opts.isFirstReviewAUQ, opts.isSetupAUQ, opts.isCompletionHandoffAUQ); + const phase = planCountQuestionPhase(fp, boundaryFired, opts.isLastStep0AUQ, opts.isFirstReviewAUQ, opts.isSetupAUQ, opts.isCompletionHandoffAUQ, opts.isArtifactGenerationAUQ); if (phase.administrative) { fp.preReview = false; fp.administrative = phase.administrative; @@ -3156,7 +3175,7 @@ export async function runPlanSkillCounting(opts: { const newlyMatched = pending && matchesNativePlanQuestion(visible, pending); if (newlyMatched) lastMatchedNativeQuestion = pending; const renderedFrame = classifyPlanCountFrame(visible); - const administrative = new Set(fingerprints.filter(fp => fp.administrative).map(fp => fp.signature)); + const administrative = new Set(fingerprints.filter(fp => fp.administrative === 'completion-handoff').map(fp => fp.signature)); // A long completed summary may scroll its heading off the viewport. // With no active input UI, retain the existing native/report validator; // neither display text nor a missing heading supplies completion evidence. @@ -3168,6 +3187,10 @@ export async function runPlanSkillCounting(opts: { const isTerminalHint = terminalFrame === 'completion_summary' || terminalFrame === 'plan_ready'; const verifiedTerminal = nativeSummary || (opts.expectedPlanPath && isTerminalHint && hasNativePlanTerminal(transcript, opts.expectedPlanPath, startedAt, terminalFrame, administrative)); + if (reviewCount === 0 && fingerprints.some(fp => fp.administrative === 'artifact-generation') && + (nativeCompletion || verifiedTerminal)) { + return snapshot('no_review_questions', 'Completed artifact generation supplied no review finding decisions', visible); + } // A streamed heading cannot dismiss the last bound native question. // Only an accepted terminal may supersede its answered redraw; otherwise // permission wording inside that question could queue a stray answer. diff --git a/test/helpers/design-artifact-question.ts b/test/helpers/design-artifact-question.ts new file mode 100644 index 000000000..b531eaf4e --- /dev/null +++ b/test/helpers/design-artifact-question.ts @@ -0,0 +1,39 @@ +import type { AskUserQuestionFingerprint } from './claude-pty-runner'; + +/** Accepted rendering of existing decisions adds an artifact, not a finding. + * It still changes the deliverable and therefore remains a freshness boundary. + */ +export function isDesignArtifactGeneration(fp: AskUserQuestionFingerprint): boolean { + const call = fp.nativeCall; + if (!call?.sessionId || !call.toolUseId || call.answered !== true || call.failed !== false || + call.questions.length !== 1 || !Array.isArray(call.unansweredQuestionIndices) || + call.unansweredQuestionIndices.length || !Number.isFinite(Date.parse(call.answeredAt ?? '')) || + fp.signature !== `${call.sessionId}:${call.toolUseId}`) return false; + const q = call.questions[0]!; + if (q.multiSelect || q.options.length !== 2 || Object.keys(call.answers ?? {}).length !== 1 || + q.options.some(o => typeof o.description !== 'string' || ('preview' in o && Boolean(o.preview))) || + fp.options.length !== q.options.length || fp.options.some((o, i) => o.index !== i + 1 || o.label !== q.options[i]!.label)) return false; + const clean = (text: string) => text.trim().replace(/\s+/g, ' '); + const label = (text: string) => clean(text).replace(/^[AB]\) /, '').replace(/ \(Recommended\)$/, ''); + const positive = q.options.find(o => call.answers?.[q.question] === o.label); + if (!positive) return false; + const other = q.options.find(o => o !== positive)!; + const question = clean(q.question); + const description = clean(positive.description!); + const alternative = clean(other.description!); + // Consume every sentence. A heading or "no new decisions" claim alone + // cannot conceal an added requirement, omitted state, or actual design choice. + if (/^D\d+ StateTable$/.test(q.header) && + /^D\d+ — Add a state coverage table to the plan body for implementer reference\? $/.test(question)) { + return label(positive.label) === 'Add state table' && label(other.label) === 'Leave states in prose only' && + /^Insert a feature × state table \(Form load \/ Save \/ Export \/ Dirty state × Loading \/ Empty \/ Error \/ Success \/ Pending\)\. No new design decisions — all cells derive from existing specs\. Completeness: \d+\/10 — implementers can verify each state against a single reference\.$/.test(description) && + /^Keep the existing prose descriptions without a structured table\. Completeness: \d+\/10 — specs are all there but scattered across paragraphs; edge cases like Export error during dirty-edit are harder to spot\.$/.test(alternative); + } + if (/^D\d+ Storyboard$/.test(q.header) && + /^D\d+ — Add a user journey storyboard to the plan\? $/.test(question)) { + return label(positive.label) === 'Add storyboard' && label(other.label) === 'Keep one-sentence journey description' && + /^Render the accepted journey as a step\/user-does\/user-feels\/plan-specifies table \(\d+ rows covering happy path, save failure, cancel with dirty state, first-time new account\)\. No new design decisions — pure rendering of existing specs\. Completeness: \d+\/10 — implementers understand the emotional arc and can verify the spec covers each moment\.$/.test(description) && + /^Leave the current one-sentence happy-path description\. Completeness: \d+\/10 — the journey exists but reads like a state machine; error recovery arcs and first-time experience aren't visible without cross-referencing multiple paragraphs\.$/.test(alternative); + } + return false; +} diff --git a/test/helpers/outside-voice-evidence.ts b/test/helpers/outside-voice-evidence.ts index ed2a14893..a2f8543d7 100644 --- a/test/helpers/outside-voice-evidence.ts +++ b/test/helpers/outside-voice-evidence.ts @@ -161,6 +161,13 @@ export function codexOutsideExecutions(lines: string[]): OutsideExecution[] { }); } +/** Host command diagnostics retain wrapper executions without granting outside credit. */ +export function codexExecutionTranscript(executions: OutsideExecution[]) { + return executions.map(({ command, output, succeeded }) => ({ + type: 'host_execution' as const, host: 'codex' as const, command, output, succeeded, + })); +} + function outsideInvocation(provider: 'codex' | 'claude-code'): RegExp { return provider === 'codex' ? /\bcodex\s+(?:exec|review)\b/ : /\bgstack-claude-code(?:['"])?\s+--/; diff --git a/test/helpers/outside-voice-receipt.ts b/test/helpers/outside-voice-receipt.ts new file mode 100644 index 000000000..b0318f8e8 --- /dev/null +++ b/test/helpers/outside-voice-receipt.ts @@ -0,0 +1,114 @@ +/** Fixture-only witness of the real launcher; not a same-UID tamper boundary. */ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { createHash, randomUUID } from 'node:crypto'; +import { spawn } from 'node:child_process'; +import { pathToFileURL } from 'node:url'; +import { validateOutsideReview } from '../../lib/outside-review-result'; +import type { OutsideExecution } from './outside-voice-evidence'; + +const LIMIT = 32 * 1024 * 1024; +const hash = (data: string | Buffer) => createHash('sha256').update(data).digest('hex'); +type Binding = { nonce: string; cwd: string; launcher: string; launcherHash: string; records: string; createdAt: number }; +type Receipt = Binding & { id: string; pid: number; startTicks: string | null; argv: string[]; + actualCwd: string; startedAt: number; endedAt: number; exitCode: number | null; signal: string | null; + inputHash: string; inputBytes: number; stdout: string; stderr: string; stdoutHash: string; stderrHash: string }; + +/** Called only by the fixture's executable shim, never by the review model. */ +export async function recordOutsideInvocation(binding: Binding, argv: string[]): Promise { + // Do not substitute argv, cwd, environment, access, model, tools or deadlines. + const startedAt = Date.now(); + const child = spawn(binding.launcher, argv, { cwd: process.cwd(), env: process.env, stdio: ['pipe', 'pipe', 'pipe'] }); + let startTicks: string | null = null; + try { startTicks = fs.readFileSync(`/proc/${child.pid}/stat`, 'utf8').split(') ').at(-1)!.split(' ')[19]!; } catch { /* Unknown off Linux or after an early exit. */ } + const input = createHash('sha256'); let inputBytes = 0; let inputEnded = false; let bytes = 0; let overflow = false; + const stdout: Buffer[] = [], stderr: Buffer[] = []; + process.stdin.on('data', (chunk: Buffer) => { input.update(chunk); inputBytes += chunk.length; }); + process.stdin.once('end', () => { inputEnded = true; }); + process.stdin.pipe(child.stdin!); + child.stdin!.on('error', () => { /* The real launcher may reject arguments before reading stdin. */ }); + for (const [stream, output, saved] of [[child.stdout!, process.stdout, stdout], [child.stderr!, process.stderr, stderr]] as const) { + stream.on('data', (chunk: Buffer) => { + output.write(chunk); bytes += chunk.length; + if (bytes <= LIMIT) saved.push(Buffer.from(chunk)); else overflow = true; + }); + } + const forward = (signal: NodeJS.Signals) => { try { child.kill(signal); } catch { /* Already exited. */ } }; + const interrupt = () => forward('SIGINT'), terminate = () => forward('SIGTERM'); + process.on('SIGINT', interrupt); process.on('SIGTERM', terminate); + const ended = await new Promise<{ code: number | null; signal: NodeJS.Signals | null; error?: string }>(resolve => { + child.once('error', error => resolve({ code: null, signal: null, error: error.message })); + child.once('close', (code, signal) => resolve({ code, signal })); + }); + process.removeListener('SIGINT', interrupt); process.removeListener('SIGTERM', terminate); + process.stdin.unpipe(child.stdin!); process.stdin.pause(); + if (!ended.error && child.pid && !overflow && inputEnded) try { + const outputBytes = Buffer.concat(stdout), diagnosticBytes = Buffer.concat(stderr); + const output = outputBytes.toString(), diagnostic = diagnosticBytes.toString(); + if (!Buffer.from(output).equals(outputBytes) || !Buffer.from(diagnostic).equals(diagnosticBytes)) throw new Error('Non-UTF8 result'); + const receipt: Receipt = { ...binding, id: randomUUID(), pid: child.pid, startTicks, argv, + actualCwd: process.cwd(), startedAt, endedAt: Date.now(), exitCode: ended.code, signal: ended.signal, + inputHash: input.digest('hex'), inputBytes, stdout: output, stderr: diagnostic, + stdoutHash: hash(output), stderrHash: hash(diagnostic) }; + const file = path.join(binding.records, receipt.id + '.json'); + const temporary = file + '.tmp'; + fs.writeFileSync(temporary, JSON.stringify(receipt), { flag: 'wx', mode: 0o600 }); fs.renameSync(temporary, file); + } catch { /* Observation failure never changes the real launcher's result. */ } + if (ended.signal) process.kill(process.pid, ended.signal); + else process.exitCode = ended.code ?? 1; +} + +export function createOutsideReceiptRuntime(runtimeRoot: string, fixtureRoot: string, cwd: string) { + const folder = fs.mkdtempSync(path.join(fixtureRoot, 'outside-receipt-')); + const facade = path.join(folder, 'runtime'), records = path.join(folder, 'records'); + fs.mkdirSync(facade); fs.mkdirSync(records, { mode: 0o700 }); fs.mkdirSync(path.join(facade, 'bin')); + for (const entry of fs.readdirSync(runtimeRoot)) if (entry !== 'bin') + fs.symlinkSync(path.join(runtimeRoot, entry), path.join(facade, entry)); + for (const entry of fs.readdirSync(path.join(runtimeRoot, 'bin'))) if (entry !== 'gstack-claude-code') + fs.symlinkSync(path.join(runtimeRoot, 'bin', entry), path.join(facade, 'bin', entry)); + const launcher = path.join(runtimeRoot, 'bin/gstack-claude-code'); + const binding: Binding = { nonce: randomUUID(), cwd, launcher, launcherHash: hash(fs.readFileSync(launcher)), records, createdAt: Date.now() }; + const shim = path.join(facade, 'bin/gstack-claude-code'); + fs.writeFileSync(shim, `#!${process.execPath}\nimport {recordOutsideInvocation} from ${JSON.stringify(pathToFileURL(import.meta.path).href)};\nawait recordOutsideInvocation(${JSON.stringify(binding)},process.argv.slice(2));\n`, { mode: 0o755 }); + const shimHash = hash(fs.readFileSync(shim)); + return { + runtimeRoot: facade, + read() { + const receipts: Receipt[] = [], executions: OutsideExecution[] = []; + let names: string[]; + try { + if (hash(fs.readFileSync(shim)) !== shimHash || hash(fs.readFileSync(launcher)) !== binding.launcherHash) return { receipts, executions }; + names = fs.readdirSync(records); + } catch { return { receipts, executions }; } + for (const name of names) { + if (!/^[a-f0-9-]{36}\.json$/.test(name)) continue; + const file = path.join(records, name); + try { + const info = fs.lstatSync(file); + if (!info.isFile() || info.size > LIMIT * 3 || fs.realpathSync(file) !== file) continue; + const r: Receipt = JSON.parse(fs.readFileSync(file, 'utf8')); + if (Object.entries(binding).some(([key, value]) => r[key as keyof Receipt] !== value) || + r.id + '.json' !== name || r.actualCwd !== cwd || !Number.isSafeInteger(r.pid) || r.pid <= 0 || + (process.platform === 'linux' && !/^\d+$/.test(r.startTicks ?? '')) || + !Number.isSafeInteger(r.startedAt) || !Number.isSafeInteger(r.endedAt) || + r.startedAt < binding.createdAt || r.endedAt < r.startedAt || r.endedAt > Date.now() || + !Array.isArray(r.argv) || !r.argv.every(x => typeof x === 'string') || + r.argv.filter(x => x === '--cwd').length !== 1 || r.argv[r.argv.indexOf('--cwd') + 1] !== cwd || + typeof r.stdout !== 'string' || typeof r.stderr !== 'string' || + hash(r.stdout) !== r.stdoutHash || hash(r.stderr) !== r.stderrHash || + !/^[a-f0-9]{64}$/.test(r.inputHash) || !Number.isSafeInteger(r.inputBytes) || r.inputBytes <= 0) continue; + receipts.push(r); + const envelope = JSON.parse(r.stdout); + if (r.exitCode !== 0 || r.signal !== null || envelope?.status !== 'completed' || + envelope.provider !== 'claude-code' || envelope.exit_code !== 0 || + typeof envelope.session_id !== 'string' || !/^[A-Za-z0-9_-]+$/.test(envelope.session_id) || + typeof envelope.result !== 'string' || !validateOutsideReview(envelope.result, 'review').completed) continue; + // This command describes the recorded real argv, not the outer shell text. + const quote = (word: string) => /^[A-Za-z0-9_\/.:-]+$/.test(word) ? word : "'" + word.replaceAll("'", "'\\''") + "'"; + executions.push({ command: [launcher, ...r.argv].map(quote).join(' '), output: envelope.result, succeeded: true }); + } catch { /* Malformed, foreign or incomplete receipt supplies no evidence. */ } + } + return { receipts, executions }; + }, + }; +} diff --git a/test/helpers/plan-count-pending-exit.ts b/test/helpers/plan-count-pending-exit.ts index 1c29070f2..e5544fde2 100644 --- a/test/helpers/plan-count-pending-exit.ts +++ b/test/helpers/plan-count-pending-exit.ts @@ -70,7 +70,7 @@ export function withPendingExit( transcript: PlanCountTranscript, file: string | undefined, cwd: string, configDir: string | null, startedAt: number, screen: string, ): PlanCountTranscript { - if (!file || !configDir || transcript.status !== 'ready' || !transcript.calls.length || + if (!file || !configDir || transcript.status !== 'ready' || !isCurrentPlanApprovalScreen(screen)) return transcript; try { const stat = fs.lstatSync(file); @@ -83,6 +83,12 @@ export function withPendingExit( sessions.size !== 1 || !sessions.has(record.sessionId) || !scopedTranscript(record.transcriptPath, configDir, record.sessionId) || !Number.isFinite(time) || time < startedAt || time > Date.now()) return transcript; + // A zero-question review still needs its owned gate for failure diagnostics. + // Bind it to actual current-session assistant output; this adds no coverage. + if (!transcript.calls.length && !transcript.assistantMessages.some(message => + message.sessionId === record.sessionId && message.text.trim() && + Number.isFinite(Date.parse(message.timestamp)) && Date.parse(message.timestamp) >= startedAt && + Date.parse(message.timestamp) <= time)) return transcript; const requests = transcript.planReadyRequests ?? []; // A flushed native record, including a failed result, always wins. if (requests.some(request => request.sessionId === record.sessionId && request.toolUseId === record.toolUseId)) return transcript; diff --git a/test/helpers/session-runner.ts b/test/helpers/session-runner.ts index 6c316b413..40d632d42 100644 --- a/test/helpers/session-runner.ts +++ b/test/helpers/session-runner.ts @@ -11,6 +11,7 @@ import * as path from 'path'; import * as os from 'os'; import { spawn } from 'child_process'; import { Readable } from 'node:stream'; +import { createHash, type Hash } from 'node:crypto'; import { getProjectEvalDir } from './eval-store'; import { hermeticChildEnv, isHermeticEnabled } from './hermetic-env'; import { killProcessGroup } from '../../scripts/test-strict-output'; @@ -126,6 +127,58 @@ function truncate(s: string, max: number): string { return s.length > max ? s.slice(0, max) + '…' : s; } +/** Diagnostic-only projection. Partial input never becomes a complete tool call. */ +function publicStreamProjection(startTime: number): (line: string) => string { + let messageId: string | undefined; + const blocks = new Map(); + return (line) => { + let row: any; + try { row = JSON.parse(line); } catch { + // A truncated line may contain private reasoning or unfinished tool input. + return JSON.stringify({ type: 'public_stream_diagnostic', kind: 'unparseable_line', + elapsedMs: Date.now() - startTime, bytes: Buffer.byteLength(line) }); + } + if (row === null || typeof row !== 'object' || Array.isArray(row)) { + return JSON.stringify({ type: 'public_stream_diagnostic', kind: 'non_object_line', + elapsedMs: Date.now() - startTime, bytes: Buffer.byteLength(line) }); + } + if (row.type === 'stream_event') { + const event = row.event ?? {}; + if (event.type === 'message_start') { + messageId = event.message?.id; + blocks.clear(); + } + const index = event.index; + if (event.type === 'content_block_start' && Number.isInteger(index)) { + blocks.set(index, { type: event.content_block?.type, tool: event.content_block?.name, + bytes: 0, hash: createHash('sha256') }); + } + const block = blocks.get(index); + if (event.type === 'content_block_delta' && block?.type === 'tool_use' + && event.delta?.type === 'input_json_delta' && typeof event.delta.partial_json === 'string') { + const chunk = Buffer.from(event.delta.partial_json); + block.bytes += chunk.length; + block.hash.update(chunk); + } + const diagnostic = { type: 'public_stream_diagnostic', kind: event.type, + session_id: row.session_id, messageId, index, elapsedMs: Date.now() - startTime, + blockType: block?.type, toolName: block?.tool, deltaType: event.delta?.type, + ...(block?.type === 'tool_use' ? { inputBytes: block.bytes, + inputSha256: block.hash.copy().digest('hex') } : {}), + ...(event.type === 'message_delta' ? { stopReason: event.delta?.stop_reason } : {}), + }; + if (event.type === 'content_block_stop') blocks.delete(index); + return JSON.stringify(diagnostic); + } + if (Array.isArray(row.message?.content)) { + row.message.content = row.message.content.map((block: any) => + block.type === 'thinking' || block.type === 'redacted_thinking' + ? { type: block.type, omitted: true } : block); + } + return JSON.stringify(row); + }; +} + // --- Main runner --- export async function runSkillTest(options: { @@ -136,6 +189,8 @@ export async function runSkillTest(options: { allowedTools?: string[]; /** Optional built-in tool availability. Omit to preserve the CLI defaults. */ tools?: string[]; + /** Opt-in public block timing/input-size diagnostics; never completion evidence. */ + publicStreamDiagnostics?: boolean; timeout?: number; testName?: string; runId?: string; @@ -204,6 +259,7 @@ export async function runSkillTest(options: { // only --tools removes unrelated built-ins such as Agent, Bash, and Skill. // Keep this opt-in: existing workflow evals intentionally use CLI defaults. if (options.tools !== undefined) args.push('--tools', options.tools.join(',')); + if (options.publicStreamDiagnostics) args.push('--include-partial-messages'); // Hermetic children get zero MCP servers (no --mcp-config is passed). // Gated on the same call-time check as the env scrub so EVALS_HERMETIC=0 // restores operator MCP along with the operator env. @@ -300,6 +356,7 @@ export async function runSkillTest(options: { const reader = stdoutWeb.getReader(); const decoder = new TextDecoder(); let buf = ''; + const projectLine = options.publicStreamDiagnostics ? publicStreamProjection(startTime) : (line: string) => line; try { while (true) { @@ -308,8 +365,9 @@ export async function runSkillTest(options: { buf += decoder.decode(value, { stream: true }); const lines = buf.split('\n'); buf = lines.pop() || ''; - for (const line of lines) { - if (!line.trim()) continue; + for (const rawLine of lines) { + if (!rawLine.trim()) continue; + const line = projectLine(rawLine); collectedLines.push(line); // Track time to first NDJSON line (measures latency from spawn to first Claude response) @@ -382,7 +440,11 @@ export async function runSkillTest(options: { // Flush remaining buffer if (buf.trim()) { - collectedLines.push(buf); + const line = projectLine(buf); + collectedLines.push(line); + if (options.publicStreamDiagnostics && runDir && safeName) { + try { fs.appendFileSync(path.join(runDir, `${safeName}.ndjson`), line + '\n'); } catch { /* non-fatal */ } + } } // Same orphan hazard as stdout: an orphaned grandchild holding stderr open diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 2ea969e81..0fd76b2fb 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -160,10 +160,10 @@ export const E2E_TOUCHFILES: Record = { // 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/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.json', 'test/helpers/ceo-completion-handoff.ts', 'test/ceo-completion-handoff.test.ts', 'test/ceo-count-s-terminals.test.ts', 'test/fixtures/ceo-count-s-paired.json', '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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', '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', 'test/fixtures/ceo-handoff-n-calls.json', 'test/ceo-completion-handoff-o.test.ts', 'test/fixtures/ceo-completion-handoff-o-call.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/helpers/ceo-approach-pick.ts', 'test/ceo-approach-pick.test.ts', 'test/fixtures/ceo-approach-q-call.json', 'test/fixtures/ceo-approach-r-call.json', 'test/fixtures/ceo-approach-r-distinct-call.json', 'test/fixtures/ceo-approach-q-paired-call.json', 'test/fixtures/ceo-completion-handoff-q-call.json', 'test/fixtures/ceo-completion-handoff-r-calls.json', 'test/fixtures/ceo-completion-handoff-t-call.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/ceo-completion-handoff-u-call.json', 'test/fixtures/ceo-completion-handoff-v-call.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json'], - '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/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', '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', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/eng-first-review-t.test.ts', 'test/fixtures/eng-batching-t-calls.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json'], - '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/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', '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', 'test/fixtures/design-review-n-calls.json', 'test/design-completion-handoff-scored.test.ts', 'test/fixtures/design-handoff-n-calls.json', 'test/fixtures/design-handoff-q-calls.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/design-completion-handoff-u.test.ts', 'test/fixtures/design-handoff-u-calls.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json'], - '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/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts', 'test/fixtures/dx-prerequisite-r-call.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/fixtures/devex-review-n-calls.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/devex-output-o.test.ts', 'test/fixtures/devex-review-o-calls.json', 'test/fixtures/devex-output-o-retry-call.json', 'test/devex-setup-remedy-o.test.ts', 'test/fixtures/devex-review-o-retry-calls.json', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/fixtures/devex-review-t-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/devex-count-u-calls.json', 'test/fixtures/devex-count-u-retry-calls.json', 'test/fixtures/devex-empathy-v-calls.json', 'test/fixtures/devex-handoff-v-call.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json'], + '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/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.json', 'test/helpers/ceo-completion-handoff.ts', 'test/ceo-completion-handoff.test.ts', 'test/ceo-count-s-terminals.test.ts', 'test/fixtures/ceo-count-s-paired.json', '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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', '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', 'test/fixtures/ceo-handoff-n-calls.json', 'test/ceo-completion-handoff-o.test.ts', 'test/fixtures/ceo-completion-handoff-o-call.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/helpers/ceo-approach-pick.ts', 'test/ceo-approach-pick.test.ts', 'test/fixtures/ceo-approach-q-call.json', 'test/fixtures/ceo-approach-r-call.json', 'test/fixtures/ceo-approach-r-distinct-call.json', 'test/fixtures/ceo-approach-q-paired-call.json', 'test/fixtures/ceo-completion-handoff-q-call.json', 'test/fixtures/ceo-completion-handoff-r-calls.json', 'test/fixtures/ceo-completion-handoff-t-call.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/ceo-completion-handoff-u-call.json', 'test/fixtures/ceo-completion-handoff-v-call.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json', 'test/plan-count-ceo-body-finding.test.ts', 'test/fixtures/ceo-count-w-paired.json', 'test/fixtures/ceo-completion-handoff-w-call.json'], + '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/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', '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', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/eng-first-review-t.test.ts', 'test/fixtures/eng-batching-t-calls.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json'], + '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/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', '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', 'test/fixtures/design-review-n-calls.json', 'test/design-completion-handoff-scored.test.ts', 'test/fixtures/design-handoff-n-calls.json', 'test/fixtures/design-handoff-q-calls.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/design-completion-handoff-u.test.ts', 'test/fixtures/design-handoff-u-calls.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json', 'test/helpers/design-artifact-question.ts', 'test/design-artifact-question.test.ts', 'test/fixtures/design-artifacts-w-calls.json'], + '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/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts', 'test/fixtures/dx-prerequisite-r-call.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/fixtures/devex-review-n-calls.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/devex-output-o.test.ts', 'test/fixtures/devex-review-o-calls.json', 'test/fixtures/devex-output-o-retry-call.json', 'test/devex-setup-remedy-o.test.ts', 'test/fixtures/devex-review-o-retry-calls.json', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/fixtures/devex-review-t-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/devex-count-u-calls.json', 'test/fixtures/devex-count-u-retry-calls.json', 'test/fixtures/devex-empathy-v-calls.json', 'test/fixtures/devex-handoff-v-call.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json'], // Gate-tier reviewCount-floor counterparts. Catch the May 2026 transcript // bug (model wrote a plan-mode plan and ExitPlanMode'd without firing any @@ -180,8 +180,8 @@ export const E2E_TOUCHFILES: Record = { // a model fires one AUQ then batches the rest into a "## Decisions to // confirm" plan write. runPlanSkillFloorCheck cannot detect that shape // (it exits on first AUQ); runPlanSkillCounting can. - 'plan-eng-multi-finding-batching': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/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/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/eng-first-review-t.test.ts', 'test/fixtures/eng-batching-t-calls.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json'], - 'plan-ceo-split-overflow': ['bin/gstack-config', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json'], + 'plan-eng-multi-finding-batching': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/eng-first-review-t.test.ts', 'test/fixtures/eng-batching-t-calls.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json'], + 'plan-ceo-split-overflow': ['bin/gstack-config', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json'], 'brain-privacy-gate': ['bin/gstack-skill-start', 'bin/gstack-skill-end', 'scripts/resolvers/preamble/generate-brain-sync-block.ts', 'scripts/resolvers/preamble.ts', 'bin/gstack-brain-sync', 'bin/gstack-artifacts-init', 'bin/gstack-config', 'test/helpers/agent-sdk-runner.ts', 'test/skill-e2e-brain-privacy-gate.test.ts'], // /setup-gbrain Path 4 (Remote MCP) — happy + bad-token end-to-end via @@ -282,6 +282,8 @@ export const E2E_TOUCHFILES: Record = { '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/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', + 'test/helpers/outside-voice-receipt.ts', 'test/outside-voice-receipt.test.ts', + 'test/claude-code-runner.test.ts', ], 'outside-voice-claude-code-to-codex': [ 'review/**', 'codex/**', 'hosts/claude.ts', 'hosts/define-host.ts', diff --git a/test/outside-voice-receipt.test.ts b/test/outside-voice-receipt.test.ts new file mode 100644 index 000000000..d13a234c7 --- /dev/null +++ b/test/outside-voice-receipt.test.ts @@ -0,0 +1,107 @@ +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 { createOutsideReceiptRuntime } from './helpers/outside-voice-receipt'; +import { codexExecutionTranscript, foundInvoiceAuthorizationDefect } from './helpers/outside-voice-evidence'; + +const finding = 'invoice.ts removes the owner check and exposes another user’s bankAccount.\nRecommendation: Restore authorization because invoice financial data crosses ownership boundaries.'; +function fixture() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'outside-receipt-free-')); + const real = path.join(dir, 'real runtime'), repo = path.join(dir, 'repo'), observed = path.join(dir, 'observed.json'); + fs.mkdirSync(path.join(real, 'bin'), { recursive: true }); fs.mkdirSync(repo); + const launcher = path.join(real, 'bin/gstack-claude-code'); + fs.writeFileSync(launcher, `#!${process.execPath}\nconst input=await Bun.stdin.text();await Bun.write(${JSON.stringify(observed)},JSON.stringify({argv:process.argv.slice(2),input,marker:process.env.RECEIPT_FREE_MARKER}));const request=JSON.parse(input);console.log(JSON.stringify(request.output));console.error('real diagnostic');if(request.signal)process.kill(process.pid,request.signal);else process.exit(request.exit??0);\n`, { mode: 0o755 }); + fs.writeFileSync(path.join(real, 'bin/unchanged'), 'unchanged'); + const runtime = createOutsideReceiptRuntime(real, dir, repo); + const shim = path.join(runtime.runtimeRoot, 'bin/gstack-claude-code'); + const envelope = { status: 'completed', provider: 'claude-code', result: finding, session_id: 'free-session-A', exit_code: 0 }; + const argv = ['--cwd', repo, '--access', 'none', '--timeout-ms', '1000']; + return { dir, real, repo, observed, launcher, runtime, shim, envelope, argv, + records: path.join(path.dirname(runtime.runtimeRoot), 'records'), + async run(output = envelope, exit = 0, signal?: string) { + const input = JSON.stringify({ output, exit, signal }); + const wrapper = path.join(dir, 'indirect.sh'); + fs.writeFileSync(wrapper, '#!/bin/sh\nexec "$@"\n'); + const child = Bun.spawn(['/bin/sh', wrapper, shim, ...argv], { + cwd: repo, env: { ...process.env, RECEIPT_FREE_MARKER: 'unchanged-env' }, + stdin: new Blob([input]), stdout: 'pipe', stderr: 'pipe', + }); + const [code, stdout, stderr] = await Promise.all([child.exited, new Response(child.stdout).text(), new Response(child.stderr).text()]); + return { code, stdout, stderr, input, command: '/bin/sh ' + wrapper }; + }, + close() { fs.rmSync(dir, { recursive: true, force: true }); }, + }; +} + +test.skipIf(process.platform === 'win32')('indirect wrapper needs a real nonce-bound completed launcher receipt, not parent stdout', async () => { + const f = fixture(); + try { + expect(f.runtime.read()).toEqual({ receipts: [], executions: [] }); + const result = await f.run(); + expect(result.code).toBe(0); expect(JSON.parse(result.stdout)).toEqual(f.envelope); expect(result.stderr).toBe('real diagnostic\n'); + expect(JSON.parse(fs.readFileSync(f.observed, 'utf8'))).toEqual({ argv: f.argv, input: result.input, marker: 'unchanged-env' }); + const parent = [{ command: result.command, output: result.stdout, succeeded: true }]; + expect(foundInvoiceAuthorizationDefect(parent, 'claude-code')).toBe(false); + expect(codexExecutionTranscript(parent)).toEqual([{ type: 'host_execution', host: 'codex', ...parent[0] }]); + const actual = f.runtime.read(); + expect(actual.receipts).toHaveLength(1); expect(actual.receipts[0]!.pid).toBeGreaterThan(0); + expect(actual.receipts[0]!.inputBytes).toBe(Buffer.byteLength(result.input)); + expect(foundInvoiceAuthorizationDefect(actual.executions, 'claude-code')).toBe(true); + expect(fs.realpathSync(path.join(f.runtime.runtimeRoot, 'bin/unchanged'))).toBe(path.join(f.real, 'bin/unchanged')); + } finally { f.close(); } +}, 10000); + +test.skipIf(process.platform === 'win32')('stale, foreign, altered, alias and malformed receipts cannot supply credit', async () => { + const f = fixture(); + try { + await f.run(); const file = path.join(f.records, fs.readdirSync(f.records).find(x => x.endsWith('.json'))!); + const raw = fs.readFileSync(file, 'utf8'), valid = JSON.parse(raw); + for (const patch of [ + { nonce: 'foreign' }, { cwd: '/foreign' }, { actualCwd: '/foreign' }, { records: '/foreign' }, + { launcher: '/foreign' }, { launcherHash: 'wrong' }, { createdAt: 0 }, + { startedAt: 0 }, { endedAt: Date.now() + 100000 }, { pid: 0 }, { inputBytes: 0 }, + { inputHash: 'bad' }, { stdout: finding }, { stdoutHash: 'bad' }, { stderrHash: 'bad' }, + { argv: ['--cwd', '/foreign'] }, { argv: [...f.argv, '--cwd', f.repo] }, + ]) { + fs.writeFileSync(file, JSON.stringify({ ...valid, ...patch })); expect(f.runtime.read().executions, JSON.stringify(patch)).toEqual([]); + } + fs.writeFileSync(file, '{broken'); expect(f.runtime.read().receipts).toEqual([]); + const other = path.join(f.dir, 'other.json'); fs.writeFileSync(other, raw); fs.unlinkSync(file); fs.symlinkSync(other, file); + expect(f.runtime.read().receipts).toEqual([]); fs.unlinkSync(file); fs.writeFileSync(file, raw); + fs.appendFileSync(f.launcher, '// altered launcher\n'); expect(f.runtime.read().receipts).toEqual([]); + } finally { f.close(); } +}, 10000); + +test.skipIf(process.platform === 'win32')('real failures, missing session, transport-only output and no-finding reviews remain negative', async () => { + const f = fixture(); + try { + const cases = [ + [{ ...f.envelope, status: 'error' }, 0], [{ ...f.envelope, exit_code: 1 }, 0], + [{ ...f.envelope, session_id: '' }, 0], [{ ...f.envelope, result: 'Tool use: Bash\ninvoice owner' }, 0], + [{ ...f.envelope, result: 'Recommendation: Accept because no issue was found.' }, 0], [f.envelope, 7], + ] as const; + for (const [output, exit] of cases) { + const result = await f.run(output, exit); expect(result.code).toBe(exit); + expect(foundInvoiceAuthorizationDefect(f.runtime.read().executions, 'claude-code')).toBe(false); + } + expect(f.runtime.read().receipts).toHaveLength(cases.length); + } finally { f.close(); } +}, 10000); + + +test.skipIf(process.platform === 'win32')('real signal and observation write failure preserve the child outcome without credit', async () => { + const f = fixture(); + try { + const signaled = await f.run(f.envelope, 0, 'SIGTERM'); + expect(signaled.code).toBe(143); + expect(f.runtime.read().receipts[0]?.signal).toBe('SIGTERM'); + expect(f.runtime.read().executions).toEqual([]); + fs.rmSync(f.records, { recursive: true }); + const unchanged = await f.run(f.envelope); + expect(unchanged.code).toBe(0); + expect(JSON.parse(unchanged.stdout)).toEqual(f.envelope); + expect(unchanged.stderr).toBe('real diagnostic\n'); + expect(f.runtime.read()).toEqual({ receipts: [], executions: [] }); + } finally { f.close(); } +}, 10000); diff --git a/test/plan-count-ceo-body-finding.test.ts b/test/plan-count-ceo-body-finding.test.ts new file mode 100644 index 000000000..0bd7bc11e --- /dev/null +++ b/test/plan-count-ceo-body-finding.test.ts @@ -0,0 +1,74 @@ +import { expect, test } from 'bun:test'; +import captured from './fixtures/ceo-count-w-paired.json'; +import { ceoFirstReviewAUQ, ceoStep0Boundary, nativePlanCallFingerprint, planCountQuestionPhase } from './helpers/claude-pty-runner'; + +test('native CEO briefs with the defect below the title start review; one batched call stays one', () => { + let started = false; + let count = 0; + for (const [i, call] of captured.calls.entries()) { + const fingerprint = nativePlanCallFingerprint(call, i, true); + const phase = planCountQuestionPhase(fingerprint, started, ceoStep0Boundary, ceoFirstReviewAUQ); + expect(phase.preReview).toBe(false); + started = phase.reviewStarted; + if (!phase.preReview) count++; + } + expect(count).toBe(3); + expect(captured.calls[2].questions[0].multiSelect).toBe(true); + expect(Object.values(captured.calls[2].answers)).toHaveLength(1); +}); + +test.each([0, 1])('actual body finding %i is recognized even without a previous mode answer', i => { + expect(ceoFirstReviewAUQ(nativePlanCallFingerprint(captured.calls[i], i, true))).toBe(true); +}); + +test('body evidence cannot replace an answered native decision or identify a setup recap as review', () => { + const mutations: Array<(call: typeof captured.calls[0]) => void> = [ + call => { call.answers = {}; }, + call => { call.questions[0].question = call.questions[0].question.split('\n')[0]; }, + call => { call.questions[0].question = call.questions[0].question.replace('plan-ceo-review-failure-sig', 'plan-ceo-review-approach'); }, + call => { call.questions[0].header = 'Approach'; }, + call => { call.questions[0].header = 'Next steps'; }, + call => { call.questions[0].options[0].label = 'HOLD SCOPE (Recommended)'; }, + call => { call.questions[0].question = call.questions[0].question.replace(/]+>/, ''); }, + call => { call.questions[0].question = call.questions[0].question.replace('D1 —', 'Context:'); }, + ]; + for (const mutate of mutations) { + const call = structuredClone(captured.calls[0]); + const originalQuestion = call.questions[0].question; + mutate(call); + if (call.questions[0].question !== originalQuestion) { + call.answers = { [call.questions[0].question]: Object.values(call.answers)[0] }; + } + expect(ceoFirstReviewAUQ(nativePlanCallFingerprint(call, 0, true)), JSON.stringify(call)).toBe(false); + } +}); + +test.each([ + 'ELI10: The plan has no missing requirements or unspecified behavior. This is a readiness check.', + 'ELI10: The previous plan had missing tests. Those gaps are resolved and the current plan is complete. Start the review now?', + 'ELI10: Here is a quotation from the training example, not a current finding:\n> The plan has missing tests.\n\nContinue to the review?', + 'ELI10: Training example: "The plan does not specify the failure contract." The current plan is complete; this is only a readiness check.', + 'ELI10: Training example: “The plan does not specify the failure contract.” The current plan is complete.', + 'ELI10: Training example: `The plan does not specify the failure contract.` The current plan is complete.', + 'ELI10: Training example:\n```text\nThe plan does not specify the failure contract.\n```\nThe current plan is complete.', + 'ELI10: Training example:\n~~~text\nThe plan does not specify the failure contract.\n~~~\nThe current plan is complete.', + 'ELI10: If the plan does not specify the failure contract, we would add it. The current plan already specifies it; this is a readiness check.', + 'ELI10: It is not true that the plan does not specify the failure contract. The current plan is complete.', +])('negated, resolved and quoted gaps do not start review: %s', body => { + const call = structuredClone(captured.calls[0]); + call.questions[0].question = 'D1 — Ready to continue? \n\n' + body; + // Even a plan-amendment option cannot turn a quotation or closed issue into + // evidence of a current defect. Retain the actual offered/answered option. + call.answers = { [call.questions[0].question]: call.questions[0].options[0].label }; + expect(ceoFirstReviewAUQ(nativePlanCallFingerprint(call, 0, true))).toBe(false); +}); + +test('a current omission mentioned during setup needs an actual remedy choice', () => { + const call = structuredClone(captured.calls[0]); + call.questions[0].options = [ + {label:'Start review', description:'Begin the existing review workflow.'}, + {label:'Pause', description:'Wait before beginning.'}, + ]; + call.answers = { [call.questions[0].question]: 'Start review' }; + expect(ceoFirstReviewAUQ(nativePlanCallFingerprint(call, 0, true))).toBe(false); +}); diff --git a/test/plan-count-pending-exit.test.ts b/test/plan-count-pending-exit.test.ts index 0a1380a62..3147b57e1 100644 --- a/test/plan-count-pending-exit.test.ts +++ b/test/plan-count-pending-exit.test.ts @@ -4,7 +4,8 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { pathToFileURL } from 'node:url'; import { createPendingExitRecorder, isCurrentPlanApprovalScreen, recordPendingExit, withPendingExit } from './helpers/plan-count-pending-exit'; -import { hasNativePlanTerminal } from './helpers/claude-pty-runner'; +import { hasNativePlanTerminal, isQuestionlessNativePlanExit } from './helpers/claude-pty-runner'; +import capturedQuestionless from './fixtures/ceo-questionless-w-native.json'; import type { PlanCountTranscript } from './helpers/plan-count-transcript'; const GATE = '────────────────────────────────────────────────────────\n' + @@ -40,6 +41,41 @@ function fixture() { } describe('pending native ExitPlanMode identity', () => { + test('actual W zero-call owned gate supplies failure-only termination', () => { + const f = fixture(); + try { + const actual = capturedQuestionless; + const transcript = structuredClone(actual.transcript) as PlanCountTranscript; + fs.writeFileSync(f.report, actual.report); + fs.utimesSync(f.report, new Date(actual.binding.reportMtimeMs), new Date(actual.binding.reportMtimeMs)); + fs.writeFileSync(f.recorder.file, JSON.stringify(actual.hook)); + const observe = (t = transcript, screen = actual.screen) => withPendingExit(t, f.recorder.file, + actual.hook.cwd, actual.binding.hook.config, actual.binding.startedAt, screen); + expect(transcript.calls).toEqual([]); + expect(isCurrentPlanApprovalScreen(actual.screen)).toBe(true); + const projected = observe(); + expect(projected.calls).toEqual([]); + expect(projected.planReadyRequests?.[0]?.source).toBe('pre_tool_use'); + expect(isQuestionlessNativePlanExit(projected, f.report, actual.binding.startedAt, actual.screen)).toBe(true); + expect(hasNativePlanTerminal(projected, f.report, actual.binding.startedAt, 'plan_ready')).toBe(false); + for (const change of [ + (t: PlanCountTranscript) => { t.assistantMessages = []; }, + (t: PlanCountTranscript) => { t.assistantMessages = t.assistantMessages.map(m => ({ ...m, text: '' })); }, + (t: PlanCountTranscript) => { t.assistantMessages[0]!.sessionId = 'foreign'; }, + (t: PlanCountTranscript) => { t.assistantMessages = t.assistantMessages.map(m => ({ ...m, timestamp: new Date(actual.binding.startedAt - 1).toISOString() })); }, + (t: PlanCountTranscript) => { t.planReadyRequests = [{ sessionId: actual.hook.sessionId, toolUseId: actual.hook.toolUseId, timestamp: actual.hook.timestamp, failed: true }]; }, + ]) { + const value = structuredClone(transcript); change(value); + expect(isQuestionlessNativePlanExit(observe(value), f.report, actual.binding.startedAt, actual.screen)).toBe(false); + } + for (const patch of [{ sessionId: 'foreign' }, { cwd: '/foreign' }, + { timestamp: new Date(actual.binding.startedAt - 1).toISOString() }]) { + fs.writeFileSync(f.recorder.file, JSON.stringify({ ...actual.hook, ...patch })); + expect(observe().planReadyRequests).toBeUndefined(); + } + } finally { f.cleanup(); } + }); + test('the captured gate can complete only with real pending identity and a fresh report', () => { const f = fixture(); try { diff --git a/test/session-runner-tools.test.ts b/test/session-runner-tools.test.ts index 082958277..3c0c57854 100644 --- a/test/session-runner-tools.test.ts +++ b/test/session-runner-tools.test.ts @@ -2,6 +2,7 @@ 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 { createHash } from 'node:crypto'; import { captureSectionReads, hasDisabledOutsideReview, LONG_SECTION_CAPTURE_MS } from './helpers/auq-sdk-capture'; import { CAPTURE_MS, CAPTURE_LONG_MS } from './helpers/eval-budgets'; import { getHermeticDirs } from './helpers/hermetic-env'; @@ -54,7 +55,41 @@ const FAKE_CLAUDE = String.raw` fs.writeFileSync('observed.json', JSON.stringify(observed)); const outputFile = fs.existsSync('active-plan-output') ? 'PLAN.md' : 'REPORT.md'; console.log(JSON.stringify({ type: 'system', subtype: 'init' })); - if (fs.existsSync('fail-cli')) { + if (fs.existsSync('diagnostic-case')) { + if (fs.readFileSync('diagnostic-case', 'utf8') === 'non-objects') { + for (const value of [null, ['PRIVATE_NON_OBJECT'], 'PRIVATE_NON_OBJECT', 42, true]) console.log(JSON.stringify(value)); + } + const emit = event => console.log(JSON.stringify({ type: 'stream_event', session_id: 'fixture-session', event })); + emit({ type: 'message_start', message: { id: 'fixture-message' } }); + emit({ type: 'content_block_start', index: 0, content_block: { type: 'thinking', thinking: 'PRIVATE_START' } }); + emit({ type: 'content_block_delta', index: 0, delta: { type: 'thinking_delta', thinking: 'PRIVATE_DELTA' } }); + emit({ type: 'content_block_stop', index: 0 }); + console.log(JSON.stringify({ type: 'assistant', message: { content: [ + { type: 'thinking', thinking: 'PRIVATE_COMPLETE', signature: 'PRIVATE_SIGNATURE' }, + { type: 'redacted_thinking', data: 'PRIVATE_REDACTED' }, + ] } })); + emit({ type: 'content_block_start', index: 1, content_block: { type: 'tool_use', id: 'partial-write', name: 'Write', input: {} } }); + for (const partial_json of ['{"file_path":"PLAN.md",', '"content":"marker😀"}']) { + emit({ type: 'content_block_delta', index: 1, delta: { type: 'input_json_delta', partial_json } }); + } + emit({ type: 'content_block_start', index: 2, content_block: { type: 'tool_use', id: 'partial-read', name: 'Read', input: {} } }); + emit({ type: 'content_block_delta', index: 2, delta: { type: 'input_json_delta', partial_json: '{"file_path":"sections/review-sections.md"}' } }); + emit({ type: 'content_block_stop', index: 2 }); + if (fs.readFileSync('diagnostic-case', 'utf8') !== 'partial') { + emit({ type: 'content_block_stop', index: 1 }); + console.log(JSON.stringify({ type: 'assistant', message: { content: [ + { type: 'tool_use', id: 'real-read', name: 'Read', input: { file_path: 'sections/review-sections.md' } }, + { type: 'tool_use', id: 'real-write', name: 'Write', input: { file_path: 'PLAN.md', content: 'complete report' } }, + ] } })); + emit({ type: 'message_delta', delta: { stop_reason: 'tool_use' } }); + emit({ type: 'message_stop' }); + console.log(JSON.stringify({ type: 'result', subtype: 'success', result: 'Finished' })); + } else { + // An old report marker and partial Read/Write blocks cannot satisfy capture. + fs.writeFileSync(outputFile, '## GSTACK REVIEW REPORT\nold artifact'); + await Bun.sleep(60_000); + } + } else if (fs.existsSync('fail-cli')) { const failure = fs.readFileSync('fail-cli', 'utf8'); const report = '## GSTACK REVIEW REPORT\nA report written before the run failed.\n'; fs.writeFileSync(outputFile, report); @@ -217,10 +252,75 @@ describe.skipIf(process.platform === 'win32')('session-runner explicit tool avai await withFakeClaude(async (dir, observed) => { await runSkillTest({ prompt: 'Default tools', workingDirectory: dir, allowedTools: ['Read'], timeout: 5_000 }); expect(observed().args).not.toContain('--tools'); + expect(observed().args).not.toContain('--include-partial-messages'); expect(flagValue(observed().args, '--allowed-tools')).toBe('Read'); }); }); + test('public stream diagnostics retain timing and input sizes without counting partial tools', async () => { + await withFakeClaude(async (dir, observed) => { + fs.writeFileSync(path.join(dir, 'diagnostic-case'), 'complete'); + const result = await runSkillTest({ prompt: 'diagnose', workingDirectory: dir, + publicStreamDiagnostics: true, tools: ['Read', 'Write'], timeout: 5_000 }); + expect(observed().args).toContain('--include-partial-messages'); + expect(result.exitReason).toBe('success'); + expect(result.output).toBe('Finished'); + expect(result.toolCalls).toEqual([ + { tool: 'Read', input: { file_path: 'sections/review-sections.md' }, output: '' }, + { tool: 'Write', input: { file_path: 'PLAN.md', content: 'complete report' }, output: '' }, + ]); + const diagnostics = result.transcript.filter(e => e.type === 'public_stream_diagnostic'); + const stop = diagnostics.find(e => e.kind === 'content_block_stop' && e.index === 1); + const input = '{"file_path":"PLAN.md","content":"marker😀"}'; + expect(stop).toMatchObject({ messageId: 'fixture-message', session_id: 'fixture-session', + blockType: 'tool_use', toolName: 'Write', inputBytes: Buffer.byteLength(input), + inputSha256: createHash('sha256').update(input).digest('hex') }); + expect(diagnostics.every(e => Number.isInteger(e.elapsedMs) && e.elapsedMs >= 0)).toBe(true); + expect(diagnostics.some(e => e.kind === 'content_block_start' && e.blockType === 'thinking')).toBe(true); + expect(diagnostics.some(e => e.kind === 'content_block_stop' && e.blockType === 'thinking')).toBe(true); + expect(diagnostics.find(e => e.kind === 'message_delta').stopReason).toBe('tool_use'); + const retained = JSON.stringify(result.transcript); + expect(retained).not.toContain('PRIVATE_'); + expect(retained).not.toContain('partial_json'); + expect(retained).not.toContain('marker😀'); + }); + }); + + test('section capture opts into diagnostics but partial Write and old report still time out', async () => { + await withFakeClaude(async (dir, observed) => { + fs.writeFileSync(path.join(dir, 'diagnostic-case'), 'partial'); + const started = Date.now(); + const result = await captureSectionReads({ planDir: dir, skillName: 'plan-ceo-review', + scenario: 'Complete the review', testName: 'partial-write', timeout: 1_500, + reportMarker: /## GSTACK REVIEW REPORT/ }); + expect(observed().args).toContain('--include-partial-messages'); + expect(result.toolCalls).toEqual([]); + expect([...result.readSections]).toEqual([]); + expect(result.reportProduced).toBe(false); + expect(Date.now() - started).toBeLessThan(4_000); + expect(result.output).toContain('## GSTACK REVIEW REPORT'); + }); + }, 5_000); + + test('public diagnostics consume non-object JSON without ending the stream', async () => { + await withFakeClaude(async (dir) => { + fs.writeFileSync(path.join(dir, 'diagnostic-case'), 'non-objects'); + for (const publicStreamDiagnostics of [false, true]) { + const result = await runSkillTest({ prompt: 'diagnose', workingDirectory: dir, + publicStreamDiagnostics, tools: ['Read', 'Write'], timeout: 5_000 }); + expect(result.exitReason).toBe('success'); + expect(result.output).toBe('Finished'); + expect(result.toolCalls.map(c => c.tool)).toEqual(['Read', 'Write']); + if (publicStreamDiagnostics) { + const rows = result.transcript.filter(e => e?.kind === 'non_object_line'); + expect(rows).toHaveLength(5); + expect(rows.every(e => e.bytes > 0 && e.elapsedMs >= 0)).toBe(true); + expect(JSON.stringify(rows)).not.toContain('PRIVATE_'); + } + } + }); + }); + test('an explicit empty list uses the CLI no-tools argument', async () => { await withFakeClaude(async (dir, observed) => { await runSkillTest({ prompt: 'No tools', workingDirectory: dir, tools: [], timeout: 5_000 }); diff --git a/test/skill-ceo-section-ordering.test.ts b/test/skill-ceo-section-ordering.test.ts index ea2b66eba..9e3c1da8d 100644 --- a/test/skill-ceo-section-ordering.test.ts +++ b/test/skill-ceo-section-ordering.test.ts @@ -38,6 +38,18 @@ describe('plan-ceo-review carve — static ordering', () => { const STOP = 'sections/review-sections.md'; // appears in the index row + STOP directive const GATE = 'GSTACK REVIEW REPORT'; + test('the interactive anti-shortcut contract is available before audit or lazy section loading', () => { + const contract = '**Anti-shortcut clause:**'; + const audit = skeleton.indexOf('## PRE-REVIEW SYSTEM AUDIT'); + expect(skeleton.indexOf(contract)).toBeGreaterThan(-1); + expect(skeleton.indexOf(contract)).toBeLessThan(audit); + expect(skeleton.split(contract)).toHaveLength(2); + expect(section).not.toContain(contract); + // Relocate the shared instruction intact; do not weaken or duplicate it. + expect(skeleton).toContain('the path from finding to ExitPlanMode goes THROUGH AskUserQuestion'); + expect(skeleton).toContain('Zero findings in every section is the only path'); + }); + test('skeleton emits a STOP-Read directive pointing at the section', () => { expect(skeleton).toContain('> **STOP.**'); expect(skeleton).toContain('plan-ceo-review/sections/review-sections.md'); diff --git a/test/skill-e2e-outside-voice.test.ts b/test/skill-e2e-outside-voice.test.ts index 2d20e637d..762843597 100644 --- a/test/skill-e2e-outside-voice.test.ts +++ b/test/skill-e2e-outside-voice.test.ts @@ -8,7 +8,8 @@ import { runSkillTest } from './helpers/session-runner'; import { runCodexSkill } from './helpers/codex-session-runner'; import { EvalCollector } from './helpers/eval-store'; import { createOutsideReviewRepo, installOutsideReviewFixture } from './helpers/outside-voice-fixture'; -import { claudeOutsideExecutions, codexOutsideExecutions, foundInvoiceAuthorizationDefect, outsideExecutionTranscript } from './helpers/outside-voice-evidence'; +import { claudeOutsideExecutions, codexOutsideExecutions, codexExecutionTranscript, foundInvoiceAuthorizationDefect, outsideExecutionTranscript } from './helpers/outside-voice-evidence'; +import { createOutsideReceiptRuntime } from './helpers/outside-voice-receipt'; import { CAPTURE_LONG_MS } from './helpers/eval-budgets'; const ROOT = path.resolve(import.meta.dir, '..'); @@ -45,7 +46,8 @@ describeLive('Installed workflows dispatch outside their host harness', () => { testIfSelected('outside-voice-codex-to-claude-code', async () => { const repo = createOutsideReviewRepo(fixtureRoot, 'codex'); - const skillDir = installOutsideReviewFixture(rendered, 'codex', repo, ROOT); + const receiptRuntime = createOutsideReceiptRuntime(ROOT, fixtureRoot, repo); + const skillDir = installOutsideReviewFixture(rendered, 'codex', repo, receiptRuntime.runtimeRoot); const result = await runCodexSkill({ skillDir, skillName: 'gstack-review', cwd: repo, prompt: prompt('gstack-review'), // Reviewer scratch files and review logs need writes; this is an isolated @@ -53,11 +55,14 @@ describeLive('Installed workflows dispatch outside their host harness', () => { sandbox: 'danger-full-access', timeoutMs: CAPTURE_LONG_MS, }); const executions = codexOutsideExecutions(result.rawLines); - const dispatched = foundInvoiceAuthorizationDefect(executions, 'claude-code'); + const observed = receiptRuntime.read(); + const dispatched = foundInvoiceAuthorizationDefect(executions, 'claude-code') || + foundInvoiceAuthorizationDefect(observed.executions, 'claude-code'); const passed = result.exitCode === 0 && dispatched; collector?.addTest({ name: 'outside-voice-codex-to-claude-code', suite: 'outside-voice', tier: 'e2e', passed, duration_ms: result.durationMs, cost_usd: 0, output: result.output.slice(0, 2000), - transcript: outsideExecutionTranscript(executions, 'claude-code'), + transcript: [...outsideExecutionTranscript(executions, 'claude-code'), ...codexExecutionTranscript(executions), + ...observed.receipts.map(receipt => ({ type: 'outside_cli_receipt', provider: 'claude-code', receipt }))], turns_used: result.toolCalls.length, exit_reason: result.exitCode === 0 ? 'success' : `exit_${result.exitCode}` }); expect(result.exitCode).toBe(0); expect(dispatched).toBe(true); diff --git a/test/skill-e2e-plan-design-finding-count.test.ts b/test/skill-e2e-plan-design-finding-count.test.ts index 39df5293c..fafcac1ce 100644 --- a/test/skill-e2e-plan-design-finding-count.test.ts +++ b/test/skill-e2e-plan-design-finding-count.test.ts @@ -11,6 +11,7 @@ import { test } from 'bun:test'; import { describeE2ETier } from './helpers/e2e-gate'; import { isDesignCountFirstReview, isDesignCompletionHandoff, pickDesignCountQuestion } from './helpers/design-count-review'; +import { isDesignArtifactGeneration } from './helpers/design-artifact-question'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -194,6 +195,7 @@ describeE2E('/plan-design-review per-finding AskUserQuestion count (periodic)', isLastStep0AUQ: designStep0Boundary, isFirstReviewAUQ: isDesignCountFirstReview, isCompletionHandoffAUQ: isDesignCompletionHandoff, + isArtifactGenerationAUQ: isDesignArtifactGeneration, fixtureFiles: { 'DESIGN.md': designSystem }, // Design's explicit opt-in is separate from codex_reviews. Keep // this native-cadence fixture within its declared review scope.