diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index 716774b94..77a9572d7 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -50,6 +50,14 @@ Completed native transcript calls establish question counts and phase coverage. Report-aware count tests also require a fresh, complete report and native completion evidence before accepting a completion heading. +The engineering and DX finding fixtures check coverage of their seeded issues +rather than cap the total number of review questions. Each decision needs a +distinct, completed native question with an offered answer; accepting, rejecting, +or deferring a recommendation all count as reviewing it. Engineering's mandatory +legacy regression tests also need affirmative plan or public-narration evidence. +Additional useful questions are allowed within the existing time limits. Generic +question counts remain diagnostic, and a fresh final review report is required. + E2E tests stream progress in real-time (tool-by-tool via `--output-format stream-json --verbose`). Results are persisted to `~/.gstack/projects//evals/` (legacy fallback `~/.gstack-dev/evals/`) with auto-comparison diff --git a/test/autoplan-artifact-permission.test.ts b/test/autoplan-artifact-permission.test.ts new file mode 100644 index 000000000..7105ff107 --- /dev/null +++ b/test/autoplan-artifact-permission.test.ts @@ -0,0 +1,182 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import fixture from './fixtures/autoplan-artifact-permission-ad-v3.json'; +import { autoplanArtifactPermissionInput } from './helpers/autoplan-artifact-permission'; +import { isPermissionDialogVisible } from './helpers/claude-pty-runner'; +import { E2E_TOUCHFILES, selectTests } from './helpers/touchfiles'; +import type { NativePublicToolEvent } from './helpers/plan-count-transcript'; + +const roots: string[] = []; +afterEach(() => { for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); }); +function replay(relative?: string) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-artifact-permission-')); roots.push(root); + const cwd = path.join(root, path.basename(fixture.cwd)); fs.mkdirSync(cwd); + const ownedStateRoot = path.join(root, 'home', '.gstack'); + const original = fixture.events.at(-1)!.input!.file_path; + const file = path.join(ownedStateRoot, 'projects', path.basename(cwd), relative ?? path.relative( + path.join(fixture.stateRoot, 'projects', path.basename(fixture.cwd)), original)); + fs.mkdirSync(path.dirname(file), { recursive: true }); + const publicTools = structuredClone(fixture.events) as NativePublicToolEvent[]; + for (const event of publicTools) if (event.input?.file_path) event.input.file_path = file; + const lastWrite = publicTools.filter(event => event.name === 'Write').at(-1)!; + fs.writeFileSync(file, lastWrite.input!.content as string); + const context = { cwd, ownedStateRoot, commandStartedAt: fixture.commandStartedAt, + now: Date.parse('2026-09-09T20:36:27.729Z'), transcriptStatus: 'ready', publicTools }; + const viewport = fixture.viewport.replaceAll(path.basename(original), path.basename(file)); + return { root, file, context, viewport }; +} +const pick = (r: ReturnType, seen = new Set()) => + autoplanArtifactPermissionInput(r.viewport, r.context, seen); + +describe('owned Autoplan artifact edit permission', () => { + test('captured cropped pane needs its pending identity; shared generic recognition stays unchanged', () => { + const r = replay(); + expect(isPermissionDialogVisible(fixture.viewport)).toBe(false); + expect(pick(r)).toEqual({ input: '1\r', signature: `${fixture.sessionId}:${fixture.events.at(-1)!.toolUseId}`, file: r.file }); + expect(pick(r, new Set([pick(r)!.signature]))).toBeNull(); + }); + + test('a later same-file Edit has a new one-time epoch even when the footer is identical', () => { + const r = replay(); const first = pick(r)!; const edit = r.context.publicTools.at(-1)!; + fs.writeFileSync(r.file, fs.readFileSync(r.file, 'utf8').replace(edit.input!.old_string as string, edit.input!.new_string as string)); + r.context.publicTools.push({ sessionId: fixture.sessionId, toolUseId: edit.toolUseId, kind: 'result', + timestamp: '2026-09-09T20:28:00.000Z', isError: false }); + r.context.publicTools.push({ ...structuredClone(edit), toolUseId: 'next-owned-edit', timestamp: '2026-09-09T20:28:01.000Z' }); + expect(pick(r, new Set([first.signature]))?.signature).toBe(`${fixture.sessionId}:next-owned-edit`); + }); + + test('a queued non-file tool cannot replace or grant the unique current Edit permission', () => { + const r = replay(); + r.context.publicTools.push({ sessionId: fixture.sessionId, toolUseId: 'queued-bash', kind: 'use', + timestamp: '2026-09-09T20:27:41.541Z', name: 'Bash', input: { command: 'echo unrelated queued work' } }); + expect(pick(r)?.input).toBe('1\r'); + r.context.publicTools.push({ ...r.context.publicTools.at(-1)!, toolUseId: 'concurrent-write', name: 'Write', + input: { file_path: r.file, content: 'other mutation' } }); + expect(pick(r)).toBeNull(); + }); + + test('the two source-declared Eng test-plan layouts have the same bounded edit path', () => { + for (const file of ['test-main-eng-review-test-plan-20260909-203000.md', 'test-main-test-plan-20260909-203000.md']) + expect(pick(replay(file))?.input).toBe('1\r'); + }); + + test('requires the exact owned project and known artifact filename; no broad state/home approval', () => { + for (const file of ['../sibling/ceo-plans/2026-09-09-user-dashboard.md', 'config.yaml', 'reviews.jsonl', + 'main-autoplan-restore-20260909-200700.md', 'ceo-plans/archive/2026-09-09-user-dashboard.md', + 'designs/screen-20260909/mockup.md', 'dx-plans/2026-09-09-plan.md', 'arbitrary.md']) + expect(pick(replay(file)), file).toBeNull(); + const r = replay(); + r.context.ownedStateRoot = undefined as any; expect(pick(r)).toBeNull(); + r.context.ownedStateRoot = path.join(r.root, 'caller-GSTACK_HOME'); expect(pick(r)).toBeNull(); + r.context.ownedStateRoot = path.join(r.root, 'home', '.gstack'); + r.context.cwd = path.join(r.root, 'sibling'); expect(pick(r)).toBeNull(); + }); + + test('regular current file and exact requested old/new text are mandatory', () => { + const r = replay(); const before = fs.readFileSync(r.file); + fs.writeFileSync(r.file, 'unrelated current content'); expect(pick(r)).toBeNull(); + fs.writeFileSync(r.file, before); + r.context.publicTools.at(-1)!.input!.new_string = 'unrelated replacement'; expect(pick(r)).toBeNull(); + fs.unlinkSync(r.file); fs.mkdirSync(r.file); expect(pick(r)).toBeNull(); + }); + + test.skipIf(process.platform === 'win32')('rejects symlink escape and symlink aliases within the owned tree', () => { + const r = replay(); const other = path.join(r.root, 'external.md'); + fs.renameSync(r.file, other); fs.symlinkSync(other, r.file); expect(pick(r)).toBeNull(); + fs.unlinkSync(r.file); fs.renameSync(other, r.file); + const directory = path.dirname(r.file); const alias = directory + '-actual'; + fs.renameSync(directory, alias); fs.symlinkSync(alias, directory); expect(pick(r)).toBeNull(); + }); + + test.skipIf(process.platform === 'win32')('trusted temp-parent aliases preserve ownership without permitting a symlink state root', () => { + const r = replay(); const alias = path.join(r.root, 'temp-parent-alias'); + fs.symlinkSync(path.join(r.root, 'home'), alias); + const target = path.join(alias, '.gstack', path.relative(r.context.ownedStateRoot, r.file)); + r.context.ownedStateRoot = path.join(alias, '.gstack'); + for (const event of r.context.publicTools) if (event.input?.file_path) event.input.file_path = target; + r.file = target; + expect(pick(r)?.input).toBe('1\r'); // e.g. macOS /var -> /private/var, above owned root + const stateAlias = path.join(r.root, 'state-alias'); + fs.symlinkSync(r.context.ownedStateRoot, stateAlias); + const other = path.join(stateAlias, path.relative(r.context.ownedStateRoot, r.file)); + r.context.ownedStateRoot = stateAlias; + for (const event of r.context.publicTools) if (event.input?.file_path) event.input.file_path = other; + r.file = other; + expect(pick(r)).toBeNull(); + }); + + test('missing, stale, future, foreign, completed, failed, duplicate and concurrent identities stay closed', () => { + const mutations: Array<(r: ReturnType) => void> = [ + r => { r.context.transcriptStatus = 'error'; }, + r => { r.context.publicTools = []; }, + r => { r.context.commandStartedAt = r.context.now + 1; }, + r => { r.context.commandStartedAt = Date.parse(r.context.publicTools.at(-1)!.timestamp) + 1; }, + r => { r.context.publicTools.at(-1)!.timestamp = '2026-09-10T00:00:00.000Z'; }, + r => { r.context.publicTools.at(-1)!.timestamp = 'invalid'; }, + r => { r.context.publicTools.at(-1)!.sessionId = 'foreign'; }, + r => { r.context.publicTools.at(-1)!.sessionId = ''; }, + r => { r.context.publicTools.at(-1)!.toolUseId = ''; }, + r => { r.context.publicTools.at(-1)!.name = 'Write'; }, + r => { r.context.publicTools.at(-1)!.input!.replace_all = true; }, + r => { r.context.publicTools.push({ ...r.context.publicTools.at(-1)!, kind: 'result', isError: false }); }, + r => { r.context.publicTools.push({ ...r.context.publicTools.at(-1)!, kind: 'result', isError: true }); }, + r => { r.context.publicTools.push(structuredClone(r.context.publicTools.at(-1)!)); }, + r => { r.context.publicTools.splice(-1, 0, { ...structuredClone(r.context.publicTools.at(-1)!), toolUseId: 'other-pending-edit' }); }, + r => { for (const event of r.context.publicTools) if (event.kind === 'result') event.isError = true; }, + r => { for (const event of r.context.publicTools.slice(0, -1)) if (event.input) event.input.file_path = r.file + '-sibling'; }, + r => { r.context.publicTools.reverse(); }, + ]; + for (const mutate of mutations) { const r = replay(); mutate(r); expect(pick(r), mutate.toString()).toBeNull(); } + }); + + test('quotes, examples, unrelated diffs, malformed menus, extra options and broad selection are rejected', () => { + const mutations = [ + (s: string) => 'Example:\n' + s, (s: string) => '```\n' + s + '\n```', + (s: string) => s.split('\n').map(line => '> ' + line).join('\n'), + (s: string) => s.replace('Success target made numeric', 'Unrelated line copied from another plan'), + (s: string) => s.replace('2026-09-09-user-dashboard.md?', 'sibling.md?'), + (s: string) => s.replace('❯ 1. Yes', ' 1. Yes').replace(' 2. Yes', '❯2. Yes'), + (s: string) => s.replace('❯ 1. Yes', '❯ 1. Yes, always allow'), + (s: string) => s.replace(' 3. No', ' 3. No\n 4. Change permission mode'), + (s: string) => s.replace('Esc to cancel · Tab to amend', 'Enter to select'), + (s: string) => s + '\nPlease choose the quoted example above.', + (s: string) => s.slice(s.indexOf(' Do you want')), // no bound diff + ]; + for (const mutate of mutations) { const r = replay(); r.viewport = mutate(r.viewport); expect(pick(r), mutate.toString()).toBeNull(); } + }); + + for (const deletion of [false, true]) test(`native ${deletion ? 'deletion' : 'replacement'} diff rows remain bound to the requested old/new text`, () => { + const r = replay(); const before = 'Old first\nOld second\nContext\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 first\nOld second'; + edit.input!.new_string = deletion ? '' : 'New first\nNew second'; + const menu = r.viewport.slice(r.viewport.indexOf(' Do you want')); + // Existing native fixtures include 102-,103-,102+,103+ replacements, + // and deleted-only rows. These small controls are projected, not live panes. + r.viewport = ' 1 -Old first\n 2 -Old second\n' + + (deletion ? '' : ' 1 +New first\n 2 +New second\n') + + ' 3 Context\n' + '╌'.repeat(20) + '\n' + menu; + expect(pick(r)?.input).toBe('1\r'); + r.viewport = r.viewport.replace(' 2 -Old second', ' 2 -Context'); + expect(pick(r)).toBeNull(); // Existing context is not part of the requested deletion. + }); + + 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', + input: { ...edit.input, file_path: r.file + '-other' } }); + events.push({ sessionId: edit.sessionId, toolUseId: edit.toolUseId, kind: 'result', + timestamp: '2026-09-09T20:28:00.000Z', isError: false }); + expect(pick(r)).toBeNull(); + }); + + test('new helper, fixture, and regression select only the existing Autoplan paid case', () => { + for (const file of ['test/helpers/autoplan-artifact-permission.ts', 'test/autoplan-artifact-permission.test.ts', + 'test/fixtures/autoplan-artifact-permission-ad-v3.json']) + expect(selectTests([file], E2E_TOUCHFILES).selected).toEqual(['autoplan-chain-pty']); + }); +}); diff --git a/test/devex-seed-coverage.test.ts b/test/devex-seed-coverage.test.ts new file mode 100644 index 000000000..8d327574a --- /dev/null +++ b/test/devex-seed-coverage.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, test } from 'bun:test'; +import { DEVEX_SEEDED_GAPS, devexSeedCoverage } from './helpers/devex-seed-coverage'; +import type { PlanCountTranscript, NativePlanQuestionCall } from './helpers/plan-count-transcript'; +import fixture from './fixtures/devex-seed-coverage-ad-v3.json'; +import { E2E_TOUCHFILES, matchGlob } from './helpers/touchfiles'; + +function transcript(attempt = 0): PlanCountTranscript { + return { status:'ready', calls:structuredClone(fixture.attempts[attempt]!.calls) as NativePlanQuestionCall[], assistantMessages:[] }; +} +function extra(id: string, sessionId: string): NativePlanQuestionCall { + const question = 'A new useful DX improvement: should we provide an offline diagnostics command?'; + 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'}; +} + +describe('DX seeded-gap coverage', () => { + for (const [i, attempt] of fixture.attempts.entries()) test(`actual attempt ${attempt.attempt} has five distinct completed seed decisions`, () => { + const result = devexSeedCoverage(transcript(i)); + expect(result.complete).toBe(true); + expect(result.missing).toEqual([]); + expect(Object.keys(result.decisions)).toEqual([...DEVEX_SEEDED_GAPS]); + expect(new Set(Object.values(result.decisions).flat()).size).toBe(5); + // Deterministic coverage cannot change the old early-stop outcome or + // establish that the uncompleted original review produced its final report. + expect(attempt.historicalOutcome).toBe('ceiling_reached'); + expect(attempt.genuineDecisions).toBe(8); + }); + test('each additional real decision remains valid and cannot replace a missing seed', () => { + for (let i = 0; i < 5; i++) { + const t=transcript();t.calls.push(...Array.from({length:6},(_,n)=>extra(`extra-${n}`,t.calls[0]!.sessionId))); + expect(devexSeedCoverage(t).complete).toBe(true); + t.calls.splice(i,1); + expect(devexSeedCoverage(t).complete).toBe(false); + expect(devexSeedCoverage(t).missing).toHaveLength(1); + } + }); + test('a valid defer or alternate repair still covers the decision', () => { + for (let a=0;a<2;a++) for (let i=0;i<5;i++) { + const t=transcript(a);const c=t.calls[i]!;const q=c.questions[0]!; + for (const option of q.options) { + c.answers = {[q.question]:option.label}; + expect(devexSeedCoverage(t).complete).toBe(true); + } + } + }); + test('five repeated questions for one seed cannot satisfy the other four', () => { + const t=transcript();t.calls=Array.from({length:5},(_,i)=>({...structuredClone(t.calls[0]!),toolUseId:`repeat-${i}`})); + expect(devexSeedCoverage(t).complete).toBe(false); + expect(devexSeedCoverage(t).missing).toHaveLength(4); + }); + test('batching all issues into one native call or one omnibus question fails', () => { + const t=transcript();const c=structuredClone(t.calls[0]!);c.questions=t.calls.flatMap(c=>c.questions);c.answers=Object.fromEntries(t.calls.flatMap(c=>Object.entries(c.answers!)));t.calls=[c]; + expect(devexSeedCoverage(t).batched).toHaveLength(1); + expect(devexSeedCoverage(t).complete).toBe(false); + c.questions=[{header:'All five',question:'Should we repair all five seeded defects together?',options:[{label:'Repair all',description:'Fix every defect.'},{label:'Defer all',description:'Defer every repair.'}]}];c.answers={[c.questions[0]!.question]:'Repair all'}; + expect(devexSeedCoverage(t).missing).toHaveLength(5); + }); + test('pending, failed, malformed completion, repeated identity and foreign sessions stay closed', () => { + const mutations: Array<(t:PlanCountTranscript)=>void> = [ + t=>{t.status='missing'},t=>{t.calls[0]!.answered=false},t=>{t.calls[0]!.failed=true}, + t=>{t.calls[0]!.answeredAt='unknown'},t=>{t.calls[0]!.answers={}}, + t=>{t.calls[0]!.answers={[t.calls[0]!.questions[0]!.question]:'Not offered'}}, + t=>{t.calls[0]!.unansweredQuestionIndices=[0]},t=>{t.calls[0]!.questions[0]!.multiSelect=true}, + t=>{t.calls[0]!.sessionId='foreign'},t=>{t.calls.push(structuredClone(t.calls[0]!))}, + t=>{t.calls[1]!.toolUseId=t.calls[0]!.toolUseId}, + ]; + for(const mutate of mutations){const t=transcript();mutate(t);expect(devexSeedCoverage(t).complete).toBe(false)} + }); + test('a quoted defect, retrospective confirmation or only generic navigation options is not a seed decision', () => { + for (const prefix of ['Quoted example: ','Suppose ','Have you read: ','Confirm already resolved: ']) { + const t=transcript();const c=t.calls[0]!;const q=c.questions[0]!;const answer=c.answers![q.question]!; + q.question=prefix+q.question;c.answers={[q.question]:answer};expect(devexSeedCoverage(t).complete).toBe(false); + } + const t=transcript();const c=t.calls[0]!;const q=c.questions[0]!;q.options=[{label:'Continue',description:'Next section.'},{label:'Stop',description:'End review.'}];c.answers={[q.question]:'Continue'}; + expect(devexSeedCoverage(t).complete).toBe(false); + }); + test('direct seed questions can ask what to do without asserting the observed wording', () => { + const titles: Record = { + Quickstart:'Should we ship examples/first_eval.py or point the quickstart at the demo?', + 'CI gate':'Should the first local demo bypass the CI check?', + Signatures:'How should we make argument order consistent between run_batch and run_eval?', + AuthError:'Should AuthError explain the invalid API key with a code, cause and fix?', + 'v1 to v2':'Should we keep a compatibility alias from Client.evaluate to Client.run during the v2 upgrade?', + }; + const t=transcript(); + for (const c of t.calls) { const q=c.questions[0]!, answer=c.answers![q.question]!; + q.question=titles[q.header]!;q.header='Decision';c.answers={[q.question]:answer}; } + expect(devexSeedCoverage(t).complete).toBe(true); + const c=t.calls[1]!,q=c.questions[0]!,answer=c.answers![q.question]!; + q.question='The first local demo might block on a CI check. Should we bypass it?';c.answers={[q.question]:answer}; + expect(devexSeedCoverage(t).complete).toBe(true); + }); + test('an explicit seed action can be accepted or rejected through terse Yes/No options', () => { + const titles: Record = { + Quickstart:'Should we ship examples/first_eval.py for the quickstart?', + 'CI gate':'Should we bypass the CI check for the first local demo?', + Signatures:'Should we unify argument order between run_eval and run_batch?', + AuthError:'Should we add a code, cause and fix to AuthError for invalid API keys?', + 'v1 to v2':'Should we keep a compatibility alias from Client.evaluate to Client.run?', + }; + for (const answer of ['Yes','No']) { + const t=transcript(); + for (const c of t.calls) { const q=c.questions[0]!; q.question=titles[q.header]!; + q.options=[{label:'Yes',description:'Accept the proposed action.'},{label:'No',description:'Keep the current plan.'}];c.answers={[q.question]:answer}; } + expect(devexSeedCoverage(t).complete).toBe(true); + for (let i=0;i<5;i++) { + const copy=structuredClone(t), c=copy.calls[i]!, q=c.questions[0]!; + q.question=q.question.replace('Should we ', 'Should we document how to ');c.answers={[q.question]:answer}; + expect(devexSeedCoverage(copy).complete).toBe(false); + } + } + }); + test('only the native DX count eval selects the new coverage files', () => { + for (const file of ['test/helpers/devex-seed-coverage.ts','test/devex-seed-coverage.test.ts','test/fixtures/devex-seed-coverage-ad-v3.json']) { + expect(Object.entries(E2E_TOUCHFILES).filter(([,files])=>files.some(pattern=>matchGlob(file,pattern))).map(([name])=>name)).toEqual(['plan-devex-finding-count']); + } + }); +}); diff --git a/test/eng-seeded-coverage.test.ts b/test/eng-seeded-coverage.test.ts new file mode 100644 index 000000000..6e8d384d1 --- /dev/null +++ b/test/eng-seeded-coverage.test.ts @@ -0,0 +1,176 @@ +import { describe, expect, test } from 'bun:test'; +import captured from './fixtures/eng-count-ad-v2.json'; +import type { NativePlanQuestionCall, PlanCountTranscript } from './helpers/plan-count-transcript'; +import { ENG_DECISION_SEEDS, evaluateEngSeedCoverage } from './helpers/eng-seeded-coverage'; +import { E2E_TOUCHFILES, matchGlob } from './helpers/touchfiles'; + +// Exact public decisions reused from the existing fixture. The report below is +// a synthetic assembly of its retained task catalog, not a claim that the old run passed. +const calls = captured.cases.first.calls as NativePlanQuestionCall[]; +const indices = [2, 4, 6, 8]; +const start = Date.parse('2026-09-09T19:00:00Z'), end = Date.parse('2026-09-09T19:30:00Z'); +const report = '# Reviewed plan\n\n' + captured.reviewedTasks.lines.join('\n') + '\n\n## GSTACK REVIEW REPORT\nEng review complete.\n'; +const transcript = (): PlanCountTranscript => ({ status: 'ready', calls: structuredClone(calls), assistantMessages: [] }); +const evaluate = (t = transcript(), p = report) => evaluateEngSeedCoverage(t, p, start, end); +function question(call: NativePlanQuestionCall, text: string) { + const answer = call.answers![call.questions[0]!.question]!; + call.questions[0]!.question = text; call.answers = { [text]: answer }; +} + +describe('Eng seeded coverage from completed native decisions', () => { + test('four separate decisions plus the auto-added regression cover all five seeds regardless of total count', () => { + const result = evaluate(); + expect(result.ok).toBe(true); + expect(Object.keys(result.decisions)).toEqual([...ENG_DECISION_SEEDS]); + expect(new Set(Object.values(result.decisions)).size).toBe(4); + expect(result.regression).toBe('plan'); + // Historical evidence remains a failure, never a retroactive live pass. + expect(captured.cases.first.actual.outcome).toBe('ceiling_reached'); + expect(captured.cases.first.actual.reviewCount).toBeGreaterThan(7); + const t = transcript(); + for (let i = 0; i < 12; i++) { + const extra = structuredClone(calls[7]!); extra.toolUseId += `-extra-${i}`; t.calls.push(extra); + } + expect(evaluate(t).ok).toBe(true); + }); + + test('every offered choice is coverage, including rejecting or deferring the recommended change', () => { + for (const index of indices) for (const option of calls[index]!.questions[0]!.options) { + const t = transcript(), call = t.calls[index]!; + call.questions[0]!.options.reverse(); + call.answers = { [call.questions[0]!.question]: option.label }; + expect(evaluate(t).ok).toBe(true); + } + const t = transcript(), c = t.calls[4]!; + c.questions[0]!.options.push({ label: 'Defer the cache change', description: 'Accept the stated risk for this release.' }); + c.answers = { [c.questions[0]!.question]: 'Defer the cache change' }; + expect(evaluate(t).ok).toBe(true); + }); + + test('presentation numbers and headings do not establish or remove seed identity', () => { + const t = transcript(); + for (const index of indices) { + const c = t.calls[index]!; c.questions[0]!.header = 'Decision'; + question(c, c.questions[0]!.question.replace(/^D\d+ — Issue \d+(?: \([^)]+\))?[: ]*/, 'Decision: ')); + } + expect(evaluate(t).ok).toBe(true); + }); + + test('a direct seeded action question can use terse Yes/No choices', () => { + const titles = [ + 'Should we reduce the four new classes spread across twelve files?', + 'Should we inject the shared global AuthCache?', + 'Should we split validateAndDispatch to remove its nested swallowing catches?', + 'Should we parallelize the five sequential IDP calls?', + ]; + for (const answer of ['Yes', 'No']) { + const t = transcript(); + indices.forEach((index, n) => { + const c = t.calls[index]!; question(c, titles[n]!); + c.questions[0]!.options = [{ label: 'Yes' }, { label: 'No' }]; + c.answers = { [c.questions[0]!.question]: answer }; + }); + expect(evaluate(t).ok).toBe(true); + indices.forEach((index, n) => { + const administrative = structuredClone(t); + const c = administrative.calls[index]!; + question(c, titles[n]!.replace('Should we ', 'Should we document how to ')); + expect(evaluate(administrative).missing).toContain(ENG_DECISION_SEEDS[n]!); + }); + } + }); + + test('omitting each seed remains missing even when unrelated completed decisions are plentiful', () => { + for (let n = 0; n < indices.length; n++) { + const t = transcript(); t.calls.splice(indices[n]!, 1); + expect(evaluate(t).missing).toContain(ENG_DECISION_SEEDS[n]!); + expect(evaluate(t).ok).toBe(false); + } + }); + + test('batched questions or one combined approval cannot supply four distinct decisions', () => { + const t = transcript(), combined = structuredClone(calls[2]!); + combined.questions = indices.map(i => structuredClone(calls[i]!.questions[0]!)); + combined.answers = Object.fromEntries(indices.map(i => Object.entries(calls[i]!.answers!)[0]!)); + t.calls = [combined]; expect(evaluate(t).missing).toHaveLength(4); + combined.questions = [structuredClone(calls[2]!.questions[0]!)]; + question(combined, indices.map(i => calls[i]!.questions[0]!.question.split('\n')[0]).join(' ')); + combined.questions[0]!.options = [ + { label: 'Reduce classes, inject cache, split errors and parallelize IDP', description: 'Approve all four changes.' }, + { label: 'Keep all four unchanged', description: 'Reject every change.' }, + ]; + combined.answers = { [combined.questions[0]!.question]: combined.questions[0]!.options[0]!.label }; + expect(evaluate(t).missing).toHaveLength(4); + }); + + test('pending, failed, stale, foreign, malformed or unoffered replies provide no decision credit', () => { + for (const mutate of [ + (c: NativePlanQuestionCall) => { c.answered = false; }, + (c: NativePlanQuestionCall) => { c.failed = true; }, + (c: NativePlanQuestionCall) => { delete c.failed; }, + (c: NativePlanQuestionCall) => { c.answers = {}; }, + (c: NativePlanQuestionCall) => { c.answers = { [c.questions[0]!.question]: 'not offered' }; }, + (c: NativePlanQuestionCall) => { c.answers!.foreign = 'Yes'; }, + (c: NativePlanQuestionCall) => { c.answeredAt = 'invalid'; }, + (c: NativePlanQuestionCall) => { c.answeredAt = new Date(start - 1).toISOString(); }, + (c: NativePlanQuestionCall) => { c.answeredAt = new Date(end + 1).toISOString(); }, + (c: NativePlanQuestionCall) => { c.sessionId = 'foreign'; }, + (c: NativePlanQuestionCall) => { c.toolUseId = ''; }, + (c: NativePlanQuestionCall) => { c.unansweredQuestionIndices = [0]; }, + (c: NativePlanQuestionCall) => { c.questions[0]!.multiSelect = true; }, + (c: NativePlanQuestionCall) => { c.questions[0]!.options[1]!.label = c.questions[0]!.options[0]!.label; }, + ]) { + const t = transcript(); mutate(t.calls[4]!); expect(evaluate(t).ok).toBe(false); + } + const duplicate = transcript(); duplicate.calls.push(structuredClone(duplicate.calls[4]!)); + expect(evaluate(duplicate).ok).toBe(false); + const missing = transcript(); missing.status = 'missing'; expect(evaluate(missing).ok).toBe(false); + }); + + test('quoted examples, resolved defects and option-only references cannot impersonate a seeded issue', () => { + for (const prefix of ['> ', 'Example: ', '```text\n', 'Hypothetical: ', 'No defect remains: ']) { + const t = transcript(); question(t.calls[4]!, prefix + t.calls[4]!.questions[0]!.question); + expect(evaluate(t).missing).toContain('shared-cache'); + } + const t = transcript(); question(t.calls[4]!, 'Which format should the final report use?'); + expect(evaluate(t).missing).toContain('shared-cache'); + }); + + test('mandatory regression evidence requires an affirmative legacy task or scoped public narration', () => { + const base = '## GSTACK REVIEW REPORT\nEng complete.\n'; + for (const text of [ + 'legacyAuthFlow will be rewritten; no regression test for prior behavior is planned.', + 'Do not add legacyAuthFlow regression characterization fixtures.', + 'Defer adding legacyAuthFlow regression characterization fixtures.', + 'Maybe add legacyAuthFlow regression characterization fixtures.', + '"Add legacyAuthFlow regression characterization fixtures before changes."', + 'Example: Add legacyAuthFlow regression characterization fixtures before changes.', + 'It is unclear whether to add legacyAuthFlow regression characterization fixtures before changes.', + 'We would add legacyAuthFlow regression characterization fixtures before changes.', + 'Add a report paragraph describing legacyAuthFlow regression characterization fixtures before changes.', + 'Record a note about legacyAuthFlow regression characterization fixtures before changes.', + 'Add regression characterization tests for newAuthFlow before changes; legacyAuthFlow is only mentioned in release notes.', + 'Record regression characterization fixtures for newAuthFlow before changes. The legacyAuthFlow documentation was updated.', + 'Add tests for legacyAuthFlow before changes; newAuthFlow gets regression characterization fixtures.', + 'legacyAuthFlow — Add regression characterization tests for newAuthFlow before changes.', + '> Add legacyAuthFlow regression characterization fixtures before changes.', + '```\nAdd legacyAuthFlow regression characterization fixtures before changes.\n```', + ]) expect(evaluate(transcript(), text + '\n\n' + base).ok).toBe(false); + expect(evaluate(transcript(), 'Add regression characterization tests for legacyAuthFlow before changes.\n\n' + base).regression).toBe('plan'); + const t = transcript(); t.assistantMessages.push({ sessionId: calls[0]!.sessionId, timestamp: new Date(end - 1).toISOString(), + text: 'Added legacyAuthFlow regression characterization fixtures before the rewrite.' }); + expect(evaluate(t, base).regression).toBe('public-narration'); + t.assistantMessages[0]!.sessionId = 'foreign'; expect(evaluate(t, base).ok).toBe(false); + t.assistantMessages[0]!.sessionId = calls[0]!.sessionId; + t.assistantMessages[0]!.timestamp = new Date(start - 1).toISOString(); expect(evaluate(t, base).ok).toBe(false); + expect(evaluate(transcript(), captured.reviewedTasks.lines.join('\n')).ok).toBe(false); + expect(evaluate(transcript(), report.replace('Eng review complete.', '')).ok).toBe(false); + }); + + test('new local evidence dependencies select only this Eng paid case', () => { + for (const file of ['test/helpers/eng-seeded-coverage.ts', 'test/eng-seeded-coverage.test.ts']) { + expect(Object.entries(E2E_TOUCHFILES).filter(([, patterns]) => patterns.some(p => matchGlob(file, p))).map(([key]) => key)) + .toEqual(['plan-eng-finding-count']); + } + }); +}); diff --git a/test/fixtures/autoplan-artifact-permission-ad-v3.json b/test/fixtures/autoplan-artifact-permission-ad-v3.json new file mode 100644 index 000000000..e89b155fd --- /dev/null +++ b/test/fixtures/autoplan-artifact-permission-ad-v3.json @@ -0,0 +1,87 @@ +{ + "provenance": { + "runLabel": "ship-source-ad-full-paid-20260909-v3", + "sourceCommit": "4636893f5201e9357f9af2dd3cbbfb679e57bfdc", + "capturedScreenSha256": "1fff662a95e7ee1d0b362d3ca9b56e0665982d201caa6fc78b2157ef8a8008bc", + "publicEventsSha256": "12aa45b4d9fd2035e7c44f5b373d60f32e1cd15c655ba9632a8eb33761f00a61", + "note": "Exact retained current viewport and public parent Write/Edit requests/results only. Event projection preserves the recorded session identity from retained native descriptor. Tests relocate the owned path and replay current file state from the final successful Write; this is projected execution, not a historical granted permission or pass.", + "commandTimestamp": "Actual retained parent user /autoplan timestamp, not reconstructed from viewport." + }, + "commandStartedAt": 1788984403953, + "sessionId": "f59fb94e-e006-49c9-8cdf-983aaa0e3a61", + "cwd": "/tmp/gstack-paid-shard-kz30Zk/tmp/gstack-autoplan-chain-PdOGYy", + "stateRoot": "/tmp/gstack-paid-shard-kz30Zk/tmp/gstack-hermetic-2065087-pJs7H4/skill-home-w8xczb/.gstack", + "viewport": " 64 +- **Success target made numeric:** 45 seconds absolute; if the production baseline is already under 60 seconds, the\n + target becomes 25% below baseline and the Final Gate premise item is escalated. \n 65 +- **Session join check is P1**, part of task T1 (a precondition to flag-on), with a fallback metric (per-member dai\n +ly median joined on member ID and day). Row 0b's query remains P2. \n 66 +- **Exposure metric labelled** with cohort (flag on or off) and entry kind (redirect or direct visit) so redirected\n + and direct visitors are compared separately. \n 67 +- **Toast triggers enumerated:** mark-all-read outcomes only. Quick actions are links and raise no toast. \n 68 +- **Route registration file** for `/dashboard` added to blast radius. \n 69 +- **Mark-all-read validation tightening** is a decision, not just a blast-radius line: rejecting malformed or futur\n +e snapshots with 422 is a security hardening accepted in the review record's security section; the API contract for\n + valid input does not change. \n 70 +- **Freshness after \"View all\":** `usePanelData` fetches on mount and on every route entry, so a keep-alive router \n +still refreshes. \n 71 +- **Row 2 (shell badge) is P3**, consistent with \"design happens when picked up\". **Row 3 (undo)** is not symmetric\n +: it must restore prior read state, so it needs state capture; effort L when picked up. \n 72 +- **Units:** \"points\" everywhere means percentage points. \n 73 +- **Row 8 criterion is an explicit proxy:** share of first actions labelled \"resume assigned work\" stands in for th\n +e trigger population (exactly one assigned item, no unread alerts), which cannot be measured until the dashboard ex\n +poses both counts. \n 74 +- **Task index:** tasks T1 to T14 are listed in the review record's CEO Implementation Tasks section and in `~/.gst\n +ack/projects/gstack-autoplan-chain-PdOGYy/tasks-ceo-review-20260909-201501.jsonl`; the Final Gate aggregates them. \n 75 + \n 76 +## Reviewer Concerns (unresolved after three iterations) \n 77 + \n 78 +- The document cannot name routes, action IDs, or the analytics session key because the repository contains no appl\n +ication source. Each is pinned by a named task at implementation start; the reviewer considers that a deferral. \n 79 +- Baseline item 1 changes \"Users land here after login\" into a flag-gated cohort redirect. This is a rollout mechan\n +ism, not a scope change, but the reviewer wants it labelled as a decision; it is recorded here as such. \n 80 + \n 81 ## Deferred to TODOS.md (one line each; design happens when the item is picked up)\n 82 - Shell unread badge (P2, S).\n 83 - Undo for mark-all-read (P3, M); needs a new mutation API decision first.\n\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\u254c\n Do you want to make this edit to 2026-09-09-user-dashboard.md?\n \u276f 1. Yes\n 2. Yes, and switch to accept edits (auto-approve file edits and common file commands) for this session (shift+tab)\n 3. No\n\n Esc to cancel \u00b7 Tab to amend\n", + "events": [ + { + "sessionId": "f59fb94e-e006-49c9-8cdf-983aaa0e3a61", + "timestamp": "2026-09-09T20:14:13.991Z", + "toolUseId": "toolu_01JEjFAdBnCZwTzBXkejvP4n", + "kind": "use", + "name": "Write", + "input": { + "file_path": "/tmp/gstack-paid-shard-kz30Zk/tmp/gstack-hermetic-2065087-pJs7H4/skill-home-w8xczb/.gstack/projects/gstack-autoplan-chain-PdOGYy/ceo-plans/2026-09-09-user-dashboard.md", + "content": "---\nstatus: ACTIVE\n---\n# CEO Plan: User Dashboard Page\nGenerated by /plan-ceo-review (via /autoplan) on 2026-09-09\nBranch: main | Mode: SELECTIVE EXPANSION\nRepo: gstack-autoplan-chain-PdOGYy (no remote configured)\n\n## Vision\n\n### 10x Check\nThe 10x version is not a better dashboard. It is a landing that already knows what the member came to do. When a member has exactly one assigned item and no unread alerts, the login lands them inside that item with a one-line \"3 changes since you left\" strip; when they have alerts, the landing leads with the alert that blocks them. The dashboard in this plan is the necessary first step: it is the only surface that can host that adaptive behavior later, and it produces the exposure and click data needed to decide which action deserves the redirect. Effort for the adaptive landing itself: human ~2 weeks / CC ~2 hours, gated on two weeks of dashboard analytics. It is deferred, not rejected.\n\n### Platonic Ideal\nNot produced (SELECTIVE EXPANSION mode).\n\n## Scope Decisions\n\n| # | Proposal | Effort | Decision | Reasoning |\n|---|----------|--------|----------|-----------|\n| 1 | \"View all\" links from each panel to the existing full activity and notification pages | S | ACCEPTED | In blast radius (panel components only), completes the 20-record cap story, zero new infra |\n| 2 | Unread-count badge in the shared page shell header | S | DEFERRED | Touches the page shell, outside this plan's files; valuable but separate |\n| 3 | Inline \"Undo\" after mark-all-read | M | DEFERRED | Requires a new unmark mutation API; the plan states no new mutation API |\n| 4 | Real-time updates for notifications (polling or push) | L | DEFERRED | New infra; the dashboard refetches on focus and after mutations, which covers v1 |\n| 5 | \"New since your last visit\" divider in the activity feed | M | DEFERRED | Requires persisting last-visit time per member; new storage |\n| 6 | Prefetch dashboard payload in the login response | M | DEFERRED | Touches the login flow, outside blast radius |\n| 7 | Keyboard shortcuts (g+d, j/k through notifications) | S | DEFERRED | Low value for the first cohort; revisit with usage data |\n| 8 | Adaptive landing: redirect straight into work when one obvious next item exists (10x) | L | DEFERRED (Phase 2) | Needs dashboard exposure/click data first; surfaced at the Final Gate as a taste item |\n\n## Accepted Scope (added to this plan)\n- Per-panel \"View all\" links to the existing full activity page and full notifications page.\n\n## Deferred to TODOS.md\n- Shell unread badge (P2, S): one badge in the page shell fed by the notifications unread count; reuses the same endpoint section.\n- Undo for mark-all-read (P3, M): needs an unmark-at-or-before-snapshot mutation; the snapshot semantics make it cheap once the API exists.\n- Real-time notifications (P3, L): polling first, push later; only if refetch-on-focus proves insufficient.\n- \"New since last visit\" activity divider (P3, M): persist last dashboard exposure timestamp per member.\n- Prefetch dashboard payload at login (P3, M): saves one round trip; measure first.\n- Keyboard shortcuts (P3, S).\n- Adaptive landing / smart redirect (P2, L): Phase 2, decided from two weeks of dashboard analytics segmented by first action type.\n" + } + }, + { + "sessionId": "f59fb94e-e006-49c9-8cdf-983aaa0e3a61", + "timestamp": "2026-09-09T20:14:16.766Z", + "toolUseId": "toolu_01JEjFAdBnCZwTzBXkejvP4n", + "kind": "result", + "content": "File created successfully at: /tmp/gstack-paid-shard-kz30Zk/tmp/gstack-hermetic-2065087-pJs7H4/skill-home-w8xczb/.gstack/projects/gstack-autoplan-chain-PdOGYy/ceo-plans/2026-09-09-user-dashboard.md (file state is current in your context \u2014 no need to Read it back)", + "isError": false + }, + { + "sessionId": "f59fb94e-e006-49c9-8cdf-983aaa0e3a61", + "timestamp": "2026-09-09T20:21:08.778Z", + "toolUseId": "toolu_01ABvTvbVrtF2d3yKu1o2ouu", + "kind": "use", + "name": "Write", + "input": { + "file_path": "/tmp/gstack-paid-shard-kz30Zk/tmp/gstack-hermetic-2065087-pJs7H4/skill-home-w8xczb/.gstack/projects/gstack-autoplan-chain-PdOGYy/ceo-plans/2026-09-09-user-dashboard.md", + "content": "---\nstatus: ACTIVE\n---\n# CEO Plan: User Dashboard Page\nGenerated by /plan-ceo-review (via /autoplan) on 2026-09-09\nBranch: main | Mode: SELECTIVE EXPANSION\nRepo: gstack-autoplan-chain-PdOGYy (no remote configured)\n\n## How to read this document\n\n- **Source plan:** `.claude/plans/ui-heavy-feature.md` in the repo. This document records scope *decisions* made during the CEO review; it does not replace the source plan. The full review, including every accepted engineering requirement referenced below, lives in the active plan file's \"Review record\" section (`## Review record`, block `autoplan-accepted:ceo`).\n- **Effort scale (one scale, used everywhere):** S = human under 1 day, CC under 30 minutes. M = human 1 to 3 days, CC under 2 hours. L = human 1 to 2 weeks, CC 2 to 4 hours. \"CC\" means implementation with Claude Code plus gstack.\n- **Priority scale:** P1 blocks shipping this plan. P2 should land on this branch or the next one. P3 is a backlog item. Priority is unrelated to \"Phase 2\", which means \"a separate later plan, after this one ships and has data\".\n- **Final Gate:** the single approval step at the end of the /autoplan pipeline where the user confirms or overrides recommendations. A \"taste item\" is a decision reasonable people could make differently; it is auto-decided with a recommendation and surfaced at the Final Gate for the user to confirm or flip.\n- **Blast radius (the files this plan may touch):** `src/pages/UserDashboard.tsx`; `src/components/dashboard/` (ActivityFeed, NotificationsPanel, QuickActions, MarkAllReadDialog, usePanelData, PanelState); `src/components/feedback/` (ToastProvider, useToast); `src/api/dashboard/` (handler, envelope); the snapshot validation in the existing mark-all-read handler; dashboard tests under `test/` and `e2e/`; `docs/dashboard-rollout.md`. Anything else (page shell, login flow, repositories, schema) is outside blast radius.\n\n## Baseline scope (unchanged from the source plan)\n\n1. New page `/dashboard` (`UserDashboard.tsx`) that becomes the post-login landing for members in the `dashboard_landing` flag cohort.\n2. Three panels: `ActivityFeed` (immutable audit history), `NotificationsPanel` (member alerts with read state), `QuickActions` (the three registry actions, filtered by server-side eligibility). Each panel has loading, empty, error and success states.\n3. Confirmation modal for \"Mark all as read\", built on the existing dialog primitive, calling the existing snapshot-bounded idempotent bulk-read API. No new mutation API.\n4. Toast feedback (a `role=\"status\"` live region) for action results; required by the existing accessibility policy.\n5. New aggregate endpoint `GET /api/dashboard` composing the three existing repository reads with per-section success or failure; no schema change.\n6. Exposure and interaction instrumentation for the new page (the source plan states the page \"still needs its own exposure and interaction instrumentation\"). Decision row 0 below makes this explicit.\n7. Out of scope per the source plan: dark mode, personalization.\n\nBehaviors cited below as \"accepted requirements\" (refetch on window focus throttled to once per 60 seconds, refetch of the notifications section after mark-all-read, 10 rendered items per panel) are recorded in the accepted-requirements block of the review record, not invented here.\n\n## Vision\n\n### 10x Check\nThe 10x version is not a better dashboard. It is a landing that already knows what the member came to do. When a member has exactly one assigned item and no unread alerts, login lands them inside that item with a one-line \"3 changes since you left\" strip; when they have alerts, the landing leads with the alert that blocks them. The dashboard in this plan is the necessary first step: it is the only surface that can host that adaptive behavior later, and it produces the exposure and click data needed to decide which action deserves the redirect. Effort for the adaptive landing itself: L, and only after the data in row 8 exists. It is deferred, not rejected.\n\n### Platonic Ideal\nNot produced (SELECTIVE EXPANSION mode).\n\n## Scope Decisions\n\n| # | Proposal | Effort | Decision | Reasoning |\n|---|----------|--------|----------|-----------|\n| 0 | Analytics events: `dashboard_exposure` on page view and `dashboard_action_click{action_id}` on quick-action click, plus first-action-after-login segmentation computed from existing login/action-start events | S | ACCEPTED (P1) | Already required by the source plan; made explicit because rows 8 and the rollout criteria depend on it |\n| 1 | \"View all\" link in the footer of ActivityFeed and NotificationsPanel, same tab, no filter state, pointing at the existing full activity page and full notifications page. Rendered only in the panel's success state (hidden while loading, on error, and when the panel is empty). Routes are the ones the existing pages already own; confirm the exact paths at implementation. | S | ACCEPTED (P1) | Inside blast radius (panel components only); gives the 10-item cap a real destination. Read-state freshness after navigating back is covered by the accepted refetch-on-mount and refetch-on-focus requirements. |\n| 2 | Unread-count badge in the shared page shell header | S | DEFERRED (P2) | The page shell is outside blast radius |\n| 3 | Inline \"Undo\" after mark-all-read | M | DEFERRED (P3) | Requires a new unmark mutation; the source plan states no new mutation API. Assumption for the estimate: the existing bulk-read API is snapshot-bounded, as the source plan states, so an inverse \"unmark at or before snapshot\" call is a symmetric M-sized API addition; if the real API marks by ID list, re-estimate. |\n| 4 | Real-time notification updates | L | DEFERRED (P3) | New infra. Revisit if, after the 100% rollout, more than 5% of dashboard sessions stay open longer than 10 minutes while new notifications arrive for that member (measurable from exposure events plus notification creation times). Until then, refetch on focus and after mutations covers v1. |\n| 5 | \"New since your last visit\" divider in the activity feed | M | DEFERRED (P3) | Requires persisting a per-member timestamp; new storage |\n| 6 | Prefetch dashboard payload in the login response | M | DEFERRED (P3) | Touches the login flow, outside blast radius |\n| 7 | Keyboard shortcuts | S | DEFERRED (P3) | Low value for the first cohort; revisit with usage data |\n| 8 | Adaptive landing: redirect straight into the assigned item when it is the member's only obvious next step; otherwise show the dashboard | L | DEFERRED, Phase 2 (P2) | Decision criterion: after two weeks at 100% rollout, if more than 60% of first actions after login are \"resume assigned work\" across at least 500 sessions, open the Phase 2 plan. Owner: the rollout kill owner named in `docs/dashboard-rollout.md`. Also the independent CEO reviewer's preferred reframing; surfaced at the Final Gate as a taste item. |\n\n## Accepted Scope (added to this plan)\n- Row 0: explicit exposure and action-click analytics events plus first-action segmentation.\n- Row 1: footer \"View all\" links on the two list panels, success state only.\n\n## Deferred to TODOS.md (one line each; design happens when the item is picked up)\n- Shell unread badge (P2, S).\n- Undo for mark-all-read (P3, M); needs a new mutation API decision first.\n- Real-time notifications (P3, L); criterion in row 4.\n- \"New since last visit\" activity divider (P3, M).\n- Prefetch dashboard payload at login (P3, M).\n- Keyboard shortcuts (P3, S).\n- Adaptive landing (Phase 2, P2, L); criterion and owner in row 8.\n" + } + }, + { + "sessionId": "f59fb94e-e006-49c9-8cdf-983aaa0e3a61", + "timestamp": "2026-09-09T20:21:09.785Z", + "toolUseId": "toolu_01ABvTvbVrtF2d3yKu1o2ouu", + "kind": "result", + "content": "The file /tmp/gstack-paid-shard-kz30Zk/tmp/gstack-hermetic-2065087-pJs7H4/skill-home-w8xczb/.gstack/projects/gstack-autoplan-chain-PdOGYy/ceo-plans/2026-09-09-user-dashboard.md has been updated successfully. (file state is current in your context \u2014 no need to Read it back)", + "isError": false + }, + { + "sessionId": "f59fb94e-e006-49c9-8cdf-983aaa0e3a61", + "timestamp": "2026-09-09T20:24:23.997Z", + "toolUseId": "toolu_01FUctxcUNfgcfnxpu5tQvFA", + "kind": "use", + "name": "Write", + "input": { + "file_path": "/tmp/gstack-paid-shard-kz30Zk/tmp/gstack-hermetic-2065087-pJs7H4/skill-home-w8xczb/.gstack/projects/gstack-autoplan-chain-PdOGYy/ceo-plans/2026-09-09-user-dashboard.md", + "content": "---\nstatus: ACTIVE\n---\n# CEO Plan: User Dashboard Page\nGenerated by /plan-ceo-review (via /autoplan) on 2026-09-09\nBranch: main | Mode: SELECTIVE EXPANSION\nRepo: gstack-autoplan-chain-PdOGYy (no remote configured)\n\n## How to read this document\n\n- **Source plan:** `.claude/plans/ui-heavy-feature.md` in the repo. This document records scope *decisions* made during the CEO review; it does not replace the source plan. The full review, including every accepted engineering requirement referenced below, lives in the active plan file's \"Review record\" section (block `autoplan-accepted:ceo`).\n- **Repository state:** the repository under review contains only `README.md` and the source plan. There is no application source, so routes, action IDs and analytics join keys cannot be read here; where this document depends on them it says so and names the implementation task that pins them down.\n- **Effort scale (one scale, used everywhere):** S = human under 1 day, CC under 30 minutes. M = human 1 to 3 days, CC under 2 hours. L = human 1 to 2 weeks, CC 2 to 4 hours. \"CC\" means implementation with Claude Code plus gstack.\n- **Priority scale (applies only to work on this branch or the next):** P1 blocks shipping this plan. P2 lands on this branch or the next one. P3 is a backlog item. Items marked \"Phase 2\" are a separate later plan that starts only after this one ships and has data; they carry no branch priority.\n- **Final Gate:** the single approval step at the end of the /autoplan pipeline where the user confirms or overrides recommendations. A \"taste item\" is a decision reasonable people could make differently; it is auto-decided with a recommendation and surfaced at the Final Gate for the user to confirm or flip.\n- **Blast radius (the files this plan may touch):** `src/pages/UserDashboard.tsx`; `src/components/dashboard/` (ActivityFeed, NotificationsPanel, QuickActions, MarkAllReadDialog, usePanelData, PanelState); `src/components/feedback/` (ToastProvider, useToast); `src/api/dashboard/` (handler, envelope); one flag-gated conditional at the existing post-login redirect site; input-validation tightening in the existing mark-all-read handler (reject malformed or future snapshots, no contract change for valid input, no new API); analytics event emission from `UserDashboard` and `QuickActions` through the existing analytics client (no new analytics module); dashboard tests under `test/` and `e2e/`; `docs/dashboard-rollout.md` (created by this plan). Anything else (rest of the page shell and login flow, repositories, schema) is outside blast radius.\n\n## Baseline scope (unchanged from the source plan)\n\n1. New page `/dashboard` (`UserDashboard.tsx`), registered for every authenticated workspace member. It renders for anyone who visits it directly. The `dashboard_landing` flag changes only the post-login redirect target for members in the cohort.\n2. Three panels: `ActivityFeed` (immutable audit history), `NotificationsPanel` (member alerts with read state), `QuickActions` (the three registry actions, filtered by server-side eligibility). Each panel has loading, empty, error and success states. The source plan names the actions by label only: \"create an item\", \"resume assigned work\", \"invite a member\"; their stable IDs come from the registry and are read at implementation (task T10).\n3. Confirmation modal for \"Mark all as read\", built on the existing dialog primitive, calling the existing bulk-read API, which the source plan states is idempotent and snapshot-bounded (marks only notifications at or before the supplied snapshot time). No new mutation API.\n4. Toast feedback (a `role=\"status\"` live region) for action results; required by the existing accessibility policy.\n5. New aggregate endpoint `GET /api/dashboard` composing the three existing repository reads with per-section success or failure; no schema change.\n6. Exposure and interaction instrumentation for the new page (the source plan states the page \"still needs its own exposure and interaction instrumentation\"). Decision row 0a below makes this explicit.\n7. Out of scope per the source plan: dark mode, personalization.\n\nBehaviors cited below as accepted requirements are recorded in the accepted-requirements block of the review record: `usePanelData` fetches on mount and refetches on window focus throttled to once per 60 seconds; the notifications section refetches after mark-all-read; each list panel renders at most 10 of the 20 returned items; `dashboard_exposure_total` fires once per route entry, never on refetch.\n\nRollout criteria, in one line (full text in the review record and `docs/dashboard-rollout.md`): cohorts 10%, 50%, 100% at 7 days each; success is median login-to-first-completed-task at or below a target restated relative to the production baseline measured before flag-on; kill if completed-task rate drops more than 2 points or permission-error rate rises at all; rollback is flag off. **Kill owner:** the rollout doc is created by this plan, so the owner is not yet named; the user names the owner at the Final Gate or in the PR, and the rollout doc cannot be merged without a named owner (task T12).\n\n## Vision\n\n### 10x Check\nThe 10x version is not a better dashboard. It is a landing that already knows what the member came to do. When a member has exactly one assigned item and no unread alerts, login lands them inside that item with a one-line \"3 changes since you left\" strip; when they have alerts, the landing leads with the alert that blocks them. The dashboard in this plan is the necessary first step: it is the only surface that can host that adaptive behavior later, and it produces the exposure and click data needed to decide which action deserves the redirect. Effort for the adaptive landing itself: L, and only after the data in row 8 exists. It is deferred, not rejected.\n\n### Platonic Ideal\nNot produced (SELECTIVE EXPANSION mode).\n\n## Scope Decisions\n\n| # | Proposal | Effort | Decision | Reasoning |\n|---|----------|--------|----------|-----------|\n| 0a | Page analytics events: `dashboard_exposure_total` once per route entry, `dashboard_action_click_total{action_id}` per quick-action click, emitted through the existing analytics client from `UserDashboard` and `QuickActions` | S | ACCEPTED (P1) | Already required by the source plan; made explicit because the rollout criteria and row 8 depend on it |\n| 0b | First-action-after-login segmentation: a saved analytics query joining existing login and action-start events by their session key, owned by the rollout owner, reviewed at the Phase 2 evaluation | S | ACCEPTED (P2) | Serves row 8 only, so it must not block shipping. Assumption to verify in task T1 before the query is written: login and action-start events share a session key. If they do not, row 8 is decided on exposure and click data alone. |\n| 1 | \"View all\" link in the footer of ActivityFeed and NotificationsPanel, same tab, no filter state, pointing at the existing full activity page and full notifications page. Rendered only in the success state and only when the section's response cursor indicates more records than were returned; hidden while loading, on error, when empty, and when everything already fits. Routes are the ones the existing pages own; they cannot be read in this repository and are pinned in task T10 at implementation start. | S | ACCEPTED (P1) | Inside blast radius (panel components only); gives the 10-item cap a real destination. Read-state freshness after navigating back is covered by the accepted fetch-on-mount requirement, not by the 60-second focus throttle. |\n| 2 | Unread-count badge in the shared page shell header | S | DEFERRED (P2) | The page shell is outside blast radius |\n| 3 | Inline \"Undo\" after mark-all-read | M | DEFERRED (P3) | Requires a new unmark mutation; the source plan states no new mutation API. Because the bulk-read API is snapshot-bounded, the inverse call is a symmetric M-sized addition when it is picked up. |\n| 4 | Real-time notification updates | L | DEFERRED (P3) | New infra. Revisit criterion, measurable with existing tooling: more than 5 support tickets or member reports per month about stale alerts on the dashboard after the 100% rollout. Until then, fetch on mount, refetch on focus and refetch after mutations cover v1. |\n| 5 | \"New since your last visit\" divider in the activity feed | M | DEFERRED (P3) | Requires persisting a per-member timestamp; new storage |\n| 6 | Prefetch dashboard payload in the login response | M | DEFERRED (P3) | Touches the login flow beyond the single redirect conditional, outside blast radius |\n| 7 | Keyboard shortcuts | S | DEFERRED (P3) | Low value for the first cohort; revisit with usage data |\n| 8 | Adaptive landing: redirect straight into the assigned item when it is the member's only obvious next step; otherwise show the dashboard | L | DEFERRED, Phase 2 (no branch priority) | Decision criterion: after two weeks at 100% rollout, if more than 60% of first actions after login are the registry action labelled \"resume assigned work\" across at least 500 sessions (from the row 0b query), open the Phase 2 plan. Owner: the rollout kill owner. Also the independent CEO reviewer's preferred reframing; surfaced at the Final Gate as a taste item. |\n\n## Accepted Scope (added to this plan)\n- Row 0a: explicit exposure and action-click analytics events (P1).\n- Row 0b: first-action segmentation query (P2, after the join-key check in T1).\n- Row 1: footer \"View all\" links on the two list panels, success state with more records only (P1).\n\n## Deferred to TODOS.md (one line each; design happens when the item is picked up)\n- Shell unread badge (P2, S).\n- Undo for mark-all-read (P3, M); needs a new mutation API decision first.\n- Real-time notifications (P3, L); criterion in row 4.\n- \"New since last visit\" activity divider (P3, M).\n- Prefetch dashboard payload at login (P3, M).\n- Keyboard shortcuts (P3, S).\n- Adaptive landing (Phase 2, L); criterion and owner in row 8.\n" + } + }, + { + "sessionId": "f59fb94e-e006-49c9-8cdf-983aaa0e3a61", + "timestamp": "2026-09-09T20:24:27.406Z", + "toolUseId": "toolu_01FUctxcUNfgcfnxpu5tQvFA", + "kind": "result", + "content": "The file /tmp/gstack-paid-shard-kz30Zk/tmp/gstack-hermetic-2065087-pJs7H4/skill-home-w8xczb/.gstack/projects/gstack-autoplan-chain-PdOGYy/ceo-plans/2026-09-09-user-dashboard.md has been updated successfully. (file state is current in your context \u2014 no need to Read it back)", + "isError": false + }, + { + "sessionId": "f59fb94e-e006-49c9-8cdf-983aaa0e3a61", + "timestamp": "2026-09-09T20:27:34.342Z", + "toolUseId": "toolu_01STRhL1CTCpvY9CAfwjquhi", + "kind": "use", + "name": "Edit", + "input": { + "replace_all": false, + "file_path": "/tmp/gstack-paid-shard-kz30Zk/tmp/gstack-hermetic-2065087-pJs7H4/skill-home-w8xczb/.gstack/projects/gstack-autoplan-chain-PdOGYy/ceo-plans/2026-09-09-user-dashboard.md", + "old_string": "## Deferred to TODOS.md (one line each; design happens when the item is picked up)", + "new_string": "## Corrections after adversarial review (three iterations, convergence guard stopped the loop)\n\n- **\"View all\" rule corrected:** shown when the section returned more items than the 10 displayed *or* the cursor indicates further pages. The earlier \"cursor only\" rule would have stranded items 11 to 20.\n- **Kill rules corrected:** measured per 7-day stage against the flag-off cohort with at least 1,000 sessions per cohort; completed-task rate down more than 2 percentage points, or permission-error rate up more than 0.5 percentage points. \"Rises at all\" was noise-prone.\n- **Success target made numeric:** 45 seconds absolute; if the production baseline is already under 60 seconds, the target becomes 25% below baseline and the Final Gate premise item is escalated.\n- **Session join check is P1**, part of task T1 (a precondition to flag-on), with a fallback metric (per-member daily median joined on member ID and day). Row 0b's query remains P2.\n- **Exposure metric labelled** with cohort (flag on or off) and entry kind (redirect or direct visit) so redirected and direct visitors are compared separately.\n- **Toast triggers enumerated:** mark-all-read outcomes only. Quick actions are links and raise no toast.\n- **Route registration file** for `/dashboard` added to blast radius.\n- **Mark-all-read validation tightening** is a decision, not just a blast-radius line: rejecting malformed or future snapshots with 422 is a security hardening accepted in the review record's security section; the API contract for valid input does not change.\n- **Freshness after \"View all\":** `usePanelData` fetches on mount and on every route entry, so a keep-alive router still refreshes.\n- **Row 2 (shell badge) is P3**, consistent with \"design happens when picked up\". **Row 3 (undo)** is not symmetric: it must restore prior read state, so it needs state capture; effort L when picked up.\n- **Units:** \"points\" everywhere means percentage points.\n- **Row 8 criterion is an explicit proxy:** share of first actions labelled \"resume assigned work\" stands in for the trigger population (exactly one assigned item, no unread alerts), which cannot be measured until the dashboard exposes both counts.\n- **Task index:** tasks T1 to T14 are listed in the review record's CEO Implementation Tasks section and in `~/.gstack/projects/gstack-autoplan-chain-PdOGYy/tasks-ceo-review-20260909-201501.jsonl`; the Final Gate aggregates them.\n\n## Reviewer Concerns (unresolved after three iterations)\n\n- The document cannot name routes, action IDs, or the analytics session key because the repository contains no application source. Each is pinned by a named task at implementation start; the reviewer considers that a deferral.\n- Baseline item 1 changes \"Users land here after login\" into a flag-gated cohort redirect. This is a rollout mechanism, not a scope change, but the reviewer wants it labelled as a decision; it is recorded here as such.\n\n## Deferred to TODOS.md (one line each; design happens when the item is picked up)" + } + } + ] +} diff --git a/test/fixtures/devex-seed-coverage-ad-v3.json b/test/fixtures/devex-seed-coverage-ad-v3.json new file mode 100644 index 000000000..905618460 --- /dev/null +++ b/test/fixtures/devex-seed-coverage-ad-v3.json @@ -0,0 +1,368 @@ +{ + "sourceHead": "4636893f5201e9357f9af2dd3cbbfb679e57bfdc", + "provenance": "Exact five seeded native questions and completions from each naturally failed V3 attempt; additional real work is preserved in the retained full census, not relabeled as a pass.", + "attempts": [ + { + "attempt": 1, + "calls": [ + { + "sessionId": "4ee2d851-255c-425b-976f-9f207925627d", + "toolUseId": "toolu_01VXrmuvLXa6RMo8tRHbZFqb", + "questions": [ + { + "question": "D3 — Journey stage INSTALL / HELLO WORLD: the README quickstart points at a file that does not ship. Fix how?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 1 of 5.\nELI10: README.md:10-11 tells the developer to run `python examples/first_eval.py` right after pip install. docs/package-contents.txt:8-9 confirms that file is in neither the published wheel nor the examples archive. Your ML engineer's very first command after install fails with \"No such file or directory\". Meanwhile the command that actually works, `python -m evalkit.demo`, is two paragraphs lower. This violates Zero Friction at T0: the golden path is broken at step 2 of 2.\nStakes if we pick wrong: Minute 1 ends in a file-not-found error and a developer who now doubts every other line of the README.\nRecommendation: A because the demo module is already shipped and working (package-contents.txt:9-10), so the README should point at it and the missing file should stop being referenced anywhere.\nCompleteness: A=10/10, B=7/10, C=4/10\nNet: one golden path that exists vs maintaining two entry points, one of which must still be built.", + "header": "Quickstart", + "multiSelect": false, + "options": [ + { + "label": "Point quickstart at evalkit.demo (recommended)", + "description": "✅ The command already ships and works; README.md:11 changes to `python -m evalkit.demo` and the missing-file reference is removed everywhere. ✅ One golden path, no decision fatigue, matches the approved delivery vehicle. ❌ Drops the idea of a standalone editable example script from the quickstart. (human: ~30 min / CC: ~2 min)" + }, + { + "label": "Ship examples/first_eval.py too", + "description": "✅ Gives developers an editable script to copy into their own project. ✅ Makes the existing README line true instead of rewriting it. ❌ Adds a second first-run path and a new file that must be packaged, tested, and kept in sync with the demo. (human: ~half day / CC: ~15 min)" + }, + { + "label": "Add a note that the file is coming", + "description": "✅ Smallest possible edit to the README. ✅ No packaging change. ❌ The first command still does nothing useful, and a 'coming soon' in a quickstart reads as abandonment." + }, + { + "label": "Acceptable friction, skip", + "description": "✅ No work in this release. ✅ Developers who read the whole README will find the demo. ❌ The documented first command fails for every new developer at minute 1." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D3 — Journey stage INSTALL / HELLO WORLD: the README quickstart points at a file that does not ship. Fix how?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 1 of 5.\nELI10: README.md:10-11 tells the developer to run `python examples/first_eval.py` right after pip install. docs/package-contents.txt:8-9 confirms that file is in neither the published wheel nor the examples archive. Your ML engineer's very first command after install fails with \"No such file or directory\". Meanwhile the command that actually works, `python -m evalkit.demo`, is two paragraphs lower. This violates Zero Friction at T0: the golden path is broken at step 2 of 2.\nStakes if we pick wrong: Minute 1 ends in a file-not-found error and a developer who now doubts every other line of the README.\nRecommendation: A because the demo module is already shipped and working (package-contents.txt:9-10), so the README should point at it and the missing file should stop being referenced anywhere.\nCompleteness: A=10/10, B=7/10, C=4/10\nNet: one golden path that exists vs maintaining two entry points, one of which must still be built.": "Point quickstart at evalkit.demo (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:10:11.801Z" + }, + { + "sessionId": "4ee2d851-255c-425b-976f-9f207925627d", + "toolUseId": "toolu_01ABAQJEV6AovtsUCJWLiWq7", + "questions": [ + { + "question": "D4 — Journey stage HELLO WORLD: the keyless local demo blocks up to 300s on a remote CI check. Keep, gate, or remove for first local results?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 2 of 5.\nELI10: docs/current-contracts.md:3-5 says the first local evaluation requires a successful remote CI check and blocks for five minutes, with no skip flag or offline path. README.md:17-23 confirms even the keyless bundled demo (mock transport, sample data, no credentials) waits on it. The approved target is under 2 minutes to first result (docs/benchmarks.md:8). A 300-second wait cannot fit inside a 120-second budget, so the plan as written promises a target it cannot meet. The progress lines and timeout message are good, but they polish a wait that should not exist for a local, mock-transport evaluation. This violates Zero Friction at T0 and Decide-for-me-let-me-override (no escape hatch). Peer SDK A finishes everything in 2 minutes.\nStakes if we pick wrong: Your ML engineer, who explicitly wants a local result before touching CI, alt-tabs at minute 2 and half of them never come back; the magical moment arrives after they have left.\nRecommendation: A because local evaluations over the mock transport or sample data have nothing to verify against CI, and the check still runs when it matters: the first remote, keyed evaluation.\nCompleteness: A=10/10, B=8/10, C=5/10\nNet: deliver the demo scores in seconds and keep the CI gate for remote work, vs keeping a mandatory wait that makes the approved target impossible.", + "header": "CI gate", + "multiSelect": false, + "options": [ + { + "label": "No CI gate for local/mock evals (recommended)", + "description": "✅ `python -m evalkit.demo` and any mock-transport or offline evaluation return in seconds; the CI binding check moves to the first live, keyed evaluation where it verifies something real. ✅ Makes the under-2-minute target achievable; existing progress and timeout messaging is reused unchanged on the remote path. ❌ Changes a documented contract; the plan must state the new rule and update current-contracts.md. (human: ~1 day / CC: ~30 min)" + }, + { + "label": "Keep gate, add --skip-ci-check / EVALKIT_SKIP_CI_CHECK", + "description": "✅ Preserves the existing default behavior for anyone relying on it. ✅ Gives an explicit, documented escape hatch and lets the demo pass the flag by default. ❌ The default first-run path still waits 300s unless the developer discovers the flag; the README must lead with it. (human: ~half day / CC: ~20 min)" + }, + { + "label": "Keep gate, run it in the background", + "description": "✅ Demo scores print immediately from the mock transport while the CI check completes asynchronously. ✅ No contract removal, just reordering. ❌ A check the result never depends on is theater; developers will ask why it exists. (human: ~1 day / CC: ~30 min)" + }, + { + "label": "Keep the mandatory wait as documented", + "description": "✅ Zero change to a contract the plan says to retain. ✅ Progress lines and timeout messages already exist. ❌ The under-2-minute target is arithmetically unreachable and the review must record it as a blocking DX debt." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D4 — Journey stage HELLO WORLD: the keyless local demo blocks up to 300s on a remote CI check. Keep, gate, or remove for first local results?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 2 of 5.\nELI10: docs/current-contracts.md:3-5 says the first local evaluation requires a successful remote CI check and blocks for five minutes, with no skip flag or offline path. README.md:17-23 confirms even the keyless bundled demo (mock transport, sample data, no credentials) waits on it. The approved target is under 2 minutes to first result (docs/benchmarks.md:8). A 300-second wait cannot fit inside a 120-second budget, so the plan as written promises a target it cannot meet. The progress lines and timeout message are good, but they polish a wait that should not exist for a local, mock-transport evaluation. This violates Zero Friction at T0 and Decide-for-me-let-me-override (no escape hatch). Peer SDK A finishes everything in 2 minutes.\nStakes if we pick wrong: Your ML engineer, who explicitly wants a local result before touching CI, alt-tabs at minute 2 and half of them never come back; the magical moment arrives after they have left.\nRecommendation: A because local evaluations over the mock transport or sample data have nothing to verify against CI, and the check still runs when it matters: the first remote, keyed evaluation.\nCompleteness: A=10/10, B=8/10, C=5/10\nNet: deliver the demo scores in seconds and keep the CI gate for remote work, vs keeping a mandatory wait that makes the approved target impossible.": "No CI gate for local/mock evals (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:10:33.870Z" + }, + { + "sessionId": "4ee2d851-255c-425b-976f-9f207925627d", + "toolUseId": "toolu_01Sw8ezPFGmW8Az8Z1fArHhp", + "questions": [ + { + "question": "D5 — Journey stage REAL USAGE: run_eval and run_batch take the same two arguments in opposite positional order. Fix how?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 3 of 5.\nELI10: docs/api.md:5-9 documents `run_eval(dataset, evaluator)` and `run_batch(evaluator, dataset)`. Same concepts, reversed order, positional only, and the draft calls the reversal intentional. Your ML engineer learns one call from the demo, writes the other by analogy, and passes a dataset where an evaluator is expected. Best case a confusing type error; worst case both are duck-typed iterables and it runs wrong. This is a beta with the version bump to 2.0 already happening, which is the cheapest moment there will ever be to make the two signatures agree. Violates Usable (consistent grammar) and the one-example test: a developer cannot use the API correctly after seeing one example.\nStakes if we pick wrong: Every developer who uses both functions pays a debugging session for an inconsistency the SDK chose on purpose, and fixing it after GA becomes a breaking change.\nRecommendation: A because 2.0.0b1 is the last free moment to align the order; the alias-and-warn path costs nothing for beta users who have no v2 code yet.\nCompleteness: A=10/10, B=8/10, C=6/10\nNet: fix the grammar once now while it is free, vs shipping a known trap into a 2.x contract.", + "header": "Signatures", + "multiSelect": false, + "options": [ + { + "label": "Align order to (dataset, evaluator) in 2.0 (recommended)", + "description": "✅ `run_batch(dataset, evaluator)` matches `run_eval`; one signature shape to learn, and the docs show both with the same example. ✅ A runtime check raises a clear TypeError when an evaluator lands in the dataset slot, so a swapped call fails loudly with the fix in the message. ❌ Changes a documented signature in the beta; anyone on 2.0.0b1 pre-release code with positional run_batch calls must swap two arguments. (human: ~half day / CC: ~15 min)" + }, + { + "label": "Keep order, require keyword arguments", + "description": "✅ `run_batch(*, dataset, evaluator)` makes the order irrelevant and every call self-documenting. ✅ No silent argument swap is possible. ❌ Positional calls break for both functions, a bigger change than aligning one; more verbose for the common case. (human: ~half day / CC: ~15 min)" + }, + { + "label": "Keep order, document it loudly", + "description": "✅ No code change. ✅ A warning box in docs/api.md and a docstring note. ❌ Documentation does not stop the by-analogy mistake; developers do not read the reference before their second call." + }, + { + "label": "Acceptable friction, skip", + "description": "✅ Zero work; the draft already calls the reversal intentional. ✅ Existing tests keep passing. ❌ Ships a known inconsistency into a 2.x public contract that will cost a breaking change to fix later." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D5 — Journey stage REAL USAGE: run_eval and run_batch take the same two arguments in opposite positional order. Fix how?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 3 of 5.\nELI10: docs/api.md:5-9 documents `run_eval(dataset, evaluator)` and `run_batch(evaluator, dataset)`. Same concepts, reversed order, positional only, and the draft calls the reversal intentional. Your ML engineer learns one call from the demo, writes the other by analogy, and passes a dataset where an evaluator is expected. Best case a confusing type error; worst case both are duck-typed iterables and it runs wrong. This is a beta with the version bump to 2.0 already happening, which is the cheapest moment there will ever be to make the two signatures agree. Violates Usable (consistent grammar) and the one-example test: a developer cannot use the API correctly after seeing one example.\nStakes if we pick wrong: Every developer who uses both functions pays a debugging session for an inconsistency the SDK chose on purpose, and fixing it after GA becomes a breaking change.\nRecommendation: A because 2.0.0b1 is the last free moment to align the order; the alias-and-warn path costs nothing for beta users who have no v2 code yet.\nCompleteness: A=10/10, B=8/10, C=6/10\nNet: fix the grammar once now while it is free, vs shipping a known trap into a 2.x contract.": "Align order to (dataset, evaluator) in 2.0 (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:10:55.952Z" + }, + { + "sessionId": "4ee2d851-255c-425b-976f-9f207925627d", + "toolUseId": "toolu_01LdUtpia2udSkQVnau2ZjkR", + "questions": [ + { + "question": "D6 — Journey stage DEBUG: an invalid API key raises AuthError(\"request failed\") with no code, cause, or fix. Fix how?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 4 of 5.\nELI10: docs/api.md:11-13 says a bad key raises `AuthError(\"request failed\")` and the plan retains that message. docs/current-contracts.md:21-23 says every OTHER error already names the cause, the argument or file, and a fix, and redacts secrets. So auth is the one error that is below the SDK's own bar, and it is the first error your ML engineer is likely to hit: right after the demo, on their first live call, with a pasted key. \"request failed\" could mean network, key, rate limit, or server. Violates Fight Uncertainty: every error must state problem, cause, and fix. The CI timeout message (EVALKIT_CI_TIMEOUT plus URL plus help link) is already the house style; auth just needs to match it.\nStakes if we pick wrong: The first live call fails with a message that gives no direction, the developer suspects the network, and the console page with the fix is one URL they are never shown.\nRecommendation: A because it brings the one substandard error up to the pattern the SDK already uses everywhere else, at trivial cost.\nCompleteness: A=10/10, B=7/10\nNet: one consistent error contract vs a lone opaque message on the most common first failure.", + "header": "AuthError", + "multiSelect": false, + "options": [ + { + "label": "Match house style: code + cause + fix + link (recommended)", + "description": "✅ Raise `AuthError` with code EVALKIT_AUTH_INVALID_KEY, message naming the cause (key rejected for project X, or EVALKIT_API_KEY unset), the fix (create or rotate at the console URL, export the variable), and a help link; key value redacted, last 4 chars shown. ✅ Distinguish unset key, malformed key, and rejected key as separate codes so the fix is exact. ❌ Changes the exception message text; any test asserting on \"request failed\" must be updated. (human: ~half day / CC: ~15 min)" + }, + { + "label": "Add a code and docs link only", + "description": "✅ Minimal change: `AuthError(\"request failed\", code=EVALKIT_AUTH)` with a link to the auth troubleshooting page. ✅ Keeps the message text stable for existing assertions. ❌ The developer still leaves the terminal to learn the cause; unset vs invalid vs revoked are not distinguished." + }, + { + "label": "Keep AuthError(\"request failed\")", + "description": "✅ No change to a retained contract. ✅ No test churn. ❌ The one error most new developers hit stays the one error the SDK explains worst." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D6 — Journey stage DEBUG: an invalid API key raises AuthError(\"request failed\") with no code, cause, or fix. Fix how?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 4 of 5.\nELI10: docs/api.md:11-13 says a bad key raises `AuthError(\"request failed\")` and the plan retains that message. docs/current-contracts.md:21-23 says every OTHER error already names the cause, the argument or file, and a fix, and redacts secrets. So auth is the one error that is below the SDK's own bar, and it is the first error your ML engineer is likely to hit: right after the demo, on their first live call, with a pasted key. \"request failed\" could mean network, key, rate limit, or server. Violates Fight Uncertainty: every error must state problem, cause, and fix. The CI timeout message (EVALKIT_CI_TIMEOUT plus URL plus help link) is already the house style; auth just needs to match it.\nStakes if we pick wrong: The first live call fails with a message that gives no direction, the developer suspects the network, and the console page with the fix is one URL they are never shown.\nRecommendation: A because it brings the one substandard error up to the pattern the SDK already uses everywhere else, at trivial cost.\nCompleteness: A=10/10, B=7/10\nNet: one consistent error contract vs a lone opaque message on the most common first failure.": "Match house style: code + cause + fix + link (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:11:16.020Z" + }, + { + "sessionId": "4ee2d851-255c-425b-976f-9f207925627d", + "toolUseId": "toolu_016VW8iF54h58quZ73na58Zt", + "questions": [ + { + "question": "D7 — Journey stage UPGRADE: v2 removes Client.evaluate() outright with no alias, warning, migration guide, or codemod. Fix how?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 5 of 5.\nELI10: docs/api.md:15-17 says v1's `Client.evaluate()` becomes `Client.run()` in v2 and the old name is removed immediately. Your ML engineer already has v1 scripts in a repo and possibly in CI. They bump the pin to 2.0.0b1 and every script dies with `AttributeError: 'Client' object has no attribute 'evaluate'`, a message that does not mention `run()`. The changelog is otherwise complete (docs/api.md:18), so this is the one break without a trail. Violates Credible (upgrade fear) and Upgrades-should-be-boring. TypeScript never breaks JS; a one-line alias with a DeprecationWarning is the Python equivalent.\nStakes if we pick wrong: The upgrade is the first thing existing users do with 2.0, and it fails for all of them with no pointer to the fix.\nRecommendation: A because a deprecated alias is a few lines, gives every v1 user a working upgrade plus the exact new name in the warning, and the removal can land in 3.0 with notice.\nCompleteness: A=10/10, B=7/10, C=5/10\nNet: a boring upgrade with a warning and a guide vs a hard break on day one of 2.0.", + "header": "v1 to v2", + "multiSelect": false, + "options": [ + { + "label": "Alias + DeprecationWarning + migration guide (recommended)", + "description": "✅ `Client.evaluate()` stays as a thin alias that emits `DeprecationWarning: Client.evaluate() is deprecated, use Client.run(); removed in 3.0` with a link to docs/migrating-v1-to-v2.md. ✅ Migration guide covers every 2.0 break (this rename plus the run_batch order fix from D5) with before/after snippets and a one-line sed or pyupgrade-style codemod. ❌ Keeps one legacy name alive through 2.x, a small maintenance cost. (human: ~1 day / CC: ~30 min)" + }, + { + "label": "Remove, but raise a helpful error", + "description": "✅ Clean 2.0 surface, no legacy name. ✅ `__getattr__` intercepts `evaluate` and raises `AttributeError: Client.evaluate() was renamed to Client.run() in 2.0; see ` so the break explains itself. ❌ Every v1 script still breaks on upgrade; the developer must edit code before anything runs. (human: ~half day / CC: ~15 min)" + }, + { + "label": "Remove, document in changelog only", + "description": "✅ Zero runtime code. ✅ Changelog entry plus a migration section in docs/api.md. ❌ The runtime error stays a bare AttributeError; developers hit the break before they read the changelog." + }, + { + "label": "Acceptable friction, skip", + "description": "✅ The plan already says to retain this behavior. ✅ No work. ❌ Every existing v1 user's first 2.0 experience is an unexplained crash." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D7 — Journey stage UPGRADE: v2 removes Client.evaluate() outright with no alias, warning, migration guide, or codemod. Fix how?\nProject/branch/task: EvalKit SDK beta polish on main; DX POLISH, Step 0F friction point 5 of 5.\nELI10: docs/api.md:15-17 says v1's `Client.evaluate()` becomes `Client.run()` in v2 and the old name is removed immediately. Your ML engineer already has v1 scripts in a repo and possibly in CI. They bump the pin to 2.0.0b1 and every script dies with `AttributeError: 'Client' object has no attribute 'evaluate'`, a message that does not mention `run()`. The changelog is otherwise complete (docs/api.md:18), so this is the one break without a trail. Violates Credible (upgrade fear) and Upgrades-should-be-boring. TypeScript never breaks JS; a one-line alias with a DeprecationWarning is the Python equivalent.\nStakes if we pick wrong: The upgrade is the first thing existing users do with 2.0, and it fails for all of them with no pointer to the fix.\nRecommendation: A because a deprecated alias is a few lines, gives every v1 user a working upgrade plus the exact new name in the warning, and the removal can land in 3.0 with notice.\nCompleteness: A=10/10, B=7/10, C=5/10\nNet: a boring upgrade with a warning and a guide vs a hard break on day one of 2.0.": "Alias + DeprecationWarning + migration guide (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:11:36.110Z" + } + ], + "historicalOutcome": "ceiling_reached", + "legacyReviewCount": 8, + "genuineDecisions": 8, + "observationSha256": "7d0eee358241a9ad11d1fdb8216ee9c0eb46d0b050d86a8f92558863fcacfb11" + }, + { + "attempt": 2, + "calls": [ + { + "sessionId": "5139711c-562b-47db-81ea-387b171beaed", + "toolUseId": "toolu_012AY75Qigv791YdPqtb9s2N", + "questions": [ + { + "question": "D3 — Journey stage HELLO WORLD: the mandatory 5-minute remote CI check blocks the first local result, including the keyless demo. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; tracing python -m evalkit.demo against the settled under-2-minute target.\nELI10: docs/current-contracts.md lines 3-5 say every first local evaluation must pass a remote CI check and blocks for five minutes, with no skip flag or offline path. README.md lines 17-23 confirm the keyless demo waits too. docs/benchmarks.md measured EvalKit at 6 minutes, 5 of which is this wait, against peers at 2 to 4 minutes and an agreed target under 2 minutes. The demo uses a mock transport and bundled data, so the check verifies a sample-project binding the developer does not own. The ML engineer wanted a local number before touching CI, and instead the SDK forces CI on them at minute one.\nStakes if we pick wrong: The 6-minute TTHW is the Needs Work tier and the target is mathematically unreachable while the gate stays. Developers alt-tab during the wait and many do not come back.\nRecommendation: A because it is the only option that hits the settled target, and the progress and timeout messages already in the contract stay useful on the live path. Violates Zero friction at T0 and Decide for me, let me override.\nCompleteness: A=10/10, B=7/10, C=5/10, D=2/10\nA) Move the CI check off the local and demo path; it runs on the first live keyed evaluation or via an explicit verify command (recommended) (human: ~3 days / CC: ~30 min)\n ✅ Demo and first local eval return in seconds; TTHW drops from 6 min to about 1 min, under target\n ✅ The check still guards production use where the sample-project binding actually matters\n ❌ Changes a documented first-run contract; needs a changelog entry and a docs update\nB) Keep the gate but make it non-blocking: print local results immediately, run the check in the background, report its outcome (human: ~3 days / CC: ~30 min)\n ✅ Developer sees scores at once while the binding still gets verified on every first run\n ✅ Preserves the existing progress and timeout messaging almost unchanged\n ❌ Background failure after success is confusing, and CI runners may exit before the check finishes\nC) Add an opt-out flag or offline mode, default stays blocking (human: ~1 day / CC: ~15 min)\n ✅ Small change to the contract and easy to document in the quickstart\n ✅ Gives the noninteractive CI mode a clean way to skip a check it does not need\n ❌ Default path still misses the target; the developer has to know the flag exists to escape\nD) Acceptable friction, keep the mandatory wait\n ✅ Zero implementation cost and no contract change in the beta\n ✅ Nothing to re-test in the first-run code path\n ❌ Locks in a 6-minute TTHW and abandons the agreed under-2-minute target\nNet: trading a documented but adoption-killing gate against a contract change that finally makes the settled benchmark reachable.", + "header": "CI gate", + "multiSelect": false, + "options": [ + { + "label": "A) Move check to live path (recommended)", + "description": "Demo and first local eval run without the remote CI check; the check runs on first keyed live eval or an explicit verify command." + }, + { + "label": "B) Non-blocking background check", + "description": "Return local results immediately; run the CI check in the background and report when done." + }, + { + "label": "C) Opt-out flag, default blocking", + "description": "Add a documented skip flag or offline mode; the default first run still waits." + }, + { + "label": "D) Keep mandatory wait", + "description": "Accept the 6-minute first run and miss the under-2-minute target." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D3 — Journey stage HELLO WORLD: the mandatory 5-minute remote CI check blocks the first local result, including the keyless demo. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; tracing python -m evalkit.demo against the settled under-2-minute target.\nELI10: docs/current-contracts.md lines 3-5 say every first local evaluation must pass a remote CI check and blocks for five minutes, with no skip flag or offline path. README.md lines 17-23 confirm the keyless demo waits too. docs/benchmarks.md measured EvalKit at 6 minutes, 5 of which is this wait, against peers at 2 to 4 minutes and an agreed target under 2 minutes. The demo uses a mock transport and bundled data, so the check verifies a sample-project binding the developer does not own. The ML engineer wanted a local number before touching CI, and instead the SDK forces CI on them at minute one.\nStakes if we pick wrong: The 6-minute TTHW is the Needs Work tier and the target is mathematically unreachable while the gate stays. Developers alt-tab during the wait and many do not come back.\nRecommendation: A because it is the only option that hits the settled target, and the progress and timeout messages already in the contract stay useful on the live path. Violates Zero friction at T0 and Decide for me, let me override.\nCompleteness: A=10/10, B=7/10, C=5/10, D=2/10\nA) Move the CI check off the local and demo path; it runs on the first live keyed evaluation or via an explicit verify command (recommended) (human: ~3 days / CC: ~30 min)\n ✅ Demo and first local eval return in seconds; TTHW drops from 6 min to about 1 min, under target\n ✅ The check still guards production use where the sample-project binding actually matters\n ❌ Changes a documented first-run contract; needs a changelog entry and a docs update\nB) Keep the gate but make it non-blocking: print local results immediately, run the check in the background, report its outcome (human: ~3 days / CC: ~30 min)\n ✅ Developer sees scores at once while the binding still gets verified on every first run\n ✅ Preserves the existing progress and timeout messaging almost unchanged\n ❌ Background failure after success is confusing, and CI runners may exit before the check finishes\nC) Add an opt-out flag or offline mode, default stays blocking (human: ~1 day / CC: ~15 min)\n ✅ Small change to the contract and easy to document in the quickstart\n ✅ Gives the noninteractive CI mode a clean way to skip a check it does not need\n ❌ Default path still misses the target; the developer has to know the flag exists to escape\nD) Acceptable friction, keep the mandatory wait\n ✅ Zero implementation cost and no contract change in the beta\n ✅ Nothing to re-test in the first-run code path\n ❌ Locks in a 6-minute TTHW and abandons the agreed under-2-minute target\nNet: trading a documented but adoption-killing gate against a contract change that finally makes the settled benchmark reachable.": "A) Move check to live path (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:17:49.115Z" + }, + { + "sessionId": "5139711c-562b-47db-81ea-387b171beaed", + "toolUseId": "toolu_01LwsoPGRLus2fTk1fjkfFTA", + "questions": [ + { + "question": "D4 — Journey stage INSTALL/QUICKSTART: README.md tells the developer to run examples/first_eval.py, but that file is not in the wheel or the examples archive. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; tracing the quickstart command in README.md line 11 against docs/package-contents.txt.\nELI10: The quickstart's second step is python examples/first_eval.py. docs/package-contents.txt lists the published files: __init__.py, client.py, demo.py, sample_responses.json, README.md. No examples directory, and the release examples archive lacks the file too. So the ML engineer's first command after pip install fails with a file-not-found error. The working demo, python -m evalkit.demo, sits three paragraphs lower. The quickstart points at the broken path and buries the working one.\nStakes if we pick wrong: The very first thing the developer runs fails. Some scroll and recover; some conclude the beta is broken and leave before ever seeing the demo output.\nRecommendation: A because the demo module already exists, already works, and is the settled magical-moment vehicle, so the quickstart should lead with it and nothing else needs shipping. Violates Zero friction at T0 and Learn by doing.\nCompleteness: A=10/10, B=8/10, C=4/10\nA) Make python -m evalkit.demo the one quickstart command; remove the examples/first_eval.py reference (recommended) (human: ~1 hour / CC: ~5 min)\n ✅ One golden path with a command that is verified to exist in the published package\n ✅ Puts the settled magical moment first, right after pip install, where the clock is ticking\n ❌ Developers who wanted a script they can copy and edit have to open demo.py instead\nB) Ship examples/first_eval.py in the package and archive so the current README works as written (human: ~half day / CC: ~15 min)\n ✅ Gives the developer an editable script they can turn into their own first live eval\n ✅ Keeps the README text stable across the beta\n ❌ Two competing first-run paths cause decision fatigue, and the new file needs its own tests and packaging check\nC) Leave the README, add a note that the file is coming later\n ✅ No packaging or docs restructuring in the beta\n ✅ Signals the roadmap to early adopters\n ❌ The first documented command still fails for every developer who follows the quickstart\nNet: one verified command as the golden path versus preserving a script reference that currently sends every new developer into a dead end.", + "header": "Quickstart", + "multiSelect": false, + "options": [ + { + "label": "A) Demo is the quickstart (recommended)", + "description": "Point the quickstart at python -m evalkit.demo and drop the missing first_eval.py reference." + }, + { + "label": "B) Ship first_eval.py", + "description": "Add the example script to the package and archive so the current README works." + }, + { + "label": "C) Note it as coming later", + "description": "Keep the README and add a note that the example file is not yet published." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D4 — Journey stage INSTALL/QUICKSTART: README.md tells the developer to run examples/first_eval.py, but that file is not in the wheel or the examples archive. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; tracing the quickstart command in README.md line 11 against docs/package-contents.txt.\nELI10: The quickstart's second step is python examples/first_eval.py. docs/package-contents.txt lists the published files: __init__.py, client.py, demo.py, sample_responses.json, README.md. No examples directory, and the release examples archive lacks the file too. So the ML engineer's first command after pip install fails with a file-not-found error. The working demo, python -m evalkit.demo, sits three paragraphs lower. The quickstart points at the broken path and buries the working one.\nStakes if we pick wrong: The very first thing the developer runs fails. Some scroll and recover; some conclude the beta is broken and leave before ever seeing the demo output.\nRecommendation: A because the demo module already exists, already works, and is the settled magical-moment vehicle, so the quickstart should lead with it and nothing else needs shipping. Violates Zero friction at T0 and Learn by doing.\nCompleteness: A=10/10, B=8/10, C=4/10\nA) Make python -m evalkit.demo the one quickstart command; remove the examples/first_eval.py reference (recommended) (human: ~1 hour / CC: ~5 min)\n ✅ One golden path with a command that is verified to exist in the published package\n ✅ Puts the settled magical moment first, right after pip install, where the clock is ticking\n ❌ Developers who wanted a script they can copy and edit have to open demo.py instead\nB) Ship examples/first_eval.py in the package and archive so the current README works as written (human: ~half day / CC: ~15 min)\n ✅ Gives the developer an editable script they can turn into their own first live eval\n ✅ Keeps the README text stable across the beta\n ❌ Two competing first-run paths cause decision fatigue, and the new file needs its own tests and packaging check\nC) Leave the README, add a note that the file is coming later\n ✅ No packaging or docs restructuring in the beta\n ✅ Signals the roadmap to early adopters\n ❌ The first documented command still fails for every developer who follows the quickstart\nNet: one verified command as the golden path versus preserving a script reference that currently sends every new developer into a dead end.": "A) Demo is the quickstart (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:18:09.216Z" + }, + { + "sessionId": "5139711c-562b-47db-81ea-387b171beaed", + "toolUseId": "toolu_01Axt6dTLrfDUfQPJzePRkAz", + "questions": [ + { + "question": "D5 — Journey stage REAL USAGE: run_eval(dataset, evaluator) and run_batch(evaluator, dataset) take the same two concepts in reversed positional order. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; reviewing the public signatures in docs/api.md lines 3-9.\nELI10: The two evaluation functions are the whole public surface the ML engineer touches after the demo. One takes dataset first, the other evaluator first, both positional, and docs/api.md says the reversal is intentional. A developer who learns run_eval and then reaches for run_batch will swap the arguments. In Python nothing catches that until runtime, and depending on duck typing it may silently score the wrong thing. The good API test is: can this developer use it correctly after seeing one example? Here, no.\nStakes if we pick wrong: Swapped arguments produce confusing runtime errors or silently wrong scores in the first real script, and the beta signature becomes the thing 2.0 final has to break again.\nRecommendation: A because the beta is the last cheap moment to fix a signature, and making both functions keyword-friendly with the same order costs almost nothing. Violates Pit of Success and Fight uncertainty.\nCompleteness: A=10/10, B=7/10, C=2/10\nA) Same order in both functions, dataset then evaluator, with a clear runtime TypeError naming both arguments when a swap is detected (recommended) (human: ~half day / CC: ~15 min)\n ✅ One mental model across the API; learning run_eval teaches run_batch for free\n ✅ The swap check turns a silent wrong score into an actionable error on the first call\n ❌ run_batch callers written against the current draft order must flip their arguments before 2.0 final\nB) Keep both orders but make the arguments keyword-only so positional swaps are impossible (human: ~2 hours / CC: ~10 min)\n ✅ Eliminates the silent swap without changing either documented order\n ✅ Call sites become self-documenting: run_batch(evaluator=e, dataset=d)\n ❌ Every call gets longer, and the inconsistency stays in the docs as a permanent oddity\nC) Keep the reversed positional order as documented\n ✅ No signature change and no changelog entry in the beta\n ✅ Existing draft call sites keep working untouched\n ❌ The first thing the developer learns about the API is that it does not follow its own pattern\nNet: fixing the signature once now, while a beta can still change, versus carrying an inconsistency that every future caller has to memorize.", + "header": "Signatures", + "multiSelect": false, + "options": [ + { + "label": "A) Unify order + swap guard (recommended)", + "description": "Both functions take dataset then evaluator; a swap raises a TypeError that names both arguments." + }, + { + "label": "B) Keyword-only arguments", + "description": "Keep both orders but require keyword arguments so positional swaps cannot happen." + }, + { + "label": "C) Keep reversed order", + "description": "Ship the signatures exactly as documented in docs/api.md." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D5 — Journey stage REAL USAGE: run_eval(dataset, evaluator) and run_batch(evaluator, dataset) take the same two concepts in reversed positional order. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; reviewing the public signatures in docs/api.md lines 3-9.\nELI10: The two evaluation functions are the whole public surface the ML engineer touches after the demo. One takes dataset first, the other evaluator first, both positional, and docs/api.md says the reversal is intentional. A developer who learns run_eval and then reaches for run_batch will swap the arguments. In Python nothing catches that until runtime, and depending on duck typing it may silently score the wrong thing. The good API test is: can this developer use it correctly after seeing one example? Here, no.\nStakes if we pick wrong: Swapped arguments produce confusing runtime errors or silently wrong scores in the first real script, and the beta signature becomes the thing 2.0 final has to break again.\nRecommendation: A because the beta is the last cheap moment to fix a signature, and making both functions keyword-friendly with the same order costs almost nothing. Violates Pit of Success and Fight uncertainty.\nCompleteness: A=10/10, B=7/10, C=2/10\nA) Same order in both functions, dataset then evaluator, with a clear runtime TypeError naming both arguments when a swap is detected (recommended) (human: ~half day / CC: ~15 min)\n ✅ One mental model across the API; learning run_eval teaches run_batch for free\n ✅ The swap check turns a silent wrong score into an actionable error on the first call\n ❌ run_batch callers written against the current draft order must flip their arguments before 2.0 final\nB) Keep both orders but make the arguments keyword-only so positional swaps are impossible (human: ~2 hours / CC: ~10 min)\n ✅ Eliminates the silent swap without changing either documented order\n ✅ Call sites become self-documenting: run_batch(evaluator=e, dataset=d)\n ❌ Every call gets longer, and the inconsistency stays in the docs as a permanent oddity\nC) Keep the reversed positional order as documented\n ✅ No signature change and no changelog entry in the beta\n ✅ Existing draft call sites keep working untouched\n ❌ The first thing the developer learns about the API is that it does not follow its own pattern\nNet: fixing the signature once now, while a beta can still change, versus carrying an inconsistency that every future caller has to memorize.": "A) Unify order + swap guard (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:18:29.315Z" + }, + { + "sessionId": "5139711c-562b-47db-81ea-387b171beaed", + "toolUseId": "toolu_01Snhr6ovogM3veYuae8g3tp", + "questions": [ + { + "question": "D6 — Journey stage DEBUG: an invalid API key raises AuthError(\"request failed\") with no code, cause, or fix. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; reviewing the authentication error in docs/api.md lines 11-13.\nELI10: After the demo, the ML engineer creates a key in the console, exports EVALKIT_API_KEY, and runs their first live evaluation. If the key is mistyped, expired, revoked, or scoped to the wrong project, they see the two words request failed. That message is indistinguishable from a network outage or a server error. docs/current-contracts.md line 22 says every other error already names the cause and an actionable fix, so this is the one error that breaks the SDK's own standard, and it fires at the exact moment the developer goes from demo to real usage.\nStakes if we pick wrong: The developer cannot tell whether to fix their key, their network, or wait for the service. They open a support ticket or give up at the first live call.\nRecommendation: A because the SDK's other errors already follow the problem-cause-fix formula, so matching it here is consistency, not new design. Violates Fight uncertainty.\nCompleteness: A=10/10, B=7/10, C=1/10\nA) Structured auth error: code EVALKIT_AUTH_INVALID_KEY, cause (rejected key, redacted prefix, project), fix (create or rotate key at the console URL, re-export the variable), and a docs link (recommended) (human: ~half day / CC: ~15 min)\n ✅ Matches the Stripe tier: type, code, message, param, doc_url, so the developer and any log parser know exactly what failed\n ✅ Reuses the redaction and help-link patterns the other errors already have\n ❌ Needs the console key-management URL and the error code documented in the API reference\nB) Better message text only: AuthError(\"Invalid API key: check EVALKIT_API_KEY or create a new key at \") (human: ~1 hour / CC: ~5 min)\n ✅ Tells the developer the cause and the fix in one line with minimal code change\n ✅ No new error-code registry to maintain in the beta\n ❌ No machine-readable code, so CI logs and support triage still grep for prose\nC) Keep AuthError(\"request failed\") as documented\n ✅ Zero change to the error contract in the beta\n ✅ Nothing new to document in docs/api.md\n ❌ The one error most developers will hit first is the one that tells them nothing\nNet: bringing the auth error up to the standard the SDK's other errors already meet versus leaving the first live-call failure unexplained.", + "header": "Auth error", + "multiSelect": false, + "options": [ + { + "label": "A) Structured code + cause + fix + link (recommended)", + "description": "EVALKIT_AUTH_INVALID_KEY with redacted key prefix, project, console URL for rotate/create, and a docs link." + }, + { + "label": "B) Better message text only", + "description": "One-line message naming the env var and console URL; no error code." + }, + { + "label": "C) Keep request failed", + "description": "Ship the auth error exactly as documented." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D6 — Journey stage DEBUG: an invalid API key raises AuthError(\"request failed\") with no code, cause, or fix. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; reviewing the authentication error in docs/api.md lines 11-13.\nELI10: After the demo, the ML engineer creates a key in the console, exports EVALKIT_API_KEY, and runs their first live evaluation. If the key is mistyped, expired, revoked, or scoped to the wrong project, they see the two words request failed. That message is indistinguishable from a network outage or a server error. docs/current-contracts.md line 22 says every other error already names the cause and an actionable fix, so this is the one error that breaks the SDK's own standard, and it fires at the exact moment the developer goes from demo to real usage.\nStakes if we pick wrong: The developer cannot tell whether to fix their key, their network, or wait for the service. They open a support ticket or give up at the first live call.\nRecommendation: A because the SDK's other errors already follow the problem-cause-fix formula, so matching it here is consistency, not new design. Violates Fight uncertainty.\nCompleteness: A=10/10, B=7/10, C=1/10\nA) Structured auth error: code EVALKIT_AUTH_INVALID_KEY, cause (rejected key, redacted prefix, project), fix (create or rotate key at the console URL, re-export the variable), and a docs link (recommended) (human: ~half day / CC: ~15 min)\n ✅ Matches the Stripe tier: type, code, message, param, doc_url, so the developer and any log parser know exactly what failed\n ✅ Reuses the redaction and help-link patterns the other errors already have\n ❌ Needs the console key-management URL and the error code documented in the API reference\nB) Better message text only: AuthError(\"Invalid API key: check EVALKIT_API_KEY or create a new key at \") (human: ~1 hour / CC: ~5 min)\n ✅ Tells the developer the cause and the fix in one line with minimal code change\n ✅ No new error-code registry to maintain in the beta\n ❌ No machine-readable code, so CI logs and support triage still grep for prose\nC) Keep AuthError(\"request failed\") as documented\n ✅ Zero change to the error contract in the beta\n ✅ Nothing new to document in docs/api.md\n ❌ The one error most developers will hit first is the one that tells them nothing\nNet: bringing the auth error up to the standard the SDK's other errors already meet versus leaving the first live-call failure unexplained.": "A) Structured code + cause + fix + link (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:18:51.429Z" + }, + { + "sessionId": "5139711c-562b-47db-81ea-387b171beaed", + "toolUseId": "toolu_01J1oQKw9i4Ye2v7KEcFfGDg", + "questions": [ + { + "question": "D7 — Journey stage UPGRADE: v2 removes Client.evaluate() immediately in favor of Client.run(), with no alias, warning, migration guide, or codemod. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; reviewing the v1-to-v2 client change in docs/api.md lines 15-18.\nELI10: Every v1 user who pins to 2.0.0b1 gets AttributeError: 'Client' object has no attribute 'evaluate' on the first call, with nothing pointing them at run(). docs/api.md says the changelog is otherwise complete, so this is the one breaking change with no path across. Upgrades should be boring. Right now the ML engineer's production CI job fails at import time with a stack trace and a guess.\nStakes if we pick wrong: v1 users hold at v1, or their CI breaks on upgrade and the first thing they learn about 2.0 is that it broke them without warning.\nRecommendation: A because a one-method alias with a DeprecationWarning is a few lines, the warning text doubles as the migration guide, and the alias can be removed at 3.0 on a schedule. Violates Upgrade fear and Credible.\nCompleteness: A=10/10, B=7/10, C=4/10, D=1/10\nA) Keep Client.evaluate() as a deprecated alias for run() through 2.x with a DeprecationWarning naming the replacement and removal version, plus a migration section in the changelog and docs (recommended) (human: ~half day / CC: ~15 min)\n ✅ v1 code keeps working on upgrade; the warning tells the developer exactly what to change and by when\n ✅ Cheap: one forwarding method, one warning string, one changelog section\n ❌ Carries the old name through the 2.x line, which the removal schedule has to enforce\nB) Deprecated alias plus a codemod that rewrites .evaluate( to .run( (human: ~2 days / CC: ~30 min)\n ✅ Next.js-style one-command upgrade; large codebases migrate in seconds\n ✅ The alias still protects anyone who does not run the codemod\n ❌ A codemod is a new tool to test and support for a single-method rename; oversized for the beta\nC) No alias, but a migration guide and a clear AttributeError hint pointing at run() (human: ~2 hours / CC: ~10 min)\n ✅ Keeps the v2 surface clean with no legacy name\n ✅ The hint turns a bare AttributeError into an actionable one-line fix\n ❌ v1 code still breaks on upgrade; only the error message improves\nD) Remove evaluate() immediately as documented\n ✅ No compatibility code to maintain\n ✅ Nothing new to write in the beta\n ❌ Every v1 user breaks with no warning and no pointer to the new name\nNet: a few lines of alias and warning text versus a hard break that teaches v1 users not to trust EvalKit upgrades.", + "header": "v1 to v2", + "multiSelect": false, + "options": [ + { + "label": "A) Deprecated alias + warning + guide (recommended)", + "description": "evaluate() forwards to run() with a DeprecationWarning through 2.x; changelog gets a migration section." + }, + { + "label": "B) Alias + codemod", + "description": "Same as A plus a script that rewrites .evaluate( to .run( across a codebase." + }, + { + "label": "C) Guide + AttributeError hint, no alias", + "description": "Remove the method but make the error and changelog point at run()." + }, + { + "label": "D) Remove immediately", + "description": "Ship the hard removal exactly as documented." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D7 — Journey stage UPGRADE: v2 removes Client.evaluate() immediately in favor of Client.run(), with no alias, warning, migration guide, or codemod. Fix it in the plan?\nProject/branch/task: EvalKit SDK beta polish on main; reviewing the v1-to-v2 client change in docs/api.md lines 15-18.\nELI10: Every v1 user who pins to 2.0.0b1 gets AttributeError: 'Client' object has no attribute 'evaluate' on the first call, with nothing pointing them at run(). docs/api.md says the changelog is otherwise complete, so this is the one breaking change with no path across. Upgrades should be boring. Right now the ML engineer's production CI job fails at import time with a stack trace and a guess.\nStakes if we pick wrong: v1 users hold at v1, or their CI breaks on upgrade and the first thing they learn about 2.0 is that it broke them without warning.\nRecommendation: A because a one-method alias with a DeprecationWarning is a few lines, the warning text doubles as the migration guide, and the alias can be removed at 3.0 on a schedule. Violates Upgrade fear and Credible.\nCompleteness: A=10/10, B=7/10, C=4/10, D=1/10\nA) Keep Client.evaluate() as a deprecated alias for run() through 2.x with a DeprecationWarning naming the replacement and removal version, plus a migration section in the changelog and docs (recommended) (human: ~half day / CC: ~15 min)\n ✅ v1 code keeps working on upgrade; the warning tells the developer exactly what to change and by when\n ✅ Cheap: one forwarding method, one warning string, one changelog section\n ❌ Carries the old name through the 2.x line, which the removal schedule has to enforce\nB) Deprecated alias plus a codemod that rewrites .evaluate( to .run( (human: ~2 days / CC: ~30 min)\n ✅ Next.js-style one-command upgrade; large codebases migrate in seconds\n ✅ The alias still protects anyone who does not run the codemod\n ❌ A codemod is a new tool to test and support for a single-method rename; oversized for the beta\nC) No alias, but a migration guide and a clear AttributeError hint pointing at run() (human: ~2 hours / CC: ~10 min)\n ✅ Keeps the v2 surface clean with no legacy name\n ✅ The hint turns a bare AttributeError into an actionable one-line fix\n ❌ v1 code still breaks on upgrade; only the error message improves\nD) Remove evaluate() immediately as documented\n ✅ No compatibility code to maintain\n ✅ Nothing new to write in the beta\n ❌ Every v1 user breaks with no warning and no pointer to the new name\nNet: a few lines of alias and warning text versus a hard break that teaches v1 users not to trust EvalKit upgrades.": "A) Deprecated alias + warning + guide (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-09T20:19:15.558Z" + } + ], + "historicalOutcome": "ceiling_reached", + "legacyReviewCount": 8, + "genuineDecisions": 8, + "observationSha256": "8412dec128155b28ce10dc0c9aaae82069bf3dfd5d22e3afabf70d1ba39da2ff" + } + ] +} diff --git a/test/fixtures/sdk-stale-table-ad-v3.json b/test/fixtures/sdk-stale-table-ad-v3.json new file mode 100644 index 000000000..f81567295 --- /dev/null +++ b/test/fixtures/sdk-stale-table-ad-v3.json @@ -0,0 +1,12 @@ +{ + "sourceHead": "4636893f5201e9357f9af2dd3cbbfb679e57bfdc", + "provenance": { + "sourcePublicReport": "/home/vercel-sandbox/gstack/.context/ship-source-ad-full-paid-20260909-v3/sdk-first-attempt-public-evidence-v1/reviewed-plan.md", + "sourcePublicReportSha256": "51627b8939fdd7cfb2bdb4a834dd53dd078a8b6f83423656875272636d03ad5a", + "sessionId": "24b6be83-9595-4562-abcd-0051268a9ef8", + "actualOutcome": "hasStaleFillRaceFinding assertion failed after successful completion", + "noRetroactivePass": true + }, + "report": "# Plan: cache profile summaries in one process\n\n## Measured problem and accepted scope\nThe existing profile-summary service has one active process. A one-week trace\nshows repeated reads of about 900 hot keys: DB CPU is 70%, with read p95 120 ms.\nAdd a process-local LRU wrapper to the existing repository. Acceptance targets\nare at least 60% cache hits, DB CPU below 50%, and read p95 below 60 ms, with the\nexisting error-rate and correctness SLOs unchanged. This is an internal backend\nchange with no UI, API, schema, pricing, or developer onboarding change.\n\n## Existing contracts retained\n- All reads and writes use this repository in the same process; there are no\n external DB writers. Multi-process operation remains unsupported and startup\n rejects that configuration while caching is enabled.\n- Authentication and authorization run before repository access. Keys encode\n the authenticated tenant ID and validated profile ID without ambiguity.\n Values are immutable profile-summary DTOs; secrets and cache keys are never\n logged. Cached results cannot bypass authorization.\n- The existing LRU adapter supports 1000 entries, a 16 MiB byte cap, and a\n 30-second TTL. Recorded hot data fits those limits. Absent records use a\n distinct sentinel with a 10-second TTL; undefined means a cache miss.\n- Cache operations are synchronous and atomic in the single JS event loop.\n On any cache failure the existing adapter bypasses the cache until an empty\n cache is reinitialized; repository errors keep the current typed API error\n mapping. The existing per-key\n single-flight wrapper coalesces simultaneous misses and releases on failure.\n- A read already in progress when a write commits may return its earlier DB\n snapshot to that caller. Every read begun after that write completes must\n observe the committed version. TTL expiry is not a substitute for this rule.\n (\"Completes\" = `writeProfile` returns to its caller. This is the invariant\n every finding below is measured against.)\n\n## Proposed wrapper integration\nKeep the current read-through repository interface and shared adapters. These\nare the complete new read/write ordering rules; no additional version checks or\ncoordination between a cache fill and a write are proposed\n*(superseded by amendment D1 below; original sketch kept for the record)*:\n\n```javascript\nasync function readProfile(key) {\n const cached = cache.get(key);\n if (cached !== undefined) return cached;\n const value = await repository.read(key);\n cache.set(key, value);\n return value;\n}\n\nasync function writeProfile(key, update) {\n const saved = await repository.write(key, update);\n cache.delete(key);\n return saved;\n}\n```\n\n## Verification and rollout\nExisting repository contract tests cover tenant isolation, key validation,\nabsence, DB failures, and authorization. New wrapper tests cover hit/miss,\neviction and byte limits, TTL, adapter-failure fallback, successful-write\ninvalidation, failed-write preservation *(superseded by D2: failed-write\ninvalidation)*, and concurrent-miss coalescing.\nThe rollout uses the existing runtime feature flag: enable for 10% of keys,\nthen 50%, then all keys after one healthy hour at each stage. Monitor hit/miss,\neviction, cache bytes, fallback errors, DB CPU, and read p95 without raw IDs.\nOn error-rate or latency regression, disable the flag immediately; both reads\nand writes bypass the cache while disabled *(amended by D6: reads consult the\nflag, writes always invalidate while a cache instance exists)*, and enabling\ncreates an empty cache.\nCold starts remain within the existing DB capacity. The service owner monitors\nthe rollout and records the results against the acceptance targets.\n\n## Out of scope\nDistributed caching, cross-process coherence, prewarming, changing consistency\nsemantics, or adding new product surfaces. The repository interface preserves a\nfuture replacement path without introducing a general cache framework now.\n\n---\n\n## Accepted plan amendments (CEO review, HOLD SCOPE, 2026-09-09)\n\nScope is unchanged. Every amendment repairs a gap between the sketch and a\nstated invariant or acceptance target. Decision IDs (D0\u2013D7) are defined in the\nDecision Record below; this section is the resulting plan text.\n\n### Amended wrapper (replaces the sketch above)\n\n```javascript\n// Ordering rule: a write invalidates the cache entry AND any in-flight fetch\n// for the key, on every outcome, regardless of per-key flag state. A fetch\n// only fills the cache if no write invalidated it while it was in flight.\n//\n// R1 miss \u2500\u2500\u25b6 DB read (v1) \u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u25b6 resume: flight.invalidated? skip fill\n// W \u2500\u2500\u25b6 DB write (v2) \u2500\u2500\u25b6 finally: cache.delete + inflight.invalidate\n// R2 (after W returns) \u2500\u2500\u25b6 miss \u2500\u2500\u25b6 fresh flight \u2500\u2500\u25b6 v2 \u2713\n\nasync function readProfile(key) {\n if (!flag.readsEnabledFor(key)) return repository.read(key); // D6: flag lookup failure \u21d2 false\n const cached = cache.get(key); // D3: null = cached ABSENT; undefined = miss\n if (cached !== undefined) return cached;\n const flight = inflight.getOrStart(key, () => repository.read(key));\n const value = await flight.promise; // typed repository errors propagate unchanged\n if (!flight.invalidated) cache.set(key, value); // D1 guard; D3: null \u21d2 ABSENT sentinel, 10 s TTL\n return value;\n}\n\nasync function writeProfile(key, update) {\n try {\n return await repository.write(key, update);\n } finally {\n cache.delete(key); // D2: on success AND failure\n inflight.invalidate(key); // D1: mark current flight stale, detach it\n }\n}\n```\n\nSingle-flight contract additions (D1): `invalidate(key)` sets\n`flight.invalidated = true` and removes it from the map so the next miss starts\na fresh fetch; settle-time release removes the map entry only if it still\npoints at the same flight object. No unbounded per-key state: the marker lives\non the existing in-flight entry.\n\n### Amended contracts\n- **D2** Write failure of any class (validation, conflict, timeout, connection\n reset) invalidates the key. Cost: one extra DB read after a failed write.\n Test \"failed-write preservation\" becomes \"failed-write invalidation\".\n- **D3** `repository.read` returns `null` for an absent record, never\n `undefined`. `cache.set(key, null)` stores the ABSENT sentinel with the 10 s\n TTL; `cache.get` translates the sentinel back to `null`. `undefined` remains\n the only miss signal. Absent-then-create is covered by the write invalidation.\n- **D4** Adapter behavior is classified: an oversized value (> byte cap) or a\n full cache is a *skip* (`set` is a no-op, counter increments, cache stays\n ACTIVE). Only a thrown adapter error (corrupt internal state, size accounting\n failure) flips the adapter to BYPASSED. Adapter methods never throw to the\n wrapper. BYPASSED persists until the flag is toggled off then on.\n- **D5** Observability is a deliverable: metrics `profile_cache_state` (gauge\n 0 = disabled, 1 = active, 2 = bypassed), `profile_cache_hits_total`,\n `_misses_total`, `_evictions_total`, `_bytes`, `_fill_skipped_stale_total`,\n `_set_skipped_oversize_total`, `_fallback_errors_total`, plus existing DB CPU\n and read p95. One structured log line on ACTIVE\u2192BYPASSED with error class,\n flag stage, and entry count (no keys). Alerts: state = BYPASSED for 5 min;\n hit ratio < 40% for 15 min at the 100% stage; fallback errors > 0 in 5 min.\n Dashboard panels: hit ratio, state, evictions, bytes, DB CPU, read p95.\n Runbook: BYPASSED \u2192 toggle flag off, confirm state 0, toggle on, confirm\n state 1 and hit ratio recovering within 2 min; regression \u2192 flag off.\n \"Healthy hour\" = no alert fired, error rate and p95 within SLO.\n- **D6** Flag semantics: per-key selection uses a stable hash of the cache key.\n Only reads consult the flag. Writes always run the `finally` invalidation\n while a cache instance exists. A failed flag lookup evaluates to \"reads\n disabled\". Fully disabling the flag drops the cache instance; enabling\n creates an empty one.\n- **D7** The ordering diagram above ships as a code comment on the wrapper and\n the single-flight `invalidate` method, plus a short section in the\n repository README naming the invariant and the four ordering rules.\n\n### Amended verification\nExisting contract tests unchanged. Wrapper tests: hit/miss, eviction and byte\nlimits, TTL (injected clock, no wall-clock sleeps), adapter-failure bypass,\noversize skip keeps ACTIVE (D4), absent \u2192 null cached 10 s then create\ninvalidates (D3), successful-write invalidation, failed-write invalidation (D2),\nconcurrent-miss coalescing, and controlled pause/release ordering tests for the\ntwo schedules in Section 4 (D1). Partial-rollout test: key cached at 50%,\ndropped to 10%, written, restored to 50% \u21d2 next read misses (D6). Metric\nassertions: each counter increments on its path (D5).\n\n---\n\n## CEO Review Decision Record\n\n### Step 0 \u2014 Premise, leverage, dream state, alternatives, mode\n- **Premise (0A):** Real, measured pain (70% DB CPU, p95 120 ms, 900 hot keys\n over one week). Doing nothing leaves DB headroom shrinking. A local\n read-through cache is the most direct path; targets are stated and checkable.\n Not a proxy problem.\n- **Existing leverage (0B):** LRU adapter (limits, TTL, sentinel, bypass),\n per-key single-flight, runtime feature flag, typed repository errors,\n repository contract tests. The plan reuses all of them and builds no parallel\n flow. Nothing is rebuilt.\n- **Dream state (0C):**\n ```\n CURRENT STATE THIS PLAN 12-MONTH IDEAL\n 1 process, no cache ---> local read-through cache ---> same interface, cache\n DB CPU 70%, p95 120ms behind the repository, backend swappable if\n flag-gated, invariant-safe multi-process ever lands\n ```\n Moves toward the ideal: the repository interface stays the seam.\n- **Alternatives (0C-bis):**\n ```\n APPROACH A: Sketch as written (delete-after-write) Effort S Risk Med\n Pros: smallest diff; reuses everything\n Cons: stale fill after concurrent write violates the read-after-write\n invariant (Section 4); no coverage of failed writes or partial rollout\n APPROACH B: Sketch + in-flight invalidation marker Effort S Risk Low (recommended)\n Pros: meets the invariant; state lives on existing single-flight entry;\n ~20 LOC delta; fully testable with pause/release\n Cons: single-flight contract grows one method; one more test family\n APPROACH C: Versioned rows + compare-on-fill Effort L Risk Med\n Pros: general coherence primitive\n Cons: needs a schema version column, which the accepted scope excludes\n ```\n **D0 (auto-accepted, recommended): Approach B.** Completeness A=5/10,\n B=10/10, C=10/10 but out of scope. Mapped to \"handle more edge cases\" and\n \"right-sized diff\".\n- **Mode (0F):** HOLD SCOPE, preselected by the user. No expansions surfaced.\n- **HOLD analysis (0D):** Touches ~4 files (wrapper, single-flight, adapter\n contract, tests) plus metrics/alert config. No complexity smell. Minimum set\n = the amended wrapper; every amendment is required by a stated invariant or\n target, none is deferrable.\n- **Temporal interrogation (0E):** Implementation decisions resolved now:\n invalidation across in-flight fetches (D1), write-failure semantics (D2),\n absent representation (D3), adapter failure vs skip (D4), alert thresholds\n and \"healthy\" definition (D5), flag semantics and hash (D6), where the\n ordering rules are documented (D7). Effort: human ~1.5 days / CC ~45 min.\n- Skipped per run rules: system audit, environment setup, landscape web\n search, brain context, prior learnings, telemetry.\n\n### Findings (each recorded once)\n\n| ID | Section | Evidence (plan text) | Failure mode | Selected remedy | Residual risk | Verification |\n|----|---------|----------------------|--------------|-----------------|---------------|--------------|\n| D1 | 1, 4 | \"no additional version checks or coordination between a cache fill and a write\"; invariant \"every read begun after that write completes must observe the committed version\" | Read misses, awaits DB (v1); write commits v2 and returns; read resumes and fills v1; later reads hit stale v1 up to 30 s. Also a post-write miss joins the pre-write in-flight fetch and receives v1 | In-flight invalidation marker; fill only if not invalidated; post-write misses start a fresh fetch. Original reader still returns v1 to its own caller (allowed: it began before commit) | None against the invariant. Extra DB read per invalidated flight, counted | Pause/release tests for both schedules in Section 4; `fill_skipped_stale_total` increments |\n| D2 | 2 | Sketch deletes only after a successful write; test \"failed-write preservation\" | Write times out after the DB committed; cache keeps v1 up to 30 s; caller retry or re-read sees stale data | `finally` invalidation on every write outcome; rename test | One wasted DB read after a failed write | Test: write rejects \u21d2 next read misses and hits DB |\n| D3 | 5 | \"undefined means a cache miss\"; sketch `cache.set(key, value)` with unspecified absent value | If `repository.read` returns `undefined` for absent, absents are never cached and every missing-profile read hits the DB; if the sentinel leaks, callers receive an internal object | Pin: absent = `null`; adapter maps null \u21c4 sentinel; wrapper passes null through | None | Test: absent read hits DB once, then served null for 10 s; create invalidates |\n| D4 | 2 | \"On any cache failure the existing adapter bypasses the cache until an empty cache is reinitialized\" | One oversized value or a full cache classified as \"failure\" trips a sticky bypass; hit rate drops to 0 with no read errors | Classify skip vs failure; adapter never throws to wrapper; bypass only on thrown adapter error | Sticky bypass still requires operator toggle (alerted by D5) | Test: oversize set is a no-op, state stays ACTIVE, counter increments; thrown adapter error \u21d2 BYPASSED |\n| D5 | 8, 9 | \"Monitor hit/miss, eviction, cache bytes, fallback errors, DB CPU, and read p95\"; no alerts, gauge, runbook, or \"healthy\" definition | Sticky BYPASSED or hit-ratio collapse is visible only if someone is watching; rollout \"healthy hour\" is undefined | Named metrics, state gauge, three alerts, dashboard panels, runbook, healthy definition | Alert thresholds may need tuning after first week | Metric assertions in tests; alert rules reviewed before 10% stage |\n| D6 | 9, 4 | \"both reads and writes bypass the cache while disabled\"; \"enable for 10% of keys\" | Stage 50% \u2192 10% \u2192 50%: keys cached at first 50% stage are not deleted by writes during the 10% stage and revive stale when restored. Flag lookup failure behavior unspecified | Writes always invalidate while a cache instance exists; stable key hash; flag failure \u21d2 reads disabled | None against the invariant | Partial-rollout test in Amended verification |\n| D7 | 10 | Ordering rules exist only in this plan | New engineer in 12 months cannot see why `invalidate` exists; stale mental model recreates D1 | Code-comment diagram on wrapper and `invalidate`; README section | Diagram maintenance is part of future changes | Reviewer checks comment matches code in PR |\n\n### Section outcomes (11 of 11 evaluated)\n1. **Architecture:** 1 finding \u2192 D1. Diagram in Diagrams \u00a71. Coupling added: wrapper \u2194 single-flight `invalidate` (justified, in-process). Single point of failure: the one process, unchanged. 10x load: LRU at 1000 entries thrashes first (watch item, see Section 7). Rollback: flag off, seconds.\n2. **Error & rescue map:** 9 error paths mapped (registry below); 2 gaps \u2192 D2, D4. No catch-all handlers introduced; `finally` runs cleanup only and rethrows nothing new.\n3. **Security & threat model:** No issues found. No new endpoint, input, dependency, or secret. Keys are tenant-scoped after authz, so no cross-tenant hit or existence oracle. Negative cache is within an already-authorized tenant. DTOs in process memory match existing handling. Keys never logged (D5 log line carries none).\n4. **Data flow & interaction edge cases:** 2 unhandled orderings \u2192 D1 (schedule in Diagrams \u00a72b), 1 rollout ordering \u2192 D6. No UI interactions; interaction table not applicable.\n5. **Code quality:** 1 finding \u2192 D3. Naming, DRY, and structure fit the existing adapter/single-flight patterns. Branch count per function \u2264 3. No over-engineering: the marker reuses existing state.\n6. **Test review:** Diagram in Diagrams \u00a77. Gaps are the consequences of D1\u2013D6 (tests listed in Amended verification) plus test hygiene: TTL tests use an injected clock. No new behavioral choice needed. 2 a.m. Friday test: pause/release stale-fill schedule. Hostile QA: write during in-flight read at every stage of the flag. Chaos: force adapter throw mid-traffic and confirm BYPASSED alert and unchanged error rate. Pyramid: many unit, 1 integration (flag + wrapper + repository fake), 0 E2E. Load test: replay the one-week hot-key trace against a staging DB before 10%.\n7. **Performance:** No issues found. Cache ops O(1) sync. Memory bounded at 16 MiB. TTL floor \u2248 30 DB reads/s for 900 keys. Watch item (not a TODO, capacity growth is an expansion): 900 hot keys against 1000 entries leaves 10% headroom; if `evictions_total` climbs while hit ratio < 60%, the entry cap is the cause.\n8. **Observability:** 1 gap \u2192 D5.\n9. **Deployment & rollout:** 1 risk \u2192 D6; \"healthy\" definition folded into D5. No migration. Deploy order: code deploy with flag off \u2192 10% \u2192 50% \u2192 100%, one healthy hour each. Diagrams \u00a75, \u00a76.\n10. **Long-term trajectory:** Reversibility 5/5 (flag plus small wrapper). Debt: 1 item \u2192 D7. Path dependency: none; repository seam preserved for a future backend. 1-year question: answered by D7.\n11. **Design & UX:** SKIPPED, justified: plan states no UI, API, or product surface change.\n\n### Outside voice\nSkipped. `.gstack-section-state-QPMuNw/config.yaml` sets `codex_reviews: disabled`. Per the documented control, no challenge prompt was built, no outside CLI or Agent fallback was dispatched, and outside coverage is reported as **disabled**. Re-enable: `gstack-config set codex_reviews enabled`. The `outside_status: disabled` review-log record was not persisted: no shell tool is available in this run.\n\n### Required outputs\n\n**NOT in scope** (considered, deferred, with reason)\n- Distributed or cross-process cache: excluded by the accepted scope statement.\n- Versioned rows / compare-on-fill (Approach C): needs a schema change.\n- Prewarming and automatic recovery from BYPASSED: alternatives to adequate accepted remedies (D5 alert + runbook); HOLD SCOPE.\n- Raising the 1000-entry cap: hypothetical capacity; watch item only.\n\n**What already exists** \u2014 LRU adapter (limits, TTL, sentinel, bypass): reused, contract pinned by D3/D4. Single-flight wrapper: reused, gains `invalidate` (D1). Runtime feature flag: reused, semantics pinned by D6. Typed repository errors and contract tests: reused unchanged.\n\n**Dream state delta** \u2014 After this plan the service has a flag-gated, invariant-safe local cache behind the unchanged repository interface. The 12-month ideal (swappable backend if multi-process arrives) remains reachable through the same seam; nothing here forecloses it.\n\n**Error & Rescue Registry**\n\n| Method/codepath | What can go wrong | Error class | Rescued? | Action | User sees |\n|---|---|---|---|---|---|\n| readProfile \u2192 flag lookup | flag service unavailable | FlagLookupError | Y (D6) | treat as reads disabled, counter | nothing |\n| readProfile \u2192 cache.get | adapter internal error | AdapterError (internal) | Y (D4) | adapter \u2192 BYPASSED, log once, read continues to DB | nothing |\n| readProfile \u2192 repository.read | record absent | (null result) | Y (D3) | cache ABSENT 10 s, return null | existing not-found |\n| readProfile \u2192 repository.read | DB timeout / connection error | DbTimeoutError / DbConnectionError | Y (existing) | single-flight releases, no fill, typed API error | existing error mapping |\n| readProfile \u2192 repository.read | authorization failure | AuthorizationError | Y (existing, before repository) | never reaches cache | existing 403 |\n| readProfile \u2192 single-flight joiners | leader rejects | same error as leader | Y (existing) | all joiners receive leader's typed error | existing error mapping |\n| readProfile \u2192 cache.set | value > byte cap / cache full | (skip, no error) | Y (D4) | no-op, `set_skipped_oversize_total` | nothing |\n| readProfile \u2192 cache.set | adapter internal error | AdapterError (internal) | Y (D4) | \u2192 BYPASSED, log once, value still returned | nothing |\n| writeProfile \u2192 repository.write | validation / conflict / timeout | typed repository errors | Y (existing + D2) | rethrow unchanged; `finally` invalidates | existing error mapping |\n\n**Failure Modes Registry**\n\n| Codepath | Failure mode | Rescued? | Test? | User sees? | Logged? |\n|---|---|---|---|---|---|\n| read fill vs concurrent write | stale fill after write returned | Y (D1) | Y (pause/release) | fresh data | counter |\n| post-write miss joins pre-write flight | stale value to new reader | Y (D1) | Y | fresh data | counter |\n| write fails after DB commit | stale entry survives | Y (D2) | Y | fresh data on re-read | typed error log (existing) |\n| absent record returns undefined | negative cache never fills | Y (D3) | Y | not-found, one DB read per 10 s | hits/misses |\n| sentinel leaks to caller | internal object returned | Y (D3) | Y | null | n/a |\n| oversized value | sticky bypass | Y (D4) | Y | nothing | counter |\n| adapter throws | sticky bypass, hit rate 0 | Y (D4, D5) | Y | nothing; alert to operator | log line + gauge + alert |\n| stage 50\u219210\u219250 | stale revival | Y (D6) | Y | fresh data | n/a |\n| flag lookup fails | undefined read path | Y (D6) | Y | nothing (DB read) | counter |\n| LRU thrash at 10x keys | hit ratio < target | N/A (watch) | load test | slower reads, correct | evictions + alert |\n\nCRITICAL GAPS remaining: 0.\n\n**TODOS.md updates** \u2014 0 proposed. Every evidenced gap is repaired in scope. Candidates rejected as expansions under HOLD SCOPE: auto-recovery from BYPASSED, cap growth, prewarming. Step evaluated, not skipped.\n\n### Diagrams\n\n\u00a71 System architecture\n```\n caller \u2500\u2500\u25b6 authn/authz \u2500\u2500\u25b6 readProfile / writeProfile (wrapper)\n \u2502 \u2502 \u2502\n \u25bc \u25bc \u25bc\n flag.readsEnabledFor LRU adapter single-flight (+invalidate)\n \u2502 \u2502\n \u2514\u2500\u2500\u25b6 repository.read/write \u2500\u2500\u25b6 DB\n metrics/logs \u25c0\u2500\u2500 wrapper + adapter (D5)\n```\n\n\u00a72a Data flow with shadow paths (read)\n```\n key \u2500\u2500\u25b6 flag? \u2500\u2500\u25b6 cache.get \u2500\u2500\u25b6 single-flight \u2500\u2500\u25b6 repository.read \u2500\u2500\u25b6 fill? \u2500\u2500\u25b6 return\n \u2502 \u2502 \u2502 \u2502 \u2502 \u2502\n nil/bad lookup undefined join or start null (absent) invalidated \u2192 skip\n key: fails: = miss; leader error \u2192 \u2192 cache null oversize \u2192 skip\n rejected reads null = ABSENT joiners get it DB error \u2192 adapter throw \u2192\n by exist- disabled hit no fill typed error BYPASSED\n ing valid.\n```\n\n\u00a72b Async ordering schedule (invariant boundary: `writeProfile` returns)\n```\n t R1 (read) W (write) R2 (read) cache result\n 1 miss, start DB read v1 \u2014\n 2 DB commits v2 \u2014\n 3 finally: delete, \u2014\n invalidate(R1 flight); returns\n 4a resume: invalidated \u2192 \u2014 R1 gets v1 (allowed)\n skip fill, return v1\n 5a miss \u2192 fresh v2 R2 gets v2 \u2713\n flight \u2192 v2, fill\n 4b miss \u2192 NOT joined \u2014\n (flight detached)\n [without D1: 4a fills v1 \u2192 R2 hits v1 \u2717 up to 30 s; 4b joins R1 \u2192 R2 gets v1 \u2717]\n Alt order: R1 resumes at t2.5 before W's finally \u2192 fills v1 \u2192 t3 delete removes it \u2713\n```\n\n\u00a73 State machine (adapter state, per process)\n```\n DISABLED \u2500\u2500flag on\u2500\u2500\u25b6 ACTIVE \u2500\u2500adapter throws\u2500\u2500\u25b6 BYPASSED\n \u25b2 \u2502 \u2502\n \u2514\u2500\u2500\u2500\u2500flag off\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500flag off\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2518\n Invalid: BYPASSED \u2192 ACTIVE without passing DISABLED (prevented: only\n flag-off drops the instance; flag-on constructs a new empty one).\n Entry states: MISS \u2192 FILLED(v, 30 s) | ABSENT(10 s) \u2192 deleted by write / TTL / eviction.\n```\n\n\u00a74 Error flow\n```\n repository error \u2500\u2500\u25b6 single-flight releases \u2500\u2500\u25b6 no fill \u2500\u2500\u25b6 typed API error (unchanged)\n adapter throw \u2500\u2500\u25b6 BYPASSED + log + gauge=2 \u2500\u2500\u25b6 read continues to DB \u2500\u2500\u25b6 alert at 5 min\n write error \u2500\u2500\u25b6 finally: delete + invalidate \u2500\u2500\u25b6 rethrow typed error\n```\n\n\u00a75 Deployment sequence\n```\n deploy (flag off) \u2192 verify gauge=0, error rate baseline\n \u2192 10% keys, 1 healthy hour \u2192 50%, 1 healthy hour \u2192 100%\n each stage: confirm hit ratio rising, evictions, bytes < 16 MiB, no alerts,\n DB CPU and p95 trending toward targets; record against acceptance targets\n```\n\n\u00a76 Rollback flowchart\n```\n alert or regression? \u2500\u2500yes\u2500\u2500\u25b6 flag off (seconds) \u2500\u2500\u25b6 gauge=0? \u2500\u2500\u25b6 error rate/p95 recover?\n \u2502 no \u2502 no\n \u25bc \u25bc\n restart process unrelated: escalate\n BYPASSED without regression \u2500\u2500\u25b6 runbook: flag off \u2192 on \u2192 gauge=1, hit ratio recovers in 2 min\n```\n\n\u00a77 Test diagram (new things introduced)\n```\n NEW UX FLOWS: none\n NEW DATA FLOWS: read-through fill; write invalidation; absent negative cache\n NEW CODEPATHS: flag gate; hit; miss+start; miss+join; invalidated skip;\n oversize skip; BYPASSED; finally invalidation\n NEW ASYNC WORK: none (all in request path)\n NEW INTEGRATIONS: none (flag service already used)\n NEW ERROR/RESCUE PATHS: see Error & Rescue Registry (9 rows)\n Coverage: unit for every row; 1 integration (flag+wrapper+repository fake);\n pause/release ordering tests (D1); partial-rollout test (D6); metric assertions (D5)\n```\n\n**Stale diagram audit** \u2014 No existing ASCII diagrams in the plan or the files it names; D7 adds the first ones, and their maintenance is part of the change.\n\n## Implementation Tasks\nSynthesized from this review's findings. Each task derives from a specific\nfinding above. Run with Claude Code or Codex; checkbox as you ship.\n\n- [ ] **T1 (P1, human: ~4h / CC: ~15min)** \u2014 wrapper + single-flight \u2014 Add `inflight.invalidate(key)` and the fill guard\n - Surfaced by: Section 1/4 \u2014 D1 stale fill after concurrent write\n - Files: profile repository wrapper, single-flight helper, wrapper tests\n - Verify: pause/release tests for schedules 4a/4b/alt pass; `fill_skipped_stale_total` increments\n- [ ] **T2 (P1, human: ~1h / CC: ~5min)** \u2014 wrapper \u2014 Move invalidation into `finally` on writes\n - Surfaced by: Section 2 \u2014 D2 write failure after commit\n - Files: wrapper, wrapper tests (rename failed-write test)\n - Verify: rejected write \u21d2 next read misses and reads DB\n- [ ] **T3 (P1, human: ~2h / CC: ~10min)** \u2014 repository + adapter \u2014 Pin absent = null \u21c4 ABSENT sentinel\n - Surfaced by: Section 5 \u2014 D3 undefined/sentinel ambiguity\n - Files: repository read, LRU adapter get/set, tests\n - Verify: absent read hits DB once then null for 10 s; create invalidates\n- [ ] **T4 (P1, human: ~3h / CC: ~10min)** \u2014 LRU adapter \u2014 Classify oversize/full as skip; bypass only on thrown error; never throw to wrapper\n - Surfaced by: Section 2 \u2014 D4 sticky bypass on oversize\n - Files: LRU adapter, adapter tests\n - Verify: oversize set no-op, state ACTIVE, counter up; injected throw \u21d2 BYPASSED\n- [ ] **T5 (P1, human: ~4h / CC: ~20min)** \u2014 metrics/alerts/runbook \u2014 Emit named metrics, state gauge, log line; add 3 alerts, dashboard panels, runbook\n - Surfaced by: Section 8/9 \u2014 D5 no alerts or healthy definition\n - Files: wrapper, adapter, metrics config, alert rules, runbook doc\n - Verify: metric assertions in tests; alert rules load; dry-run BYPASSED alert in staging\n- [ ] **T6 (P1, human: ~2h / CC: ~10min)** \u2014 flag gate \u2014 Reads consult stable-hash flag; writes always invalidate; flag failure \u21d2 reads disabled\n - Surfaced by: Section 9/4 \u2014 D6 stale revival across stages\n - Files: wrapper, flag helper, integration test\n - Verify: 50\u219210\u219250 test misses; flag lookup failure test reads DB\n- [ ] **T7 (P2, human: ~1h / CC: ~5min)** \u2014 docs \u2014 Code-comment ordering diagram and README section\n - Surfaced by: Section 10 \u2014 D7 knowledge concentration\n - Files: wrapper, single-flight helper, repository README\n - Verify: PR reviewer confirms comment matches code\n- [ ] **T8 (P2, human: ~1h / CC: ~5min)** \u2014 tests \u2014 Injected clock for TTL and sentinel-TTL tests\n - Surfaced by: Section 6 \u2014 flakiness risk (time dependence)\n - Files: wrapper tests, adapter tests\n - Verify: tests pass with no wall-clock sleeps\n- _No new tasks from Section 3 (Security), Section 7 (Performance), Section 11 (skipped)._\n\nJSONL task artifact for `/autoplan`: **not written**. No shell tool is available in this run, so the `jq` writer could not execute. Re-run `/plan-ceo-review` interactively or write the eight rows above by hand to `~/.gstack/projects//tasks-ceo-review-.jsonl`.\n\n### Completion Summary\n```\n +====================================================================+\n | MEGA PLAN REVIEW \u2014 COMPLETION SUMMARY |\n +====================================================================+\n | Mode selected | HOLD SCOPE (preselected by user) |\n | System Audit | skipped per run rules (no shell, no repo scan)|\n | Step 0 | premise real; approach B (D0); HOLD; 0E done |\n | Section 1 (Arch) | 1 issue found (D1) |\n | Section 2 (Errors) | 9 error paths mapped, 2 GAPS (D2, D4) closed |\n | Section 3 (Security)| 0 issues found, 0 High severity |\n | Section 4 (Data/UX) | 3 edge cases mapped, 0 unhandled after D1/D6 |\n | Section 5 (Quality) | 1 issue found (D3) |\n | Section 6 (Tests) | Diagram produced, 0 open gaps (T1\u2013T6, T8) |\n | Section 7 (Perf) | 0 issues found (1 watch item) |\n | Section 8 (Observ) | 1 gap found (D5) |\n | Section 9 (Deploy) | 1 risk flagged (D6) |\n | Section 10 (Future) | Reversibility: 5/5, debt items: 1 (D7) |\n | Section 11 (Design) | SKIPPED (no UI scope) |\n +--------------------------------------------------------------------+\n | NOT in scope | written (4 items) |\n | What already exists | written |\n | Dream state delta | written |\n | Error/rescue registry| 9 rows, 0 CRITICAL GAPS |\n | Failure modes | 10 total, 0 CRITICAL GAPS |\n | TODOS.md updates | 0 items proposed |\n | Scope proposals | 0 proposed, 0 accepted (HOLD) |\n | CEO plan | skipped (HOLD) |\n | Outside voice | skipped (codex_reviews disabled) |\n | Lake Score | 8/8 recommendations chose complete option |\n | Diagrams produced | 8 (arch, data flow, async schedule, state, |\n | | error, deploy, rollback, test) |\n | Stale diagrams found | 0 |\n | Unresolved decisions | 0 |\n +====================================================================+\n```\n\nAuto-chosen decisions (automated run, no human present): D0\u2013D7 each took the\nrecommended option; none was destructive. Next-step recommendation: run\n`/plan-eng-review` (required shipping gate); no design review needed.\nNot executed in this run (no shell tool): `gstack-review-log`,\n`gstack-decision-log`, `gstack-review-read`, JSONL task write, telemetry,\nlearnings log. No durable learnings this session beyond the findings above.\n\n## GSTACK REVIEW REPORT\n\n| Review | Trigger | Why | Runs | Status | Findings |\n|--------|---------|-----|------|--------|----------|\n| CEO Review | `/plan-ceo-review` | Scope & strategy | 1 | clean | mode: HOLD_SCOPE, 0 critical gaps (7 findings D1\u2013D7, all remedied in plan) |\n| Outside Review | codex (`codex_reviews: disabled`) | Independent 2nd opinion | 0 | disabled | none; step skipped by config, no native fallback |\n| Eng Review | `/plan-eng-review` | Architecture & tests (required) | 0 | \u2014 | \u2014 |\n| Design Review | `/plan-design-review` | UI/UX gaps | 0 | \u2014 | \u2014 |\n| DX Review | `/plan-devex-review` | Developer experience gaps | 0 | \u2014 | \u2014 |\n\n- **OUTSIDE COVERAGE:** provider codex, phase plan-review, status disabled by `codex_reviews: disabled`, 0 findings. Review-log record not persisted (no shell in this run).\n- **VERDICT:** CEO CLEARED \u2014 eng review required\n\nNO UNRESOLVED DECISIONS\n", + "table": "| ID | Section | Evidence (plan text) | Failure mode | Selected remedy | Residual risk | Verification |\n|----|---------|----------------------|--------------|-----------------|---------------|--------------|\n| D1 | 1, 4 | \"no additional version checks or coordination between a cache fill and a write\"; invariant \"every read begun after that write completes must observe the committed version\" | Read misses, awaits DB (v1); write commits v2 and returns; read resumes and fills v1; later reads hit stale v1 up to 30 s. Also a post-write miss joins the pre-write in-flight fetch and receives v1 | In-flight invalidation marker; fill only if not invalidated; post-write misses start a fresh fetch. Original reader still returns v1 to its own caller (allowed: it began before commit) | None against the invariant. Extra DB read per invalidated flight, counted | Pause/release tests for both schedules in Section 4; `fill_skipped_stale_total` increments |" +} diff --git a/test/helpers/autoplan-artifact-permission.ts b/test/helpers/autoplan-artifact-permission.ts new file mode 100644 index 000000000..92e1c8ba6 --- /dev/null +++ b/test/helpers/autoplan-artifact-permission.ts @@ -0,0 +1,114 @@ +/** One-time input for a cropped native Edit of an already-owned review artifact. */ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import type { NativePublicToolEvent } from './plan-count-transcript'; + +interface ArtifactPermissionContext { + cwd: string; + /** Set only by the launcher that created HOME/.gstack, never from ambient env. */ + ownedStateRoot?: string; + commandStartedAt: number; + now?: number; + transcriptStatus: string; + publicTools: NativePublicToolEvent[]; +} + +const MAX_BYTES = 1024 * 1024; +const compact = (text: string) => text.replace(/\s/g, ''); + +function ownedPlan(file: string, context: ArtifactPermissionContext): boolean { + if (!context.ownedStateRoot || !path.isAbsolute(file) || path.resolve(file) !== file) return false; + const project = path.join(context.ownedStateRoot, 'projects', path.basename(context.cwd)); + const relative = path.relative(project, file).split(path.sep).join('/'); + // Current CEO plan and both explicit Eng test-plan layouts. Design/DX amend + // ACTIVE_PLAN; they have no separate state-root plan directory. Do not admit + // restore points, methodology snapshots, task logs, config, or mockups. + if (!/^ceo-plans\/\d{4}-\d{2}-\d{2}-[a-z0-9][a-z0-9-]*\.md$/.test(relative) && + !/^[a-zA-Z0-9][a-zA-Z0-9_.-]*-test-plan-\d{8}-\d{6}\.md$/.test(relative)) return false; + try { + // System temp parents may be aliases (/var -> /private/var on macOS). + // Canonicalize above the owned root; no symlink at or below it is admitted. + const expected = path.join(fs.realpathSync(context.ownedStateRoot), path.relative(context.ownedStateRoot, file)); + return fs.lstatSync(context.ownedStateRoot).isDirectory() && + fs.realpathSync(file) === expected && fs.lstatSync(file).isFile() && fs.statSync(file).size <= MAX_BYTES; + } catch { return false; } +} + +/** Require the whole current cropped diff, exact menu, and requested edit text. */ +function matchesCroppedEdit(viewport: string, file: string, before: string, removed: string, after: string): boolean { + if (viewport.length > MAX_BYTES) return false; + const text = viewport.replace(/\r\n?/g, '\n'); + const menu = /^ {0,3}Do you want to make this edit to ([^\n?]+)\? *\n {0,3}❯ *1\. Yes *\n {0,3}2\. Yes, and switch to accept edits \(auto-approve file edits and common file commands\) for this session(?: \(shift\+tab\))? *\n {0,3}3\. No *\n\s*Esc to cancel [·•] Tab to amend\s*$/m.exec(text); + if (!menu || menu.index + menu[0].length !== text.length || menu[1] !== path.basename(file)) return false; + const rows = text.slice(0, menu.index).trimEnd().split('\n'); + if (!/^[╌─]{8,}$/.test(rows.pop() ?? '')) return false; + const chunks: Array<{ kind: string; text: string }> = []; + for (const row of rows) { + const numbered = /^ {0,3}(\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]! }); + } else { + const wrapped = /^ {4}([+ -])(.*)$/.exec(row); + const last = chunks.at(-1); + if (!wrapped || !last || wrapped[1] !== last.kind) return false; + last.text += wrapped[2]!; + } + } + // The crop itself cannot contain an example introduction, quote, unrelated + // prompt, or arbitrary diff: each row must occur in this exact pending edit. + const originals = before.split('\n').map(compact); + const deletions = removed.split('\n').map(compact); + const replacements = after.split('\n').map(compact); + return chunks.some(chunk => chunk.kind !== ' ' && compact(chunk.text)) && + chunks.every(chunk => (chunk.kind === '+' ? replacements : + chunk.kind === '-' ? deletions : originals).includes(compact(chunk.text))); +} + +export function autoplanArtifactPermissionInput( + viewport: string, context: ArtifactPermissionContext, seen: ReadonlySet, +): { input: '1\r'; signature: string; file: string } | null { + const now = context.now ?? Date.now(); + if (context.transcriptStatus !== 'ready' || !Number.isFinite(context.commandStartedAt) || + context.commandStartedAt > now || context.publicTools.length > 10_000 || + context.publicTools.some(event => !Number.isFinite(Date.parse(event.timestamp)))) return null; + const events = context.publicTools.filter(event => Date.parse(event.timestamp) >= context.commandStartedAt); + if (!events.length || events.some(event => !event.sessionId || !event.toolUseId || + !Number.isFinite(Date.parse(event.timestamp)) || Date.parse(event.timestamp) > now) || + new Set(events.map(event => event.sessionId)).size !== 1) return null; + // Bind the latest file mutation, which must be the sole unresolved Write/Edit. + // Claude may publish a queued Bash while its current Edit permission is open; + // that unrelated request supplies no file permission authority. + const edit = events.filter(event => event.kind === 'use' && + (event.name === 'Write' || event.name === 'Edit')).at(-1); + if (!edit || edit.kind !== 'use' || edit.name !== 'Edit' || typeof edit.input?.file_path !== 'string' || + typeof edit.input.old_string !== 'string' || !edit.input.old_string || + typeof edit.input.new_string !== 'string' || edit.input.new_string === edit.input.old_string || + (edit.input.replace_all !== undefined && edit.input.replace_all !== false)) return null; + const signature = `${edit.sessionId}:${edit.toolUseId}`; + if (seen.has(signature) || !ownedPlan(edit.input.file_path, context)) return null; + const uses = new Map(); + const results = new Map(); + let previousTime = context.commandStartedAt; + for (const event of events) { + const time = Date.parse(event.timestamp); + if (time < previousTime) return null; + previousTime = time; + const map = event.kind === 'use' ? uses : results; + if (map.has(event.toolUseId)) return null; + map.set(event.toolUseId, event); + if (event.kind === 'result' && !uses.has(event.toolUseId)) return null; + } + const writes = [...uses.values()].filter(event => event.name === 'Write' || event.name === 'Edit'); + if (results.has(edit.toolUseId) || writes.filter(event => !results.has(event.toolUseId)).length !== 1) return null; + if (!writes.some(event => event.toolUseId !== edit.toolUseId && + event.input?.file_path === edit.input!.file_path && results.has(event.toolUseId) && + results.get(event.toolUseId)!.isError === false)) return null; + try { + const before = fs.readFileSync(edit.input.file_path, 'utf8'); + if (!before.includes(edit.input.old_string) || + !matchesCroppedEdit(viewport, edit.input.file_path, before, edit.input.old_string, edit.input.new_string)) return null; + return { input: '1\r', signature, file: edit.input.file_path }; + } catch { return null; } +} diff --git a/test/helpers/ceo-section-loading-fixture.ts b/test/helpers/ceo-section-loading-fixture.ts index 69912ebc8..c62fb4830 100644 --- a/test/helpers/ceo-section-loading-fixture.ts +++ b/test/helpers/ceo-section-loading-fixture.ts @@ -210,7 +210,9 @@ function hasProseStaleFillFinding(report: string): boolean { (/\b(?:guard|serialize|serialise|coordinate|prevent|reject|skip)\w*\b/i.test(sentence) && /\b(?:cache|fill|refill|write|mutation|invalidation)\w*\b/i.test(sentence)) || /\bper[- ]key\s+(?:epoch|generation|version)\b/i.test(sentence)); - const claims = context.split(/(?:[.!?]\s+|\b(?:but|however|nevertheless|yet)\s*[:,]?\s+)/i); + // Table cells and semicolon-separated statements have separate owners; + // retain an explicit "that return" continuation with the return it names. + const claims = context.split(/(?:[.!?]\s+|;\s+(?!that\s+return\b)|\s+\|\s+|\b(?:but|however|nevertheless|yet)\s*[:,]?\s+)/i); const violation = claims.some(claim => /\b(?:violat\w*|break\w*)\b[^.!?]*\b(?:contract|guarantee|consistency|rule)\b/i.test(claim) && !/\b(?:not|no|never)\b/i.test(claim)); for (const [claimIndex, claim] of claims.entries()) { @@ -226,7 +228,8 @@ function hasProseStaleFillFinding(report: string): boolean { // The model declaration must accept the stale consequence itself. // A normative freshness requirement called an accepted model is not a // dismissal. Bare "This" can refer only to the preceding stale claim. - const modelDeclaration = /^(.+?)\s+(?:is|remains)\s+(?:(?:the|an?)\s+)?(?:accepted|expected|intentional|documented)\s+consistency\s+(?:model|contract|policy|semantics)\b/i.exec(allowanceText.trim()); + const modelDeclaration = /^(.+?)\s+(?:is|remains)\s+(?:(?:the|an?)\s+)?(?:accepted|expected|intentional|documented)\s+consistency\s+(?:model|contract|policy|semantics)\b/i.exec(allowanceText.trim()) + ?? /^(.+?)\s+(?:is|remains)\s+(?:accepted|expected|intentional|documented)[.!?]?$/i.exec(allowanceText.trim()); const subject = modelDeclaration?.[1] ?? ''; const previousClaim = claims[claimIndex - 1] ?? ''; const explicitStaleSubject = /^(?:this|the|an?)\s+(?:bounded\s+)?(?:inconsistency|staleness|stale[- ](?:read|fill)|stale\s+(?:read|fill|refill))(?:\s+(?:window|behavior|behaviour|race|consequence))?$/i.test(subject); @@ -235,7 +238,7 @@ function hasProseStaleFillFinding(report: string): boolean { && /\b(?:read|fetch|fill|refill|repopulat)\w*\b/i.test(previousClaim) && !/\b(?:must|shall|requires?|violat\w*|not|cannot|can't)\b/i.test(previousClaim); const acceptedStaleModel = Boolean(modelDeclaration) && (explicitStaleSubject || impliedStaleSubject); - const dismissal = /\b(?:not|isn't)\s+(?:a\s+|an\s+)?(?:gap|bug|defect|issue|violation|problem)\b|\bno\s+(?:gap|bug|defect|issue|violation|race)\b/i.test(claim) + const dismissal = /\b(?:not|isn't)\s+(?:a\s+|an\s+)?(?:(?:stale|late)[- ]fill\s+)?(?:gap|bug|defect|issue|violation|problem|race)\b|\bno\s+(?:(?:stale|late)[- ]fill\s+)?(?:gap|bug|defect|issue|violation|race)\b/i.test(claim) || /\b(?:accepted|expected|intentional|documented)\s+(?:invariant|behavior|trade[- ]off|stale[- ]read\s+window)\b|\b(?:allowed|permitted|acceptable)\b/i.test(allowanceText) || acceptedStaleModel || (!proposedPrevention && /\b(?:cannot|can't|never|does not|will not)\s+(?:\w+\s+){0,3}(?:refill|repopulate|populate|insert|store|cache|set|violate)\b/i.test(claim)) @@ -243,10 +246,16 @@ function hasProseStaleFillFinding(report: string): boolean { || /\bno\s+(?:fix|change|coordination|guard)\s+(?:is\s+)?(?:needed|required)\b/i.test(claim); if (!dismissal) continue; const originalCaller = /\b(?:original|already[- ]pending)\s+(?:pending\s+)?(?:caller|reader|request)\b|\bpending\s+caller\b/i.test(claim); + // A finding can name versions instead of calling them "old". Explicit + // start-before-commit and return-to-own-caller evidence scopes this + // allowance to that already-started call, never to cache/later readers. + const explicitlyEarlierCall = originalCaller && /\bto\s+its\s+own\s+caller\b/i.test(claim) + && !/\b(?:if|unless|whether|might|may|could)\b/i.test(claim) + && /\b(?:it|(?:the\s+)?(?:original\s+)?(?:read|request|call))\s+(?:began|started)\s+before\s+(?:(?:the|that)\s+)?(?:write\s+)?commit\b/i.test(claim); const onlyEarlierReturn = originalCaller && /\b(?:return|receiv|observ)\w*\b/i.test(claim) - && /\b(?:old|earlier|previous|pre[- ]write)\s+(?:snapshot|value|result|version)\b/i.test(claim) + && (/\b(?:old|earlier|previous|pre[- ]write)\s+(?:snapshot|value|result|version)\b/i.test(claim) || explicitlyEarlierCall) && !fillPattern.test(claim) && !/\b(?:next|later|subsequent|new|fresh|future)\s+(?:read\w*|request\w*|caller\w*)\b/i.test(claim); - if (!(onlyEarlierReturn && subsequentRead && (violation || remedy))) return false; + if (!(onlyEarlierReturn && subsequentRead && (violation || remedy || explicitlyEarlierCall))) return false; } return finding || subsequentRead || remedy; }); diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts index 62543cd4b..6ad44eb26 100644 --- a/test/helpers/claude-pty-runner.ts +++ b/test/helpers/claude-pty-runner.ts @@ -162,6 +162,8 @@ export interface ClaudePtySession { * the dir name ends in `/.claude` by contract). */ hermeticConfigDir: string | null; + /** Owned HOME/.gstack created by the seeded launcher; absent for caller overrides. */ + hermeticSkillStateRoot?: string; /** Owned pre-tool identity record, removed by close(). */ pendingPlanReadyFile?: string; pendingQuestionFile?: string; @@ -2534,11 +2536,13 @@ export async function launchClaudePty( // Hermetic by default (test/helpers/hermetic-env.ts): operator session // context never reaches the child; per-test opts.env merges last. let childEnv = hermeticChildEnv(opts.env); + let hermeticSkillStateRoot: string | undefined; if (opts.seedSkills && hermetic && !opts.env?.CLAUDE_CONFIG_DIR) { childEnv.CLAUDE_CONFIG_DIR = hermeticSkillsConfigDir(); if (opts.env?.HOME === undefined) { const runtime = withHermeticSkillRuntime(childEnv); childEnv = runtime.env; + hermeticSkillStateRoot = runtime.stateRoot; // Installed sections and generated ~/.gstack snapshots are owned inputs // outside the fixture cwd. Grant only these two runtime directories; // explicit GSTACK_HOME paths and operator permission settings stay separate. @@ -2762,6 +2766,7 @@ export async function launchClaudePty( exited: () => exited, exitCode: () => exitCodeCaptured, hermeticConfigDir: hermetic ? childEnv.CLAUDE_CONFIG_DIR ?? null : null, + hermeticSkillStateRoot, pendingPlanReadyFile: pendingExit?.file, pendingQuestionFile: pendingQuestion?.file, pendingFilePermissionFiles: pendingFiles.map(({ expected, recorder }) => ({ expected, file: recorder.file })), diff --git a/test/helpers/devex-seed-coverage.ts b/test/helpers/devex-seed-coverage.ts new file mode 100644 index 000000000..0cce58b24 --- /dev/null +++ b/test/helpers/devex-seed-coverage.ts @@ -0,0 +1,77 @@ +import type { NativePlanQuestion, PlanCountTranscript } from './plan-count-transcript'; + +export const DEVEX_SEEDED_GAPS = [ + 'local-ci-gate', 'missing-quickstart', 'reversed-arguments', 'opaque-auth-error', 'breaking-upgrade', +] as const; +export type DevexSeededGap = typeof DEVEX_SEEDED_GAPS[number]; + +/** Identify a dedicated seed decision by its subject and meaningful alternatives. */ +function decisionGaps(q: NativePlanQuestion): DevexSeededGap[] { + const title = q.question.split('\n')[0]!.trim().replace(/^D\s*\d+\s*[—–:-]\s*/i, '') + .replace(/`([^`\n]+)`/g, '$1'); + if (!title.endsWith('?') || (title.match(/\?/g)?.length ?? 0) !== 1 || + /^(?:>|"|“|Example\b|Quoted\b|Suppose\b)|\bhypothetical\b/i.test(title) || + /\b(?:continue|proceed|next section|move on|format|already (?:fixed|resolved))\b/i.test(title) || + /\b(?:have|did)\b[^?]*\bread\b|\b(?:narrative|trace|recap|summary)\b[^?]*\b(?:accurate|match|confirm)\b/i.test(title) || + /\b(?:report|summary|recap)\b[^?]*\b(?:mention|include|reference|list)\b|\b(?:mention|include|reference|list)\b[^?]*\b(?:report|summary|recap)\b/i.test(title)) return []; + const options = q.options.map(o => `${o.label} ${o.description ?? ''}`); + const labels = q.options.map(o => o.label.trim().replace(/\s*\(recommended\)$/i, '').toLowerCase()); + const yesNo = labels.length === 2 && labels.includes('yes') && labels.includes('no'); + // A terse Yes/No panel still resolves an action explicitly asked in the + // main question; action words in background prose never supply this arm. + const directAction = (verbs: string) => yesNo && new RegExp( + `^(?:should|shall|can|do|would) (?:we|I) (?:${verbs})\\b[^?]*\\?$`, 'i').test(title); + + const found: DevexSeededGap[] = []; + if (/\bCI\b/i.test(title) && /\b(?:local|demo|first)\b/i.test(title) && + /\b(?:gate|check|blocks?|waits?|bypass|mandatory|required)\b/i.test(title) && + (options.some(o => /\b(?:no CI gate|remove|move|skip|bypass|gate)\b/i.test(o) && /\b(?:CI|check|gate|local|demo)\b/i.test(o)) || directAction('remove|move|skip|bypass|gate'))) found.push('local-ci-gate'); + if (/\bquickstart\b|examples\/first_eval\.py/i.test(title) && + /\b(?:README|file|example|demo|missing|absent|package|wheel|ship|point)\b|first_eval\.py/i.test(title) && + (options.some(o => /\b(?:point|ship|add|demo is)\b/i.test(o) && /\bquickstart\b|first_eval\.py/i.test(o)) || directAction('point|ship|add|replace|fix'))) found.push('missing-quickstart'); + if (/\brun_eval\b/i.test(title) && /\brun_batch\b/i.test(title) && + /\b(?:arguments?|order|positional|reversed|opposite|consistent|align|unify|dataset|evaluator)\b/i.test(title) && + (options.some(o => /\b(?:align|unify|standardize|keyword|swap)\b/i.test(o) && /\b(?:order|dataset|arguments?|positional)\b/i.test(o)) || directAction('align|unify|standardize|enforce|make'))) found.push('reversed-arguments'); + if (/\bAuthError\b|\binvalid API key\b/i.test(title) && + /\b(?:error|message|code|cause|fix|guidance|opaque|explain)\b|request failed/i.test(title) && + (options.some(o => /\bcode\b/i.test(o) && /\b(?:cause|fix|link)\b/i.test(o)) || directAction('add|include|explain|replace|report|give'))) found.push('opaque-auth-error'); + if (/Client\.evaluate\b/i.test(title) && + /Client\.run\b|\b(?:v\d+|version \d+|alias|deprecation|migration)\b/i.test(title) && + /\b(?:alias|warning|compatibility|deprecat\w*|migration|remov\w*|rename|keep)\b/i.test(title) && + (options.some(o => /\balias\b/i.test(o) && /\b(?:warning|DeprecationWarning|migration)\b/i.test(o)) || directAction('keep|add|preserve|provide|retain'))) found.push('breaking-upgrade'); + return found; +} + +/** Extra real decisions are permitted; each seeded gap needs its own completed native call. */ +export function devexSeedCoverage(transcript: PlanCountTranscript) { + const decisions = Object.fromEntries(DEVEX_SEEDED_GAPS.map(gap => [gap, []])) as Record; + const batched: string[] = []; + const invalid: string[] = []; + const sessions = new Set(transcript.calls.map(c => c.sessionId)); + if (transcript.status !== 'ready' || sessions.size !== 1 || sessions.has('')) invalid.push('missing or mixed native session'); + const ids = new Set(); + for (const call of transcript.calls) { + const id = `${call.sessionId}:${call.toolUseId}`; + if (!call.toolUseId || ids.has(id)) { invalid.push(`missing or repeated native call: ${id}`); continue; } + ids.add(id); + const gaps = call.questions.flatMap(decisionGaps); + if (!gaps.length) continue; + if (call.questions.length !== 1 || gaps.length !== 1 || call.questions[0]!.multiSelect) { + batched.push(id); continue; + } + const q = call.questions[0]!; + const labels = q.options.map(o => o.label); + const complete = call.answered === true && call.failed === false && + Array.isArray(call.unansweredQuestionIndices) && call.unansweredQuestionIndices.length === 0 && + Number.isFinite(Date.parse(call.answeredAt ?? '')) && q.options.length >= 2 && q.options.length <= 4 && + new Set(labels).size === labels.length && labels.every(Boolean) && + Object.keys(call.answers ?? {}).length === 1 && labels.includes(call.answers?.[q.question] ?? ''); + if (complete) decisions[gaps[0]!]!.push(id); + } + const missing = DEVEX_SEEDED_GAPS.filter(gap => decisions[gap].length === 0); + const matchedIds = new Set(Object.values(decisions).flat()); + return { + complete: invalid.length === 0 && batched.length === 0 && missing.length === 0 && matchedIds.size >= DEVEX_SEEDED_GAPS.length, + missing, decisions, batched, invalid, + }; +} diff --git a/test/helpers/eng-seeded-coverage.ts b/test/helpers/eng-seeded-coverage.ts new file mode 100644 index 000000000..a9456f923 --- /dev/null +++ b/test/helpers/eng-seeded-coverage.ts @@ -0,0 +1,104 @@ +import type { NativePlanQuestionCall, PlanCountTranscript } from './plan-count-transcript'; + +/** Evidence for this fixture's four decision seeds; regression coverage is auto-added by the skill. */ +export const ENG_DECISION_SEEDS = ['complexity', 'shared-cache', 'swallowed-errors', 'sequential-idp'] as const; +type Seed = typeof ENG_DECISION_SEEDS[number]; + +// Ignore displayed examples/code, while retaining inline code identifiers. +function prose(text: string): string { + let fence: string | undefined; + return text.split('\n').filter(line => { + const mark = line.match(/^ {0,3}(`{3,}|~{3,})/); + if (mark) { + if (!fence) fence = mark[1]; + else if (mark[1][0] === fence[0] && mark[1].length >= fence.length) fence = undefined; + return false; + } + return !fence && !/^(?: {0,3}>| {4}|\t)/.test(line); + }).join('\n').replace(/[`*]/g, ''); +} + +function seedSubjects(call: NativePlanQuestionCall): Seed[] { + const q = call.questions[0]!; + // The actual issue subject, not cross-references in recommendations or other options, + // assigns credit. ELI10 can identify what a terse Promise.all title operates on. + const subject = q.question.split('\n').find(line => line.trim())?.trim() ?? ''; + if (/^(?:>|`{3}|~{3}|example\b|quote\b|["“])|\b(?:no (?:issue|defect)|already (?:fixed|resolved)|hypothetical)\b/i.test(subject)) return []; + const title = subject.replace(/[`*]/g, ''); + const offered = q.options.map(o => `${o.label} ${o.description ?? ''}`).join('\n'); + const directAction = title.match(/\b(?:should|shall|can|do|would)\s+(?:we|I)\s+([^?]+)\?\s*$/i)?.[1]; + const action = (re: RegExp) => re.test(offered) || Boolean(directAction && new RegExp(`^(?:${re.source})`, re.flags).test(directAction)); + const ids: Seed[] = []; + if (/\b(?:scope|complexity|classes|types|abstractions)\b/i.test(title) && + /\b(?:files|classes|types|abstractions)\b/i.test(title) && + action(/\b(?:reduce|cut|simplify|remove|collapse|merge|pure function)\b/i)) ids.push('complexity'); + if (/\b(?:AuthCache|cache)\b/i.test(title) && + /\b(?:global|module[ -]level|mutab\w*|both|shar\w*|ownership|writers)\b/i.test(title) && + action(/\b(?:inject\w*|DI|serializ\w*|single[ -]writer|ownership|composition root)\b/i)) ids.push('shared-cache'); + if (/\b(?:validateAndDispatch|catch\w*)\b/i.test(title) && + /\b(?:swallow\w*|nested|silent\w*|hidden|suppres\w*)\b/i.test(title) && + action(/\b(?:split|rethrow|typed|flatten|propagat\w*)\b/i)) ids.push('swallowed-errors'); + if (/\b(?:sequential|parallel\w*|Promise\.all(?:Settled)?)\b/i.test(title) && + /\b(?:IDP|identity provider)\b/i.test(prose(q.question)) && + action(/\b(?:parallel\w*|Promise\.all(?:Settled)?)\b/i)) ids.push('sequential-idp'); + return ids; +} + +function completedDecision(call: NativePlanQuestionCall, startedAt: number, finishedAt: number): boolean { + const answeredAt = Date.parse(call.answeredAt ?? ''); + if (!call.sessionId || !call.toolUseId || call.answered !== true || call.failed !== false || + !Number.isFinite(answeredAt) || answeredAt < startedAt || answeredAt > finishedAt || + call.questions.length !== 1 || !Array.isArray(call.unansweredQuestionIndices) || + call.unansweredQuestionIndices.length !== 0) return false; + const q = call.questions[0]!; + return !q.multiSelect && q.options.length >= 2 && q.options.length <= 4 && + q.options.every(o => o.label.trim()) && new Set(q.options.map(o => o.label)).size === q.options.length && + Object.keys(call.answers ?? {}).length === 1 && + q.options.some(o => call.answers?.[q.question] === o.label); +} + +function regressionEvidence(text: string): boolean { + return prose(text).split(/\n\s*\n|\n(?=\s*[-#])/).some(block => { + const task = block.trim().replace(/^[-+]\s+(?:\[[ xX]\]\s*)?/, '') + .replace(/^T\d+(?:\s*\([^\n)]*\))?\s*[—–:-]\s*/, ''); + const legacySubject = /^legacyAuthFlow(?:\(\))?\s*[—–:-]\s*/i; + const action = task.replace(legacySubject, ''); + const instruction = action.match(/^(?:(?:I|we)\s+)?(?:add(?:ed)?|record(?:ed)?|write|wrote|require(?:d)?|include(?:d)?)\s+((?:(?:a|the|new|required|legacyAuthFlow(?:\(\))?|regression|characterization|baseline|prior-behavior)\s+)*(?:tests?|fixtures?))\b([^.;\n]*)/i); + const explicitTarget = instruction && /^\s+(?:for|of|covering|characterizing)\b/i.test(instruction[2]!); + const legacyTarget = instruction && /^\s+(?:for|of|covering|characterizing)\s+(?:the\s+)?(?:prior behavior of\s+)?legacyAuthFlow\b/i.test(instruction[2]!); + const target = instruction && (!explicitTarget || legacyTarget) && + (legacySubject.test(task) || /\blegacyAuthFlow\b/.test(instruction[1]!) || legacyTarget); + // An affirmative task or completed addition, not an example, quotation, + // conditional proposal or an uncertain discussion of whether to add it. + return Boolean(target) && + /\b(?:regression|characterization)\b/i.test(instruction![0]) && + /\b(?:before|prior behavior|parity|compatibility|characterization)\b/i.test(instruction![0]) && + !/\b(?:no|not|never|skip\w*|defer\w*|maybe|might|could|if|unless|optional)\b/i.test(block); + }); +} + +export function evaluateEngSeedCoverage(transcript: PlanCountTranscript, plan: string, + startedAt: number, finishedAt: number) { + const decisions: Partial> = {}; + const problems: string[] = []; + const sessions = new Set(transcript.calls.map(c => c.sessionId)); + const identities = transcript.calls.map(c => `${c.sessionId}:${c.toolUseId}`); + const bound = transcript.status === 'ready' && sessions.size === 1 && !sessions.has('') && + identities.length === new Set(identities).size && Number.isFinite(startedAt) && + Number.isFinite(finishedAt) && startedAt <= finishedAt; + if (!bound) problems.push('missing, ambiguous or unbound native transcript'); + if (bound) for (const call of transcript.calls) { + if (!completedDecision(call, startedAt, finishedAt)) continue; + const seeds = seedSubjects(call); + // One combined approval cannot replace separate decisions for independent seeds. + if (seeds.length === 1) decisions[seeds[0]!] ??= `${call.sessionId}:${call.toolUseId}`; + } + const missing = ENG_DECISION_SEEDS.filter(seed => !decisions[seed]); + const regression = regressionEvidence(plan) ? 'plan' : bound && transcript.assistantMessages.some(m => + sessions.has(m.sessionId) && Date.parse(m.timestamp) >= startedAt && Date.parse(m.timestamp) <= finishedAt && + regressionEvidence(m.text)) ? 'public-narration' : undefined; + if (!regression) problems.push('mandatory legacy regression coverage absent'); + // The caller also retains the existing fresh owned-path/native completion and D19 checks. + if (!/^## GSTACK REVIEW REPORT[\t ]*\n\s*\S/m.test(prose(plan))) problems.push('final review report absent or empty'); + return { ok: bound && missing.length === 0 && problems.length === 0, decisions, missing, regression, problems }; +} diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 67416908e..aed526de1 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -148,22 +148,22 @@ export const E2E_TOUCHFILES: Record = { 'tpa-absent-darwin': ['scripts/resolvers/third-party-actions.ts', 'ship/SKILL.md.tmpl', 'ship/sections/apple-release.md.tmpl', 'scripts/gen-skill-docs.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-third-party-actions.test.ts', 'test/helpers/third-party-actions.ts'], 'tpa-apple-ban': ['scripts/resolvers/third-party-actions.ts', 'ship/SKILL.md.tmpl', 'ship/sections/apple-release.md.tmpl', 'scripts/gen-skill-docs.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-third-party-actions.test.ts', 'test/helpers/third-party-actions.ts'], 'ship-section-loading': ['ship/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/skill-e2e-ship-section-loading.test.ts'], - 'plan-ceo-section-loading': ['plan-ceo-review/**', 'test/skill-ceo-section-ordering.test.ts', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/helpers/ceo-section-loading-fixture.ts', 'test/ceo-section-loading-fixture.test.ts', 'test/skill-e2e-plan-ceo-review-section-loading.test.ts', 'test/fixtures/ceo-section-loading-l-report.md', 'test/fixtures/ceo-section-loading-q-report.md', 'test/fixtures/ceo-section-r-rejected-report.md', 'test/fixtures/ceo-section-s-trace-report.md', 'test/fixtures/ceo-section-u-report.md', 'test/fixtures/ceo-section-y-report.md', 'test/fixtures/ceo-section-aa-report.md'], + 'plan-ceo-section-loading': ['plan-ceo-review/**', 'test/skill-ceo-section-ordering.test.ts', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/helpers/ceo-section-loading-fixture.ts', 'test/ceo-section-loading-fixture.test.ts', 'test/skill-e2e-plan-ceo-review-section-loading.test.ts', 'test/fixtures/ceo-section-loading-l-report.md', 'test/fixtures/ceo-section-loading-q-report.md', 'test/fixtures/ceo-section-r-rejected-report.md', 'test/fixtures/ceo-section-s-trace-report.md', 'test/fixtures/ceo-section-u-report.md', 'test/fixtures/ceo-section-y-report.md', 'test/fixtures/ceo-section-aa-report.md', 'test/sdk-stale-table-ad-v3.test.ts', 'test/fixtures/sdk-stale-table-ad-v3.json'], // Data-driven behavioral guard for the 'plan'/'prompt' carves (eng, design, // devex, office-hours + future PR2 carves). One file iterating CARVE_GUARDS; // the selector sets GSTACK_CARVE_SKILL= to scope cost to the changed // skill (D-CODEX A). Touching the registry/helper or sections.ts runs all. 'carve-section-loading': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'design-html/**', 'design-shotgun/**', 'qa/**', 'browse/**', 'retro/**', 'autoplan/**', 'spec/**', 'setup-gbrain/**', 'review/**', 'codex/**', 'land-and-deploy/**', 'plan-eng-review/**', 'plan-design-review/**', 'plan-devex-review/**', 'office-hours/**', 'document-release/**', 'design-consultation/**', 'cso/**', 'test/helpers/carve-guards.ts', 'scripts/resolvers/sections.ts', 'scripts/resolvers/redact-doc.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/carve-section-loading.test.ts', 'bin/gstack-autoplan-snapshot.ts', 'test/autoplan-snapshot.test.ts', 'test/autoplan-init.test.ts', 'test/autoplan-obligations.test.ts', 'test/fixtures/autoplan/t-ceo-omitted-obligations.json', 'test/fixtures/autoplan/u-ceo-original-loss.json', 'test/fixtures/autoplan/v-ceo-dangling-references.json'], - 'autoplan-chain-pty': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/autoplan-dashboard.md', 'test/autoplan-chain-fixture.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'bin/gstack-config', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/helpers/autoplan-phase-observer.ts', 'test/autoplan-phase-observer.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/skill-e2e-autoplan-chain.test.ts', 'bin/gstack-autoplan-snapshot.ts', 'test/autoplan-snapshot.test.ts', 'test/autoplan-init.test.ts', 'test/autoplan-obligations.test.ts', 'test/fixtures/autoplan/t-ceo-omitted-obligations.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/helpers/eval-budgets.ts', 'test/autoplan-eval-budget.test.ts', 'test/eval-budgets-policy.test.ts', 'test/eval-detach-timeout-floor.test.ts', 'scripts/test-paid-shards.ts', '.github/workflows/evals-periodic.yml', 'test/fixtures/autoplan-routing-n-screen.txt', 'test/autoplan-routing-o.test.ts', 'test/fixtures/autoplan-routing-o-screen.txt', 'test/autoplan-setup-packet-o.test.ts', 'test/fixtures/autoplan-setup-packet-o-screen.txt', 'test/fixtures/autoplan-setup-packet-o-call.json', 'test/fixtures/autoplan/u-ceo-original-loss.json', 'test/fixtures/autoplan/v-ceo-dangling-references.json', 'test/fixtures/autoplan-setup-z-packet.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/helpers/autoplan-method-read-audit.ts', 'test/autoplan-method-read-audit.test.ts', 'test/fixtures/autoplan-method-read-aa-events.json', 'test/autoplan-routing-manual-skills.test.ts', 'test/fixtures/autoplan-routing-manual-skills-ac.json', 'test/helpers/plan-count-pending-question.ts', 'test/autoplan-pending-question.test.ts', 'test/plan-pending-question-pty.test.ts', 'test/pending-question-completion.test.ts', 'test/fixtures/pending-question-completion-ad.json', 'test/fixtures/autoplan-setup-ad-v2-packet.json'], + 'autoplan-chain-pty': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/autoplan-dashboard.md', 'test/autoplan-chain-fixture.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'bin/gstack-config', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/helpers/autoplan-phase-observer.ts', 'test/autoplan-phase-observer.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/skill-e2e-autoplan-chain.test.ts', 'bin/gstack-autoplan-snapshot.ts', 'test/autoplan-snapshot.test.ts', 'test/autoplan-init.test.ts', 'test/autoplan-obligations.test.ts', 'test/fixtures/autoplan/t-ceo-omitted-obligations.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/helpers/eval-budgets.ts', 'test/autoplan-eval-budget.test.ts', 'test/eval-budgets-policy.test.ts', 'test/eval-detach-timeout-floor.test.ts', 'scripts/test-paid-shards.ts', '.github/workflows/evals-periodic.yml', 'test/fixtures/autoplan-routing-n-screen.txt', 'test/autoplan-routing-o.test.ts', 'test/fixtures/autoplan-routing-o-screen.txt', 'test/autoplan-setup-packet-o.test.ts', 'test/fixtures/autoplan-setup-packet-o-screen.txt', 'test/fixtures/autoplan-setup-packet-o-call.json', 'test/fixtures/autoplan/u-ceo-original-loss.json', 'test/fixtures/autoplan/v-ceo-dangling-references.json', 'test/fixtures/autoplan-setup-z-packet.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/helpers/autoplan-method-read-audit.ts', 'test/autoplan-method-read-audit.test.ts', 'test/fixtures/autoplan-method-read-aa-events.json', 'test/autoplan-routing-manual-skills.test.ts', 'test/fixtures/autoplan-routing-manual-skills-ac.json', 'test/helpers/plan-count-pending-question.ts', 'test/autoplan-pending-question.test.ts', 'test/plan-pending-question-pty.test.ts', 'test/pending-question-completion.test.ts', 'test/fixtures/pending-question-completion-ad.json', 'test/fixtures/autoplan-setup-ad-v2-packet.json', 'test/helpers/autoplan-artifact-permission.ts', 'test/autoplan-artifact-permission.test.ts', 'test/fixtures/autoplan-artifact-permission-ad-v3.json', 'scripts/resolvers/testing.ts'], // Per-finding AskUserQuestion count + review-report-at-bottom assertion. // Each test drives its skill end-to-end; touchfiles include preamble + // completion-status resolvers because they affect question cadence and // terminal output (the regression surface this test catches). 'plan-ceo-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-ceo-review/**', 'test/skill-ceo-section-ordering.test.ts', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.json', 'test/helpers/ceo-completion-handoff.ts', 'test/ceo-completion-handoff.test.ts', 'test/ceo-count-s-terminals.test.ts', 'test/fixtures/ceo-count-s-paired.json', 'test/fixtures/ceo-completion-handoff-calls.json', 'test/fixtures/ceo-completion-handoff-j-calls.json', 'test/fixtures/ceo-completion-handoff-k-calls.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/skill-e2e-plan-ceo-finding-count.test.ts', 'test/ceo-completion-handoff-l.test.ts', 'test/fixtures/ceo-completion-handoff-l-calls.json', 'test/ceo-completion-handoff-m.test.ts', 'test/fixtures/ceo-completion-handoff-m-call.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/fixtures/ceo-handoff-n-calls.json', 'test/ceo-completion-handoff-o.test.ts', 'test/fixtures/ceo-completion-handoff-o-call.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/helpers/ceo-approach-pick.ts', 'test/ceo-approach-pick.test.ts', 'test/fixtures/ceo-approach-q-call.json', 'test/fixtures/ceo-approach-r-call.json', 'test/fixtures/ceo-approach-r-distinct-call.json', 'test/fixtures/ceo-approach-q-paired-call.json', 'test/fixtures/ceo-completion-handoff-q-call.json', 'test/fixtures/ceo-completion-handoff-r-calls.json', 'test/fixtures/ceo-completion-handoff-t-call.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/ceo-completion-handoff-u-call.json', 'test/fixtures/ceo-completion-handoff-v-call.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json', 'test/plan-count-ceo-body-finding.test.ts', 'test/fixtures/ceo-count-w-paired.json', 'test/fixtures/ceo-completion-handoff-w-call.json', 'test/fixtures/ceo-approach-y-call.json', 'test/fixtures/ceo-approach-y-screen.txt', 'test/ceo-handoff-y.test.ts', 'test/fixtures/ceo-handoff-y-call.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/fixtures/ceo-handoff-z-call.json', 'test/fixtures/ceo-approach-aa-call.json', 'test/review-handoffs-aa.test.ts', 'test/fixtures/review-handoff-aa-ceo.json', 'test/fixtures/review-handoff-aa-dx.json', 'test/helpers/ceo-mode-option.ts', 'test/ceo-count-mode.test.ts', 'test/fixtures/ceo-count-mode-ab-call.json', 'test/ceo-count-ac.test.ts', 'test/fixtures/ceo-count-ac-calls.json', 'test/fixtures/ceo-count-ac-later-calls.json', 'test/plan-count-permission-ac.test.ts', 'test/fixtures/plan-count-permission-ac.json', 'test/fixtures/plan-count-permission-ad.json', 'test/ceo-count-ad-v2.test.ts', 'test/fixtures/ceo-count-ad-v2.json', 'test/fixtures/plan-count-permission-target-ad-v2.json'], - 'plan-eng-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/skill-e2e-plan-eng-finding-count.test.ts', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/eng-first-review-t.test.ts', 'test/fixtures/eng-batching-t-calls.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json', 'test/eng-scope-y.test.ts', 'test/fixtures/eng-scope-y-calls.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/eng-binding-z.test.ts', 'test/fixtures/eng-binding-z-calls.json', 'test/eng-binding-retry-z.test.ts', 'test/fixtures/eng-binding-retry-z-calls.json', 'test/plan-count-permission-ac.test.ts', 'test/fixtures/plan-count-permission-ac.json', 'test/fixtures/plan-count-permission-ad.json', 'test/fixtures/plan-count-permission-target-ad-v2.json', 'test/helpers/eng-completion-handoff.ts', 'test/eng-count-ad-v2.test.ts', 'test/fixtures/eng-count-ad-v2.json'], + 'plan-eng-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/skill-e2e-plan-eng-finding-count.test.ts', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/eng-first-review-t.test.ts', 'test/fixtures/eng-batching-t-calls.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json', 'test/eng-scope-y.test.ts', 'test/fixtures/eng-scope-y-calls.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/eng-binding-z.test.ts', 'test/fixtures/eng-binding-z-calls.json', 'test/eng-binding-retry-z.test.ts', 'test/fixtures/eng-binding-retry-z-calls.json', 'test/plan-count-permission-ac.test.ts', 'test/fixtures/plan-count-permission-ac.json', 'test/fixtures/plan-count-permission-ad.json', 'test/fixtures/plan-count-permission-target-ad-v2.json', 'test/helpers/eng-completion-handoff.ts', 'test/eng-count-ad-v2.test.ts', 'test/fixtures/eng-count-ad-v2.json', 'test/helpers/eng-seeded-coverage.ts', 'test/eng-seeded-coverage.test.ts'], 'plan-design-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-design-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/helpers/design-count-review.ts', 'test/design-count-review.test.ts', 'test/fixtures/design-review-j-calls.json', 'test/helpers/design-count-outside.ts', 'test/design-count-outside.test.ts', 'test/skill-e2e-plan-design-finding-count.test.ts', 'test/fixtures/design-review-l-calls.json', 'test/design-completion-handoff.test.ts', 'test/fixtures/design-handoff-l-calls.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/fixtures/design-review-n-calls.json', 'test/design-completion-handoff-scored.test.ts', 'test/fixtures/design-handoff-n-calls.json', 'test/fixtures/design-handoff-q-calls.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/design-completion-handoff-u.test.ts', 'test/fixtures/design-handoff-u-calls.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json', 'test/helpers/design-artifact-question.ts', 'test/design-artifact-question.test.ts', 'test/fixtures/design-artifacts-w-calls.json', 'test/fixtures/design-outside-y-calls.json', 'test/fixtures/design-boundaries-y-calls.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/fixtures/design-gap-z-calls.json', 'test/plan-count-permission-ac.test.ts', 'test/fixtures/plan-count-permission-ac.json', 'test/fixtures/plan-count-permission-ad.json', 'test/fixtures/plan-count-permission-target-ad-v2.json', 'test/design-count-ad-v2.test.ts', 'test/fixtures/design-count-ad-v2.json'], - 'plan-devex-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-devex-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts', 'test/fixtures/dx-prerequisite-r-call.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/fixtures/devex-review-n-calls.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/devex-output-o.test.ts', 'test/fixtures/devex-review-o-calls.json', 'test/fixtures/devex-output-o-retry-call.json', 'test/devex-setup-remedy-o.test.ts', 'test/fixtures/devex-review-o-retry-calls.json', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/fixtures/devex-review-t-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/devex-count-u-calls.json', 'test/fixtures/devex-count-u-retry-calls.json', 'test/fixtures/devex-empathy-v-calls.json', 'test/fixtures/devex-handoff-v-call.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json', 'test/fixtures/devex-count-y-calls.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/fixtures/devex-count-z-calls.json', 'test/fixtures/devex-handoff-z-call.json', 'test/review-handoffs-aa.test.ts', 'test/fixtures/review-handoff-aa-ceo.json', 'test/fixtures/review-handoff-aa-dx.json', 'test/devex-empathy-ab.test.ts', 'test/fixtures/devex-empathy-ab-calls.json', 'test/devex-ac-accounting.test.ts', 'test/fixtures/devex-ac-first-attempt-calls.json', 'test/plan-count-permission-ac.test.ts', 'test/fixtures/plan-count-permission-ac.json', 'test/fixtures/plan-count-permission-ad.json', 'test/fixtures/plan-count-permission-target-ad-v2.json', 'test/devex-reconfirmation-ad-v2.test.ts', 'test/fixtures/devex-reconfirmation-ad-v2.json', 'test/plan-count-history.test.ts'], + 'plan-devex-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-devex-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/plan-count-dx-handoff.test.ts', 'test/fixtures/devex-handoff-n-call.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/autoplan-public-narration.test.ts', 'test/fixtures/autoplan-public-narration-ad.json', 'test/helpers/plan-count-pending-exit.ts', 'test/plan-count-pending-exit.test.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/plan-count-timeout.test.ts', 'test/plan-count-navigation-r.test.ts', 'test/plan-count-prerequisite-n.test.ts', 'test/fixtures/ceo-prerequisite-n-call.json', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts', 'test/fixtures/dx-prerequisite-r-call.json', 'test/fixtures/devex-review-l-calls.json', 'test/plan-count-checkbox.test.ts', 'test/fixtures/ceo-checkbox-l.screen.txt', 'test/fixtures/devex-review-n-calls.json', 'test/plan-count-empty-review.test.ts', 'test/fixtures/ceo-count-s-distinct.json', 'test/fixtures/plan-count-design-questionless-report.md', 'test/devex-output-o.test.ts', 'test/fixtures/devex-review-o-calls.json', 'test/fixtures/devex-output-o-retry-call.json', 'test/devex-setup-remedy-o.test.ts', 'test/fixtures/devex-review-o-retry-calls.json', 'test/plan-count-dx-handoff-o.test.ts', 'test/fixtures/devex-handoff-o-call.json', 'test/eng-devex-s-count.test.ts', 'test/fixtures/eng-devex-s-first-calls.json', 'test/fixtures/eng-devex-s-retry-calls.json', 'test/fixtures/devex-review-t-calls.json', 'test/helpers/plan-count-file-permission.ts', 'test/plan-count-file-permission.test.ts', 'test/fixtures/plan-count-edit-permission-t.json', 'test/plan-count-preview-footer.test.ts', 'test/fixtures/ceo-preview-u-call.json', 'test/fixtures/ceo-preview-u-screen.txt', 'test/fixtures/devex-count-u-calls.json', 'test/fixtures/devex-count-u-retry-calls.json', 'test/fixtures/devex-empathy-v-calls.json', 'test/fixtures/devex-handoff-v-call.json', 'test/fixtures/design-preview-v-screen.txt', 'test/plan-count-owned-permission.test.ts', 'test/fixtures/plan-count-owned-permission-v.json', 'test/fixtures/ceo-questionless-w-native.json', 'test/fixtures/devex-count-y-calls.json', 'test/plan-count-truncated-question.test.ts', 'test/fixtures/ceo-approach-z-call.json', 'test/fixtures/ceo-approach-z-screen.txt', 'test/fixtures/devex-count-z-calls.json', 'test/fixtures/devex-handoff-z-call.json', 'test/review-handoffs-aa.test.ts', 'test/fixtures/review-handoff-aa-ceo.json', 'test/fixtures/review-handoff-aa-dx.json', 'test/devex-empathy-ab.test.ts', 'test/fixtures/devex-empathy-ab-calls.json', 'test/devex-ac-accounting.test.ts', 'test/fixtures/devex-ac-first-attempt-calls.json', 'test/plan-count-permission-ac.test.ts', 'test/fixtures/plan-count-permission-ac.json', 'test/fixtures/plan-count-permission-ad.json', 'test/fixtures/plan-count-permission-target-ad-v2.json', 'test/devex-reconfirmation-ad-v2.test.ts', 'test/fixtures/devex-reconfirmation-ad-v2.json', 'test/plan-count-history.test.ts', 'test/helpers/devex-seed-coverage.ts', 'test/devex-seed-coverage.test.ts', 'test/fixtures/devex-seed-coverage-ad-v3.json'], // Gate-tier reviewCount-floor counterparts. Catch the May 2026 transcript // bug (model wrote a plan-mode plan and ExitPlanMode'd without firing any diff --git a/test/hermetic-skill-runtime.test.ts b/test/hermetic-skill-runtime.test.ts index a3b2409aa..ebf495b56 100644 --- a/test/hermetic-skill-runtime.test.ts +++ b/test/hermetic-skill-runtime.test.ts @@ -112,6 +112,10 @@ try { try { await session.waitFor('RUNTIME_PROBE_READY',{timeoutMs:7000,pollMs:20}); } catch (error) { failed = true; if (!${Boolean(item.failure)}) throw error; } if (failed !== ${Boolean(item.failure)}) throw new Error('unexpected startup outcome'); + const recordPath = ${JSON.stringify(record)}; + const record = JSON.parse(await Bun.file(recordPath).text()); + record.ownedStateRoot = session.hermeticSkillStateRoot ?? null; + await Bun.write(recordPath, JSON.stringify(record)); } finally { await session.close(); } `); const child = Bun.spawn([process.execPath,worker], { @@ -142,6 +146,7 @@ try { // an inherited/explicit GSTACK_HOME, or a broad policy-setting answer. expect(result.methodologyAllowed, item.name + ': owned methodology Read is covered').toBe(true); expect(result.stateDirectoryExists).toBe(true); + expect(result.ownedStateRoot).toBe(result.stateDirectory); expect(result.addDirs).toEqual([path.join(result.home,'.claude','skills','gstack'),path.join(result.home,'.gstack')]); for (const external of [operatorHome,state,path.dirname(result.home),result.codexHome,result.browserCache]) expect(result.addDirs).not.toContain(external); @@ -163,6 +168,7 @@ try { expect(result.home).toBe(operatorHome); expect(result.phaseHash).toBe(originalSection); expect(result.configValue).toBe('stale-runtime'); expect(result.runtimeAllowed).toBe(false); expect(result.addDirs).toEqual([]); expect(result.methodologyAllowed).toBe(false); + expect(result.ownedStateRoot).toBeNull(); } if (item.name === 'explicit-config') expect(result.configDir).toBe(customConfig); expect(() => process.kill(result.pid,0)).toThrow(); diff --git a/test/sdk-stale-table-ad-v3.test.ts b/test/sdk-stale-table-ad-v3.test.ts new file mode 100644 index 000000000..0e694acea --- /dev/null +++ b/test/sdk-stale-table-ad-v3.test.ts @@ -0,0 +1,73 @@ +import {expect,test} from 'bun:test'; +import fixture from './fixtures/sdk-stale-table-ad-v3.json'; +import {hasStaleFillRaceFinding as found} from './helpers/ceo-section-loading-fixture'; +const allowance='Original reader still returns v1 to its own caller (allowed: it began before commit)'; +test('actual table finding distinguishes forbidden later stale reads from the permitted original caller',()=>{ + expect(found(fixture.report)).toBe(true); + expect(found(fixture.table)).toBe(true); + expect(fixture.provenance.noRetroactivePass).toBe(true); +}); +test('the already-started original read may use a version label without changing ownership',()=>{ + for(const token of ['v17','VERSION_A','snapshot-A'])expect(found(fixture.table.replaceAll('v1',token))).toBe(true); +}); +test('allowance cannot migrate to later readers, a post-commit start, or a cache fill',()=>{ + for(const changed of [ + 'Later readers return v1 (allowed: they began after commit)', + 'Original reader still returns v1 to its own caller (allowed: it began after commit)', + 'Original reader still returns v1 to its own caller (allowed: it never began before commit)', + 'Original reader fills the cache with v1 (allowed: it began before commit)', + 'Original reader still returns v1 to later readers (allowed: it began before commit)', + ])expect(found(fixture.table.replace(allowance,changed))).toBe(false); +}); +test('a permitted original caller cannot hide acceptance of later stale reads or no required fix',()=>{ + for(const suffix of [' This stale-read behavior is accepted.',' No fix is required.',' Later readers may return stale data; this is the accepted consistency model.']) + expect(found(fixture.table.replace('None against the invariant.','None against the invariant.'+suffix))).toBe(false); +}); +test('copied table source and absent late-fill evidence cannot provide coverage',()=>{ + expect(found('```text\n'+fixture.table+'\n```')).toBe(false); + expect(found(fixture.table.split('\n').map(x=>'> '+x).join('\n'))).toBe(false); + expect(found(fixture.table.split('\n').map(x=>' '+x).join('\n'))).toBe(false); + const rows=fixture.table.split('\n'),cells=rows[2]!.split('|'); + cells[4]=' There is no stale-fill race; later reads observe the committed value. '; + rows[2]=cells.join('|');expect(found(rows.join('\n'))).toBe(false); +}); + + +test('original-caller exception requires asserted chronology for that reader',()=>{ + for(const changed of [ + 'Original reader still returns v1 to its own caller (allowed: it may have begun before commit)', + 'Original reader still returns v1 to its own caller (allowed: it did not begin before commit)', + 'Original reader still returns v1 to its own caller (allowed: it began before commit only if the write failed)', + 'Original reader still returns v1 to its own caller (allowed: another reader began before commit)', + 'Original reader still returns v1 to its own caller (allowed: the write began before commit)', + 'If the original reader still returns v1 to its own caller, that is allowed: it began before commit', + ])expect(found(fixture.table.replace(allowance,changed))).toBe(false); +}); + +test('an original-return allowance cannot erase another allowed stale consequence',()=>{ + for(const changed of [ + allowance+' and stores that v1 in the cache for later readers', + allowance+'; later readers may reuse this old value and that is allowed', + allowance+'. New readers may reuse this old value and that is permitted', + allowance+'. The stale cache refill is acceptable', + ])expect(found(fixture.table.replace(allowance,changed))).toBe(false); +}); + +test('table rows cannot borrow an ordering defect from another issue or from quoted source',()=>{ + const rows=fixture.table.split('\n'),cells=rows[2]!.split('|'); + const originalFailure=cells[4]!; + cells[4]=' The original reader receives its pre-commit snapshot; later reads observe the committed version. '; + const missing=rows.slice(0,2).concat(cells.join('|')).join('\n'); + expect(found(missing)).toBe(false); + const other=cells.slice();other[1]=' D2 ';other[4]=originalFailure; + other[5]=' This stale-read behavior is accepted; no fix is required. '; + expect(found(missing+'\n'+other.join('|'))).toBe(false); + expect(found('> '+originalFailure+'\n\n'+missing)).toBe(false); + expect(found('```text\n'+originalFailure+'\n```\n\n'+missing)).toBe(false); +}); + +import {E2E_TOUCHFILES,selectTests} from './helpers/touchfiles'; +test('the new table evidence selects only the existing SDK section-loading case',()=>{ + for(const file of ['test/sdk-stale-table-ad-v3.test.ts','test/fixtures/sdk-stale-table-ad-v3.json']) + expect(selectTests([file],E2E_TOUCHFILES,[]).selected).toEqual(['plan-ceo-section-loading']); +}); diff --git a/test/skill-e2e-autoplan-chain.test.ts b/test/skill-e2e-autoplan-chain.test.ts index fa41d64e0..bbc1f3390 100644 --- a/test/skill-e2e-autoplan-chain.test.ts +++ b/test/skill-e2e-autoplan-chain.test.ts @@ -30,6 +30,7 @@ import { isNumberedOptionListVisible, selectPtyNumberedOption, } from './helpers/claude-pty-runner'; +import { autoplanArtifactPermissionInput } from './helpers/autoplan-artifact-permission'; import { autoplanSetupDecision, type AutoplanSetupDecision } from './helpers/autoplan-setup-question'; import { autoplanPhaseCompletions, type AutoplanPhaseHit } from './helpers/autoplan-phase-observer'; import { readPlanCountTranscript, type PlanCountTranscript, type NativePublicToolEvent } from './helpers/plan-count-transcript'; @@ -96,8 +97,9 @@ describeE2E('/autoplan native chain ordering (periodic)', () => { let commandStartedAt = Date.now(); const saveSnapshot = createPlanCountSnapshotWriter(); let artifacts: { artifactDir?: string; artifactError?: string } = {}; + let publicTools: NativePublicToolEvent[] = []; const observe = () => { - const publicTools: NativePublicToolEvent[] = []; + publicTools = []; transcript = session.hermeticConfigDir ? readPlanCountTranscript(session.hermeticConfigDir, tempDir, event => publicTools.push(event)) : { status: 'error', calls: [], assistantMessages: [], error: 'No isolated autoplan transcript directory' }; @@ -112,6 +114,7 @@ describeE2E('/autoplan native chain ordering (periodic)', () => { skillName: 'autoplan', cwd: tempDir, claudeConfigDir: session.hermeticConfigDir, raw: session.rawOutput(), visible: session.visibleText(), viewport, observation: { state, hits, native: transcript, pendingSetupQuestion, methodologyAudit, exitCode: session.exitCode(), unsupportedSetup, + ownedArtifactStateRoot: session.hermeticSkillStateRoot, pendingQuestionRecorder:pendingQuestionRecorderStatus(session.pendingQuestionFile, tempDir, session.hermeticConfigDir), retention: 'Current raw/visible/viewport and parsed native metadata only; full parent JSONL retention is not guaranteed.' }, }); @@ -126,6 +129,7 @@ describeE2E('/autoplan native chain ordering (periodic)', () => { const budgetMs = AUTOPLAN_CHAIN_BUDGET.workMs; const start = Date.now(); let lastPermSig = ''; + const seenArtifactPermissions = new Set(); let lastCheckpointAt = start; const seenSetupQuestions = new Set(); while (Date.now() - start < budgetMs) { @@ -143,6 +147,19 @@ describeE2E('/autoplan native chain ordering (periodic)', () => { } const visible = viewport; + // Cropped artifact edits require current native identity and owned + // file/diff binding. Option 1 grants only this request, once per ID. + const artifactPermission = autoplanArtifactPermissionInput(visible, { + cwd: tempDir, ownedStateRoot: session.hermeticSkillStateRoot, + commandStartedAt, transcriptStatus: transcript.status, publicTools, + }, seenArtifactPermissions); + if (artifactPermission) { + seenArtifactPermissions.add(artifactPermission.signature); + session.send(artifactPermission.input); + await Bun.sleep(2000); + continue; + } + // Auto-grant any permission dialog so autoplan can keep moving // through its phases. The autoplan template auto-decides review // questions it owns. Classify on tail to avoid stale matches. diff --git a/test/skill-e2e-plan-ceo-finding-count.test.ts b/test/skill-e2e-plan-ceo-finding-count.test.ts index 34ba72f9b..a002d8d86 100644 --- a/test/skill-e2e-plan-ceo-finding-count.test.ts +++ b/test/skill-e2e-plan-ceo-finding-count.test.ts @@ -72,6 +72,8 @@ const CEILING_PAIRED = 4; // observability, and rollout gaps when those baseline facts were unspecified. const planCeo5Findings = (planPath: string) => [ `Please review this plan thoroughly in HOLD SCOPE mode. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`, + 'Proceed directly to the requested CEO review; skip the optional /office-hours prerequisite.', + 'Finish after this CEO review; I will handle subsequent reviews manually.', '', '# Plan: Payment Processing Integration', '', @@ -144,6 +146,8 @@ const planCeo5Findings = (planPath: string) => [ const planCeo2PairedFindings = (planPath: string) => [ `Please review this plan thoroughly in HOLD SCOPE mode. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`, + 'Proceed directly to the requested CEO review; skip the optional /office-hours prerequisite.', + 'Finish after this CEO review; I will handle subsequent reviews manually.', '', '# Plan: Payment Processing — Test Coverage', '', diff --git a/test/skill-e2e-plan-devex-finding-count.test.ts b/test/skill-e2e-plan-devex-finding-count.test.ts index f06915cdc..7811f45df 100644 --- a/test/skill-e2e-plan-devex-finding-count.test.ts +++ b/test/skill-e2e-plan-devex-finding-count.test.ts @@ -2,7 +2,8 @@ * /plan-devex-review per-finding AskUserQuestion count (periodic, paid, real-PTY). * * Same shape as skill-e2e-plan-ceo-finding-count: drives /plan-devex-review - * against a 5-finding seeded plan and asserts substantive issue AUQ count ∈ [N-1, N+2]. + * against a seeded plan and requires a distinct completed decision for each known gap. + * Additional real findings and deferred TODO decisions remain valid work. * DevEx deliberately resolves friction during Step 0, before scoring passes; * count those decisions too, while excluding administrative confirmations. * Plus D19: review report at bottom of produced plan file. @@ -11,6 +12,7 @@ */ import { test } from 'bun:test'; +import { devexSeedCoverage } from './helpers/devex-seed-coverage'; import { describeE2ETier } from './helpers/e2e-gate'; import * as fs from 'node:fs'; import * as os from 'node:os'; @@ -29,13 +31,9 @@ import { const describeE2E = describeE2ETier('periodic'); -const N = 5; -const FLOOR = N - 1; -const CEILING = N + 2; - describeE2E('/plan-devex-review per-finding AskUserQuestion count (periodic)', () => { test( - `5-finding plan emits ${FLOOR}-${CEILING} substantive issue AskUserQuestions`, + 'all five seeded gaps receive distinct decisions and a final review report', async () => { // Per-run artifact dir: a hardcoded shared /tmp path collides under // --retry, EVALS_JOBS>1, or concurrent worktrees (a sibling's finally- @@ -47,18 +45,19 @@ describeE2E('/plan-devex-review per-finding AskUserQuestion count (periodic)', ( const obs = await runPlanSkillCounting({ skillName: 'plan-devex-review', slashCommand: '/plan-devex-review', - followUpPrompt: planDevexCountFixture(planPath), + followUpPrompt: planDevexCountFixture(planPath) + '\nFinish this DX review; I will handle subsequent reviews manually.', expectedPlanPath: planPath, fixtureFiles: DEVEX_COUNT_FILES, isLastStep0AUQ: devexStep0Boundary, isReviewAUQ: isDevexReviewIssue, pickAUQ: devexReviewModePick, - reviewCountCeiling: CEILING + 1, + // Valid additional findings are bounded by the existing wall deadline. + reviewCountCeiling: Infinity, timeoutMs: 1_500_000, env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' }, }); - if (!['plan_ready', 'completion_summary', 'ceiling_reached'].includes(obs.outcome)) { + if (!['plan_ready', 'completion_summary'].includes(obs.outcome)) { throw new Error( `plan-devex-review finding-count FAILED: outcome=${obs.outcome}\n` + `step0=${obs.step0Count} review=${obs.reviewCount} elapsed=${obs.elapsedMs}ms\n` + @@ -73,21 +72,10 @@ describeE2E('/plan-devex-review per-finding AskUserQuestion count (periodic)', ( `\n--- evidence (last 3KB) ---\n${obs.evidence}`, ); } - if (obs.reviewCount < FLOOR) { - throw new Error( - `BAND FAIL (below floor): reviewCount=${obs.reviewCount} < FLOOR=${FLOOR}.\n` + - `Likely batching regression. Review-phase fingerprints:\n` + - obs.fingerprints - .filter((f) => !f.preReview) - .map((f) => ` - "${f.promptSnippet.slice(0, 80)}"`) - .join('\n'), - ); - } - if (obs.reviewCount > CEILING) { - throw new Error( - `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING}.\n` + - `Captured observation:\n${JSON.stringify(obs, null, 2)}`, - ); + const coverage = devexSeedCoverage(obs.transcript); + if (!coverage.complete) { + throw new Error(`SEEDED COVERAGE FAIL: ${JSON.stringify(coverage)}\n` + + `legacy diagnostic reviewCount=${obs.reviewCount}; outcome=${obs.outcome}`); } if (!fs.existsSync(planPath)) { diff --git a/test/skill-e2e-plan-eng-finding-count.test.ts b/test/skill-e2e-plan-eng-finding-count.test.ts index 58f4ed7f3..afc02c0d3 100644 --- a/test/skill-e2e-plan-eng-finding-count.test.ts +++ b/test/skill-e2e-plan-eng-finding-count.test.ts @@ -1,8 +1,9 @@ /** - * /plan-eng-review per-finding AskUserQuestion count (periodic, paid, real-PTY). + * /plan-eng-review seeded issue coverage (periodic, paid, real-PTY). * - * Same shape as skill-e2e-plan-ceo-finding-count: drives /plan-eng-review - * against a 5-finding seeded plan and asserts review-phase AUQ count ∈ [N-1, N+2]. + * Each of four seeded decisions needs its own completed native AskUserQuestion. + * The fifth seed, legacy regression coverage, is auto-added by the skill and + * needs affirmative final-plan or public-narration evidence, not another AUQ. * Plus D19: review report at bottom of produced plan file. * * Tier: periodic (~25 min, ~$5/run). Sequential by default per plan §D15. @@ -22,13 +23,10 @@ import { } from './helpers/claude-pty-runner'; import { isEngCompletionHandoff } from './helpers/eng-completion-handoff'; +import { evaluateEngSeedCoverage } from './helpers/eng-seeded-coverage'; const describeE2E = describeE2ETier('periodic'); -const N = 5; -const FLOOR = N - 1; // 4 -const CEILING = N + 2; // 7 - // Native controls found separate cache-validity, tenant-key, and new-code // coverage gaps when these surrounding contracts were omitted. The shared // mutable state and missing legacy regression below remain deliberate defects. @@ -71,9 +69,9 @@ const planEng5Findings = (planPath: string) => [ 'SessionMint, AuthCache, RequestPolicy). Worth flagging the complexity check.', ].join('\n'); -describeE2E('/plan-eng-review per-finding AskUserQuestion count (periodic)', () => { +describeE2E('/plan-eng-review seeded issue coverage (periodic)', () => { test( - `5-finding plan emits ${FLOOR}-${CEILING} review-phase AskUserQuestions`, + '5-finding plan receives distinct native decisions and a completed review report', async () => { // Per-run artifact dir: a hardcoded shared /tmp path collides under // --retry, EVALS_JOBS>1, or concurrent worktrees (a sibling's finally- @@ -82,6 +80,7 @@ describeE2E('/plan-eng-review per-finding AskUserQuestion count (periodic)', () const planPath = path.join(tmpDir, 'gstack-test-plan-eng.md'); try { + const startedAt = Date.now(); const obs = await runPlanSkillCounting({ skillName: 'plan-eng-review', slashCommand: '/plan-eng-review', @@ -94,12 +93,14 @@ describeE2E('/plan-eng-review per-finding AskUserQuestion count (periodic)', () try { return isEngCompletionHandoff(fp, fs.readFileSync(planPath, 'utf8')); } catch { return false; } // Unpublished work cannot establish a closed handoff. }, - reviewCountCeiling: CEILING + 1, + // Extra legitimate decisions are not a failure. The unchanged wall limit + // bounds runaway reviews; coverage below uses scoped completed native calls. + reviewCountCeiling: Infinity, timeoutMs: 1_500_000, env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' }, }); - if (!['plan_ready', 'completion_summary', 'ceiling_reached'].includes(obs.outcome)) { + if (!['plan_ready', 'completion_summary'].includes(obs.outcome)) { throw new Error( `plan-eng-review finding-count FAILED: outcome=${obs.outcome}\n` + `step0=${obs.step0Count} review=${obs.reviewCount} elapsed=${obs.elapsedMs}ms\n` + @@ -114,23 +115,6 @@ describeE2E('/plan-eng-review per-finding AskUserQuestion count (periodic)', () `\n--- evidence (last 3KB) ---\n${obs.evidence}`, ); } - if (obs.reviewCount < FLOOR) { - throw new Error( - `BAND FAIL (below floor): reviewCount=${obs.reviewCount} < FLOOR=${FLOOR}.\n` + - `Likely batching regression. Review-phase fingerprints:\n` + - obs.fingerprints - .filter((f) => !f.preReview) - .map((f) => ` - "${f.promptSnippet.slice(0, 80)}"`) - .join('\n'), - ); - } - if (obs.reviewCount > CEILING) { - throw new Error( - `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING}.\n` + - `Captured observation:\n${JSON.stringify(obs, null, 2)}`, - ); - } - if (!fs.existsSync(planPath)) { throw new Error( `D19 FAIL: agent did not produce expected plan file at ${planPath}. ` + @@ -138,6 +122,10 @@ describeE2E('/plan-eng-review per-finding AskUserQuestion count (periodic)', () ); } const planContent = fs.readFileSync(planPath, 'utf-8'); + const coverage = evaluateEngSeedCoverage(obs.transcript, planContent, startedAt, Date.now()); + if (!coverage.ok) { + throw new Error(`SEED COVERAGE FAIL: ${JSON.stringify(coverage)}; observed reviewCount=${obs.reviewCount}`); + } const verdict = assertReviewReportAtBottom(planContent); if (!verdict.ok) { throw new Error(