diff --git a/test/devex-count-fixture.test.ts b/test/devex-count-fixture.test.ts new file mode 100644 index 000000000..e24965360 --- /dev/null +++ b/test/devex-count-fixture.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, test } from 'bun:test'; +import type { AskUserQuestionFingerprint } from './helpers/claude-pty-runner'; +import { + DEVEX_COUNT_FILES, + planDevexCountFixture, + isDevexReviewIssue, + devexReviewModePick, +} from './helpers/devex-count-fixture'; + +let nextCall = 0; +function call(question: string, labels = ['Add to plan', 'Defer']): AskUserQuestionFingerprint { + const toolUseId = `tool-${++nextCall}`; + return { + signature: `session:${toolUseId}`, promptSnippet: question, + options: labels.map((label, i) => ({ index: i + 1, label })), + observedAtMs: 0, preReview: true, + nativeCall: { + sessionId: 'session', toolUseId, answered: true, + answers: { [question]: labels[0]! }, + questions: [{ header: 'DX decision', question, options: labels.map(label => ({ label })) }], + }, + }; +} + +const issues = [ + ['CI gate', 'Journey Stage: HELLO WORLD. The mandatory five-minute CI gate blocks the first local evaluation. Remove the gate or make it optional for local runs?'], + ['Argument order', 'run_eval(dataset, evaluator) and run_batch(evaluator, dataset) reverse the positional order. Should we standardize these signatures or require keyword arguments?'], + ['Authentication error', 'An invalid API key raises AuthError("request failed"), with no explanation or recovery guidance. How should we replace this opaque error?'], + ['Packaged example', 'The quickstart tells developers to run examples/first_eval.py, but it is absent from the published package. Include the example or fix the documented command?'], + ['Breaking rename', 'Version 2 removes Client.evaluate and replaces it with Client.run without a migration guide or deprecation warning. Add a compatibility alias or a migration path?'], +] as const; + +describe('DevEx substantive finding coverage', () => { + test('the observed mandatory confirmations alone contribute zero findings', () => { + const confirmations = [ + 'No design doc found. Run /office-hours first? ', + 'Who is your primary target developer? ', + 'Does the empathy narrative match reality? ', + // A concrete defect in a benchmark recap does not make the target + // confirmation itself a resolution decision for that defect. + 'Remove the mandatory CI wait before first eval to reach the agreed benchmark. Which tier do you confirm? ', + 'What should the magical first-eval moment look like? ', + 'How deep should this DX review go? ', + 'Confusion report reviewed. Which items should be addressed? ', + 'Which onboarding setup should run next? ', + ]; + expect(confirmations.map(question => call(question)).filter(isDevexReviewIssue)).toEqual([]); + }); + + test.each(issues)('%s is a finding in investigation or scoring', (_name, question) => { + const fp = call(question); + expect(isDevexReviewIssue(fp)).toBe(true); + fp.preReview = false; + expect(isDevexReviewIssue(fp)).toBe(true); + }); + + test('full native question evidence survives a short diagnostic snippet', () => { + const fp = call('Context from the SDK audit. '.repeat(20) + issues[3][1]); + fp.promptSnippet = fp.promptSnippet.slice(0, 240); + expect(fp.promptSnippet).not.toContain('examples/first_eval.py'); + expect(isDevexReviewIssue(fp)).toBe(true); + }); + + test('a real argument-order decision does not need a particular resolution verb', () => { + const fp = call('Which argument order should run_eval and run_batch use?', [ + 'Dataset first in both functions', 'Evaluator first in both functions', + ]); + expect(isDevexReviewIssue(fp)).toBe(true); + }); + + test.each([ + 'Design doc', 'Target persona', 'Narrative check', 'TTHW target', + 'Magic delivery', 'Review mode', 'Fix scope', + ])('observed administrative header %s cannot borrow a defect from its recap', header => { + const fp = call(`${issues[0][1]} This is the context for our confirmation.`); + fp.nativeCall!.questions[0]!.header = header; + expect(isDevexReviewIssue(fp)).toBe(false); + }); + + test('a CI issue stays substantive when it references persona and TTHW evidence', () => { + const fp = call('The target persona confirmed our TTHW target. The mandatory CI gate blocks the first eval. Which local bypass should the SDK support?'); + fp.nativeCall!.questions[0]!.header = 'CI gate fix'; + expect(isDevexReviewIssue(fp)).toBe(true); + }); + + test('one call batching the defects does not become five finding decisions', () => { + const distinct = issues.map(([, question]) => call(question)); + expect(distinct.filter(isDevexReviewIssue)).toHaveLength(5); + const batched = call('Review these issues together.'); + batched.nativeCall!.questions = distinct.flatMap(fp => fp.nativeCall!.questions); + batched.nativeCall!.answers = Object.assign({}, ...distinct.map(fp => fp.nativeCall!.answers)); + expect([batched].filter(isDevexReviewIssue)).toHaveLength(1); + }); + + test('an unanswered issue tab cannot turn an administrative answer into coverage', () => { + const admin = call('How deep should this DX review go? '); + const issue = call(issues[0][1]); + admin.nativeCall!.questions.push(issue.nativeCall!.questions[0]!); + admin.nativeCall!.unansweredQuestionIndices = [1]; + expect(isDevexReviewIssue(admin)).toBe(false); + Object.assign(admin.nativeCall!.answers!, issue.nativeCall!.answers); + admin.nativeCall!.unansweredQuestionIndices = []; + expect(isDevexReviewIssue(admin)).toBe(true); + }); + + test.each([ + 'Which files should I review? ', + 'I noted the mandatory CI gate before first eval. Can we continue the setup?', + 'Should I add a developer community Slack channel?', + 'Should the plan reference run_eval and run_batch?', + 'The package includes examples/first_eval.py. Shall I read it?', + 'Authentication errors already include a cause and a fix. Ready to continue?', + ])('unknown or unsupported prompts do not count: %s', question => { + expect(isDevexReviewIssue(call(question))).toBe(false); + }); + + test('a generic question cannot borrow issue evidence from its option labels', () => { + expect(isDevexReviewIssue(call('What should I inspect next?', [issues[0][1], issues[1][1]]))).toBe(false); + }); +}); + +describe('DevEx count review-mode selection', () => { + const modeQuestion = 'D6 — How deep should this DX review go? '; + + test('selects POLISH from the actual menu that previously chose EXPANSION', () => { + expect(devexReviewModePick(call(modeQuestion, [ + 'DX EXPANSION (Recommended)', 'DX POLISH', 'DX TRIAGE', + ]))).toBe(2); + }); + + test('retains the observed POLISH index after menu reordering', () => { + expect(devexReviewModePick(call(modeQuestion, [ + 'DX TRIAGE', 'DX EXPANSION', 'DX POLISH (Recommended)', + ]))).toBe(3); + }); + + test('recognizes the same mode question without a question ID', () => { + expect(devexReviewModePick(call('HowdeepshouldthisDXreviewgo?', [ + 'DXEXPANSION(Recommended)', 'DXPOLISH', 'DXTRIAGE', + ]))).toBe(2); + }); + + test('unrelated questions cannot select a mode from quoted labels', () => { + expect(devexReviewModePick(call('Which documentation example should be included?', [ + 'DX EXPANSION', 'DX POLISH', 'DX TRIAGE', + ]))).toBeNull(); + expect(devexReviewModePick(call(issues[0][1]))).toBeNull(); + }); + + test('missing or ambiguous mode menus keep the existing choice policy', () => { + expect(devexReviewModePick(call(modeQuestion, ['DX EXPANSION', 'DX TRIAGE']))).toBeNull(); + expect(devexReviewModePick(call(modeQuestion, [ + 'DX EXPANSION', 'DX POLISH', 'DX POLISH', 'DX TRIAGE', + ]))).toBeNull(); + expect(devexReviewModePick(call(modeQuestion, [ + 'DX EXPANSION │ DX POLISH', 'Example │ DX POLISH', 'DX TRIAGE', + ]))).toBeNull(); + }); + + test('a multi-question call is not treated as a single mode menu', () => { + const fp = call(modeQuestion, ['DX EXPANSION', 'DX POLISH', 'DX TRIAGE']); + fp.nativeCall!.questions.push(call(issues[0][1]).nativeCall!.questions[0]!); + expect(devexReviewModePick(fp)).toBeNull(); + }); +}); + +describe('DevEx calibrated fixture instructions', () => { + test('keeps the reviewed artifact path without telling the model an expected count', () => { + const plan = planDevexCountFixture('/tmp/owned-plan.md'); + expect(plan).toContain('write your plan-mode plan to /tmp/owned-plan.md'); + const suppliedContext = [plan, ...Object.values(DEVEX_COUNT_FILES)].join('\n'); + expect(suppliedContext).not.toMatch(/(?:exactly|at least|at most)\s+(?:five|5)|(?:five|5)[- ]findings?|4[-–]7|reviewCount|CEILING|FLOOR/i); + }); +}); diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts index 25d3f625d..dbd9d9237 100644 --- a/test/helpers/claude-pty-runner.ts +++ b/test/helpers/claude-pty-runner.ts @@ -28,6 +28,8 @@ import * as path from 'path'; import { stripVTControlCharacters } from 'node:util'; import { hermeticChildEnv, hermeticSkillsConfigDir, isHermeticEnabled } from './hermetic-env'; import { createPlanCountFixture } from './plan-count-fixture'; +import { createPlanCountSnapshotWriter } from './plan-count-artifacts'; +import { readPlanCountTranscript, unresolvedPlanQuestionCalls, type NativePlanQuestionCall, type PlanCountTranscript } from './plan-count-transcript'; import { trustDialogInput } from './pty-trust-dialog'; /** Strip ANSI escapes for pattern-matching against visible text. */ @@ -1058,6 +1060,22 @@ export interface AskUserQuestionFingerprint { observedAtMs: number; /** True if observed BEFORE the Step-0 boundary fired. */ preReview: boolean; + /** Lossless source metadata for counted calls; UI-only fingerprints omit it. */ + nativeCall?: NativePlanQuestionCall; +} + +/** Full question text feeds routing/phase predicates before diagnostics truncate it. */ +export function nativePlanCallFingerprint(call: NativePlanQuestionCall, observedAtMs: number, preReview: boolean): AskUserQuestionFingerprint { + // An unanswered mode tab cannot establish the selected review mode. Keep + // every question in nativeCall, but classify and summarize only the + // actually answered questions once a native call has completed. + const questions = call.answered ? call.questions.filter(q => call.answers?.[q.question]) : call.questions; + return { + signature: `${call.sessionId}:${call.toolUseId}`, + promptSnippet: questions.map(q => `${q.header} ${q.question}`).join('\n\n'), + options: questions.flatMap(q => q.options.map((o, i) => ({ index: i + 1, label: o.label }))), + observedAtMs, preReview, nativeCall: call, + }; } /** @@ -1074,10 +1092,21 @@ export type Step0BoundaryPredicate = ( answeredFingerprint: AskUserQuestionFingerprint, ) => boolean; +/** A first finding starts review immediately; a final setup question does not count as a finding. */ +export function planCountQuestionPhase( + fp: AskUserQuestionFingerprint, + reviewStarted: boolean, + isLastStep0AUQ: Step0BoundaryPredicate, + isFirstReviewAUQ?: Step0BoundaryPredicate, +): { preReview: boolean; reviewStarted: boolean } { + const inReview = reviewStarted || Boolean(isFirstReviewAUQ?.(fp)); + return { preReview: !inReview, reviewStarted: inReview || isLastStep0AUQ(fp) }; +} + /** * Parse the rendered question prompt out of a visible TTY buffer. The prompt - * is the 1–3 lines of text immediately ABOVE the latest `❯ 1.` cursor line — - * not part of the option list, not the permission-dialog header. + * starts at the active boxed question header when available, otherwise at + * the lines immediately ABOVE the latest `❯ 1.` cursor line. * * Returns the prompt normalized to a single-spaced 240-char snippet (strip * ANSI residue, collapse internal whitespace, trim) — short enough to use as @@ -1124,6 +1153,23 @@ export function parseQuestionPrompt(visible: string): string { } } + // Native boxed AUQs can contain many paragraphs separated by a lone │. + // Walking upward from the options stops at that border (or the six-line + // limit) and loses the question's identity. Start at the latest header + // inside the same fixed, cursor-anchored context window instead. Reject + // earlier menus / Planning chrome so an old header cannot label a new + // unboxed dialog after its question has been dismissed. + const questionStart = [...tail.matchAll(/(?:^|\n)[\t │┃]*[☐□][\t ]*(?=\S)/g)].at(-1); + if (questionStart) { + const question = tail.slice(questionStart.index, tail.length - cursor[0].length); + if (!/❯\s*[1-9]\./.test(question) && !/(?:^|\n)\s*Plan+ing\s*:/i.test(question)) { + const prompt = question + .replace(/^[\t ─━┄┅┈┉┌┐└┘├┤┬┴┼│┃☐□■]+/gm, '') + .replace(/\s+/g, ' ').trim(); + if (prompt) return prompt.slice(0, 240); + } + } + // Walk up at most 6 lines collecting prompt text. Stop at: // - a blank line preceded by another blank line (paragraph break) // - top of buffer @@ -1246,9 +1292,11 @@ export function planCountSubmissionInput(visible: string): string | null { const bar = bars.at(-1); if (!bar) return null; const panel = visible.slice(bar.index + bar[0].length).replace(/\s+/g, ''); - const cursor = [...panel.matchAll(/❯1\./g)].at(-1); - if (!/Reviewyouranswers/i.test(panel) || !cursor || - !/^❯1\.Submit/i.test(panel.slice(cursor.index))) return null; + // Captured cursor redraws can omit the dot after 1 and the e in answers. + // The native tab bar and explicit Submit label still identify this control. + const cursor = [...panel.matchAll(/❯1\.?/g)].at(-1); + if (!/Reviewyouranswe?rs/i.test(panel) || !cursor || + !/^❯1\.?Submit/i.test(panel.slice(cursor.index))) return null; const tabs = [...bar[0].matchAll(/[☐☒]/g)].map((match) => match[0]); const unanswered = tabs.indexOf('☐'); // At the Submit tab, move back to the first unanswered question. Only @@ -1368,6 +1416,17 @@ export const ceoStep0Boundary: Step0BoundaryPredicate = (fp) => // directly to review-phase. Boundary fires on the scope AUQ itself. fp.options.some((o) => /skip\s+interview|plan\s+immediately/i.test(o.label)); +/** Native finding evidence when CEO mode selection is omitted or left unanswered. */ +export const ceoFirstReviewAUQ: Step0BoundaryPredicate = (fp) => + fp.nativeCall?.questions.some(q => { + if (fp.nativeCall?.answered && !fp.nativeCall.answers?.[q.question]) return false; + const id = / /scope\s*reduction\s*recommendation|cross[\s-]*project\s*learnings/i.test( fp.promptSnippet, @@ -1386,6 +1445,19 @@ export const designStep0Boundary: Step0BoundaryPredicate = (fp) => fp.promptSnippet, ); +/** Positive review identity when a design run goes directly to findings without a focus AUQ. */ +export const designFirstReviewAUQ: Step0BoundaryPredicate = (fp) => { + // A numbered setup decision is not sufficient. Require the design review's + // question ID as well, and exclude its scope/focus/onboarding identities. + const id = / /developer\s*persona|target\s*persona|persona\s*selection|TTHW\s*target/i.test( fp.promptSnippet, @@ -2018,7 +2090,7 @@ export async function runPlanSkillObservation(opts: { // ──────────────────────────────────────────────────────────────────────────── // runPlanSkillCounting — drives a plan-* skill end-to-end through Step 0 then -// counts distinct review-phase AskUserQuestion fingerprints. The actual +// counts completed review-phase AskUserQuestion calls. The actual // product asserted by the per-finding-count tests. // ──────────────────────────────────────────────────────────────────────────── @@ -2028,11 +2100,15 @@ export async function runPlanSkillObservation(opts: { * dumps when an assertion fails. */ export interface PlanSkillCountObservation { + /** Durable full raw/visible PTY output plus JSON observation, when EVALS_RUN_ID is set. */ + artifactDir?: string; + artifactError?: string; outcome: | 'plan_ready' | 'completion_summary' | 'ceiling_reached' | 'silent_write' + | 'transcript_unavailable' | 'exited' | 'timeout'; summary: string; @@ -2042,6 +2118,8 @@ export interface PlanSkillCountObservation { elapsedMs: number; /** All distinct AskUserQuestions observed, in observation order. */ fingerprints: AskUserQuestionFingerprint[]; + /** Actual native calls, including unanswered/failed ones that add no coverage. */ + transcript: PlanCountTranscript; /** Count of fingerprints with `preReview === true`. */ step0Count: number; /** Count of fingerprints with `preReview === false`. */ @@ -2065,16 +2143,15 @@ export interface PlanSkillCountObservation { * review incorrectly starts against the operator's live branch. * 3. Poll loop: * - Skip permission dialogs (auto-grant with `defaultPick`). - * - On a new numbered-option list, parse prompt + options, build - * fingerprint via `auqFingerprint`. Empty-prompt parses are skipped - * and re-polled (avoids the empty-prompt collision documented in - * the auqFingerprint contract). - * - First time we see a fingerprint: push it, classify as Step 0 or - * review-phase based on `boundaryFired`, press `defaultPick` to - * advance. Decline only the recognized optional office-hours + * - Read fixture-scoped native JSONL. Count each AskUserQuestion call + * once after its matching successful answer record, regardless of + * how many questions the call batches. Full native metadata feeds + * phase predicates; ANSI redraws and permissions cannot add counts. + * - On a new numbered-option list, keep the existing PTY answer driver. + * Decline only the recognized optional office-hours * prerequisite by label so a different skill does not rewrite the * seeded plan before this review begins. - * - After pressing, evaluate `isLastStep0AUQ(fingerprint)`. If true, + * - After the native answer, evaluate `isLastStep0AUQ(fingerprint)`. If true, * all subsequent AUQs are review-phase. * - Hard ceiling: if `reviewCount >= reviewCountCeiling`, return * `ceiling_reached`. This bounds runaway counts; tests should set @@ -2089,10 +2166,9 @@ export interface PlanSkillCountObservation { * where Step-0-final and Section-1-first AUQs straddle a section header * regex match. * - * Fingerprint composition (D9): `auqFingerprint(prompt, options)` mixes - * normalized prompt text with the options signature so distinct findings - * with shared menu structure (the generic A/B/C TODO menu) get distinct - * fingerprints. + * UI fingerprints still dedupe redraws. Counted fingerprints use the + * native session/tool call IDs, so shared answer labels never collapse + * different calls and one multi-question call remains one finding. */ export async function runPlanSkillCounting(opts: { /** Skill name, e.g. 'plan-ceo-review'. Used for diagnostic strings only. */ @@ -2103,6 +2179,14 @@ export async function runPlanSkillCounting(opts: { followUpPrompt: string; /** Per-skill predicate: which answered AUQ is the last Step-0 question. */ isLastStep0AUQ: Step0BoundaryPredicate; + /** Optional positive identity for a first finding when no final setup AUQ was emitted. */ + isFirstReviewAUQ?: Step0BoundaryPredicate; + /** Optional issue classifier across phases; receives full native call metadata. */ + isReviewAUQ?: (fp: AskUserQuestionFingerprint) => boolean; + /** Narrow caller-specific selection; null retains the normal answer policy. */ + pickAUQ?: (fp: AskUserQuestionFingerprint) => number | null; + /** Additional versioned files available in the isolated fixture before the skill starts. */ + fixtureFiles?: Record; /** Hard cap on review-phase count; helper returns when reached. Should be * set ABOVE the test's assertion ceiling so the test sees the cap as a * failure rather than a silent stop. */ @@ -2133,7 +2217,7 @@ export async function runPlanSkillCounting(opts: { const defaultPick = opts.defaultPick ?? 1; const timeoutMs = opts.timeoutMs ?? 1_500_000; - const fixture = createPlanCountFixture(opts.followUpPrompt, { nativeReviewOnly: true }); + const fixture = createPlanCountFixture(opts.followUpPrompt, { nativeReviewOnly: true, files: opts.fixtureFiles }); let session: ClaudePtySession; try { session = await launchClaudePty({ @@ -2151,10 +2235,19 @@ export async function runPlanSkillCounting(opts: { const fingerprints: AskUserQuestionFingerprint[] = []; const seen = new Set(); + const countedCalls = new Set(); + let transcript: PlanCountTranscript = { status: 'missing', calls: [], assistantMessages: [] }; let boundaryFired = false; let step0Count = 0; let reviewCount = 0; let isFirstAUQ = true; + const saveSnapshot = createPlanCountSnapshotWriter(); + let lastCheckpointAt = Date.now(); + + const capture = (observation: object) => saveSnapshot({ + skillName: opts.skillName, observation, raw: session.rawOutput(), visible: session.visibleText(), + cwd: fixture.cwd, claudeConfigDir: session.hermeticConfigDir, + }); function snapshot( outcome: PlanSkillCountObservation['outcome'], @@ -2164,7 +2257,7 @@ export async function runPlanSkillCounting(opts: { const clean = (text: string) => stripVTControlCharacters(text) .replace(/[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/g, ''); const failed = outcome === 'exited' || outcome === 'timeout'; - return { + const observation: PlanSkillCountObservation = { outcome, summary, evidence: failed @@ -2173,9 +2266,15 @@ export async function runPlanSkillCounting(opts: { : visible.slice(-3000), elapsedMs: Date.now() - startedAt, fingerprints, + transcript, step0Count, reviewCount, }; + const artifacts = capture(observation); + Object.assign(observation, artifacts); + if (artifacts.artifactDir) observation.evidence += `\nFull PTY artifacts: ${artifacts.artifactDir}`; + if (artifacts.artifactError) observation.evidence += `\nPTY artifact write failed: ${artifacts.artifactError}`; + return observation; } try { @@ -2187,6 +2286,39 @@ export async function runPlanSkillCounting(opts: { while (Date.now() - budgetStart < timeoutMs) { await Bun.sleep(2000); const visible = session.visibleSince(since); + transcript = session.hermeticConfigDir + ? readPlanCountTranscript(session.hermeticConfigDir, fixture.cwd) + : { status: 'error', calls: [], assistantMessages: [], error: 'Claude count session has no isolated transcript directory' }; + if (transcript.status === 'error') { + return snapshot('transcript_unavailable', transcript.error!, visible); + } + for (const call of transcript.calls) { + const signature = `${call.sessionId}:${call.toolUseId}`; + if (!call.answered || countedCalls.has(signature)) continue; + const fp = nativePlanCallFingerprint(call, Date.now() - startedAt, !boundaryFired); + const phase = planCountQuestionPhase(fp, boundaryFired, opts.isLastStep0AUQ, opts.isFirstReviewAUQ); + fp.preReview = opts.isReviewAUQ ? !opts.isReviewAUQ(fp) : phase.preReview; + fp.promptSnippet = fp.promptSnippet.replace(/\s+/g, ' ').slice(0, 240); + fingerprints.push(fp); + countedCalls.add(signature); + if (fp.preReview) step0Count += 1; + else reviewCount += 1; + boundaryFired = phase.reviewStarted; + } + if (reviewCount >= opts.reviewCountCeiling) { + if (unresolvedPlanQuestionCalls(transcript.calls).length) { + return snapshot('transcript_unavailable', 'Question count reached its ceiling with unresolved failed native calls', visible); + } + return snapshot('ceiling_reached', `review-phase AUQ count reached ceiling (${opts.reviewCountCeiling})`, visible); + } + // An outer test timeout/cancellation may prevent a terminal snapshot. + // Keep bounded-cadence evidence without adding a timer to clean up. + if (Date.now() - lastCheckpointAt >= 30_000) { + lastCheckpointAt = Date.now(); + const saved = capture({ state: 'in_progress', elapsedMs: Date.now() - startedAt, + fingerprints, step0Count, reviewCount, transcript }); + if (saved.artifactError) console.error(`PTY artifact write failed: ${saved.artifactError}`); + } // Process exited? if (session.exited()) { @@ -2240,6 +2372,10 @@ export async function runPlanSkillCounting(opts: { // Soft terminal signals — check before AUQ processing so a final // completion-summary doesn't get misclassified as a bonus AUQ. if (frame === 'completion_summary') { + if (transcript.status !== 'ready' || transcript.calls.some(c => !c.answered && !c.failed) || + unresolvedPlanQuestionCalls(transcript.calls).length) { + return snapshot('transcript_unavailable', 'Completion has no complete native question transcript', visible); + } return snapshot( 'completion_summary', `skill emitted completion summary / verdict / status line (step0=${step0Count}, review=${reviewCount})`, @@ -2247,6 +2383,10 @@ export async function runPlanSkillCounting(opts: { ); } if (frame === 'plan_ready') { + if (transcript.status !== 'ready' || transcript.calls.some(c => !c.answered && !c.failed) || + unresolvedPlanQuestionCalls(transcript.calls).length) { + return snapshot('transcript_unavailable', 'Plan-ready gate has no complete native question transcript', visible); + } return snapshot( 'plan_ready', `skill emitted plan-mode "Ready to execute" confirmation (step0=${step0Count}, review=${reviewCount})`, @@ -2261,33 +2401,20 @@ export async function runPlanSkillCounting(opts: { // findings often reuse the same Add to plan / Defer / Skip menu. const fp = capturePlanCountQuestion(visible, seen, Date.now() - startedAt, !boundaryFired); if (!fp) continue; - fingerprints.push(fp); - if (boundaryFired) reviewCount += 1; - else step0Count += 1; - // Press to advance — first AUQ may use the override pick. - const prerequisitePick = planCountPrerequisitePick(fp); - const pickIdx = prerequisitePick ?? - (isFirstAUQ && opts.firstAUQPick ? opts.firstAUQPick(fp) : defaultPick); + const pending = transcript.calls.find(c => !c.answered && !c.failed); + const routing = pending?.questions.length === 1 + ? nativePlanCallFingerprint(pending, fp.observedAtMs, fp.preReview) : fp; + const prerequisitePick = planCountPrerequisitePick(routing); + // Native tool records may flush only after the answer. Let a guarded + // caller recognize that visible menu, but never override a known packet. + const callerPick = !pending || pending.questions.length === 1 ? opts.pickAUQ?.(routing) ?? null : null; + const pickIdx = prerequisitePick ?? callerPick ?? + (isFirstAUQ && opts.firstAUQPick ? opts.firstAUQPick(routing) : defaultPick); isFirstAUQ = false; - if (prerequisitePick !== null) await selectPtyNumberedOption(session, pickIdx); + if (prerequisitePick !== null || callerPick !== null) await selectPtyNumberedOption(session, pickIdx); else session.send(`${pickIdx}\r`); - // Evaluate boundary AFTER pressing — if THIS AUQ was the last Step 0 - // question, all subsequent AUQs go to reviewCount. - if (!boundaryFired && opts.isLastStep0AUQ(fp)) { - boundaryFired = true; - } - - // Hard ceiling — runaway protection. - if (reviewCount >= opts.reviewCountCeiling) { - return snapshot( - 'ceiling_reached', - `review-phase AUQ count reached ceiling (${opts.reviewCountCeiling})`, - session.visibleSince(since), - ); - } - // Give the agent a beat to advance to the next state. await Bun.sleep(2000); } diff --git a/test/helpers/claude-pty-runner.unit.test.ts b/test/helpers/claude-pty-runner.unit.test.ts index 0e4c77ba6..8587c6bea 100644 --- a/test/helpers/claude-pty-runner.unit.test.ts +++ b/test/helpers/claude-pty-runner.unit.test.ts @@ -36,6 +36,7 @@ import { TAIL_SCAN_BYTES, optionsSignature, parseQuestionPrompt, + stripAnsi, auqFingerprint, COMPLETION_SUMMARY_RE, classifyPlanCountFrame, @@ -46,6 +47,8 @@ import { ceoStep0Boundary, engStep0Boundary, designStep0Boundary, + designFirstReviewAUQ, + planCountQuestionPhase, devexStep0Boundary, type ClaudePtyOptions, type AskUserQuestionFingerprint, @@ -114,6 +117,18 @@ describe('isPermissionDialogVisible', () => { expect(isPermissionDialogVisible(sample.replace('Esctocancel·Tabtoamend', 'Enter to select'))).toBe(false); }); + test('the captured paired-CEO Edit grant is permission, not another review finding', () => { + const sample = [ + 'Do youwt to makehis dittogstack-test-plan-ceo-paired.md?', + '❯1.Yes', + '2.Yes,andswitchtoacceptedits(auto-approvefileeditsandcommonfilecommands)forthissession;Yes,and', + 'alwaysallowaccessto/tmp/gstack-paid-shard-EbUl9j/tmp/gstack-e2e-plan-ceo-paired-gkjAd5forthissession', + '(shift+tab)', '3.No', 'Esctocancel·Tabtoamend', + ].join('\r'); + expect(isPermissionDialogVisible(sample)).toBe(true); + expect(classifyPlanCountFrame(sample)).toBe('permission'); + }); + test('recognizes permission labels whose cursor-positioning spaces disappeared', () => { expect(isPermissionDialogVisible('Yes,andalwaysallowaccessto/tmp/fixtureforthissession')).toBe(true); expect(isPermissionDialogVisible('Yes,allowalleditsduringthissession')).toBe(true); @@ -866,6 +881,58 @@ describe('optionsSignature', () => { }); describe('parseQuestionPrompt', () => { + test('keeps the captured boxed learnings header across native CR and blank borders', () => { + // Exact active-menu bytes from the targeted-a engineering batching run. + // Its answered setup AUQ lost the title at the standalone box border, + // leaving every later finding classified as preReview. + const raw = "☐ Learnings\u001b[K\r\u001b[1B\u001b[K\r\u001b[1B│ D1 — Cross-project learnings scope \u001b[K\r\u001b[1B│\u001b[3G\u001b[K\r\r\n│\u001b[3Ggstack\u001b[10Gcan\u001b[14Gsearch\u001b[21Glearnings\u001b[31Gfrom\u001b[36Gyour\u001b[41Gother\u001b[47Gprojects\u001b[56Gon\u001b[59Gthis\u001b[64Gmachine\u001b[72Gto\u001b[75Gfind\u001b[80Gpatterns\u001b[89Gthat\u001b[94Gmight\u001b[100Gapply\u001b[106Ghere.\u001b[112GThis\r\r\n│\u001b[3Gstays\u001b[9Glocal\u001b[15G—\u001b[17Gno\u001b[20Gdata\u001b[25Gleaves\u001b[32Gyour\u001b[37Gmachine.\u001b[46GRecommended\u001b[58Gfor\u001b[62Gsolo\u001b[67Gdevelopers.\u001b[79GSkip\u001b[84Gif\u001b[87Gyou\u001b[91Gwork\u001b[96Gon\u001b[99Gmultiple\u001b[108Gclient\r\r\n│\u001b[3Gcodebases\u001b[13Gwhere\u001b[19Gcross-contamination\u001b[39Gwould\u001b[45Gbe\u001b[48Ga\u001b[50Gconcern.\r\r\n\r\r\n❯\u001b[3G1.\u001b[6GEnable\u001b[13Gcross-project\u001b[27Glearnings\u001b[37G(Recommended)\r\r\n\u001b[6GSearch\u001b[13Glearnings\u001b[23Gfrom\u001b[28Gall\u001b[32Gprojects\u001b[41Gon\u001b[44Gthis\u001b[49Gmachine\u001b[57G—\u001b[59Gsurfaces\u001b[68Gpatterns\u001b[77Gand\u001b[81Gpitfalls\u001b[90Gfrom\u001b[95Gprior\u001b[101Gsessions.\r\r\n\u001b[3G2.\u001b[6GKeep\u001b[11Glearnings\u001b[21Gproject-scoped\u001b[36Gonly\r\r\n\u001b[6GOnly\u001b[11Guse\u001b[15Glearnings\u001b[25Gfrom\u001b[30Gthis\u001b[35Gproject.\u001b[44GSafe\u001b[49Gfor\u001b[53Gmulti-client\u001b[66Genvironments.\r\r\n\u001b[3G3.\u001b[6GType\u001b[11Gsomething.\r\r\n────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\r\r\n\u001b[3G4.\u001b[6GChat\u001b[11Gabout\u001b[17Gthis\r\r\n\r\r\nEnter\u001b[7Gto\u001b[10Gselect\u001b[17G·\u001b[19G↑/↓\u001b[23Gto\u001b[26Gnavigate\u001b[35G·\u001b[37GEsc\u001b[41Gto\u001b[44Gcancel"; + const visible = stripAnsi(raw); + const question = capturePlanCountQuestion(visible, new Set(), 0, true)!; + expect(question.promptSnippet).toStartWith('Learnings D1 — Cross-project learnings scope'); + expect(question.promptSnippet).toContain(''); + expect(engStep0Boundary(question)).toBe(true); + const phase = planCountQuestionPhase(question, false, engStep0Boundary); + expect(phase).toEqual({ preReview: true, reviewStarted: true }); + }); + + test('keeps a long boxed question identity instead of its closing recommendation', () => { + const frame = [ + 'Planning: /tmp/hermetic/.claude/plans/review.md', + '─'.repeat(120), + '☐ Architecture', + '│ D2 — Architecture: custom retry scheduler vs library built-in ', + '│', + ...Array.from({ length: 12 }, (_, i) => `│ Review context line ${i}: the proposed retry behavior and its tradeoffs.`), + '│', + '│ Net: If the library hook is configurable, use the existing implementation.', + '❯1.Use library built-in (Recommended)', + '2.Extract shared retry envelope', + ].join('\r\r\n'); + const seen = new Set(); + const question = capturePlanCountQuestion(frame, seen, 0, false)!; + expect(question.promptSnippet).toStartWith('Architecture D2 — Architecture: custom retry scheduler'); + expect(question.promptSnippet).toContain(''); + expect(question.promptSnippet).not.toContain('Planning:'); + expect(question.promptSnippet.length).toBeLessThanOrEqual(240); + expect(capturePlanCountQuestion(frame + '\n' + '·'.repeat(6000), seen, 1, false)).toBeNull(); + }); + + test('does not reuse an old boxed header for a later unboxed menu', () => { + const visible = [ + '☐ Old setup', + 'D1 — Cross-project learnings scope', + '❯1.Enable', + '2.Skip', + 'Planning: /tmp/hermetic/.claude/plans/review.md', + 'D2 — Choose the retry behavior', + '❯1.Use library built-in', + '2.Extract shared retry envelope', + ].join('\n'); + const prompt = parseQuestionPrompt(visible); + expect(prompt).toBe('D2 — Choose the retry behavior'); + expect(prompt).not.toContain('Old setup'); + }); + test('captures 1-line prompt above the cursor', () => { const visible = ` D1 — Pick a mode @@ -1290,6 +1357,27 @@ describe('classifyPlanCountFrame replay', () => { }); describe('planCountSubmissionInput replay', () => { + test('the captured mode Submit panel with a damaged caption and dotless cursor returns to its unanswered tab', () => { + // Exact final active panel from targeted-a's SCOPE EXPANSION retry. + const captured = [ + '← ☒ Routing rule ☐ Design doc ✔ Submit →', + '', + 'Review your answrs', + '⚠ You hvenot answered all questions', + " ● Add gstack skill routing rules tothisproject'sCLAUDE.md?", + '→dd routing rues (Recommnded)', + '', + 'Ready to submit your answers?', + '', + '❯1Submit answers', + ' 2. Cancel', + ].join('\r'); + expect(planCountSubmissionInput(captured)).toBe('\x1b[Z'); + const answered = captured.replace('☐ Design doc', '☒ Design doc').replace('⚠ You hvenot answered all questions', ''); + expect(planCountSubmissionInput(answered)).toBe('\r'); + expect(planCountSubmissionInput(captured + '\r☐ Design doc\rRun office hours?\r❯1Run now\r2.Skip')).toBeNull(); + }); + const incomplete = [ '← ☒ Learnings scope ☐ Approach ✔ Submit →', 'Review your answers', @@ -1507,6 +1595,54 @@ describe('Step0BoundaryPredicate per-skill', () => { }); }); + describe('design review begins without an optional focus question', () => { + // Captured in the second07:53 paid attempt: real D1-D7 questions were + // all incorrectly marked preReview, producing reviewCount=0 at completion. + const questions = [ + '☐Buttonstyle │D1—Howshouldthe4headerbuttons(Save,Reset,Cancel,Export)bevisuallydifferentiated? │', + '☐ Loading UX │D2—Whatloadingindicatorshouldappearduringthe2-5secondSaveoperation? │', + '☐ Spacing │D3—Whichspacingscaleshouldthesettingspagestandardizeon?', + '☐Typography │D4—Which2-sizetypographysystemshouldthesettingspageuse?', + '☐Mobile layout │D5 — On obile (<768px), how should the4-buton header behave?', + '☐DEIGN.md TODO │D6 — TODO: Create a DESIGN.md file codifying the5 decisions mdein this revew ', + '☐PartialfailTODO │D7—TODO:Specifythepartial-failurestate—whatdoestheuserseeifSavesucceedsforsomefieldsbutfailsfor others? ', + ]; + + test('counts the first captured finding and every subsequent finding', () => { + let reviewStarted = false; + const phases = questions.map(question => { + const phase = planCountQuestionPhase(fp(question, ['Apply', 'Defer']), reviewStarted, + designStep0Boundary, designFirstReviewAUQ); + reviewStarted = phase.reviewStarted; + return phase.preReview; + }); + expect(phases).toEqual([false, false, false, false, false, false, false]); + }); + + test('keeps the observed focus gate separate when it is emitted', () => { + const focus = fp("☐ Focus areas │ I've rated this plan2/10 on design completeness. Review all7 dimensions?", ['All7dimensions', 'Priority gaps']); + const setup = planCountQuestionPhase(focus, false, designStep0Boundary, designFirstReviewAUQ); + expect(setup).toEqual({ preReview: true, reviewStarted: true }); + expect(planCountQuestionPhase(fp(questions[0], ['Apply', 'Defer']), setup.reviewStarted, + designStep0Boundary, designFirstReviewAUQ)).toEqual({ preReview: false, reviewStarted: true }); + }); + + test('requires review identity, not just a D1 label or setup question ID', () => { + for (const question of [ + '☐ Setup │D1—Enable cross-project learnings?', + '☐ Review target │D1—Which plan should I review?', + '☐ Focus │D1—What should this design review focus on?', + '☐ Scope │I will review Pass1 through Pass7 after setup. Proceed?', + ]) expect(designFirstReviewAUQ(fp(question, ['Yes', 'No']))).toBe(false); + expect(designFirstReviewAUQ(fp('☐ Page structure │ Pass1 — Information Architecture: what page structure should this use?', ['Standard', 'Sidebar']))).toBe(true); + }); + + test('leaves callers without a first-review predicate unchanged', () => { + expect(planCountQuestionPhase(fp(questions[0], ['Apply', 'Defer']), false, designStep0Boundary)) + .toEqual({ preReview: true, reviewStarted: false }); + }); + }); + describe('devexStep0Boundary', () => { test('FIRES on developer persona selection', () => { const f = fp('Pick the target persona for this review', ['Senior backend', 'Junior frontend', 'Other']); diff --git a/test/helpers/devex-count-fixture.ts b/test/helpers/devex-count-fixture.ts new file mode 100644 index 000000000..67d75efbb --- /dev/null +++ b/test/helpers/devex-count-fixture.ts @@ -0,0 +1,168 @@ +import type { AskUserQuestionFingerprint } from './claude-pty-runner'; + +/** Concrete product decisions, separate from the skill's mandatory Step-0 confirmations. */ +export const DEVEX_COUNT_FILES: Record = { + 'README.md': `# EvalKit SDK + +EvalKit is a Python SDK for ML engineers evaluating LLM responses. The primary +developer writes Python daily, uses a terminal, and wants a local result before +connecting the SDK to production CI. The agreed review posture is DX POLISH: +improve the existing SDK's touchpoints within the beta release scope. + +## Getting started + +Install with \`python -m pip install evalkit==2.0.0b1\`, set EVALKIT_API_KEY, +then follow the quickstart's command: \`python examples/first_eval.py\`. +The published package inventory is in docs/package-contents.txt. + +The chosen first-success experience is an included, copy-paste demo command: +\`python -m evalkit.demo\`. It evaluates bundled sample responses and prints +real per-example scores plus an overall score. It needs no hosted playground +or new interactive UI. Like every first evaluation, it currently waits for the +mandatory CI check described in docs/current-contracts.md. + +See docs/api.md for public API and upgrade behavior, and docs/benchmarks.md for +the completed onboarding study. These documents describe the existing SDK's +behavior; its runtime is maintained separately from this release-planning repo. +`, + 'docs/benchmarks.md': `# Completed onboarding study + +The internal comparison measured Python SDK onboarding with the same developer +and machine. Peer SDK A took 2 minutes, B took 4 minutes, and C took 3 minutes. +EvalKit took 6 minutes, including the mandatory 5-minute CI wait. The measurement +starts before installation and ends at the first real evaluation result. + +The agreed target is under 2 minutes. The study, target persona, and terminal +demo delivery vehicle are already approved. Timing instrumentation and the +post-beta feedback survey exist and will continue unchanged. +`, + 'docs/current-contracts.md': `# Existing SDK contracts + +On a developer's first local evaluation, the SDK requires a successful remote +CI check and blocks for five minutes before returning an evaluation result. +There is no skip flag or offline first-run path. The beta plan retains this gate. + +Authentication errors behave exactly as documented in docs/api.md. All other +errors already identify the cause, relevant argument or file, and an actionable +fix. Errors redact secrets. API timeouts, cancellation, rate limits, and retries +are bounded and documented; evaluation IDs prevent duplicate submitted jobs. + +The SDK supports Python 3.10+, macOS, Linux, and Windows without Docker. Its +type annotations, offline sample data, mock transport, noninteractive CI mode, +API reference, support contact, changelog, and contributor guide already work. +Telemetry is opt-in. No new hosted service, language binding, or community +program is proposed in this release. +`, + 'docs/api.md': `# Public API retained by the beta plan + +The two evaluation functions accept positional arguments: + +- \`run_eval(dataset, evaluator)\` +- \`run_batch(evaluator, dataset)\` + +Both argument names describe the same concepts. The reversed positional order +is intentional in the current draft; neither function requires keyword arguments. + +For an invalid API key, the SDK raises \`AuthError("request failed")\`. +There is no authentication error code, explanation of the cause, or instruction +for replacing the key. The plan retains this message. + +Version 1 exposes \`Client.evaluate()\`. Version 2 replaces it with +\`Client.run()\` and removes the old name immediately. No compatibility alias, +deprecation warning, migration guide, or codemod is provided. Other public APIs +retain their existing behavior, and the release changelog is otherwise complete. +`, + 'docs/package-contents.txt': `Published evalkit 2.0.0b1 package inventory: +evalkit/__init__.py +evalkit/client.py +evalkit/demo.py +evalkit/sample_responses.json +README.md + +The quickstart references examples/first_eval.py, but that file is absent from +both the published package and the release examples archive. The terminal demo +module and its sample data are included and work as documented. +`, +}; + +export function planDevexCountFixture(planPath: string): string { + return [ + `Please review this plan thoroughly. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`, + '', + '# Plan: EvalKit SDK beta release polish', + '', + 'The primary developer, onboarding benchmark, and terminal demo experience', + 'are settled in README.md and docs/benchmarks.md. Use DX POLISH for the', + 'existing release scope. Review the actual documented contracts and proposed', + 'behavior, including the first-run CI requirement, public function signatures,', + 'authentication error, packaged quickstart, and v1-to-v2 client upgrade.', + '', + 'The current draft ships the behavior in docs/current-contracts.md and', + 'docs/api.md unchanged, using the package inventory in docs/package-contents.txt.', + 'Recommendations that repair those developer-facing contracts belong in this', + 'plan. Existing working contracts remain the baseline for the review.', + ].join('\n'); +} + +function questionRecords(fp: AskUserQuestionFingerprint, answeredOnly = false): Array<{ header: string; question: string }> { + if (!fp.nativeCall) return [{ header: '', question: fp.promptSnippet }]; + return fp.nativeCall.questions.filter(q => !answeredOnly + || (fp.nativeCall!.answered && Boolean(fp.nativeCall!.answers?.[q.question]))); +} + +const ADMINISTRATIVE_HEADERS = new Set([ + 'design doc', 'prerequisite', 'routing rules', 'routing setup', 'cross-project', + 'target persona', 'developer persona', 'persona selection', 'empathy check', + 'narrative check', 'tthw target', 'competitive benchmark', 'benchmark confirmation', + 'magic delivery', 'review mode', 'fix scope', 'confusion scope', +]); + +function administrativeQuestion(header: string, question: string): boolean { + // These decisions establish the review's evidence and scope. Mentioning a + // defect in their recap does not turn a confirmation into a finding. + if (ADMINISTRATIVE_HEADERS.has(header.toLowerCase().replace(/\s+/g, ' ').trim())) return true; + const id = [...question.matchAll(/]+)>/gi)].at(-1)?.[1]; + if (id && /^(?:routing-injection|cross-project-learnings|plan-devex-review-(?:office-hours-preflight|prereq|persona|empathy(?:-check|-narrative)?|tthw-tier|competitive-tier|benchmark-tier|magical-moment|mode|confusion-report))$/i.test(id)) return true; + return /how deep should this dx review|which (?:dx )?review mode|\b(?:can|shall|should) we (?:continue|proceed|begin)(?: (?:the )?(?:setup|review)| now)?\?\s*$/i.test(question); +} + +/** The answered native call proves a decision; its content must identify a concrete problem. */ +function substantiveIssue({ header, question }: { header: string; question: string }): boolean { + if (administrativeQuestion(header, question)) return false; + const normalized = `${header} ${question}`.replace(/\s+/g, ' '); + const ciGate = /\b(?:CI|continuous integration)\b/i.test(normalized) + && /\b(?:first[- ]run|first eval(?:uation)?|local eval(?:uation)?|hello world)\b/i.test(normalized) + && /\b(?:mandatory|required|blocks?|five[- ]minute|5[- ]min(?:ute)?|wait|gate)\b/i.test(normalized); + const argumentsReversed = /\brun_eval\b/i.test(normalized) && /\brun_batch\b/i.test(normalized) + && /\b(?:revers\w*|inconsisten\w*|swapp\w*|different|order|positional)\b/i.test(normalized); + const opaqueAuth = /\b(?:AuthError|API[- ]?key|authentication|invalid key)\b/i.test(normalized) + && /request failed|\b(?:opaque|generic|unactionable|cryptic)\b|no (?:cause|guidance|fix|explanation|instruction)|doesn.t (?:explain|guide)/i.test(normalized); + const missingExample = /examples\/first_eval\.py|\b(?:packaged|quickstart|quick-start) example\b/i.test(normalized) + && /\b(?:missing|absent|omitted|FileNotFoundError)\b|not (?:included|packaged|shipped)|doesn.t (?:exist|ship)/i.test(normalized); + const breakingRename = /Client\.evaluate|Client\.run|\bmethod rename\b/i.test(normalized) + && /\b(?:breaking|remov\w*|renam\w*)\b/i.test(normalized) + && /\b(?:migration|deprecation|compatibility|alias|codemod)\b/i.test(normalized); + return ciGate || argumentsReversed || opaqueAuth || missingExample || breakingRename; +} + +/** A batched native call remains one decision; the caller owns call-ID deduplication. */ +export function isDevexReviewIssue(fp: AskUserQuestionFingerprint): boolean { + return questionRecords(fp, true).some(substantiveIssue); +} + +/** Select POLISH only on the recognized mode menu; leave all other answers unchanged. */ +export function devexReviewModePick(fp: AskUserQuestionFingerprint): number | null { + if (fp.nativeCall && fp.nativeCall.questions.length !== 1) return null; + const record = questionRecords(fp)[0]; + const text = record ? `${record.header} ${record.question}` : ''; + if (!//i.test(text) + && !/how\s*deep\s*should\s*this\s*dx\s*review|which\s*(?:dx\s*)?review\s*mode/i.test(text)) return null; + const modes = fp.options.map(option => ({ + index: option.index, + mode: /^(?:[A-C][.)])?DX(POLISH|EXPANSION|TRIAGE)(?:$|[^A-Z])/.exec( + option.label.split(/[│┌\r\n]/, 1)[0]!.replace(/\s+/g, '').toUpperCase(), + )?.[1], + })); + if (!['POLISH', 'EXPANSION', 'TRIAGE'].every(mode => modes.filter(option => option.mode === mode).length === 1)) return null; + return modes.find(option => option.mode === 'POLISH')!.index; +} diff --git a/test/helpers/plan-count-artifacts.ts b/test/helpers/plan-count-artifacts.ts new file mode 100644 index 000000000..3568fbdb4 --- /dev/null +++ b/test/helpers/plan-count-artifacts.ts @@ -0,0 +1,51 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { getProjectEvalDir } from './eval-store'; + +interface PlanCountSnapshot { + skillName: string; + observation: object; + raw: string; + visible: string; + cwd: string; + claudeConfigDir: string | null; +} + +/** One owned directory per count attempt; periodic captures replace files atomically. */ +export function createPlanCountSnapshotWriter(env: NodeJS.ProcessEnv = process.env): + (input: PlanCountSnapshot) => { artifactDir?: string; artifactError?: string } { + let artifactDir: string | undefined; + return (input) => { + if (!env.EVALS_RUN_ID) return {}; + try { + if (!artifactDir) { + const segment = (text: string) => text.replace(/[^a-zA-Z0-9_-]/g, '_').slice(0, 120) || 'run'; + const root = path.resolve(env.GSTACK_EVAL_DIR || getProjectEvalDir(), 'pty-count', segment(env.EVALS_RUN_ID)); + fs.mkdirSync(root, { recursive: true, mode: 0o700 }); + artifactDir = fs.mkdtempSync(path.join(root, `${segment(input.skillName)}-${Date.now()}-`)); + } + const write = (name: string, content: string) => { + const target = path.join(artifactDir!, name); + fs.writeFileSync(`${target}.tmp`, content, { mode: 0o600 }); + fs.renameSync(`${target}.tmp`, target); + }; + write('terminal.raw.log', input.raw); + write('terminal.visible.log', input.visible); + write('observation.json', JSON.stringify({ + ...input.observation, artifactDir, + capture: { skill: input.skillName, runId: env.EVALS_RUN_ID, cwd: input.cwd, + claudeConfigDir: input.claudeConfigDir, at: new Date().toISOString() }, + }, null, 2) + '\n'); + return { artifactDir }; + } catch (error) { + // Preserve any partial evidence and the original test outcome; make the + // write failure visible instead of claiming diagnostics were retained. + return { artifactDir, artifactError: String(error) }; + } + }; +} + +/** Keep a single snapshot outside the temporary fixture that setup later removes. */ +export function persistPlanCountSnapshot(input: PlanCountSnapshot, env: NodeJS.ProcessEnv = process.env) { + return createPlanCountSnapshotWriter(env)(input); +} diff --git a/test/helpers/plan-count-fixture.ts b/test/helpers/plan-count-fixture.ts index 0529f7f39..78333de25 100644 --- a/test/helpers/plan-count-fixture.ts +++ b/test/helpers/plan-count-fixture.ts @@ -10,7 +10,7 @@ import { getHermeticDirs } from './hermetic-env'; * bare slash command starts: a later message can remain queued behind the * skill's first AskUserQuestion and leave it reviewing the live branch. */ -export function createPlanCountFixture(prompt: string, opts: { nativeReviewOnly?: boolean } = {}): { +export function createPlanCountFixture(prompt: string, opts: { nativeReviewOnly?: boolean; files?: Record } = {}): { cwd: string; env: Record; cleanup(): void; @@ -26,6 +26,17 @@ export function createPlanCountFixture(prompt: string, opts: { nativeReviewOnly? } }; try { + const files = Object.entries(opts.files ?? {}); + for (const [name] of files) { + const parts = name.split(/[\\/]/); + // The fixture owns its seed plan, instructions and Git metadata. Extra + // context must be an ordinary relative file, never an overwrite/escape. + if (path.isAbsolute(name) || path.win32.isAbsolute(name) || parts.some(part => + part === '' || part === '.' || part === '..' || part.toLowerCase() === '.git') || + /^(?:plan|claude)\.md$/i.test(name)) { + throw new Error(`Invalid plan-count fixture file: ${name}`); + } + } if (opts.nativeReviewOnly) { // Seeded-N count bands measure the main review's finding cadence. // An outside review can legitimately add findings beyond that band; @@ -63,6 +74,11 @@ export function createPlanCountFixture(prompt: string, opts: { nativeReviewOnly? prompt, '', ].join('\n')); + for (const [name, content] of files) { + const target = path.join(cwd, name); + fs.mkdirSync(path.dirname(target), { recursive: true }); + fs.writeFileSync(target, content); + } const git = (args: string[]) => { const result = spawnSync('git', args, { @@ -75,7 +91,7 @@ export function createPlanCountFixture(prompt: string, opts: { nativeReviewOnly? } }; git(['init', '-b', 'main']); - git(['add', 'PLAN.md', 'CLAUDE.md']); + git(['add', '--', 'PLAN.md', 'CLAUDE.md', ...files.map(([name]) => name)]); git(['-c', 'user.name=Plan Count Fixture', '-c', 'user.email=plan-count@example.test', '-c', 'commit.gpgsign=false', 'commit', '--no-verify', '-m', 'Seed review plan']); git(['update-ref', 'refs/remotes/origin/main', 'HEAD']); diff --git a/test/helpers/plan-count-transcript.ts b/test/helpers/plan-count-transcript.ts new file mode 100644 index 000000000..c1de9670d --- /dev/null +++ b/test/helpers/plan-count-transcript.ts @@ -0,0 +1,133 @@ +/** Lossless, read-only question metadata from one isolated Claude fixture. */ +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +export interface NativePlanQuestion { + header: string; + question: string; + options: Array<{ label: string; description?: string }>; + multiSelect?: boolean; +} + +export interface NativePlanQuestionCall { + sessionId: string; + toolUseId: string; + questions: NativePlanQuestion[]; + answered: boolean; + failed?: boolean; + failure?: string; + answers?: Record; + unansweredQuestionIndices?: number[]; + answeredAt?: string; +} + +export interface PlanCountTranscript { + status: 'missing' | 'ready' | 'error'; + calls: NativePlanQuestionCall[]; + assistantMessages: Array<{ sessionId: string; text: string; timestamp: string }>; + error?: string; +} + +/** A rejected/refused call needs an actual later answer, not unrelated progress. */ +export function unresolvedPlanQuestionCalls(calls: NativePlanQuestionCall[]): NativePlanQuestionCall[] { + return calls.filter((call, index) => call.failed && !call.questions.every(q => + calls.slice(index + 1).some(later => later.answered && later.answers?.[q.question]))); +} + +const MAX_BYTES = 32 * 1024 * 1024; +const MAX_FILES = 64; +const object = (value: unknown): value is Record => + value !== null && typeof value === 'object' && !Array.isArray(value); +const validTimestamp = (value: unknown): value is string => + typeof value === 'string' && Number.isFinite(Date.parse(value)); + +function validQuestions(value: unknown): value is NativePlanQuestion[] { + return Array.isArray(value) && value.length > 0 && value.every(q => + object(q) && typeof q.header === 'string' && typeof q.question === 'string' && q.question.trim() && + Array.isArray(q.options) && q.options.length >= 2 && q.options.every((o: unknown) => + object(o) && typeof o.label === 'string' && o.label.trim())); +} + +/** + * Count callers consume each answered (sessionId, toolUseId) once, regardless + * of questions[].length. A batched tool call must never become N findings. + * Partial final lines remain pending; missing/foreign/sidechain records add + * no coverage. Traversal stays inside the owned config's projects directory. + */ +export function readPlanCountTranscript(configDir: string, cwd: string): PlanCountTranscript { + const calls = new Map(); + const assistantMessages: PlanCountTranscript['assistantMessages'] = []; + let matched = false; + let bytes = 0; + let files = 0; + const projects = path.join(configDir, 'projects'); + try { + if (!fs.existsSync(projects)) return { status: 'missing', calls: [], assistantMessages: [] }; + const dirs = fs.readdirSync(projects, { withFileTypes: true }).filter(d => d.isDirectory()); + if (dirs.length > MAX_FILES) throw new Error('too many project directories'); + for (const dir of dirs) { + const project = path.join(projects, dir.name); + for (const entry of fs.readdirSync(project, { withFileTypes: true })) { + if (!entry.isFile() || !entry.name.endsWith('.jsonl')) continue; + if (++files > MAX_FILES) throw new Error('too many transcript files'); + const file = path.join(project, entry.name); + bytes += fs.statSync(file).size; + if (bytes > MAX_BYTES) throw new Error('transcript exceeds 32 MiB read limit'); + const text = fs.readFileSync(file, 'utf8'); + // Claude appends JSONL during rendering; an unfinished record is not + // evidence of a call or an answer until its newline has been written. + for (const line of text.slice(0, text.lastIndexOf('\n') + 1).split('\n')) { + if (!line.trim()) continue; + const record = JSON.parse(line); + if (!object(record) || record.cwd !== cwd || record.isSidechain !== false || + typeof record.sessionId !== 'string' || entry.name !== `${record.sessionId}.jsonl` || + !object(record.message) || !Array.isArray(record.message.content)) continue; + matched = true; + for (const block of record.message.content) { + if (!object(block)) continue; + if (record.message.role === 'assistant' && block.type === 'text' && + typeof block.text === 'string' && block.text.trim() && validTimestamp(record.timestamp)) { + assistantMessages.push({ sessionId: record.sessionId, text: block.text, timestamp: record.timestamp }); + } + if (record.message.role === 'assistant' && block.type === 'tool_use' && block.name === 'AskUserQuestion' && + typeof block.id === 'string' && object(block.input) && validQuestions(block.input.questions)) { + const key = `${record.sessionId}:${block.id}`; + const prior = calls.get(key); + if (prior && JSON.stringify(prior.questions) !== JSON.stringify(block.input.questions)) { + throw new Error('conflicting question metadata for one tool call'); + } + if (!prior) calls.set(key, { sessionId: record.sessionId, toolUseId: block.id, + questions: block.input.questions, answered: false }); + } else if (record.message.role === 'user' && block.type === 'tool_result' && + typeof block.tool_use_id === 'string') { + const call = calls.get(`${record.sessionId}:${block.tool_use_id}`); + const answers = record.toolUseResult?.answers; + const validAnswers = call && object(answers) ? Object.fromEntries(call.questions + .filter(q => typeof answers[q.question] === 'string' && answers[q.question].trim()) + .map(q => [q.question, answers[q.question]])) : {}; + if (call && block.is_error !== true && Object.keys(validAnswers).length > 0) { + // The CLI allows submitting a multi-question packet with + // unanswered tabs. This completes ONE call, not N questions. + call.answered = true; + call.failed = false; + delete call.failure; + call.answers = validAnswers; + call.unansweredQuestionIndices = call.questions.flatMap((q, i) => q.question in validAnswers ? [] : [i]); + call.answeredAt = validTimestamp(record.timestamp) ? record.timestamp : undefined; + } else if (call) { + if (call.answered) throw new Error('conflicting successful and failed results for one question call'); + call.failed = true; + call.failure = block.is_error === true ? 'Native question tool returned is_error' : 'Native question returned no matching nonempty answers'; + } + } + } + } + } + } + return { status: matched ? 'ready' : 'missing', calls: [...calls.values()], assistantMessages }; + } catch (error) { + // A failed read cannot silently turn an incomplete transcript into a + // complete review. Keep the diagnostic explicit and return no coverage. + return { status: 'error', calls: [], assistantMessages: [], error: `Claude question transcript: ${String(error)}` }; + } +} diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 627c15ea7..39f658450 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -160,10 +160,10 @@ export const E2E_TOUCHFILES: Record = { // Each test drives its skill end-to-end; touchfiles include preamble + // completion-status resolvers because they affect question cadence and // terminal output (the regression surface this test catches). - 'plan-ceo-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-ceo-review/**', '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/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-finding-count.test.ts'], - '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/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-eng-finding-count.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/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-design-finding-count.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/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts'], + 'plan-ceo-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-ceo-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/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-finding-count.test.ts'], + '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/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-eng-finding-count.test.ts'], + '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/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-design-finding-count.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/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts'], // Gate-tier reviewCount-floor counterparts. Catch the May 2026 transcript // bug (model wrote a plan-mode plan and ExitPlanMode'd without firing any @@ -180,8 +180,8 @@ export const E2E_TOUCHFILES: Record = { // a model fires one AUQ then batches the rest into a "## Decisions to // confirm" plan write. runPlanSkillFloorCheck cannot detect that shape // (it exits on first AUQ); runPlanSkillCounting can. - 'plan-eng-multi-finding-batching': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts'], - 'plan-ceo-split-overflow': ['bin/gstack-config', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts'], + 'plan-eng-multi-finding-batching': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/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/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts'], + 'plan-ceo-split-overflow': ['bin/gstack-config', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/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/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts'], 'brain-privacy-gate': ['bin/gstack-skill-start', 'bin/gstack-skill-end', 'scripts/resolvers/preamble/generate-brain-sync-block.ts', 'scripts/resolvers/preamble.ts', 'bin/gstack-brain-sync', 'bin/gstack-artifacts-init', 'bin/gstack-config', 'test/helpers/agent-sdk-runner.ts', 'test/skill-e2e-brain-privacy-gate.test.ts'], // /setup-gbrain Path 4 (Remote MCP) — happy + bad-token end-to-end via diff --git a/test/plan-count-artifacts.test.ts b/test/plan-count-artifacts.test.ts new file mode 100644 index 000000000..95bc20e95 --- /dev/null +++ b/test/plan-count-artifacts.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { createPlanCountSnapshotWriter, persistPlanCountSnapshot } from './helpers/plan-count-artifacts'; + +const input = { + skillName: 'plan-design-review', observation: { outcome: 'timeout', reviewCount: 3 }, + raw: '\x1b[2JSTART\r' + 'raw-frame\r'.repeat(2000) + '\x1b[31mEND', + visible: 'START\r' + 'visible-frame\r'.repeat(2000) + 'END', + cwd: '/temporary/count-fixture', claudeConfigDir: '/temporary/claude-config', +}; + +describe('plan-count diagnostic artifacts', () => { + test('refreshes one in-progress attempt through final outcome without keeping old copies', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-count-artifacts-')); + try { + const save = createPlanCountSnapshotWriter({ EVALS_RUN_ID: 'checkpoint', GSTACK_EVAL_DIR: root }); + const progress = save({ ...input, observation: { state: 'in_progress', reviewCount: 1 } }); + const recordPath = path.join(progress.artifactDir!, 'observation.json'); + expect(JSON.parse(fs.readFileSync(recordPath, 'utf8'))).toMatchObject({ state: 'in_progress', reviewCount: 1 }); + const final = save({ ...input, raw: input.raw + '\nlast question', observation: { outcome: 'completion_summary', reviewCount: 5 } }); + expect(final.artifactDir).toBe(progress.artifactDir); + const record = JSON.parse(fs.readFileSync(recordPath, 'utf8')); + expect(record).toMatchObject({ outcome: 'completion_summary', reviewCount: 5 }); + expect(record.state).toBeUndefined(); + expect(fs.readdirSync(final.artifactDir!).sort()).toEqual(['observation.json', 'terminal.raw.log', 'terminal.visible.log']); + expect(fs.readFileSync(path.join(final.artifactDir!, 'terminal.raw.log'), 'utf8')).toBe(input.raw + '\nlast question'); + } finally { fs.rmSync(root, { recursive: true, force: true }); } + }); + test('keeps complete output and distinct retry records in the configured eval directory', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-count-artifacts-')); + try { + const env = { EVALS_RUN_ID: 'same-run', GSTACK_EVAL_DIR: root }; + const first = persistPlanCountSnapshot(input, env); + const second = persistPlanCountSnapshot({ ...input, raw: 'retry' }, env); + expect(first.artifactError).toBeUndefined(); + expect(second.artifactError).toBeUndefined(); + expect(first.artifactDir).not.toBe(second.artifactDir); + expect(path.relative(root, first.artifactDir!)).toStartWith(path.join('pty-count', 'same-run')); + expect(fs.readFileSync(path.join(first.artifactDir!, 'terminal.raw.log'), 'utf8')).toBe(input.raw); + expect(fs.readFileSync(path.join(first.artifactDir!, 'terminal.visible.log'), 'utf8')).toBe(input.visible); + const record = JSON.parse(fs.readFileSync(path.join(first.artifactDir!, 'observation.json'), 'utf8')); + expect(record).toMatchObject({ ...input.observation, artifactDir: first.artifactDir, + capture: { skill: input.skillName, cwd: input.cwd, claudeConfigDir: input.claudeConfigDir } }); + expect(fs.readFileSync(path.join(second.artifactDir!, 'terminal.raw.log'), 'utf8')).toBe('retry'); + if (process.platform !== 'win32') { + expect(fs.statSync(path.join(first.artifactDir!, 'terminal.raw.log')).mode & 0o777).toBe(0o600); + } + } finally { fs.rmSync(root, { recursive: true, force: true }); } + }); + + test('does not persist ordinary free runs without EVALS_RUN_ID', () => { + const missing = path.join(os.tmpdir(), `unused-count-artifacts-${crypto.randomUUID()}`); + expect(persistPlanCountSnapshot(input, { GSTACK_EVAL_DIR: missing })).toEqual({}); + expect(fs.existsSync(missing)).toBe(false); + }); + + test('keeps run and skill identifiers within the owned artifact directory', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-count-artifacts-')); + try { + const saved = persistPlanCountSnapshot({ ...input, skillName: '../../other' }, { + EVALS_RUN_ID: '../outside', GSTACK_EVAL_DIR: root, + }); + expect(saved.artifactError).toBeUndefined(); + expect(path.relative(root, saved.artifactDir!)).not.toStartWith('..'); + } finally { fs.rmSync(root, { recursive: true, force: true }); } + }); + + test('reports a diagnostic write failure without replacing the original observation', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-count-artifacts-')); + const file = path.join(root, 'file'); + fs.writeFileSync(file, 'unchanged'); + try { + const result = persistPlanCountSnapshot(input, { EVALS_RUN_ID: 'run', GSTACK_EVAL_DIR: file }); + expect(result.artifactError).toBeDefined(); + expect(input.observation).toEqual({ outcome: 'timeout', reviewCount: 3 }); + expect(fs.readFileSync(file, 'utf8')).toBe('unchanged'); + } finally { fs.rmSync(root, { recursive: true, force: true }); } + }); +}); diff --git a/test/plan-count-fixture.test.ts b/test/plan-count-fixture.test.ts index 82ca00fe1..ee003eb60 100644 --- a/test/plan-count-fixture.test.ts +++ b/test/plan-count-fixture.test.ts @@ -83,6 +83,37 @@ describe('plan-count fixtures', () => { expect(fs.existsSync(second.cwd)).toBe(false); }); + test('additional design context exists in the initial clean fixture commit', () => { + const design = '# Approved design system\nUse the existing components.\n' + PROMPT; + const fixture = createPlanCountFixture(PROMPT, { files: { 'DESIGN.md': design, 'docs/accepted behavior.md': 'Save is atomic.\n' } }); + try { + const committed = spawnSync('git', ['show', 'HEAD:DESIGN.md'], { cwd: fixture.cwd, encoding: 'utf8', timeout: 10_000 }); + expect(committed.status, committed.stderr).toBe(0); + expect(committed.stdout).toBe(design); + expect(fs.readFileSync(path.join(fixture.cwd, 'docs/accepted behavior.md'), 'utf8')).toBe('Save is atomic.\n'); + expect(spawnSync('git', ['status', '--porcelain'], { cwd: fixture.cwd, encoding: 'utf8', timeout: 10_000 }).stdout).toBe(''); + } finally { + fixture.cleanup(); + } + expect(fs.existsSync(fixture.cwd)).toBe(false); + }); + + test('additional context cannot escape the fixture or replace its instructions and Git metadata', () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-count-outside-')); + const sentinel = path.join(outside, 'untouched.md'); + fs.writeFileSync(sentinel, 'user-owned'); + try { + for (const name of [sentinel, '../escape.md', 'nested/../escape.md', '..\\escape.md', + 'C:\\escape.md', '/escape.md', '.git/config', 'nested/.git/config', 'PLAN.md', 'CLAUDE.md']) { + expect(() => createPlanCountFixture(PROMPT, { files: { [name]: 'overwrite' } })) + .toThrow('Invalid plan-count fixture file:'); + } + expect(fs.readFileSync(sentinel, 'utf8')).toBe('user-owned'); + } finally { + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + test.skipIf(process.platform === 'win32')('cleans the fixture when the PTY executable cannot launch', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-count-launch-failure-')); const fixtureTmp = path.join(dir, 'fixtures'); @@ -133,11 +164,19 @@ try { fs.mkdirSync(hostState); fs.writeFileSync(path.join(hostState, 'config.yaml'), hostConfig); const cases = [ - { name: 'design', skillName: 'plan-design-review', prompt: PROMPT, mode: 'complete' }, + { name: 'design', skillName: 'plan-design-review', prompt: PROMPT, mode: 'complete', files: { 'DESIGN.md': '# Approved design\nKeep the existing layout.\n' } }, + { name: 'design-direct', skillName: 'plan-design-review', prompt: PROMPT, mode: 'direct-finding' }, + { name: 'design-batched', skillName: 'plan-design-review', prompt: PROMPT, mode: 'batched-finding' }, + { name: 'failed-native', skillName: 'plan-design-review', prompt: PROMPT, mode: 'failed-call' }, + { name: 'permission', skillName: 'plan-design-review', prompt: PROMPT, mode: 'permission' }, + { name: 'missing-transcript', skillName: 'plan-design-review', prompt: PROMPT, mode: 'missing-transcript' }, { name: 'ceo', skillName: 'plan-ceo-review', prompt: '# Independent CEO plan\nUnique product context.', mode: 'complete' }, { name: 'exited', skillName: 'plan-eng-review', prompt: '# Early-exit plan\nStill clean up.', mode: 'exit' }, { name: 'skip-first', skillName: 'plan-devex-review', prompt: '# Native DX plan', mode: 'prerequisite', skipIndex: 1 }, { name: 'skip-second', skillName: 'plan-devex-review', prompt: '# Native DX plan', mode: 'prerequisite', skipIndex: 2 }, + { name: 'caller-policy', skillName: 'plan-devex-review', prompt: '# Native DX plan', mode: 'prerequisite', skipIndex: 2, custom: true }, + { name: 'late-mode', skillName: 'plan-devex-review', prompt: '# Native DX plan', mode: 'late-mode' }, + { name: 'batched-mode', skillName: 'plan-devex-review', prompt: '# Native DX plan', mode: 'batched-mode' }, ].map((item) => ({ ...item, record: path.join(dir, `${item.name}.jsonl`) })); // The fake snapshots its environment BEFORE installing the first stdin @@ -147,6 +186,26 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { spawnSync } from 'node:child_process'; const record = (event) => fs.appendFileSync(process.env.FIXTURE_RECORD, JSON.stringify(event) + '\n'); +const sessionId = 'fixture-' + process.pid; +const project = path.join(process.env.CLAUDE_CONFIG_DIR, 'projects', sessionId); +fs.mkdirSync(project, { recursive: true }); +const native = (role, content, extra = {}) => { + if (process.env.FIXTURE_MODE === 'missing-transcript') return; + fs.appendFileSync(path.join(project, sessionId + '.jsonl'), JSON.stringify({ + cwd: process.cwd(), sessionId, isSidechain: false, message: { role, content }, ...extra, + }) + '\n'); +}; +native('assistant', [{ type: 'text', text: 'Fixture CLI started.' }]); +let callId = 0; +let nativeQuestions = []; +const ask = (questions) => { + nativeQuestions = questions; + native('assistant', [{ type: 'tool_use', id: 'question-' + (++callId), name: 'AskUserQuestion', input: { questions } }]); +}; +const answer = () => native('user', [{ type: 'tool_result', tool_use_id: 'question-' + callId, content: 'Your questions have been answered.' }], + { toolUseResult: { answers: Object.fromEntries(nativeQuestions.map(q => [q.question, q.options[0].label])) } }); +const questionMetadata = (header, question, labels) => ({ header, question, options: labels.map(label => ({ label })) }); +const modeQuestion = questionMetadata('Review mode', 'How deep should this DX review go? ', ['DX EXPANSION', 'DX POLISH', 'DX TRIAGE']); const skillDir = path.join(process.env.CLAUDE_CONFIG_DIR, 'skills', process.env.FIXTURE_SKILL); const planPath = path.join(process.cwd(), 'PLAN.md'); const contextPath = path.join(process.cwd(), 'CLAUDE.md'); @@ -164,6 +223,7 @@ record({ plan: fs.existsSync(planPath) ? fs.readFileSync(planPath, 'utf8') : null, context: fs.existsSync(contextPath) ? fs.readFileSync(contextPath, 'utf8') : null, inheritedDesign: fs.existsSync('DESIGN.md'), inheritedTodos: fs.existsSync('TODOS.md'), + design: fs.existsSync('DESIGN.md') ? fs.readFileSync('DESIGN.md', 'utf8') : null, skill: fs.existsSync(path.join(skillDir, 'SKILL.md')) ? fs.readFileSync(path.join(skillDir, 'SKILL.md'), 'utf8') : null, sections: fs.existsSync(path.join(skillDir, 'sections', 'review-sections.md')) ? fs.readFileSync(path.join(skillDir, 'sections', 'review-sections.md'), 'utf8') : null, @@ -179,17 +239,54 @@ process.stdin.on('data', (data) => { const input = data.toString('utf8'); record({ type: 'input', data: input }); if (!firstInput) { + if (['late-mode', 'batched-mode'].includes(process.env.FIXTURE_MODE)) { + selected += input.replace(/\r/g, ''); + if (input.includes('\r')) { + // The real CLI can defer its native call record until after the UI + // answer. A known multi-question call must still block the override. + if (process.env.FIXTURE_MODE === 'late-mode') ask([modeQuestion]); + const label = modeQuestion.options[Number(selected) - 1]?.label; + record({ type: 'mode-answer', selected, label }); + native('user', [{ type: 'tool_result', tool_use_id: 'question-' + callId, content: 'Your questions have been answered.' }], + { toolUseResult: { answers: { [modeQuestion.question]: label } } }); + process.stdout.write('\nGSTACK REVIEW REPORT\n'); + } + return; + } + if (['direct-finding', 'failed-call'].includes(process.env.FIXTURE_MODE) && input.includes('\r')) { + answer(); + process.stdout.write('\nGSTACK REVIEW REPORT\n'); + return; + } + if (process.env.FIXTURE_MODE === 'batched-finding' && input.includes('\r')) { + if (question === 1) { + question = 2; + process.stdout.write('\r☐Loading\rD2 — Define the loading state \r❯1.Add spinner\r2.Keep blank\r'); + } else { + answer(); + process.stdout.write('\nGSTACK REVIEW REPORT\n'); + } + return; + } + if (process.env.FIXTURE_MODE === 'permission' && input.includes('\r')) { + process.stdout.write('\nGSTACK REVIEW REPORT\n'); + return; + } if (process.env.FIXTURE_MODE !== 'prerequisite') return; if (question === 1 && input.includes('\r')) { + answer(); record({ type: 'generic-answer', input }); question = 2; + selected = ''; const labels = ['Run /office-hours now', 'Skip — proceed with standard review']; if (process.env.FIXTURE_SKIP_INDEX === '1') labels.reverse(); + ask([questionMetadata('Prerequisite', 'No design doc found. Run /office-hours first?', labels)]); process.stdout.write('\r☐ Prerequisite\rNo design doc found. Run /office-hours first?\r❯1.' + labels[0] + '\r2.' + labels[1] + '\r'); return; } selected += input.replace(/\r/g, ''); if (input.includes('\r')) { + answer(); record({ type: 'prerequisite-answer', selected }); process.stdout.write(selected === process.env.FIXTURE_SKIP_INDEX ? '\nGSTACK REVIEW REPORT\n' : '\nWRONG_PREREQUISITE_CHOICE\n'); } @@ -197,11 +294,35 @@ process.stdin.on('data', (data) => { } firstInput = false; if (process.env.FIXTURE_MODE === 'exit') process.exit(7); + if (['late-mode', 'batched-mode'].includes(process.env.FIXTURE_MODE)) { + if (process.env.FIXTURE_MODE === 'batched-mode') ask([modeQuestion, + questionMetadata('Separate decision', 'Which fixture target should be used?', ['First target', 'Second target'])]); + process.stdout.write('\r☐ Review mode\r' + modeQuestion.question + '\r❯1.DX EXPANSION\r2.DX POLISH\r3.DX TRIAGE\r'); + return; + } + if (['direct-finding', 'batched-finding', 'failed-call'].includes(process.env.FIXTURE_MODE)) { + question = 1; + if (process.env.FIXTURE_MODE === 'failed-call') { + ask([questionMetadata('Missing answer', 'Should the save retry be idempotent?', ['Yes', 'No'])]); + native('user', [{ type: 'tool_result', tool_use_id: 'question-' + callId, is_error: true, content: 'Question rejected' }]); + } + const questions = [questionMetadata('Button style', 'D1 — How should the four header buttons differ? ', ['Filled primary', 'Ghost buttons'])]; + if (process.env.FIXTURE_MODE === 'batched-finding') questions.push(questionMetadata('Loading', 'D2 — Define the loading state ', ['Add spinner', 'Keep blank'])); + ask(questions); + process.stdout.write('\r☐Buttonstyle\r│D1—Howshouldthe4headerbuttonsbedifferentiated?\r❯1.Filledprimary\r2.Ghostbuttons\r'); + return; + } if (process.env.FIXTURE_MODE === 'prerequisite') { question = 1; + const suffix = process.env.FIXTURE_CUSTOM === 'true' ? ' Context '.repeat(50) + 'routing-proof-after-240' : ''; + ask([questionMetadata('Setup', 'Which fixture setup should be used?' + suffix, ['First setup', 'Second setup'])]); process.stdout.write('\r☐ Setup\rWhich fixture setup should be used?\r❯1.First setup\r2.Second setup\r'); return; } + if (process.env.FIXTURE_MODE === 'permission') { + process.stdout.write('\rDo you want to create PLAN.md?\r❯1.Yes\r2.Yes, and switch to accept edits\r3.No\r'); + return; + } // Longer than the old 3 s delayed fixture send: record that regression // even if the helper would otherwise return on our completion marker. completion = setTimeout(() => process.stdout.write('\nGSTACK REVIEW REPORT\n'), 4100); @@ -216,9 +337,11 @@ process.stdin.resume(); fs.chmodSync(fakePath, 0o755); const runnerUrl = pathToFileURL(path.join(ROOT, 'test/helpers/claude-pty-runner.ts')).href; const hermeticUrl = pathToFileURL(path.join(ROOT, 'test/helpers/hermetic-env.ts')).href; + const devexUrl = pathToFileURL(path.join(ROOT, 'test/helpers/devex-count-fixture.ts')).href; fs.writeFileSync(workerPath, ` -import { runPlanSkillCounting } from ${JSON.stringify(runnerUrl)}; +import { runPlanSkillCounting, designFirstReviewAUQ } from ${JSON.stringify(runnerUrl)}; import { getHermeticDirs } from ${JSON.stringify(hermeticUrl)}; +import { devexReviewModePick } from ${JSON.stringify(devexUrl)}; import * as fs from 'node:fs'; import * as path from 'node:path'; const shared = getHermeticDirs().gstackHome; @@ -232,12 +355,18 @@ const results = await Promise.all(cases.map(async (item) => ({ skillName: item.skillName, slashCommand: '/' + item.skillName, followUpPrompt: item.prompt, + fixtureFiles: item.files, isLastStep0AUQ: () => false, + isFirstReviewAUQ: ['direct-finding', 'batched-finding', 'failed-call'].includes(item.mode) ? designFirstReviewAUQ : undefined, + isReviewAUQ: item.custom ? fp => fp.promptSnippet.includes('routing-proof-after-240') : undefined, + pickAUQ: ['late-mode', 'batched-mode'].includes(item.mode) ? devexReviewModePick + : item.custom ? fp => fp.promptSnippet.includes('routing-proof-after-240') ? 1 : null : undefined, reviewCountCeiling: 8, timeoutMs: 15000, - firstAUQPick: () => 2, + firstAUQPick: () => ['late-mode', 'batched-mode'].includes(item.mode) ? 1 : 2, env: { FIXTURE_RECORD: item.record, FIXTURE_SKILL: item.skillName, FIXTURE_MODE: item.mode, + FIXTURE_CUSTOM: String(item.custom ?? false), FIXTURE_SKIP_INDEX: String(item.skipIndex ?? ''), FIXTURE_CONFIG_BIN: ${JSON.stringify(path.join(ROOT, 'bin/gstack-config'))}, GSTACK_HOME: ${JSON.stringify(hostState)}, GSTACK_STATE_ROOT: ${JSON.stringify(hostState)}, }, @@ -252,7 +381,8 @@ await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify({ results, onboard const startedAt = Date.now(); const child = Bun.spawn([process.execPath, workerPath], { cwd: ROOT, - env: { ...process.env, BROWSE_TERMINAL_BINARY: fakePath, EVALS_HERMETIC: '1', GSTACK_HOME: hostState, GSTACK_STATE_ROOT: hostState }, + env: { ...process.env, BROWSE_TERMINAL_BINARY: fakePath, EVALS_HERMETIC: '1', GSTACK_HOME: hostState, GSTACK_STATE_ROOT: hostState, + EVALS_RUN_ID: 'fixture-integration', GSTACK_EVAL_DIR: path.join(dir, 'eval-artifacts') }, stdout: 'pipe', stderr: 'pipe', }); @@ -288,17 +418,43 @@ await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify({ results, onboard expect(startup.context).toContain('PLAN.md'); expect(startup.context).toContain(item.prompt); expect(startup.argv[startup.argv.indexOf('--permission-mode') + 1]).toBe('plan'); - expect(startup.inheritedDesign).toBe(false); + expect(startup.inheritedDesign).toBe(Boolean(item.files?.['DESIGN.md'])); + expect(startup.design).toBe(item.files?.['DESIGN.md'] ?? null); expect(startup.inheritedTodos).toBe(false); expect(startup.skill).toContain(`name: ${item.skillName}`); expect(startup.sections).toBe(fs.readFileSync(path.join(ROOT, item.skillName, 'sections/review-sections.md'), 'utf8')); expect(events.filter((event) => event.type === 'input').map((event) => event.data).join('')) - .toBe(`/${item.skillName}\r` + (item.mode === 'prerequisite' ? `2\r${item.skipIndex}\r` : '')); + .toBe(`/${item.skillName}\r` + (item.mode === 'prerequisite' ? `${item.custom ? 1 : 2}\r${item.skipIndex}\r` + : item.mode === 'batched-finding' ? '2\r1\r' : item.mode === 'batched-mode' ? '1\r' + : ['direct-finding', 'failed-call', 'permission', 'late-mode'].includes(item.mode) ? '2\r' : '')); expect(fs.existsSync(startup.cwd)).toBe(false); expect(fs.existsSync(startup.stateRoot)).toBe(false); expect(() => process.kill(startup.pid, 0)).toThrow(); const result = results.find((result) => result.name === item.name); - expect(result.observation.outcome).toBe(item.mode === 'exit' ? 'exited' : 'completion_summary'); + expect(result.observation.outcome, `${item.name}: ${JSON.stringify(result.observation)}`).toBe(item.mode === 'exit' ? 'exited' + : ['missing-transcript', 'failed-call'].includes(item.mode) ? 'transcript_unavailable' : 'completion_summary'); + const artifacts = result.observation.artifactDir; + expect(result.observation.artifactError).toBeUndefined(); + expect(fs.existsSync(artifacts)).toBe(true); // Survives the temporary fixture's cleanup. + const captured = JSON.parse(fs.readFileSync(path.join(artifacts, 'observation.json'), 'utf8')); + expect(captured.outcome).toBe(result.observation.outcome); + expect(captured.capture.cwd).toBe(startup.cwd); + expect(fs.readFileSync(path.join(artifacts, 'terminal.raw.log'), 'utf8')).toContain(item.mode === 'exit' ? 'STARTUP_DIAGNOSTIC' : 'GSTACK REVIEW REPORT'); + expect(fs.readFileSync(path.join(artifacts, 'terminal.visible.log'), 'utf8')).toContain(item.mode === 'exit' ? 'STARTUP_DIAGNOSTIC' : 'GSTACK REVIEW REPORT'); + if (['direct-finding', 'batched-finding', 'failed-call'].includes(item.mode)) { + expect(result.observation.reviewCount).toBe(1); + expect(result.observation.step0Count).toBe(0); + expect(result.observation.fingerprints).toHaveLength(1); + expect(result.observation.fingerprints[0].nativeCall.questions).toHaveLength(item.mode === 'batched-finding' ? 2 : 1); + } + if (item.mode === 'failed-call') { + expect(result.observation.transcript.calls[0].failure).toContain('is_error'); + expect(result.observation.transcript.calls[0].answered).toBe(false); + } + if (['permission', 'missing-transcript'].includes(item.mode)) { + expect(result.observation.reviewCount).toBe(0); + expect(result.observation.step0Count).toBe(0); + } if (item.mode === 'exit') { expect(result.observation.evidence).toContain('exitCode=7'); expect(result.observation.evidence).toContain('STARTUP_DIAGNOSTIC fixture CLI booted'); @@ -307,8 +463,19 @@ await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify({ results, onboard if (item.mode === 'complete') expect(events.at(-1).type).toBe('closed'); if (item.mode === 'prerequisite') { expect(events.find(event => event.type === 'prerequisite-answer').selected).toBe(String(item.skipIndex)); - expect(result.observation.step0Count).toBe(2); + expect(result.observation.step0Count).toBe(item.custom ? 1 : 2); + expect(result.observation.reviewCount).toBe(item.custom ? 1 : 0); + } + if (['late-mode', 'batched-mode'].includes(item.mode)) { + const expectedLabel = item.mode === 'late-mode' ? 'DX POLISH' : 'DX EXPANSION'; + expect(events.find(event => event.type === 'mode-answer').label).toBe(expectedLabel); + expect(result.observation.step0Count).toBe(1); expect(result.observation.reviewCount).toBe(0); + expect(result.observation.transcript.calls).toHaveLength(1); + const call = result.observation.transcript.calls[0]; + expect(call.answers[call.questions[0].question]).toBe(expectedLabel); + expect(call.questions).toHaveLength(item.mode === 'late-mode' ? 1 : 2); + expect(call.unansweredQuestionIndices).toEqual(item.mode === 'late-mode' ? [] : [1]); } cwds.add(startup.cwd); stateRoots.add(startup.stateRoot); diff --git a/test/plan-count-transcript.test.ts b/test/plan-count-transcript.test.ts new file mode 100644 index 000000000..f4cb7ff84 --- /dev/null +++ b/test/plan-count-transcript.test.ts @@ -0,0 +1,220 @@ +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 { readPlanCountTranscript, unresolvedPlanQuestionCalls } from './helpers/plan-count-transcript'; +import { nativePlanCallFingerprint, planCountQuestionPhase, engStep0Boundary, ceoStep0Boundary, ceoFirstReviewAUQ } from './helpers/claude-pty-runner'; + +const dirs: string[] = []; +afterEach(() => { for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); }); + +function fixture() { + const config = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-count-transcript-')); + dirs.push(config); + const cwd = path.join(config, 'fixture'); + const project = path.join(config, 'projects', 'fixture'); + fs.mkdirSync(project, { recursive: true }); + const sessionId = 'session-a'; + const file = path.join(project, `${sessionId}.jsonl`); + const record = (role: string, content: object[], extra = {}) => ({ + cwd, sessionId, isSidechain: false, timestamp: '2026-09-08T15:31:13.607Z', + message: { role, content }, ...extra, + }); + const question = (text = 'D1 — Cross-project learnings scope ') => ({ + header: 'Learnings', question: text, + options: [{ label: 'Enable cross-project learnings (Recommended)' }, { label: 'Keep project-scoped' }], + }); + const ask = (id: string, questions = [question()], extra = {}) => record('assistant', [ + { type: 'tool_use', id, name: 'AskUserQuestion', input: { questions } }, + ], extra); + const answer = (id: string, questions = [question()], extra = {}) => record('user', [ + { type: 'tool_result', tool_use_id: id, content: 'Your questions have been answered.' }, + ], { toolUseResult: { answers: Object.fromEntries(questions.map(q => [q.question, q.options[0].label])) }, ...extra }); + const append = (...records: object[]) => fs.appendFileSync(file, records.map(r => JSON.stringify(r) + '\n').join('')); + const read = () => readPlanCountTranscript(config, cwd); + return { config, cwd, project, file, question, ask, answer, append, read, record }; +} + +describe('native plan-count transcripts', () => { + test('the actual partially answered CEO setup packet counts once without selecting its unanswered mode', () => { + const f = fixture(); + // Native question/header/label metadata from targeted-a's paired CEO + // call. The successful tool_result answered only routing and approach. + const packet = [{"header":"Routing","question":"gstack works best when your project's CLAUDE.md includes skill routing rules — should I add them? ","options":[{"label":"Add routing rules (Recommended)"},{"label":"Skip for now"}]},{"header":"Prerequisites","question":"No design doc found for this branch. Run /office-hours to capture structured problem context first, or proceed directly to the plan review? ","options":[{"label":"Skip — review the plan directly (Recommended)"},{"label":"Run /office-hours first"}]},{"header":"Test Scope","question":"Which implementation approach should the tests follow? This shapes the review scope. \n\nD1 — Approach selection for processPayment() test coverage\nProject: Payment Processing — adding missing unit tests\nELI10: The plan calls for exactly 2 tests. Adding a few more for the most common real-world Stripe failures (card declined, rate limit) costs ~10 extra minutes with CC but closes the gaps users actually hit. The question is whether to stay at 2 or expand to ~5-6 tests.\nStakes if we pick wrong: Choosing minimal leaves 402 (card declined) untested — the most common production failure. Choosing full adds ~5 min of CC work.\nRecommendation: B (Core Stripe paths) because 402 card declined is the highest-volume real-world failure and CC compresses the extra work to near-zero.\nCompleteness: A=7/10, B=9/10","options":[{"label":"Minimal — 2 tests as planned (7/10)"},{"label":"Core paths — ~5 tests (9/10) (Recommended)"}]},{"header":"Review Mode","question":"D2 — Review mode for this plan?\nProject: Payment Processing test coverage — adding missing unit tests for processPayment()\nELI10: The plan adds 2 missing unit tests. HOLD SCOPE means: take the scope as given, review it with maximum rigor — catch every ambiguity, failure mode, edge case, observability gap. SELECTIVE EXPANSION means: do all that AND surface cherry-pick expansions (additional test scenarios, receipt schema validation, etc.) one at a time for your approval.\nStakes if we pick wrong: HOLD keeps the review tight and fast. SELECTIVE surfaces more opportunities but adds round-trips.\nRecommendation: HOLD SCOPE — this is a focused gap-fill, and rigor matters more than ambition here.\nNote: options differ in kind, not coverage — no completeness score. ","options":[{"label":"HOLD SCOPE — maximum rigor (Recommended)"},{"label":"SELECTIVE EXPANSION — rigor + cherry-picks"}]}]; + f.append(f.ask('actual-packet', packet), f.answer('actual-packet', [packet[0], packet[2]])); + const [call] = f.read().calls; + expect(call.answered).toBe(true); + expect(call.failed).toBe(false); + expect(call.unansweredQuestionIndices).toEqual([1, 3]); + expect(f.read().calls.filter(c => c.answered)).toHaveLength(1); + const setup = nativePlanCallFingerprint(call, 0, true); + expect(setup.options.some(o => /HOLD SCOPE/.test(o.label))).toBe(false); + expect(ceoStep0Boundary(setup)).toBe(false); + expect(ceoFirstReviewAUQ(setup)).toBe(false); + + const finding = [{"header":"Receipt Schema","question":"D3 — The plan says 'assert correct receipt is generated' but doesn't define what a correct receipt looks like. Without a schema, the test will pass even if the receipt is missing critical fields.\n\nELI10: Right now the test could assert `receipt != nil` and call it a day. That test passes even if the receipt has the wrong amount or no charge ID. We need to specify what fields a correct receipt must have.\n\nStakes if we pick wrong: Tests pass in CI but fail to catch a real receipt bug — e.g., wrong charge_id linked to wrong customer.\n\nRecommendation: A — specify the receipt schema in the plan now; costs ~5 min, prevents a class of silent bugs.\nCompleteness: A=9/10, B=7/10, C=3/10 ","options":[{"label":"Specify schema in plan (Recommended)"},{"label":"Specify during implementation"},{"label":"Skip — too much detail for a plan"}]}]; + f.append(f.ask('receipt-schema', finding), f.answer('receipt-schema', finding)); + const review = nativePlanCallFingerprint(f.read().calls[1], 1, true); + expect(planCountQuestionPhase(review, false, ceoStep0Boundary, ceoFirstReviewAUQ)) + .toEqual({ preReview: false, reviewStarted: true }); + + // A refused/empty packet must not receive the same completion credit. + f.append(f.ask('empty-packet', packet), f.answer('empty-packet', [])); + const empty = f.read().calls[2]; + expect(empty.answered).toBe(false); + expect(empty.failure).toContain('no matching nonempty answers'); + }); + + test('CEO first-finding fallback requires native finding identity and rejects setup decisions', () => { + const f = fixture(); + for (const [id, text] of [ + ['setup', 'D1 — Missing context: choose a scope '], + ['mode', 'D1 — Missing context: choose a mode '], + ['unscoped', 'D1 — Missing receipt schema'], + ['arbitrary', 'D1 — Pick an option '], + ['next', 'D1 — Missing engineering review: what next? '], + ]) { + const questions = [f.question(text)]; + f.append(f.ask(id, questions), f.answer(id, questions)); + } + for (const call of f.read().calls) expect(ceoFirstReviewAUQ(nativePlanCallFingerprint(call, 0, true))).toBe(false); + }); + + test('requires a matching successful answer and preserves full native question metadata', () => { + const f = fixture(); + const questions = [f.question('D1 — Cross-project learnings scope\n' + 'full context '.repeat(40) + '')]; + f.append(f.ask('one', questions)); + expect(f.read().calls[0].answered).toBe(false); + f.append(f.answer('one', questions)); + const before = fs.readFileSync(f.file, 'utf8'); + const observed = f.read(); + expect(observed.status).toBe('ready'); + expect(observed.calls).toHaveLength(1); + expect(observed.calls[0].answered).toBe(true); + expect(observed.calls[0].questions).toEqual(questions); + const fp = nativePlanCallFingerprint(observed.calls[0], 1, true); + expect(fp.promptSnippet.length).toBeGreaterThan(240); + expect(fp.promptSnippet).toContain(''); + expect(planCountQuestionPhase(fp, false, engStep0Boundary)).toEqual({ preReview: true, reviewStarted: true }); + expect(fs.readFileSync(f.file, 'utf8')).toBe(before); + }); + + test('one completed batched AskUserQuestion call remains one count even with unanswered questions', () => { + const f = fixture(); + const questions = [f.question('D1 — Retry behavior?'), f.question('D2 — Error handling?')]; + f.append(f.ask('batch', questions), f.answer('batch', [questions[0]])); + expect(f.read().calls.filter(c => c.answered)).toHaveLength(1); + expect(f.read().calls[0].unansweredQuestionIndices).toEqual([1]); + f.append(f.answer('batch', questions)); + expect(f.read().calls.filter(c => c.answered)).toHaveLength(1); + expect(f.read().calls[0].questions).toHaveLength(2); + }); + + test('partial JSONL appends add no call or completion until the final newline', () => { + const f = fixture(); + const call = JSON.stringify(f.ask('one')); + fs.writeFileSync(f.file, call.slice(0, 80)); + expect(f.read()).toEqual({ status: 'missing', calls: [], assistantMessages: [] }); + fs.appendFileSync(f.file, call.slice(80)); + expect(f.read().calls).toEqual([]); + fs.appendFileSync(f.file, '\n'); + expect(f.read().calls[0].answered).toBe(false); + const result = JSON.stringify(f.answer('one')); + fs.appendFileSync(f.file, result.slice(0, 80)); + expect(f.read().calls[0].answered).toBe(false); + fs.appendFileSync(f.file, result.slice(80) + '\n'); + expect(f.read().calls[0].answered).toBe(true); + }); + + test('repeated records and polls dedupe by tool ID, while identical questions in distinct calls remain distinct', () => { + const f = fixture(); + f.append(f.ask('one'), f.ask('one'), f.answer('one'), f.answer('one'), f.ask('two'), f.answer('two')); + const first = f.read(); + expect(first.calls.filter(c => c.answered)).toHaveLength(2); + expect(f.read()).toEqual(first); + expect(first.calls.map(c => nativePlanCallFingerprint(c, 0, false).signature)).toEqual(['session-a:one', 'session-a:two']); + }); + + test('wrong cwd, sidechain, session, tool ID, and missing scope cannot provide an answer', () => { + const f = fixture(); + f.append(f.ask('one')); + for (const extra of [{ cwd: '/other' }, { isSidechain: true }, { sessionId: 'other' }, { cwd: undefined }, { isSidechain: undefined }]) { + f.append(f.answer('one', undefined, extra), f.ask('foreign', undefined, extra)); + } + f.append(f.answer('wrong-id')); + expect(f.read().calls).toHaveLength(1); + expect(f.read().calls[0].answered).toBe(false); + fs.writeFileSync(path.join(f.project, 'other.jsonl'), JSON.stringify(f.answer('one', undefined, { sessionId: 'other' })) + '\n'); + expect(f.read().calls[0].answered).toBe(false); + }); + + test('native permissions, schema failures, errors and refusals do not become completed questions', () => { + const f = fixture(); + f.append(f.record('assistant', [{ type: 'tool_use', id: 'write', name: 'Write', input: { file_path: 'PLAN.md' } }])); + f.append(f.answer('write')); + f.append(f.record('assistant', [{ type: 'tool_use', id: 'bad', name: 'AskUserQuestion', input: { questions: [] } }])); + f.append(f.answer('bad')); + f.append(f.ask('error')); + f.append(f.record('user', [{ type: 'tool_result', tool_use_id: 'error', is_error: true, content: 'Rejected' }], + { toolUseResult: { answers: { [f.question().question]: 'Yes' } } })); + f.append(f.ask('refusal'), f.answer('refusal', undefined, { toolUseResult: { answers: {} } })); + expect(f.read().calls).toHaveLength(2); + expect(f.read().calls.filter(c => c.answered)).toHaveLength(0); + expect(f.read().calls.every(c => c.failed)).toBe(true); + }); + + test('unrelated answered questions cannot conceal a refusal, but an actual retry can resolve it', () => { + const f = fixture(); + const missing = [f.question('Should retries preserve idempotency?')]; + const unrelated = [f.question('Should errors be logged?')]; + f.append(f.ask('refusal', missing), f.answer('refusal', [], { toolUseResult: { answers: {} } })); + f.append(f.ask('unrelated', unrelated), f.answer('unrelated', unrelated)); + expect(unresolvedPlanQuestionCalls(f.read().calls).map(c => c.toolUseId)).toEqual(['refusal']); + f.append(f.ask('retry', missing), f.answer('retry', missing)); + expect(unresolvedPlanQuestionCalls(f.read().calls)).toEqual([]); + expect(f.read().calls.filter(c => c.answered)).toHaveLength(2); + }); + + test('missing transcripts and files from another fixture produce no positive coverage', () => { + const f = fixture(); + expect(f.read()).toEqual({ status: 'missing', calls: [], assistantMessages: [] }); + f.append(f.ask('other', undefined, { cwd: '/different-fixture' }), f.answer('other', undefined, { cwd: '/different-fixture' })); + expect(f.read()).toEqual({ status: 'missing', calls: [], assistantMessages: [] }); + expect(readPlanCountTranscript(path.join(f.config, 'absent'), f.cwd)).toEqual({ status: 'missing', calls: [], assistantMessages: [] }); + }); + + test('invalid completed JSON and excessive bytes fail explicitly without retaining partial coverage', () => { + const f = fixture(); + f.append(f.ask('one'), f.answer('one')); + fs.appendFileSync(f.file, 'not JSON\n'); + expect(f.read().status).toBe('error'); + expect(f.read().calls).toEqual([]); + fs.writeFileSync(f.file, ''); + fs.truncateSync(f.file, 32 * 1024 * 1024 + 1); + expect(f.read().error).toContain('32 MiB'); + expect(f.read().calls).toEqual([]); + }); + + test('assistant posture evidence is scoped, timestamped text rather than tool results or partial records', () => { + const f = fixture(); + f.append(f.record('assistant', [{ type: 'text', text: 'Earlier assistant posture.' }], { timestamp: '2026-09-08T15:31:10.000Z' })); + f.append(f.ask('mode'), f.answer('mode')); + f.append(f.record('assistant', [{ type: 'text', text: 'Later assistant posture.' }], { timestamp: '2026-09-08T15:31:20.000Z' })); + for (const extra of [{ cwd: '/other' }, { isSidechain: true }, { sessionId: 'other' }, { timestamp: 'invalid' }, { timestamp: undefined }]) { + f.append(f.record('assistant', [{ type: 'text', text: 'Not evidence.' }], extra)); + } + f.append(f.record('user', [{ type: 'text', text: 'User prose is not assistant posture.' }])); + f.append(f.record('user', [{ type: 'tool_result', tool_use_id: 'other', content: 'Tool result posture is not evidence.' }])); + const observed = f.read(); + expect(observed.assistantMessages).toEqual([ + { sessionId: 'session-a', text: 'Earlier assistant posture.', timestamp: '2026-09-08T15:31:10.000Z' }, + { sessionId: 'session-a', text: 'Later assistant posture.', timestamp: '2026-09-08T15:31:20.000Z' }, + ]); + expect(observed.calls[0].answeredAt).toBe('2026-09-08T15:31:13.607Z'); + fs.appendFileSync(f.file, JSON.stringify(f.record('assistant', [{ type: 'text', text: 'Still writing.' }]))); + expect(f.read().assistantMessages).toEqual(observed.assistantMessages); + fs.appendFileSync(f.file, '\ninvalid JSON\n'); + expect(f.read().assistantMessages).toEqual([]); + expect(f.read().calls).toEqual([]); + }); +}); diff --git a/test/skill-e2e-plan-ceo-finding-count.test.ts b/test/skill-e2e-plan-ceo-finding-count.test.ts index 9aff272dd..f2e6482ca 100644 --- a/test/skill-e2e-plan-ceo-finding-count.test.ts +++ b/test/skill-e2e-plan-ceo-finding-count.test.ts @@ -23,6 +23,7 @@ import * as path from 'node:path'; import { runPlanSkillCounting, ceoStep0Boundary, + ceoFirstReviewAUQ, assertReviewReportAtBottom, type AskUserQuestionFingerprint, } from './helpers/claude-pty-runner'; @@ -65,8 +66,8 @@ const FLOOR_PAIRED = 2; const CEILING_PAIRED = 4; // Keep the five seeded defects distinct from already-satisfied surrounding -// contracts. The live control correctly found extra signature, deduplication, -// concurrency, and naming gaps when those baseline facts were unspecified. +// contracts. Live controls correctly found extra ingress, missing-user, +// observability, and rollout gaps when those baseline facts were unspecified. const planCeo5Findings = (planPath: string) => [ `Please review this plan thoroughly. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`, '', @@ -79,6 +80,12 @@ const planCeo5Findings = (planPath: string) => [ 'and an existing per-user transaction lock serializes payment updates.', 'The new handler runs inside those unchanged guards; this plan does not', 'replace signature verification, event deduplication, or update locking.', + 'The existing lookup-result guard acknowledges unknown/deleted users with', + 'HTTP 200, logs the event, and stops before user updates or email fan-out.', + 'The ingress wrapper already logs event IDs, outcomes, and durations, with', + 'alerts for failed webhook processing. Those controls remain in place.', + 'The deployment already has a handler feature flag and a documented, tested', + 'rollback to the prior handler; this change uses that existing rollout path.', '', '## Architecture', "We're adding a new `StripePaymentWebhookHandler` class that will handle Stripe webhooks.", @@ -106,10 +113,20 @@ const planCeo2PairedFindings = (planPath: string) => [ '', '# Plan: Payment Processing — Test Coverage', '', + '## Existing coverage and test infrastructure retained', + 'This changes unit tests only; processPayment() production behavior stays as-is.', + 'The Stripe adapter suite already covers network timeouts, card declines (402),', + 'rate limits (429), and recovery when an initial 502 is followed by a successful', + 'charge. Receipt-builder failure behavior has its own passing regression tests.', + 'The payment test factory explicitly configures max_retries=1 and exposes the', + 'Stripe mock call history. Its injected virtual sleeper records backoff without', + 'real delays, so an exhausted 502 operation makes exactly two charge attempts.', + 'These existing helpers and regression suites remain in use for this change.', + '', '## Tests', 'We need test coverage for `processPayment()`. Specifically:', '1. The happy path (successful Stripe charge — assert correct receipt is generated).', - '2. The error/timeout path (Stripe returns 502 — assert retry-with-backoff fires once, then fails clean).', + '2. The 502 error path (Stripe returns 502 — assert retry-with-backoff fires once, then fails clean).', '', 'Currently neither has a unit test. These are deliberately separate concerns:', 'the success path is correctness, the failure path is graceful degradation.', @@ -131,6 +148,7 @@ describeE2E('/plan-ceo-review per-finding AskUserQuestion count (periodic)', () slashCommand: '/plan-ceo-review', followUpPrompt: planCeo5Findings(planPath), isLastStep0AUQ: ceoStep0Boundary, + isFirstReviewAUQ: ceoFirstReviewAUQ, reviewCountCeiling: CEILING_DISTINCT + 1, // hard cap above assertion ceiling firstAUQPick: pickSkipInterview, // bypass scope-selection, route to review timeoutMs: 1_500_000, // 25 min @@ -214,6 +232,7 @@ describeE2E('/plan-ceo-review per-finding AskUserQuestion count (periodic)', () slashCommand: '/plan-ceo-review', followUpPrompt: planCeo2PairedFindings(planPath), isLastStep0AUQ: ceoStep0Boundary, + isFirstReviewAUQ: ceoFirstReviewAUQ, reviewCountCeiling: CEILING_PAIRED + 1, timeoutMs: 1_500_000, env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' }, diff --git a/test/skill-e2e-plan-design-finding-count.test.ts b/test/skill-e2e-plan-design-finding-count.test.ts index dfd4bb02a..3bb7dcbc0 100644 --- a/test/skill-e2e-plan-design-finding-count.test.ts +++ b/test/skill-e2e-plan-design-finding-count.test.ts @@ -16,6 +16,7 @@ import * as path from 'node:path'; import { runPlanSkillCounting, designStep0Boundary, + designFirstReviewAUQ, assertReviewReportAtBottom, } from './helpers/claude-pty-runner'; @@ -25,11 +26,79 @@ const N = 5; const FLOOR = N - 1; const CEILING = N + 2; +// A known surrounding design prevents missing layout/journey/state contracts +// from becoming legitimate extra findings unrelated to the five seeded gaps. +const designSystem = [ + '# Approved account-settings design system', + '', + 'Reuse the existing single-column settings shell: persistent app navigation,', + 'page title and description, action group, then Profile and Notifications fieldsets.', + 'The form has a 640px maximum width. Reuse existing Button, Field, InlineStatus,', + 'ErrorSummary and ConfirmationDialog components; no new component family is needed.', + 'Profile contains Display name (text) and Email (email). Notifications contains', + 'Weekly digest and Product tips switches. Existing server defaults are the', + 'account name/email, Weekly digest on, and Product tips off; labels/order stay fixed.', + '', + 'Save is the only filled primary action (#1d4ed8 with white text). Reset, Cancel', + 'and Export are neutral ghost buttons; destructive intent is explained in the', + 'existing confirmation dialog, whose default is Cancel. All targets are at least 44px.', + 'Spacing uses an 8px base: sections 32px, field groups 24px, label-to-input 8px.', + 'Typography has two roles: 16px body/form labels/helper text, 20px section headings.', + 'Use error.text #991b1b on error.surface #fef2f2 with an icon and explicit text.', + 'All text must meet WCAG AA contrast; never communicate status through color alone.', + 'Focus-visible is the existing 2px solid #1d4ed8 outline, offset 2px on white;', + 'its measured contrast exceeds 3:1. Reuse it on all controls and dialog actions.', + '', + 'The established pending-action pattern is an inline spinner beside “Saving…”', + 'inside the disabled Save button, aria-busy=true, with reduced-motion support.', + 'Success uses the persistent inline status “Saved” plus the save time (aria-live=polite).', + 'The existing formatter renders “Saved at HH:mm” in the user’s local 24-hour time.', + 'Reset dialog: “Discard unsaved changes?” / “Your saved preferences will be restored.”', + 'Its actions are “Keep editing” (default) and “Discard changes”. Cancel navigation', + 'uses the same dialog with “Keep editing” (default) and “Discard and leave”.', + 'Validation errors appear beside fields and in a linked error summary; focus the', + 'first invalid field. Network errors preserve edits and show Retry in that status area.', +].join('\n'); + const planDesign5Findings = (planPath: string) => [ `Please review this plan thoroughly. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`, + // The skill explicitly supports this opt-out. The cadence test measures + // review findings; generated-variant preference questions have separate evals. + 'Text-only review; skip mockups. Review all seven design dimensions.', '', '# Plan: Settings Page UI redesign', '', + '## Existing product and accepted behavior', + 'This updates an existing account-settings form using the checked-in DESIGN.md.', + 'The shell and components already exist. Profile and Notifications are the only', + 'sections, with visible headings and associated field labels. The page header', + 'contains the title, a short description, and Save/Reset/Cancel/Export actions.', + 'Preserve the approved single-column structure and component behavior.', + '', + 'Journey: a user arrives from account navigation wanting to adjust preferences,', + 'edits the labeled fields, saves, and reads the inline Saved timestamp before', + 'leaving. The feedback preserves confidence that their preferences were stored.', + 'Initial loading uses the existing form skeleton. A new account sees useful', + 'default preferences as specified in DESIGN.md rather than an empty page. Read failures show Retry.', + 'Save is atomic: all fields persist together or none do, so partial success is', + 'not exposed. Field validation, network failure, and successful-save feedback', + 'use the exact existing DESIGN.md patterns. Preserve unsaved values after errors.', + 'Disable repeat Save submissions while pending. Reset restores saved values only', + 'after confirmation; Cancel confirms discarding dirty edits before returning to', + 'the previous page; Export downloads the current saved preferences as JSON.', + '', + 'Responsive behavior: above 640px keep the header action group in one row; at', + '640px and below, place full-width Save first and the three secondary actions', + 'below it, preserving DOM/tab order. The form fits 320px without horizontal scroll.', + 'All controls have visible focus rings and 44px targets. Use semantic fieldsets,', + 'labels, a main landmark and heading order; errors link through aria-describedby.', + 'Dialogs trap focus and restore it on close; Escape cancels. Export remains a', + 'clearly labeled button. Respect reduced motion. No additional visual exploration', + 'or component replacement is part of this established form update.', + '', + '## Planned implementation gaps', + 'The proposed form still has the following inconsistencies with that design:', + '', '## Visual Hierarchy', 'The "Save" button is rendered with the same size, weight, and color as', 'three other buttons in the page header (Reset, Cancel, Export). Nothing', @@ -68,6 +137,8 @@ describeE2E('/plan-design-review per-finding AskUserQuestion count (periodic)', slashCommand: '/plan-design-review', followUpPrompt: planDesign5Findings(planPath), isLastStep0AUQ: designStep0Boundary, + isFirstReviewAUQ: designFirstReviewAUQ, + fixtureFiles: { 'DESIGN.md': designSystem }, reviewCountCeiling: CEILING + 1, timeoutMs: 1_500_000, env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' }, diff --git a/test/skill-e2e-plan-devex-finding-count.test.ts b/test/skill-e2e-plan-devex-finding-count.test.ts index 4b4762435..839c9922b 100644 --- a/test/skill-e2e-plan-devex-finding-count.test.ts +++ b/test/skill-e2e-plan-devex-finding-count.test.ts @@ -2,7 +2,9 @@ * /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 review-phase AUQ count ∈ [N-1, N+2]. + * against a 5-finding seeded plan and asserts substantive issue AUQ count ∈ [N-1, N+2]. + * 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. * * Tier: periodic (~25 min, ~$5/run). Sequential by default per plan §D15. @@ -18,6 +20,12 @@ import { devexStep0Boundary, assertReviewReportAtBottom, } from './helpers/claude-pty-runner'; +import { + DEVEX_COUNT_FILES, + planDevexCountFixture, + isDevexReviewIssue, + devexReviewModePick, +} from './helpers/devex-count-fixture'; const describeE2E = describeE2ETier('periodic'); @@ -25,36 +33,9 @@ const N = 5; const FLOOR = N - 1; const CEILING = N + 2; -const planDevex5Findings = (planPath: string) => [ - `Please review this plan thoroughly. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`, - '', - '# Plan: Public SDK Beta Launch', - '', - '## Persona', - "The plan doesn't specify which developer persona is the target — we're", - "shipping for \"everyone,\" which means we tune for nobody.", - '', - '## TTHW (time to hello world)', - 'Time-to-hello-world is not measured. No benchmark data referenced. We', - "don't know if first-run takes 5 minutes or 50.", - '', - '## Friction Point', - 'First-run currently requires a 5-minute mandatory CI step before the', - 'developer can run their first eval. There is no way to skip it.', - '', - '## Magical Moment', - 'Getting-started flow has no delight beat. Pure documentation, no', - 'interactive demo, no "ah-ha" moment that makes the developer trust us.', - '', - '## Competitive Blind Spot', - "The plan doesn't reference how peer SDKs (LangChain, Semantic Kernel,", - 'OpenAI) handle this DX surface. We may be reinventing worse versions', - 'of solved problems.', -].join('\n'); - describeE2E('/plan-devex-review per-finding AskUserQuestion count (periodic)', () => { test( - `5-finding plan emits ${FLOOR}-${CEILING} review-phase AskUserQuestions`, + `5-finding plan emits ${FLOOR}-${CEILING} substantive issue AskUserQuestions`, async () => { // Per-run artifact dir: a hardcoded shared /tmp path collides under // --retry, EVALS_JOBS>1, or concurrent worktrees (a sibling's finally- @@ -66,8 +47,11 @@ describeE2E('/plan-devex-review per-finding AskUserQuestion count (periodic)', ( const obs = await runPlanSkillCounting({ skillName: 'plan-devex-review', slashCommand: '/plan-devex-review', - followUpPrompt: planDevex5Findings(planPath), + followUpPrompt: planDevexCountFixture(planPath), + fixtureFiles: DEVEX_COUNT_FILES, isLastStep0AUQ: devexStep0Boundary, + isReviewAUQ: isDevexReviewIssue, + pickAUQ: devexReviewModePick, reviewCountCeiling: CEILING + 1, timeoutMs: 1_500_000, env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' }, diff --git a/test/skill-e2e-plan-eng-finding-count.test.ts b/test/skill-e2e-plan-eng-finding-count.test.ts index 8c0c32c26..b99b0b303 100644 --- a/test/skill-e2e-plan-eng-finding-count.test.ts +++ b/test/skill-e2e-plan-eng-finding-count.test.ts @@ -25,11 +25,23 @@ 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. const planEng5Findings = (planPath: string) => [ `Please review this plan thoroughly. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`, '', '# Plan: Multi-tenant Auth Refactor', '', + '## Existing contracts retained', + 'The existing cache adapter keys entries by tenant ID, issuer, audience,', + 'and policy version. It evicts expired tokens and invalidates entries on', + 'logout, token revocation, or tenant suspension. AuthCache retains these', + 'unchanged validity and tenant-key rules; they do not serialize mutations.', + 'Unit and integration coverage is planned for the new components and their', + 'success/error paths. That coverage does not exercise legacyAuthFlow() or', + 'assert compatibility with its prior behavior.', + '', '## Architecture', 'Two new services (`AuthBroker` and `SessionMint`) share a global mutable', '`AuthCache` instance via module-level export. Both services mutate it.',