From d4b1675be96bcd390d1fa107a26e1f3a88028e3d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 11 Sep 2026 05:23:28 +0000 Subject: [PATCH] test: recognize valid review evidence and bind legacy diff requests --- test/autoplan-artifact-permission.test.ts | 32 +++++ test/autoplan-clipped-suffix-aq.test.ts | 3 + test/autoplan-edit-digests-al.test.ts | 12 ++ test/autoplan-edit-prefix-ai.test.ts | 23 +++- test/autoplan-pending-artifact.test.ts | 28 ++++ test/design-count-review.test.ts | 113 +++++++++++++++++ test/devex-seed-coverage.test.ts | 116 +++++++++++++++++ test/eng-option-b-scope-al.test.ts | 15 ++- test/eng-owned-seeds-av.test.ts | 85 ++++++++++++- test/eng-scheduled-regression.test.ts | 39 ++++++ test/helpers/autoplan-artifact-permission.ts | 73 +++++++---- test/helpers/claude-pty-runner.ts | 55 +++++++- test/helpers/design-count-review.ts | 52 ++++++-- test/helpers/devex-seed-coverage.ts | 43 ++++++- test/helpers/eng-seeded-coverage.ts | 62 ++++++++- test/helpers/plan-scope-selection.ts | 7 +- test/plan-count-completion.test.ts | 127 +++++++++++++++++++ test/plan-scope-selection.test.ts | 41 +++++- 18 files changed, 861 insertions(+), 65 deletions(-) diff --git a/test/autoplan-artifact-permission.test.ts b/test/autoplan-artifact-permission.test.ts index 7105ff107..b40609b79 100644 --- a/test/autoplan-artifact-permission.test.ts +++ b/test/autoplan-artifact-permission.test.ts @@ -165,6 +165,38 @@ describe('owned Autoplan artifact edit permission', () => { expect(pick(r)).toBeNull(); // Existing context is not part of the requested deletion. }); + // AZ's public line 116 wraps at column five, not the old fixed column four. + // These small panes exercise the same renderer rule without a transcript corpus. + for (const [line, numbered, continuation] of [ + [7, ' 7 ', ' '], [17, ' 17 ', ' '], + [116, ' 116 ', ' '], [1024, ' 1024 ', ' '], + ] as const) test(`wrapped line ${line} binds its own marker column and exact requested bytes`, () => { + const r = replay(), old = 'Old first portion kept together', replacement = 'New first portion kept together'; + const before = Array.from({ length: line - 1 }, (_, n) => `Context ${n}`).concat(old, 'Context tail').join('\n'); + fs.writeFileSync(r.file, before); + r.context.publicTools.filter(event => event.name === 'Write').at(-1)!.input!.content = before; + const edit = r.context.publicTools.at(-1)!; + edit.input!.old_string = old; edit.input!.new_string = replacement; + const menu = r.viewport.slice(r.viewport.indexOf(' Do you want')); + const rows = `${numbered}-Old first portion\n${continuation}- kept together\n` + + `${numbered}+New first portion\n${continuation}+ kept together\n`; + const pane = rows + '╌'.repeat(20) + '\n' + menu; + r.viewport = pane; + expect(pick(r)).toEqual({ input: '1\r', signature: `${edit.sessionId}:${edit.toolUseId}`, file: r.file }); + expect(pick(r, new Set([pick(r)!.signature]))).toBeNull(); + for (const invalid of [ + pane.replaceAll(`\n${continuation}`, `\n${continuation.slice(1)}`), // left-shifted continuation + pane.replaceAll(`\n${continuation}`, `\n ${continuation}`), // right-shifted continuation + pane.replace(`${continuation}- kept`, `${continuation}+ kept`), // different kind + pane.replace(`${numbered}+New`, ` ${numbered}+New`), // mixed complete-row columns + `${continuation}- kept together\n` + pane, // no owning numbered row + pane.replace('New first portion', 'Foreign replacement'), + pane.replace(numbered, ' 0 '), + pane.replace(numbered, ' 01 '), + pane.replace(numbered, ' 9007199254740992 '), + ]) { r.viewport = invalid; expect(pick(r), invalid).toBeNull(); } + }); + test('an earlier unresolved mutation cannot make the latest completed Edit current', () => { const r = replay(); const events = r.context.publicTools; const edit = events.at(-1)!; events.splice(-1, 0, { ...structuredClone(edit), toolUseId: 'earlier-unresolved-edit', diff --git a/test/autoplan-clipped-suffix-aq.test.ts b/test/autoplan-clipped-suffix-aq.test.ts index f7f559917..b57dcfbc0 100644 --- a/test/autoplan-clipped-suffix-aq.test.ts +++ b/test/autoplan-clipped-suffix-aq.test.ts @@ -46,6 +46,9 @@ test('exact suffix, corresponding line, next line and complete crop content are (s:string)=>s.replace(/^ \+t\./,' +x.'), (s:string)=>s.replace(/^ \+t\./,' +t!'), (s:string)=>s.replace(/^ \+t\./,' +t.'),(s:string)=>s.replace(/^ \+t\./,' +t.'), (s:string)=>s.replace(/^ \+t\./,' -t.'),(s:string)=>s.replace(/^ \+t\./,' Source: t.'), + // A forged deletion marker cannot make rejected digest rows use legacy authority. + (s:string)=>s.replace(/^ \+t\./,' -t.').replace(/^ 139 /m,' 140 '), + (s:string)=>s.replace(/^ \+t\./,' -t.').replace('Snapshot consistency','Foreign consistency'), (s:string)=>s.replace(/^ 139 /m,' 140 '),(s:string)=>s.replace('Snapshot consistency','Foreign consistency'), (s:string)=>s.replace('authoritative gate','unrequested gate'),(s:string)=>'> source\n'+s, (s:string)=>s.replace('3. No','3. Maybe'),(s:string)=>s.replace('❯ 1. Yes','❯ 2. Yes'), diff --git a/test/autoplan-edit-digests-al.test.ts b/test/autoplan-edit-digests-al.test.ts index 6abf8e908..d71b31675 100644 --- a/test/autoplan-edit-digests-al.test.ts +++ b/test/autoplan-edit-digests-al.test.ts @@ -137,5 +137,17 @@ test.each([ r.context.pending!.editDigest={...digest,beforeSHA256:'malformed'};expect(pick(r)).toBeNull(); r.context.pending!.editDigest=digest; const viewport=r.viewport;r.viewport=r.viewport.replace(/^((?: {0,3}\d+ | {4})-).*$/gm,'$1Foreign unowned deletion');expect(pick(r)).toBeNull();r.viewport=viewport; + // The digest's request ownership remains binding through the legacy crop path. + r.context.pending!.editDigest={...digest,oldLineHashes:[autoplanEditLineHash('Context')]};expect(pick(r)).toBeNull(); + r.context.pending!.editDigest=digest; + if(c.rows.some(row=>/^[ ]*\d+ \+/.test(row))){ + r.viewport=viewport.replace(/^([ ]*\d+ \+).*$/gm,'$1Context');expect(pick(r)).toBeNull();r.viewport=viewport; + } + if(c.name==='leading partial deletion'){ + r.viewport=viewport.replace(' -full line',' -Context');expect(pick(r)).toBeNull();r.viewport=viewport; + } + if(c.name==='leading partial context'){ + r.viewport=viewport.replace(' full line',' +full line');expect(pick(r)).toBeNull();r.viewport=viewport; + } fs.unlinkSync(r.file);expect(pick(r)).toBeNull(); }); diff --git a/test/autoplan-edit-prefix-ai.test.ts b/test/autoplan-edit-prefix-ai.test.ts index 73aeacc63..8dd900bed 100644 --- a/test/autoplan-edit-prefix-ai.test.ts +++ b/test/autoplan-edit-prefix-ai.test.ts @@ -35,7 +35,7 @@ test('the exact retained prior diff output does not hide the current published o }); test('completed diff rows are ignored only before one complete current native panel', () => { - for (const prefix of [' 1 +Previous completed output\n\n', ' +cropped prior row\n 2 +next prior row\n +wrapped row\n\n', ' 1 -Old value\n 1 +New value\n\n']) { + for (const prefix of [' 1 +Previous completed output\n\n', ' +cropped prior row\n 12 +next prior row\n +wrapped row\n\n', ' 1 -Old value\n 1 +New value\n\n']) { const r = replay(); r.viewport = prefix + r.panel; expect(pick(r)?.input).toBe('1\r'); } }); @@ -51,11 +51,30 @@ test('competing headers, previous panels, misleading prose and quotes remain rej }); test('malformed completed-output gutters cannot become a panel delimiter', () => { - for (const prefix of [' 1 +wrong indent\n', ' 0 +zero line\n', ' 9007199254740992 +unsafe line\n', ' 1 +row\n +short wrap\n', ' 1 +row\n -wrong kind\n', ' +only a cropped fragment\n']) { + for (const prefix of [' 1 +wrong indent\n', ' 0 +zero line\n', ' 9007199254740992 +unsafe line\n', ' 11 +row\n +short wrap\n', ' 11 +row\n -wrong kind\n', ' +only a cropped fragment\n']) { const r = replay(); r.viewport = prefix + r.panel; expect(pick(r)).toBeNull(); } }); +for (const [numbered, continuation] of [ + [' 7 ', ' '], [' 17 ', ' '], + [' 116 ', ' '], [' 1024 ', ' '], +] as const) test(`completed prefix ${numbered.trim()} infers one column before checking cropped and wrapped rows`, () => { + const r = replay(); + const prefix = `${continuation}+leading cropped fragment\n${numbered}+Previous completed\n${continuation}+ output\n\n`; + r.viewport = prefix + r.panel; + expect(pick(r)?.input).toBe('1\r'); + for (const invalid of [ + prefix.replaceAll(continuation + '+', continuation.slice(1) + '+'), + prefix.replaceAll(continuation + '+', ' ' + continuation + '+'), + prefix.replace(continuation + '+ output', continuation + '- output'), + prefix + numbered.replace(/(\d+) /, '$10 ') + '+mixed column\n', + prefix.replace(numbered + '+', ' ' + numbered.trim() + ' +'), + prefix.replace(numbered + '+Previous completed\n', ''), + 'Example:\n' + prefix, + ]) { r.viewport = invalid + r.panel; expect(pick(r), invalid).toBeNull(); } +}); + test('the complete current header, exact target, menu and requested replacement remain binding', () => { for (const change of [ (r: ReturnType) => { r.viewport = r.viewport.replace('● Update(~/.gstack/', '● Update(/foreign/'); }, diff --git a/test/autoplan-pending-artifact.test.ts b/test/autoplan-pending-artifact.test.ts index 6867004d8..14dcb2146 100644 --- a/test/autoplan-pending-artifact.test.ts +++ b/test/autoplan-pending-artifact.test.ts @@ -116,6 +116,34 @@ test('queued unrelated public tools do not confer permission or block the curren r.context.publicTools.at(-1)!.name='Write';expect(pick(r)).toBeNull(); }); +for (const [line, numbered, next, continuation] of [ + [7, ' 7 ', ' 8 ', ' '], [17, ' 17 ', ' 18 ', ' '], + [116, ' 116 ', ' 117 ', ' '], [1024, ' 1024 ', ' 1025 ', ' '], +] as const) test(`legacy pending deletion line ${line} binds leading and wrapped fragments to its numbered column`, () => { + const r = replay(); + expect(r.context.pending?.editDigest).toBeUndefined(); + const before = Array.from({ length: line - 2 }, (_, n) => `Context ${n}`) + .concat('Head before crop tail', 'Old complete row', 'Context').join('\n'); + fs.writeFileSync(r.file, before); + const at = new Date(Date.parse(r.context.pending!.timestamp) - 1); fs.utimesSync(r.file, at, at); + const menu = r.screen.slice(r.screen.indexOf(' Do you want')); + const rows = `${continuation}-tail\n${numbered}-Old complete\n${continuation}- row\n` + + `${numbered}+New complete\n${continuation}+ row\n${next} Context\n`; + const pane = rows + '╌'.repeat(20) + '\n' + menu; + r.screen = pane; + expect(pick(r)?.input).toBe('1\r'); + expect(pick(r, new Set([pick(r)!.signature]))).toBeNull(); + for (const invalid of [ + pane.replaceAll(continuation + '-', continuation.slice(1) + '-'), + pane.replaceAll(continuation + '-', ' ' + continuation + '-'), + pane.replace(continuation + '- row', continuation + '+ row'), + pane.replace(next + ' Context', ' ' + next + ' Context'), + pane.replace('Old complete', 'Unrelated deleted'), + pane.replace(continuation + '-tail', continuation + '-foreign suffix'), + pane.replaceAll(numbered, ' 0 '), + ]) { r.screen = invalid; expect(pick(r), invalid).toBeNull(); } +}); + test.skipIf(process.platform==='win32')('real launcher installs only opt-in owned hooks and removes records on close or early exit',async()=>{ const root=fs.mkdtempSync(path.join(os.tmpdir(),'gstack-artifact-launch-'));roots.push(root); const fake=path.join(root,'fake-claude');fs.writeFileSync(fake,`#!${process.execPath}\n`+String.raw` diff --git a/test/design-count-review.test.ts b/test/design-count-review.test.ts index a15b3f22c..d88249c03 100644 --- a/test/design-count-review.test.ts +++ b/test/design-count-review.test.ts @@ -41,6 +41,119 @@ function replay(input: NativePlanQuestionCall[], first = isDesignCountFirstRevie return { ...counts, started, phases }; } +describe('a declared primary-action issue owns its native amendment and open gap', () => { + // Minimal AZ public question and choices; the full transcript stays local. + const current = (): NativePlanQuestionCall => { + const question = 'D2 — Issue 1 (G1): make Save the visible primary action\n' + + 'Project/branch/task: main branch, account-settings header action group.\n' + + 'ELI10: Four header buttons currently share one style. A user who just edited their email has to read all four labels to find the one that stores the change.'; + const options = [ + {label: '1A Apply DESIGN.md token (recommended)', description: 'Save filled #1d4ed8 white; Reset, Cancel, Export neutral ghost. Verify ghost text and border contrast.'}, + {label: '1B Spacing-only separation', description: 'Keep four equal buttons, add a gap before Save. Violates DESIGN.md.'}, + {label: '1C Defer', description: 'Leave G1 open and record it as unresolved.'}, + ]; + return {sessionId: 'az-design', toolUseId: 'primary', answered: true, failed: false, + unansweredQuestionIndices: [], answeredAt: '2026-09-11T04:04:10.000Z', + questions: [{header: 'Issue 1', question, options, multiSelect: false}], + answers: {[question]: options[0]!.label}}; + }; + const changed = (change: (q: NativePlanQuestionCall['questions'][number]) => void) => { + const c = current(), q = c.questions[0]!; change(q); + c.answers = {[q.question]: q.options[0]!.label}; return fingerprint(c); + }; + test('the declared gap starts review for every offered answer, with optional question punctuation', () => { + const c = current(), q = c.questions[0]!; + for (const option of q.options) { + c.answers = {[q.question]: option.label}; + expect(isDesignCountFirstReview(fingerprint(c))).toBe(true); + } + expect(isDesignCountFirstReview(changed(q => {q.question = q.question.replace('action\n', 'action?\n');}))).toBe(true); + expect(isDesignCountFirstReview(changed(q => { + q.question = q.question.replaceAll('Save', 'Publish').replace('(G1)', '(G7)').replace('Issue 1', 'Issue 3') + .replace('Four', '4').replace('share one style', 'look identical').replace('\nELI10:', '\n[P1]\nELI10:'); + q.header = 'Issue 3'; + q.options = q.options.map(o => ({label: o.label.replace(/^1/, '3'), description: o.description.replaceAll('Save', 'Publish') + .replace('#1d4ed8 white', '#ffee22 with black text').replace('Reset, Cancel, Export', 'Export/Reset/Cancel').replace('G1', 'G7')})); + }))).toBe(true); + expect(isDesignCountFirstReview(changed(q => { + q.question = q.question.replace(' (G1)', ''); q.options[2]!.description = 'Leave Issue 1 open and record it as unresolved.'; + }))).toBe(true); + expect(isDesignCountFirstReview(changed(q => { + q.question = q.question.replace(' (G1)', '').replace('action\n', 'action?\n'); + q.options[2]!.description = 'Leave Issue 1 open and record it as unresolved.'; + }))).toBe(true); + }); + test('current gap, distinct primary/peers, native authority and owned deferral are required together', () => { + const changes: Array<(q: NativePlanQuestionCall['questions'][number]) => void> = [ + q => {q.question = q.question.replace('currently share', 'used to share');}, + q => {q.question = q.question.replace('Four', 'Three');}, + q => {q.question = q.question.replace('ELI10:', '> ELI10:');}, + q => {q.question = q.question.replace('Four header', 'If approved, four header');}, + q => {q.question += '\nELI10: Four header buttons currently share one style.';}, + q => {q.question = 'Historical example:\n' + q.question;}, + q => {q.question = q.question.replace('\nELI10:', '\nSource example:\nELI10:');}, + q => {q.header = 'Issue 2';}, + q => {q.options[0]!.description = q.options[0]!.description.replace('Save filled', 'Publish filled');}, + q => {q.options[0]!.description = q.options[0]!.description.replace('Reset, Cancel, Export', 'Save, Cancel, Export');}, + q => {q.options[0]!.description = q.options[0]!.description.replace('Reset, Cancel, Export', 'Reset, Cancel, Cancel');}, + q => {q.options[0]!.description = q.options[0]!.description.replace('#1d4ed8', 'blue');}, + q => {q.options[0]!.description = q.options[0]!.description.replace('neutral ghost', 'filled primary');}, + q => {q.options[0]!.label = '2A Apply DESIGN.md token (recommended)';}, + q => {q.options[0]!.label = '1A Prepare the review';}, + q => {q.options[1]!.description = q.options[0]!.description; q.options[0]!.description = 'Prepare the review.';}, + q => {q.options[2]!.label = '2C Defer';}, + q => {q.options[2]!.description = 'Leave G2 open and record it as unresolved.';}, + q => {q.options[2]!.description = 'Leave G1 closed and record it as resolved.';}, + q => {q.options[2]!.description = 'Prepare the next review.';}, + q => {q.question = q.question.replaceAll('Save', 'Fix'); q.options[0]!.description = 'This applies the next review step.';}, + q => {q.options[0]!.description += ' This amendment keeps all four header buttons identical.';}, + q => {q.options[2]!.description += ' Correction: do not leave G1 open.';}, + q => {q.options[2]!.description += ' Correction: never defer Issue 1.';}, + q => {q.question += ' G1 is historical.';}, + q => {q.options[0]!.description += ' This amendment applies only to another project.';}, + ]; + for (const change of changes) expect(isDesignCountFirstReview(changed(change)), change.toString()).toBe(false); + }); + test('owned withdrawals and approval conditions cannot hide in any evidence body', () => { + for (const suffix of [ + ' This finding is withdrawn.', ' Issue 1 is "closed".', ' G1 is ‘resolved’.', + ' Assuming approval, proceed with this option.', ' G1 applies only if approved.', + ' This finding requires approval.', ' This token contract is withdrawn.', + ' G1 is "historical".', + ]) for (const owner of [-1, 0, 2]) { + expect(isDesignCountFirstReview(changed(q => { + if (owner === -1) q.question += suffix; + else q.options[owner]!.description += suffix; + })), owner + suffix).toBe(false); + } + for (const owner of [-1, 0, 2]) expect(isDesignCountFirstReview(changed(q => { + if (owner === -1) q.question += '\n"G1 is closed." G2 is closed.'; + else q.options[owner]!.description += ' "G1 is closed." G2 is closed.'; + }))).toBe(true); + expect(isDesignCountFirstReview(changed(q => { + q.options[0]!.description += ' "This amendment keeps all four header buttons identical."'; + q.options[2]!.description += ' "Correction: do not leave G1 open." Do not leave G2 open.'; + }))).toBe(true); + expect(isDesignCountFirstReview(changed(q => { + q.question += ' "G1 is historical." G2 is historical.'; + q.options[0]!.description += ' "This amendment applies only to another project."'; + }))).toBe(true); + }); + test('the new declaration preserves native completion, answer and signature checks', () => { + for (const change of [ + (c: NativePlanQuestionCall) => {c.answered = false;}, + (c: NativePlanQuestionCall) => {c.failed = true;}, + (c: NativePlanQuestionCall) => {delete c.answeredAt;}, + (c: NativePlanQuestionCall) => {c.answers = {};}, + (c: NativePlanQuestionCall) => {c.answers = {foreign: '1A Apply DESIGN.md token (recommended)'};}, + (c: NativePlanQuestionCall) => {c.unansweredQuestionIndices = [0];}, + (c: NativePlanQuestionCall) => {c.questions.push(structuredClone(c.questions[0]!));}, + (c: NativePlanQuestionCall) => {c.questions[0]!.multiSelect = true;}, + ]) {const c = current(); change(c); expect(isDesignCountFirstReview(fingerprint(c))).toBe(false);} + expect(isDesignCountFirstReview({...fingerprint(current()), signature: 'foreign'})).toBe(false); + }); +}); + describe('A descriptive hierarchy header owns its primary and peer controls', () => { // Minimal public excerpt of AY D3: retain its question, current gap and native // options, without copying the full review or its repeated option prose. diff --git a/test/devex-seed-coverage.test.ts b/test/devex-seed-coverage.test.ts index 85a3db64c..420b82314 100644 --- a/test/devex-seed-coverage.test.ts +++ b/test/devex-seed-coverage.test.ts @@ -13,6 +13,122 @@ function extra(id: string, sessionId: string): NativePlanQuestionCall { return {sessionId,toolUseId:id,questions:[{header:'Extra',question,multiSelect:false,options:[{label:'Add command',description:'Add the command after the beta.'},{label:'Defer',description:'Defer the command.'}]}],answered:true,failed:false,answers:{[question]:'Defer'},unansweredQuestionIndices:[],answeredAt:'2026-09-09T20:23:00Z'}; } +// Minimal public AZ D6 evidence and offered correction. Keep the exact full +// failed attempt for replay; recognizing this decision grants no paid pass. +function evidenceTranscript(): PlanCountTranscript { + const t = transcript(), c = t.calls[2]!, q = c.questions[0]!; + q.question = [ + 'D6 — Journey stage REAL USAGE: the two public evaluation functions take the same two arguments in opposite positional order', + 'Project/branch/task: EvalKit beta DX review, branch main.', + 'Evidence: docs/api.md lines 5-9: `run_eval(dataset, evaluator)` and `run_batch(evaluator, dataset)`. Both arguments describe the same concepts; the reversed order is described as intentional; neither requires keywords.', + 'ELI10: Your ML engineer learns `run_eval(dataset, evaluator)` from the demo, then scales up to `run_batch` and writes the arguments in the same order.', + ].join('\n'); + q.options = [ + { label: 'A) Align to (dataset, evaluator) (recommended)', description: 'Same order in both functions, keywords accepted, swap detected with a clear error during beta.' }, + { label: 'B) Make both keyword-only', description: 'Force run_eval(dataset=..., evaluator=...) and same for run_batch.' }, + { label: 'C) Keep order, distinct types', description: 'Leave positional order; rely on type annotations to flag swaps.' }, + { label: 'D) Acceptable friction, skip', description: 'Keep the reversed order as documented.' }, + ]; + c.answers = { [q.question]: q.options[0]!.label }; + return t; +} + +describe('DX signature evidence within the current decision', () => { + test('the observed correction binds one distinct seed, including genuine alternate answers', () => { + const t = evidenceTranscript(), c = t.calls[2]!, q = c.questions[0]!; + for (const option of q.options) { + c.answers = { [q.question]: option.label }; + expect(devexSeedCoverage(t).complete).toBe(true); + expect(devexSeedCoverage(t).decisions['reversed-arguments']).toEqual([`${c.sessionId}:${c.toolUseId}`]); + } + t.calls.splice(2, 1); + expect(devexSeedCoverage(t).missing).toEqual(['reversed-arguments']); + }); + test('citation location, formatting and repair prose can vary without changing the evidence', () => { + for (const edit of [ + (s: string) => s.replace('opposite positional order\n', 'reversed positional order.\n'), + (s: string) => s.replace('public evaluation functions', 'public functions').replace('docs/api.md lines 5-9', 'docs/public-api.md:12–16'), + (s: string) => s.replaceAll('`', '').replaceAll('(dataset, evaluator)', '( dataset , evaluator )'), + (s: string) => s.replace('ELI10: Your', 'Impact: Your').replace('Evidence: docs', 'ELI10: docs'), + ]) { const t = evidenceTranscript(); changeDeclaration(t, 2, edit); expect(devexSeedCoverage(t).complete).toBe(true); } + const t = evidenceTranscript(), c = t.calls[2]!, q = c.questions[0]!; + q.options[0] = { label: 'Unify call order to (dataset, evaluator)', description: 'Both functions use the same positional order; keywords supported; swaps are rejected with an actionable message.' }; + c.answers = { [q.question]: q.options[0]!.label }; + expect(devexSeedCoverage(t).complete).toBe(true); + }); + test('the asserted pair cannot come from healthy, foreign, borrowed or quoted evidence', () => { + for (const edit of [ + (s: string) => s.replace('opposite positional order', 'the same positional order'), + (s: string) => s.replace('Journey stage REAL USAGE: ', 'Journey stage REAL USAGE: If approved, '), + (s: string) => '> ' + s, + (s: string) => s.replace('`run_batch(evaluator, dataset)`', '`run_batch(dataset, evaluator)`'), + (s: string) => s.replace('`run_batch(evaluator, dataset)`', '`other_batch(evaluator, dataset)`'), + (s: string) => s.replace('`run_eval(dataset, evaluator)`', '`other_eval(dataset, evaluator)`'), + (s: string) => s.replace(' and `run_batch(evaluator, dataset)`', ''), + (s: string) => s.replace(' and `run_batch(evaluator, dataset)`', '\nEvidence: docs/api.md: `run_batch(evaluator, dataset)`'), + (s: string) => s.replace('Evidence: ', 'Evidence: Another issue is worth discussing. '), + (s: string) => s + '\nELI10: Another explanation.', + ...['> ', 'Source excerpt: ', 'Historical example: ', 'If approved: ', '"', '`'].map(prefix => (s: string) => s.replace('Evidence: ', 'Evidence: ' + prefix)), + (s: string) => s.replace(/^(Evidence:.*)$/m, '```\n$1\n```'), + (s: string) => s.replace(/^(Evidence:.*)\n(ELI10:.*)$/m, '$2\n$1'), + ]) { + const t = evidenceTranscript(); changeDeclaration(t, 2, edit); + expect(devexSeedCoverage(t).missing, edit(t.calls[2]!.questions[0]!.question)).toContain('reversed-arguments'); + } + }); + test('current withdrawals defeat the evidence while literal historical quotations do not', () => { + for (const status of [ + 'These functions are now aligned.', 'These signatures are historical.', + 'This evidence is withdrawn.', 'This evidence is no longer current.', 'This evidence is cancelled.', 'This evidence is hypothetical.', + 'This finding applies only if approved.', 'D6 is cancelled.', + ]) for (const quoted of [false, true]) { + const t = evidenceTranscript(); + changeDeclaration(t, 2, s => s.replace(/^(Evidence:.*)$/m, '$1 ' + (quoted ? JSON.stringify(status) : status))); + expect(devexSeedCoverage(t).complete, `${quoted}: ${status}`).toBe(quoted); + } + const t = evidenceTranscript(); changeDeclaration(t, 2, s => s + '\nThis evidence is "withdrawn".'); + expect(devexSeedCoverage(t).missing).toContain('reversed-arguments'); + const scalar = evidenceTranscript(); changeDeclaration(scalar, 2, s => s + "\nThis evidence is 'withdrawn'."); + expect(devexSeedCoverage(scalar).missing).toContain('reversed-arguments'); + }); + test('one current offered action must align this pair and retain the swap correction', () => { + for (const edit of [ + (s: string) => s.replace('Same order', 'Opposite order'), + (s: string) => s.replace('both functions', 'other functions'), + (s: string) => s.replace('both functions', 'both functions run_score and run_many'), + (s: string) => s.replace('keywords accepted, ', ''), + (s: string) => s.replace('swap detected', 'swap ignored'), + (s: string) => s.replace('clear error', 'generic failure'), + (s: string) => 'If approved, ' + s, + (s: string) => JSON.stringify(s), + (s: string) => s + ' Correction: this option is withdrawn.', + (s: string) => s + ' This option is historical.', + (s: string) => s + ' This correction applies to another project.', + (s: string) => s + ' Do not align these functions.', + ]) { + const t = evidenceTranscript(); t.calls[2]!.questions[0]!.options[0]!.description = edit(t.calls[2]!.questions[0]!.options[0]!.description!); + expect(devexSeedCoverage(t).missing, edit.name).toContain('reversed-arguments'); + } + const t = evidenceTranscript(), c = t.calls[2]!, q = c.questions[0]!; + q.options[0]!.label = 'Align to (evaluator, dataset)'; c.answers = { [q.question]: q.options[0]!.label }; + expect(devexSeedCoverage(t).missing).toContain('reversed-arguments'); + q.options = q.options.slice(1); c.answers = { [q.question]: q.options[0]!.label }; + expect(devexSeedCoverage(t).missing).toContain('reversed-arguments'); + }); + test('native completion, session ownership and batching gates still govern the new evidence', () => { + for (const mutate of [ + (c: NativePlanQuestionCall) => { c.answered = false; }, + (c: NativePlanQuestionCall) => { c.failed = true; }, + (c: NativePlanQuestionCall) => { c.answeredAt = 'invalid'; }, + (c: NativePlanQuestionCall) => { c.sessionId = 'foreign'; }, + (c: NativePlanQuestionCall) => { c.unansweredQuestionIndices = [0]; }, + (c: NativePlanQuestionCall) => { c.answers = { 'Other question': c.questions[0]!.options[0]!.label }; }, + (c: NativePlanQuestionCall) => { c.questions[0]!.multiSelect = true; }, + (c: NativePlanQuestionCall) => { c.questions.push(structuredClone(c.questions[0]!)); }, + ]) { const t = evidenceTranscript(); mutate(t.calls[2]!); expect(devexSeedCoverage(t).complete).toBe(false); } + }); +}); + // Exact public AY headings and signature trace, applied to the existing native // completion fixture. Full public replay remains separate from paid-run credit. const tracedAyTitles = [ diff --git a/test/eng-option-b-scope-al.test.ts b/test/eng-option-b-scope-al.test.ts index 7cb950719..d22de25f2 100644 --- a/test/eng-option-b-scope-al.test.ts +++ b/test/eng-option-b-scope-al.test.ts @@ -4,18 +4,25 @@ import type { NativePublicToolEvent, PlanCountTranscript } from './helpers/plan- import { E2E_TOUCHFILES, selectTests } from './helpers/touchfiles'; import fixture from './fixtures/eng-option-b-scope-al.json'; -const input = (attempt = 1) => structuredClone(fixture.attempts[attempt]!.projection); +const actualInput = (attempt = 1) => structuredClone(fixture.attempts[attempt]!.projection); +const input = () => { + const p = actualInput(); + // Mutate the named declaration alone; the earlier spoken introduction is + // independently valid and remains present in the exact replays below. + p.transcript.assistantMessages = p.transcript.assistantMessages.filter(m => m.text !== "I'll run the eng review skill on this draft plan."); + return p; +}; type Input = ReturnType; const verdict = (p = input()) => nativeSeededPlanSelection(p.transcript as PlanCountTranscript, p.tools as NativePublicToolEvent[], p.opts); const declaration = (p: Input) => p.transcript.assistantMessages.find(m => m.text.startsWith("I've selected option B,"))!; test('both named retry and fresh unique-draft first introduction bind; original outcomes stay intact', () => { expect(fixture.attempts.map(a => a.rawScopeGateAutoSelectObserved)).toEqual([false, false]); - expect(verdict(input(0))).toBe(true); - expect(verdict(input(1))).toBe(true); + expect(verdict(actualInput(0))).toBe(true); + expect(verdict(actualInput(1))).toBe(true); }); -test('the named target is necessary; an unnamed option-B notice supplies no selection', () => { +test('an unnamed option-B notice supplies no selection without a draft introduction', () => { const p = input(); p.transcript.assistantMessages = p.transcript.assistantMessages.filter(m => m !== declaration(p)); expect(verdict(p)).toBe(false); for (const replacement of ['option A,', 'option C,', 'option B if approved,', 'option B, possibly']) { diff --git a/test/eng-owned-seeds-av.test.ts b/test/eng-owned-seeds-av.test.ts index c22ddcd02..9858f977d 100644 --- a/test/eng-owned-seeds-av.test.ts +++ b/test/eng-owned-seeds-av.test.ts @@ -82,7 +82,7 @@ describe('Eng current decomposition and post-rewrite error choices', () => { }); test.each([0,1])('owned scalar statuses, including Markdown, close family %i',i=>{ for(const status of ['withdrawn','no longer current','hypothetical','resolved'])for(const [open,close]of [['',''],['"','"'],["'","'"],['‘','’'],['`','`']])for(const bold of ['', '**']) { - for(const owner of ['This finding',`D${i===0?1:5}`])rejected(i,c=>question(c,s=>`${s}\n${bold}${owner}${bold} is ${open}${status}${close}.`)); + for(const owner of ['This finding','This decision',`D${i===0?1:5}`])rejected(i,c=>question(c,s=>`${s}\n${bold}${owner}${bold} is ${open}${status}${close}.`)); rejected(i,c=>options(c,o=>{o.description+=`\n${bold}This option${bold} is ${open}${status}${close}.`;})); } }); @@ -91,7 +91,7 @@ describe('Eng current decomposition and post-rewrite error choices', () => { for(const status of ['This option proceeds once approved.','This remedy applies only if the user agrees.','Do not '+(i===0?'cut AuthCache and TokenStore.':'split or flatten the function.')])rejected(i,c=>options(c,o=>{o.description+='\n'+status;})); }); test.each([0,1])('foreign and quoted historical withdrawals do not close family %i',i=>{ - const c=fresh(i);question(c,s=>s+'\nD99 is withdrawn.\nEarlier reviewer said "This finding is withdrawn."'); + const c=fresh(i);question(c,s=>s+'\nD99 is withdrawn.\nEarlier reviewer said "This finding is withdrawn." Earlier reviewer said "This decision is withdrawn."'); options(c,o=>{o.description+='\nEarlier reviewer said "This option is withdrawn."';}); expect(evaluate([c]).decisions[seeds[i]!]).toBeDefined(); }); @@ -136,3 +136,84 @@ test('current named-object resolutions supersede the owned defect',()=>{ const scope=fresh(0);question(scope,s=>s+'\nvalidateAndDispatch() now rethrows every error.');expect(evaluate([scope]).decisions.complexity).toBeDefined(); const errors=fresh(1);question(errors,s=>s+'\nAuthCache now has independent policy rules.');expect(evaluate([errors]).decisions['swallowed-errors']).toBeDefined(); }); + +// Exact public AZ question; synthetic identity/time only, without recrediting the failed run. +const step0InventoryQuestion = { + "question": "D3 — Step 0 complexity check: reduce the plan's moving parts, or proceed as-is?\nProject/branch/task: main, eng-reviewing PLAN.md (Multi-tenant Auth Refactor).\nELI10: PLAN.md:35-36 says this touches 12 files and adds 4 new classes (TokenStore, SessionMint, AuthCache, RequestPolicy) plus the AuthBroker service. That trips the complexity smell (8+ files or 2+ new classes). PLAN.md:7-13 also says an existing cache adapter already keys tokens by tenant, evicts, and invalidates on logout/revocation/suspension, and AuthCache is just a facade over it. So TokenStore looks like a second token store next to the one you already have, and RequestPolicy is a class for logic that currently has one consumer. Fewer new nouns means fewer places a tenant-isolation bug can hide and a smaller diff to review.\nStakes if we pick wrong: over-reduce and you re-add a class mid-implementation; under-reduce and you maintain two token stores with two invalidation stories, which is exactly how cross-tenant cache leaks start.\nRecommendation: A because the existing adapter already does what TokenStore describes, and RequestPolicy can start as a plain function and become a class when a second caller appears (engineered enough, not over-engineered).\nNote: options differ in kind, not coverage — no completeness score. Caveat: I cannot read the source here, so if TokenStore holds something the adapter does not (refresh tokens, mint receipts), say so and keep it.\nNet: 3 new classes with one backing store vs. 4 classes and a duplicate store vs. no facade at all.", + "header": "Scope", + "multiSelect": false, + "options": [ + { + "label": "A) Reduce: cut TokenStore, demote RequestPolicy (recommended)", + "description": "✅ One token store, one invalidation story: the existing adapter behind the AuthCache facade. (human: ~1 day less / CC: ~10 min less)\n✅ AuthCache facade stays as the single seam where the shared-state fix lands in Section 1.\n❌ If TokenStore was meant to hold data the adapter cannot key, you add it back later. ~8 files, 3 new classes." + }, + { + "label": "B) Proceed as-is: 4 classes, 12 files", + "description": "✅ No re-planning; every component named in the plan ships in this PR. (human: ~1 week / CC: ~1 hr)\n✅ RequestPolicy as a class is ready for a second consumer on day one.\n❌ Two token-holding components (TokenStore + adapter) means two invalidation paths to keep consistent under tenant suspension." + }, + { + "label": "C) Reduce harder: no AuthCache facade, inject adapter directly", + "description": "✅ Smallest diff: 2 new services, ~6 files, zero new cache classes. (human: ~3 days / CC: ~30 min)\n✅ Both services depend on the adapter interface the existing tests already cover.\n❌ Loses the one place to serialize mutations and add tenant-scoped guards; both services must re-implement that themselves." + } + ] +}; + +function step0InventoryCall(): NativePlanQuestionCall { + const q=structuredClone(step0InventoryQuestion); + return {sessionId:'step0-inventory',toolUseId:'owned-decision',questions:[q],answered:true,failed:false, + answers:{[q.question]:q.options[0]!.label},unansweredQuestionIndices:[],answeredAt:new Date(start+1000).toISOString()}; +} +const inventorySeed=(c:NativePlanQuestionCall)=>evaluate([c]).decisions.complexity; +test('a current Step 0 decision owns its inventory and reduction in the explanation',()=>{ + expect(inventorySeed(step0InventoryCall())).toBe('step0-inventory:owned-decision'); + for(const option of step0InventoryQuestion.options){const c=step0InventoryCall();c.answers={[c.questions[0]!.question]:option.label};expect(inventorySeed(c)).toBeDefined();} + const c=step0InventoryCall();question(c,s=>s.replace('touches 12 files','touches 13 files'));options(c,o=>{o.label=o.label.replace('12 files','13 files');});expect(inventorySeed(c)).toBeDefined(); + question(c,s=>s+'\nD99 is withdrawn.\nEarlier reviewer said "This finding is withdrawn." Earlier reviewer said "This decision is withdrawn."');expect(inventorySeed(c)).toBeDefined(); +}); +test('Step 0 inventory, present overlap, and a current single-option reduction are required',()=>{ + for(const [before,after] of [ + ['says this touches','might touch'],['adds 4 new classes','adds 5 new classes'], + ['(TokenStore, SessionMint, AuthCache, RequestPolicy)','(TokenStore, SessionMint, AuthCache, TokenStore)'], + ['plus the AuthBroker service','plus another service'],['already keys tokens by tenant','might someday key tokens by tenant'], + ['AuthCache is just a facade over it','AuthCache has independent policy rules'], + ['looks like a second token store next to the one you already have','stores different data from the adapter'], + ['currently has one consumer','already has two consumers'],['ELI10: ','ELI10: Source: '], + ['ELI10: ','ELI10: Historical assessment. '],['ELI10: ','ELI10: If approved, '],['ELI10: ','ELI10: If the user approves, '], + ]){const c=step0InventoryCall();question(c,s=>s.replace(before!,after!));expect(inventorySeed(c)).toBeUndefined();} + for(const transform of [(s:string)=>s.replace(/^ELI10: (.*)$/m,'ELI10: "$1"'),(s:string)=>s.replace(/^ELI10:.*$/m,'ELI10: General naming discussion.'), + (s:string)=>s+'\nThis decision applies only if the user agrees.',(s:string)=>s+'\nTokenStore now has a documented independent purpose.',(s:string)=>s+'\nRequestPolicy now has a second consumer.']){ + const c=step0InventoryCall();question(c,transform);expect(inventorySeed(c)).toBeUndefined(); + } + for(const change of [ + (c:NativePlanQuestionCall)=>{c.questions[0]!.options[0]!.label='A) Keep TokenStore (recommended)';}, + (c:NativePlanQuestionCall)=>{c.questions[0]!.options[0]!.description='Choose later.';}, + (c:NativePlanQuestionCall)=>{const q=c.questions[0]!;q.options[1]!.description=q.options[0]!.description;q.options[0]!.description='Choose later.';}, + (c:NativePlanQuestionCall)=>{c.questions[0]!.options[1]!.label='B) Proceed as-is: 5 classes, 12 files';}, + (c:NativePlanQuestionCall)=>{c.questions[0]!.options[1]!.description='There is one storage layer already.';}, + (c:NativePlanQuestionCall)=>{c.questions[0]!.options[0]!.description='Source:\n'+c.questions[0]!.options[0]!.description;}, + (c:NativePlanQuestionCall)=>{c.questions[0]!.options[0]!.description+='\nDo not cut TokenStore.';}, + (c:NativePlanQuestionCall)=>{c.questions[0]!.options[0]!.description+='\nThis remedy proceeds once approved.';}, + ]){const c=step0InventoryCall();change(c);c.answers={[c.questions[0]!.question]:c.questions[0]!.options[0]!.label};expect(inventorySeed(c)).toBeUndefined();} +}); +test('current scalar statuses and completed native ownership still gate the Step 0 decision',()=>{ + for(const scalar of ['withdrawn',"'no longer current'",'`no longer current`','conditional on approval'])for(const owner of ['finding','decision','repair','opposed']){ + const c=step0InventoryCall();if(owner==='finding'||owner==='decision')question(c,s=>s+`\nThis ${owner} is ${scalar}.`); + else c.questions[0]!.options[owner==='repair'?0:1]!.description+=`\nThis option is ${scalar}.`; + expect(inventorySeed(c)).toBeUndefined(); + } + for(const change of [(c:NativePlanQuestionCall)=>{c.answered=false;},(c:NativePlanQuestionCall)=>{c.failed=true;},(c:NativePlanQuestionCall)=>{c.answers={};},(c:NativePlanQuestionCall)=>{c.answeredAt=new Date(end+1).toISOString();}]){ + const c=step0InventoryCall();change(c);expect(inventorySeed(c)).toBeUndefined(); + } +}); + +// Meaning-preserving wording keeps the same inventory, overlap and opposed repairs. +test('the Step 0 relations do not depend on the original paragraph or option prose',()=>{ + const c=step0InventoryCall();question(c,s=>s.replace('complexity check: reduce','complexity decision: simplify') + .replace('says this touches 12 files and adds 4 new classes','changes 12 files and introduces 4 new classes') + .replace('an existing cache adapter already keys','the current adapter keys').replace('AuthCache is just a facade over it','AuthCache remains a facade for that adapter') + .replace('TokenStore looks like a second token store next to the one you already have','TokenStore duplicates the current adapter token storage') + .replace('RequestPolicy is a class for logic that currently has one consumer','RequestPolicy serves a single consumer')); + const q=c.questions[0]!;q.options[0]!.label='A) Remove TokenStore; make RequestPolicy a plain function';q.options[0]!.description='Keep the current adapter as the single backing store behind AuthCache.'; + q.options[1]!.label='B) Keep the plan';q.options[1]!.description='Retain 4 classes across 12 files, with two invalidation paths.';c.answers={[q.question]:q.options[0]!.label}; + expect(inventorySeed(c)).toBeDefined(); +}); diff --git a/test/eng-scheduled-regression.test.ts b/test/eng-scheduled-regression.test.ts index 2af089818..4586d8fdd 100644 --- a/test/eng-scheduled-regression.test.ts +++ b/test/eng-scheduled-regression.test.ts @@ -73,3 +73,42 @@ test.each(controls)('%s cannot provide mandatory regression coverage', (_, chang test('the regression evidence test selects its existing Eng workflow', () => { expect(Object.entries(E2E_TOUCHFILES).filter(([, paths]) => paths.includes('test/eng-scheduled-regression.test.ts')).map(([name]) => name)).toEqual(['plan-eng-finding-count']); }); + +// Verbatim owned rule, T1 and ordered verification from the failed AZ report. +const orderedRuleReport = "# Current reviewed plan\n\n### REGRESSION RULE — CRITICAL, no approval needed (skill iron rule)\n\nPLAN.md:27-28 rewrites `legacyAuthFlow()` with no regression test;\nPLAN.md:14-16 excluded it from coverage. That is modified existing behavior\nwith no covering test. **Before** the rewrite, add\n`legacyAuthFlow.characterization.test.ts` capturing current outputs for:\nvalid token, expired token, wrong tenant, wrong audience, revoked token,\nIDP unavailable. The rewrite must pass the same suite unchanged.\n\n## Implementation Tasks\n- [ ] **T1 (P1, human: ~half day / CC: ~15min)** — legacyAuthFlow — Write characterization suite for 6 prior behaviors BEFORE rewrite\n - Surfaced by: Test review — REGRESSION RULE, PLAN.md:27-28 and 14-16\n - Files: `legacyAuthFlow.characterization.test.ts`\n - Verify: suite green on current code; green again after rewrite\n\n## Verification (end to end)\n1. Run T1's characterization suite on unmodified code: green.\n2. Implement T2-T6; run unit suites: green, no shared-state ordering flakes (run with shuffled order).\n3. Run T7 E2E: A/B isolation, suspend-mid-mint denial, double-submit consistency all green.\n4. Re-run T1 after the rewrite: green, unchanged.\n"; + +test('an iron-rule declaration and ordered task verification establish the mandatory baseline',()=>{ + expect(regression(orderedRuleReport)).toBe('plan'); + for(const change of [(s:string)=>s.replaceAll('T1','T17'),(s:string)=>s.replaceAll('legacyAuthFlow.characterization.test.ts','spec/legacy-golden.test.ts'),(s:string)=>s.replace(/[`*]/g,''), + (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(regression(change(orderedRuleReport))).toBe('plan'); +}); +test('the ordered baseline stays owned, required, and unchanged across the rewrite',()=>{ + for(const [before,after] of [ + ['no approval needed','optional if approved'],['skill iron rule','hypothetical example'],['legacyAuthFlow','otherAuthFlow'], + ['**Before** the rewrite','After the rewrite'],['capturing current outputs','capturing expected outputs'], + ['The rewrite must pass the same suite unchanged.','The rewrite may update the expectations.'], + ['suite green on current code; green again after rewrite','suite green on changed code; green again after rewrite'], + ['suite green on current code','suite is not green on current code'],['green again after rewrite','not green again after rewrite'], + ['on unmodified code: green.','on unmodified code: not green.'],['after the rewrite: green, unchanged.','after the rewrite: failing, unchanged.'], + ['1. Run T1','1. Run T9'],['on unmodified code: green','on changed code: green'],['1. Run ','1. If approved, Run '], + ['4. Re-run T1','4. Re-run T9'],['green, unchanged.','green, with updated expectations.'], + ['## Implementation Tasks\n','## Implementation Tasks\nSource:\n'],['## Implementation Tasks\n','## Implementation Tasks\nOnce approved,\n'], + ['PLAN.md:27-28','Source:\nPLAN.md:27-28'],['Current reviewed plan','Historical reviewed plan'], + ]){const changed=orderedRuleReport.replaceAll(before!,after!);expect(changed).not.toBe(orderedRuleReport);expect(regression(changed)).toBeUndefined();} + for(const change of [(s:string)=>s.replace(/^ - Files: .*$/m,' - Files: different.test.ts'),(s:string)=>s.replace(/^1\. .*$/m,''), + (s:string)=>s.replace(/^1\. .*$/m,'1. Rewrite legacyAuthFlow() before recording T1.'),(s:string)=>s.replace(/^(1\. )(.*)$/m,'$1"$2"'), + (s:string)=>s.replace(/^4\. .*$/m,''),(s:string)=>s.split('\n').map(l=>'> '+l).join('\n'),(s:string)=>'```\n'+s+'\n```', + (s:string)=>s+'\n## Current assessment\nT1 is withdrawn.\n',(s:string)=>s+'\n## Current assessment\nT1 verification is "optional".\n', + (s:string)=>s+'\n## Current assessment\nDo not run T1.\n',(s:string)=>s+'\n## Current assessment\nlegacyAuthFlow() is rewritten before T1.\n', + (s:string)=>s+'\n## Current assessment\nUpdate T1 assertions.\n']){const changed=change(orderedRuleReport);expect(changed).not.toBe(orderedRuleReport);expect(regression(changed)).toBeUndefined();} +}); + +test('required regression relations survive heading, task and verification paraphrases',()=>{ + const changed=orderedRuleReport.replace('REGRESSION RULE — CRITICAL, no approval needed (skill iron rule)','Required characterization baseline') + .replace('The rewrite must pass the same suite unchanged.','The same suite must remain green unchanged after the rewrite.') + .replace('Write characterization suite for 6 prior behaviors BEFORE rewrite','Add characterization tests for existing outputs') + .replace('suite green on current code; green again after rewrite','current implementation passes; after the rewrite the suite passes again') + .replace("1. Run T1's characterization suite on unmodified code: green.",'1) Execute characterization task T1 against untouched code; it must pass.') + .replace('4. Re-run T1 after the rewrite: green, unchanged.','4) Execute the same T1 tests unchanged after the refactor; they must pass.'); + expect(regression(changed)).toBe('plan'); +}); diff --git a/test/helpers/autoplan-artifact-permission.ts b/test/helpers/autoplan-artifact-permission.ts index 36d997c7f..054ab0045 100644 --- a/test/helpers/autoplan-artifact-permission.ts +++ b/test/helpers/autoplan-artifact-permission.ts @@ -2,7 +2,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { createHash } from 'node:crypto'; -import { validAutoplanEditDigest, readAutoplanDigestFile, matchesAutoplanDigestRows, createAutoplanEditDigest } from './autoplan-artifact-digest'; +import { validAutoplanEditDigest, readAutoplanDigestFile, matchesAutoplanDigestRows, createAutoplanEditDigest, autoplanEditLineHash } from './autoplan-artifact-digest'; import type { PendingAutoplanArtifact } from './autoplan-artifact-recorder'; import type { NativePublicToolEvent } from './plan-count-transcript'; @@ -49,21 +49,24 @@ function ownedEditDiffRows(rows: string[], file: string, ownedStateRoot?: string // A completed native tool's diff may remain above the active edit panel. // Only its indented diff output is ignored; competing panels or prose are // not evidence for the current request and cannot be used as a prefix. + const prefix = rows.slice(0, header), fullRow = /^ {6}([1-9]\d*) ([+ -])/; + const first = prefix.map(row => fullRow.exec(row)).find(Boolean); + if (!first) return null; + const markerColumn = 6 + first[1]!.length + 1; let kind: string | undefined; - let numbered = 0; - for (const row of rows.slice(0, header)) { + for (const row of prefix) { if (!row.trim()) continue; - const full = /^ {6}(\d+) ([+ -])/.exec(row); + const full = fullRow.exec(row); if (full) { - if (!Number.isSafeInteger(Number(full[1])) || Number(full[1]) < 1) return null; - numbered++; kind = full[2]; + if (!Number.isSafeInteger(Number(full[1])) || 6 + full[1]!.length + 1 !== markerColumn) return null; + kind = full[2]; } else { - const wrap = /^ {9}([+ -])/.exec(row); - if (!wrap || (kind !== undefined && wrap[1] !== kind)) return null; - kind = wrap[1]; + const wrappedKind = row[markerColumn]; + if (!row.startsWith(' '.repeat(markerColumn)) || !['+', ' ', '-'].includes(wrappedKind ?? '') || + (kind !== undefined && wrappedKind !== kind)) return null; + kind = wrappedKind; } } - if (!numbered) return null; rows = rows.slice(header); } // A redraw can repeat the same native tool title above one current panel. @@ -146,17 +149,21 @@ function matchesCroppedEdit(viewport: string, file: string, before: string, remo const diffRows = ownedEditDiffRows(rows,file,ownedStateRoot); if (!diffRows) return false; const chunks: Array<{ kind: string; text: string }> = []; + let markerColumn: number | undefined; for (const row of diffRows) { - const numbered = /^ {0,3}(\d+) ([+ -])(.*)$/.exec(row); + const numbered = /^( {0,3})([1-9]\d*) ([+ -])(.*)$/.exec(row); if (numbered) { - const line = Number(numbered[1]); - if (!Number.isSafeInteger(line) || line < 1) return false; - chunks.push({ kind: numbered[2]!, text: numbered[3]! }); + const line = Number(numbered[2]), column = numbered[1]!.length + numbered[2]!.length + 1; + // Match the digest parser: every row owns one marker column, regardless + // of line-number width; continuations retain that column and diff kind. + if (!Number.isSafeInteger(line) || (markerColumn !== undefined && markerColumn !== column)) return false; + markerColumn = column; + chunks.push({ kind: numbered[3]!, text: numbered[4]! }); } else { - const wrapped = /^ {4}([+ -])(.*)$/.exec(row); const last = chunks.at(-1); - if (!wrapped || !last || wrapped[1] !== last.kind) return false; - last.text += wrapped[2]!; + if (markerColumn === undefined || !row.startsWith(' '.repeat(markerColumn)) || + !last || row[markerColumn] !== last.kind) return false; + last.text += row.slice(markerColumn + 1); } } // The crop itself cannot contain an example introduction, quote, unrelated @@ -455,25 +462,28 @@ export function pendingAutoplanArtifactPermissionInput(viewport: string, if (!before || createHash('sha256').update(before).digest('hex') !== p.editDigest.beforeSHA256) return null; if (matchesAutoplanDigestRows(diffRows,before,p.editDigest,currentViewport !== viewport)) return {input:'1\r', signature, file:p.file}; if (currentViewport !== viewport) return null; // The new prefix path requires the exact digest, including additions. - // Digest authority adds insertion-only crops; existing anchored deletion - // authority remains available after the current-file binding succeeds. + // Legacy deletion crops below must still honor the recorded request digest. } const originals = fs.readFileSync(p.file, 'utf8').split('\n').map(compact); const chunks: Array<{kind:string; text:string; partial?:boolean}> = []; + const fullRow = /^( {0,3})([1-9]\d*) ([+ -])(.*)$/; + const first = diffRows.map(row => fullRow.exec(row)).find(Boolean); + if (!first) return null; + const markerColumn = first[1]!.length + first[2]!.length + 1; let numbered = 0; for (const row of diffRows) { - const full = /^ {0,3}(\d+) ([+ -])(.*)$/.exec(row); + const full = fullRow.exec(row); if (full) { - if (!Number.isSafeInteger(Number(full[1])) || Number(full[1]) < 1) return null; - numbered++; chunks.push({kind:full[2]!, text:full[3]!}); + if (!Number.isSafeInteger(Number(full[2])) || full[1]!.length + full[2]!.length + 1 !== markerColumn) return null; + numbered++; chunks.push({kind:full[3]!, text:full[4]!}); } else { - const wrap = /^ {4}([+ -])(.*)$/.exec(row); - if (!wrap) return null; - if (!chunks.length) chunks.push({kind:wrap[1]!, text:wrap[2]!, partial:true}); + const kind = row[markerColumn], text = row.slice(markerColumn + 1); + if (!row.startsWith(' '.repeat(markerColumn)) || !['+', ' ', '-'].includes(kind ?? '')) return null; + if (!chunks.length) chunks.push({kind:kind!, text, partial:true}); else { const previous = chunks.at(-1)!; - if (previous.kind !== wrap[1]) return null; - previous.text += wrap[2]!; + if (previous.kind !== kind) return null; + previous.text += text; } } } @@ -482,6 +492,15 @@ export function pendingAutoplanArtifactPermissionInput(viewport: string, if (numbered < 2 || !chunks.some(c => c.kind === '-' && compact(c.text)) || chunks.some(c => c.kind !== '+' && !originals.some(line => c.partial ? line.endsWith(compact(c.text)) : line === compact(c.text)))) return null; + const digest = p.editDigest; + if (digest && chunks.some(chunk => { + const hash = autoplanEditLineHash(chunk.text); + if (chunk.kind === '+') return chunk.partial || !digest.newLineHashes.includes(hash); + if (chunk.kind !== '-') return false; // Current-file context was checked above. + return chunk.partial + ? !originals.some(line => line.endsWith(compact(chunk.text)) && digest.oldLineHashes.includes(autoplanEditLineHash(line))) + : !digest.oldLineHashes.includes(hash); + })) return null; return {input:'1\r', signature, file:p.file}; } catch { return null; } } diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts index d1254e889..3b26a85d3 100644 --- a/test/helpers/claude-pty-runner.ts +++ b/test/helpers/claude-pty-runner.ts @@ -1807,8 +1807,25 @@ export function hasNativePlanCompletion( return hasCompletePlanReport(expectedPlanPath, Math.max(startedAt, ...answerTimes), finishedAt); } +// Shared only by the opt-in Design report binding and its native completion +// route. A quoted example is inert; an owned quoted status remains evidence. +const DESIGN_CLOSURE_PROVISIONAL = /\b(?:example|sample|template|historical|previous|earlier|quoted|hypothetical|if|unless|until|once|assuming|provided|pending|would|will|could|might|may)\b/i; +function designClosureText(text: string, expectedPlanPath: string): string { + const state = /^(?:(?:still|now) )?(?:pending|failed|incomplete|unfinished|unresolved|open|withdrawn|superseded|cancelled|canceled|historical|not complete|not passed|not current)$/i; + return text.replace(/\*\*/g, '') + .replace(/`([^`\n]+)`/g, (_, value: string) => + value === expectedPlanPath || state.test(value) ? value : '') + .replace(/"[^"\n]*"|“[^”\n]*”|(? + state.test(value.slice(1, -1)) ? value.slice(1, -1) : ''); +} +function conflictingDesignClosure(text: string): boolean { + const owner = '(?:(?:the )?Design review(?: of PLAN\\.md)?|DESIGN CLEARED|(?:the )?(?:Design review )?exit gate|(?:the |this )?(?:review|report|gate|verdict|reviewed plan)|(?:(?:this|the|one|a|[1-9]\\d*) )?(?:design )?(?:decision|issue|finding)s?)'; + return new RegExp(`(?:^|[.!?;]\\s+|\\n)(?:Correction:\\s*)?${owner} (?:(?:is|are|remains?|was|were|has been|have been) (?:(?:still|now) )?(?:not (?:the )?(?:complete|passed|current)|incomplete|unfinished|pending|failed|unresolved|open|withdrawn|superseded|cancelled|canceled|historical)|failed|did not pass|has not passed)\\b|${owner} (?:requires approval|applies only if approved)\\b|${owner}[^.!?\\n]*\\b(?:only if|conditional on|subject to)\\b|${owner} (?:belongs to|applies only to) (?:an? )?(?:another|different) (?:plan|project|review)\\b`, 'i').test(text) || + new RegExp(`(?:^|[.!?;]\\s+|\\n)(?:If|When|Once|Unless|Assuming|Provided)\\b[^.!?\\n]*\\b${owner}\\b`, 'i').test(text); +} + function hasCompletePlanReport(expectedPlanPath: string, minimumMtime: number, maximumMtime: number, - allowRunHeaderForFailure = false): boolean { + allowRunHeaderForFailure = false, requiredReview?: 'Design'): boolean { if (!path.isAbsolute(expectedPlanPath)) return false; try { const stat = fs.lstatSync(expectedPlanPath); @@ -1853,6 +1870,20 @@ function hasCompletePlanReport(expectedPlanPath: string, minimumMtime: number, m const trailing = rows.slice(decisions + 1).filter(line => line.trim()); const closed = rows.filter(line => line.trim()).at(-1) === 'NO UNRESOLVED DECISIONS' || (decisions >= 0 && trailing.length > 0 && trailing.every(line => /^[-*] \S|^\+ \d+ unresolved from prior reviews$/.test(line))); + if (requiredReview === 'Design') { + const cells = (row: string) => row.split('|').slice(1, -1).map(cell => cell.trim()); + const header = cells(rows[table] ?? ''); + const design = rows.slice(table + 2).filter(row => row.startsWith('|')) + .map(cells).filter(row => row[header.indexOf('Review')] === 'Design Review'); + const verdicts = rows.filter(row => /^(?:[-*] )?(?:\*\*)?VERDICT:/i.test(row)); + const verdict = designClosureText(verdicts[0] ?? '', expectedPlanPath).replace(/^(?:[-*] )?VERDICT:\s*/i, ''); + if (design.length !== 1 || !/^(?:clean|clear(?: \(full\))?|complete[d]?)$/i.test(design[0]![header.indexOf('Status')] ?? '') || + verdicts.length !== 1 || !/^DESIGN CLEARED\b/i.test(verdict) || + /\?/.test(verdict) || /^DESIGN CLEARED\s+(?:is|was|were|has been|had been|not|never|no longer)\b/i.test(verdict) || + DESIGN_CLOSURE_PROVISIONAL.test(verdict) || conflictingDesignClosure(verdict) || + conflictingDesignClosure(designClosureText(report, expectedPlanPath)) || + rows.filter(row => row.trim()).at(-1) !== 'NO UNRESOLVED DECISIONS') return false; + } return completeTable && /^(?:[-*] )?(?:\*\*)?VERDICT:(?:\*\*)?[ \t]*[A-Za-z]/m.test(report) && closed; } catch { return false; } } @@ -1946,6 +1977,28 @@ export function hasNativePlanTerminal( if (!fence && !/^(?: {4}|\t| {0,3}>)/.test(line)) lines.push(line); } if (fence) return false; + // A Design review may finish at the user's manual handoff without invoking + // ExitPlanMode. Bind its affirmative review and passed gate to this report; + // the common native ownership, answered-call and fresh-file checks above + // still apply. Other skills retain their existing completion routes below. + const designText = designClosureText(lines.join('\n'), expectedPlanPath); + const designParagraphs = designText.split(/\n\s*\n/).map(p => p.trim()); + const designComplete = /(?:^|[.!]\s+)(?:The )?Design review(?: of PLAN\.md)? (?:is complete|has been completed)[.!](?:\s|$)/i; + const designGate = /(?:^|[.!]\s+)(?:The )?(?:Design review )?exit gate (?:has )?passed[.:!](?:\s|$)/i; + const escapedPlanPath = expectedPlanPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const designReport = new RegExp(`(?:^|[.!]\\s+)(?:The )?(?:reviewed plan|review report) (?:is at|was written to|is saved at) ${escapedPlanPath}[.!](?:\\s|$)`, 'i'); + const designCompleted = designParagraphs.filter(p => designComplete.test(p) && designReport.test(p)); + const designPassed = designParagraphs.filter(p => designGate.test(p)); + const sourcedDesign = (paragraph: string) => DESIGN_CLOSURE_PROVISIONAL.test(paragraph) || + /\b(?:example|sample|template|historical|previous|earlier|quote|source|emit|print)\b.*[:：]\s*$/i + .test(designParagraphs[designParagraphs.indexOf(paragraph) - 1] ?? ''); + if (designCompleted.length === 1 && designPassed.length === 1 && + (designText.match(/\b(?:reviewed plan|review report) (?:is at|was written to|is saved at)\b/gi)?.length ?? 0) === 1 && + ![...designCompleted, ...designPassed].some(sourcedDesign) && + !conflictingDesignClosure(designText) && + Date.parse(final.timestamp) <= Date.now() && + hasCompletePlanReport(expectedPlanPath, Math.max(startedAt, ...modifyingAnswers), + Date.parse(final.timestamp), false, 'Design')) return true; const summary = lines.findIndex(line => /^(?:#{1,6}\s*)?(?:\*\*)?Completion\s+summary(?:\*\*)?\s*:?[ \t]*$/i.test(line.trim())); const preceding = lines.slice(0, summary).filter(line => line.trim()).at(-1) ?? ''; if (summary < 0 || /\b(?:example|sample|template|quote|emit|print)\b.*[:：]\s*$/i.test(preceding)) return false; diff --git a/test/helpers/design-count-review.ts b/test/helpers/design-count-review.ts index 9f3cf3f55..ce30bfb67 100644 --- a/test/helpers/design-count-review.ts +++ b/test/helpers/design-count-review.ts @@ -68,7 +68,13 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { const headerActionIssue = /^(?:D[1-9]\d*\s*[—–:-]\s*)?Issue ([1-9]\d*)(?: \(F[1-9]\d*\))?: (How should the header action group establish the primary action)\?$/i.exec(title); const signaledPrimaryIssue = /^(?:D[1-9]\d*\s*[—–:-]\s*)?Issue ([1-9]\d*) \(G[1-9]\d*\): (How should the header action group signal that [A-Za-z][A-Za-z0-9 _-]{0,39} is the primary action)\?$/i.exec(title); const distinguishedPrimaryIssue = /^D[1-9]\d*\s*[—–:-]\s*Issue ([1-9]\d*): How should ([A-Za-z][A-Za-z0-9 _-]{0,39}) (?:be distinguished|stand out) from ([A-Za-z][A-Za-z0-9 ,_-]{0,119}?)(?: in the header)?\?$/i.exec(title); - const issue = /^(?:D[1-9]\d*\s*[—–:-]\s*)?Issue ([1-9]\d*)(?: \((?:(?:G[1-9]\d*|Pass [1-7]), )?(?:Visual Hierarchy|Spacing|Color|Typography|Motion)\))?: ([^?]+)\?$/i.exec(title) ?? headerActionIssue ?? signaledPrimaryIssue; + const questionIssue = /^(?:D[1-9]\d*\s*[—–:-]\s*)?Issue ([1-9]\d*)(?: \((?:(?:G[1-9]\d*|Pass [1-7]), )?(?:Visual Hierarchy|Spacing|Color|Typography|Motion)\))?: ([^?]+)\?$/i.exec(title) ?? headerActionIssue ?? signaledPrimaryIssue; + // A declaration can own the same primary-action decision. Its body and + // native choices below must prove the gap, complete styling and deferral. + const declaredPrimaryIssue = (!questionIssue || /\nELI10: (?:two|three|four|five|six|seven|eight|nine|ten|[1-9]\d*) header buttons currently /i.test(q.question)) && + /^(?:D[1-9]\d*\s*[—–:-]\s*)?Issue ([1-9]\d*)(?: \(G[1-9]\d*\))?: ([^?\n]+)\??$/i.exec(title); + const issue = questionIssue || declaredPrimaryIssue; + const declaredGap = declaredPrimaryIssue && /\(G([1-9]\d*)\)/.exec(title)?.[1]; const descriptivePrimaryHeader = (signaledPrimaryIssue && /^(?!(?:focus|scope|setup|routing|learnings|outside voices|next steps?)$)[A-Za-z][A-Za-z _-]{0,39}$/i.test(q.header.trim())) || (distinguishedPrimaryIssue && /^(?:Visual )?Hierarchy$/i.test(q.header.trim())); if (!issue || !(new RegExp(`^Issue ${issue[1]}(?:: [A-Za-z][A-Za-z0-9 _-]{0,39})?$`, 'i').test(q.header.trim()) || descriptivePrimaryHeader) || @@ -81,7 +87,7 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { if (!distinguishedPrimaryIssue && !/\b(?:buttons?|primary(?: header)? actions?|primary emphasis|hierarchy|spacing|contrast|colou?rs?|labels?|typography|fonts?|loading|spinner|skeleton|motion)\b/i.test(issue[2]!)) return false; const opposed = q.options.filter(o => /^(?:[1-9]\d*[A-Z](?:[).:]\s*|\s+))?(?:Defer|Decline|Leave|Keep|Accept the gap)\b/i.test(o.label) || (distinguishedPrimaryIssue && /^(?:Keep|Leave)\b/i.test(o.description?.trim() ?? ''))); - const repair = !headerActionIssue && !distinguishedPrimaryIssue && /\b(?:fix|resolve|address)\b/i.test(title) && + const repair = !headerActionIssue && !distinguishedPrimaryIssue && !declaredPrimaryIssue && /\b(?:fix|resolve|address)\b/i.test(title) && q.options.some(o => /\b(?:closing|closes|fixes|resolves?|applies?)\b/i.test(o.description ?? '')); // A source citation alone can describe a report or the next reviewer. // Bind the alternate wording to a named control's concrete style amendment @@ -92,7 +98,8 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { /^How should (?:the )?(?:header )?actions establish that ([A-Za-z][A-Za-z0-9 _-]{0,39}) is the primary action$/i.exec(issue[2]!) ?? (distinguishedPrimaryIssue && /^How should ([A-Za-z][A-Za-z0-9 _-]{0,39}) (?:be distinguished|stand out) from /i.exec(issue[2]!)) ?? (headerActionIssue && new RegExp(`^Issue ${issue[1]}: ([A-Za-z][A-Za-z0-9 _-]{0,39})$`, 'i').exec(q.header.trim())); - const primaryEmphasisIssue = !!signaledPrimaryIssue || !!distinguishedPrimaryIssue || /^Give [A-Za-z][A-Za-z0-9 _-]{0,39} primary emphasis in the header action group$/i.test(issue[2]!); + if (declaredPrimaryIssue && (!primary || q.options.length > 4 || Object.keys(call.answers ?? {}).length !== 1)) return false; + const primaryEmphasisIssue = !!signaledPrimaryIssue || !!distinguishedPrimaryIssue || !!declaredPrimaryIssue || /^Give [A-Za-z][A-Za-z0-9 _-]{0,39} primary emphasis in the header action group$/i.test(issue[2]!); const scopedPrimaryStatus = !!headerActionIssue || primaryEmphasisIssue; const explicitStyle = primary && `${primary[1]} filled (?:primary )?#[0-9a-f]{6}(?:/| with )(?:white|black)(?: text)?; ` + '[A-Za-z][A-Za-z0-9 ,/_-]{0,99} neutral ghost(?: buttons)?\\.'; @@ -112,8 +119,8 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { ]; const primaryHeader = !q.header.includes(':') || q.header.split(':')[1]!.trim().toLowerCase() === primary?.[1]?.toLowerCase(); const ownedStatus = (value: string, index: number, source: string) => - /^(?:withdrawn|superseded|resolved|closed|hypothetical|rejected|cancelled|canceled|not current|no longer current)$/i.test(value) && - /(?:^|[.!?;]\s+|\n)(?:Correction:\s*)?(?:(?:This (?:issue|finding|question|amendment|deferral|style|fix|remedy|choice|option|(?:DESIGN\.md |token )?(?:requirement|contract))|Issue [1-9]\d*) (?:is|was|has been)|(?:these|the|this) (?:tokens?|styles?|primary treatment) (?:are|is|were|was|have been|has been)) $/i.test(source.slice(0, index)); + /^(?:withdrawn|superseded|resolved|closed|historical|hypothetical|rejected|cancelled|canceled|not current|no longer current)$/i.test(value) && + /(?:^|[.!?;]\s+|\n)(?:Correction:\s*)?(?:(?:This (?:issue|finding|question|amendment|deferral|style|fix|remedy|choice|option|(?:DESIGN\.md |token )?(?:requirement|contract))|(?:Issue |G)[1-9]\d*) (?:is|was|has been)|(?:these|the|this) (?:tokens?|styles?|primary treatment) (?:are|is|were|was|have been|has been)) $/i.test(source.slice(0, index)); // The style wordings share one owned decision: a current equal-weight gap, // a named control's DESIGN.md amendment, and a different choice retaining it. // A following status assertion remains current after a parenthesized effort @@ -136,15 +143,16 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { const assessments = [...questionText.matchAll(/^ELI10: (.+)$/gm)]; const prefix = questionText.slice(0, assessments[0]?.index ?? 0) .split('\n').filter(line => line.trim()).slice(1); - const sourceAssessment = /\b(?:historical|hypothetical|quoted|source|earlier review)\s+(?:example|excerpt|assessment|material|text)\b|\bnot\s+(?:the\s+)?current\s+(?:UI|assessment|finding|amendment|deferral|remedy|choice|option)\b/i; + const sourceAssessment = /\b(?:historical|hypothetical|quoted|source|earlier review)\s+(?:example|excerpt|assessment|material|text)\b|\bnot\s+(?:the\s+)?current\s+(?:UI|assessment|finding|amendment|deferral|remedy|choice|option)\b|\bthis (?:finding|amendment|deferral|remedy|choice|option) (?:applies only to|belongs to) (?:an? )?(?:another|different) (?:project|plan|review)\b/i; const assessment = assessments.length === 1 && prefix.every(line => /^(?:Project\/branch\/task:|\[P[0-3]\])/.test(line)) && !/^(?:Project\/branch\/task:|\[P[0-3]\])\s*(?:If|When|Unless|Provided|Assuming)\b/im.test(prefix.join('\n')) && !sourceAssessment.test(prefix.join(' ')) && !sourceAssessment.test(assessments[0]![1]!) ? assessments[0]![1]! : ''; const headerPeers = primary && headerActionIssue && new RegExp(`^${primary[1]}, ([A-Za-z][A-Za-z0-9 _-]{0,39}(?:, [A-Za-z][A-Za-z0-9 _-]{0,39})*(?:,? and [A-Za-z][A-Za-z0-9 _-]{0,39})?) currently look (?:the same|identical)\\.`, 'i').exec(assessment); - const countedHeader = distinguishedPrimaryIssue && /^(?:Right now|Today) (?:all|the) (two|three|four|five|six|seven|eight|nine|ten|[1-9]\d*) header buttons look (?:the same|identical)\./i.exec(assessment); - const primaryAssessment = distinguishedPrimaryIssue ? countedHeader?.[0] : headerActionIssue ? headerPeers?.[0] : + const countedHeader = distinguishedPrimaryIssue && /^(?:Right now|Today) (?:all|the) (two|three|four|five|six|seven|eight|nine|ten|[1-9]\d*) header buttons look (?:the same|identical)\./i.exec(assessment) || + declaredPrimaryIssue && /^(two|three|four|five|six|seven|eight|nine|ten|[1-9]\d*) header buttons currently (?:share one style|look identical)\./i.exec(assessment); + const primaryAssessment = distinguishedPrimaryIssue || declaredPrimaryIssue ? countedHeader?.[0] : headerActionIssue ? headerPeers?.[0] : primary && new RegExp(`^(?:Right now|Today) ${primary[1]}(?:, [A-Za-z][A-Za-z0-9 _-]{0,39})+(?:,? and [A-Za-z][A-Za-z0-9 _-]{0,39})? (?:(?:all )?look (?:the same|identical)|are (?:all )?(?:(?:two|three|four|five|six|seven|eight|nine|ten|[1-9]\\d*) )?identical buttons)\\b`, 'i').exec(assessment)?.[0]; const premiseSentence = assessment.split(/[.!?](?:\s|$)/)[0] ?? ''; const currentPrimary = !!primaryAssessment && !/\b(?:not|never|no longer)\b/i.test(primaryAssessment) && @@ -169,8 +177,10 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { const conditionalHeader = (text: string) => /(?:^|[.!?;]\s+|\n)(?:[✅❌]\s*)?(?:Correction:\s*)?(?:If|When|Unless|Assuming|Provided)\b/i.test(text); // Approval conditions suspend this offered decision; explanatory conditions // about user behavior do not make an otherwise current amendment optional. - const pendingPrimaryApproval = (text: string) => primaryEmphasisIssue && /(?:^|[.!?;]\s+|\n)(?:[✅❌]\s*)?(?:Correction:\s*)?(?:If|When|Once|Provided|Assuming|Pending)\s+(?:approval|approved|acceptance|accepted|(?:we|you)\s+(?:approve|accept))\b/i.test(text); - const currentHeaderContract = distinguishedPrimaryIssue ? (countedHeader && headerControls.length > 0 && + const pendingPrimaryApproval = (text: string) => primaryEmphasisIssue && ( + /(?:^|[.!?;]\s+|\n)(?:[✅❌]\s*)?(?:Correction:\s*)?(?:If|When|Once|Provided|Assuming|Pending)\s+(?:approval|approved|acceptance|accepted|(?:we|you)\s+(?:approve|accept))\b/i.test(text) || + (declaredPrimaryIssue && new RegExp(`(?:^|[.!?;]\\s+|\\n)(?:Correction:\\s*)?(?:This (?:issue|finding|amendment|deferral|option)|Issue ${issue[1]}${declaredGap ? `|G${declaredGap}` : ''}) (?:requires approval|applies only if approved)\\b`, 'i').test(text))); + const currentHeaderContract = declaredPrimaryIssue ? countedHeader && !conditionalHeader(questionText) : distinguishedPrimaryIssue ? (countedHeader && headerControls.length > 0 && new Set(headerControls).size === headerControls.length && !headerControls.includes(primary![1]!.toLowerCase()) && numberValue(countedHeader[1]!) === headerControls.length + 1 && !conditionalHeader(questionText)) : !headerActionIssue || (headerContract && headerControls.length > 0 && new Set(headerControls).size === headerControls.length && !headerControls.includes(primary![1]!.toLowerCase()) && @@ -179,7 +189,7 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { numberValue((variantContract || namedContract)![1]!) === otherControls && !/\b(?:proposed|hypothetical|quoted|historical|source)\s+(?:example|contract|requirement)\b/i.test(assessment.slice(0, (variantContract || namedContract)!.index)) && !invalidContract.test(questionText); - const withdrawn = new RegExp(`(?:^|${statusBoundary}\\s+|\\n)(?:Correction:\\s*)?(?:(?:This (?:issue|finding|question|amendment|deferral|style|fix|remedy|choice|option)|Issue ${issue[1]}) (?:is|was|has been) (?:withdrawn|superseded|resolved|closed|hypothetical|rejected|cancelled|canceled|not current|no longer current)|We have (?:resolved|closed|withdrawn) this (?:issue|finding)|No current (?:issue|finding|gap|violation) (?:remains|exists))\\b`, 'i'); + const withdrawn = new RegExp(`(?:^|${statusBoundary}\\s+|\\n)(?:Correction:\\s*)?(?:(?:This (?:issue|finding|question|amendment|deferral|style|fix|remedy|choice|option)|Issue ${issue[1]}${declaredGap ? `|G${declaredGap}` : ''}) (?:is|was|has been) (?:withdrawn|superseded|resolved|closed|historical|hypothetical|rejected|cancelled|canceled|not current|no longer current)|We have (?:resolved|closed|withdrawn) this (?:issue|finding)|No current (?:issue|finding|gap|violation) (?:remains|exists))\\b`, 'i'); const closedGap = /(?:^|[.!?;]\s+|\n)(?:Correction:\s*)?(?:this|the|that) (?:gap|violation) (?:is|was|has been) (?:already\s+|now\s+)?(?:resolved|fixed|closed)\b/i; const cancelledStyle = /(?:^|[.!?;]\s+|\n)(?:Correction:\s*)?(?:do not|don't|never|skip|cancel|withdraw)\s+(?:apply|use|add|keep)\s+(?:(?:these|the|this)\s+)?(?:tokens?|styles?|primary treatment)\b/i; const withdrawnStyles = /(?:^|[.!?;]\s+|\n)(?:Correction:\s*)?(?:these|the|this) (?:tokens?|styles?|primary treatment) (?:are|is|were|was|have been|has been) (?:withdrawn|rejected|cancelled|canceled|not current|no longer current)\b/i; @@ -194,6 +204,15 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { q.options.some(amendment => { const body = currentText(amendment.description ?? ''); if (pendingPrimaryApproval(body)) return false; + const declaredStyle = primary && declaredPrimaryIssue && + new RegExp(`^(?:✅\\s*)?${primary[1]} filled #[0-9a-f]{6}(?: with)? (?:white|black)(?: text)?; ([A-Za-z][A-Za-z0-9 ,/_-]{0,119}) neutral ghost(?: buttons)?\\.`, 'i').exec(body); + if (declaredPrimaryIssue) { + const peers = declaredStyle && controlNames(declaredStyle[1]!.replaceAll('/', ',')); + if (!peers || peers.length !== numberValue(countedHeader![1]!) - 1 || + new Set(peers).size !== peers.length || peers.includes(primary![1]!.toLowerCase()) || + !new RegExp(`^${issue[1]}[A-Z][).:]?\\s+Apply DESIGN\\.md tokens?(?: \\(recommended\\))?$`, 'i').test(amendment.label) || + conditionalHeader(body) || invalidContract.test(body)) return false; + } const headerStyle = primary && headerActionIssue && new RegExp(`^(?:✅\\s*)?${primary[1]} becomes the only filled #[0-9a-f]{6} button with (?:white|black) text; ([A-Za-z][A-Za-z0-9 ,_-]{0,119}) become neutral ghost buttons, exactly as DESIGN\\.md states\\.`, 'i').exec(body); // A descriptive header still owns a concrete primary and every peer. // The native label supplies the filled role; its own body supplies tokens. @@ -201,9 +220,10 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { new RegExp(`^(?:✅\\s*)?${primary[1]} is #[0-9a-f]{6} with (?:white|black) text; ([A-Za-z][A-Za-z0-9 ,_-]{0,119}) are neutral ghost buttons per DESIGN\\.md\\.`, 'i').exec(body) ?? new RegExp(`^(?:✅\\s*)?${primary[1]} becomes the only filled button \\(#[0-9a-f]{6}, (?:white|black) text\\); ([A-Za-z][A-Za-z0-9 ,/_-]{0,119}) use the existing neutral ghost variant` + '(?: \\(human: ~?[0-9]+(?:\\.[0-9]+)?(?:h|min) / CC: ~?[0-9]+(?:\\.[0-9]+)?(?:h|min)\\))?\\. (?:✅\\s*)?Matches DESIGN\\.md exactly\\b', 'i').exec(body)); - const style = distinguishedPrimaryIssue ? distinguishedStyle?.[0] : headerActionIssue ? headerStyle?.[0] : amendments.map(pattern => pattern.exec(body)).find(Boolean)?.[0]; + const style = declaredPrimaryIssue ? declaredStyle?.[0] : distinguishedPrimaryIssue ? distinguishedStyle?.[0] : headerActionIssue ? headerStyle?.[0] : amendments.map(pattern => pattern.exec(body)).find(Boolean)?.[0]; + if ((declaredPrimaryIssue || distinguishedPrimaryIssue) && + /(?:^|[.!?;]\s+|\n)(?:Correction:\s*)?(?:the|this) (?:current )?(?:amendment|fix) keeps (?:all )?(?:two|three|four|five|six|seven|eight|nine|ten|[1-9]\d*) (?:header )?buttons identical\b/i.test(body)) return false; if (distinguishedPrimaryIssue && (!distinguishedStyle || conditionalHeader(body) || invalidContract.test(body) || - /(?:^|[.!?;]\s+|\n)(?:Correction:\s*)?(?:the|this) (?:current )?(?:amendment|fix) keeps (?:all )?(?:two|three|four|five|six|seven|eight|nine|ten|[1-9]\d*) (?:header )?buttons identical\b/i.test(body) || !new RegExp(`^[1-9]\\d*[A-Z][).:]?\\s+Filled primary (?:(?:\\+|and|with) ghosts|${primary![1]})(?: \\(recommended\\))?$`, 'i').test(amendment.label) || JSON.stringify(controlNames(distinguishedStyle[1]!.replaceAll('/', ','))) !== JSON.stringify(headerControls))) return false; if (headerActionIssue && (!headerStyle || conditionalHeader(body) || invalidContract.test(body) || @@ -227,6 +247,12 @@ function ordinaryDesignIssue(fp: AskUserQuestionFingerprint): boolean { return opposed.some(defer => { const declined = currentText(defer.description ?? ''); if (pendingPrimaryApproval(declined)) return false; + if (declaredPrimaryIssue) return defer !== amendment && + new RegExp(`^${issue[1]}[A-Z][).:]?\\s+Defer(?: \\(recommended\\))?$`, 'i').test(defer.label) && + new RegExp(`^Leave ${declaredGap ? `G${declaredGap}` : `Issue ${issue[1]}`} open and record it as unresolved\\.`, 'i').test(declined) && + !new RegExp(`(?:^|[.!?;]\\s+|\\n)(?:Correction:\\s*)?(?:do not|don't|never|skip|cancel|withdraw) (?:leave|keep|defer) (?:${declaredGap ? `G${declaredGap}|` : ''}Issue ${issue[1]})\\b`, 'i').test(declined) && + !conditionalHeader(declined) && !sourceAssessment.test(declined) && !withdrawn.test(declined) && + !closedGap.test(declined) && !invalidContract.test(declined) && !cancelledStyle.test(declined) && !withdrawnStyles.test(declined); // Native menus can list current benefits before the gap retained by // declining. Only consume a complete affirmative pro/con prefix; prose // framing a source example or a future condition cannot expose an icon. diff --git a/test/helpers/devex-seed-coverage.ts b/test/helpers/devex-seed-coverage.ts index 41f16a2cc..75c837634 100644 --- a/test/helpers/devex-seed-coverage.ts +++ b/test/helpers/devex-seed-coverage.ts @@ -9,7 +9,14 @@ export type DevexSeededGap = typeof DEVEX_SEEDED_GAPS[number]; function explainedReversedSignatures(q: NativePlanQuestion, title: string): boolean { const question = /^(?:Journey stage [A-Z ]+: )?the two public functions take the same two arguments in (?:opposite|reversed) positional order\. How should (?:the plan|we) (?:fix|align|unify) the signatures\?$/i.test(title); const traced = /^Journey stage: REAL USAGE\. Two sibling functions take the same two arguments in (?:opposite|reversed) order\.$/i.test(title); - const declaration = traced || /^Journey stage(?: REAL USAGE:|: REAL USAGE\.) The two public evaluation functions take the same two arguments in (?:opposite|reversed) order\.$/i.test(title); + const declared = traced || /^Journey stage(?: REAL USAGE:|: REAL USAGE\.) The two public evaluation functions take the same two arguments in (?:opposite|reversed) order\.$/i.test(title); + // A dedicated assertion can put its named signatures in its own Evidence + // field. Bind subject, source identities and repair instead of menu wording. + const subject = title.replace(/^Journey stage(?: REAL USAGE:|: REAL USAGE\.)\s*/i, ''); + const evidenced = !question && !declared && + /^(?:the )?(?:two|both) public (?:evaluation )?functions take\b/i.test(subject) && + /\bthe same two arguments\b/i.test(subject) && /\b(?:opposite|reversed) (?:positional )?order\.?$/i.test(subject); + const declaration = declared || evidenced; if (!question && !declaration) return false; const lines = q.question.split('\n'); if (lines[0]!.trim().replace(/^D\s*\d+\s*[—–:-]\s*/i, '') !== title) return false; @@ -17,7 +24,7 @@ function explainedReversedSignatures(q: NativePlanQuestion, title: string): bool const context = lines.slice(1, explanation).filter(line => line.trim()); const project = traced ? /^Project\/branch\/task: [^;\n]+; ([\w./-]+):\d+(?:[-–]\d+)?\.$/.exec(context[0] ?? '') : declaration && context.length === 1 ? /^Project\/branch\/task: [^;\n]+; ([\w./-]+) lines? \d+(?: to |[-–])\d+\.$/.exec(context[0]!) : null; - if (explanation < 1 || (declaration && !project) || (!traced && context.some(line => + if (explanation < 1 || (declared && !project) || (!traced && !evidenced && context.some(line => (!declaration && !/^Project\/branch\/task: [^;\n]+; reviewing the public function signatures in [\w./-]+\.$/.test(line)) || /\b(?:quoted|source excerpt|source example|hypothetical|historical|not (?:a )?current|if approved)\b/i.test(line)))) return false; // Inline code may name each signature; a quoted/fenced explanation, earlier @@ -25,25 +32,47 @@ function explainedReversedSignatures(q: NativePlanQuestion, title: string): bool const declaredSignatures = traced ? /^I traced the first real integration after the demo\. ([\w./-]+) lists the two evaluation functions: (`?)run_eval\(\s*dataset\s*,\s*evaluator\s*\)\2 and (`?)run_batch\(\s*evaluator\s*,\s*dataset\s*\)\3\./.exec(context[1] ?? '') : declaration && /^ELI10: ([\w./-]+) documents (`?)run_eval\(\s*dataset\s*,\s*evaluator\s*\)\2 and (`?)run_batch\(\s*evaluator\s*,\s*dataset\s*\)\3\. Same two concepts, reversed positional order, and neither function requires keywords\./.exec(lines[explanation]!); - if (declaration ? !declaredSignatures || declaredSignatures[1] !== project?.[1] - : !/^ELI10: [\w./-]+(?: lines? \d+(?:\s*[-–]\s*\d+)?)? define (`?)run_eval\(\s*dataset\s*,\s*evaluator\s*\)\1 and (`?)run_batch\(\s*evaluator\s*,\s*dataset\s*\)\2\./.test(lines[explanation]!)) return false; + if (!evidenced && (declaration ? !declaredSignatures || declaredSignatures[1] !== project?.[1] + : !/^ELI10: [\w./-]+(?: lines? \d+(?:\s*[-–]\s*\d+)?)? define (`?)run_eval\(\s*dataset\s*,\s*evaluator\s*\)\1 and (`?)run_batch\(\s*evaluator\s*,\s*dataset\s*\)\2\./.test(lines[explanation]!))) return false; const currentProse = (text: string) => { let fence = false; return text.split('\n').filter(line => { if (/^\s*(?:```|~~~)/.test(line)) { fence = !fence; return false; } return !fence && !/^\s*>/.test(line); - }).join('\n').replace(/`[^`\n]*`|"[^"\n]*"|“[^”\n]*”/g, ''); + }).join('\n') + .replace(/(^|[.!?\n]\s*)((?:Correction:\s*)?(?:this|that|the) (?:evidence|trace) (?:is|was|has been) )["“'‘`](withdrawn|rejected|cancelled|canceled|superseded|historical|hypothetical|(?:not|no longer) current)["”'’`]/gi, '$1$2$3') + .replace(/`[^`\n]*`|"[^"\n]*"|“[^”\n]*”/g, ''); }; // The traced declaration owns its named signatures before ELI10, so its // currentness must include that same source paragraph. - const current = currentProse(lines.slice(traced ? 1 : explanation).join('\n')); + const current = currentProse(lines.slice(traced || evidenced ? 1 : explanation).join('\n')); if ((current.match(/^ELI10:/gm)?.length ?? 0) !== 1) return false; if (declaration && /\b(?:if|once|when|unless) (?:approved|accepted)|\b(?:after|pending) approval\b/i.test(current)) return false; if (declaration && /(?:^|[.!?\n]\s*)(?:Correction:\s*)?(?:these|the) (?:functions|signatures) (?:are (?:now|already)|have been) (?:aligned|consistent)\b/i.test(current)) return false; - if (traced && /(?:^|[.!?\n]\s*)(?:Correction:\s*)?(?:this|that|the) trace (?:is|was|has been) (?:withdrawn|rejected|historical|(?:not|no longer) current)\b/i.test(current)) return false; + if ((traced || evidenced) && /(?:^|[.!?\n]\s*)(?:Correction:\s*)?(?:this|that|the) (?:trace|evidence) (?:is|was|has been) (?:withdrawn|rejected|cancelled|canceled|superseded|historical|hypothetical|(?:not|no longer) current)\b/i.test(current)) return false; if (/(?:^|[.!?\n]\s*)(?:Correction:\s*)?(?:(?:this|that|the) (?:finding|explanation)|(?:(?:this|that|the) )?argument[- ]order (?:issue|defect)|these signatures)\b[^.\n]*\b(?:withdrawn|rejected|(?:already )?(?:fixed|resolved)|historical|(?:not|no longer) current)\b/i.test(current) || /(?:^|[.!?\n]\s*)(?:Correction:\s*)?(?:there is|there's) no argument[- ]order (?:issue|defect)\b/i.test(current) || /(?:^|[.!?\n]\s*)(?:Correction:\s*)?run_eval and run_batch now (?:use|take) the same positional order\b/i.test(current)) return false; + if (evidenced) { + // Only an asserted citation at the start of this decision's field owns + // the pair; quoted examples, later borrowed prose and split fields do not. + const fields = lines.slice(1, explanation + 1).filter(line => /^(?:Evidence|ELI10):/.test(line)); + const pair = /^(?:Evidence|ELI10):\s*[\w./-]+(?: lines? \d+(?:[-–]\d+)?|:\d+(?:[-–]\d+)?)?:\s*(`?)run_eval\(\s*dataset\s*,\s*evaluator\s*\)\1 and (`?)run_batch\(\s*evaluator\s*,\s*dataset\s*\)\2(?:[.;]|$)/; + if (!fields.some(line => pair.test(line)) || fields.some(line => + /^(?:Evidence|ELI10):\s*(?:>|`|"|“|Source\b|Quoted\b|Historical\b|Earlier\b|Example\b|Hypothetical\b|If\b|Assuming\b|Provided\b)/i.test(line))) return false; + return q.options.some(option => { + const label = currentProse(option.label.replace(/`(\(\s*dataset\s*,\s*evaluator\s*\))`/g, '$1')); + const remedy = currentProse(option.description ?? ''); + const first = remedy.split(/[.!?\n]/)[0] ?? ''; + return /^(?:[A-D]\)\s*)?(?:Align|Unify|Standardize)\b/i.test(label) && /\(\s*dataset\s*,\s*evaluator\s*\)/.test(label) && + /\bsame (?:positional )?order\b/i.test(first) && /\bboth functions\b/i.test(first) && + /\bkeywords? (?:accepted|supported)\b|\baccept keywords\b/i.test(remedy) && + /\bswaps? (?:is |are )?(?:detected|caught|rejected)\b/i.test(remedy) && /\b(?:clear|actionable) (?:error|message)\b/i.test(remedy) && + !/\b(?:if|unless|when|once|after|pending)\b|\b(?:no|not|never|without|do not|don't)\b|\b(?:other|another|foreign|different) (?:functions?|API|pair|project|issue)\b/i.test(`${label}\n${remedy}`) && + !/(?:^|[.!?\n]\s*)(?:Correction:\s*)?(?:this|that|the) (?:option|action|correction) (?:is|was|has been) (?:historical|withdrawn|rejected|cancelled|canceled|superseded|(?:not|no longer) current)\b/i.test(remedy) && + !/\brun_(?!eval\b|batch\b)\w+\b/.test(remedy); + }); + } // A declared reversal may offer a keyword-only repair instead of a swap // guard. It must bind both arguments to both functions in the same option. if (declaration) return q.options.some(option => diff --git a/test/helpers/eng-seeded-coverage.ts b/test/helpers/eng-seeded-coverage.ts index 5d12d1ca9..fa3d32e0e 100644 --- a/test/helpers/eng-seeded-coverage.ts +++ b/test/helpers/eng-seeded-coverage.ts @@ -41,7 +41,7 @@ function seedSubjects(q: NativePlanQuestionCall['questions'][number]): Seed[] { if (explainedSeed) { const ordinal = /^D([1-9]\d*)\s*[—–:-]/.exec(title)?.[1]; const status = '(?:withdrawn|rejected|cancelled|canceled|superseded|resolved|fixed|optional|hypothetical|unproven|not current|no longer current)'; - const owner = `(?:(?:this|the|that) (?:finding|issue|gap|defect|assessment|explanation)${ordinal ? `|D${ordinal}` : ''})`; + const owner = `(?:(?:this|the|that) (?:finding|issue|decision|gap|defect|assessment|explanation)${ordinal ? `|D${ordinal}` : ''})`; const current = (value: string, option = false) => { const subject = option ? `(?:${owner}|(?:this|the|that) (?:option|action|remedy|correction))` : owner; const scalar = new RegExp(`((?:^|[.!?;]\\s+|\\n)[\\t ]*(?:Correction:\\s*)?${subject} (?:is|was|has been) )["“'‘\x60](${status})["”'’\x60]`, 'gim'); @@ -162,7 +162,7 @@ function seedSubjects(q: NativePlanQuestionCall['questions'][number]): Seed[] { 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 owner = `(?:(?:this|the|that) (?:finding|issue|decision|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) @@ -187,6 +187,33 @@ function explainedSeedSubjects(q: NativePlanQuestionCall['questions'][number]): 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[] = []; + // Step 0 may name the complexity decision in its title and put the concrete + // inventory in its own explanation. The reduction and retained backing + // store must belong to one current option, opposed by that same inventory. + if (/^(?:Step 0 )?complexity (?:check|decision):\s*(?:reduce|simplify|trim)\b/i.test(title)) { + const fileCounts = [...explanation.matchAll(/\b(?:touches|changes|modifies|spans) ([1-9]\d*) files\b/gi)]; + const inventories = [...explanation.matchAll(/\b(?:adds|introduces) ([1-9]\d*) (?:new )?(?:classes|types) \(([^)]+)\)/gi)]; + const names = inventories[0]?.[2]?.split(/,\s*(?:and )?| and /) ?? []; + const conditional = /\b(?:this|the|that) (?:finding|issue|decision|option|action|remedy) (?:(?:applies|proceeds|will proceed) (?:only )?(?:if|once|when)|(?:is|was|has been) conditional on (?:user )?approval)\b|(?:^|\n|[.!?;]\s+)(?:(?:ELI10|Project\/branch\/task):\s*)?(?:if|once|when|assuming|provided) (?:the )?(?:user|owner|reviewer) (?:approves|agrees|accepts)\b/i; + const currentOptions = options.filter(o => !conditional.test(o)); + if (fileCounts.length === 1 && inventories.length === 1 && names.length === Number(inventories[0]![1]) + && new Set(names).size === names.length && (Number(fileCounts[0]![1]) >= 8 || names.length >= 2) + && ['TokenStore', 'SessionMint', 'AuthCache', 'RequestPolicy'].every(name => names.includes(name)) && /\bAuthBroker\b/.test(explanation) + && /\b(?:existing|current) (?:cache )?adapter (?:already )?keys tokens by tenant\b[^.!?]{0,40}\bevicts?\b[^.!?]{0,40}\binvalidates?\b/i.test(explanation) + && /\bAuthCache (?:is|remains) (?:just |only )?a facade (?:over (?:it|the (?:existing |current )?(?:cache )?adapter)|for (?:that|the) adapter)\b/i.test(explanation) + && /\bTokenStore (?:looks like|is|adds) (?:a |another )?(?:second|duplicate) (?:token )?store\b|\bTokenStore duplicates (?:the )?(?:existing |current )?adapter(?:'s)? token storage\b/i.test(explanation) + && /\bRequestPolicy\b[^.!?]{0,90}\b(?:currently )?(?:has|serves) (?:only )?(?:one|a single) consumer\b/i.test(explanation) + && !conditional.test(text) + && !/\bTokenStore (?:now |already )?has (?:a documented )?independent purpose|\bRequestPolicy (?:now |already )?has (?:two|multiple|a second) consumers?\b/i.test(text) + && currentOptions.some(o => /^(?:[A-D][):.]\s*)?(?:Reduce:\s*)?(?:cut|remove|drop) TokenStore\b/i.test(o) + && /\b(?:demote|inline|flatten) RequestPolicy\b|\b(?:make|turn) RequestPolicy (?:into )?(?:a )?(?:plain|pure) function\b/i.test(o) + && /\b(?:one|single|only) (?:(?:token|backing) )?(?:store|storage layer|cache)\b[^.!?\n]{0,80}\b(?:existing|current) (?:cache )?adapter\b|\b(?:existing|current) (?:cache )?adapter (?:as|is|remains|provides) (?:the )?(?:one|single|only) (?:(?:token|backing) )?(?:store|storage layer|cache)\b/i.test(o) + && /\bAuthCache\b/.test(o)) + && currentOptions.some(o => /^(?:[A-D][):.]\s*)?(?:Proceed as-is|keep|retain)\b/i.test(o) + && new RegExp(`\\b${inventories[0]![1]} (?:new )?(?:classes|types)\\b`, 'i').test(o) + && new RegExp(`\\b${fileCounts[0]![1]} files\\b`, 'i').test(o) + && /\b(?:two|2|separate) invalidation (?:paths|stories)\b/i.test(o))) ids.push('complexity'); + } // 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) @@ -1034,8 +1061,9 @@ function hasScheduledLegacyRegression(current: ReadonlyArray<{ title: string; bo match: /^- (?:\[[ xX]\] )?(T[1-9]\d*)(?: \([^\n)]*\))? [—–:-] (.+)(?:\n|$)/.exec(body) })); }); for (const declaration of current) { - if (!/\b(?:regression|characterization)\b/i.test(declaration.title) || !/\bmandatory\b/i.test(declaration.title) - || /\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 requiredRule = /\b(?:mandatory|critical|required)\b/i.test(unquoted(declaration.title)); + if (!/\b(?:regression|characterization)\b/i.test(declaration.title) || !requiredRule + || /\b(?:not|never|no longer) (?:mandatory|critical|required)\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]!); const beforeAndAfter = /\bmust (?:pass|be green) before and after (?:this|the) (?:refactor|rewrite|change)\b/i.test(text); @@ -1083,7 +1111,31 @@ function hasScheduledLegacyRegression(current: ReadonlyArray<{ title: string; bo 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; + // The rule, uniquely named file/task and ordered verification together + // establish the unchanged-code baseline without repeating CRITICAL and + // "before" on every line of that same requirement. + const green = (value: string) => /\b(?:pass(?:es)?|green)\b/i.test(value) + && !/\b(?:not|never|no longer|fail(?:s|ed|ing)?|red)\b/i.test(value); + const orderedBaseline = requiredRule && scheduled && taskFiles[0]![1] === files[0] + && /\b(?:write|add|create|implement|record|capture|pin)\b/i.test(title) + && runs.some(run => green(run) && /\b(?:current|unmodified|untouched) (?:code|implementation|legacy path)\b/i.test(run)) + && runs.some(run => green(run) && /\bafter (?:the )?(?:rewrite|refactor|change)\b/i.test(run)) + && /\b(?:captures?|capturing|records?|recording|pins?|pinning) (?:the )?(?:current|prior|existing) (?:outputs|outcomes|behavior)\b/i.test(text) + && /\bmust (?:pass|satisfy) (?:the )?same (?:suite|tests|assertions) unchanged\b|\b(?:the )?same (?:suite|tests|assertions) must (?:pass|remain green) unchanged (?:after|across) (?:the )?(?:rewrite|refactor|change)\b/i.test(text) + && tasks.filter(t => new RegExp(`^ - Files: ${files[0]!.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`, 'm').test(t.body)).length === 1 + && current.filter(s => /^Verification(?: \([^)]*\))?$/i.test(s.title)).some(s => { + const steps = s.body.join('\n').trim().split(/\n(?=\d+[.)] )/); + const first = unquoted(steps[0] ?? '').trim(); + return steps.length >= 2 && owned(steps[0]!) && /^1[.)] (?:run|execute|test|verify)\b/i.test(first) + && new RegExp(`\\b${id}\\b`).test(first) && /\b(?:unmodified|untouched) (?:code|implementation|legacy path)\b/i.test(first) && green(first) + && steps.slice(1).some(step => { + const statement = unquoted(step).trim(); + return owned(step) && /^(?:[2-9]|[1-9]\\d+)[.)] (?:re-run|rerun|run|execute)\b/i.test(statement) + && new RegExp(`\\b${id}\\b`).test(statement) && /\bafter (?:the )?(?:rewrite|refactor|change)\b/i.test(statement) + && green(statement) && /\bunchanged\b/i.test(statement); + }); + }); + const scopedBaseline = linkedBaseline || committedBaseline || orderedBaseline; if (!scopedBaseline && !scheduledVerification) continue; // An explicit ordered step provides the old-code oracle; matching a task label alone cannot. const baseline = scopedBaseline || current.filter(s => /^Verification(?: \([^)]*\))?$/i.test(s.title)).some(s => { diff --git a/test/helpers/plan-scope-selection.ts b/test/helpers/plan-scope-selection.ts index a7b5651bc..23327a653 100644 --- a/test/helpers/plan-scope-selection.ts +++ b/test/helpers/plan-scope-selection.ts @@ -1,7 +1,7 @@ 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); + return /^(?:I'll|I will) (?:review|(?:run|invoke) (?:the )?\/?([\w:-]+(?:[ \t]+[\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 { @@ -102,7 +102,10 @@ export function nativeSeededPlanSelection( // 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 && describesTitle(draft[2], title) && (!draft[1] || names.includes(draft[1].toLowerCase())) && remainsSelected(message.timestamp)) return true; + // Human role names remain tied to this successfully loaded skill. + names.push(opts.skillName.replace(/-/g, ' '), opts.skillName.replace(/^plan-/, '').replace(/-/g, ' ')); + if (opts.skillName === 'plan-eng-review') names.push('eng-manager plan review'); + if (draft && describesTitle(draft[2], title) && (!draft[1] || names.includes(draft[1].toLowerCase().replace(/[ \t]+/g, ' '))) && 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/plan-count-completion.test.ts b/test/plan-count-completion.test.ts index 04de4d0b8..cbc23a722 100644 --- a/test/plan-count-completion.test.ts +++ b/test/plan-count-completion.test.ts @@ -62,6 +62,133 @@ function fixture() { return { dir, file, startedAt, transcript, cleanup: () => fs.rmSync(dir, { recursive: true, force: true }) }; } +describe('Design manual completion binds its current exit gate to the fresh report', () => { + const designFixture = () => { + const f = fixture(); + fs.writeFileSync(f.file, REPORT.replaceAll('DX', 'Design').replace('Design CLEARED', 'DESIGN CLEARED')); + const time = Date.parse('2026-09-08T17:48:30Z') / 1000; fs.utimesSync(f.file, time, time); + return f; + }; + // AZ's final public declaration; detailed score/decision tables are omitted. + const done = (file: string) => 'Exit gate passed: every issue has its own recorded decision, the plan file\'s last heading is the review report, it has the table and VERDICT, and the final line is the unbolded sentinel.\n\n' + + ' **STATUS: DONE.** Design review of PLAN.md is complete. The reviewed plan is at `' + file + '`.'; + test('recognizes the bound Design declaration independently of terminal layout or ExitPlanMode', () => { + const f = designFixture(); + try { + for (const text of [ + done(f.file), done(f.file).replace('**STATUS: DONE.** ', '').replace('Design review of PLAN.md is complete', 'The Design review has been completed') + .replace('The reviewed plan is at', 'The review report was written to'), + done(f.file) + '\n\nEng review is pending. Implementation should not start until you leave plan mode.', + done(f.file) + '\n\n"Design review is pending."', + done(f.file) + '\n\n"This report is historical."', + ]) { + f.transcript.assistantMessages[0]!.text = text; + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary'), text).toBe(true); + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'plan_ready')).toBe(false); + } + } finally {f.cleanup();} + }); + test('rejects unbound, quoted, historical, provisional and conflicting current completion', () => { + const f = designFixture(); + try { + const complete = done(f.file); + for (const text of [ + complete.replace('Design review of PLAN.md is complete.', 'Everything is done.'), + complete.replaceAll('Design review', 'Eng review'), + complete.replace('PLAN.md', 'OTHER.md'), complete.replace(f.file, f.file + '.other'), + complete.replace('Exit gate passed:', 'Exit gate pending:'), complete.replace('Exit gate passed:', 'Exit gate did not pass:'), + complete.replace('is complete.', 'is not complete.'), complete.replace('is complete.', 'will be complete.'), + complete.replace('is complete.', 'is complete if approved.'), + complete.replace('Exit gate passed:', 'If approved, exit gate passed:'), + complete.replace('**STATUS:', 'Historical example:\n**STATUS:'), + complete.replace('The reviewed plan', '\n\nThe reviewed plan'), + '> ' + complete.replaceAll('\n', '\n> '), '```text\n' + complete + '\n```', + 'An example follows:\n\n"' + complete.replaceAll('\n', ' ') + '"', + 'Source example:\n\n' + complete, + complete.replace('Design review of PLAN.md is complete.', '"Design review of PLAN.md is complete."'), + complete + '\n\nThe reviewed plan is at `' + f.file + '.other`.', + complete + '\n\nDesign review is pending.', + complete + '\n\nDesign review is "still pending".', + complete + '\n\nDesign review is complete only if approved.', + complete + '\n\nThis review is subject to approval.', + complete + '\n\nThe exit gate failed.', + complete + '\n\nThe Design review is cancelled.', + complete + '\n\nThis report is historical.', + complete.replace('Exit gate passed:', 'This gate belongs to a different plan. Exit gate passed:'), + complete + '\n\nThis gate belongs to a different plan.', + complete + '\n\nThe exit gate has been "failed".', + complete + '\n\nThe exit gate did not pass.', + complete + '\n\nThe exit gate has not passed.', + complete + '\n\nThis report is ‘withdrawn’.', + complete + '\n\nOne design decision remains unresolved.', + complete + '\n\nThis review requires approval.', + complete + '\n\nIf approved, this review is complete.', + complete + '\n\nWaiting for your decision.', + complete + '\n\nPlease confirm?', + ]) { + f.transcript.assistantMessages[0]!.text = text; + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary'), text).toBe(false); + } + } finally {f.cleanup();} + }); + test('the Design route preserves native ownership, answer order and artifact readiness', () => { + const f = designFixture(); + try { + f.transcript.assistantMessages[0]!.text = done(f.file); + for (const change of [ + (t: any) => {t.status = 'missing';}, (t: any) => {t.calls = [];}, + (t: any) => {t.calls[0].answered = false;}, (t: any) => {t.calls[0].failed = true;}, + (t: any) => {t.calls[0].sessionId = 'foreign';}, (t: any) => {t.calls[0].answeredAt = '2026-09-08T17:50:00Z';}, + (t: any) => {t.calls[0].answeredAt = undefined;}, + (t: any) => {t.assistantMessages[0].timestamp = '2999-01-01T00:00:00Z';}, + (t: any) => {t.planReadyRequests = [{sessionId: CAPTURED_CALL.sessionId, toolUseId: 'exit', timestamp: '2026-09-08T17:49:40Z', failed: true}];}, + (t: any) => {t.assistantMessages.push({...t.assistantMessages[0], timestamp: '2026-09-08T17:50:00Z', text: 'Still reviewing.'});}, + ]) { + const t = structuredClone(f.transcript); change(t); + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'completion_summary'), change.toString()).toBe(false); + } + for (const timestamp of ['2026-09-08T17:44:00Z', '2026-09-08T17:50:00Z']) { + const time = Date.parse(timestamp) / 1000; fs.utimesSync(f.file, time, time); + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary')).toBe(false); + } + for (const report of ['# Draft\n', REPORT + '\n## Still editing\n', REPORT, + REPORT.replaceAll('DX', 'Design').replace('Design CLEARED', 'DESIGN CLEARED').replace('clean', 'pending'), + REPORT.replaceAll('DX', 'Design').replace('Design CLEARED', 'DESIGN CLEARED') + .replace('NO UNRESOLVED DECISIONS', 'VERDICT: NOT CLEARED\n\nNO UNRESOLVED DECISIONS'), + ]) { + fs.writeFileSync(f.file, report); + const time = Date.parse('2026-09-08T17:48:30Z') / 1000; fs.utimesSync(f.file, time, time); + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary')).toBe(false); + } + fs.rmSync(f.file); + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary')).toBe(false); + } finally {f.cleanup();} + }); + test('a cleared prefix cannot hide a conditional, historical, cancelled or foreign report verdict', () => { + const f = designFixture(); + try { + f.transcript.assistantMessages[0]!.text = done(f.file); + const report = fs.readFileSync(f.file, 'utf8'); + for (const verdict of [ + 'DESIGN CLEARED only if approval is granted', + 'DESIGN CLEARED is not the current verdict; review remains incomplete', + 'DESIGN CLEARED is historical; this report has been cancelled', + 'DESIGN CLEARED. This report is "historical".', + 'DESIGN CLEARED. This report belongs to a different plan.', + 'DESIGN CLEARED?', 'DESIGN CLEARED is a placeholder verdict', + ]) { + fs.writeFileSync(f.file, report.replace('DESIGN CLEARED — eng review required', verdict)); + const time = Date.parse('2026-09-08T17:48:30Z') / 1000; fs.utimesSync(f.file, time, time); + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary'), verdict).toBe(false); + } + fs.writeFileSync(f.file, report.replace('DESIGN CLEARED — eng review required', + 'DESIGN CLEARED (2026-09-11, commit 1a3f728) — eng review required. "This report is historical."')); + const time = Date.parse('2026-09-08T17:48:30Z') / 1000; fs.utimesSync(f.file, time, time); + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary')).toBe(true); + } finally {f.cleanup();} + }); +}); + describe('native plan completion and final report', () => { test('recognizes captured completed native DevEx prose despite a nonmatching terminal heading', () => { const f = fixture(); diff --git a/test/plan-scope-selection.test.ts b/test/plan-scope-selection.test.ts index 215e7e756..ce6c7265d 100644 --- a/test/plan-scope-selection.test.ts +++ b/test/plan-scope-selection.test.ts @@ -205,9 +205,10 @@ console.log(JSON.stringify(obs)); }, 40_000); -test('AG actual first declaration absence stays negative while the explicit retry selection binds', () => { +test('AG spoken first declaration and explicit retry selection both bind their requested skill', () => { const [first, retry] = agFixture.observations; - expect(nativeSeededPlanSelection(first!.transcript as PlanCountTranscript, first!.tools as NativePublicToolEvent[], first!.opts)).toBe(false); + // The recorded failed outcome is unchanged; the first public intro names the design review skill. + expect(nativeSeededPlanSelection(first!.transcript as PlanCountTranscript, first!.tools as NativePublicToolEvent[], first!.opts)).toBe(true); expect(nativeSeededPlanSelection(retry!.transcript as PlanCountTranscript, retry!.tools as NativePublicToolEvent[], retry!.opts)).toBe(true); }); @@ -388,3 +389,39 @@ test('the AW public fixture selects the existing scope helper owners without a n expect(selectTests(['test/fixtures/plan-scope-target-aw.json'], E2E_TOUCHFILES, []).selected) .toEqual(selectTests(['test/helpers/plan-scope-selection.ts'], E2E_TOUCHFILES, []).selected); }); + +// Exact public AZ intros; identities and timestamps use the existing synthetic fixture. +const spokenEngIntros = [ + "I'll run the eng review skill against your draft plan.", + "I'll run the eng-manager plan review skill against this draft plan.", +]; +const spokenEngFixture = (text: string) => { + const f = fixture(text); f.tools[0]!.input!.skill = 'plan-eng-review'; + f.transcript.assistantMessages[0]!.timestamp = timestamp(2); + f.tools[0]!.timestamp = timestamp(4); f.tools[1]!.timestamp = timestamp(5); + return f; +}; +const engScope = { ...opts, skillName: 'plan-eng-review' }; +test('human-readable role names bind only the successfully requested skill', () => { + for (const text of [...spokenEngIntros, "I'll run the plan eng review skill against this draft plan.", "I'll run the eng review skill against this draft plan.", "I'll run the gstack:plan-eng-review skill against this draft plan."]) { + expect(verdict(spokenEngFixture(text), engScope), text).toBe(true); + } + expect(verdict(fixture("I'll run the design review skill against this draft plan."))).toBe(true); +}); +test('spoken skill names retain scope ownership, currentness and target boundaries', () => { + for (const text of spokenEngIntros) { + for (const invalid of [text.replace(/(?:eng review|eng-manager plan review)/, 'design review'), text.replace(/(?:eng review|eng-manager plan review)/, 'eng review and design review'), text.replace(/(?:eng review|eng-manager plan review)/, 'Claude reviewer'), `"${text}"`, `Source:\n${text}`, `If approved, ${text}`]) + expect(verdict(spokenEngFixture(invalid), engScope), invalid).toBe(false); + for (const mutate of [ + (f: ReturnType) => { f.tools[0]!.input!.skill = 'plan-design-review'; }, + (f: ReturnType) => { f.tools[1]!.isError = true; }, + (f: ReturnType) => { f.transcript.assistantMessages[0]!.sessionId = 'foreign'; }, + (f: ReturnType) => { f.transcript.assistantMessages[0]!.timestamp = timestamp(-1); }, + (f: ReturnType) => { f.transcript.assistantMessages[0]!.timestamp = timestamp(0); }, + (f: ReturnType) => { f.tools.push({kind:'use',name:'Read',sessionId:'owned',toolUseId:'work',timestamp:timestamp(1)}); }, + (f: ReturnType) => { f.transcript.assistantMessages.push({sessionId:'owned',timestamp:timestamp(4),text:'This selection is "withdrawn".'}); }, + (f: ReturnType) => { f.transcript.assistantMessages.push({sessionId:'owned',timestamp:timestamp(4),text:'I will review your checkout plan.'}); }, + ]) { const f=spokenEngFixture(text); mutate(f); expect(verdict(f,engScope)).toBe(false); } + expect(verdict(spokenEngFixture(text),{...engScope,seed:opts.seed+'\n# Another plan'})).toBe(false); + } +});