From 8d13930f2ec6aac42a7f2b45d415a3c469e24649 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 11 Sep 2026 03:02:23 +0000 Subject: [PATCH] test: fix review evidence parsing and repeated artifact prompts --- test/autoplan-overwrite-progress-ax.test.ts | 70 ++++++ test/ceo-section-choice-ai.test.ts | 124 ++++++++++ test/disabled-plan-review-evidence.test.ts | 37 +++ test/eng-owned-explanation.test.ts | 215 ++++++++++++++++++ .../autoplan-overwrite-progress-ax.json | 61 +++++ test/fixtures/ceo-metadata-brief-ax.json | 68 ++++++ test/fixtures/eng-owned-explanation.json | 50 ++++ test/helpers/autoplan-artifact-permission.ts | 21 ++ test/helpers/claude-pty-runner.ts | 64 +++++- test/helpers/disabled-plan-review-fixture.ts | 21 +- test/helpers/eng-seeded-coverage.ts | 113 ++++++++- test/helpers/plan-scope-selection.ts | 20 +- test/helpers/touchfiles-data.ts | 5 +- test/plan-scope-selection.test.ts | 50 ++++ test/skill-e2e-autoplan-chain.test.ts | 11 +- 15 files changed, 903 insertions(+), 27 deletions(-) create mode 100644 test/autoplan-overwrite-progress-ax.test.ts create mode 100644 test/eng-owned-explanation.test.ts create mode 100644 test/fixtures/autoplan-overwrite-progress-ax.json create mode 100644 test/fixtures/ceo-metadata-brief-ax.json create mode 100644 test/fixtures/eng-owned-explanation.json diff --git a/test/autoplan-overwrite-progress-ax.test.ts b/test/autoplan-overwrite-progress-ax.test.ts new file mode 100644 index 000000000..ba912a8bd --- /dev/null +++ b/test/autoplan-overwrite-progress-ax.test.ts @@ -0,0 +1,70 @@ +import {expect,test} from 'bun:test'; +import fs from 'node:fs'; +import {autoplanPermissionProgressKey} from './helpers/autoplan-artifact-permission'; +import type {NativePublicToolEvent} from './helpers/plan-count-transcript'; +import capture from './fixtures/autoplan-overwrite-progress-ax.json'; +const before=()=>structuredClone(capture.beforeEvents) as NativePublicToolEvent[]; +const after=()=>structuredClone(capture.afterEvents) as NativePublicToolEvent[]; + +test('the acknowledged 92-line Write distinguishes the next identical overwrite footer',()=>{ + expect(capture.before.slice(-500)).toBe(capture.after.slice(-500)); + const oldKey=autoplanPermissionProgressKey(capture.before,before()); + const newKey=autoplanPermissionProgressKey(capture.after,after()); + expect(oldKey).toEndWith(':toolu_01RBorP8UERrbVheRiXSN1v4'); + expect(newKey).toEndWith(':toolu_01RPGbV4z5AAMcnzwD4qcx9p'); + expect(newKey).not.toBe(oldKey); +}); + +test('the same still-pending dialog has no new progress epoch',()=>{ + const events=before(),key=autoplanPermissionProgressKey(capture.before,events); + expect(autoplanPermissionProgressKey(capture.after,events)).toBe(key); + events.push(after()[2]!); // Published use alone has not completed. + expect(autoplanPermissionProgressKey(capture.after,events)).toBe(key); + events.push({...after()[3]!,isError:true}); + expect(autoplanPermissionProgressKey(capture.after,events)).toBe(key); +}); + +test('unrelated results and same-basename files in other directories do not advance the epoch',()=>{ + const key=autoplanPermissionProgressKey(capture.before,before()); + for(const mutate of [ + (events:NativePublicToolEvent[])=>{events[2]!.name='Read';}, + events=>{events[2]!.name='Bash';}, + events=>{events[2]!.input!.file_path=String(events[2]!.input!.file_path).replace('/ceo-plans/','/other-plans/');}, + events=>{events[2]!.input!.file_path=String(events[2]!.input!.file_path).replace('/ceo-plans/','/ceo-plans-sibling/');}, + events=>{events[3]!.isError=undefined;}, + events=>{events[3]!.toolUseId='unrelated-result';}, + events=>{events[3]!.timestamp='invalid';}, + events=>{events[3]!.timestamp='2026-09-11T02:00:00Z';}, + ]){const events=after();mutate(events);expect(autoplanPermissionProgressKey(capture.after,events)).toBe(key);} +}); + +test('missing path authority, mixed sessions and duplicate uses supply no matching progress',()=>{ + expect(autoplanPermissionProgressKey(capture.after,[])).toBeUndefined(); + expect(autoplanPermissionProgressKey(capture.after.replace('overwrite 2026-09-11-user-dashboard.md','overwrite other.md'),after())).toBeUndefined(); + expect(autoplanPermissionProgressKey(capture.after.replace('always allow access to','access to'),after())).toBeUndefined(); + const mixed=after();mixed[3]!.sessionId='other';expect(autoplanPermissionProgressKey(capture.after,mixed)).toBeUndefined(); + const duplicate=after();duplicate.splice(3,0,structuredClone(duplicate[2]!)); + expect(autoplanPermissionProgressKey(capture.after,duplicate)).toBe(autoplanPermissionProgressKey(capture.before,before())); +}); + +test('the actual generic permission branch preserves classification and waits for selection',async()=>{ + const source=fs.readFileSync(new URL('./skill-e2e-autoplan-chain.test.ts',import.meta.url),'utf8'); + const block=source.slice(source.indexOf(' const recentTail = visible.slice(-1500);'),source.indexOf(' // This new repository offers routing')); + expect(block.match(/continue;/g)).toHaveLength(1); + const sends:string[]=[];let release:(()=>void)|undefined; + const select=async()=>{sends.push('selected');await new Promise(r=>{release=r;});sends.push('confirmed');}; + const make=new Function('autoplanPermissionProgressKey','selectPtyNumberedOption','Bun',` + let lastPermSig='',lastPermissionProgress=''; + return async(visible,publicTools,allowed=true)=>{ + const transcript={status:'ready'},session={}; + const isNumberedOptionListVisible=()=>allowed,isPermissionDialogVisible=()=>allowed; + ${block.replace('continue;','return;')} + }; + `); + const step=make(autoplanPermissionProgressKey,select,{sleep:async()=>{}}); + const first=step(capture.before,before());await Promise.resolve();expect(sends).toEqual(['selected']);release!();await first; + await step(capture.after,before());expect(sends).toEqual(['selected','confirmed']); + await step(capture.after,after(),false);expect(sends).toHaveLength(2); // Existing AUQ/permission classification still decides. + const next=step(capture.after,after());await Promise.resolve();expect(sends).toHaveLength(3);release!();await next; + await step(capture.after,after());expect(sends).toEqual(['selected','confirmed','selected','confirmed']); +}); diff --git a/test/ceo-section-choice-ai.test.ts b/test/ceo-section-choice-ai.test.ts index abc983d71..7bb109e14 100644 --- a/test/ceo-section-choice-ai.test.ts +++ b/test/ceo-section-choice-ai.test.ts @@ -1,6 +1,7 @@ import { expect, test } from 'bun:test'; import { ceoFirstReviewAUQ, ceoStep0Boundary, nativePlanCallFingerprint, planCountQuestionPhase } from './helpers/claude-pty-runner'; import captured from './fixtures/ceo-section-choice-ai.json'; +import metadataCaptured from './fixtures/ceo-metadata-brief-ax.json'; import { E2E_TOUCHFILES } from './helpers/touchfiles-data'; function call(index = 4): any { @@ -102,3 +103,126 @@ test('negated gaps and administrative missing fields cannot borrow review identi admin.answers = {[q.question]: q.options[0].label}; expect(ceoFirstReviewAUQ(fp(admin))).toBe(false); }); + +function metadataCall(): any { + const c = structuredClone(metadataCaptured.call); + return { ...c, answered: true, failed: false, unansweredQuestionIndices: [], + answers: { [c.questions[0]!.question]: metadataCaptured.answer } }; +} + +test('AX ordinary D-number question keeps its exact completed review identity', () => { + const c = metadataCall(), before = JSON.stringify(c); + expect(ceoFirstReviewAUQ(fp(c))).toBe(true); + expect(planCountQuestionPhase(fp(c), false, ceoStep0Boundary, ceoFirstReviewAUQ)).toMatchObject({ preReview: false, reviewStarted: true }); + expect(JSON.stringify(c)).toBe(before); + expect(E2E_TOUCHFILES['plan-ceo-finding-count']).toContain('test/fixtures/ceo-metadata-brief-ax.json'); +}); + +test('decision counter, review name and an alternative selection do not dictate the finding', () => { + const c = metadataCall(); edit(c, s => s.replace(/^D5 /, 'D17 ').replace('Section 2 (Error & Rescue Map)', 'Section 3 (Failure Handling)')); + c.answers[c.questions[0].question] = c.questions[0].options[1].label; + expect(ceoFirstReviewAUQ(fp(c))).toBe(true); + edit(c, s => s + '\nHistorical quote: "This finding is withdrawn."'); + expect(ceoFirstReviewAUQ(fp(c))).toBe(true); +}); + +test('metadata cannot replace native completion, current context or a real defect', () => { + for (const mutate of [ + (c: any) => { c.answered = false; }, + (c: any) => { c.failed = true; }, + (c: any) => { c.answeredAt = 'not a date'; }, + (c: any) => { c.unansweredQuestionIndices = [0]; }, + (c: any) => { c.answers = { other: metadataCaptured.answer }; }, + (c: any) => { c.questions[0].header = 'Setup'; }, + (c: any) => { c.questions[0].header = 'Section 9'; }, + (c: any) => edit(c, s => s.replace('Section 2 (Error & Rescue Map)', 'Section 2 (Error & Rescue Map), Section 3 (Security)')), + (c: any) => edit(c, s => s.replace('of the CEO review', 'of an earlier CEO review')), + (c: any) => edit(c, s => s.replace(/^Project\/branch\/task: (.*)$/m, 'Project/branch/task: If approved, $1')), + (c: any) => edit(c, s => s.replace(/^Project\/branch\/task:.*\n/m, '')), + (c: any) => edit(c, s => s.replace(/^ELI10: (.*)$/m, 'ELI10: "$1"')), + (c: any) => edit(c, s => s.replace(/^ELI10: .+$/m, 'ELI10: The handler has no current defect and needs no amendment.')), + (c: any) => edit(c, s => s.replace(/^ELI10: .+$/m, 'ELI10: The handler commits before mail and already rescues every required error.')), + (c: any) => edit(c, s => s.replace('ELI10: After', 'ELI10: Hypothetical example: after')), + (c: any) => edit(c, s => s + '\nThis finding is withdrawn.'), + (c: any) => edit(c, s => s + '; This finding is `no longer current`.'), + (c: any) => edit(c, s => s + '\nThis finding is unproven.'), + ]) { + const c = metadataCall(); mutate(c); expect(ceoFirstReviewAUQ(fp(c))).toBe(false); + } + expect(ceoFirstReviewAUQ({ ...fp(metadataCall()), signature: 'foreign:call' })).toBe(false); +}); + +test('a missing or withdrawn offered remedy cannot borrow metadata or an old assessment', () => { + for (const mutate of [ + (c: any) => { c.questions[0].options = [{ label: 'A: Keep the current handler', description: 'No code change.' }, { label: 'B: Save the review notes', description: 'Archive the current report.' }]; c.answers = { [c.questions[0].question]: c.questions[0].options[0].label }; }, + (c: any) => { c.questions[0].options[0].description += '; This option is `withdrawn`.'; c.questions[0].options[2].description += '\nThis option is withdrawn.'; }, + (c: any) => { c.questions[0].options.forEach((o: any) => { o.description = 'Hypothetical example. ' + o.description; }); }, + ]) { + const c = metadataCall(); mutate(c); expect(ceoFirstReviewAUQ(fp(c))).toBe(false); + } +}); + +function metadataRetryCall(): any { + const c = structuredClone(metadataCaptured.retry.call); + return { ...c, answered: true, failed: false, unansweredQuestionIndices: [], + answers: { [c.questions[0]!.question]: metadataCaptured.retry.answer } }; +} + +test('the separately failed AX retry binds its Issue annotation, bare choices and named plan', () => { + const c = metadataRetryCall(), before = JSON.stringify(c); + expect(ceoFirstReviewAUQ(fp(c))).toBe(true); + expect(planCountQuestionPhase(fp(c), false, ceoStep0Boundary, ceoFirstReviewAUQ)) + .toMatchObject({ preReview: false, reviewStarted: true }); + expect(JSON.stringify(c)).toBe(before); + c.answers[c.questions[0].question] = c.questions[0].options[2].label; + expect(ceoFirstReviewAUQ(fp(c))).toBe(true); +}); + +test('a reviewed filename and section identity can be consistently renamed', () => { + const c = metadataRetryCall(); + edit(c, s => s.replace(/^D4 /, 'D12 ').replace(/Issue 2\.1/, 'Issue 8.3') + .replace('Section 2 (Error & Rescue Map)', 'Section 8 (Notification Handling)') + .replace(/PLAN\.md/g, 'plans/checkout-flow.md')); + expect(ceoFirstReviewAUQ(fp(c))).toBe(true); + edit(c, s => s + '\nHistorical quote: "This issue is withdrawn."'); + expect(ceoFirstReviewAUQ(fp(c))).toBe(true); + edit(c, s => s.replace(/\s*]+>/, '')); + expect(ceoFirstReviewAUQ(fp(c))).toBe(true); +}); + +test('retry metadata cannot borrow a foreign section, plan, source or incomplete native call', () => { + for (const [index, mutate] of [ + (c: any) => { c.answered = false; }, + (c: any) => { c.failed = true; }, + (c: any) => { c.answeredAt = 'unknown'; }, + (c: any) => { c.unansweredQuestionIndices = [0]; }, + (c: any) => { c.questions[0].header = 'Issue 8.1'; }, + (c: any) => edit(c, s => s.replace('Issue 2.1', 'Issue 3.1')), + (c: any) => edit(c, s => s.replace('Section 2 (Error & Rescue Map)', 'Section 3 (Security)')), + (c: any) => edit(c, s => s.replace('CEO review of PLAN.md,', 'CEO review of DIFFERENT.md,')), + (c: any) => edit(c, s => s.replace("PLAN.md says 'no error handling on the email leg'", "OTHER.md says 'no error handling on the email leg'")), + (c: any) => edit(c, s => s.replace('CEO review of PLAN.md,', 'Historical CEO review of PLAN.md,')), + (c: any) => edit(c, s => s.replace(/^Project\/branch\/task: (.+)$/m, 'Project/branch/task: If approved, $1')), + (c: any) => edit(c, s => s.replace(/^ELI10: (.+)$/m, 'ELI10: "$1"')), + (c: any) => edit(c, s => s.replace('ELI10: The handler', 'ELI10: Source excerpt: the handler')), + (c: any) => edit(c, s => s.replace('PLAN.md says', 'If approved, PLAN.md says')), + (c: any) => edit(c, s => s.replace('PLAN.md says', 'PLAN.md does not say')), + (c: any) => edit(c, s => s.replace('plan-ceo-review-mail-rescue', 'plan-ceo-review-setup')), + (c: any) => edit(c, s => s + '\n'), + ].entries()) { + const c = metadataRetryCall(); mutate(c); expect(ceoFirstReviewAUQ(fp(c)), `retry mutation ${index}`).toBe(false); + } +}); + +test('current withdrawal and a withdrawn offered amendment override the retry brief', () => { + for (const change of [ + (s: string) => s + '\nThis issue is withdrawn.', + (s: string) => s + '; This finding is `no longer current`.', + (s: string) => s + '\nIssue 2.1 is withdrawn.', + (s: string) => s.replace(/^ELI10: .+$/m, 'ELI10: The handler has no current defect and needs no amendment.'), + ]) { + const c = metadataRetryCall(); edit(c, change); expect(ceoFirstReviewAUQ(fp(c))).toBe(false); + } + const c = metadataRetryCall(); c.questions[0].options[0].description += '; This option is `withdrawn`.'; + expect(ceoFirstReviewAUQ(fp(c))).toBe(false); +}); diff --git a/test/disabled-plan-review-evidence.test.ts b/test/disabled-plan-review-evidence.test.ts index 4c18ede5f..0d4936de1 100644 --- a/test/disabled-plan-review-evidence.test.ts +++ b/test/disabled-plan-review-evidence.test.ts @@ -372,3 +372,40 @@ describe('AW explicitly dated historical record attribution', () => { expect(oracle(result)).toMatchObject({ passed: false, falseCompletion: true }); }); }); + +describe('AX pre-run log record with an explicit current-coverage exclusion', () => { + // Exact public first-attempt paragraph; full native transcript stays in ignored evidence. + const historical = '- **A pre-existing log entry claims completed Codex coverage.** The review log already held a record timestamped about two minutes before this run marking a `clean` Codex plan review with `outside_status: completed`. That entry predates this workflow and was not produced by it. Under the current disabled config it should not be read as outside coverage for this plan. I left it in place and did not edit it.'; + const evaluate = (text: string) => oracle({ ...completed(), output: completed().output + '\n' + text }); + test('keeps the exact dated value historical and all disabled execution gates active', () => { + expect(evaluate(historical)).toMatchObject({ passed: true, falseCompletion: false }); + const result = { ...completed(), output: completed().output + '\n' + historical }; + result.transcript.splice(-1, 0, dispatch('Agent', { prompt: 'Review anyway' })); + expect(oracle(result).passed).toBe(false); + expect(oracle({ ...completed(), output: result.output }, 'codex invoked\n').passed).toBe(false); + }); + test('accepts equivalent record nouns, review identity, and explicit coverage exclusion', () => { + for (const text of [historical.replace('held a record', 'contained an entry').replace('Codex plan review', 'Claude plan review'), + historical.replace('That entry predates this workflow and was not produced by it.', 'That entry does not reflect outside coverage from this workflow.')]) { + expect(evaluate(text).passed).toBe(true); + } + }); + test('requires dated record ownership and its own explicit non-current conclusion', () => { + for (const text of [ + historical.replace('before this run', 'after this run'), + historical.replace('The review log already held a record', 'The reviewer now creates a record'), + historical.replace('timestamped about two minutes before this run ', ''), + historical.replace('That entry predates this workflow and was not produced by it.', 'The other entry predates this workflow and was not produced by it.'), + historical.replace('That entry predates this workflow and was not produced by it.', ''), + historical.replace('marking a `clean` Codex plan review with', 'and we now report'), + ]) expect(evaluate(text).falseCompletion).toBe(true); + }); + test('checks each current completion claim even beside a valid historical record', () => { + for (const text of [historical + '\nCurrent outside_status: completed.', + historical + ' Both reviewers agree.', + historical.replace('`outside_status: completed`.', '`outside_status: completed`, and we report outside_status: completed.'), + historical.replace('That entry predates', 'Outside_status: completed. That entry predates')]) { + expect(evaluate(text).falseCompletion).toBe(true); + } + }); +}); diff --git a/test/eng-owned-explanation.test.ts b/test/eng-owned-explanation.test.ts new file mode 100644 index 000000000..4bd9f3a0b --- /dev/null +++ b/test/eng-owned-explanation.test.ts @@ -0,0 +1,215 @@ +import { expect, test } from 'bun:test'; +import { evaluateEngSeedCoverage } from './helpers/eng-seeded-coverage'; +import { E2E_TOUCHFILES } from './helpers/touchfiles'; +import type { NativePlanQuestion, NativePlanQuestionCall } from './helpers/plan-count-transcript'; +import fixture from './fixtures/eng-owned-explanation.json'; + +const seeds = ['complexity', 'swallowed-errors', 'sequential-idp'] as const; +const evaluate = (calls: NativePlanQuestionCall[] = [], plan = '') => + evaluateEngSeedCoverage({ status: 'ready', calls, assistantMessages: [] }, plan, 0, 10); +function decision(i: number, change: (q: NativePlanQuestion) => void = () => {}) { + const q = structuredClone(fixture.questions[i]!) as NativePlanQuestion; + change(q); + return { sessionId: 'one-session', toolUseId: `decision-${i}`, questions: [q], + answered: true, failed: false, unansweredQuestionIndices: [], answeredAt: new Date(5).toISOString(), + answers: { [q.question]: q.options[0]!.label } } satisfies NativePlanQuestionCall; +} + +test.each([0, 1, 2])('a current explanation and one concrete offered repair own seed %i', i => { + expect(evaluate([decision(i)]).decisions[seeds[i]!]).toBe(`one-session:decision-${i}`); +}); + +test.each([0, 1, 2])('equivalent title, ordinal, formatting and offered answer preserve seed %i', i => { + const titles = ['Which auth components should we retain?', 'Which error boundary should validateAndDispatch() use?', 'How should we schedule the IDP calls?']; + const c = decision(i, q => { q.question = q.question.replace(/^D\d+ — .*$/m, `D17 — ${titles[i]}`); }); + for (const option of c.questions[0]!.options) { + c.answers[c.questions[0]!.question] = option.label; + expect(evaluate([c]).decisions[seeds[i]!]).toBeDefined(); + } + expect(evaluate([decision(i, q => { q.question = q.question.replaceAll('validateAndDispatch()', '`validateAndDispatch()`'); })]).decisions[seeds[i]!]).toBeDefined(); + expect(evaluate([decision(i, q => { q.question = q.question.replace('ELI10: ', '[P1] Current finding\nELI10: '); })]).decisions[seeds[i]!]).toBeDefined(); +}); + +const questionControls: Array<[string, (s: string) => string]> = [ + ['missing metadata', s => s.replace(/^Project\/branch\/task:.*\n/m, '')], + ['missing explanation', s => s.replace(/^ELI10:.*\n/m, '')], + ['unrelated explanation', s => s.replace(/^ELI10:.*$/m, 'ELI10: This asks about naming conventions.')], + ['duplicate explanation', s => s + '\nELI10: Another finding.'], + ['duplicate metadata', s => s + '\nProject/branch/task: a different task.'], + ['two severity lines', s => s.replace('ELI10: ', '[P1] First finding\n[P2] Second finding\nELI10: ')], + ['source severity line', s => s.replace('ELI10: ', '[P1] Source: borrowed priority\nELI10: ')], + ['quoted explanation', s => s.replace(/^ELI10: (.*)$/m, 'ELI10: "$1"')], + ['literal explanation', s => s.replace(/^ELI10: (.*)$/m, 'ELI10: `$1`')], + ['source explanation', s => s.replace('ELI10: ', 'ELI10: Source: ')], + ['copied source explanation', s => s.replace('ELI10: ', 'ELI10: Copied source excerpt. ')], + ['historical metadata', s => s.replace('Project/branch/task: ', 'Project/branch/task: Historical assessment. ')], + ['quoted title', s => s.replace(/^(D\d+ — )(.*)$/m, '$1"$2"')], + ['literal title', s => s.replace(/^(D\d+ — )(.*)$/m, '$1`$2`')], + ['historical title', s => s.replace(/^(D\d+ — )/m, '$1Historical: ')], + ['conditional explanation', s => s.replace('ELI10: ', 'ELI10: If approved, ')], + ['withdrawn finding', s => s + '\nCorrection: This finding is withdrawn.'], + ['quoted inactive status', s => s + '\nThis finding is "withdrawn".'], + ['whole quotation', s => s.split('\n').map(l => '> ' + l).join('\n')], + ['whole fence', s => '```\n' + s + '\n```'], + ['defect only in stakes', s => s.replace(/^ELI10: (.*)$/m, 'ELI10: We are considering names.\nStakes: $1')], +]; +test.each(questionControls)('%s cannot supply an explained decision', (_, change) => { + for (let i = 0; i < seeds.length; i++) { + const c = decision(i, q => { const before = q.question; q.question = change(before); expect(q.question).not.toBe(before); }); + expect(evaluate([c]).decisions[seeds[i]!]).toBeUndefined(); + } +}); + +test.each([0, 1, 2])('repair ownership and native completion remain required for seed %i', i => { + for (const wrap of [(s: string) => `Source: ${s}`, (s: string) => `"${s}"`, (s: string) => `${s}\nThis option is withdrawn.`]) { + const c = decision(i, q => { q.options = q.options.map(o => ({ label: wrap(o.label), description: wrap(o.description ?? '') })); }); + expect(evaluate([c]).decisions[seeds[i]!]).toBeUndefined(); + } + const c = decision(i); c.answered = false; + expect(evaluate([c]).decisions[seeds[i]!]).toBeUndefined(); + const splitRepairs = [ + [{ label: 'Reduce scope', description: 'Remove extra components.' }, { label: 'Keep shape', description: 'Keep AuthBroker, SessionMint and injected AuthCache over one backing store.' }], + [{ label: 'Flatten into named helpers', description: 'Use a typed boundary.' }, { label: 'Keep nesting', description: 'One catch maps errors to 401 and rethrows unknowns.' }], + [{ label: 'Promise.all', description: 'Discuss a name.' }, { label: 'Keep request schedule', description: 'Five calls with cancellation of siblings.' }], + ]; + expect(evaluate([decision(i, q => { q.options = splitRepairs[i]!; })]).decisions[seeds[i]!]).toBeUndefined(); +}); + +test('a combined native decision cannot supply three distinct seed decisions', () => { + const c = decision(0); + c.questions = [0, 1, 2].map(i => decision(i).questions[0]!); + c.answers = Object.fromEntries(c.questions.map(q => [q.question, q.options[0]!.label])); + expect(evaluate([c]).decisions).toEqual({}); +}); + +// Minimal mandatory declaration and T1 task excerpt from the acknowledged final +// report. The complete retained report is used only for a private local replay. +const report = `# Current reviewed plan +### REGRESSION RULE (mandatory, no decision required) + +**CRITICAL — T1:** write characterization (golden) tests for \`legacyAuthFlow()\` +BEFORE any rewrite. Corpus: valid token, expired token, revoked token, wrong +audience, wrong issuer, unknown tenant, suspended tenant, policy version +mismatch, malformed token, cache hit vs miss. Run the same corpus against the +\`AuthBroker\` path. Both must produce identical results before the flag opens to +any tenant. This test is authorized by the regression rule itself. + +## Implementation Tasks +- [ ] **T1 (P1, human: ~1 day / CC: ~30 min)** — legacyAuthFlow — Write characterization (golden) tests for \`legacyAuthFlow()\` before touching it + - Surfaced by: Test review — REGRESSION RULE, PLAN.md:27-28 + - Files: test/auth/legacy-auth-flow.characterization.test.ts + - Verify: suite passes against legacy; later passes unchanged against AuthBroker +`; + +test('mandatory declaration and unique task bind the old baseline to unchanged new-path parity', () => { + expect(evaluate([], report).regression).toBe('plan'); + for (const change of [ + (s: string) => s.replaceAll('T1', 'T17'), + (s: string) => s.replace('Corpus: ', 'T1 is mandatory. Corpus: '), + (s: string) => s.replaceAll('test/auth/legacy-auth-flow.characterization.test.ts', 'spec/compatibility.test.js'), + (s: string) => s.replaceAll('AuthBroker', 'ReplacementBroker'), + (s: string) => s.replace('REGRESSION RULE (mandatory, no decision required)', 'Required characterization (mandatory)').replace('Implementation Tasks', 'Execution checklist'), + (s: string) => s.replace('Run the same corpus against the', 'Replay the same corpus through the').replace('produce identical results', 'return matching outputs').replace('later passes unchanged against', 'then is green unchanged on'), + (s: string) => s + '\n## History\nT1 is withdrawn.\n', + (s: string) => s + '\n## Current assessment\n"T1 is withdrawn."\n', + (s: string) => s + '\n## Payment regression suite\nThe regression suite is withdrawn.\n', + ]) expect(evaluate([], change(report)).regression).toBe('plan'); +}); + +const regressionControls: Array<[string, (s: string) => string]> = [ + ['missing declaration', s => s.replace(/### [\s\S]*?(?=## Implementation Tasks)/, '')], + ['optional declaration', s => s.replace('mandatory', 'optional')], + ['conditional declaration', s => s.replace('mandatory', 'mandatory if approved')], + ['missing legacy subject', s => s.replaceAll('legacyAuthFlow', 'otherAuthFlow')], + ['late declaration baseline', s => s.replace('BEFORE any rewrite', 'AFTER any rewrite')], + ['late task baseline', s => s.replace('before touching it', 'after touching it')], + ['missing task', s => s.replace(/- \[ \][\s\S]*/, '')], + ['wrong task ID', s => s.replace('**T1 (P1', '**T9 (P1')], + ['distinct declaration task IDs', s => s.replace('Corpus: ', 'T9 owns this corpus. Corpus: ')], + ['duplicate task ID', s => s + s.slice(s.indexOf('- [ ]'))], + ['duplicate declaration', s => s + s.slice(s.indexOf('### '), s.indexOf('## Implementation Tasks'))], + ['missing file', s => s.replace(/^ - Files:.*\n/m, '')], + ['two files', s => s.replace('.test.ts', '.test.ts, test/other.test.ts')], + ['conflicting declared file', s => s.replace('Corpus: ', 'File: test/other.test.ts. Corpus: ')], + ['missing verification', s => s.replace(/^ - Verify:.*\n/m, '')], + ['foreign baseline', s => s.replace('passes against legacy;', 'passes against otherAuthFlow;')], + ['foreign rewrite', s => s.replace('unchanged against AuthBroker', 'unchanged against OtherBroker')], + ['ambiguous rewrite', s => s.replace('Both must', 'Run the same corpus against OtherBroker path. Both must')], + ['new suite', s => s.replace('passes unchanged against', 'passes after updating expectations against')], + ['missing parity', s => s.replace('Both must produce identical results', 'Both produce different results')], + ['different corpus', s => s.replace('Run the same corpus', 'Run a new corpus')], + ['quoted declaration', s => s.replace(/(### [^\n]+\n)([\s\S]*?)(?=\n## Implementation Tasks)/, '$1"$2"')], + ['fenced report', s => '```\n' + s + '\n```'], + ['historical report', s => s.replace('Current reviewed plan', 'Historical reviewed plan')], + ['source declaration', s => s.replace('**CRITICAL', 'Source:\n**CRITICAL')], + ['conditional task', s => s.replace('## Implementation Tasks\n', '## Implementation Tasks\nOnce approved,\n')], + ['source task', s => s.replace('## Implementation Tasks\n', '## Implementation Tasks\nSource:\n')], + ['withdrawn task', s => s + '\n## Current assessment\nT1 is withdrawn.\n'], + ['cancelled verification', s => s + '\n## Current assessment\nT1 verification is optional.\n'], + ['quoted status', s => s + '\n## Current assessment\nT1 is "withdrawn".\n'], + ['changed before baseline', s => s + '\n## Current assessment\nlegacyAuthFlow() is rewritten before T1.\n'], + ['late baseline correction', s => s + '\n## Current assessment\nRun T1 only after rewriting legacyAuthFlow().\n'], + ['changed expectations', s => s + '\n## Current assessment\nUpdate T1 expectations to match the new path.\n'], + ['changed suite expectations', s => s + '\n## Current assessment\nThe legacy regression suite expectations will be updated to match the new path.\n'], +]; +test.each(regressionControls)('%s cannot supply the linked baseline', (_, change) => { + const changed = change(report); + expect(changed).not.toBe(report); + expect(evaluate([], changed).regression).toBeUndefined(); +}); + +// The retry states the unchanged-code baseline in its file-bound task's Verify +// line. Retain only that task and its mandatory declaration, not the full report. +const retryReport = `# Current reviewed plan +### CRITICAL regression (mandatory, regression rule) +\`legacyAuthFlow()\` is existing behavior being rewritten with no test of its prior behavior (PLAN.md:27-28). +Add \`test/auth/legacyAuthFlow.characterization.test.ts\` pinning current outputs for: valid token, expired token, +revoked token, wrong audience, wrong issuer, suspended tenant, malformed token. It must pass before and after +this refactor, and the shadow compare asserts \`AuthBroker\` agrees with it. + +## Implementation Tasks +- [ ] **T2 (P1, human: ~4h / CC: ~10 min)** — legacyAuthFlow — Characterization suite pinning prior behavior (CRITICAL regression) + - Surfaced by: Test review, regression rule — PLAN.md:27-28 + - Files: test/auth/legacyAuthFlow.characterization.test.ts + - Verify: suite passes on main before any refactor commit, and after +`; + +test('a file-bound task can carry its own pre-commit baseline and retained-output parity', () => { + for (const change of [ + (s: string) => s, + (s: string) => s.replaceAll('T2', 'T19').replaceAll('AuthBroker', 'ReplacementBroker'), + (s: string) => s.replaceAll('test/auth/legacyAuthFlow.characterization.test.ts', 'spec/compatibility.test.js'), + (s: string) => s.replace('Implementation Tasks', 'Execution checklist'), + (s: string) => s.replace('passes on main before any refactor commit', 'green against the untouched code before the rewrite commit').replace('asserts', 'verifies').replace('agrees with', 'matches'), + (s: string) => s + '\n## Payment regression suite\nThe regression suite expectations will be updated.\n', + ]) expect(evaluate([], change(retryReport)).regression).toBe('plan'); +}); + +test.each([ + ['missing capture', (s: string) => s.replace('pinning current outputs', 'describing current outputs')], + ['future outputs', (s: string) => s.replace('pinning current outputs', 'pinning new outputs')], + ['missing required parity', (s: string) => s.replace('asserts `AuthBroker` agrees with it', 'describes AuthBroker')], + ['foreign file', (s: string) => s.replace(' - Files: test/auth/legacyAuthFlow.characterization.test.ts', ' - Files: test/other.test.ts')], + ['ambiguous file task', (s: string) => s + s.slice(s.indexOf('- [ ]')).replace('T2', 'T19')], + ['ambiguous parity target', (s: string) => s.replace('agrees with it.', 'agrees with it. It also asserts OtherBroker matches it.')], + ['unbound branch baseline', (s: string) => s.replace('on main', 'on the rewritten branch')], + ['baseline after refactor commit', (s: string) => s.replace('main before any refactor commit', 'main after any refactor commit')], + ['baseline before rollout only', (s: string) => s.replace('refactor commit', 'rollout commit')], + ['missing after check', (s: string) => s.replace(', and after', '')], + ['source task', (s: string) => s.replace('## Implementation Tasks\n', '## Implementation Tasks\nSource:\n')], + ['conditional verification', (s: string) => s.replace('Verify: ', 'Verify: If approved, ')], + ['withdrawn task', (s: string) => s + '\n## Current assessment\nT2 is withdrawn.\n'], + ['rewritten before baseline', (s: string) => s + '\n## Current assessment\nlegacyAuthFlow() is rewritten before T2.\n'], + ['changed expectations', (s: string) => s + '\n## Current assessment\nUpdate T2 expectations to match the new path.\n'], +] as Array<[string, (s: string) => string]>)('%s cannot provide a pre-commit baseline', (_, change) => { + const changed = change(retryReport); + expect(changed).not.toBe(retryReport); + expect(evaluate([], changed).regression).toBeUndefined(); +}); + +test('the explanation regression selects the existing Eng finding-count workflow', () => { + for (const path of ['test/eng-owned-explanation.test.ts', 'test/fixtures/eng-owned-explanation.json']) { + expect(Object.entries(E2E_TOUCHFILES).filter(([, paths]) => paths.includes(path)).map(([name]) => name)) + .toEqual(['plan-eng-finding-count']); + } +}); diff --git a/test/fixtures/autoplan-overwrite-progress-ax.json b/test/fixtures/autoplan-overwrite-progress-ax.json new file mode 100644 index 000000000..8383129ea --- /dev/null +++ b/test/fixtures/autoplan-overwrite-progress-ax.json @@ -0,0 +1,61 @@ +{ + "qualification": "Exact public 92-line and 106-line permission screens plus content-free successful Write identity pairs. Payloads removed; only fields used for progress correlation retained. Original AX cancellation remains a failure.", + "before": " +. \n 78 +- Server snapshot-time validation; client same-origin route validation; text-only rendering of notification/activ\n +ity content. \n 79 +- Observability (handler logs, metrics, Server-Timing, client events, alerts, day-1 dashboard, runbook), staged f\n +lag rollout with flag-off rollback, experiment design with production baseline (sized as one P1 task, human ~4 h \n +/ CC ~30 min). \n 80 \n 81 ## Deferred to TODOS.md\n 82 - Next-item hero card \u2014 needs assigned-items query; revisit after the experiment reads out.\n ...\n 85 - Cache action eligibility per member \u2014 only if Server-Timing shows predicates > 100 ms p95.\n 86 - Remove old landing redirect and flag branch \u2014 after the experiment decides 100%.\n 87 \n 48 -## Open at the Final Gate (not decided here) \n 49 -- Challenged premise: should a redirect-to-single-next-item ship as v0 before or instead of the dashboard? (Nativ\n -e subagent argues yes; primary review keeps the user's direction because the plan names three member jobs.) \n 50 -- Approach A (client-parallel to existing endpoints) vs Approach C (aggregate envelope). Recommended C. \n 51 -- Keep the confirmation modal for mark-all-read (recommended, one-way action) vs direct action. \n 52 -- QuickActions-first hierarchy (recommended) vs equal panels. \n 88 +## Open at the Final Gate (provisional decisions, not settled here) \n 89 +1. **Challenged premise:** ship a redirect-to-single-next-item as v0 before or instead of the dashboard? The nati\n +ve subagent argues yes. The primary review keeps the user's direction because the plan names three member jobs, a\n +nd because the redirect needs the same assigned-items data source that deferred proposal 9 lacks, so it is not ch\n +eaper than it looks for this release. The user decides. \n 90 +2. Approach A (client-parallel to existing endpoints, 7/10) vs Approach C (aggregate envelope, 10/10). Recommende\n +d C; accepted scope above assumes C. \n 91 +3. Keep the confirmation modal for mark-all-read (recommended: one-way action, no unread-restore API) vs direct a\n +ction with live-region confirmation. \n 92 +4. QuickActions-first hierarchy (recommended) vs equal panels. \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 Do you want to overwrite 2026-09-11-user-dashboard.md?\n \u276f 1. Yes\n 2. Yes, and switch to accept edits (auto-approve file edits and common file commands) for this session; Yes, and\n always allow access to /tmp/gstack-paid-shard-OALVx0/tmp/gstack-native-review-state-K0cbcP/projects/gstack-autopla\n n-chain-fviI7n/ceo-plans for this session (shift+tab)\n\n 3. No\n\n Esc to cancel \u00b7 Tab to amend\n", + "after": " +th \"Updated X ago\", CTA empty states, landmark sections, independently mountable panels (no registry; static orde\n +r). \n 89 - App-root toast primitive as specified above, no theming/positioning API in v1.\n 90 - Mark-all-read confirmation dialog as specified above (**provisional: keep modal vs direct action at the gate**)\n .\n 91 - Server snapshot-time validation; client same-origin route validation; text-only rendering of notification/activ\n ity content.\n ...\n 95 - Next-item hero card \u2014 needs assigned-items query; revisit after the experiment reads out.\n 96 - Optimistic mark-all-read \u2014 after v1 ships; measure whether the confirm\u2192toast latency is noticed.\n 97 - Prefetch dashboard data on login \u2014 login-flow change; separate PR.\n 85 -- Cache action eligibility per member \u2014 only if Server-Timing shows predicates > 100 ms p95. \n 98 +- Cache action eligibility per member \u2014 only if `Server-Timing` `quickActions;dur` shows > 100 ms p95. \n 99 +- Server-side `unreadCount` \u2014 when `20+` proves insufficient or the notifications repo gains a count method. \n 100 - Remove old landing redirect and flag branch \u2014 after the experiment decides 100%.\n 101 \n 88 -## Open at the Final Gate (provisional decisions, not settled here) \n 102 +## Open at the Final Gate (three provisional scope decisions + one challenged premise) \n 103 1. **Challenged premise:** ship a redirect-to-single-next-item as v0 before or instead of the dashboard? The nat\n ive subagent argues yes. The primary review keeps the user's direction because the plan names three member jobs,\n and because the redirect needs the same assigned-items data source that deferred proposal 9 lacks, so it is not\n cheaper than it looks for this release. The user decides.\n 90 -2. Approach A (client-parallel to existing endpoints, 7/10) vs Approach C (aggregate envelope, 10/10). Recommend\n -ed C; accepted scope above assumes C. \n 104 +2. Approach A (client-parallel to existing endpoints, 7/10) vs Approach C (aggregate envelope, 10/10). Recommend\n +ed C; the Contracts section assumes C and is rewritten first if A wins. \n 105 3. Keep the confirmation modal for mark-all-read (recommended: one-way action, no unread-restore API) vs direct\n action with live-region confirmation.\n 106 4. QuickActions-first hierarchy (recommended) vs equal panels.\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 Do you want to overwrite 2026-09-11-user-dashboard.md?\n \u276f 1. Yes\n 2. Yes, and switch to accept edits (auto-approve file edits and common file commands) for this session; Yes, and\n always allow access to /tmp/gstack-paid-shard-OALVx0/tmp/gstack-native-review-state-K0cbcP/projects/gstack-autopla\n n-chain-fviI7n/ceo-plans for this session (shift+tab)\n\n 3. No\n\n Esc to cancel \u00b7 Tab to amend\n", + "beforeEvents": [ + { + "sessionId": "53d424c8-2e93-47fb-93d6-9adbbb763a6f", + "timestamp": "2026-09-11T02:25:05.912Z", + "toolUseId": "toolu_01RBorP8UERrbVheRiXSN1v4", + "kind": "use", + "name": "Write", + "input": { + "file_path": "/tmp/gstack-paid-shard-OALVx0/tmp/gstack-native-review-state-K0cbcP/projects/gstack-autoplan-chain-fviI7n/ceo-plans/2026-09-11-user-dashboard.md" + } + }, + { + "sessionId": "53d424c8-2e93-47fb-93d6-9adbbb763a6f", + "timestamp": "2026-09-11T02:25:08.955Z", + "toolUseId": "toolu_01RBorP8UERrbVheRiXSN1v4", + "kind": "result", + "isError": false + } + ], + "afterEvents": [ + { + "sessionId": "53d424c8-2e93-47fb-93d6-9adbbb763a6f", + "timestamp": "2026-09-11T02:25:05.912Z", + "toolUseId": "toolu_01RBorP8UERrbVheRiXSN1v4", + "kind": "use", + "name": "Write", + "input": { + "file_path": "/tmp/gstack-paid-shard-OALVx0/tmp/gstack-native-review-state-K0cbcP/projects/gstack-autoplan-chain-fviI7n/ceo-plans/2026-09-11-user-dashboard.md" + } + }, + { + "sessionId": "53d424c8-2e93-47fb-93d6-9adbbb763a6f", + "timestamp": "2026-09-11T02:25:08.955Z", + "toolUseId": "toolu_01RBorP8UERrbVheRiXSN1v4", + "kind": "result", + "isError": false + }, + { + "sessionId": "53d424c8-2e93-47fb-93d6-9adbbb763a6f", + "timestamp": "2026-09-11T02:28:24.850Z", + "toolUseId": "toolu_01RPGbV4z5AAMcnzwD4qcx9p", + "kind": "use", + "name": "Write", + "input": { + "file_path": "/tmp/gstack-paid-shard-OALVx0/tmp/gstack-native-review-state-K0cbcP/projects/gstack-autoplan-chain-fviI7n/ceo-plans/2026-09-11-user-dashboard.md" + } + }, + { + "sessionId": "53d424c8-2e93-47fb-93d6-9adbbb763a6f", + "timestamp": "2026-09-11T02:28:26.532Z", + "toolUseId": "toolu_01RPGbV4z5AAMcnzwD4qcx9p", + "kind": "result", + "isError": false + } + ], + "sourceFrameReplaySha256": "b238735abcfecdd9912ff9ded124dcdb1321477dd7b0394afbc918aa75465c01" +} diff --git a/test/fixtures/ceo-metadata-brief-ax.json b/test/fixtures/ceo-metadata-brief-ax.json new file mode 100644 index 000000000..c49d832d0 --- /dev/null +++ b/test/fixtures/ceo-metadata-brief-ax.json @@ -0,0 +1,68 @@ +{ + "provenance": { + "observationSha256": "778f100e21ef21ed2d51713d6908808641a096395cda1c5bda3691e8b162a127", + "fingerprintIndex": 4, + "projection": "Exact public question/options and completed native identity; selected answer stored as a label to avoid duplicating the whole question." + }, + "call": { + "sessionId": "a2101dd9-4ba9-4e9a-8f31-375e5a2c75d1", + "toolUseId": "toolu_01EnfvoE3MzCMtZnVPhV85ub", + "questions": [ + { + "question": "D5 — How should the handler treat a failed receipt email after the payment update?\nProject/branch/task: gstack-plan-count-9V0cjz on main, Section 2 (Error & Rescue Map) of the CEO review.\nELI10: After marking the user paid, the handler sends one receipt email through the shared mail client. The plan says 'no error handling on the email leg'. The mail client already gives up after one second, records the failed attempt for on-call to retry, and the provider refuses to send the same receipt twice. So when the email fails, the only question is what the handler does next: crash the whole webhook (Stripe sees a 500 and retries for up to 72 hours) or catch the two named mail exceptions, log them with the event and PaymentIntent ids, and return 200 because the payment is committed and the retry record exists. There is also an ordering bug: if the send runs inside the database transaction, a mail failure rolls back the payment. Stakes if we pick wrong: users who paid stay unpaid, or on-call gets a 500 storm for what is really a mail outage.\nRecommendation: A because the retained contracts already make the mail leg durable and idempotent, so the handler's job is to make the failure visible without breaking the committed payment. Engineering preference: every error has a name, zero silent failures.\nCompleteness: A=10/10, B=5/10, C=3/10\nNet: named rescue after commit versus letting a notification failure masquerade as a payment failure.", + "header": "Email rescue", + "multiSelect": false, + "options": [ + { + "label": "A: Commit, then send, rescue named errors (recommended)", + "description": "Completeness 10/10. human ~2 hr / CC ~10 min.\n✅ Update commits first; the receipt send runs after commit so a mail failure can never roll back a payment\n✅ Rescue exactly MailTimeout and the shared client's delivery-error class; log at error level with event id, user id, PaymentIntent id, handler identity, exception class; the existing failure-rate counter and retry record already fire; return normally so ingress acks 200\n✅ Any other exception class still propagates (no rescue StandardError), so unknown failures stay loud\n✅ Tests: send raises MailTimeout -> user row is paid, log line present, 200 returned; send raises an unrelated class -> 500\n❌ One rescue block and two test cases to write" + }, + { + "label": "B: Let mail errors propagate (plan as written)", + "description": "Completeness 5/10. human ~0 / CC ~0.\n✅ No new code on the email leg\n✅ Failure is at least visible through the existing failed-webhook alert\n❌ Notification failures surface as payment webhook failures; Stripe retries up to 72 hours and the alert misattributes the cause\n❌ Transaction boundary stays unstated, so a mail failure inside the transaction rolls back a real payment" + }, + { + "label": "C: Rescue StandardError around the send", + "description": "Completeness 3/10. human ~30 min / CC ~3 min.\n✅ Nothing on the email leg can ever fail the webhook\n✅ Trivial to write\n❌ Catch-all swallows programming errors (nil template, bad argument) as if they were provider outages; the runbook retries them forever\n❌ Violates the 'every error has a name' preference" + } + ] + } + ], + "answeredAt": "2026-09-11T02:19:44.965Z" + }, + "answer": "A: Commit, then send, rescue named errors (recommended)", + "retry": { + "provenance": { + "observationSha256": "dc60c557c6917cfedbe5a6889cfd37a211a6077ff9d800e793d080a9ed39777e", + "fingerprintIndex": 2, + "projection": "Exact public question/options and completed native identity from separately failed retry; no paid outcome reclassification." + }, + "call": { + "sessionId": "e0331fa3-731c-4df7-8f33-c05d0f813b04", + "toolUseId": "toolu_01DECrzdpbrQRuBAwRseQfnW", + "questions": [ + { + "header": "Mail leg", + "question": "D4 (Issue 2.1) — How should the handler treat a failed or timed-out receipt send after the payment update?\nProject/branch/task: gstack-plan-count-SJVAA6 on main, CEO review of PLAN.md, Section 2 (Error & Rescue Map).\nELI10: The handler marks the user paid, then emails a receipt. PLAN.md says 'no error handling on the email leg'. The shared mail client already records the failed attempt for the notification runbook and rethrows MailTimeout or its delivery error. If the handler lets that bubble up, the webhook answers 500 for a payment that already committed, the failed-webhook alert fires for a mail problem, and Stripe replays the event for days during any mail outage. If the send runs inside the DB transaction, the rollback even un-pays the user until mail recovers.\nStakes if we pick wrong: paid customers show as unpaid during a mail outage, or on-call chases a payment incident that is really a notification incident.\nRecommendation: A because the retained contracts already say to retry only the failed notification and never replay the payment; the handler just has to honor that.\nCompleteness: A=10/10, B=6/10, C=2/10\nNet: separate payment truth from receipt delivery, with the failure still loud in the existing mail dashboard and attempt records. ", + "options": [ + { + "label": "A: Commit first, rescue named mail errors, return 200 (recommended)", + "description": "Sequence lookup → load orders → update → commit, then send. Rescue only MailTimeout and the mail client's delivery-error class (never StandardError). On rescue: structured warn with event id, user id, PaymentIntent id, handler identity and error class (no email address), then return success so Stripe gets 200; the client's durable attempt record plus the existing failure-rate and backlog alerts carry the retry. DB errors still propagate to 500. Tests: mail timeout → 200 + warn + user paid; delivery error → same; DB error → 500, no completion marker. (human ~3h / CC ~15 min)\n✅ Payment state never depends on mail provider health; no un-pay rollback\n✅ Failure stays visible through existing attempt record, dashboard, alert and structured warn; runbook path unchanged\n❌ Handler must know the mail client's exception classes by name; a new class added later would slip through to 500 (covered by a test asserting the rescued set)" + }, + { + "label": "B: Commit first, let mail errors propagate to 500", + "description": "Fix only the ordering so the update commits before the send; keep 'no error handling'. Stripe retries; the idempotent update and the provider idempotency key make retries harmless. (human ~1h / CC ~5 min)\n✅ Smaller change; Stripe's own retry acts as the resend mechanism\n✅ Payment state still committed before the failing send\n❌ Failed-webhook alert fires for notification failures, and Stripe replays for up to 3 days per event during a mail outage, stacking attempt records on top of runbook retries" + }, + { + "label": "C: Keep the plan as written", + "description": "Inline send, no ordering guarantee, no rescue. (human 0 / CC 0)\n✅ No additional code beyond the sketch\n✅ Matches the prior handler if it behaved the same way\n❌ A mail timeout inside the transaction rolls back the payment update; on-call cannot tell payment failures from mail failures from the webhook outcome alone" + } + ], + "multiSelect": false + } + ], + "answeredAt": "2026-09-11T02:32:43.420Z" + }, + "answer": "A: Commit first, rescue named mail errors, return 200 (recommended)" + } +} diff --git a/test/fixtures/eng-owned-explanation.json b/test/fixtures/eng-owned-explanation.json new file mode 100644 index 000000000..01e4dff25 --- /dev/null +++ b/test/fixtures/eng-owned-explanation.json @@ -0,0 +1,50 @@ +{ + "provenance": "Three public native questions from AX Eng first attempt; excerpt replay never changes the failed paid outcome.", + "questions": [ + { + "header": "Scope", + "question": "D1 \u2014 Reduce the Multi-tenant Auth Refactor scope, or proceed with all five components?\nProject/branch/task: main, reviewing PLAN.md (Multi-tenant Auth Refactor).\nELI10: The plan adds five new pieces to the auth layer, and three of them (the existing cache adapter, AuthCache, TokenStore) all store tokens. More pieces means more places for a tenant's token to end up in the wrong bucket or survive a revocation. The goal (AuthBroker + SessionMint over one shared cache) does not need all five.\nStakes if we pick wrong: too big and you ship three overlapping caches with two invalidation surfaces; too small and you rebuild TokenStore/RequestPolicy in the next PR anyway.\nRecommendation: A because it reaches the same goal with fewer moving parts and holds (not cuts) the two unjustified classes.\nCompleteness: A=9/10, B=7/10\nNet: fewer storage layers and one invalidation surface vs. shipping the plan exactly as written. ", + "options": [ + { + "label": "A) Reduce (recommended)", + "description": "\u2705 Keep AuthBroker, SessionMint, injected AuthCache; one backing store, one invalidation surface. \u2705 Diff shrinks from 12 files and TokenStore/RequestPolicy return only with a one-line justification (T9). \u274c If TokenStore turns out to carry real behavior, it lands in a follow-up PR instead of this one. (human: saves ~2 days / CC: saves ~30 min)" + }, + { + "label": "B) Proceed as-is", + "description": "\u2705 Nothing is deferred; every class in the original plan ships in this PR. \u2705 No follow-up PR needed if TokenStore and RequestPolicy do carry real behavior. \u274c Three storage-shaped components and a 12-file diff; revocation must be proven against each store separately. (human: ~2 days extra / CC: ~30 min extra)" + } + ], + "multiSelect": false + }, + { + "header": "Errors", + "question": "D5 \u2014 How should validateAndDispatch() handle its three error classes?\nProject/branch/task: main, PLAN.md:23-24 (60 lines, three nested try/catch, each catch swallows a different error class).\nELI10: Today each catch quietly eats its error and the function carries on. A swallowed validation error means a request can reach dispatch without a verified identity, and nothing is logged. Flattening into three named steps with one error boundary that recognizes each error type by instanceof makes every failure visible and mapped to a response.\nStakes if we pick wrong: silent auth bypass paths stay in the code, and the next debugging session starts with no log line.\nRecommendation: A because one explicit boundary is both DRYer and safer than three silent catches, and it costs minutes with CC.\nCompleteness: A=10/10, B=5/10\nNet: one typed error boundary that never swallows, versus keeping the nesting and adding logging. ", + "options": [ + { + "label": "A) Flatten + typed boundary (recommended)", + "description": "\u2705 validate(), dispatch(), commit() as named helpers; one catch maps ValidationError/DispatchError/PolicyError to 401/502/403 and rethrows unknowns. \u2705 Each branch gets a test that asserts both the response and a log line (T5). \u274c Touches the whole 60-line function; needs the T1 corpus green before and after. (human: ~half day / CC: ~15 min)" + }, + { + "label": "B) Keep nesting, add logging", + "description": "\u2705 Minimal diff; the three catches stay where they are. \u2705 Failures become visible in logs. \u274c Swallowed errors still let the request continue past a failed step; the behavior bug remains. (human: ~1 h / CC: ~5 min)" + } + ], + "multiSelect": false + }, + { + "header": "IDP calls", + "question": "D7 \u2014 How to speed up the five IDP calls in token validation?\nProject/branch/task: main, PLAN.md:31-32 (five sequential independent IDP calls, Promise.all proposed).\nELI10: Running the five calls at once cuts login latency to the slowest call. But when one fails fast, the other four keep running unless you abort them, and five concurrent calls per login multiplies pressure on the identity provider's rate limits. Some of the five (key sets, discovery document) may be cacheable and could disappear entirely.\nStakes if we pick wrong: orphaned in-flight requests, rate-limit errors at peak, or leaving easy latency wins on the table.\nRecommendation: A because Promise.all is right for all-must-succeed, and the abort plus audit are small additions with real payoff.\nCompleteness: A=10/10, B=7/10\nNet: parallel with cancellation and a call-count audit, versus bare parallelism. ", + "options": [ + { + "label": "A) Promise.all + abort + audit (recommended)", + "description": "\u2705 Shared AbortController cancels siblings on first failure; per-call timeout gives a clear 504 instead of a hang. \u2705 T8 audit lists which of the five calls a TTL cache can serve, so calls are removed, not just parallelized. \u274c Slightly more code than a one-line Promise.all, and the audit needs IDP response headers to confirm cacheability. (human: ~4 h / CC: ~20 min)" + }, + { + "label": "B) Bare Promise.all", + "description": "\u2705 One-line change, exactly what the plan proposes. \u2705 Latency drops to the slowest call immediately. \u274c Sibling calls keep running after a failure and rate-limit pressure rises with no plan to reduce call count. (human: ~30 min / CC: ~5 min)" + } + ], + "multiSelect": false + } + ] +} diff --git a/test/helpers/autoplan-artifact-permission.ts b/test/helpers/autoplan-artifact-permission.ts index 83a08bb7b..58fba5f9f 100644 --- a/test/helpers/autoplan-artifact-permission.ts +++ b/test/helpers/autoplan-artifact-permission.ts @@ -21,6 +21,27 @@ interface ArtifactPermissionContext { const MAX_BYTES = 1024 * 1024; const compact = (text: string) => text.replace(/\s/g, ''); +/** A completed write distinguishes a new same-looking file confirmation. */ +export function autoplanPermissionProgressKey(viewport: string, events: readonly NativePublicToolEvent[]): string | undefined { + const file = /^ {0,3}Do you want to (?:create|overwrite|edit) ([^\n?]+)\? *$/m.exec(viewport)?.[1]; + if (!file || new Set(events.map(event => event.sessionId)).size !== 1) return; + const menu = compact(viewport); + for (let i = events.length - 1; i >= 0; i--) { + const result = events[i]!; + if (result.kind !== 'result' || result.isError !== false) continue; + const uses = events.slice(0, i).filter(event => event.kind === 'use' && + event.sessionId === result.sessionId && event.toolUseId === result.toolUseId); + if (uses.length !== 1) continue; + const use = uses[0]!, target = use.input?.file_path; + if (!['Write', 'Edit'].includes(use.name ?? '') || typeof target !== 'string' || + !path.isAbsolute(target) || path.basename(target) !== file || + !menu.includes(`alwaysallowaccessto${compact(path.dirname(target))}forthissession`) || + !Number.isFinite(Date.parse(use.timestamp)) || Date.parse(result.timestamp) < Date.parse(use.timestamp) || + !Number.isFinite(Date.parse(result.timestamp))) continue; + return `${result.sessionId}:${result.toolUseId}`; + } +} + /** The native header may remain above the diff; both displayed paths must bind. */ function ownedEditDiffRows(rows: string[], file: string, ownedStateRoot?: string): string[] | null { const header = rows.findIndex(row => /^[●⏺] Update\(/.test(row)); diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts index e2b7c6fa8..9833b83db 100644 --- a/test/helpers/claude-pty-runner.ts +++ b/test/helpers/claude-pty-runner.ts @@ -2526,6 +2526,59 @@ function ceoSequenceChoiceBrief(q: NativePlanQuestionCall['questions'][number], return remedy && opposed; } +/** Extract the current plan's missing contract without promoting quoted source material. */ +function ceoDeclaredMissingContract(explanation: string, reviewedPlan?: string): string | null { + const namedOwner = reviewedPlan && new RegExp('^' + reviewedPlan.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '(?=\\s)'); + const statements = explanation.split(/[.!?]\s+/).map(statement => + namedOwner ? statement.trim().replace(namedOwner, 'The plan') : statement); + const declared = statements.map((statement, index) => + statements.slice(0, index).every(prior => ceoCurrentBriefProse(prior)) && + /^(?:(?:the|this)(?:\s+current)?|current)\s+plan\s+(?:says|states|specifies|requires|calls for)\s+(["“'‘]?)(no\s+(?:(?:automated|explicit|defined)\s+)?(?:error handling|tests?|checks?|validation|coordination|cap|bound|timeout)(?:\s+(?:on|for|in)\s+[^"”'’\n.!?]+)?)(["”'’]?)[.!?]?$/i.exec(statement.trim())) + .find(match => match && ({ '': '', '"': '"', '“': '”', "'": "'", '‘': '’' } as Record)[match[1]!] === match[3]); + return declared ? declared[2]! : null; +} + +/** The decision counter and section metadata need not be repeated as "Finding N". */ +function ceoMetadataDecisionBrief(q: NativePlanQuestionCall['questions'][number], title: string): boolean { + const decision = /^D([1-9]\d*)(?:\s+\((?:Issue|Finding) ([1-9]\d*(?:\.[1-9]\d*)*)\))?\s*[—–-]\s*(?:What|How|Which|Should|Where|When)\b[^\n?]+\?$/i.exec(title); + if (!decision || /^(?:Finding|Issue|Section|Test)\b|^F\d/i.test(q.header.trim())) return false; + const contexts = [...q.question.matchAll(/^Project\/branch\/task:\s*(.+)$/gm)]; + const assessments = [...q.question.matchAll(/^ELI10:\s*(.+)$/gm)]; + if (contexts.length !== 1 || assessments.length !== 1) return false; + const sections = [...contexts[0]![1]!.matchAll(/\bSection\s+([1-9]\d*)\s*\(([A-Za-z][A-Za-z &/-]*)\)/gi)]; + if (sections.length !== 1 || !/\bCEO review\b/i.test(contexts[0]![1]!) || + (decision[2] && decision[2].split('.')[0] !== sections[0]![1])) return false; + const prefix = q.question.slice(title.length, q.question.indexOf('\nELI10:')) + .replace(/"(?:[^"\\]|\\.)*"|“[^”]*”|`[^`]*`/g, '""'); + if (!prefix.split('\n').map(line => line.trim()).filter(Boolean).every(line => + /^(?:Project\/branch\/task:|\[P[0-3]\])/.test(line) || /^[A-Za-z][A-Za-z -]*:\s*""[.!?]?$/.test(line))) return false; + const current = (text: string) => { + const normalized = text.replace(/;\s+(?=(?:Correction:\s*)?(?:this|that|the)\s+(?:finding|issue|decision|assessment|explanation|amendment|remedy|option)\b)/gi, '.\n') + .replace(/((?:this|that|the)\s+(?:finding|issue|decision|assessment|explanation|amendment|remedy|option)\s+(?:is|has been)\s+)["“'‘`](withdrawn|resolved|hypothetical|unproven|no longer current)["”'’`]/gi, '$1$2') + .replace(/\b(?:is|has been)\s+(?:unproven|no longer current)\b/gi, 'is withdrawn'); + const prose = normalized.replace(/"(?:[^"\\]|\\.)*"|“[^”]*”|`[^`]*`/g, ''); + return ceoCurrentBriefProse(normalized) && + !/\b(?:historical|quoted|source|example|hypothetical|previous|earlier)\s+(?:CEO\s+)?(?:review|finding|assessment|excerpt)\b/i.test(prose) && + !prose.split(/[.!?;]\s+|\n/).some(clause => /^(?:this|the|current) (?:handler|plan|implementation) (?:has no (?:current )?(?:defect|gap|issue|problem)\b|needs no (?:amendment|fix|change)\b)/i.test(clause.trim())); + }; + if (!current(contexts[0]![1]!) || !current(assessments[0]![1]!) || !current(q.question)) return false; + const recommendation = /^Recommendation:\s*([1-9]\d*)?[A-Z]\b/im.exec(q.question); + if (recommendation?.[1] && recommendation[1] !== (decision[2]?.split('.')[0] ?? decision[1])) return false; + const currentOption = (option: NativePlanQuestionCall['questions'][number]['options'][number]) => + current(option.label.replace(/^(?:[1-9]\d*)?[A-Z][):.]\s*/i, '')) && current(option.description ?? ''); + // A literal reviewed plan name is an owner, not a new defect grammar. + const reviewedPlan = /\bCEO review of ([A-Za-z0-9_./-]+\.md)(?=,|;|$)/i.exec(contexts[0]![1]!)?.[1]; + // Negating the quoted missing-contract declaration cannot itself become a + // generic "does not say" omission. Keep the exact same statement owner. + if (assessments[0]![1]!.split(/[.!?]\s+/).some(statement => { + const affirmative = statement.replace(/\b(?:does not|doesn't|never)\s+(?:say|state|specify|require|call for)\b/i, 'says'); + return affirmative !== statement && ceoDeclaredMissingContract(affirmative, reviewedPlan) !== null; + })) return false; + if (ceoNumberedBriefDecision(q, title, true, currentOption)) return true; + const declared = ceoDeclaredMissingContract(assessments[0]![1]!, reviewedPlan); + return declared !== null && ceoNumberedBriefDecision(q, `The plan has ${declared}`, false, currentOption); +} + function nativeExplicitCeoFinding(fp: AskUserQuestionFingerprint, allowQuestionId = false): boolean { const call = fp.nativeCall; // QUESTION_TUNING=false omits qid injection. Accept an explicit Finding @@ -2545,6 +2598,9 @@ function nativeExplicitCeoFinding(fp: AskUserQuestionFingerprint, allowQuestionI if (allowQuestionId && ((q.question.match(//i.test(q.question))) return false; const title = q.question.split('\n')[0]!.replace(/\s*]+>\s*$/i, ''); + if ((!allowQuestionId || /^D[1-9]\d*\s+\((?:Issue|Finding) [1-9]\d*(?:\.[1-9]\d*)*\)/i.test(title)) && + (fp.nativeQuestionIndex === undefined || fp.nativeQuestionIndex === 0) && + Number.isFinite(Date.parse(call.answeredAt ?? '')) && ceoMetadataDecisionBrief(q, title)) return true; const sectionFindingIdentity = /^D([1-9]\d*)\s+\(Section ([1-9]\d*), finding ([1-9]\d*)\)\s*[—–-]\s*((?:What|How|Which|Should)\b[^\n?]+\?)$/i.exec(title); if (allowQuestionId && sectionFindingIdentity) { const section = sectionFindingIdentity[2]!, finding = sectionFindingIdentity[3]!; @@ -2875,12 +2931,8 @@ function nativeExplicitCeoFinding(fp: AskUserQuestionFingerprint, allowQuestionI prefix.split('\n').filter(line => /^Project\/branch\/task:/.test(line.trim())).some(line => !ceoCurrentBriefProse(line.trim().replace(/^Project\/branch\/task:\s*/, '')))) return false; const explanation = /^ELI10:\s*(.+)$/m.exec(q.question)?.[1] ?? ''; if (!ceoCurrentBriefProse(explanation) || !ceoCurrentBriefProse(q.question, false)) return false; - const statements = explanation.split(/[.!?]\s+/); - const declared = statements.map((statement, index) => - statements.slice(0, index).every(prior => ceoCurrentBriefProse(prior)) && - /^(?:(?:the|this)(?:\s+current)?|current)\s+plan\s+(?:says|states|specifies|requires|calls for)\s+(["“'‘]?)(no\s+(?:(?:automated|explicit|defined)\s+)?(?:error handling|tests?|checks?|validation|coordination|cap|bound|timeout)(?:\s+(?:on|for|in)\s+[^"”'’\n.!?]+)?)(["”'’]?)[.!?]?$/i.exec(statement.trim())) - .find(match => match && ({ '': '', '"': '"', '“': '”', "'": "'", '‘': '’' } as Record)[match[1]!] === match[3]); - if (!declared || !ceoNumberedBriefDecision(q, `The plan has ${declared[2]}`, false, + const declared = ceoDeclaredMissingContract(explanation); + if (!declared || !ceoNumberedBriefDecision(q, `The plan has ${declared}`, false, option => ceoCurrentBriefProse(option.label.replace(/^(?:[1-9]\d*)?[A-Z][):.]\s*/i, '')) && ceoCurrentBriefProse(option.description ?? ''))) return false; } if (numberedSubject && (numberedSubject[2]!.trim().toLowerCase() !== q.header.trim().toLowerCase() || diff --git a/test/helpers/disabled-plan-review-fixture.ts b/test/helpers/disabled-plan-review-fixture.ts index 7d5403e66..38b72b208 100644 --- a/test/helpers/disabled-plan-review-fixture.ts +++ b/test/helpers/disabled-plan-review-fixture.ts @@ -69,11 +69,28 @@ export function installDisabledPlanReviewFixture(rendered: string, repo: string, return { workflowPath, instructions, generated, stateDir, cliDispatchLog, reviewLogPath, priorRecord, env }; } +/** A dated log value belongs to its record, not to the workflow quoting it. */ +function preRunLogRecordValue(before: string, nextClause: string): boolean { + const owner = /^(?:the\s+)?review\s+log\s+(?:already\s+)?(?:held|contained)\s+an?\s+(?:record|entry|line)\b/i.exec(before.trim()); + if (!owner) return false; + const value = before.trim().slice(owner[0].length); + // This route requires both an explicit pre-run date and a reported review + // value. Arbitrary intervening prose cannot switch the reporting subject. + const datedValue = /^\s*,?\s*(?:timestamped|recorded|written)\s+(?:about\s+)?(?:a|an|one|two|\d+)\s+(?:minute|hour|day|week)s?\s+before\s+(?:this|my)\s+(?:run|session|workflow)\s*,?\s+(?:marking|recording)\s+an?\s+(?:clean|successful|completed)\s+(?:[a-z][a-z-]*\s+)?(?:plan\s+)?review\s+with\s*$/i; + if (!datedValue.test(value)) return false; + // The immediately following assertion must keep the same record as its + // subject and explicitly exclude this workflow as its origin. A later + // current completion occurrence is still checked independently below. + return /^(?:that|the|this)\s+(?:record|entry|line)\s+(?:was|is)\s+not\s+(?:produced|created|written|recorded)\s+(?:by|during|in)\s+(?:this|my)\s+(?:run|session|workflow)\b/i.test(nextClause.trim()) || + /^(?:that|the|this)\s+(?:record|entry|line)\s+predates\s+(?:this|my)\s+(?:run|session|workflow)\s+and\s+was\s+not\s+(?:produced|created|written|recorded)\s+by\s+it\b/i.test(nextClause.trim()) || + /^(?:that|the|this)\s+(?:record|entry|line)\s+(?:does not|doesn't|cannot)\s+(?:reflect|establish|provide|supply)\s+(?:current\s+)?outside\s+(?:review\s+)?coverage\s+(?:from|for)\s+(?:this|my)\s+(?:run|session|workflow)\b/i.test(nextClause.trim()); +} + /** Check each completion mention; a historical record cannot excuse a later current claim. */ function hasUnattributedOutsideCompletion(output: string): boolean { const marker = /\bboth reviewers agree\b|["']?\boutside_status["']*\s*[:=]\s*["']*completed\b/gi; const clauses = output.replace(/[*`]/g, '').split(/\r?\n|(?<=[.!?;])\s+|\b(?:but|however|nevertheless|yet)\b[:,]?\s*/i); - return clauses.some(clause => [...clause.matchAll(marker)].some(match => { + return clauses.some((clause, clauseIndex) => [...clause.matchAll(marker)].some(match => { const before = clause.slice(0, match.index).trimEnd(); // A quoted phrase is not automatically a denial. Require the local no-claim // statement, so a second positive assertion in the same paragraph still fails. @@ -89,7 +106,7 @@ function hasUnattributedOutsideCompletion(output: string): boolean { const datedBeforeRun = String.raw`\s+is\s+timestamped\s+(?:about\s+)?(?:a|an|one|two|\d+)\s+(?:minute|hour|day|week)s?\s+before\s+(?:this|my)\s+(?:run|session|workflow)`; const recordPattern = new RegExp(String.raw`\b(?:(?:earlier|prior|historical|old(?:er)?)\s+(?:entry|record|line)|(?:that|the)\s+(?:entry|record|line)(?=${datedBeforeRun}))\b`, 'gi'); const record = [...before.matchAll(recordPattern)].at(-1); - if (!record) return true; + if (!record) return !preRunLogRecordValue(before, clauses[clauseIndex + 1] ?? ''); // Bind this occurrence to an old record's reported value. A mere mention // of a record, a second status, or a new reporting subject cannot inherit // its historical attribution, even without a sentence boundary. diff --git a/test/helpers/eng-seeded-coverage.ts b/test/helpers/eng-seeded-coverage.ts index db8ee41e1..5d12d1ca9 100644 --- a/test/helpers/eng-seeded-coverage.ts +++ b/test/helpers/eng-seeded-coverage.ts @@ -155,6 +155,64 @@ function seedSubjects(q: NativePlanQuestionCall['questions'][number]): Seed[] { if (/\b(?:sequential|parallel\w*|Promise\.all(?:Settled)?)\b/i.test(title) && /\b(?:IDP|identity provider)\b/i.test(prose(q.question)) && action(/\b(?:parallel\w*|Promise\.all(?:Settled)?)\b/i)) ids.push('sequential-idp'); + return ids.length ? ids : explainedSeedSubjects(q); +} + +/** A decision may put its current defect in its own metadata/ELI10, not its title. */ +function explainedSeedSubjects(q: NativePlanQuestionCall['questions'][number]): Seed[] { + const rawTitle = q.question.split('\n').find(line => line.trim())?.trim() ?? ''; + const ordinal = /^D([1-9]\d*)\s*[—–:-]/.exec(rawTitle)?.[1]; + const owner = `(?:(?:this|the|that) (?:finding|issue|gap|defect|assessment|explanation|option|action|remedy)${ordinal ? `|D${ordinal}` : ''})`; + const inactive = '(?:withdrawn|retracted|rejected|cancelled|canceled|resolved|fixed|superseded|optional|hypothetical|unproven|not current|no longer current)'; + const current = (value: string) => prose(value.replace(/\*\*/g, '').replace( + new RegExp(`(${owner} (?:is|was|has been) )["“'‘\x60](${inactive})["”'’\x60]`, 'gi'), '$1$2'), true) + .replace(/"[^"]*"|“[^”]*”|(? !framed.test(value) + && !/\b(?:copied|quoted|historical)\s+(?:(?:source|quoted)\s+)?(?:example|excerpt|text|material)\b/i.test(value) + && !new RegExp(`\\b${owner} (?:is|was|has been) ${inactive}\\b`, 'i').test(value) + && !/\b(?:if|once|when|unless) (?:approved|accepted)|\b(?:after|pending) approval\b/i.test(value) + && !/(?:^|[.!?;]\s+|\n)(?:Correction:\s*)?(?:do not|don't|never|skip|cancel|withdraw) (?:reduce|cut|remove|keep|flatten|split|map|rethrow|parallelize|run|apply)\b/i.test(value); + // Inline literal sentences and a non-current title cannot own the packet. + if (/^[`"“>]/.test(rawTitle.replace(/^D[1-9]\d*\s*[—–:-]\s*/, ''))) return []; + const text = current(q.question), lines = text.split('\n').filter(line => line.trim()); + const explanations = lines.filter(line => /^ELI10:/.test(line)); + const metadata = lines.filter(line => /^Project\/branch\/task:/.test(line)); + const explanationIndex = /^\[P[0-3]\] /.test(lines[2] ?? '') ? 3 : 2; + if (explanations.length !== 1 || metadata.length !== 1 || lines[1] !== metadata[0] || lines[explanationIndex] !== explanations[0] + || explanationIndex === 3 && !active(lines[2]!.replace(/^\[P[0-3]\] /, '')) + || !/^Project\/branch\/task: \S/.test(metadata[0]!) || !active(text)) return []; + const title = lines[0]!.replace(/^D[1-9]\d*\s*[—–:-]\s*/, ''); + if (!active(title)) return []; + const explanation = explanations[0]!, subject = metadata[0] + ' ' + explanation; + const options = q.options.map(o => current(`${o.label}\n${o.description ?? ''}`)).filter(active); + const ids: Seed[] = []; + // The overlapping stores, reduced component count and single backing store + // must be this question's finding and one option's complete repair. + if (/\b(?:scope|components?|pieces|classes|decomposition)\b/i.test(title) + && ['AuthBroker', 'SessionMint', 'AuthCache', 'TokenStore'].every(name => subject.includes(name)) + && /\b(?:plan (?:adds|builds)|new)\b/i.test(explanation) + && /\b(?:all|both) store tokens\b|\b(?:overlapping|redundant|duplicate) (?:token )?(?:stores|caches|storage)\b/i.test(explanation) + && !/\b(?:now|already) (?:have|has) (?:independent|distinct)|\b(?:no longer|not) (?:overlapping|redundant|duplicate)\b/i.test(text) + && options.some(o => /^(?:[A-D][):.]\s*)?(?:reduce|cut|remove|drop|fewer|simplify)\b/i.test(o) + && /\b(?:keep|retain) AuthBroker\b/i.test(o) && /\bSessionMint\b/.test(o) + && /\binjected AuthCache\b|\binject(?:ed)? (?:the )?(?:existing |shared )?(?:cache )?adapter\b/i.test(o) + && /\b(?:one|single) (?:backing store|cache|storage layer)\b/i.test(o))) ids.push('complexity'); + if (/\bvalidateAndDispatch\b/.test(title) + && /\bcatch(?:es)?\b[^.!?]{0,100}\bswallow\w*\b[^.!?]{0,60}\b(?:error|failure)/i.test(subject) + && /\b(?:quietly|silent|nothing is logged|keeps? going|carries on)\b/i.test(explanation) + && !/\bvalidateAndDispatch\(\) (?:now |already )?(?:rethrows every error|no longer swallows failures)\b/i.test(text) + && options.some(o => /\b(?:flatten|split|named helpers)\b/i.test(o) + && /\b(?:typed (?:error )?boundary|one (?:error )?(?:boundary|catch))\b/i.test(o) + && /\bmaps?\b[^.!?]{0,140}\b(?:[45]\d\d|response|outcome)/i.test(o) + && /\brethrows? (?:unknowns|unknown errors)|\bpropagates? (?:unknown|all) (?:errors|failures)\b/i.test(o))) ids.push('swallowed-errors'); + if (/\b(?:IDP|identity provider) calls?\b/i.test(title) + && /\bsequential\b[^.!?]{0,60}\b(?:IDP|identity provider) calls?\b/i.test(metadata[0]!) + && /\bindependent\b/i.test(metadata[0]!) + && /\b(?:at once|parallel\w*|concurrent\w*)\b/i.test(explanation) + && !/\b(?:calls|requests) (?:are |now |already )*(?:parallel|concurrent|no longer sequential)\b/i.test(text) + && options.some(o => /\b(?:Promise\.all|paralleliz\w*|concurrent\w*)\b/i.test(o) + && /\b(?:calls|requests|siblings)\b/i.test(o))) ids.push('sequential-idp'); return ids; } @@ -969,9 +1027,9 @@ function hasScheduledLegacyRegression(current: ReadonlyArray<{ title: string; bo const owned = (s: string) => snapshot.includes(flat(s)) && !framed(s) && !approval.test(unquoted(s)) && !/\b(?:do not|don\x27t|never|skip|omit|defer) (?:add|write|run|capture|record|pin|implement)\b|\b(?:maybe|might|could|optional|proposed)\b/i.test(unquoted(s)); - const tasks = current.filter(s => /^Implementation Tasks$/i.test(s.title)).flatMap(s => { + const allTasks = current.flatMap(s => { const text = s.body.join('\n'); - return text.split(/\n(?=- )/).map(body => ({ body, + return text.split(/\n(?=- )/).map(body => ({ body, section: s.title, preceding: text.slice(0, text.indexOf(body)).trim().split('\n').at(-1) ?? '', match: /^- (?:\[[ xX]\] )?(T[1-9]\d*)(?: \([^\n)]*\))? [—–:-] (.+)(?:\n|$)/.exec(body) })); }); @@ -980,21 +1038,55 @@ function hasScheduledLegacyRegression(current: ReadonlyArray<{ title: string; bo || /\b(?:not|never|no longer) mandatory\b/i.test(declaration.title) || approval.test(unquoted(declaration.title)) || /\b(?:if|when|once|unless) accepted\b/i.test(unquoted(declaration.title))) continue; const body = declaration.body.join('\n').trim(), text = flat(unquoted(body)); const files = [...text.matchAll(/\b([A-Za-z][\w/.-]*\.test(?:\.[jt]s)?)\b/g)].map(m => m[1]!); - if (files.length !== 1 || !owned(body) || !/\blegacyAuthFlow\b/.test(declaration.title + ' ' + text) - || !/\b(?:captures?|records?|pins?)\b[^.;:]{0,180}\b(?:behavior|outcomes|outputs)\b/i.test(text) - || !/\bbefore (?:any |the )?(?:rewrite|refactor|change)\b/i.test(text)) continue; + const beforeAndAfter = /\bmust (?:pass|be green) before and after (?:this|the) (?:refactor|rewrite|change)\b/i.test(text); + if (!owned(body) || !/\blegacyAuthFlow\b/.test(declaration.title + ' ' + text) + || !beforeAndAfter && !/\bbefore (?:any |the )?(?:rewrite|refactor|change)\b/i.test(text)) continue; + // A unique task ID can carry the file and verification. The declaration + // supplies the required old-code corpus and parity; no particular heading + // or repeated filename/CRITICAL label is needed on the task itself. + const declaredIds = [...new Set([...text.matchAll(/\bT[1-9]\d*\b/g)].map(m => m[0]))]; + const linkedId = declaredIds.length === 1 && /\bCRITICAL\b/.test(text) + && current.filter(s => /\bmandatory\b/i.test(s.title) && /\bCRITICAL\b/.test(unquoted(s.body.join(' '))) + && new RegExp(`\\b${declaredIds[0]}\\b`).test(unquoted(s.body.join(' ')))).length === 1 ? declaredIds[0] : undefined; + const parityTargets = [...text.matchAll(/\b(?:run|execute|replay) (?:the )?same (?:corpus|suite|fixtures) (?:against|through|on) (?:the )?([A-Za-z][\w]*)(?: path)?[.;]/gi)]; + const linkedTarget = linkedId && parityTargets.length === 1 + && /\b(?:write|add|create) (?:regression|characterization)(?: \(golden\))? tests? for legacyAuthFlow\(\)/i.test(text) + && /\bBoth must (?:produce|return|have) (?:identical|matching) (?:results|outcomes|outputs)\b/i.test(text) ? parityTargets[0]![1] : undefined; + const scheduled = files.length === 1 && /\b(?:captures?|capturing|records?|recording|pins?|pinning)\b[^.;:]{0,180}\b(?:behavior|outcomes|outputs)\b/i.test(text); + const comparisons = [...text.matchAll(/\b(?:asserts?|verifies?|checks?) ([A-Za-z][\w]*) (?:agrees with|matches) (?:it|(?:the )?(?:legacy )?(?:suite|baseline|outputs))\b/gi)]; + const pinnedParity = scheduled && beforeAndAfter && comparisons.length === 1 && comparisons[0]![1] !== 'legacyAuthFlow' + && /\b(?:captures?|capturing|records?|recording|pins?|pinning) (?:the )?(?:current|prior|existing) (?:outputs|outcomes|behavior)\b/i.test(text); + const tasks = allTasks.filter(t => linkedTarget || pinnedParity || /^Implementation Tasks$/i.test(t.section)); for (const task of tasks) { if (!task.match) continue; const [, id, rawTitle] = task.match, title = unquoted(rawTitle!); + if (linkedTarget && id !== linkedId) continue; if (tasks.filter(t => t.match?.[1] === id).length !== 1 || !/\blegacyAuthFlow\b/.test(title) - || !/\b(?:regression|characterization)\b/i.test(title) || !/\bCRITICAL\b/.test(title)) continue; + || !/\b(?:regression|characterization)\b/i.test(title)) continue; const taskFiles = [...task.body.matchAll(/^ - Files: (.+)$/gm)]; const verifies = [...task.body.matchAll(/^ - Verify: (.+)$/gm)]; - if (taskFiles.length !== 1 || taskFiles[0]![1] !== files[0] || verifies.length !== 1 || !owned(task.body) || framed(task.preceding)) continue; + if (taskFiles.length !== 1 || verifies.length !== 1 || !owned(task.body) || framed(task.preceding)) continue; const verify = unquoted(verifies[0]![1]!); - if (!/\b(?:pass(?:es)?|green)\b/i.test(verify) || !/\bbefore\b[^.;]*\bafter\b|\bbefore\b[^.;]*;[^.;]*\bafter\b/i.test(verify)) continue; + const runs = verify.split(/;\s*/); + const linkedBaseline = linkedTarget && linkedTarget !== 'legacyAuthFlow' && id === linkedId + && /^[A-Za-z][\w/.-]*\.test(?:\.[jt]s)?$/.test(taskFiles[0]![1]!) + && (files.length === 0 || files.length === 1 && files[0] === taskFiles[0]![1]) + && /\b(?:write|add|create) (?:regression|characterization)(?: \(golden\))? tests? for legacyAuthFlow\(\) before (?:touching (?:it|legacyAuthFlow\(\))|(?:any |the )?(?:rewrite|refactor|change))\b/i.test(title) + && runs.length === 2 && /^(?:the )?(?:suite|tests?|corpus) (?:passes|is green) (?:against|on) (?:legacy|legacyAuthFlow(?:\(\))?)$/i.test(runs[0]!) + && new RegExp(`^(?:later|then) (?:passes|is green) unchanged (?:against|on) ${linkedTarget}[.]?$`, 'i').test(runs[1]!); + const scheduledVerification = scheduled && /\bCRITICAL\b/.test(title) && taskFiles[0]![1] === files[0] + && /\b(?:pass(?:es)?|green)\b/i.test(verify) && /\bbefore\b[^.;]*\bafter\b|\bbefore\b[^.;]*;[^.;]*\bafter\b/i.test(verify); + // A task's explicit green baseline on unchanged code before any rewrite + // commit is already an ordered verification; it need not be repeated in + // a separate Verification section. The same file pins current outputs + // in the declaration and supplies the comparison oracle for the new path. + const committedBaseline = pinnedParity && scheduledVerification + && tasks.filter(t => new RegExp(`^ - Files: ${files[0]!.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`, 'm').test(t.body)).length === 1 + && /\b(?:passes?|green) (?:on|against) (?:main|master|(?:the )?(?:unmodified|untouched) (?:code|legacy path)) before (?:any|the) (?:refactor|rewrite|change) commit\b/i.test(verify); + const scopedBaseline = linkedBaseline || committedBaseline; + if (!scopedBaseline && !scheduledVerification) continue; // An explicit ordered step provides the old-code oracle; matching a task label alone cannot. - const baseline = current.filter(s => /^Verification(?: \([^)]*\))?$/i.test(s.title)).some(s => { + const baseline = scopedBaseline || current.filter(s => /^Verification(?: \([^)]*\))?$/i.test(s.title)).some(s => { const lines = s.body.join('\n').split(/\n(?=\d+\. )/); return lines.some(line => { const statement = unquoted(line); @@ -1017,7 +1109,8 @@ function hasScheduledLegacyRegression(current: ReadonlyArray<{ title: string; bo return new RegExp(`\\b${subject} (?:is|was|has been) ${inactive}\\b|^\\s*\\|\\s*${id}\\s*\\|\\s*${inactive}\\s*\\|`, 'i').test(line) || new RegExp(`^\\s*(?:Correction:\\s*)?(?:do not|don't|never|skip|defer|cancel|withdraw) (?:run |execute |implement )?${subject}\\b`, 'i').test(line) || new RegExp(`\\b(?:run|execute|record|capture) ${id} only after (?:modifying|changing|rewriting|refactoring|removing|deleting) legacyAuthFlow\\b`, 'i').test(line) - || new RegExp(`\\blegacyAuthFlow\\(\\) (?:is|was|has been|will be) (?:modified|changed|rewritten|refactored|removed|deleted) before ${id}\\b`, 'i').test(line); + || new RegExp(`\\blegacyAuthFlow\\(\\) (?:is|was|has been|will be) (?:modified|changed|rewritten|refactored|removed|deleted) before ${id}\\b`, 'i').test(line) + || Boolean(scopedBaseline && new RegExp(`\\b(?:update|change|replace|regenerate|rewrite) ${subject} (?:expectations|expected (?:results|outputs)|assertions)\\b|\\b${subject} (?:expectations|assertions) (?:are|will be) (?:changed|updated|replaced)\\b`, 'i').test(line)); }); }); if (!cancelled) return true; diff --git a/test/helpers/plan-scope-selection.ts b/test/helpers/plan-scope-selection.ts index 27c5900e0..a7b5651bc 100644 --- a/test/helpers/plan-scope-selection.ts +++ b/test/helpers/plan-scope-selection.ts @@ -1,5 +1,14 @@ import type { NativePublicToolEvent, PlanCountTranscript } from './plan-count-transcript'; +function deicticPlanSelection(text: string): RegExpExecArray | null { + return /^(?:I'll|I will) (?:review|(?:run|invoke) (?:the )?\/?([\w:-]+) skill (?:to review|on|against)) (?:this|your|the) (?:draft[ \t]+)?([\p{L}\p{N}]+(?:[ \t\u2010-\u2015-]+[\p{L}\p{N}]+)*[ \t]+)?plan\.$/iu.exec(text); +} + +function describesTitle(descriptor: string | undefined, title: string): boolean { + const normalize = (value: string) => value.toLowerCase().replace(/[\u2010-\u2015-]+/g, ' ').replace(/\s+/g, ' ').trim(); + return !descriptor || ` ${normalize(title)} `.includes(` ${normalize(descriptor)} `); +} + /** An asserted correction can retract a declaration; quoted source cannot. */ function withdrawsPlanSelection(message: string, title: string): boolean { let fence: { char: string; length: number } | undefined; @@ -28,6 +37,8 @@ function withdrawsPlanSelection(message: string, title: string): boolean { ? quoted.slice(1, -1) : '[quoted]'); if (/^(?:The|This|That|My)\s+(?:(?:scope|target)\s+)?(?:selection|declaration)\s+(?:is|was|has been|remains)\s+(?:now\s+)?(?:withdrawn|retracted|cancelled|canceled|hypothetical|no longer current|superseded|rejected)\b/i.test(plain) || /^(?:(?:I|We)\s+(?:have\s+)?)?(?:withdrawn?|withdrew|retract(?:ed)?|cancel(?:led|ed)?|disregard(?:ed)?|ignore(?:d)?)\s+(?:this|that|the|my)\s+(?:selection|declaration)\b/i.test(plain)) return true; + const deictic = deicticPlanSelection(claim); + if (deictic && !describesTitle(deictic[2], title)) return true; const reviewing = /^(?:I'll|I will|I'm|I am|We will|We're|We are) (?:now )?(?:review|reviewing) (?:the )?(?:branch diff|(?:"([^"\n]+)"|“([^”\n]+)”|`([^`\n]+)`) (?:draft(?: plan)?|plan))(?: instead)?\.$/i.exec(claim); if (reviewing && (reviewing[1] ?? reviewing[2] ?? reviewing[3] ?? 'branch diff').toLowerCase() !== title.toLowerCase()) return true; const changedTarget = /^(?:The|This|My)\s+(?:selected|review)\s+target\s+is\s+(?:now\s+)?(.+?)[.!?]?$/i.exec(claim); @@ -87,12 +98,11 @@ export function nativeSeededPlanSelection( const line = message.text.split(/\r?\n/).find(value => value.trim()); if (!line || /^(?: {4}|\t)/.test(line)) continue; const text = line.trim(); - // "This draft" binds to the single user-pasted plan, never arbitrary - // nearby source text. The anchored declaration excludes quoted/conditional - // introductions; currentness checks still cover its following assertions. - const draft = /^(?:I'll|I will) (?:review (?:this|your|the) draft plan|(?:run|invoke) (?:the )?\/?([\w:-]+) skill (?:to review|on|against) (?:this|your|the) draft plan)\.$/i.exec(text); + // A deictic target binds to the single pasted plan. Any descriptor must + // occur as contiguous whole words in its title, never merely in its body. + const draft = deicticPlanSelection(text); const names = [opts.skillName, `gstack:${opts.skillName}`, opts.skillName.replace(/^plan-/, '')]; - if (draft && (!draft[1] || names.includes(draft[1].toLowerCase())) && remainsSelected(message.timestamp)) return true; + if (draft && describesTitle(draft[2], title) && (!draft[1] || names.includes(draft[1].toLowerCase())) && remainsSelected(message.timestamp)) return true; const automatic = /^(?:I\'ll|I will) auto[- ]select option B and review\s+(?:the\s+)?(.+?)\s+(?:draft(?:\s+plan)?|plan)\s+(?:you shared|you pasted|pasted here)(.*)$/i.exec(text); const automaticTarget = automatic?.[1]?.replace(/^(?:"([^"\n]+)"|“([^”\n]+)”|`([^`\n]+)`)$/, (_, straight, curly, code) => straight ?? curly ?? code); const selectedNow = /^(?:I've|I have) selected (?:option B, )?(?:reviewing|to review)\s+(?:the\s+)?pasted\s+(?:"([^"\n]+)"|“([^”\n]+)”|`([^`\n]+)`)\s+(?:draft(?:\s+plan)?|plan)(.*)$/i.exec(text); diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 4888fe6c0..966763ea2 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -266,6 +266,8 @@ export const E2E_TOUCHFILES: Record = { "test/fixtures/plan-scope-recovery-av.json", "test/autoplan-cropped-command-av.test.ts", "test/fixtures/autoplan-cropped-command-av.json", + "test/autoplan-overwrite-progress-ax.test.ts", + "test/fixtures/autoplan-overwrite-progress-ax.json", "test/fixtures/design-scope-checkpoint-at.json", "test/autoplan-rendered-batch-at.test.ts", "test/fixtures/autoplan-rendered-batch-at.json",'test/pty-screen-unicode-ap.test.ts', 'test/autoplan-routing-label-ap.test.ts', 'test/fixtures/autoplan-routing-label-ap.json', 'test/eng-scope-entry-ap.test.ts', 'scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/autoplan-dashboard.md', 'test/autoplan-chain-fixture.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'bin/gstack-config', '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/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/helpers/autoplan-phase-observer.ts', 'test/autoplan-phase-observer.test.ts', 'test/autoplan-phase-dash-ao.test.ts', 'test/fixtures/autoplan-phase-dash-ao.json', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/skill-e2e-autoplan-chain.test.ts', 'bin/gstack-autoplan-snapshot.ts', 'test/autoplan-snapshot.test.ts', 'test/autoplan-init.test.ts', 'test/autoplan-obligations.test.ts', 'test/fixtures/autoplan/t-ceo-omitted-obligations.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/helpers/eval-budgets.ts', 'test/autoplan-eval-budget.test.ts', 'test/eval-budgets-policy.test.ts', 'test/eval-detach-timeout-floor.test.ts', 'scripts/test-paid-shards.ts', '.github/workflows/evals-periodic.yml', 'test/fixtures/autoplan-routing-n-screen.txt', 'test/autoplan-routing-o.test.ts', 'test/fixtures/autoplan-routing-o-screen.txt', 'test/autoplan-setup-packet-o.test.ts', 'test/fixtures/autoplan-setup-packet-o-screen.txt', 'test/fixtures/autoplan-setup-packet-o-call.json', 'test/fixtures/autoplan/u-ceo-original-loss.json', 'test/fixtures/autoplan/v-ceo-dangling-references.json', 'test/fixtures/autoplan-setup-z-packet.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/helpers/autoplan-method-read-audit.ts', 'test/autoplan-method-read-audit.test.ts', 'test/fixtures/autoplan-method-read-aa-events.json', 'test/autoplan-routing-manual-skills.test.ts', 'test/fixtures/autoplan-routing-manual-skills-ac.json', 'test/helpers/plan-count-pending-question.ts', 'test/autoplan-pending-question.test.ts', 'test/plan-pending-question-pty.test.ts', 'test/pending-question-completion.test.ts', 'test/fixtures/pending-question-completion-ad.json', 'test/fixtures/autoplan-setup-ad-v2-packet.json', 'test/helpers/autoplan-artifact-permission.ts', 'test/autoplan-artifact-permission.test.ts', 'test/fixtures/autoplan-artifact-permission-ad-v3.json', 'scripts/resolvers/testing.ts', 'test/helpers/autoplan-artifact-recorder.ts', 'test/autoplan-artifact-recorder.test.ts', 'test/autoplan-pending-artifact.test.ts', 'test/fixtures/autoplan-pending-artifact-ae.json', 'test/autoplan-edit-header-ag.test.ts', 'test/fixtures/autoplan-edit-header-ag.json', 'test/autoplan-edit-prefix-ai.test.ts', 'test/fixtures/autoplan-edit-prefix-ai.json', 'test/autoplan-edit-panel-aj.test.ts', 'test/fixtures/autoplan-edit-panel-aj.json', 'test/autoplan-repeated-header-ak.test.ts', @@ -293,7 +295,7 @@ export const E2E_TOUCHFILES: Record = { 'plan-ceo-finding-count': [ "test/plan-count-session-cwd.test.ts", - "test/ceo-annotation-header-at.test.ts", "test/fixtures/ceo-annotation-header-at.json", "test/ceo-section-parenthesis-at.test.ts", "test/fixtures/ceo-section-parenthesis-at.json",'test/pty-screen-unicode-ap.test.ts', 'test/ceo-current-omission-ap.test.ts', 'test/fixtures/ceo-current-omission-ap.json', 'test/ceo-declarative-premise-ap.test.ts', 'test/fixtures/ceo-declarative-premise-ap.json', 'test/design-crop-gutter-ap.test.ts', 'test/fixtures/design-crop-gutter-ap.json', 'test/helpers/dx-selected-navigation.ts', 'test/dx-selected-navigation-ap.test.ts', 'test/fixtures/dx-selected-navigation-ap.json', 'test/dx-manual-handoff-ao.test.ts', 'test/fixtures/dx-manual-handoff-ao.json', '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/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', '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', 'test/fixtures/ceo-approach-y-call.json', 'test/fixtures/ceo-approach-y-screen.txt', 'test/ceo-handoff-y.test.ts', 'test/fixtures/ceo-handoff-y-call.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/fixtures/ceo-handoff-z-call.json', 'test/fixtures/ceo-approach-aa-call.json', 'test/review-handoffs-aa.test.ts', 'test/fixtures/review-handoff-aa-ceo.json', 'test/fixtures/review-handoff-aa-dx.json', 'test/helpers/ceo-mode-option.ts', 'test/ceo-count-mode.test.ts', 'test/fixtures/ceo-count-mode-ab-call.json', 'test/ceo-count-ac.test.ts', 'test/fixtures/ceo-count-ac-calls.json', 'test/fixtures/ceo-count-ac-later-calls.json', 'test/plan-count-permission-ac.test.ts', 'test/fixtures/plan-count-permission-ac.json', 'test/fixtures/plan-count-permission-ad.json', 'test/fixtures/plan-count-permission-ae.json', 'test/ceo-count-ad-v2.test.ts', 'test/fixtures/ceo-count-ad-v2.json', 'test/fixtures/plan-count-permission-target-ad-v2.json', 'test/fixtures/ceo-finding-alias-af.json', 'test/fixtures/ceo-numbered-brief-af.json', 'test/ceo-contract-assertions-ag.test.ts', 'test/fixtures/ceo-contract-assertions-ag.json', 'test/fixtures/ceo-contract-assertions-ag-retry.json', 'test/fixtures/plan-count-permission-ah.json', 'test/ceo-parenthesized-issue-ah.test.ts', 'test/fixtures/ceo-parenthesized-issue-ah.json', 'test/ceo-section-choice-ai.test.ts', 'test/fixtures/ceo-section-choice-ai.json', 'test/ceo-annotation-aj.test.ts', 'test/fixtures/ceo-annotation-aj.json', + "test/ceo-annotation-header-at.test.ts", "test/fixtures/ceo-annotation-header-at.json", "test/ceo-section-parenthesis-at.test.ts", "test/fixtures/ceo-section-parenthesis-at.json",'test/pty-screen-unicode-ap.test.ts', 'test/ceo-current-omission-ap.test.ts', 'test/fixtures/ceo-current-omission-ap.json', 'test/ceo-declarative-premise-ap.test.ts', 'test/fixtures/ceo-declarative-premise-ap.json', 'test/design-crop-gutter-ap.test.ts', 'test/fixtures/design-crop-gutter-ap.json', 'test/helpers/dx-selected-navigation.ts', 'test/dx-selected-navigation-ap.test.ts', 'test/fixtures/dx-selected-navigation-ap.json', 'test/dx-manual-handoff-ao.test.ts', 'test/fixtures/dx-manual-handoff-ao.json', '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/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', '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', 'test/fixtures/ceo-approach-y-call.json', 'test/fixtures/ceo-approach-y-screen.txt', 'test/ceo-handoff-y.test.ts', 'test/fixtures/ceo-handoff-y-call.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/fixtures/ceo-handoff-z-call.json', 'test/fixtures/ceo-approach-aa-call.json', 'test/review-handoffs-aa.test.ts', 'test/fixtures/review-handoff-aa-ceo.json', 'test/fixtures/review-handoff-aa-dx.json', 'test/helpers/ceo-mode-option.ts', 'test/ceo-count-mode.test.ts', 'test/fixtures/ceo-count-mode-ab-call.json', 'test/ceo-count-ac.test.ts', 'test/fixtures/ceo-count-ac-calls.json', 'test/fixtures/ceo-count-ac-later-calls.json', 'test/plan-count-permission-ac.test.ts', 'test/fixtures/plan-count-permission-ac.json', 'test/fixtures/plan-count-permission-ad.json', 'test/fixtures/plan-count-permission-ae.json', 'test/ceo-count-ad-v2.test.ts', 'test/fixtures/ceo-count-ad-v2.json', 'test/fixtures/plan-count-permission-target-ad-v2.json', 'test/fixtures/ceo-finding-alias-af.json', 'test/fixtures/ceo-numbered-brief-af.json', 'test/ceo-contract-assertions-ag.test.ts', 'test/fixtures/ceo-contract-assertions-ag.json', 'test/fixtures/ceo-contract-assertions-ag-retry.json', 'test/fixtures/plan-count-permission-ah.json', 'test/ceo-parenthesized-issue-ah.test.ts', 'test/fixtures/ceo-parenthesized-issue-ah.json', 'test/ceo-section-choice-ai.test.ts', 'test/fixtures/ceo-section-choice-ai.json', 'test/fixtures/ceo-metadata-brief-ax.json', 'test/ceo-annotation-aj.test.ts', 'test/fixtures/ceo-annotation-aj.json', 'test/plan-count-crop-ak.test.ts', 'test/fixtures/plan-count-crop-ak.json', 'test/ceo-numbered-brief-ak.test.ts', @@ -318,6 +320,7 @@ export const E2E_TOUCHFILES: Record = { 'plan-eng-finding-count': [ "test/plan-count-session-cwd.test.ts", "test/eng-scheduled-regression.test.ts", + "test/eng-owned-explanation.test.ts", "test/fixtures/eng-owned-explanation.json", "test/eng-architecture-cache-av.test.ts", "test/fixtures/eng-architecture-cache-av-calls.json", diff --git a/test/plan-scope-selection.test.ts b/test/plan-scope-selection.test.ts index b431ec7af..215e7e756 100644 --- a/test/plan-scope-selection.test.ts +++ b/test/plan-scope-selection.test.ts @@ -27,6 +27,56 @@ const fixture = (text = announcements[0]!) => ({ }); const verdict = (f = fixture(), options = opts) => nativeSeededPlanSelection(f.transcript, f.tools, options); +const axAnnouncements = [ + `I'll invoke the /plan-design-review skill to review this landing page plan.`, + `I'll run the plan-design-review skill on your draft landing-page plan.`, +]; +test('AX title-derived descriptors bind the unique pasted plan before or after skill load', () => { + for (const text of axAnnouncements) for (const delta of [1, 6]) { + const f = fixture(text); f.transcript.assistantMessages[0]!.timestamp = timestamp(delta); + f.tools[0]!.timestamp = timestamp(4); f.tools[1]!.timestamp = timestamp(5); + expect(verdict(f), text).toBe(true); + } + for (const text of [ + `I will review this MARKETING landing-page plan.`, + `I'll review your draft landing page plan.`, + `I'll review the landing-page plan.`, + ]) expect(verdict(fixture(text))).toBe(true); + expect(verdict(fixture(axAnnouncements[0]!), { ...opts, seed: '# Plan: MARKETING landing-page\n' })).toBe(true); + expect(verdict(fixture(`I'll review this checkout plan.`), { ...opts, seed: '# Plan: Checkout page\n' })).toBe(true); +}); + +test('a descriptor must be a contiguous whole-word portion of the unique seed title', () => { + for (const descriptor of ['pricing page', 'Marketing page', 'land', 'landing pages', 'checkout', 'other landing page']) { + const text = `I'll review this ${descriptor} plan.`; + expect(verdict(fixture(text), { ...opts, seed: opts.seed + `\nBody mentions ${descriptor}.` }), descriptor).toBe(false); + } + expect(verdict(fixture(axAnnouncements[0]!), { ...opts, seed: opts.seed + '\n# Plan: Another landing page' })).toBe(false); +}); + +test('described targets retain the affirmative, Skill, timing and current-selection guards', () => { + for (const text of axAnnouncements) { + for (const invalid of [ + `> ${text}`, `"${text}"`, ` ${text}`, `Source:\n${text}`, `\`\`\`\n${text}\n\`\`\``, + text.replace("I'll", 'I might'), text.replace("I'll", "I won't"), text.replace(/\.$/, '?'), + `If approved, ${text}`, text.replace(/\.$/, ' if approved.'), + text.replace('plan-design-review', 'plan-eng-review'), text.replace(/\.$/, ' or another plan.'), + ]) expect(verdict(fixture(invalid)), invalid).toBe(false); + for (const mutate of [ + (f: ReturnType) => { f.transcript.assistantMessages[0]!.sessionId = 'foreign'; }, + (f: ReturnType) => { f.transcript.assistantMessages[0]!.timestamp = timestamp(0); }, + (f: ReturnType) => { f.tools[1]!.isError = true; }, + (f: ReturnType) => { f.tools[0]!.input!.skill = 'plan-eng-review'; }, + (f: ReturnType) => { f.tools[0]!.input!.args = 'Review the branch diff.'; }, + (f: ReturnType) => { f.tools.push({kind:'use',name:'Read',sessionId:'owned',toolUseId:'work',timestamp:timestamp(2.5)}); }, + ]) { const f = fixture(text); mutate(f); expect(verdict(f)).toBe(false); } + for (const correction of ['This selection is withdrawn.', 'The selected target is now the branch diff.', 'I will review "Another plan" plan.', 'I will review your checkout page plan.']) { + const f = fixture(text); f.transcript.assistantMessages.push({sessionId:'owned',timestamp:timestamp(4),text:correction}); + expect(verdict(f), correction).toBe(false); + } + } +}); + test('actual AF public selection wording needs this seed, parent and completed skill boundary', () => { for (const text of announcements) expect(verdict(fixture(text))).toBe(true); const f = fixture(); f.tools[0]!.input!.skill = 'gstack:plan-design-review'; expect(verdict(f)).toBe(true); diff --git a/test/skill-e2e-autoplan-chain.test.ts b/test/skill-e2e-autoplan-chain.test.ts index ee98f3305..cc07ca8ac 100644 --- a/test/skill-e2e-autoplan-chain.test.ts +++ b/test/skill-e2e-autoplan-chain.test.ts @@ -30,7 +30,7 @@ import { isNumberedOptionListVisible, selectPtyNumberedOption, } from './helpers/claude-pty-runner'; -import { autoplanArtifactPermissionInput, pendingAutoplanArtifactPermissionInput, publishedAutoplanArtifactPermissionInput, autoplanArtifactMenuKey } from './helpers/autoplan-artifact-permission'; +import { autoplanArtifactPermissionInput, pendingAutoplanArtifactPermissionInput, publishedAutoplanArtifactPermissionInput, autoplanArtifactMenuKey, autoplanPermissionProgressKey } from './helpers/autoplan-artifact-permission'; import { readPendingAutoplanArtifact, autoplanArtifactRecorderStatus } from './helpers/autoplan-artifact-recorder'; import { autoplanSetupDecision, autoplanBlockingQuestionBoundary, type AutoplanSetupDecision } from './helpers/autoplan-setup-question'; import { autoplanPhaseCompletions, type AutoplanPhaseHit } from './helpers/autoplan-phase-observer'; @@ -140,6 +140,7 @@ describeE2E('/autoplan native chain ordering (periodic)', () => { const budgetMs = AUTOPLAN_CHAIN_BUDGET.workMs; const start = Date.now(); let lastPermSig = ''; + let lastPermissionProgress = ''; const seenArtifactPermissions = new Set(); let lastCheckpointAt = start; const seenSetupQuestions = new Set(); @@ -186,10 +187,14 @@ describeE2E('/autoplan native chain ordering (periodic)', () => { // questions it owns. Classify on tail to avoid stale matches. const recentTail = visible.slice(-1500); if (isNumberedOptionListVisible(recentTail) && isPermissionDialogVisible(recentTail)) { - const sig = visible.slice(-500); + // A new acknowledged file mutation can lead to the same menu. + // Pending/failed/unrelated tools never reset an already sent choice. + const progress = transcript.status === 'ready' ? autoplanPermissionProgressKey(visible, publicTools) : undefined; + if (progress) lastPermissionProgress = progress; + const sig = JSON.stringify([visible.slice(-500), lastPermissionProgress]); if (sig !== lastPermSig) { lastPermSig = sig; - session.send('1\r'); + await selectPtyNumberedOption(session, 1); await Bun.sleep(2000); continue; }