From 123d43d4e946640b3ec5f5b74191d82abf78a234 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 07:57:30 +0000 Subject: [PATCH] test: stabilize native review counting and interactive navigation Co-Authored-By: OpenAI Codex --- test/autoplan-setup-question.test.ts | 70 ++++ test/ceo-mode-option.test.ts | 89 ++++- test/helpers/autoplan-setup-question.ts | 22 ++ test/helpers/ceo-mode-option.ts | 65 ++++ test/helpers/claude-pty-runner.ts | 237 ++++++++---- test/helpers/claude-pty-runner.unit.test.ts | 339 ++++++++++++++++++ test/helpers/plan-count-fixture.ts | 40 ++- test/helpers/touchfiles-data.ts | 16 +- test/plan-count-fixture.test.ts | 118 +++++- test/pty-option-selection.test.ts | 107 ++++++ test/skill-e2e-autoplan-chain.test.ts | 19 +- test/skill-e2e-plan-ceo-finding-count.test.ts | 9 +- test/skill-e2e-plan-ceo-mode-routing.test.ts | 73 ++-- ...kill-e2e-plan-design-finding-count.test.ts | 3 +- ...skill-e2e-plan-devex-finding-count.test.ts | 3 +- test/skill-e2e-plan-eng-finding-count.test.ts | 3 +- 16 files changed, 1071 insertions(+), 142 deletions(-) create mode 100644 test/autoplan-setup-question.test.ts create mode 100644 test/helpers/autoplan-setup-question.ts create mode 100644 test/pty-option-selection.test.ts diff --git a/test/autoplan-setup-question.test.ts b/test/autoplan-setup-question.test.ts new file mode 100644 index 000000000..55bb1c32c --- /dev/null +++ b/test/autoplan-setup-question.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, test } from 'bun:test'; +import { autoplanRoutingSetupInput } from './helpers/autoplan-setup-question'; +import { E2E_TOUCHFILES, selectTests } from './helpers/touchfiles'; + +// Sanitized terminal frame from the 2026-09-08 autoplan timeout. The qid is +// visibly incomplete; the prompt body and explicit choices remain intact. +const CAPTURE = [ + '─'.repeat(120), + 'Planning: /tmp/hermetic/.claude/plans/modular-bouncing-swing.md', + '─'.repeat(120), + ' ☐ Routing rules', + "│ gstack works best when your project's CLAUDE.md includes skill routing rules. Add them now?", + '│', + '❯1.Addroutingrules(Recommended)', + 'CreatesCLAUDE.mdwithskillroutingrulessogstackknowswhentoinvoke/office-hours,/autoplan,/ship,/qa,', + "etc.automatically.We'lldothisafterthereview.", + '2.Nothanks', + "Skip—I'llinvokeskillsmanually.Youcanenablethislaterbyrunninggstack-configsetrouting_declinedfalse.", + '3.Typesomething.', + '4.Chataboutthis', + 'Entertoselect·↑/↓tonavigate·Esctocancel', +].join('\r\r'); + +describe('autoplan routing setup handling', () => { + test('answers the captured setup once, using the full question identity', () => { + const seen = new Set(); + expect(autoplanRoutingSetupInput(CAPTURE, seen)).toBe('1\r'); + expect(autoplanRoutingSetupInput(CAPTURE, seen)).toBeNull(); + expect(autoplanRoutingSetupInput(CAPTURE.replace('works best', 'works best'), seen)).toBeNull(); + }); + + test('chooses Add routing rules by label when option order changes', () => { + const reordered = CAPTURE.replace('❯1.Addroutingrules(Recommended)', '❯1.Nothanks') + .replace('2.Nothanks', '2.Add routing rules (Recommended)'); + expect(autoplanRoutingSetupInput(reordered, new Set())).toBe('2\r'); + }); + + test('accepts the full option labels captured from the subsequent live setup prompt', () => { + const fullLabels = CAPTURE.replace('Addroutingrules(Recommended)', 'Add routing rules to CLAUDE.md (Recommended)') + .replace('2.Nothanks', "2.No thanks, I'll invoke skills manually"); + expect(autoplanRoutingSetupInput(fullLabels, new Set())).toBe('1\r'); + expect(autoplanRoutingSetupInput(fullLabels.replace('CLAUDE.md (Recommended)', 'product routes (Recommended)'), new Set())).toBeNull(); + expect(autoplanRoutingSetupInput(fullLabels.replace("I'll invoke skills manually", 'delete the existing rules'), new Set())).toBeNull(); + }); + + test('waits for complete recognized choices rather than guessing a default', () => { + expect(autoplanRoutingSetupInput(CAPTURE.replace('2.Nothanks', '2.Ask me later'), new Set())).toBeNull(); + expect(autoplanRoutingSetupInput(CAPTURE.replace('Addroutingrules(Recommended)', 'Accept recommendation'), new Set())).toBeNull(); + expect(autoplanRoutingSetupInput('❯1.Addroutingrules(Recommended)\r2.Nothanks', new Set())).toBeNull(); + }); + + test('never answers review or taste questions, even with a routing qid or the same choices', () => { + const prompts = [ + 'Which visual direction should this settings page use?', + 'Should the payment handler bypass the existing dispatcher?', + 'Add routing rules to the product API now? ', + 'The plan quotes CLAUDE.md skill routing rules. Should we change this feature?', + ]; + for (const prompt of prompts) { + const frame = `☐ Review decision\r${prompt}\r❯1.Addroutingrules(Recommended)\r2.Nothanks`; + expect(autoplanRoutingSetupInput(frame, new Set())).toBeNull(); + } + }); + + test('setup helper and captured-frame changes select the autoplan eval only', () => { + for (const file of ['test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts']) { + expect(selectTests([file], E2E_TOUCHFILES).selected).toEqual(['autoplan-chain-pty']); + } + }); +}); diff --git a/test/ceo-mode-option.test.ts b/test/ceo-mode-option.test.ts index 8f98862f1..df9f3d69d 100644 --- a/test/ceo-mode-option.test.ts +++ b/test/ceo-mode-option.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'bun:test'; -import { findCeoModeOption } from './helpers/ceo-mode-option'; +import { findCeoModeOption, hasPostAnswerCeoPosture, nextCeoModeNavigation } from './helpers/ceo-mode-option'; import { parseNumberedOptions, stripAnsi } from './helpers/claude-pty-runner'; import { E2E_TOUCHFILES, selectTests } from './helpers/touchfiles'; @@ -63,8 +63,93 @@ describe('CEO mode option matching', () => { }); test('helper and regression changes select only the mode-routing paid eval', () => { - for (const file of ['test/helpers/ceo-mode-option.ts', 'test/ceo-mode-option.test.ts']) { + for (const file of ['test/helpers/ceo-mode-option.ts', 'test/ceo-mode-option.test.ts', 'test/pty-option-selection.test.ts']) { expect(selectTests([file], E2E_TOUCHFILES).selected).toEqual(['plan-ceo-mode-routing']); } }); }); + +describe('CEO mode navigation replay', () => { + test('advances different setup questions with the same choices and ignores redraws', () => { + const seen = new Set(); + const first = '☐Routing\rEnable skill routing?\r❯1.Enable\r2.Skip'; + const next = '☐Learnings\rEnable cross-project learnings?\r❯1.Enable\r2.Skip'; + expect(nextCeoModeNavigation(first, 'HOLD SCOPE', seen).kind).toBe('question'); + expect(nextCeoModeNavigation(first, 'HOLD SCOPE', seen).kind).toBe('wait'); + expect(nextCeoModeNavigation(next, 'HOLD SCOPE', seen).kind).toBe('question'); + expect(seen.size).toBe(2); + }); + + test('navigates the captured unanswered setup tab before submitting, without counting Submit', () => { + const seen = new Set(); + const partial = [ + '← ☒ Skill routing ☐ Learnings scope ✔ Submit →', + 'Review your answers', + '⚠You have not answered all questions', + '❯1.Submit aswers', + '2Cancel', + ].join('\r\r'); + expect(nextCeoModeNavigation(partial, 'HOLD SCOPE', seen)).toEqual({ kind: 'submission', input: '\x1b[Z' }); + expect(seen.size).toBe(0); + const question = '← ☒ Skill routing ☐ Learnings scope ✔ Submit →\rEnable cross-project learnings?\r❯1.Enable\r2.Skip'; + expect(nextCeoModeNavigation(`${partial}\r${question}`, 'HOLD SCOPE', seen).kind).toBe('question'); + const answered = partial.replace('☐ Learnings scope', '☒ Learnings scope').replace('⚠You have not answered all questions', ''); + expect(nextCeoModeNavigation(answered, 'HOLD SCOPE', seen)).toEqual({ kind: 'submission', input: '\r' }); + expect(seen.size).toBe(1); + }); + + test('handles native permission controls before question parsing and dedup', () => { + const seen = new Set(); + const permission = 'DoyouwanttooverwriteCLAUDE.md?\r❯1.Yes\r2.No\rEsctocancel·Tabtoamend'; + expect(nextCeoModeNavigation(permission, 'HOLD SCOPE', seen)).toEqual({ kind: 'permission', input: '1\r' }); + expect(seen.size).toBe(0); + const actual = '☐ Approaches\rWhich storage strategy?\r❯1.Server\r2.Local'; + expect(nextCeoModeNavigation(`${permission}\r${actual}`, 'HOLD SCOPE', seen).kind).toBe('question'); + }); + + test('selects the intended mode from the observed menu, preserving its index', () => { + const frame = '☐ReviewMode\rWhat review posture should I use?\r❯1.SELECTIVEEXPANSION(recommended)\r2.HOLDSCOPE\r3.SCOPEEXPANSION\r4.SCOPEREDUCTION'; + for (const [mode, index] of [['HOLD SCOPE', 2], ['SCOPE EXPANSION', 3]] as const) { + const action = nextCeoModeNavigation(frame, mode, new Set()); + expect(action.kind).toBe('mode'); + if (action.kind === 'mode') expect(action.index).toBe(index); + } + }); +}); + +describe('CEO posture evidence after mode selection', () => { + const posture = /\b(rigor|bulletproof|hold\s*scope|maximum\s+rigor)\b/i; + const menu = [ + '☐ Review mode', + '❯1.SELECTIVEEXPANSION(recommended)', + '2.HOLD SCOPE │ Code is written. Make it bulletproof.', + '3.SCOPE EXPANSION', + '4.SCOPE REDUCTION', + 'Enter to select · ↑/↓ to navigate', + ].join('\r'); + + test('menu redraw and native selected-option echo cannot satisfy the posture gate', () => { + expect(posture.test(menu)).toBe(true); // The old unscoped check passed here. + expect(hasPostAnswerCeoPosture(menu, posture)).toBe(false); + const answer = "⏺ User answered Claude's questions:\r⎿ · Review mode? → HOLD SCOPE"; + expect(hasPostAnswerCeoPosture(`${menu}\r${answer}`, posture)).toBe(false); + expect(hasPostAnswerCeoPosture('❯ HOLD SCOPE\r● HOLD SCOPE', posture)).toBe(false); + expect(hasPostAnswerCeoPosture('● Selected option: HOLD SCOPE', posture)).toBe(false); + expect(hasPostAnswerCeoPosture('● HOLD SCOPE\r✶ Honking… (5s · ↓ 300 tokens)', posture)).toBe(false); + }); + + test('assistant output must itself contain the existing posture evidence', () => { + expect(hasPostAnswerCeoPosture(`${menu}\r● I will inspect the plan now.`, posture)).toBe(false); + expect(hasPostAnswerCeoPosture('⏺ Read(plan-ceo-review/SKILL.md)\r Review with maximum rigor.', posture)).toBe(false); + expect(hasPostAnswerCeoPosture('● high · /effort\rHOLD SCOPE', posture)).toBe(false); + expect(hasPostAnswerCeoPosture(`● I will inspect the plan now.\r${menu}`, posture)).toBe(false); + }); + + test('accepts new assistant posture after the answered-question echo, including wrapped prose', () => { + const answer = "⏺UseransweredClaude'squestions:\r⎿Reviewmode?→HOLDSCOPE"; + expect(hasPostAnswerCeoPosture(`${answer}\r● HOLD SCOPE. I will review the existing scope for failure modes.`, posture)).toBe(true); + expect(hasPostAnswerCeoPosture(`${answer}\r⏺\rI will apply maximum rigor\rto the agreed scope.`, posture)).toBe(true); + const expansion = /\b(expansion|10x|delight|dream|cathedral|opt[\s-]?in)\b/i; + expect(hasPostAnswerCeoPosture(`${answer}\r● I will explore expansion opportunities that improve the saved-view workflow.`, expansion)).toBe(true); + }); +}); diff --git a/test/helpers/autoplan-setup-question.ts b/test/helpers/autoplan-setup-question.ts new file mode 100644 index 000000000..2edb7946b --- /dev/null +++ b/test/helpers/autoplan-setup-question.ts @@ -0,0 +1,22 @@ +import { capturePlanCountQuestion } from './claude-pty-runner'; + +/** Answer only gstack's routing setup prompt in the isolated autoplan fixture. */ +export function autoplanRoutingSetupInput(visible: string, seen: Set): string | null { + const question = capturePlanCountQuestion(visible, seen, 0, true); + if (!question) return null; + + // The live capture lost characters from its qid, so require the full setup + // question and both specific choices instead of guessing from a qid fragment. + const prompt = question.promptSnippet.replace(/\s+/g, ''); + if (!/gstackworksbestwhenyourproject['’]sCLAUDE\.mdincludesskillroutingrules\.Addthemnow\?/i.test(prompt)) { + return null; + } + const options = question.options.map(option => ({ + index: option.index, + title: option.label.split(/[│┌\r\n]/, 1)[0]!.replace(/\s+/g, ''), + })); + const add = options.find(option => /^Addroutingrules(?:toCLAUDE\.md)?(?:\(Recommended\))?$/i.test(option.title)); + const decline = options.find(option => /^Nothanks(?:,I['’]llinvokeskillsmanually)?(?:\(Recommended\))?$/i.test(option.title)); + if (!add || !decline) return null; + return `${add.index}\r`; +} diff --git a/test/helpers/ceo-mode-option.ts b/test/helpers/ceo-mode-option.ts index a542ff584..d7c4b6f90 100644 --- a/test/helpers/ceo-mode-option.ts +++ b/test/helpers/ceo-mode-option.ts @@ -1,4 +1,12 @@ /** Match CEO mode labels after the PTY capture strips cursor-spacing escapes. */ +import { + capturePlanCountQuestion, + classifyPlanCountFrame, + isNumberedOptionListVisible, + planCountSubmissionInput, + type AskUserQuestionFingerprint, +} from './claude-pty-runner'; + type CeoMode = 'HOLD SCOPE' | 'SCOPE EXPANSION' | 'SELECTIVE EXPANSION' | 'SCOPE REDUCTION'; export function findCeoModeOption( @@ -24,3 +32,60 @@ export function findCeoModeOption( } return target.index; } + +type ModeNavigationAction = + | { kind: 'wait' } + | { kind: 'permission' | 'submission'; input: string } + | { kind: 'question'; question: AskUserQuestionFingerprint } + | { kind: 'mode'; index: number; question: AskUserQuestionFingerprint }; + +/** Handle native controls before deduping actual navigation questions. */ +export function nextCeoModeNavigation( + visible: string, + targetMode: CeoMode, + seenQuestions: Set, +): ModeNavigationAction { + const frame = classifyPlanCountFrame(visible); + if (frame === 'permission') return { kind: 'permission', input: '1\r' }; + const submission = frame === null ? planCountSubmissionInput(visible) : null; + if (submission !== null) return { kind: 'submission', input: submission }; + if (!isNumberedOptionListVisible(visible)) return { kind: 'wait' }; + const question = capturePlanCountQuestion(visible, seenQuestions, 0, true); + if (!question) return { kind: 'wait' }; + const index = findCeoModeOption(question.options, targetMode); + return index === null ? { kind: 'question', question } : { kind: 'mode', index, question }; +} + +/** Match new assistant prose, never the native mode menu or answer echo. */ +export function hasPostAnswerCeoPosture(visible: string, posture: RegExp): boolean { + let assistant: string[] | null = null; + const matches = () => { + if (!assistant?.length) return false; + const compact = assistant.join('').replace(/\s+/g, ''); + // Tool headings use the same bullet as assistant messages. Their output + // can quote the selected mode or the skill's posture instructions. + if (/^(?:UseransweredClaude['’]squestions|[A-Za-z][\w.:_-]*\(|(?:high|medium|low)·\/effort)/i.test(compact)) return false; + // A bare selected title gains no evidentiary value when the next terminal + // update appends a spinner or other chrome to the same captured block. + const prose = assistant.filter(line => + !/^(?:(?:You)?selected(?:option|mode)?[:：]?)?(?:HOLDSCOPE|SCOPEEXPANSION|SELECTIVEEXPANSION|SCOPEREDUCTION)(?:\(recommended\))?\.?$/i.test(line.replace(/\s+/g, '')) && + !/^[✶✻✽✢·]/.test(line), + ).join('\n'); + return prose.search(posture) !== -1; + }; + + for (const line of visible.replace(/\r\n?/g, '\n').split('\n')) { + const text = line.trim(); + const message = /^[●⏺]\s*(.*)$/.exec(text); + if (message) { + if (matches()) return true; + assistant = message[1] ? [message[1]] : []; + } else if (/^(?:[☐☒❯⎿│┌└─⏸]|←|\d+[.)]\s*|Enter\s*to\s*select)/i.test(text)) { + if (matches()) return true; + assistant = null; + } else if (assistant && text) { + assistant.push(text); + } + } + return matches(); +} diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts index 2a48bd946..25d3f625d 100644 --- a/test/helpers/claude-pty-runner.ts +++ b/test/helpers/claude-pty-runner.ts @@ -153,6 +153,19 @@ export interface ClaudePtySession { close(): Promise; } +/** Let a numbered menu apply its selection before confirming it. */ +export async function selectPtyNumberedOption( + session: Pick, + index: number, +): Promise { + if (!Number.isInteger(index) || index < 1 || index > 9) { + throw new RangeError(`Invalid numbered option: ${index}`); + } + session.send(String(index)); + await Bun.sleep(500); + session.send('\r'); +} + /** Detect a complete, recognized workspace-trust menu. */ export function isTrustDialogVisible(visible: string): boolean { return trustDialogInput(visible) !== null; @@ -288,6 +301,19 @@ export const TAIL_SCAN_BYTES = 1500; * remain unconditional. */ export function isPermissionDialogVisible(visible: string): boolean { + // Cursor-positioning escapes supply spaces visually, but stripping those + // escapes leaves labels such as "alwaysallowaccessto" in captured frames. + const compact = visible.replace(/\s+/g, ''); + if (/requestedpermissions?to|allowalledits|alwaysallowaccessto|Bashcommand.*requirespermission/i.test(compact)) { + return true; + } + // Native Write/Edit confirmation captured during the design-count eval. + // Require the native footer as well as the file question so an AUQ about + // whether the plan should overwrite a file remains a real skill question. + if (/Doyouwantto(?:overwrite|create|edit)\S+\?/i.test(compact) && + /Esctocancel[·•]Tabtoamend/i.test(compact)) { + return true; + } // Standalone signatures — high specificity, never appear in skill questions. if (/requested\s+permissions?\s+to/i.test(visible)) return true; // "Yes / Yes, allow all edits / No" shape — file-edit permission grants. @@ -710,7 +736,7 @@ export function isScopeGateAutoSelectVisible(visible: string): boolean { export function parseNumberedOptions( visible: string, ): Array<{ index: number; label: string }> { - visible = stripPtyResidue(visible); + visible = stripPtyResidue(visible).replace(/\r+\n?/g, '\n'); const tail = visible.length > 4096 ? visible.slice(-4096) : visible; // Split on lines, look for `❯ N.` or ` N.` patterns. Up to N=9. // The `\s*` after `.` (not `\s+`) is required because stripAnsi removes @@ -765,6 +791,14 @@ export function parseNumberedOptions( const cursorSegment = cursorStart >= 0 ? cursorLine.slice(cursorStart) : cursorLine; const tokenRe = /(?:^|[^0-9])([1-9])\.(?!\d)\s*/g; const tokens: Array<{ idx: number; labelStart: number; matchStart: number }> = []; + // The known cursor anchor is unambiguously an option, even when its label + // starts with a digit (captured: "❯1.1retryattempt..."). Keep the decimal + // guard for number-like text inside the remaining labels. + const firstToken = /^[\s❯]*1\.\s*/.exec(cursorSegment); + if (firstToken) { + tokens.push({ idx: 1, labelStart: firstToken[0].length, matchStart: 0 }); + tokenRe.lastIndex = firstToken[0].length; + } for (let m = tokenRe.exec(cursorSegment); m !== null; m = tokenRe.exec(cursorSegment)) { tokens.push({ idx: Number(m[1]), @@ -1056,21 +1090,16 @@ export type Step0BoundaryPredicate = ( * the same options + empty prompt across two distinct questions collide. */ export function parseQuestionPrompt(visible: string): string { - // Tail-only — older prompts higher in the buffer are stale. - const tail = visible.length > 4096 ? visible.slice(-4096) : visible; + visible = visible.replace(/\r+\n?/g, '\n'); + // Anchor context to the latest menu, not the moving end of output. After + // an answer, spinner/prose output can push just the prompt's beginning + // out of a trailing 4KB window while leaving its options visible. That + // shortened same prompt must not acquire a new finding fingerprint. + const cursor = [...visible.matchAll(/❯\s*1\./g)].at(-1); + if (!cursor) return ''; + const tail = visible.slice(Math.max(0, cursor.index - 4096), cursor.index + cursor[0].length); const lines = tail.split('\n'); - - // Find the latest line containing `❯1.` (matching parseNumberedOptions — - // unanchored to handle the box-layout case where cursor is mid-line after - // divider + header + prompt text on the same logical line). - let cursorLineIdx = -1; - for (let i = lines.length - 1; i >= 0; i--) { - if (/❯\s*1\./.test(lines[i] ?? '')) { - cursorLineIdx = i; - break; - } - } - if (cursorLineIdx < 0) return ''; + const cursorLineIdx = lines.length - 1; // Box-layout case: prompt text may be ON the cursor line, BEFORE `❯1.`. // Extract that prefix (after stripping leading box-drawing characters and @@ -1080,11 +1109,19 @@ export function parseQuestionPrompt(visible: string): string { let inlinePrompt = ''; const cursorPos = cursorLine.search(/❯\s*1\./); if (cursorPos > 0) { - inlinePrompt = cursorLine - .slice(0, cursorPos) + const prefix = cursorLine.slice(0, cursorPos); + const boxStart = Math.max(prefix.lastIndexOf('☐'), prefix.lastIndexOf('□')); + inlinePrompt = prefix + .slice(boxStart >= 0 ? boxStart : 0) // Strip box-drawing chars + dividers + leading checkbox sigil. .replace(/^[─━┄┅┈┉─┌┐└┘├┤┬┴┼│┃☐□■\s]+/, '') .trim(); + // A complete inline AUQ starts at its checkbox/header. Earlier lines + // belong to the CLI's Planning chrome or prior output, not this prompt. + // Including them can consume all 240 chars before the question starts. + if (boxStart >= 0 && inlinePrompt.length > 0) { + return inlinePrompt.replace(/\s+/g, ' ').slice(0, 240); + } } // Walk up at most 6 lines collecting prompt text. Stop at: @@ -1102,9 +1139,12 @@ export function parseQuestionPrompt(visible: string): string { continue; } blankRun = 0; + if (/^[─━┄┅┈┉╌┌┐└┘├┤┬┴┼│┃]+$/.test(trimmed) || + /^Plan+ing\s*:/i.test(trimmed)) break; // Stop if we hit what looks like a previous numbered list. if (/^[\s❯]*[1-9]\.\s+\S/.test(raw)) break; promptLines.unshift(trimmed); + if (/[☐□]/.test(trimmed)) break; } const all = inlinePrompt.length > 0 ? [...promptLines, inlinePrompt] : promptLines; @@ -1132,6 +1172,36 @@ export function auqFingerprint( return (Bun as any).hash(normalized + '||' + sig).toString(16); } +/** Capture each distinct question once, including questions sharing a menu. */ +export function capturePlanCountQuestion( + visible: string, + seen: Set, + observedAtMs: number, + preReview: boolean, +): AskUserQuestionFingerprint | null { + const options = parseNumberedOptions(visible); + if (options.length < 2) return null; + const promptSnippet = parseQuestionPrompt(visible); + if (promptSnippet === '') return null; + const signature = auqFingerprint(promptSnippet, options); + if (seen.has(signature)) return null; + seen.add(signature); + return { signature, promptSnippet, options, observedAtMs, preReview }; +} + +/** Keep a seeded count plan intact by declining its optional prerequisite. */ +export function planCountPrerequisitePick(fp: AskUserQuestionFingerprint): number | null { + // Require the recognized prerequisite body AND both opposed labels. A + // generic Skip, an outside-review offer, or a review finding keeps its + // existing answer policy. Collapsed whitespace occurs in captured PTYs. + if (!fp.preReview || !/\/office-hours/i.test(fp.promptSnippet) || + !/(?:no\s*design\s*doc|produce\s*a\s*design\s*doc)/i.test(fp.promptSnippet)) return null; + const run = fp.options.filter(({ label }) => /^Run\s*\/office-hours\s*(?:now|first)/i.test(label)); + const skip = fp.options.filter(({ label }) => + /^Skip\s*[—–-]\s*(?:proceed\s*with\s*)?standard\s*review(?:\s*\(recommended\))?$/i.test(label)); + return run.length === 1 && skip.length === 1 ? skip[0].index : null; +} + /** * Detects when a plan-* skill has reached its Completion Summary / Review * Report — a terminal signal complementary to plan-mode's "Ready to execute" @@ -1143,7 +1213,48 @@ export function auqFingerprint( * stop signal; this regex is the "we're done, go gracefully" hint. */ export const COMPLETION_SUMMARY_RE = - /(GSTACK REVIEW REPORT|## Completion [Ss]ummary|Status:\s*(clean|issues_open)|^VERDICT:)/m; + /^[\t ]*(?:[⏺●][\t ]*)?(?:#{1,6}[\t ]*)?(?:GSTACK[\t ]*REVIEW[\t ]*REPORT|Completion[\t ]*[Ss]ummary|Status:[\t ]*(?:clean|issues_open)|(?:\*\*)?VERDICT:)/m; + +/** Classify a counting frame before treating numbered native dialogs as AUQs. */ +export function classifyPlanCountFrame( + visible: string, +): 'permission' | 'completion_summary' | 'plan_ready' | null { + const report = [...visible.matchAll(new RegExp(COMPLETION_SUMMARY_RE.source, 'gm'))].at(-1); + const menuCursor = [...visible.matchAll(/❯\s*[1-9]\./g)].at(-1)?.index ?? -1; + const activeQuestionStart = Math.max( + visible.lastIndexOf('☐', menuCursor), visible.lastIndexOf('☒', menuCursor), + ); + const permissionTail = visible.slice(Math.max(visible.length - TAIL_SCAN_BYTES, activeQuestionStart, 0)); + if (isNumberedOptionListVisible(visible) && + isPermissionDialogVisible(permissionTail) && + (!report || report.index <= menuCursor)) { + return 'permission'; + } + // Headings must be assistant output, not numbered Read/Write diff rows + // such as "409 +## GSTACK REVIEW REPORT". The anchored matcher also + // leaves a completed tool's diff in scrollback without ending the review. + // A later assistant report supersedes a dismissed permission menu still + // present in short scrollback; a pending menu below the report does not. + if (report && report.index > menuCursor) return 'completion_summary'; + if (isPlanReadyVisible(visible)) return 'plan_ready'; + return null; +} + +/** Navigate native multi-question review without counting Submit as a finding. */ +export function planCountSubmissionInput(visible: string): string | null { + const bars = [...visible.matchAll(/←[^\r\n]*[☐☒][^\r\n]*✔\s*Submit\s*→/g)]; + 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; + 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 + // submit after every question tab carries the native answered marker. + return unanswered >= 0 ? '\x1b[Z'.repeat(tabs.length - unanswered) : '\r'; +} /** * Result of asserting that a plan file ends with `## GSTACK REVIEW REPORT` @@ -1258,7 +1369,7 @@ export const ceoStep0Boundary: Step0BoundaryPredicate = (fp) => fp.options.some((o) => /skip\s+interview|plan\s+immediately/i.test(o.label)); export const engStep0Boundary: Step0BoundaryPredicate = (fp) => - /scope reduction recommendation|cross[\s-]?project learnings/i.test( + /scope\s*reduction\s*recommendation|cross[\s-]*project\s*learnings/i.test( fp.promptSnippet, ) || // plan-eng-review's Step 0 may legitimately end with NO scope-reduction / @@ -1271,12 +1382,12 @@ export const engStep0Boundary: Step0BoundaryPredicate = (fp) => /gstack-qid:\s*(?:plan-)?eng-review-/i.test(fp.promptSnippet); export const designStep0Boundary: Step0BoundaryPredicate = (fp) => - /design system|design posture|design score|first dimension/i.test( + /design\s*(?:system|posture|score|completeness)|first\s*dimension/i.test( fp.promptSnippet, ); export const devexStep0Boundary: Step0BoundaryPredicate = (fp) => - /developer persona|target persona|persona selection|TTHW target/i.test( + /developer\s*persona|target\s*persona|persona\s*selection|TTHW\s*target/i.test( fp.promptSnippet, ); @@ -1938,12 +2049,16 @@ export interface PlanSkillCountObservation { } /** - * Drive a plan-* skill in plan mode and count distinct review-phase - * AskUserQuestions until a terminal signal fires. + * Drive a plan-* skill in plan mode and count distinct native review-phase + * AskUserQuestions until a terminal signal fires. Each run disables the + * extra outside review in its own gstack config: independent reviewers can + * add valid findings unrelated to the seeded-N cadence band. These counts + * do not assert outside-review dispatch or its approval-question cadence. * * Flow: * 1. Seed the complete fixture request in an isolated git repository's - * PLAN.md and initial CLAUDE.md context, then boot the PTY in that cwd + * PLAN.md and initial CLAUDE.md context, with owned native-only config, + * then boot the PTY in that cwd * (8s grace + auto-trust dialog). Skills remain registered in user scope. * 2. Send `slashCommand` alone. The fixture is already in context; sending * it later can queue it behind the skill's first question while the @@ -1956,7 +2071,9 @@ export interface PlanSkillCountObservation { * 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. + * advance. 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, * all subsequent AUQs are review-phase. * - Hard ceiling: if `reviewCount >= reviewCountCeiling`, return @@ -1994,7 +2111,8 @@ export async function runPlanSkillCounting(opts: { defaultPick?: number; /** * Optional override for the FIRST AUQ observed. Receives the fingerprint; - * returns the option index to press. Subsequent AUQs always use defaultPick. + * returns the option index to press. Subsequent review AUQs use defaultPick; + * only the recognized optional office-hours prerequisite is declined by label. * * Skill-specific routing helper: /plan-ceo-review's first AUQ asks "what * scope?" with options like "branch diff" / "describe inline" / "skip @@ -2015,14 +2133,14 @@ export async function runPlanSkillCounting(opts: { const defaultPick = opts.defaultPick ?? 1; const timeoutMs = opts.timeoutMs ?? 1_500_000; - const fixture = createPlanCountFixture(opts.followUpPrompt); + const fixture = createPlanCountFixture(opts.followUpPrompt, { nativeReviewOnly: true }); let session: ClaudePtySession; try { session = await launchClaudePty({ permissionMode: 'plan', cwd: fixture.cwd, timeoutMs: timeoutMs + 60_000, - env: opts.env, + env: { ...opts.env, ...fixture.env }, model: opts.model, seedSkills: true, }); @@ -2037,7 +2155,6 @@ export async function runPlanSkillCounting(opts: { let step0Count = 0; let reviewCount = 0; let isFirstAUQ = true; - let lastSig = ''; function snapshot( outcome: PlanSkillCountObservation['outcome'], @@ -2079,6 +2196,7 @@ export async function runPlanSkillCounting(opts: { visible, ); } + if (visible.includes('Unknown command:')) { return snapshot( 'exited', @@ -2087,6 +2205,20 @@ export async function runPlanSkillCounting(opts: { ); } + const frame = classifyPlanCountFrame(visible); + if (frame === 'permission') { + session.send(`${defaultPick}\r`); + await Bun.sleep(1500); + continue; + } + + const submissionInput = frame === null ? planCountSubmissionInput(visible) : null; + if (submissionInput !== null) { + session.send(submissionInput); + await Bun.sleep(1500); + continue; + } + // Silent write detection — only fires if no numbered prompt is on // screen (otherwise the write is gated by a permission/AUQ). const writeRe = /⏺\s*(?:Write|Edit)\(([^)]+)\)/g; @@ -2107,14 +2239,14 @@ 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 (COMPLETION_SUMMARY_RE.test(visible)) { + if (frame === 'completion_summary') { return snapshot( 'completion_summary', `skill emitted completion summary / verdict / status line (step0=${step0Count}, review=${reviewCount})`, visible, ); } - if (isPlanReadyVisible(visible)) { + if (frame === 'plan_ready') { return snapshot( 'plan_ready', `skill emitted plan-mode "Ready to execute" confirmation (step0=${step0Count}, review=${reviewCount})`, @@ -2125,46 +2257,21 @@ export async function runPlanSkillCounting(opts: { // Numbered option list? if (!isNumberedOptionListVisible(visible)) continue; - // Permission dialog? Auto-grant with defaultPick. Only act on the - // recent tail to avoid re-triggering on stale dialogs in scrollback. - if (isPermissionDialogVisible(visible.slice(-TAIL_SCAN_BYTES))) { - session.send(`${defaultPick}\r`); - await Bun.sleep(1500); - continue; - } - - // Parse the active AUQ. Skip same-redraw and empty-prompt cases. - const options = parseNumberedOptions(visible); - if (options.length < 2) continue; - const sig = optionsSignature(options); - if (sig === lastSig) continue; - const promptSnippet = parseQuestionPrompt(visible); - if (promptSnippet === '') continue; // not yet rendered, poll again - lastSig = sig; - - const fingerprintHash = auqFingerprint(promptSnippet, options); - if (seen.has(fingerprintHash)) { - // Same content, already counted (TTY redrew with whitespace diff). - continue; - } - seen.add(fingerprintHash); - - const fp: AskUserQuestionFingerprint = { - signature: fingerprintHash, - promptSnippet, - options, - observedAtMs: Date.now() - startedAt, - preReview: !boundaryFired, - }; + // Dedupe the complete question, not just its answer labels: separate + // 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 pickIdx = - isFirstAUQ && opts.firstAUQPick ? opts.firstAUQPick(fp) : defaultPick; + const prerequisitePick = planCountPrerequisitePick(fp); + const pickIdx = prerequisitePick ?? + (isFirstAUQ && opts.firstAUQPick ? opts.firstAUQPick(fp) : defaultPick); isFirstAUQ = false; - session.send(`${pickIdx}\r`); + if (prerequisitePick !== 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. diff --git a/test/helpers/claude-pty-runner.unit.test.ts b/test/helpers/claude-pty-runner.unit.test.ts index 05f54d8ee..0e4c77ba6 100644 --- a/test/helpers/claude-pty-runner.unit.test.ts +++ b/test/helpers/claude-pty-runner.unit.test.ts @@ -38,6 +38,10 @@ import { parseQuestionPrompt, auqFingerprint, COMPLETION_SUMMARY_RE, + classifyPlanCountFrame, + capturePlanCountQuestion, + planCountPrerequisitePick, + planCountSubmissionInput, assertReviewReportAtBottom, ceoStep0Boundary, engStep0Boundary, @@ -98,6 +102,24 @@ describe('isPermissionDialogVisible', () => { expect(isPermissionDialogVisible(sample)).toBe(true); }); + test('recognizes the captured collapsed native overwrite confirmation', () => { + const sample = [ + 'Doyouwanttooverwritegstack-test-plan-design.md?', + '❯1.Yes', + '2.Yes,andswitchtoacceptedits(auto-approvefileeditsandcommonfilecommands)forthissession', + '3.No', + 'Esctocancel·Tabtoamend', + ].join('\n'); + expect(isPermissionDialogVisible(sample)).toBe(true); + expect(isPermissionDialogVisible(sample.replace('Esctocancel·Tabtoamend', 'Enter to select'))).toBe(false); + }); + + test('recognizes permission labels whose cursor-positioning spaces disappeared', () => { + expect(isPermissionDialogVisible('Yes,andalwaysallowaccessto/tmp/fixtureforthissession')).toBe(true); + expect(isPermissionDialogVisible('Yes,allowalleditsduringthissession')).toBe(true); + expect(isPermissionDialogVisible('Bashcommandrequirespermission')).toBe(true); + }); + test('does NOT match a skill AskUserQuestion list', () => { const sample = ` D1 — Premise challenge: do users actually want this? @@ -921,6 +943,57 @@ describe('parseQuestionPrompt', () => { expect(prompt).not.toContain('❯'); expect(prompt).not.toMatch(/^☐/); }); + + test('keeps the captured design scope prompt ahead of long Planning chrome', () => { + // The first failed live attempt fingerprinted only the divider/Planning + // path. Its actual AUQ was later on the active cursor line. + const visible = [ + '─'.repeat(120), + `Planning: /tmp/hermetic/.claude/plans/${'long-path-'.repeat(24)}plan.md`, + '─'.repeat(120), + "☐Reviewfocus I've rated this Settings Page UI redesign plan 2/10 on design completeness. Want me to focus on specific areas? ❯1.All7passes(Recommended)", + '2.All7passesbutskipmockups', + ].join('\n'); + const prompt = parseQuestionPrompt(visible); + expect(prompt).toStartWith('Reviewfocus'); + expect(prompt).toContain('design completeness'); + expect(prompt).not.toContain('Planning:'); + expect(designStep0Boundary({ + signature: 'captured-design-scope', promptSnippet: prompt, + options: parseNumberedOptions(visible), observedAtMs: 0, preReview: true, + })).toBe(true); + }); + + test('keeps the captured devex persona header when cursor spacing collapses', () => { + const visible = [ + `Planning: /tmp/hermetic/.claude/plans/${'long-path-'.repeat(24)}plan.md`, + '─'.repeat(120), + '☐Targetpersona D2—WhoistheprimarydeveloperthisSDKtargets? ❯1.AIappbuilder/startupfounder(Recommended)', + '2.Backend/platformengineer', + ].join('\n'); + const prompt = parseQuestionPrompt(visible); + expect(prompt).toStartWith('Targetpersona'); + expect(devexStep0Boundary({ + signature: 'captured-devex-persona', promptSnippet: prompt, + options: parseNumberedOptions(visible), observedAtMs: 0, preReview: true, + })).toBe(true); + }); + + test('retains a multiline question while excluding the preceding CLI divider', () => { + const visible = [ + 'Planning: /tmp/hermetic/.claude/plans/plan.md', + '─'.repeat(120), + '☐ Review focus', + 'This plan is 2/10 on design completeness.', + 'Want me to focus on specific areas? ❯1.All 7 passes', + '2.Skip mockups', + ].join('\n'); + const prompt = parseQuestionPrompt(visible); + expect(prompt).toContain('Review focus'); + expect(prompt).toContain('design completeness'); + expect(prompt).toContain('specific areas?'); + expect(prompt).not.toContain('Planning:'); + }); }); describe('auqFingerprint', () => { @@ -976,6 +1049,156 @@ describe('auqFingerprint', () => { }); }); +describe('capturePlanCountQuestion replay', () => { + test('keeps captured CEO/eng fingerprints stable as later output trims the trailing window', () => { + // Exact prompt/option fields from the 07:30 corrected paid attempts. + // Both counted an answered Step0 question again as a review finding + // once the moving tail omitted the beginning of its prompt. + const captures = [ + { + prompt: '☐ RevewMode Which review mode should I use for the remaining sections?', + labels: [ + 'HOLD SCOPE — make it ┌┐', + 'SELECTIVEEXPANSION—│Focus:catcheverylandmineinApproachA│', + 'SCOPEREDUCTION—strip│Tests:whatmustbecovered│', + 'SCOPEEXPANSION—think│Observability:whatlogs/metricsareneeded│', + ], + }, + { + prompt: '☐ Scope cut │ D2 — Scope reduction proposal: drop TokenStore and RequestPolicy as standalone classes, inject AuthCache rather than │ exportitglobally.Acceptthisreductionbeforethesection-by-sectionreviewbegins? │ `${i === 0 ? '❯' : ''}${i + 1}.${label}`).join('\n'); + const frame = `${capture.prompt}\n${options}`; + const seen = new Set(); + const first = capturePlanCountQuestion(frame, seen, 0, true)!; + expect(first).not.toBeNull(); + // Leave the original menu within the trailing4KB, but move the + // start of that window into its question text, twice in succession. + const paddingLength = 4096 - options.length - 30; + for (const extra of [0, 15]) { + const advanced = frame + '\n' + '·'.repeat(paddingLength + extra - 1); + expect(advanced.slice(-4096)).not.toContain(capture.prompt); + expect(parseNumberedOptions(advanced)).toEqual(first.options); + expect(parseQuestionPrompt(advanced)).toBe(first.promptSnippet); + expect(auqFingerprint(parseQuestionPrompt(advanced), parseNumberedOptions(advanced))).toBe(first.signature); + expect(capturePlanCountQuestion(advanced, seen, extra + 1, false)).toBeNull(); + } + const next = `${frame}\n${'·'.repeat(paddingLength)}\n☐ Next decision Should the revised plan use these same choices?\n${options}`; + const distinct = capturePlanCountQuestion(next, seen, 20, false)!; + expect(distinct).not.toBeNull(); + expect(distinct.signature).not.toBe(first.signature); + expect(distinct.preReview).toBe(false); + expect(seen.size).toBe(2); + } + }); + + test('counts consecutive findings with identical choices and ignores redraws', () => { + const options = '\n❯1.Add to plan\n2.Defer\n3.Skip'; + const seen = new Set(); + const frames = [ + `D5 — SQL: interpolate the request parameter?${options}`, + `D5 — SQL: interpolate the request parameter?${options}`, + `D6 — Tests: no coverage for the webhook?${options}`, + `D6 — Tests: no coverage for the webhook?${options}`, + ]; + const captured = frames.map((frame, i) => capturePlanCountQuestion(frame, seen, i, false)); + expect(captured.map((question) => question !== null)).toEqual([true, false, true, false]); + expect(captured[0]?.signature).not.toBe(captured[2]?.signature); + expect(captured[2]?.promptSnippet).toContain('Tests: no coverage'); + }); + + test('does not consume an incomplete frame before its prompt arrives', () => { + const seen = new Set(); + const options = '❯1.Add to plan\n2.Defer'; + expect(capturePlanCountQuestion(options, seen, 0, true)).toBeNull(); + expect(capturePlanCountQuestion(`D1 — Pick an approach\n${options}`, seen, 1, true)).not.toBeNull(); + }); + + test('answers the captured CEO retry question with a numeric-leading first label', () => { + // The live timeout sat on this question because the first label begins + // with "1retryattempt"; it was incorrectly rejected as a decimal token. + const frame = [ + ' ☐ Retry spec', + "│ Section 5/6 finding: 'retry-with-backoff fires once, then fails clean' is ambiguous.", + "│ What does 'fires once' mean?", + '❯1.1retryattempt—Stripecalledexactly2timestotal(Recommended)', + 'Themostnaturalreading:1originalattempt+1retry=2totalStripecalls.', + '2.Addaclarifyingcommenttotheplan—lettheimplementerdecide', + '3.Theretrymechanismhandlesit—justassertfailureisreturned', + '4.Typesomething.', + '5.Chataboutthis', + 'Entertoselect·↑/↓tonavigate·Esctocancel', + ].join('\r\r'); + const question = capturePlanCountQuestion(frame, new Set(), 0, false); + expect(question?.options.map(({ index }) => index)).toEqual([1, 2, 3, 4, 5]); + expect(question?.options[0]?.label).toBe('1retryattempt—Stripecalledexactly2timestotal(Recommended)'); + expect(question?.promptSnippet).toContain('Section 5/6 finding'); + expect(question?.promptSnippet).not.toContain('Planning:'); + }); + + test('still ignores decimal numbers inside option labels', () => { + const frame = 'Choose the retry delay\r❯1.1.5 seconds\r2.Wait 2.5 seconds\r3.No retry'; + expect(parseNumberedOptions(frame)).toEqual([ + { index: 1, label: '1.5 seconds' }, + { index: 2, label: 'Wait 2.5 seconds' }, + { index: 3, label: 'No retry' }, + ]); + }); +}); + +describe('planCountPrerequisitePick replay', () => { + test('declines captured office-hours prerequisite menus by label in either order', () => { + // Captured 2026-09-08 CEO/Devex prerequisite surfaces: the default index + // sometimes starts office-hours, changing the seeded review's input. + const captures = [ + { + prompt: 'No design doc found for this branch. `/office-hours` produces a structured problem statement, premise challenge, and explored alternatives — it gives this review much sharper input. Run it now, or skip and proceed with standard review?', + labels: ['Skip — proceed with standard review (Recommended)', 'Run /office-hours first'], + }, + { + prompt: 'D2 — No design doc found. Run /office-hours first? ', + labels: ['Skip — standard review (recommended)', 'Run /office-hours now'], + }, + { + prompt: 'D3 — Run /office-hours first to produce a design doc for sharper input?', + labels: ['Skip — proceed with standard review (recommended)', 'Run /office-hours now'], + }, + ]; + for (const { prompt, labels } of captures) { + for (const reversed of [false, true]) { + for (const collapsed of [false, true]) { + const ordered = reversed ? [...labels].reverse() : labels; + const text = ['☐ Prerequisite', prompt, `❯1.${ordered[0]}`, `2.${ordered[1]}`, '3.Type something.', '4.Chat about this'].join('\r'); + const frame = collapsed ? text.replace(/ /g, '') : text; + const fp = capturePlanCountQuestion(frame, new Set(), 0, true)!; + expect(fp).not.toBeNull(); + expect(planCountPrerequisitePick(fp)).toBe(reversed ? 2 : 1); + expect(planCountPrerequisitePick({ ...fp, preReview: false })).toBeNull(); + } + } + } + }); + + test('keeps existing answers for incomplete, unrelated, and ambiguous menus', () => { + const fp = capturePlanCountQuestion( + '☐ Prerequisite\rNo design doc found. Run /office-hours first?\r❯1.Run /office-hours now\r2.Skip — proceed with standard review', + new Set(), 0, true, + )!; + expect(planCountPrerequisitePick({ ...fp, promptSnippet: 'No design doc found.' })).toBeNull(); + expect(planCountPrerequisitePick({ ...fp, promptSnippet: 'Should /office-hours skip the required SDK validation finding?' })).toBeNull(); + expect(planCountPrerequisitePick({ ...fp, promptSnippet: 'Want a second opinion from /office-hours?' })).toBeNull(); + expect(planCountPrerequisitePick({ ...fp, options: [{ index: 1, label: 'Run /office-hours now' }, { index: 2, label: 'Skip' }] })).toBeNull(); + expect(planCountPrerequisitePick({ ...fp, options: [{ index: 1, label: 'Add to plan' }, fp.options[1]] })).toBeNull(); + expect(planCountPrerequisitePick({ ...fp, options: [...fp.options, { index: 3, label: 'Skip — standard review' }] })).toBeNull(); + }); +}); + describe('COMPLETION_SUMMARY_RE', () => { test('matches GSTACK REVIEW REPORT heading', () => { expect(COMPLETION_SUMMARY_RE.test('## GSTACK REVIEW REPORT')).toBe(true); @@ -999,6 +1222,110 @@ describe('COMPLETION_SUMMARY_RE', () => { // VERDICT must be at the start of a line to count. expect(COMPLETION_SUMMARY_RE.test('the final verdict: undecided')).toBe(false); }); + + test('does NOT treat source or proposed diff rows as assistant completion', () => { + for (const line of [ + '409 +## GSTACK REVIEW REPORT', + '419 +**VERDICT:** Design Review complete — 8 decisions made.', + '+## GSTACK REVIEW REPORT', + '409→## GSTACK REVIEW REPORT', + 'The plan must end with ## GSTACK REVIEW REPORT.', + ]) expect(COMPLETION_SUMMARY_RE.test(line)).toBe(false); + }); +}); + +describe('classifyPlanCountFrame replay', () => { + test('waits through proposed Write approval and tool output, then accepts the actual report', () => { + // Sanitized rows and native prompt from the failed design-count attempt. + const proposedDiff = [ + '409 +## GSTACK REVIEW REPORT', + '416 +| Design Review | 1 | issues_open | score: 2/10 → 8/10, 8 decisions |', + '419 +**VERDICT:** Design Review complete — 8 decisions made.', + ].join('\n'); + const permission = [ + 'Doyouwanttooverwritegstack-test-plan-design.md?', + '❯1.Yes', + '2.Yes,andswitchtoacceptedits(auto-approvefileeditsandcommonfilecommands)forthissession;Yes,and', + 'alwaysallowaccessto/tmp/fixtureforthissession', + '3.No', + 'Esctocancel·Tabtoamend', + ].join('\n'); + const frames = [ + `${proposedDiff}\n${permission}`, + `${proposedDiff}\n⏺ Updated gstack-test-plan-design.md`, + `${proposedDiff}\n⏺ ## GSTACK REVIEW REPORT\nDesign Review complete — 8 decisions made.`, + ]; + expect(frames.map(classifyPlanCountFrame)).toEqual(['permission', null, 'completion_summary']); + }); + + test('a pending native permission beats even an unnumbered report heading', () => { + const visible = '## GSTACK REVIEW REPORT\nDoyouwanttooverwriteplan.md?\n❯1.Yes\n2.No\nEsctocancel·Tabtoamend'; + expect(classifyPlanCountFrame(visible)).toBe('permission'); + }); + + test('a later report supersedes the granted menu still in short scrollback', () => { + const permission = 'Doyouwanttooverwriteplan.md?\n❯1.Yes\n2.No\nEsctocancel·Tabtoamend'; + expect(classifyPlanCountFrame(permission)).toBe('permission'); + expect(classifyPlanCountFrame(`${permission}\n● ## GSTACK REVIEW REPORT`)).toBe('completion_summary'); + }); + + test('an active question after a prior report keeps the counter running', () => { + expect(classifyPlanCountFrame('## GSTACK REVIEW REPORT\nOne more choice\n❯1.Add to plan\n2.Defer')).toBeNull(); + }); + + test('an active AUQ supersedes a granted permission menu in short scrollback', () => { + const permission = 'Doyouwanttooverwriteplan.md?\n❯1.Yes\n2.No\nEsctocancel·Tabtoamend'; + const question = '☐ Error handling\nWhich failure path should we test?\n❯1.Timeout\n2.Refusal'; + expect(classifyPlanCountFrame(`${permission}\n${question}`)).toBeNull(); + }); + + test('preserves actual report variants and the native plan-ready terminal', () => { + for (const report of [ + '## GSTACK REVIEW REPORT', '⏺##GSTACKREVIEWREPORT', '●GSTACKREVIEWREPORT', + '## Completion Summary', '● ## Completion Summary', 'Status: clean', 'Status: issues_open', + 'VERDICT: CLEARED — Eng Review passed', '**VERDICT:** Design Review complete.', + ]) expect(classifyPlanCountFrame(report)).toBe('completion_summary'); + expect(classifyPlanCountFrame('Ready to execute the plan?\n❯1.Yes\n2.No, keep planning')).toBe('plan_ready'); + }); +}); + +describe('planCountSubmissionInput replay', () => { + const incomplete = [ + '← ☒ Learnings scope ☐ Approach ✔ Submit →', + 'Review your answers', + '⚠You have not answered all questions', + ' │ ●D1 — Cross-project learnings: Enable searching learnings from your other local projects?', + '→Enable cross-project (Recommended)', + 'Ready t submit your answers?', + '❯1.Submit aswers', + '2Cancel', + ].join('\r\r'); + + test('returns to the unanswered tab, then submits only after both answers', () => { + expect(planCountSubmissionInput(incomplete)).toBe('\x1b[Z'); + const nextQuestion = [ + '← ☒ Learnings scope ☐ Approach ✔ Submit →', + '│ Which approach should this plan use?', + '❯1.Extend the existing dispatcher', + '2.Add a separate handler', + ].join('\r\r'); + expect(planCountSubmissionInput(`${incomplete}\r${nextQuestion}`)).toBeNull(); + const question = capturePlanCountQuestion(nextQuestion, new Set(), 0, true); + expect(question?.promptSnippet).toContain('Which approach'); + expect(question?.options).toHaveLength(2); + const answered = incomplete.replace('☐ Approach', '☒ Approach').replace('⚠You have not answered all questions', ''); + expect(planCountSubmissionInput(answered)).toBe('\r'); + }); + + test('navigates to the first unanswered tab when more than one remains', () => { + const frame = incomplete.replace('☒ Learnings scope ☐ Approach', '☐ Learnings scope ☐ Approach ☒ Mode'); + expect(planCountSubmissionInput(frame)).toBe('\x1b[Z\x1b[Z\x1b[Z'); + }); + + test('does not revisit a stale submit panel when a later single question is active', () => { + expect(planCountSubmissionInput(`${incomplete}\r☐ Retry spec\rRetry once?\r❯1.Yes\r2.No`)).toBeNull(); + expect(planCountSubmissionInput('Ready to submit the plan?\n❯1.Submit\n2.Cancel')).toBeNull(); + }); }); describe('assertReviewReportAtBottom', () => { @@ -1140,6 +1467,18 @@ describe('Step0BoundaryPredicate per-skill', () => { expect(engStep0Boundary(f)).toBe(true); }); + test('recognizes the captured cross-project gate after cursor spacing collapses', () => { + const frame = [ + '☐Cross-project gstackcansearchlearningsfromyourotherprojectsonthismachinetofindpatternsthatmightapplytothisreview.Enablecross-projectlearnings?', + '❯1.Enablecross-projectlearnings(Recommended)', + '2.Keeplearningsproject-scopedonly', + ].join('\r\r'); + const question = capturePlanCountQuestion(frame, new Set(), 0, true)!; + expect(question).not.toBeNull(); + expect(engStep0Boundary(question)).toBe(true); + expect(engStep0Boundary(fp('Scopereductionrecommendation:cuttoMVP?', ['Reduce', 'Proceed']))).toBe(true); + }); + test('FIRES on scope reduction recommendation', () => { const f = fp('Scope reduction recommendation: cut to MVP?', ['Reduce', 'Proceed', 'Modify']); expect(engStep0Boundary(f)).toBe(true); diff --git a/test/helpers/plan-count-fixture.ts b/test/helpers/plan-count-fixture.ts index 7570a6e76..0529f7f39 100644 --- a/test/helpers/plan-count-fixture.ts +++ b/test/helpers/plan-count-fixture.ts @@ -2,6 +2,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { spawnSync } from 'node:child_process'; +import { getHermeticDirs } from './hermetic-env'; /** * Count evals review a seeded plan, never the checkout that supplies skills. @@ -9,13 +10,46 @@ import { spawnSync } from 'node:child_process'; * 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): { +export function createPlanCountFixture(prompt: string, opts: { nativeReviewOnly?: boolean } = {}): { cwd: string; + env: Record; cleanup(): void; } { const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-plan-count-')); - const cleanup = () => fs.rmSync(cwd, { recursive: true, force: true }); + let stateRoot: string | undefined; + const env: Record = {}; + const cleanup = () => { + try { + fs.rmSync(cwd, { recursive: true, force: true }); + } finally { + if (stateRoot) fs.rmSync(stateRoot, { recursive: true, force: true }); + } + }; try { + if (opts.nativeReviewOnly) { + // Seeded-N count bands measure the main review's finding cadence. + // An outside review can legitimately add findings beyond that band; + // these fixtures do not cover its separate approval-question cadence. + // Opt in only from runPlanSkillCounting: mode fixtures keep defaults. + const sharedState = getHermeticDirs().gstackHome; + stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-plan-count-state-')); + for (const entry of fs.readdirSync(sharedState, { withFileTypes: true })) { + // Preserve onboarding seeds, without copying sibling review logs or + // artifacts from the per-process shared hermetic state directory. + if (entry.isFile() && (entry.name === '.activated' || + /^\..*(?:-seen|-prompted|-shown)$/.test(entry.name) || + entry.name.startsWith('.feature-prompted-'))) { + fs.copyFileSync(path.join(sharedState, entry.name), path.join(stateRoot, entry.name)); + } + } + const config = fs.readFileSync(path.join(sharedState, 'config.yaml'), 'utf8') + .replace(/^codex_reviews:.*(?:\r?\n|$)/gm, ''); + fs.writeFileSync(path.join(stateRoot, 'config.yaml'), config + '\ncodex_reviews: disabled\n'); + // Config readers prefer STATE_ROOT, while onboarding writers use HOME. + // Both must resolve to owned state, even with explicit caller overrides. + env.GSTACK_HOME = stateRoot; + env.GSTACK_STATE_ROOT = stateRoot; + } fs.writeFileSync(path.join(cwd, 'PLAN.md'), prompt); fs.writeFileSync(path.join(cwd, 'CLAUDE.md'), [ '# Plan review fixture', @@ -45,7 +79,7 @@ export function createPlanCountFixture(prompt: string): { 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']); - return { cwd, cleanup }; + return { cwd, env, cleanup }; } catch (error) { cleanup(); throw error; diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 72e0b156b..627c15ea7 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -139,7 +139,7 @@ export const E2E_TOUCHFILES: Record = { // Each one tests behavior the SDK harness can't observe (rendered TTY, // numbered-option lists, multi-phase ordering, idempotency state echo). 'auq-format-gate': ['plan-ceo-review/**', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completeness-section.ts', 'scripts/resolvers/preamble.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/helpers/llm-judge.ts', 'test/skill-e2e-ask-user-question-format-compliance.test.ts'], - 'plan-ceo-mode-routing': ['plan-ceo-review/**', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/ceo-mode-option.ts', 'test/ceo-mode-option.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-mode-routing.test.ts'], + 'plan-ceo-mode-routing': ['plan-ceo-review/**', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/ceo-mode-option.ts', 'test/ceo-mode-option.test.ts', 'test/pty-option-selection.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-mode-routing.test.ts'], 'plan-design-with-ui-scope': ['plan-design-review/**', 'test/fixtures/plans/ui-heavy-feature.md', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/skill-e2e-plan-design-with-ui.test.ts'], 'ship-idempotency-pty': ['ship/**', 'bin/gstack-next-version', 'bin/gstack-version-bump', 'scripts/resolvers/sections.ts', 'lib/worktree.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/skill-e2e-ship-idempotency.test.ts'], 'tpa-present': ['scripts/resolvers/third-party-actions.ts', 'ship/SKILL.md.tmpl', 'ship/sections/apple-release.md.tmpl', 'scripts/gen-skill-docs.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-third-party-actions.test.ts'], @@ -154,16 +154,16 @@ export const E2E_TOUCHFILES: Record = { // the selector sets GSTACK_CARVE_SKILL= to scope cost to the changed // skill (D-CODEX A). Touching the registry/helper or sections.ts runs all. 'carve-section-loading': ['design-html/**', 'design-shotgun/**', 'qa/**', 'browse/**', 'retro/**', 'autoplan/**', 'spec/**', 'setup-gbrain/**', 'review/**', 'codex/**', 'land-and-deploy/**', 'plan-eng-review/**', 'plan-design-review/**', 'plan-devex-review/**', 'office-hours/**', 'document-release/**', 'design-consultation/**', 'cso/**', 'test/helpers/carve-guards.ts', 'scripts/resolvers/sections.ts', 'scripts/resolvers/redact-doc.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/carve-section-loading.test.ts'], - 'autoplan-chain-pty': ['autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/ui-heavy-feature.md', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/skill-e2e-autoplan-chain.test.ts'], + 'autoplan-chain-pty': ['autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/ui-heavy-feature.md', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/skill-e2e-autoplan-chain.test.ts'], // Per-finding AskUserQuestion count + review-report-at-bottom assertion. // Each test drives its skill end-to-end; touchfiles include preamble + // completion-status resolvers because they affect question cadence and // terminal output (the regression surface this test catches). - 'plan-ceo-finding-count': ['bin/gstack-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-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/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-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/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-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/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-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'], // 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-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/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': ['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-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'], '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-fixture.test.ts b/test/plan-count-fixture.test.ts index 95a6e0183..82ca00fe1 100644 --- a/test/plan-count-fixture.test.ts +++ b/test/plan-count-fixture.test.ts @@ -6,6 +6,7 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { pathToFileURL } from 'node:url'; import { createPlanCountFixture } from './helpers/plan-count-fixture'; +import { getHermeticDirs } from './helpers/hermetic-env'; const ROOT = path.resolve(import.meta.dir, '..'); const PROMPT = '# Seeded settings plan\n\nReview each issue separately.\n' + @@ -30,12 +31,41 @@ describe('plan-count fixtures', () => { expect(fs.existsSync(path.join(fixture.cwd, 'DESIGN.md'))).toBe(false); expect(fs.existsSync(path.join(fixture.cwd, 'TODOS.md'))).toBe(false); expect(fs.existsSync(path.join(fixture.cwd, 'never'))).toBe(false); + expect(fixture.env).toEqual({}); // Mode-routing callers keep normal config. } finally { fixture.cleanup(); } expect(fs.existsSync(fixture.cwd)).toBe(false); }); + test('native count config owns independent state and preserves shared onboarding seeds', () => { + const shared = getHermeticDirs().gstackHome; + const before = fs.readFileSync(path.join(shared, 'config.yaml'), 'utf8'); + const markers = fs.readdirSync(shared).filter(name => name.startsWith('.')); + const first = createPlanCountFixture('first native plan', { nativeReviewOnly: true }); + const second = createPlanCountFixture('second native plan', { nativeReviewOnly: true }); + try { + expect(first.env.GSTACK_HOME).not.toBe(shared); + expect(first.env.GSTACK_HOME).not.toBe(second.env.GSTACK_HOME); + expect(first.env.GSTACK_STATE_ROOT).toBe(first.env.GSTACK_HOME); + expect(fs.readFileSync(path.join(first.env.GSTACK_HOME, 'config.yaml'), 'utf8')) + .toBe(before.replace(/^codex_reviews:.*(?:\r?\n|$)/gm, '') + '\ncodex_reviews: disabled\n'); + for (const marker of markers) { + expect(fs.readFileSync(path.join(first.env.GSTACK_HOME, marker), 'utf8')) + .toBe(fs.readFileSync(path.join(shared, marker), 'utf8')); + } + first.cleanup(); + first.cleanup(); + expect(fs.existsSync(first.env.GSTACK_HOME)).toBe(false); + expect(fs.existsSync(second.env.GSTACK_HOME)).toBe(true); + expect(fs.readFileSync(path.join(shared, 'config.yaml'), 'utf8')).toBe(before); + } finally { + first.cleanup(); + second.cleanup(); + } + expect(fs.existsSync(second.env.GSTACK_HOME)).toBe(false); + }); + test('concurrent fixtures have independent content and cleanup owns only its directory', () => { const first = createPlanCountFixture('first plan'); const second = createPlanCountFixture('second plan'); @@ -98,10 +128,16 @@ try { const fakePath = path.join(dir, 'fake-claude'); const workerPath = path.join(dir, 'worker.ts'); const resultPath = path.join(dir, 'result.json'); + const hostState = path.join(dir, 'host-state'); + const hostConfig = 'codex_reviews: enabled\nexplain_level: expert\n'; + fs.mkdirSync(hostState); + fs.writeFileSync(path.join(hostState, 'config.yaml'), hostConfig); const cases = [ { name: 'design', skillName: 'plan-design-review', prompt: PROMPT, mode: 'complete' }, { 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 }, ].map((item) => ({ ...item, record: path.join(dir, `${item.name}.jsonl`) })); // The fake snapshots its environment BEFORE installing the first stdin @@ -114,8 +150,16 @@ const record = (event) => fs.appendFileSync(process.env.FIXTURE_RECORD, JSON.str 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'); +const config = (key) => { + const result = spawnSync('bash', [process.env.FIXTURE_CONFIG_BIN, 'get', key], { encoding: 'utf8', timeout: 10000 }); + if (result.status !== 0) throw new Error('Fixture config failed: ' + result.stderr); + return result.stdout; +}; record({ type: 'startup', pid: process.pid, cwd: process.cwd(), argv: process.argv.slice(2), + stateRoot: process.env.GSTACK_STATE_ROOT, gstackHome: process.env.GSTACK_HOME, + codexReviews: config('codex_reviews'), explainLevel: config('explain_level'), + onboarding: fs.readdirSync(process.env.GSTACK_HOME).filter(name => name.startsWith('.')), gitRoot: spawnSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', timeout: 10_000 }).stdout.trim(), plan: fs.existsSync(planPath) ? fs.readFileSync(planPath, 'utf8') : null, context: fs.existsSync(contextPath) ? fs.readFileSync(contextPath, 'utf8') : null, @@ -124,15 +168,40 @@ record({ sections: fs.existsSync(path.join(skillDir, 'sections', 'review-sections.md')) ? fs.readFileSync(path.join(skillDir, 'sections', 'review-sections.md'), 'utf8') : null, }); +fs.writeFileSync(path.join(process.env.GSTACK_HOME, 'child-owned'), 'isolated'); if (process.env.FIXTURE_MODE === 'exit') process.stdout.write('\x1b[?25lSTARTUP_DIAGNOSTIC fixture CLI booted\x1b[?25h\n'); process.stdin.setRawMode?.(true); let firstInput = true; let completion; +let question = 0; +let selected = ''; process.stdin.on('data', (data) => { - record({ type: 'input', data: data.toString('utf8') }); - if (!firstInput) return; + const input = data.toString('utf8'); + record({ type: 'input', data: input }); + if (!firstInput) { + if (process.env.FIXTURE_MODE !== 'prerequisite') return; + if (question === 1 && input.includes('\r')) { + record({ type: 'generic-answer', input }); + question = 2; + const labels = ['Run /office-hours now', 'Skip — proceed with standard review']; + if (process.env.FIXTURE_SKIP_INDEX === '1') labels.reverse(); + 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')) { + record({ type: 'prerequisite-answer', selected }); + process.stdout.write(selected === process.env.FIXTURE_SKIP_INDEX ? '\nGSTACK REVIEW REPORT\n' : '\nWRONG_PREREQUISITE_CHOICE\n'); + } + return; + } firstInput = false; if (process.env.FIXTURE_MODE === 'exit') process.exit(7); + if (process.env.FIXTURE_MODE === 'prerequisite') { + question = 1; + process.stdout.write('\r☐ Setup\rWhich fixture setup should be used?\r❯1.First setup\r2.Second setup\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); @@ -146,8 +215,16 @@ 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; fs.writeFileSync(workerPath, ` import { runPlanSkillCounting } from ${JSON.stringify(runnerUrl)}; +import { getHermeticDirs } from ${JSON.stringify(hermeticUrl)}; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +const shared = getHermeticDirs().gstackHome; +fs.appendFileSync(path.join(shared, 'config.yaml'), 'codex_reviews: enabled\\nexplain_level: beginner\\n'); +const sharedBefore = fs.readFileSync(path.join(shared, 'config.yaml'), 'utf8'); +const onboarding = fs.readdirSync(shared).filter(name => name.startsWith('.')); const cases = ${JSON.stringify(cases)}; const results = await Promise.all(cases.map(async (item) => ({ name: item.name, @@ -158,16 +235,24 @@ const results = await Promise.all(cases.map(async (item) => ({ isLastStep0AUQ: () => false, reviewCountCeiling: 8, timeoutMs: 15000, - env: { FIXTURE_RECORD: item.record, FIXTURE_SKILL: item.skillName, FIXTURE_MODE: item.mode }, + firstAUQPick: () => 2, + env: { + FIXTURE_RECORD: item.record, FIXTURE_SKILL: item.skillName, FIXTURE_MODE: item.mode, + 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)}, + }, }), }))); -await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify(results)); +await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify({ results, onboarding, sharedBefore, + sharedAfter: fs.readFileSync(path.join(shared, 'config.yaml'), 'utf8'), + sharedHasChildFile: fs.existsSync(path.join(shared, 'child-owned')), +})); `); const startedAt = Date.now(); const child = Bun.spawn([process.execPath, workerPath], { cwd: ROOT, - env: { ...process.env, BROWSE_TERMINAL_BINARY: fakePath, EVALS_HERMETIC: '1' }, + env: { ...process.env, BROWSE_TERMINAL_BINARY: fakePath, EVALS_HERMETIC: '1', GSTACK_HOME: hostState, GSTACK_STATE_ROOT: hostState }, stdout: 'pipe', stderr: 'pipe', }); @@ -180,14 +265,25 @@ await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify(results)); ]); expect(exitCode, stdout + stderr).toBe(0); expect(Date.now() - startedAt).toBeLessThan(35_000); - const results = JSON.parse(fs.readFileSync(resultPath, 'utf8')); + const report = JSON.parse(fs.readFileSync(resultPath, 'utf8')); + const results = report.results; + expect(report.sharedAfter).toBe(report.sharedBefore); + expect(report.sharedHasChildFile).toBe(false); + expect(fs.readFileSync(path.join(hostState, 'config.yaml'), 'utf8')).toBe(hostConfig); + expect(fs.readdirSync(hostState)).toEqual(['config.yaml']); const cwds = new Set(); + const stateRoots = new Set(); for (const item of cases) { const events = fs.readFileSync(item.record, 'utf8').trim().split('\n').map((line) => JSON.parse(line)); const startup = events[0]; expect(startup.type).toBe('startup'); expect(startup.cwd).not.toBe(ROOT); expect(startup.gitRoot).toBe(startup.cwd); + expect(startup.stateRoot).toBe(startup.gstackHome); + expect(startup.stateRoot).not.toBe(hostState); + expect(startup.codexReviews).toBe('disabled'); + expect(startup.explainLevel).toBe('beginner'); + expect(startup.onboarding).toEqual(report.onboarding); expect(startup.plan).toBe(item.prompt); expect(startup.context).toContain('PLAN.md'); expect(startup.context).toContain(item.prompt); @@ -197,8 +293,9 @@ await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify(results)); 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`); + .toBe(`/${item.skillName}\r` + (item.mode === 'prerequisite' ? `2\r${item.skipIndex}\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'); @@ -208,9 +305,16 @@ await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify(results)); expect(result.observation.evidence).not.toContain('\x1b'); } 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.reviewCount).toBe(0); + } cwds.add(startup.cwd); + stateRoots.add(startup.stateRoot); } expect(cwds.size).toBe(cases.length); + expect(stateRoots.size).toBe(cases.length); } finally { clearTimeout(killer); child.kill('SIGKILL'); diff --git a/test/pty-option-selection.test.ts b/test/pty-option-selection.test.ts new file mode 100644 index 000000000..6bda606b8 --- /dev/null +++ b/test/pty-option-selection.test.ts @@ -0,0 +1,107 @@ +/** Free PTY replay of numbered-menu selection with deferred cursor updates. */ +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 { pathToFileURL } from 'node:url'; + +describe('PTY numbered-option selection', () => { + test.skipIf(process.platform === 'win32')('waits for the selected option to update before confirming', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-option-replay-')); + const fake = path.join(dir, 'fake-claude'); + const worker = path.join(dir, 'worker.ts'); + const resultFile = path.join(dir, 'result.json'); + const cases = [ + { name: 'coalesced-second', index: 2, coalesced: true, expected: 1 }, + { name: 'selected-second', index: 2, coalesced: false, expected: 2 }, + { name: 'selected-third', index: 3, coalesced: false, expected: 3 }, + ].map(item => ({ ...item, cwd: path.join(dir, item.name), record: path.join(dir, item.name + '.jsonl') })); + for (const item of cases) fs.mkdirSync(item.cwd); + + fs.writeFileSync(fake, `#!${process.execPath}\n` + String.raw` +import * as fs from 'node:fs'; +const item = JSON.parse(process.env.OPTION_REPLAY); +const record = (event) => fs.appendFileSync(item.record, JSON.stringify(event) + '\n'); +record({type:'startup', pid:process.pid, cwd:process.cwd()}); +process.stdin.setRawMode?.(true); +let selected = 1; +let confirmed = false; +process.stdin.on('data', (data) => { + record({type:'input', data:data.toString()}); + for (const key of data.toString()) { + if (/^[1-3]$/.test(key)) { + // Model a menu whose selection state commits on a later render. + // Enter in the same input turn sees the previous selected option. + setTimeout(() => { + selected = Number(key); + record({type:'selected', selected}); + }, 100); + } else if (key === '\r' && !confirmed) { + confirmed = true; + record({type:'confirmed', selected}); + process.stdout.write('\nCONFIRMED:' + selected + '\n'); + } + } +}); +process.stdout.write('OPTION_MENU_READY\n❯1.First\n2.Second\n3.Third\n'); +process.on('SIGINT', () => process.exit(0)); +process.stdin.resume(); +`); + fs.chmodSync(fake, 0o755); + const runner = pathToFileURL(path.resolve(import.meta.dir, 'helpers/claude-pty-runner.ts')).href; + fs.writeFileSync(worker, ` +import { launchClaudePty, selectPtyNumberedOption } from ${JSON.stringify(runner)}; +const cases = ${JSON.stringify(cases)}; +const results = await Promise.all(cases.map(async item => { + const session = await launchClaudePty({cwd:item.cwd, timeoutMs:5000, env:{OPTION_REPLAY:JSON.stringify(item)}}); + try { + await session.waitFor('OPTION_MENU_READY', {timeoutMs:2000, pollMs:20}); + if (item.coalesced) session.send(String(item.index) + '\\r'); + else await selectPtyNumberedOption(session, item.index); + await session.waitFor('CONFIRMED:', {timeoutMs:2000, pollMs:20}); + const selected = Number(session.visibleText().match(/CONFIRMED:([1-3])/)?.[1]); + return {name:item.name, selected}; + } finally { await session.close(); } +})); +await Bun.write(${JSON.stringify(resultFile)}, JSON.stringify(results)); +`); + const child = Bun.spawn([process.execPath, worker], { + env: { ...process.env, BROWSE_TERMINAL_BINARY: fake, EVALS_HERMETIC: '1' }, + stdout: 'pipe', stderr: 'pipe', + }); + const killer = setTimeout(() => child.kill('SIGKILL'), 8000); + try { + const [exitCode, stdout, stderr] = await Promise.all([ + child.exited, new Response(child.stdout).text(), new Response(child.stderr).text(), + ]); + expect(exitCode, stdout + stderr).toBe(0); + expect(JSON.parse(fs.readFileSync(resultFile, 'utf8'))).toEqual( + cases.map(item => ({ name: item.name, selected: item.expected })), + ); + for (const item of cases) { + const events = fs.readFileSync(item.record, 'utf8').trim().split('\n').map(line => JSON.parse(line)); + expect(events[0].cwd).toBe(fs.realpathSync(item.cwd)); + const inputs = events.filter(event => event.type === 'input'); + expect(inputs.map(event => event.data).join('')).toBe(`${item.index}\r`); + const confirmations = events.filter(event => event.type === 'confirmed'); + expect(confirmations).toEqual([{ type: 'confirmed', selected: item.expected }]); + if (!item.coalesced) { + expect(inputs.map(event => event.data)).toEqual([String(item.index), '\r']); + const selectionIndex = events.findIndex(event => event.type === 'selected'); + expect(selectionIndex).toBeGreaterThan(-1); + expect(selectionIndex).toBeLessThan(events.findIndex(event => event.type === 'confirmed')); + } + expect(() => process.kill(events[0].pid, 0)).toThrow(); + } + } finally { + clearTimeout(killer); + child.kill('SIGKILL'); + for (const item of cases) { + if (!fs.existsSync(item.record)) continue; + const first = JSON.parse(fs.readFileSync(item.record, 'utf8').split('\n')[0]!); + try { process.kill(first.pid, 'SIGKILL'); } catch { /* already reaped */ } + } + fs.rmSync(dir, { recursive: true, force: true }); + } + }, 10_000); +}); diff --git a/test/skill-e2e-autoplan-chain.test.ts b/test/skill-e2e-autoplan-chain.test.ts index 9d9e669c7..79d780b3b 100644 --- a/test/skill-e2e-autoplan-chain.test.ts +++ b/test/skill-e2e-autoplan-chain.test.ts @@ -38,7 +38,9 @@ import { isPlanReadyVisible, isPermissionDialogVisible, isNumberedOptionListVisible, + selectPtyNumberedOption, } from './helpers/claude-pty-runner'; +import { autoplanRoutingSetupInput } from './helpers/autoplan-setup-question'; const describeE2E = describeE2ETier('periodic'); @@ -103,6 +105,7 @@ describeE2E('/autoplan chain ordering (periodic)', () => { const phasePattern = /\*\*Phase\s+(\d+(?:\.\d+)?)\s+complete\.?\*\*/g; let lastPermSig = ''; + const seenSetupQuestions = new Set(); while (Date.now() - start < budgetMs) { await Bun.sleep(5000); if (session.exited()) { @@ -113,9 +116,8 @@ describeE2E('/autoplan chain ordering (periodic)', () => { const visible = session.visibleSince(since); // Auto-grant any permission dialog so autoplan can keep moving - // through its phases. The autoplan template auto-decides AskUserQuestions - // it owns; only permission prompts (file/tool grants) need our - // hand-pressing. Classify on tail to avoid stale matches. + // through its phases. The autoplan template auto-decides review + // questions it owns. Classify on tail to avoid stale matches. const recentTail = visible.slice(-1500); if (isNumberedOptionListVisible(recentTail) && isPermissionDialogVisible(recentTail)) { const sig = visible.slice(-500); @@ -127,6 +129,17 @@ describeE2E('/autoplan chain ordering (periodic)', () => { } } + // This new repository also asks once to add gstack routing to + // CLAUDE.md. Answer only that recognized setup prompt; review and + // taste decisions remain autoplan's responsibility. The helper + // deduplicates the complete question before returning an input. + const setupInput = autoplanRoutingSetupInput(visible, seenSetupQuestions); + if (setupInput !== null) { + await selectPtyNumberedOption(session, Number(setupInput.trim())); + await Bun.sleep(2000); + continue; + } + // Re-scan for any phase markers we haven't yet recorded. phasePattern.lastIndex = 0; let m: RegExpExecArray | null; diff --git a/test/skill-e2e-plan-ceo-finding-count.test.ts b/test/skill-e2e-plan-ceo-finding-count.test.ts index b173a65e8..9aff272dd 100644 --- a/test/skill-e2e-plan-ceo-finding-count.test.ts +++ b/test/skill-e2e-plan-ceo-finding-count.test.ts @@ -166,11 +166,7 @@ describeE2E('/plan-ceo-review per-finding AskUserQuestion count (periodic)', () if (obs.reviewCount > CEILING_DISTINCT) { throw new Error( `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING_DISTINCT}.\n` + - `Possible over-asking regression. Review-phase fingerprints:\n` + - obs.fingerprints - .filter((f) => !f.preReview) - .map((f) => ` - "${f.promptSnippet.slice(0, 80)}"`) - .join('\n'), + `Captured observation:\n${JSON.stringify(obs, null, 2)}`, ); } @@ -243,7 +239,8 @@ describeE2E('/plan-ceo-review per-finding AskUserQuestion count (periodic)', () } if (obs.reviewCount > CEILING_PAIRED) { throw new Error( - `PAIRED CONTROL FAIL: reviewCount=${obs.reviewCount} > CEILING=${CEILING_PAIRED} (over-asking on a 2-finding fixture).`, + `PAIRED CONTROL FAIL: reviewCount=${obs.reviewCount} > CEILING=${CEILING_PAIRED} (over-asking on a 2-finding fixture).\n` + + `Captured observation:\n${JSON.stringify(obs, null, 2)}`, ); } } finally { diff --git a/test/skill-e2e-plan-ceo-mode-routing.test.ts b/test/skill-e2e-plan-ceo-mode-routing.test.ts index a46bb5dad..d09b035ff 100644 --- a/test/skill-e2e-plan-ceo-mode-routing.test.ts +++ b/test/skill-e2e-plan-ceo-mode-routing.test.ts @@ -36,14 +36,11 @@ import { describeE2ETier } from './helpers/e2e-gate'; import { launchClaudePty, isNumberedOptionListVisible, - isPermissionDialogVisible, - parseNumberedOptions, isPlanReadyVisible, - optionsSignature, - TAIL_SCAN_BYTES, + selectPtyNumberedOption, type ClaudePtySession, } from './helpers/claude-pty-runner'; -import { findCeoModeOption } from './helpers/ceo-mode-option'; +import { hasPostAnswerCeoPosture, nextCeoModeNavigation } from './helpers/ceo-mode-option'; import { createPlanCountFixture } from './helpers/plan-count-fixture'; const describeE2E = describeE2ETier('periodic'); @@ -101,7 +98,7 @@ async function navigateToModeAskUserQuestion( const budgetMs = opts.budgetMs ?? 420_000; const start = Date.now(); let priorAnswered = 0; - let lastSeenList: Array<{ index: number; label: string }> = []; + const seenQuestions = new Set(); while (Date.now() - start < budgetMs) { if (session.exited()) { @@ -112,44 +109,25 @@ async function navigateToModeAskUserQuestion( } await Bun.sleep(2000); const visible = session.visibleSince(since); - if (!isNumberedOptionListVisible(visible)) continue; - const opts = parseNumberedOptions(visible); - if (opts.length < 2) continue; - - // Has the rendered list changed since last poll? If not, we're seeing - // the same prompt and shouldn't double-press. - const sig = optionsSignature(opts); - const lastSig = optionsSignature(lastSeenList); - if (sig === lastSig) continue; - lastSeenList = opts; - - // Is THIS the mode AskUserQuestion? - const modeIndex = findCeoModeOption(opts, targetMode); - if (modeIndex !== null) { - return { modeIndex, visibleAtMode: visible }; - } - - // Permission dialog? Grant with "1" but don't count it against nav budget. - // Classify on the recent tail only — old permission text persists in - // visibleSince and would re-trigger forever. - // - // Note: runPlanSkillObservation has its own permission-dialog filter that - // simply skips classification (since it observes, doesn't drive). This nav - // loop drives the PTY directly via launchClaudePty and so owns its own - // dialog handling — granting with "1" so the workflow advances. Both - // paths share TAIL_SCAN_BYTES as the recent-tail window so tuning stays - // in sync. - if (isPermissionDialogVisible(visible.slice(-TAIL_SCAN_BYTES))) { - session.send('1\r'); + const action = nextCeoModeNavigation(visible, targetMode, seenQuestions); + if (action.kind === 'wait') continue; + // Native permission and multi-question Submit menus are controls, not + // review questions, so neither consumes the navigation question budget. + if (action.kind === 'permission' || action.kind === 'submission') { + session.send(action.input); await Bun.sleep(1500); continue; } + if (action.kind === 'mode') { + return { modeIndex: action.index, visibleAtMode: visible }; + } // Not the mode AskUserQuestion — answer with option 1 (recommended) and continue. if (priorAnswered >= maxNav) { throw new Error( `Navigated ${maxNav} prior AskUserQuestions without reaching the mode AskUserQuestion. ` + - `Last list:\n${opts.map(o => ` ${o.index}. ${o.label}`).join('\n')}`, + `Target: ${targetMode}. Last question: ${action.question.promptSnippet}\n` + + `Last list:\n${action.question.options.map(o => ` ${o.index}. ${o.label}`).join('\n')}`, ); } priorAnswered++; @@ -157,7 +135,10 @@ async function navigateToModeAskUserQuestion( // Give the agent a beat to advance before re-polling. await Bun.sleep(2000); } - throw new Error(`Mode AskUserQuestion not reached within ${budgetMs}ms`); + throw new Error( + `Mode AskUserQuestion for "${targetMode}" not reached within ${budgetMs}ms; priorAnswered=${priorAnswered}.\n` + + `--- latest visible navigation (last 3KB) ---\n${session.visibleSince(since).slice(-3000)}`, + ); } describeE2E('/plan-ceo-review mode routing (gate)', () => { @@ -178,11 +159,12 @@ describeE2E('/plan-ceo-review mode routing (gate)', () => { const since = session.mark(); session.send('/plan-ceo-review\r'); - const { modeIndex } = await navigateToModeAskUserQuestion(session, since, c.mode); + const { modeIndex, visibleAtMode } = await navigateToModeAskUserQuestion(session, since, c.mode); - // Snapshot the visible buffer at mode-pick time, then send the index. - const sincePick = session.rawOutput().length; - session.send(`${modeIndex}\r`); + // The digit redraws the menu before Enter confirms it. Start after + // confirmation so its mode names cannot count as assistant posture. + await selectPtyNumberedOption(session, modeIndex); + const sincePick = session.mark(); // Wait for downstream evidence: either next AskUserQuestion or plan_ready or // a posture-distinctive substring shows up. @@ -194,12 +176,12 @@ describeE2E('/plan-ceo-review mode routing (gate)', () => { await Bun.sleep(2500); if (session.exited()) { throw new Error( - `claude exited (code=${session.exitCode()}) after mode pick.\n` + + `claude exited (code=${session.exitCode()}) after mode "${c.mode}" option ${modeIndex}.\n` + `Downstream:\n${session.visibleSince(sincePick).slice(-2000)}`, ); } downstreamSnapshot = session.visibleSince(sincePick); - if (c.postureRe.test(downstreamSnapshot)) { + if (hasPostAnswerCeoPosture(downstreamSnapshot, c.postureRe)) { postureMatched = true; break; } @@ -209,7 +191,7 @@ describeE2E('/plan-ceo-review mode routing (gate)', () => { if ( isPlanReadyVisible(downstreamSnapshot) && isNumberedOptionListVisible(downstreamSnapshot) && - !c.postureRe.test(downstreamSnapshot) + !hasPostAnswerCeoPosture(downstreamSnapshot, c.postureRe) ) { // Plan-ready AND a follow-up AskUserQuestion are both visible but // posture text has not appeared yet. Keep polling for a bit. @@ -217,7 +199,8 @@ describeE2E('/plan-ceo-review mode routing (gate)', () => { } if (!postureMatched) { throw new Error( - `Mode "${c.mode}" routing FAILED: no posture match for ${c.postureRe.source}.\n` + + `Mode "${c.mode}" routing FAILED after sending option ${modeIndex}: no posture match for ${c.postureRe.source}.\n` + + `--- observed mode menu (last 3KB) ---\n${visibleAtMode.slice(-3000)}\n` + `--- downstream visible since mode pick (last 3KB) ---\n` + downstreamSnapshot.slice(-3000), ); diff --git a/test/skill-e2e-plan-design-finding-count.test.ts b/test/skill-e2e-plan-design-finding-count.test.ts index 073b68470..dfd4bb02a 100644 --- a/test/skill-e2e-plan-design-finding-count.test.ts +++ b/test/skill-e2e-plan-design-finding-count.test.ts @@ -105,7 +105,8 @@ describeE2E('/plan-design-review per-finding AskUserQuestion count (periodic)', } if (obs.reviewCount > CEILING) { throw new Error( - `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING}.`, + `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING}.\n` + + `Captured observation:\n${JSON.stringify(obs, null, 2)}`, ); } diff --git a/test/skill-e2e-plan-devex-finding-count.test.ts b/test/skill-e2e-plan-devex-finding-count.test.ts index 1e4edb068..4b4762435 100644 --- a/test/skill-e2e-plan-devex-finding-count.test.ts +++ b/test/skill-e2e-plan-devex-finding-count.test.ts @@ -100,7 +100,8 @@ describeE2E('/plan-devex-review per-finding AskUserQuestion count (periodic)', ( } if (obs.reviewCount > CEILING) { throw new Error( - `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING}.`, + `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING}.\n` + + `Captured observation:\n${JSON.stringify(obs, null, 2)}`, ); } diff --git a/test/skill-e2e-plan-eng-finding-count.test.ts b/test/skill-e2e-plan-eng-finding-count.test.ts index 566776dd9..8c0c32c26 100644 --- a/test/skill-e2e-plan-eng-finding-count.test.ts +++ b/test/skill-e2e-plan-eng-finding-count.test.ts @@ -99,7 +99,8 @@ describeE2E('/plan-eng-review per-finding AskUserQuestion count (periodic)', () } if (obs.reviewCount > CEILING) { throw new Error( - `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING}.`, + `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING}.\n` + + `Captured observation:\n${JSON.stringify(obs, null, 2)}`, ); }