From 4781852947223db8abb1c626e2f7cc4206ddffe7 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 19:47:01 +0000 Subject: [PATCH] fix: preserve planning decisions and terminal evidence Load installed host reviews at autoplan phase entry and wait for completed reviewers and saved artifacts. Reuse approved remedies while preserving individual finding decisions. Drive interactive evals from the current terminal viewport, bind native questions across scrolling, and require complete native report evidence. Cover captured stale menus, permission lifecycles, setup classification, and disabled-review tool availability with deterministic regressions. Advance release metadata and the upgrade migration to the unclaimed 1.83.0.0 slot. --- CHANGELOG.md | 4 +- TODOS.md | 2 +- VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- autoplan/SKILL.md | 29 +- autoplan/SKILL.md.tmpl | 29 +- autoplan/sections/ceo-phase.md | 46 ++- autoplan/sections/ceo-phase.md.tmpl | 46 ++- autoplan/sections/design-phase.md | 50 ++- autoplan/sections/design-phase.md.tmpl | 50 ++- autoplan/sections/dx-phase.md | 52 ++- autoplan/sections/dx-phase.md.tmpl | 52 ++- autoplan/sections/eng-phase.md | 48 ++- autoplan/sections/eng-phase.md.tmpl | 48 ++- docs/TESTING_INTERNALS.md | 14 + .../migrations/{v1.82.0.0.sh => v1.83.0.0.sh} | 2 +- package.json | 2 +- plan-ceo-review/SKILL.md | 1 + plan-ceo-review/SKILL.md.tmpl | 1 + plan-ceo-review/sections/review-sections.md | 17 + .../sections/review-sections.md.tmpl | 17 + plan-design-review/SKILL.md | 5 +- plan-design-review/SKILL.md.tmpl | 5 +- .../sections/review-sections.md | 14 + .../sections/review-sections.md.tmpl | 14 + scripts/resolvers/composition.ts | 20 + scripts/resolvers/index.ts | 3 +- test/autoplan-phase-order.test.ts | 58 +++ test/autoplan-review-discovery.test.ts | 94 +++++ test/autoplan-setup-question.test.ts | 49 ++- test/ceo-mode-option.test.ts | 21 + test/claude-code-migration.test.ts | 2 +- test/disabled-plan-review-evidence.test.ts | 37 +- test/fixtures/pty-screen/autoplan.json | 94 +++++ test/fixtures/pty-screen/design.json | 95 +++++ test/helpers/autoplan-setup-question.ts | 6 + test/helpers/ceo-mode-option.ts | 34 +- test/helpers/claude-pty-runner.ts | 384 ++++++++++++++---- test/helpers/claude-pty-runner.unit.test.ts | 299 +++++++++++++- test/helpers/disabled-plan-review-fixture.ts | 4 +- test/helpers/plan-count-artifacts.ts | 2 + test/helpers/plan-count-transcript.ts | 14 +- test/helpers/pty-screen.ts | 82 ++++ test/helpers/touchfiles-data.ts | 18 +- test/plan-count-completion.test.ts | 254 +++++++++++- test/plan-count-fixture.test.ts | 111 +++-- test/plan-count-transcript.test.ts | 32 ++ test/pty-screen-session.test.ts | 149 +++++++ test/pty-screen.test.ts | 175 ++++++++ test/skill-e2e-autoplan-chain.test.ts | 23 +- test/skill-e2e-plan-ceo-finding-count.test.ts | 2 + ...kill-e2e-plan-design-finding-count.test.ts | 1 + test/skill-e2e-plan-eng-finding-count.test.ts | 1 + 53 files changed, 2331 insertions(+), 285 deletions(-) rename gstack-upgrade/migrations/{v1.82.0.0.sh => v1.83.0.0.sh} (90%) create mode 100644 test/autoplan-review-discovery.test.ts create mode 100644 test/fixtures/pty-screen/autoplan.json create mode 100644 test/fixtures/pty-screen/design.json create mode 100644 test/helpers/pty-screen.ts create mode 100644 test/pty-screen-session.test.ts create mode 100644 test/pty-screen.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cadcf245..64b231e2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## [1.82.0.0] - 2026-09-08 +## [1.83.0.0] - 2026-09-08 ### Added @@ -16,6 +16,8 @@ - Failed, refused, empty, or malformed outside reviews can no longer count as a clean pass. Claude runner failures include authentication, timeout, and output overflow diagnoses, and stale skills stop before invoking their own harness. - Spec review stops when redaction fails, before sending the spec to a reviewer or saving it downstream. +- Planning reviews request each unresolved decision before editing and carry approved remedies across sections without asking again. Choosing a scope or approach does not approve every finding. +- Autoplan reads the review skills installed for the current harness at each phase, waits for its reviewers, and reports completion after saving the required outputs. Disabling extra plan or documentation review also skips replacement reviewers. ## [1.81.0.0] - 2026-09-06 diff --git a/TODOS.md b/TODOS.md index 568b4f32d..c736a6da2 100644 --- a/TODOS.md +++ b/TODOS.md @@ -3423,7 +3423,7 @@ needs one paid run to validate, so it didn't ride the ship. **Priority:** P1 **Depends on:** None -**Completed:** v1.82.0.0 (2026-09-08). Shipped as `/claude-code`, with automatic outside-review routing and safe installation migration. +**Completed:** v1.83.0.0 (2026-09-08). Shipped as `/claude-code`, with automatic outside-review routing and safe installation migration. ### P3: Carve the always-loaded `{{PREAMBLE}}` reference blocks into an on-demand doc diff --git a/VERSION b/VERSION index 72bb02920..d88a7fc52 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.82.0.0 +1.83.0.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index cb41f9c16..8b22ae921 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.82.0.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.83.0.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index ec90a20c7..9ec8e9d2c 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -629,8 +629,10 @@ shipping gate, so it must review the FINAL amended plan — every other phase's amendments land before it. Each phase MUST complete fully before the next begins. NEVER run phases in parallel — each builds on the previous. -Between each phase, emit a phase-transition summary and verify that all required -outputs from the prior phase are written before starting the next. +End each phase by verifying all required writes and reviewer outcomes, then emit +its completion announcement as an actual assistant message before the next phase. +Plan headings and promises are not announcements. Missing outside coverage does +not block native completion; report its status accurately. --- @@ -732,13 +734,22 @@ Then prepend a one-line HTML comment to the plan file: on top of) or if an AI agent is the primary user (OpenClaw actions, Claude Code skills, MCP servers). -### Step 3: Load skill files from disk +### Step 3: Locate review skills; load each at phase entry -Read each file using the Read tool: -- `~/.claude/skills/gstack/plan-ceo-review/SKILL.md` -- `~/.claude/skills/gstack/plan-design-review/SKILL.md` (only if UI scope detected) -- `~/.claude/skills/gstack/plan-eng-review/SKILL.md` -- `~/.claude/skills/gstack/plan-devex-review/SKILL.md` (only if DX scope detected) +At the start of each applicable phase, read its full review SKILL.md using the Read +tool, then follow that skill's lazy-section Read triggers before doing their work: +- Phase 1: `~/.claude/skills/gstack/plan-ceo-review/SKILL.md` +- Phase 2: `~/.claude/skills/gstack/plan-design-review/SKILL.md` (only if UI scope detected) +- Phase 2.5: `~/.claude/skills/gstack/plan-devex-review/SKILL.md` (only if DX scope detected) +- Phase 3: `~/.claude/skills/gstack/plan-eng-review/SKILL.md` + +Use the same installed skill registry as /autoplan. Resolve sibling paths from its +discovered SKILL.md directory, never cwd/runtime assets. Missing skill: report the +missing phase and setup repair; never substitute another harness or claim completion. + +Do not prefetch future phase sections or review skills. Read each at its trigger; +load the tasks aggregator at Phase 4. All applicable skills and required lazy +sections still run in full. **Section skip list — when following a loaded skill file, SKIP these sections (they are already handled by /autoplan):** @@ -759,7 +770,7 @@ Read each file using the Read tool: Follow ONLY the review-specific methodology, sections, and required outputs. Output: "Here's what I'm working with: [plan summary]. UI scope: [yes/no]. DX scope: [yes/no]. -Loaded review skills from disk. Starting full review pipeline with auto-decisions." +Review skills will load at each phase entry. Starting full review pipeline with auto-decisions." --- diff --git a/autoplan/SKILL.md.tmpl b/autoplan/SKILL.md.tmpl index 9b221911d..2b50f77bf 100644 --- a/autoplan/SKILL.md.tmpl +++ b/autoplan/SKILL.md.tmpl @@ -114,8 +114,10 @@ shipping gate, so it must review the FINAL amended plan — every other phase's amendments land before it. Each phase MUST complete fully before the next begins. NEVER run phases in parallel — each builds on the previous. -Between each phase, emit a phase-transition summary and verify that all required -outputs from the prior phase are written before starting the next. +End each phase by verifying all required writes and reviewer outcomes, then emit +its completion announcement as an actual assistant message before the next phase. +Plan headings and promises are not announcements. Missing outside coverage does +not block native completion; report its status accurately. --- @@ -217,13 +219,22 @@ Then prepend a one-line HTML comment to the plan file: on top of) or if an AI agent is the primary user (OpenClaw actions, Claude Code skills, MCP servers). -### Step 3: Load skill files from disk +### Step 3: Locate review skills; load each at phase entry -Read each file using the Read tool: -- `~/.claude/skills/gstack/plan-ceo-review/SKILL.md` -- `~/.claude/skills/gstack/plan-design-review/SKILL.md` (only if UI scope detected) -- `~/.claude/skills/gstack/plan-eng-review/SKILL.md` -- `~/.claude/skills/gstack/plan-devex-review/SKILL.md` (only if DX scope detected) +At the start of each applicable phase, read its full review SKILL.md using the Read +tool, then follow that skill's lazy-section Read triggers before doing their work: +- Phase 1: {{AUTOPLAN_REVIEW_FILE:plan-ceo-review}} +- Phase 2: {{AUTOPLAN_REVIEW_FILE:plan-design-review}} (only if UI scope detected) +- Phase 2.5: {{AUTOPLAN_REVIEW_FILE:plan-devex-review}} (only if DX scope detected) +- Phase 3: {{AUTOPLAN_REVIEW_FILE:plan-eng-review}} + +Use the same installed skill registry as /autoplan. Resolve sibling paths from its +discovered SKILL.md directory, never cwd/runtime assets. Missing skill: report the +missing phase and setup repair; never substitute another harness or claim completion. + +Do not prefetch future phase sections or review skills. Read each at its trigger; +load the tasks aggregator at Phase 4. All applicable skills and required lazy +sections still run in full. **Section skip list — when following a loaded skill file, SKIP these sections (they are already handled by /autoplan):** @@ -244,7 +255,7 @@ Read each file using the Read tool: Follow ONLY the review-specific methodology, sections, and required outputs. Output: "Here's what I'm working with: [plan summary]. UI scope: [yes/no]. DX scope: [yes/no]. -Loaded review skills from disk. Starting full review pipeline with auto-decisions." +Review skills will load at each phase entry. Starting full review pipeline with auto-decisions." --- diff --git a/autoplan/sections/ceo-phase.md b/autoplan/sections/ceo-phase.md index 72accfc2d..25ce25d4b 100644 --- a/autoplan/sections/ceo-phase.md +++ b/autoplan/sections/ceo-phase.md @@ -1,6 +1,7 @@ -Follow plan-ceo-review/SKILL.md — all sections, full depth. +Read `~/.claude/skills/gstack/plan-ceo-review/SKILL.md` in full now; follow its lazy-section triggers. +Apply /autoplan's skip list; all other work runs in full. Override: every AskUserQuestion → auto-decide using the 6 principles. **Override rules:** @@ -21,6 +22,27 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. Claude Code v2.1.198, so the flag must be explicitly false), then Codex (Bash). Both must complete before building the consensus table. + **Claude CEO subagent** (via Agent tool): + Native subagent tool; Claude Code Agent arguments: + ```json + { "run_in_background": false } + ``` + Set on the call, not in prompt text. Other harnesses use native dispatch/wait. + + "Read the plan file at . You are an independent CEO/strategist + reviewing this plan. You have NOT seen any prior review. Evaluate: + 1. Is this the right problem to solve? Could a reframing yield 10x impact? + 2. Are the premises stated or just assumed? Which ones could be wrong? + 3. What's the 6-month regret scenario — what will look foolish? + 4. What alternatives were dismissed without sufficient analysis? + 5. What's the competitive risk — could someone else solve this first/better? + For each finding: what's wrong, severity (critical/high/medium), and the fix." + + + **Native completion barrier:** `isAsync: true` / `status: "async_launched"` is pending: + wait for that same agent's final result/terminal failure before outside dispatch or parent review. + No inline substitute; apply the existing failure policy. + **Codex CEO voice** (via Bash): Outside prompt (include the full current plan content and the context requested below, using the Write tool): @@ -79,15 +101,6 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar For this phase (ceo), retain the historical review-log skill identifier. Add `"host":"claude","outside_provider":"codex","outside_status":"completed|unavailable|disabled|skipped","phase":"ceo"`. Record each attempted pass separately when outcomes differ. Use `source:"codex"` only for completed external CLI output, and `source:"in-host"` for a native pass. Historical `source:"claude"` continues to mean a native Claude subagent. CLI availability or a native fallback does not count as outside completion. Preserve reported modelUsage, including multiple models; unknown model identity stays unknown. - **Claude CEO subagent** (via Agent tool): - "Read the plan file at . You are an independent CEO/strategist - reviewing this plan. You have NOT seen any prior review. Evaluate: - 1. Is this the right problem to solve? Could a reframing yield 10x impact? - 2. Are the premises stated or just assumed? Which ones could be wrong? - 3. What's the 6-month regret scenario — what will look foolish? - 4. What alternatives were dismissed without sufficient analysis? - 5. What's the competitive risk — could someone else solve this first/better? - For each finding: what's wrong, severity (critical/high/medium), and the fix." **Error handling:** Both calls block in foreground. Codex auth/timeout/empty → proceed with Claude subagent only, tagged `[single-model]`. If Claude subagent also fails → @@ -146,10 +159,15 @@ Sections 1-10 — for EACH section, run the evaluation criteria from the loaded - Dream state delta (where this plan leaves us vs 12-month ideal) - Completion Summary (the full summary table from the CEO skill) -**PHASE 1 COMPLETE.** Emit phase-transition summary: -> **Phase 1 complete.** Codex: [N concerns]. Claude subagent: [N issues]. -> Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. -> Passing to Phase 2. +**Close this phase before continuing:** Check successful Write/Edit results for +all required plan/artifact outputs and both reviewers' terminal status (including +unavailable/disabled coverage). Do not announce completion while required work remains. +Emit this filled-in summary as an actual assistant message: + +**Phase 1 complete.** +Codex: [completed: N concerns / unavailable / disabled]. Claude subagent: [completed: N issues / unavailable]. +Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. +Passing to Phase 2. Do NOT begin Phase 2 until all Phase 1 outputs are written to the plan file, including the premise assessment (queued premise challenges travel to the diff --git a/autoplan/sections/ceo-phase.md.tmpl b/autoplan/sections/ceo-phase.md.tmpl index b809b2c23..ee627b3a9 100644 --- a/autoplan/sections/ceo-phase.md.tmpl +++ b/autoplan/sections/ceo-phase.md.tmpl @@ -1,4 +1,5 @@ -Follow plan-ceo-review/SKILL.md — all sections, full depth. +Read {{AUTOPLAN_REVIEW_FILE:plan-ceo-review}} in full now; follow its lazy-section triggers. +Apply /autoplan's skip list; all other work runs in full. Override: every AskUserQuestion → auto-decide using the 6 principles. **Override rules:** @@ -19,6 +20,27 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. Claude Code v2.1.198, so the flag must be explicitly false), then {{OUTSIDE_LABEL}} (Bash). Both must complete before building the consensus table. + **{{NATIVE_LABEL}} CEO subagent** (via Agent tool): + Native subagent tool; Claude Code Agent arguments: + ```json + { "run_in_background": false } + ``` + Set on the call, not in prompt text. Other harnesses use native dispatch/wait. + + "Read the plan file at . You are an independent CEO/strategist + reviewing this plan. You have NOT seen any prior review. Evaluate: + 1. Is this the right problem to solve? Could a reframing yield 10x impact? + 2. Are the premises stated or just assumed? Which ones could be wrong? + 3. What's the 6-month regret scenario — what will look foolish? + 4. What alternatives were dismissed without sufficient analysis? + 5. What's the competitive risk — could someone else solve this first/better? + For each finding: what's wrong, severity (critical/high/medium), and the fix." + + + **Native completion barrier:** `isAsync: true` / `status: "async_launched"` is pending: + wait for that same agent's final result/terminal failure before outside dispatch or parent review. + No inline substitute; apply the existing failure policy. + **{{OUTSIDE_LABEL}} CEO voice** (via Bash): Outside prompt (include the full current plan content and the context requested below, using the Write tool): @@ -38,15 +60,6 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar {{OUTSIDE_PROVENANCE:ceo}} - **{{NATIVE_LABEL}} CEO subagent** (via Agent tool): - "Read the plan file at . You are an independent CEO/strategist - reviewing this plan. You have NOT seen any prior review. Evaluate: - 1. Is this the right problem to solve? Could a reframing yield 10x impact? - 2. Are the premises stated or just assumed? Which ones could be wrong? - 3. What's the 6-month regret scenario — what will look foolish? - 4. What alternatives were dismissed without sufficient analysis? - 5. What's the competitive risk — could someone else solve this first/better? - For each finding: what's wrong, severity (critical/high/medium), and the fix." **Error handling:** Both calls block in foreground. {{OUTSIDE_LABEL}} auth/timeout/empty → proceed with {{NATIVE_LABEL}} subagent only, tagged `[single-model]`. If {{NATIVE_LABEL}} subagent also fails → @@ -105,10 +118,15 @@ Sections 1-10 — for EACH section, run the evaluation criteria from the loaded - Dream state delta (where this plan leaves us vs 12-month ideal) - Completion Summary (the full summary table from the CEO skill) -**PHASE 1 COMPLETE.** Emit phase-transition summary: -> **Phase 1 complete.** {{OUTSIDE_LABEL}}: [N concerns]. {{NATIVE_LABEL}} subagent: [N issues]. -> Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. -> Passing to Phase 2. +**Close this phase before continuing:** Check successful Write/Edit results for +all required plan/artifact outputs and both reviewers' terminal status (including +unavailable/disabled coverage). Do not announce completion while required work remains. +Emit this filled-in summary as an actual assistant message: + +**Phase 1 complete.** +{{OUTSIDE_LABEL}}: [completed: N concerns / unavailable / disabled]. {{NATIVE_LABEL}} subagent: [completed: N issues / unavailable]. +Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. +Passing to Phase 2. Do NOT begin Phase 2 until all Phase 1 outputs are written to the plan file, including the premise assessment (queued premise challenges travel to the diff --git a/autoplan/sections/design-phase.md b/autoplan/sections/design-phase.md index 77f3a9e73..b3802f09b 100644 --- a/autoplan/sections/design-phase.md +++ b/autoplan/sections/design-phase.md @@ -1,6 +1,7 @@ -Follow plan-design-review/SKILL.md — all 7 dimensions, full depth. +Read `~/.claude/skills/gstack/plan-design-review/SKILL.md` in full now; follow its lazy-section triggers. +Apply /autoplan's skip list; all other work runs in full. Override: every AskUserQuestion → auto-decide using the 6 principles. **Override rules:** @@ -10,6 +11,28 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. - Design system alignment: auto-fix if DESIGN.md exists and fix is obvious - Dual voices: always run BOTH Claude subagent AND Codex if available (P6). + **Claude design subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): + Native subagent tool; Claude Code Agent arguments: + ```json + { "run_in_background": false } + ``` + Set on the call, not in prompt text. Other harnesses use native dispatch/wait. + + "Read the plan file at . You are an independent senior product designer + reviewing this plan. You have NOT seen any prior review. Evaluate: + 1. Information hierarchy: what does the user see first, second, third? Is it right? + 2. Missing states: loading, empty, error, success, partial — which are unspecified? + 3. User journey: what's the emotional arc? Where does it break? + 4. Specificity: does the plan describe SPECIFIC UI or generic patterns? + 5. What design decisions will haunt the implementer if left ambiguous? + For each finding: what's wrong, severity (critical/high/medium), and the fix." + NO prior-phase context — subagent must be truly independent. + + + **Native completion barrier:** `isAsync: true` / `status: "async_launched"` is pending: + wait for that same agent's final result/terminal failure before outside dispatch or parent review. + No inline substitute; apply the existing failure policy. + **Codex design voice** (via Bash): Outside prompt (include the full current plan content and the context requested below, using the Write tool): @@ -74,16 +97,6 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar For this phase (design), retain the historical review-log skill identifier. Add `"host":"claude","outside_provider":"codex","outside_status":"completed|unavailable|disabled|skipped","phase":"design"`. Record each attempted pass separately when outcomes differ. Use `source:"codex"` only for completed external CLI output, and `source:"in-host"` for a native pass. Historical `source:"claude"` continues to mean a native Claude subagent. CLI availability or a native fallback does not count as outside completion. Preserve reported modelUsage, including multiple models; unknown model identity stays unknown. - **Claude design subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): - "Read the plan file at . You are an independent senior product designer - reviewing this plan. You have NOT seen any prior review. Evaluate: - 1. Information hierarchy: what does the user see first, second, third? Is it right? - 2. Missing states: loading, empty, error, success, partial — which are unspecified? - 3. User journey: what's the emotional arc? Where does it break? - 4. Specificity: does the plan describe SPECIFIC UI or generic patterns? - 5. What design decisions will haunt the implementer if left ambiguous? - For each finding: what's wrong, severity (critical/high/medium), and the fix." - NO prior-phase context — subagent must be truly independent. Error handling: same as Phase 1 (both foreground/blocking, degradation matrix applies). @@ -103,9 +116,14 @@ For this phase (design), retain the historical review-log skill identifier. Add 3. Passes 1-7: Run each from loaded skill. Rate 0-10. Auto-decide each issue. DISAGREE items from scorecard → raised in the relevant pass with both perspectives. -**PHASE 2 COMPLETE.** Emit phase-transition summary: -> **Phase 2 complete.** Codex: [N concerns]. Claude subagent: [N issues]. -> Consensus: [X/Y confirmed, Z disagreements → surfaced at gate]. -> Passing to Phase 3. +**Close this phase before continuing:** Check successful Write/Edit results for +all required plan/artifact outputs and both reviewers' terminal status (including +unavailable/disabled coverage). Do not announce completion while required work remains. +Emit this filled-in summary as an actual assistant message: -Do NOT begin Phase 3 until all Phase 2 outputs (if run) are written to the plan file. +**Phase 2 complete.** +Codex: [completed: N concerns / unavailable / disabled]. Claude subagent: [completed: N issues / unavailable]. +Consensus: [X/Y confirmed, Z disagreements → surfaced at gate]. +Passing to Phase 2.5 (DX Review) if DX scope was detected; otherwise Phase 3 (Eng Review). + +Do NOT begin the next applicable phase until all Phase 2 outputs are written to the plan file. diff --git a/autoplan/sections/design-phase.md.tmpl b/autoplan/sections/design-phase.md.tmpl index 79737eba1..9d5c9c14d 100644 --- a/autoplan/sections/design-phase.md.tmpl +++ b/autoplan/sections/design-phase.md.tmpl @@ -1,4 +1,5 @@ -Follow plan-design-review/SKILL.md — all 7 dimensions, full depth. +Read {{AUTOPLAN_REVIEW_FILE:plan-design-review}} in full now; follow its lazy-section triggers. +Apply /autoplan's skip list; all other work runs in full. Override: every AskUserQuestion → auto-decide using the 6 principles. **Override rules:** @@ -8,6 +9,28 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. - Design system alignment: auto-fix if DESIGN.md exists and fix is obvious - Dual voices: always run BOTH {{NATIVE_LABEL}} subagent AND {{OUTSIDE_LABEL}} if available (P6). + **{{NATIVE_LABEL}} design subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): + Native subagent tool; Claude Code Agent arguments: + ```json + { "run_in_background": false } + ``` + Set on the call, not in prompt text. Other harnesses use native dispatch/wait. + + "Read the plan file at . You are an independent senior product designer + reviewing this plan. You have NOT seen any prior review. Evaluate: + 1. Information hierarchy: what does the user see first, second, third? Is it right? + 2. Missing states: loading, empty, error, success, partial — which are unspecified? + 3. User journey: what's the emotional arc? Where does it break? + 4. Specificity: does the plan describe SPECIFIC UI or generic patterns? + 5. What design decisions will haunt the implementer if left ambiguous? + For each finding: what's wrong, severity (critical/high/medium), and the fix." + NO prior-phase context — subagent must be truly independent. + + + **Native completion barrier:** `isAsync: true` / `status: "async_launched"` is pending: + wait for that same agent's final result/terminal failure before outside dispatch or parent review. + No inline substitute; apply the existing failure policy. + **{{OUTSIDE_LABEL}} design voice** (via Bash): Outside prompt (include the full current plan content and the context requested below, using the Write tool): @@ -33,16 +56,6 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar {{OUTSIDE_PROVENANCE:design}} - **{{NATIVE_LABEL}} design subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): - "Read the plan file at . You are an independent senior product designer - reviewing this plan. You have NOT seen any prior review. Evaluate: - 1. Information hierarchy: what does the user see first, second, third? Is it right? - 2. Missing states: loading, empty, error, success, partial — which are unspecified? - 3. User journey: what's the emotional arc? Where does it break? - 4. Specificity: does the plan describe SPECIFIC UI or generic patterns? - 5. What design decisions will haunt the implementer if left ambiguous? - For each finding: what's wrong, severity (critical/high/medium), and the fix." - NO prior-phase context — subagent must be truly independent. Error handling: same as Phase 1 (both foreground/blocking, degradation matrix applies). @@ -62,9 +75,14 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar 3. Passes 1-7: Run each from loaded skill. Rate 0-10. Auto-decide each issue. DISAGREE items from scorecard → raised in the relevant pass with both perspectives. -**PHASE 2 COMPLETE.** Emit phase-transition summary: -> **Phase 2 complete.** {{OUTSIDE_LABEL}}: [N concerns]. {{NATIVE_LABEL}} subagent: [N issues]. -> Consensus: [X/Y confirmed, Z disagreements → surfaced at gate]. -> Passing to Phase 3. +**Close this phase before continuing:** Check successful Write/Edit results for +all required plan/artifact outputs and both reviewers' terminal status (including +unavailable/disabled coverage). Do not announce completion while required work remains. +Emit this filled-in summary as an actual assistant message: -Do NOT begin Phase 3 until all Phase 2 outputs (if run) are written to the plan file. +**Phase 2 complete.** +{{OUTSIDE_LABEL}}: [completed: N concerns / unavailable / disabled]. {{NATIVE_LABEL}} subagent: [completed: N issues / unavailable]. +Consensus: [X/Y confirmed, Z disagreements → surfaced at gate]. +Passing to Phase 2.5 (DX Review) if DX scope was detected; otherwise Phase 3 (Eng Review). + +Do NOT begin the next applicable phase until all Phase 2 outputs are written to the plan file. diff --git a/autoplan/sections/dx-phase.md b/autoplan/sections/dx-phase.md index 3f311ee0e..069d95613 100644 --- a/autoplan/sections/dx-phase.md +++ b/autoplan/sections/dx-phase.md @@ -1,6 +1,7 @@ -Follow plan-devex-review/SKILL.md — all 8 DX dimensions, full depth. +Read `~/.claude/skills/gstack/plan-devex-review/SKILL.md` in full now; follow its lazy-section triggers. +Apply /autoplan's skip list; all other work runs in full. Override: every AskUserQuestion → auto-decide using the 6 principles. **Override rules:** @@ -14,6 +15,28 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. - DX taste decisions (e.g., opinionated defaults vs flexibility): mark TASTE DECISION - Dual voices: always run BOTH Claude subagent AND Codex if available (P6). + **Claude DX subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): + Native subagent tool; Claude Code Agent arguments: + ```json + { "run_in_background": false } + ``` + Set on the call, not in prompt text. Other harnesses use native dispatch/wait. + + "Read the plan file at . You are an independent DX engineer + reviewing this plan. You have NOT seen any prior review. Evaluate: + 1. Getting started: how many steps from zero to hello world? What's the TTHW? + 2. API/CLI ergonomics: naming consistency, sensible defaults, progressive disclosure? + 3. Error handling: does every error path specify problem + cause + fix + docs link? + 4. Documentation: copy-paste examples? Information architecture? Interactive elements? + 5. Escape hatches: can developers override every opinionated default? + For each finding: what's wrong, severity (critical/high/medium), and the fix." + NO prior-phase context — subagent must be truly independent. + + + **Native completion barrier:** `isAsync: true` / `status: "async_launched"` is pending: + wait for that same agent's final result/terminal failure before outside dispatch or parent review. + No inline substitute; apply the existing failure policy. + **Codex DX voice** (via Bash): Outside prompt (include the full current plan content and the context requested below, using the Write tool): @@ -23,7 +46,7 @@ IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definitio Also consider these findings from prior review phases: CEO: - Eng: + Design: You are a developer who has never seen this product. Evaluate: 1. Time to hello world: how many steps from zero to working? Target is under 5 minutes. @@ -78,16 +101,6 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar For this phase (dx), retain the historical review-log skill identifier. Add `"host":"claude","outside_provider":"codex","outside_status":"completed|unavailable|disabled|skipped","phase":"dx"`. Record each attempted pass separately when outcomes differ. Use `source:"codex"` only for completed external CLI output, and `source:"in-host"` for a native pass. Historical `source:"claude"` continues to mean a native Claude subagent. CLI availability or a native fallback does not count as outside completion. Preserve reported modelUsage, including multiple models; unknown model identity stays unknown. - **Claude DX subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): - "Read the plan file at . You are an independent DX engineer - reviewing this plan. You have NOT seen any prior review. Evaluate: - 1. Getting started: how many steps from zero to hello world? What's the TTHW? - 2. API/CLI ergonomics: naming consistency, sensible defaults, progressive disclosure? - 3. Error handling: does every error path specify problem + cause + fix + docs link? - 4. Documentation: copy-paste examples? Information architecture? Interactive elements? - 5. Escape hatches: can developers override every opinionated default? - For each finding: what's wrong, severity (critical/high/medium), and the fix." - NO prior-phase context — subagent must be truly independent. Error handling: same as Phase 1 (both foreground/blocking, degradation matrix applies). @@ -131,8 +144,13 @@ Missing voice = N/A (not CONFIRMED). Single critical finding from one voice = fl - DX Implementation Checklist - TTHW assessment with target -**PHASE 2.5 COMPLETE.** Emit phase-transition summary: -> **Phase 2.5 complete.** DX overall: [N]/10. TTHW: [N] min → [target] min. -> Codex: [N concerns]. Claude subagent: [N issues]. -> Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. -> Passing to Phase 3 (Eng Review — the required gate reviews the final amended plan). +**Close this phase before continuing:** Check successful Write/Edit results for +all required plan/artifact outputs and both reviewers' terminal status (including +unavailable/disabled coverage). Do not announce completion while required work remains. +Emit this filled-in summary as an actual assistant message: + +**Phase 2.5 complete.** +DX overall: [N]/10. TTHW: [N] min → [target] min. +Codex: [completed: N concerns / unavailable / disabled]. Claude subagent: [completed: N issues / unavailable]. +Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. +Passing to Phase 3 (Eng Review — the required gate reviews the final amended plan). diff --git a/autoplan/sections/dx-phase.md.tmpl b/autoplan/sections/dx-phase.md.tmpl index 5840db5da..4fead4207 100644 --- a/autoplan/sections/dx-phase.md.tmpl +++ b/autoplan/sections/dx-phase.md.tmpl @@ -1,4 +1,5 @@ -Follow plan-devex-review/SKILL.md — all 8 DX dimensions, full depth. +Read {{AUTOPLAN_REVIEW_FILE:plan-devex-review}} in full now; follow its lazy-section triggers. +Apply /autoplan's skip list; all other work runs in full. Override: every AskUserQuestion → auto-decide using the 6 principles. **Override rules:** @@ -12,6 +13,28 @@ Override: every AskUserQuestion → auto-decide using the 6 principles. - DX taste decisions (e.g., opinionated defaults vs flexibility): mark TASTE DECISION - Dual voices: always run BOTH {{NATIVE_LABEL}} subagent AND {{OUTSIDE_LABEL}} if available (P6). + **{{NATIVE_LABEL}} DX subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): + Native subagent tool; Claude Code Agent arguments: + ```json + { "run_in_background": false } + ``` + Set on the call, not in prompt text. Other harnesses use native dispatch/wait. + + "Read the plan file at . You are an independent DX engineer + reviewing this plan. You have NOT seen any prior review. Evaluate: + 1. Getting started: how many steps from zero to hello world? What's the TTHW? + 2. API/CLI ergonomics: naming consistency, sensible defaults, progressive disclosure? + 3. Error handling: does every error path specify problem + cause + fix + docs link? + 4. Documentation: copy-paste examples? Information architecture? Interactive elements? + 5. Escape hatches: can developers override every opinionated default? + For each finding: what's wrong, severity (critical/high/medium), and the fix." + NO prior-phase context — subagent must be truly independent. + + + **Native completion barrier:** `isAsync: true` / `status: "async_launched"` is pending: + wait for that same agent's final result/terminal failure before outside dispatch or parent review. + No inline substitute; apply the existing failure policy. + **{{OUTSIDE_LABEL}} DX voice** (via Bash): Outside prompt (include the full current plan content and the context requested below, using the Write tool): @@ -21,7 +44,7 @@ IMPORTANT: Do NOT read or execute any SKILL.md files or files in skill definitio Also consider these findings from prior review phases: CEO: - Eng: + Design: You are a developer who has never seen this product. Evaluate: 1. Time to hello world: how many steps from zero to working? Target is under 5 minutes. @@ -37,16 +60,6 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar {{OUTSIDE_PROVENANCE:dx}} - **{{NATIVE_LABEL}} DX subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): - "Read the plan file at . You are an independent DX engineer - reviewing this plan. You have NOT seen any prior review. Evaluate: - 1. Getting started: how many steps from zero to hello world? What's the TTHW? - 2. API/CLI ergonomics: naming consistency, sensible defaults, progressive disclosure? - 3. Error handling: does every error path specify problem + cause + fix + docs link? - 4. Documentation: copy-paste examples? Information architecture? Interactive elements? - 5. Escape hatches: can developers override every opinionated default? - For each finding: what's wrong, severity (critical/high/medium), and the fix." - NO prior-phase context — subagent must be truly independent. Error handling: same as Phase 1 (both foreground/blocking, degradation matrix applies). @@ -90,8 +103,13 @@ Missing voice = N/A (not CONFIRMED). Single critical finding from one voice = fl - DX Implementation Checklist - TTHW assessment with target -**PHASE 2.5 COMPLETE.** Emit phase-transition summary: -> **Phase 2.5 complete.** DX overall: [N]/10. TTHW: [N] min → [target] min. -> {{OUTSIDE_LABEL}}: [N concerns]. {{NATIVE_LABEL}} subagent: [N issues]. -> Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. -> Passing to Phase 3 (Eng Review — the required gate reviews the final amended plan). +**Close this phase before continuing:** Check successful Write/Edit results for +all required plan/artifact outputs and both reviewers' terminal status (including +unavailable/disabled coverage). Do not announce completion while required work remains. +Emit this filled-in summary as an actual assistant message: + +**Phase 2.5 complete.** +DX overall: [N]/10. TTHW: [N] min → [target] min. +{{OUTSIDE_LABEL}}: [completed: N concerns / unavailable / disabled]. {{NATIVE_LABEL}} subagent: [completed: N issues / unavailable]. +Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. +Passing to Phase 3 (Eng Review — the required gate reviews the final amended plan). diff --git a/autoplan/sections/eng-phase.md b/autoplan/sections/eng-phase.md index 87d70cd54..d19c9df49 100644 --- a/autoplan/sections/eng-phase.md +++ b/autoplan/sections/eng-phase.md @@ -1,12 +1,35 @@ -Follow plan-eng-review/SKILL.md — all sections, full depth. +Read `~/.claude/skills/gstack/plan-eng-review/SKILL.md` in full now; follow its lazy-section triggers. +Apply /autoplan's skip list; all other work runs in full. Override: every AskUserQuestion → auto-decide using the 6 principles. **Override rules:** - Scope challenge: never reduce (P2) - Dual voices: always run BOTH Claude subagent AND Codex if available (P6). + **Claude eng subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): + Native subagent tool; Claude Code Agent arguments: + ```json + { "run_in_background": false } + ``` + Set on the call, not in prompt text. Other harnesses use native dispatch/wait. + + "Read the plan file at . You are an independent senior engineer + reviewing this plan. You have NOT seen any prior review. Evaluate: + 1. Architecture: Is the component structure sound? Coupling concerns? + 2. Edge cases: What breaks under 10x load? What's the nil/empty/error path? + 3. Tests: What's missing from the test plan? What would break at 2am Friday? + 4. Security: New attack surface? Auth boundaries? Input validation? + 5. Hidden complexity: What looks simple but isn't? + For each finding: what's wrong, severity, and the fix." + NO prior-phase context — subagent must be truly independent. + + + **Native completion barrier:** `isAsync: true` / `status: "async_launched"` is pending: + wait for that same agent's final result/terminal failure before outside dispatch or parent review. + No inline substitute; apply the existing failure policy. + **Codex eng voice** (via Bash): Outside prompt (include the full current plan content and the context requested below, using the Write tool): @@ -67,16 +90,6 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar For this phase (eng), retain the historical review-log skill identifier. Add `"host":"claude","outside_provider":"codex","outside_status":"completed|unavailable|disabled|skipped","phase":"eng"`. Record each attempted pass separately when outcomes differ. Use `source:"codex"` only for completed external CLI output, and `source:"in-host"` for a native pass. Historical `source:"claude"` continues to mean a native Claude subagent. CLI availability or a native fallback does not count as outside completion. Preserve reported modelUsage, including multiple models; unknown model identity stays unknown. - **Claude eng subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): - "Read the plan file at . You are an independent senior engineer - reviewing this plan. You have NOT seen any prior review. Evaluate: - 1. Architecture: Is the component structure sound? Coupling concerns? - 2. Edge cases: What breaks under 10x load? What's the nil/empty/error path? - 3. Tests: What's missing from the test plan? What would break at 2am Friday? - 4. Security: New attack surface? Auth boundaries? Input validation? - 5. Hidden complexity: What looks simple but isn't? - For each finding: what's wrong, severity, and the fix." - NO prior-phase context — subagent must be truly independent. Error handling: same as Phase 1 (both foreground/blocking, degradation matrix applies). @@ -140,7 +153,12 @@ Missing voice = N/A (not CONFIRMED). Single critical finding from one voice = fl - Completion Summary (the full summary from the Eng skill) - TODOS.md updates (collected from all phases) -**PHASE 3 COMPLETE.** Emit phase-transition summary: -> **Phase 3 complete.** Codex: [N concerns]. Claude subagent: [N issues]. -> Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. -> Passing to Phase 4 (Final Gate). +**Close this phase before continuing:** Check successful Write/Edit results for +all required plan/artifact outputs and both reviewers' terminal status (including +unavailable/disabled coverage). Do not announce completion while required work remains. +Emit this filled-in summary as an actual assistant message: + +**Phase 3 complete.** +Codex: [completed: N concerns / unavailable / disabled]. Claude subagent: [completed: N issues / unavailable]. +Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. +Passing to Phase 4 (Final Gate). diff --git a/autoplan/sections/eng-phase.md.tmpl b/autoplan/sections/eng-phase.md.tmpl index d9107f13e..b4f528ff4 100644 --- a/autoplan/sections/eng-phase.md.tmpl +++ b/autoplan/sections/eng-phase.md.tmpl @@ -1,10 +1,33 @@ -Follow plan-eng-review/SKILL.md — all sections, full depth. +Read {{AUTOPLAN_REVIEW_FILE:plan-eng-review}} in full now; follow its lazy-section triggers. +Apply /autoplan's skip list; all other work runs in full. Override: every AskUserQuestion → auto-decide using the 6 principles. **Override rules:** - Scope challenge: never reduce (P2) - Dual voices: always run BOTH {{NATIVE_LABEL}} subagent AND {{OUTSIDE_LABEL}} if available (P6). + **{{NATIVE_LABEL}} eng subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): + Native subagent tool; Claude Code Agent arguments: + ```json + { "run_in_background": false } + ``` + Set on the call, not in prompt text. Other harnesses use native dispatch/wait. + + "Read the plan file at . You are an independent senior engineer + reviewing this plan. You have NOT seen any prior review. Evaluate: + 1. Architecture: Is the component structure sound? Coupling concerns? + 2. Edge cases: What breaks under 10x load? What's the nil/empty/error path? + 3. Tests: What's missing from the test plan? What would break at 2am Friday? + 4. Security: New attack surface? Auth boundaries? Input validation? + 5. Hidden complexity: What looks simple but isn't? + For each finding: what's wrong, severity, and the fix." + NO prior-phase context — subagent must be truly independent. + + + **Native completion barrier:** `isAsync: true` / `status: "async_launched"` is pending: + wait for that same agent's final result/terminal failure before outside dispatch or parent review. + No inline substitute; apply the existing failure policy. + **{{OUTSIDE_LABEL}} eng voice** (via Bash): Outside prompt (include the full current plan content and the context requested below, using the Write tool): @@ -26,16 +49,6 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar {{OUTSIDE_PROVENANCE:eng}} - **{{NATIVE_LABEL}} eng subagent** (via Agent tool, `run_in_background: false` — same foreground contract as Phase 1): - "Read the plan file at . You are an independent senior engineer - reviewing this plan. You have NOT seen any prior review. Evaluate: - 1. Architecture: Is the component structure sound? Coupling concerns? - 2. Edge cases: What breaks under 10x load? What's the nil/empty/error path? - 3. Tests: What's missing from the test plan? What would break at 2am Friday? - 4. Security: New attack surface? Auth boundaries? Input validation? - 5. Hidden complexity: What looks simple but isn't? - For each finding: what's wrong, severity, and the fix." - NO prior-phase context — subagent must be truly independent. Error handling: same as Phase 1 (both foreground/blocking, degradation matrix applies). @@ -99,7 +112,12 @@ Missing voice = N/A (not CONFIRMED). Single critical finding from one voice = fl - Completion Summary (the full summary from the Eng skill) - TODOS.md updates (collected from all phases) -**PHASE 3 COMPLETE.** Emit phase-transition summary: -> **Phase 3 complete.** {{OUTSIDE_LABEL}}: [N concerns]. {{NATIVE_LABEL}} subagent: [N issues]. -> Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. -> Passing to Phase 4 (Final Gate). +**Close this phase before continuing:** Check successful Write/Edit results for +all required plan/artifact outputs and both reviewers' terminal status (including +unavailable/disabled coverage). Do not announce completion while required work remains. +Emit this filled-in summary as an actual assistant message: + +**Phase 3 complete.** +{{OUTSIDE_LABEL}}: [completed: N concerns / unavailable / disabled]. {{NATIVE_LABEL}} subagent: [completed: N issues / unavailable]. +Consensus: [X/6 confirmed, Y disagreements → surfaced at gate]. +Passing to Phase 4 (Final Gate). diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index 4e16f0d40..9ac966d50 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -36,6 +36,20 @@ two gate-tier canaries in `test/skill-e2e-hermetic-canary.test.ts`, and the seeding tripwires in `test/hermetic-skills-seeding.test.ts` / `test/pty-skill-seeding-wiring.test.ts`. +Seeded planning sessions also receive an isolated runtime home through +`test/helpers/hermetic-skill-runtime.ts`, so absolute lazy-section paths resolve +to the working tree under test. Explicit per-test home overrides remain intact. +Autoplan resolves each review skill from its own installed host registry. + +**Interactive planning evidence.** Finding-count and autoplan-chain drivers use +`observeScreen: true` and await `currentScreen()` before choosing an input. The +existing xterm dependency interprets cursor moves and erases; old menus in the +raw stream cannot establish a current prompt. Snapshots preserve +`terminal.raw.log`, `terminal.visible.log`, and `terminal.screen.log` separately. +Completed native transcript calls establish question counts and phase coverage. +Report-aware count tests also require a fresh, complete report and native +completion evidence before accepting a completion heading. + E2E tests stream progress in real-time (tool-by-tool via `--output-format stream-json --verbose`). Results are persisted to `~/.gstack/projects//evals/` (legacy fallback `~/.gstack-dev/evals/`) with auto-comparison diff --git a/gstack-upgrade/migrations/v1.82.0.0.sh b/gstack-upgrade/migrations/v1.83.0.0.sh similarity index 90% rename from gstack-upgrade/migrations/v1.82.0.0.sh rename to gstack-upgrade/migrations/v1.83.0.0.sh index 2f9486518..9d50e9c35 100755 --- a/gstack-upgrade/migrations/v1.82.0.0.sh +++ b/gstack-upgrade/migrations/v1.83.0.0.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Migration: v1.82.0.0 — /claude becomes /claude-code on non-Claude hosts. +# Migration: v1.83.0.0 — /claude becomes /claude-code on non-Claude hosts. # Affected: existing generated skills, including copied and dangling installs. # The same helper runs before setup builds, so replacements are installed before # shared old renders are retired. This post-setup pass repairs missed upgrades. diff --git a/package.json b/package.json index 47b8582a2..ed5fc712c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.82.0", + "version": "1.83.0", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/plan-ceo-review/SKILL.md b/plan-ceo-review/SKILL.md index 850a6e7e3..459a88038 100644 --- a/plan-ceo-review/SKILL.md +++ b/plan-ceo-review/SKILL.md @@ -921,6 +921,7 @@ Rules: - **These two approaches have equal weight.** Don't default to "minimal viable" just because it's smaller. Recommend whichever best serves the user's goal. If the right answer is a rewrite, say so. - If only one approach exists, explain concretely why alternatives were eliminated. - Do NOT proceed to mode selection (0F) without user approval of the chosen approach. +- Selecting an approach approves the direction, not a batch of review findings. Keep each unresolved finding for its own decision in the review sections before applying its remedy, unless the user explicitly already approved those particular changes. Present these approach options via AskUserQuestion using the preamble's AskUserQuestion Format section: include RECOMMENDATION and `Completeness: N/10` on every option. These approaches differ in coverage (minimal viable vs ideal architecture), so completeness scoring applies directly. diff --git a/plan-ceo-review/SKILL.md.tmpl b/plan-ceo-review/SKILL.md.tmpl index 3b59b0999..beee7ab8b 100644 --- a/plan-ceo-review/SKILL.md.tmpl +++ b/plan-ceo-review/SKILL.md.tmpl @@ -279,6 +279,7 @@ Rules: - **These two approaches have equal weight.** Don't default to "minimal viable" just because it's smaller. Recommend whichever best serves the user's goal. If the right answer is a rewrite, say so. - If only one approach exists, explain concretely why alternatives were eliminated. - Do NOT proceed to mode selection (0F) without user approval of the chosen approach. +- Selecting an approach approves the direction, not a batch of review findings. Keep each unresolved finding for its own decision in the review sections before applying its remedy, unless the user explicitly already approved those particular changes. Present these approach options via AskUserQuestion using the preamble's AskUserQuestion Format section: include RECOMMENDATION and `Completeness: N/10` on every option. These approaches differ in coverage (minimal viable vs ideal architecture), so completeness scoring applies directly. diff --git a/plan-ceo-review/sections/review-sections.md b/plan-ceo-review/sections/review-sections.md index 677e10047..e93669d03 100644 --- a/plan-ceo-review/sections/review-sections.md +++ b/plan-ceo-review/sections/review-sections.md @@ -6,6 +6,23 @@ **Anti-shortcut clause:** The plan file is the OUTPUT of the interactive review, not a substitute for it. Writing every finding into one plan write and calling ExitPlanMode without firing AskUserQuestion is the precise failure mode of the May 2026 transcript bug — the model explored, found issues, and dumped them into a deliverable rather than walking the user through them. If you have ANY non-trivial finding in any review section, the path from finding to ExitPlanMode goes THROUGH AskUserQuestion. Zero findings in every section is the only path to ExitPlanMode that bypasses AskUserQuestion. If you find yourself wanting to write a plan with findings before asking, stop and call AskUserQuestion now — that's the bug, recognize it. +**Carry decisions across sections.** Track each finding by its failure mode and +individually approved remedy. Selecting a scope or approach alone does not approve +every finding within it; each unresolved finding still needs its first individual +decision, unless the user explicitly already approved those particular changes. +Before raising a finding, check the existing contract and the +user's earlier decisions. Present a complete remedy for that one issue, including +the validation and failure observability needed to prove it works. Do not split +those consequences of the same remedy into repeated approval questions. Keep +independent issues separate, even when they affect the same component or test. + +When a later section encounters the same issue, verify and reference the approved +remedy. Do not reopen it merely to restate the fix or suggest an alternative with +no evidenced requirement. New evidence that leaves a failure mode unresolved +still needs its own decision; explain what the earlier remedy does not cover. +This does not approve an unraised finding or a new TODO: continue to present each +new finding and each potential TODO individually under the rules below. + ### Section 1: Architecture Review Evaluate and diagram: * Overall system design and component boundaries. Draw the dependency graph. diff --git a/plan-ceo-review/sections/review-sections.md.tmpl b/plan-ceo-review/sections/review-sections.md.tmpl index 4ac72b35e..7610ad299 100644 --- a/plan-ceo-review/sections/review-sections.md.tmpl +++ b/plan-ceo-review/sections/review-sections.md.tmpl @@ -4,6 +4,23 @@ {{ANTI_SHORTCUT_CLAUSE}} +**Carry decisions across sections.** Track each finding by its failure mode and +individually approved remedy. Selecting a scope or approach alone does not approve +every finding within it; each unresolved finding still needs its first individual +decision, unless the user explicitly already approved those particular changes. +Before raising a finding, check the existing contract and the +user's earlier decisions. Present a complete remedy for that one issue, including +the validation and failure observability needed to prove it works. Do not split +those consequences of the same remedy into repeated approval questions. Keep +independent issues separate, even when they affect the same component or test. + +When a later section encounters the same issue, verify and reference the approved +remedy. Do not reopen it merely to restate the fix or suggest an alternative with +no evidenced requirement. New evidence that leaves a failure mode unresolved +still needs its own decision; explain what the earlier remedy does not cover. +This does not approve an unraised finding or a new TODO: continue to present each +new finding and each potential TODO individually under the rules below. + ### Section 1: Architecture Review Evaluate and diagram: * Overall system design and component boundaries. Draw the dependency graph. diff --git a/plan-design-review/SKILL.md b/plan-design-review/SKILL.md index d764e6c1a..30e8fe959 100644 --- a/plan-design-review/SKILL.md +++ b/plan-design-review/SKILL.md @@ -1145,7 +1145,10 @@ A gap already listed in the input plan is still an unresolved review finding. Knowing its cause or the matching DESIGN.md token does not approve the change. Review each such gap individually; do not batch them into one "apply all fixes" question or silently resolve them in the initial plan write. Honor an explicit -user decision already made for that exact change; do not ask for it again. +user decision already made for that exact change across all passes. Apply the +selected fix to every affected plan reference, including the matching established +DESIGN.md tokens, without asking again. Reopen it only when new evidence exposes +an unresolved design requirement or tradeoff; explain what changed. Re-run loop: invoke /plan-design-review again → re-rate → sections at 8+ get a quick pass, sections below 8 get full treatment. diff --git a/plan-design-review/SKILL.md.tmpl b/plan-design-review/SKILL.md.tmpl index 9a2360c4a..f70e37f3f 100644 --- a/plan-design-review/SKILL.md.tmpl +++ b/plan-design-review/SKILL.md.tmpl @@ -281,7 +281,10 @@ A gap already listed in the input plan is still an unresolved review finding. Knowing its cause or the matching DESIGN.md token does not approve the change. Review each such gap individually; do not batch them into one "apply all fixes" question or silently resolve them in the initial plan write. Honor an explicit -user decision already made for that exact change; do not ask for it again. +user decision already made for that exact change across all passes. Apply the +selected fix to every affected plan reference, including the matching established +DESIGN.md tokens, without asking again. Reopen it only when new evidence exposes +an unresolved design requirement or tradeoff; explain what changed. Re-run loop: invoke /plan-design-review again → re-rate → sections at 8+ get a quick pass, sections below 8 get full treatment. diff --git a/plan-design-review/sections/review-sections.md b/plan-design-review/sections/review-sections.md index 6573dab79..5b4a6b71c 100644 --- a/plan-design-review/sections/review-sections.md +++ b/plan-design-review/sections/review-sections.md @@ -44,6 +44,20 @@ matches a past learning, display: This makes the compounding visible. The user should see that gstack is getting smarter on their codebase over time. +**Carry decisions across passes.** Before each pass, reconcile the plan, its +inherited DESIGN.md requirements, and the user's decisions so far. An issue is an +unresolved design requirement or tradeoff, not each paragraph where it appears. Apply an +approved fix to all affected plan references and matching established tokens; +reference that decision in later passes instead of asking for it again. + +Create required review artifacts, such as the journey storyboard, from accepted +design requirements without asking about their format. Restating an inherited requirement +(such as an accepted contrast standard) also needs no new design decision. If +that work exposes a missing design requirement, conflicting requirements, or a new +tradeoff, +raise that unresolved issue individually and wait for approval. A gap explicitly +listed in the input still needs its first individual approval before it is fixed. + ### Pass 1: Information Architecture Rate 0-10: Does the plan define what the user sees first, second, third? FIX TO 10: Add information hierarchy to the plan. Include ASCII diagram of screen/page structure and navigation flow. Apply "constraint worship" — if you can only show 3 things, which 3? diff --git a/plan-design-review/sections/review-sections.md.tmpl b/plan-design-review/sections/review-sections.md.tmpl index 8b9c0c4bc..4de037eb1 100644 --- a/plan-design-review/sections/review-sections.md.tmpl +++ b/plan-design-review/sections/review-sections.md.tmpl @@ -6,6 +6,20 @@ {{LEARNINGS_SEARCH}} +**Carry decisions across passes.** Before each pass, reconcile the plan, its +inherited DESIGN.md requirements, and the user's decisions so far. An issue is an +unresolved design requirement or tradeoff, not each paragraph where it appears. Apply an +approved fix to all affected plan references and matching established tokens; +reference that decision in later passes instead of asking for it again. + +Create required review artifacts, such as the journey storyboard, from accepted +design requirements without asking about their format. Restating an inherited requirement +(such as an accepted contrast standard) also needs no new design decision. If +that work exposes a missing design requirement, conflicting requirements, or a new +tradeoff, +raise that unresolved issue individually and wait for approval. A gap explicitly +listed in the input still needs its first individual approval before it is fixed. + ### Pass 1: Information Architecture Rate 0-10: Does the plan define what the user sees first, second, third? FIX TO 10: Add information hierarchy to the plan. Include ASCII diagram of screen/page structure and navigation flow. Apply "constraint worship" — if you can only show 3 things, which 3? diff --git a/scripts/resolvers/composition.ts b/scripts/resolvers/composition.ts index b8d3483d9..4a33c748d 100644 --- a/scripts/resolvers/composition.ts +++ b/scripts/resolvers/composition.ts @@ -1,4 +1,6 @@ import type { TemplateContext } from './types'; +import * as path from 'path'; +import { getHostConfig } from '../../hosts'; /** * {{INVOKE_SKILL:skill-name}} — emits prose instructing Claude to read @@ -46,3 +48,21 @@ ${allSkips.map(s => `- ${s}`).join('\n')} Execute every other section at full depth. When the loaded skill's instructions are complete, continue with the next step below.`; } + +/** Autoplan reads methodology from this host's skill registry, not its runtime assets. */ +export function generateAutoplanReviewFile(ctx: TemplateContext, args?: string[]): string { + const skill = args?.[0]; + if (!skill || !['plan-ceo-review', 'plan-design-review', 'plan-devex-review', 'plan-eng-review'].includes(skill)) { + throw new Error('AUTOPLAN_REVIEW_FILE requires an autoplan review skill'); + } + if (ctx.host === 'claude') return `\`${ctx.paths.skillRoot}/${skill}/SKILL.md\``; + + const host = getHostConfig(ctx.host); + const file = `gstack-${skill}/SKILL.md`; + const local = `${path.posix.dirname(host.localSkillRoot)}/${file}`; + const global = `~/${path.posix.dirname(host.globalRoot)}/${file}`; + // Resolve from the discovered entrypoint's directory: GSTACK_ROOT is an + // independently configurable runtime asset tree, not a skill registry. + // This also preserves custom CODEX_HOME installations without guessing HOME. + return `the sibling registry file \`../${file}\`, relative to the installed \`/autoplan\` SKILL.md directory (local: \`${local}\`; global: \`${global}\`${ctx.host === 'codex' ? ', or the corresponding skills directory under CODEX_HOME when configured' : ''})`; +} diff --git a/scripts/resolvers/index.ts b/scripts/resolvers/index.ts index abc586728..db6568595 100644 --- a/scripts/resolvers/index.ts +++ b/scripts/resolvers/index.ts @@ -26,7 +26,7 @@ import { generateReviewDashboard, generatePlanFileReviewReport, generateExitPlan import { generateSlugEval, generateSlugSetup, generateBaseBranchDetect, generateDeployBootstrap, generateQAMethodology, generateCoAuthorTrailer, generateChangelogWorkflow, generateCodexWebSearchFlag, generateSetupCommand } from './utility'; import { generateLearningsSearch, generateLearningsLog } from './learnings'; import { generateConfidenceCalibration } from './confidence'; -import { generateInvokeSkill } from './composition'; +import { generateInvokeSkill, generateAutoplanReviewFile } from './composition'; import { generateReviewArmy } from './review-army'; import { generateDxFramework } from './dx'; import { generateGBrainContextLoad, generateGBrainSaveResults, generateBrainPreflight, generateBrainCacheRefresh, generateBrainWriteBack } from './gbrain'; @@ -101,6 +101,7 @@ export const RESOLVERS: Record = { LEARNINGS_LOG: generateLearningsLog, CONFIDENCE_CALIBRATION: generateConfidenceCalibration, INVOKE_SKILL: generateInvokeSkill, + AUTOPLAN_REVIEW_FILE: generateAutoplanReviewFile, CHANGELOG_WORKFLOW: generateChangelogWorkflow, REVIEW_ARMY: generateReviewArmy, CROSS_REVIEW_DEDUP: generateCrossReviewDedup, diff --git a/test/autoplan-phase-order.test.ts b/test/autoplan-phase-order.test.ts index c7c3b2eea..1443c3097 100644 --- a/test/autoplan-phase-order.test.ts +++ b/test/autoplan-phase-order.test.ts @@ -68,3 +68,61 @@ describe('autoplan phase order (Eng always last)', () => { expect(ceo).toContain('Final'); }); }); + +describe('autoplan phase execution checkpoints', () => { + const tmpl = read('autoplan/SKILL.md.tmpl'); + const phases = ['ceo', 'design', 'dx', 'eng']; + + test('loads full review skills at phase entry instead of prefetching future phases', () => { + const intake = tmpl.split('### Step 3:')[1]?.split('## Phase 0.5:')[0] ?? ''; + expect(intake).toContain('At the start of each applicable phase'); + expect(intake).toContain('read its full review SKILL.md'); + expect(intake).toContain('Do not prefetch future phase sections or review skills'); + for (const phase of phases) { + expect(read(`autoplan/sections/${phase}-phase.md.tmpl`)).toMatch(/^Read \{\{AUTOPLAN_REVIEW_FILE:plan-[a-z-]+\}\} in full now/); + } + }); + + for (const phase of phases) { + test(`${phase} places explicit foreground dispatch and its completion barrier before the outside invocation`, () => { + const section = read(`autoplan/sections/${phase}-phase.md.tmpl`); + const native = section.indexOf(`**{{NATIVE_LABEL}} ${phase === 'design' ? 'design' : phase === 'dx' ? 'DX' : phase === 'ceo' ? 'CEO' : 'eng'} subagent**`); + const outside = section.indexOf('{{OUTSIDE_INVOCATION:autoplan}}'); + expect(native).toBeGreaterThan(-1); + expect(native).toBeLessThan(outside); + const dispatch = section.slice(native, outside); + expect(dispatch).toContain('"run_in_background": false'); + expect(dispatch).toContain('isAsync: true'); + expect(dispatch).toContain('wait for that same agent'); + expect(dispatch).toContain('before outside dispatch or parent review'); + // Provider preflight, timeout and native fallback remain at every call. + expect(section).toContain('Outer tool timeout: 720000ms'); + expect(section).toContain('Disabled skips the outside invocation; it retains the native pass.'); + expect(section).toContain(`{{OUTSIDE_PROVENANCE:${phase}}}`); + }); + } + + test('each completed phase announces only after persisted full outputs and settled reviewers', () => { + for (const [phase, number] of [['ceo', '1'], ['design', '2'], ['dx', '2.5'], ['eng', '3']]) { + const section = read(`autoplan/sections/${phase}-phase.md.tmpl`); + const barrier = section.indexOf('**Close this phase before continuing:**'); + const announcement = section.indexOf(`\n**Phase ${number} complete.**\n`); + expect(barrier).toBeGreaterThan(-1); + expect(barrier).toBeLessThan(announcement); + const checkpoint = section.slice(barrier, announcement); + expect(checkpoint).toContain('successful Write/Edit results'); + expect(checkpoint).toContain('terminal status'); + expect(checkpoint).toContain('actual assistant message'); + expect(checkpoint).toContain('Do not announce completion while required work remains'); + } + }); + + test('Design hands off to conditional DX and DX never requests a future Eng result', () => { + const design = read('autoplan/sections/design-phase.md.tmpl'); + const dx = read('autoplan/sections/dx-phase.md.tmpl'); + expect(design).toContain('Passing to Phase 2.5 (DX Review) if DX scope was detected; otherwise Phase 3'); + expect(design).not.toContain('> Passing to Phase 3.'); + expect(dx).toContain("Design: "); + expect(dx).not.toContain('Eng: '); + }); +}); diff --git a/test/autoplan-review-discovery.test.ts b/test/autoplan-review-discovery.test.ts new file mode 100644 index 000000000..b3423b83a --- /dev/null +++ b/test/autoplan-review-discovery.test.ts @@ -0,0 +1,94 @@ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { spawnSync } from 'child_process'; +import { ALL_HOST_CONFIGS } from '../hosts'; +import { generateAutoplanReviewFile } from '../scripts/resolvers/composition'; +import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; +import { E2E_TOUCHFILES } from './helpers/touchfiles'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const REVIEWS = ['plan-ceo-review', 'plan-design-review', 'plan-devex-review', 'plan-eng-review']; +let owned: string; +let rendered: string; + +function installFile(source: string, destination: string, mode: 'copy' | 'symlink') { + fs.mkdirSync(path.dirname(destination), { recursive: true }); + if (mode === 'copy') fs.copyFileSync(source, destination); + else fs.symlinkSync(source, destination); +} + +beforeAll(() => { + owned = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-autoplan-discovery-')); + rendered = path.join(owned, 'rendered'); + const result = spawnSync(process.execPath, ['run', 'scripts/gen-skill-docs.ts', '--host', 'all', '--out-dir', rendered], { + cwd: ROOT, encoding: 'utf8', timeout: 120_000, + }); + expect(result.status, result.stderr).toBe(0); +}, 120_000); + +afterAll(() => { if (owned) fs.rmSync(owned, { recursive: true, force: true }); }); + +describe('autoplan reads installed host methodology', () => { + for (const host of ALL_HOST_CONFIGS) { + for (const mode of (process.platform === 'win32' ? ['copy'] : ['copy', 'symlink']) as Array<'copy' | 'symlink'>) { + test(`${host.name} reads its generated review files from ${host.name === 'claude' ? 'the canonical global path' : 'local and global paths'} in ${mode} installations`, () => { + const generatedRoot = host.name === 'claude' ? rendered : path.join(rendered, host.hostSubdir, 'skills'); + const entryName = host.name === 'claude' ? 'autoplan' : 'gstack-autoplan'; + const generatedEntry = path.join(generatedRoot, entryName, 'SKILL.md'); + const body = fs.readFileSync(generatedEntry, 'utf8'); + if (host.name === 'claude') { + for (const review of REVIEWS) expect(body).toContain(`~/.claude/skills/gstack/${review}/SKILL.md`); + } else { + // Read the path actually emitted in the entrypoint. Resolving it from + // the installed entrypoint works for copied installs and symlinked ones. + const refs = [...body.matchAll(/`(\.\.\/gstack-plan-[a-z-]+\/SKILL\.md)`/g)].map(match => match[1]!); + expect([...new Set(refs)].sort()).toEqual(REVIEWS.map(name => `../gstack-${name}/SKILL.md`).sort()); + for (const review of REVIEWS) expect(body).not.toContain(`$GSTACK_ROOT/${review}/SKILL.md`); + expect(body).toContain('same installed skill registry as /autoplan'); + } + const roots = host.name === 'claude' + ? [path.join(owned, host.name, mode, 'home', host.globalRoot)] + : [path.join(owned, host.name, mode, 'repo', path.dirname(host.localSkillRoot)), path.join(owned, host.name, mode, 'home', path.dirname(host.globalRoot))]; + if (host.name === 'codex') roots.push(path.join(owned, 'codex', mode, 'custom-codex-home', 'skills')); + for (const registry of roots) { + const entry = path.join(registry, entryName, 'SKILL.md'); + installFile(generatedEntry, entry, mode); + for (const review of REVIEWS) { + const reviewName = host.name === 'claude' ? review : `gstack-${review}`; + const generatedReview = path.join(generatedRoot, reviewName, 'SKILL.md'); + installFile(generatedReview, path.join(registry, reviewName, 'SKILL.md'), mode); + // A runtime root may be an unrelated checkout. Its old canonical + // review file must never be selected instead of the installed host. + const stale = path.join(registry, 'gstack', review, 'SKILL.md'); + fs.mkdirSync(path.dirname(stale), { recursive: true }); + fs.writeFileSync(stale, 'STALE FOREIGN HARNESS SKILL'); + const reference = host.name === 'claude' ? `../${review}/SKILL.md` : [...body.matchAll(/`(\.\.\/gstack-plan-[a-z-]+\/SKILL\.md)`/g)].find(match => match[1] === `../gstack-${review}/SKILL.md`)![1]!; + const loaded = fs.readFileSync(path.resolve(path.dirname(entry), reference), 'utf8'); + expect(loaded).toBe(fs.readFileSync(generatedReview, 'utf8')); + expect(loaded).not.toContain('STALE FOREIGN HARNESS SKILL'); + if (host.name === 'codex') expect(loaded).toContain('"outside_provider":"claude-code"'); + } + } + }); + } + } + + test('host identity, not the model overlay, selects the registry; invalid skills fail closed', () => { + for (const host of ALL_HOST_CONFIGS) { + const ctx = { skillName: 'autoplan', tmplPath: '', host: host.name, paths: HOST_PATHS[host.name] } as TemplateContext; + for (const review of REVIEWS) { + expect(generateAutoplanReviewFile({ ...ctx, model: 'gpt' }, [review])).toBe(generateAutoplanReviewFile({ ...ctx, model: 'claude' }, [review])); + } + expect(() => generateAutoplanReviewFile(ctx, ['../foreign'])).toThrow(); + } + }); + + test('the new discovery contract selects the affected live autoplan workflows', () => { + for (const name of ['autoplan-chain-pty', 'autoplan-dual-voice', 'carve-section-loading']) { + expect(E2E_TOUCHFILES[name]).toContain('test/autoplan-review-discovery.test.ts'); + expect(E2E_TOUCHFILES[name]).toContain('scripts/resolvers/composition.ts'); + } + }); +}); diff --git a/test/autoplan-setup-question.test.ts b/test/autoplan-setup-question.test.ts index def47dd86..a620f3f7f 100644 --- a/test/autoplan-setup-question.test.ts +++ b/test/autoplan-setup-question.test.ts @@ -137,7 +137,7 @@ describe('autoplan routing setup handling', () => { 'No thanks, delete the file manually', 'No thanks, manual data migration', 'No thanks, invoke the deploy manually', - 'Manual invocation', + 'Manual deployment invocation', 'Accept recommendation', 'No thanks, manual invocation then delete CLAUDE.md', ]) { @@ -270,3 +270,50 @@ describe('autoplan routing setup handling', () => { } }); }); + +// Source-G's retry remained at this actual captured menu until shard timeout. +// The action is intact; cumulative ANSI stripping loses the courtesy's 'o'. +// A real xterm replay retains it in the prior screen cell. +const G_ROUTING_CAPTURE = [ + '☐Routingrules', + "│gstackworksbestwhenyourproject'sCLAUDE.mdincludesskillroutingrules.Wouldyouliketoaddthem?", + '❯1.AddroutingrulestoCLAUDE.md', + 'AppendsstandardskillroutingrulestoCLAUDE.md(creatingitifabsent)andcommits.Meansgstackskillslike', + '/autoplan,/ship,/qaetc.getinvokedautomaticallywhenthetaskmatches.(recommended)', + "2. N thanks, I'll invokeskillsmanually", + 'Skiprouting setup. You can re-enable later by removing the routing_declined flag.', + '3.Typesomething.', + '4.Chataboutthis', + 'Enter toselect · ↑/↓ to navigate · Esc to cancel', +].join('\r'); + +describe('autoplan routing action survives courtesy repaint', () => { + test('selects the explicit Add action once in the captured G menu, in both orders', () => { + const seen = new Set(); + expect(autoplanRoutingSetupInput(G_ROUTING_CAPTURE, seen)).toBe('1\r'); + expect(autoplanRoutingSetupInput(G_ROUTING_CAPTURE, seen)).toBeNull(); + const reversed = G_ROUTING_CAPTURE.replace('❯1.AddroutingrulestoCLAUDE.md', "❯1.N thanks, I'll invokeskillsmanually") + .replace("2. N thanks, I'll invokeskillsmanually", '2.AddroutingrulestoCLAUDE.md'); + expect(autoplanRoutingSetupInput(reversed, new Set())).toBe('2\r'); + }); + + test('the actual manual-invocation action needs no courtesy formula', () => { + for (const action of ['Manual invocation', 'Invoke skills manually', "I'll invoke skills manually", 'Thanks, invoke manually']) { + expect(autoplanRoutingSetupInput(G_ROUTING_CAPTURE.replace("N thanks, I'll invokeskillsmanually", action), new Set()), action).toBe('1\r'); + } + }); + + test('still requires exact opposed setup actions and a genuine routing premise', () => { + for (const label of [ + 'N thanks', 'Invoke the deployment manually', 'N thanks, manual data migration', + 'Delete CLAUDE.md, invoke skills manually', 'No thanks, invoke skills manually then delete CLAUDE.md', + 'Skip the review, invoke skills manually', 'Skip the review thanks, invoke skills manually', + ]) expect(autoplanRoutingSetupInput(G_ROUTING_CAPTURE.replace("N thanks, I'll invokeskillsmanually", label), new Set()), label).toBeNull(); + for (const frame of [ + G_ROUTING_CAPTURE.replace("gstackworksbestwhenyourproject'sCLAUDE.mdincludesskillroutingrules.Wouldyouliketoaddthem?", 'Which application router should we implement?'), + G_ROUTING_CAPTURE.replace('AddroutingrulestoCLAUDE.md', 'AddrutingrulestoCLAUDE.md'), + G_ROUTING_CAPTURE.replace('3.Typesomething.', '3.Invoke skills manually'), + G_ROUTING_CAPTURE.replace('3.Typesomething.', '3.Add routing rules'), + ]) expect(autoplanRoutingSetupInput(frame, new Set()), frame).toBeNull(); + }); +}); diff --git a/test/ceo-mode-option.test.ts b/test/ceo-mode-option.test.ts index c728aa3a9..43b259e06 100644 --- a/test/ceo-mode-option.test.ts +++ b/test/ceo-mode-option.test.ts @@ -112,6 +112,27 @@ describe('CEO mode navigation replay', () => { expect(nextCeoModeNavigation(`${permission}\r${actual}`, 'HOLD SCOPE', seen).kind).toBe('question'); }); + test('file permission lifecycle is shared by navigation and posture without becoming an AUQ', () => { + const seen = new Set(); + const permission = 'Do you want to overwrite CLAUDE.md?\n❯1.Yes\n2.No\nEsc to cancel · Tab to amend'; + const transcript: PlanCountTranscript = { status: 'missing', calls: [], assistantMessages: [] }; + expect(nextCeoModeNavigation(permission, 'HOLD SCOPE', seen).kind).toBe('permission'); + expect(nextCeoModeNavigation(permission, 'HOLD SCOPE', seen).kind).toBe('wait'); + expect(nextCeoPostureContinuation(permission, transcript, 'HOLD SCOPE', 0, seen, true)).toBeNull(); + const completed = permission + '\n⎿ Added1line\n'; + expect(nextCeoPostureContinuation(completed, transcript, 'HOLD SCOPE', 0, seen, true)).toBeNull(); + expect(nextCeoModeNavigation(completed, 'HOLD SCOPE', seen).kind).toBe('wait'); + const again = completed + permission; + expect(nextCeoPostureContinuation(again, transcript, 'HOLD SCOPE', 0, seen, true)).toBe('permission'); + expect(nextCeoModeNavigation(again, 'HOLD SCOPE', seen).kind).toBe('wait'); + expect(seen.size).toBe(0); + const question = '☐ Approaches\nWhich storage strategy?\n❯1.Server\n2.Local'; + expect(nextCeoModeNavigation(again + '\n' + question, 'HOLD SCOPE', seen).kind).toBe('question'); + expect(seen.size).toBe(1); + // Different sessions retain independent permission state. + expect(nextCeoModeNavigation(permission, 'HOLD SCOPE', new Set()).kind).toBe('permission'); + }); + 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) { diff --git a/test/claude-code-migration.test.ts b/test/claude-code-migration.test.ts index dd01abdd3..60ab840a4 100644 --- a/test/claude-code-migration.test.ts +++ b/test/claude-code-migration.test.ts @@ -166,7 +166,7 @@ describe('Claude wrapper installed-name migration', () => { expect(setup).toContain('export GSTACK_DEFER_CLAUDE_RENAME_PRUNE=1'); expect(setup).toContain('unset GSTACK_DEFER_CLAUDE_RENAME_PRUNE'); expect(setup).toContain('[ "$n" = "gstack-claude" ]'); - const version = path.join(ROOT, 'gstack-upgrade/migrations/v1.82.0.0.sh'); + const version = path.join(ROOT, 'gstack-upgrade/migrations/v1.83.0.0.sh'); expect(fs.statSync(version).mode & 0o111).not.toBe(0); expect(fs.readFileSync(version, 'utf8')).toContain('gstack-migrate-claude-code'); }); diff --git a/test/disabled-plan-review-evidence.test.ts b/test/disabled-plan-review-evidence.test.ts index 7a32f6d9c..12d9864c3 100644 --- a/test/disabled-plan-review-evidence.test.ts +++ b/test/disabled-plan-review-evidence.test.ts @@ -38,6 +38,37 @@ describe('disabled outside-plan live oracle', () => { test('requires a real disabled preflight with Agent available and valid completion', () => { expect(oracle(completed(), '').passed).toBe(true); }); + test('captured native init exposes the requested subagent as Task on Claude Code 2.1.257', () => { + const result = completed(); + // G's actual init canonicalized the requested Agent tool to Task. + result.transcript[0] = { + type: 'system', subtype: 'init', + tools: ['Task', 'Bash', 'Glob', 'Grep', 'Read', 'Write'], + model: 'claude-sonnet-4-6', claude_code_version: '2.1.257', + }; + expect(oracle(result)).toMatchObject({ passed: true, completed: true, + agentAvailable: true, persistedDisabled: true, fallbackCalls: [] }); + }); + test('unknown, malformed and lookalike tools do not establish subagent availability', () => { + for (const tools of [undefined, null, 'Agent', {}, [], ['Bash', 'Read'], + ['AgentTool'], ['TaskCreate', 'TaskList', 'TaskOutput', 'TaskStop'], + ['agent', 'task'], ['mcp__custom__Agent'], [{ name: 'Agent' }]]) { + const result = completed(); result.transcript[0].tools = tools; + expect(oracle(result)).toMatchObject({ passed: false, agentAvailable: false }); + } + }); + test('subagent availability must come from the native system init', () => { + for (const init of [ + { type: 'assistant', subtype: 'init', tools: ['Task'] }, + { type: 'system', subtype: 'other', tools: ['Agent'] }, + { type: 'system', subtype: 'init' }, + ]) { + const result = completed(); result.transcript[0] = init; + expect(oracle(result)).toMatchObject({ passed: false, agentAvailable: false }); + } + const result = completed(); result.transcript.shift(); + expect(oracle(result)).toMatchObject({ passed: false, agentAvailable: false }); + }); test('claimed disabled status without matching successful tool evidence cannot pass', () => { for (const mutate of [ (r: ReturnType) => { r.transcript.splice(1, 2); }, @@ -67,9 +98,11 @@ describe('disabled outside-plan live oracle', () => { } }); test('Agent/Task fallback dispatch fails even when the parent reports disabled', () => { - for (const tool of ['Agent', 'Task']) { - const result = completed(); result.transcript.splice(-1, 0, dispatch(tool, { prompt: 'Review the plan independently' })); + for (const available of ['Agent', 'Task']) for (const tool of ['Agent', 'Task']) { + const result = completed(); result.transcript[0].tools = ['Bash', 'Read', available]; + result.transcript.splice(-1, 0, dispatch(tool, { prompt: 'Review the plan independently' })); const evidence = oracle(result, ''); + expect(evidence.agentAvailable).toBe(true); expect(evidence.passed).toBe(false); expect(evidence.fallbackCalls).toHaveLength(1); } diff --git a/test/fixtures/pty-screen/autoplan.json b/test/fixtures/pty-screen/autoplan.json new file mode 100644 index 000000000..de88132d7 --- /dev/null +++ b/test/fixtures/pty-screen/autoplan.json @@ -0,0 +1,94 @@ +{ + "source": "Captured source-G synthetic eval session,2026-09-08; actual prior viewport plus following VT redraw.", + "cols": 120, + "rows": 40, + "initial": [ + "", + " ▐▛███▛█ Claude Code v2.1.257", + "▝▜██████▀ Sonnet 4.6 · API Usage Billing", + " ▝▝ ▝▝ /tmp/gstack-paid-shard-W6Y9pi/tmp/gstack-autoplan-chain-n4IM84", + "", + "", + "❯ /autoplan", + "", + "● Starting the autoplan pipeline — running the preamble first.", + "", + " Ran 1 shell command", + "", + "● I have the preamble. There's a one-time routing instruction to handle, and I need to understand the repo before", + " proceeding. Let me run context recovery and explore the repo structure in parallel.", + "", + " Listed 1 directory, ran 2 shell commands", + "", + "● There's a plan file to review. Let me read it along with the README.", + "", + " Read 2 files", + "", + "● Good — there's a plan at .claude/plans/ui-heavy-feature.md (User Dashboard page) to review. Before starting the", + " pipeline, I need to handle the routing-injection onboarding step from the preamble.", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "· Philosophising… (1m 32s · ↓ 3.5k tokens)", + " ⎿  Tip: Use /btw to ask a quick side question without interrupting Claude's current work", + " ✔ Update installed · Restart to update", + "────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────", + "❯ ", + "────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────", + " ⏸ plan mode on (shift+tab to cycle) · esc to interrupt · ← for agents" + ], + "cursor": { + "x": 2, + "y": 37 + }, + "update": "\u001b[H\u001b[2G▐▛███▛█\u001b[12GClaude\u001b[19GCode\u001b[24Gv2.1.257\r\u001b[1B▝▜█\u001b[7G█\u001b[9G▀\u001b[12GSonnet\u001b[19G4.6 · API Usage\u001b[35GBilling\r\u001b[1B ▝▝ ▝▝ \u001b[12G/tmp/gstack-paid-sh\u001b[32Grd-W6Y9pi/tmp/gstack-autoplan-chain-n4IM84\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2B❯\u001b[3G/autoplan\r\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GStarting\u001b[12Gthe\u001b[16Gautoplan\u001b[25Gpipeline\u001b[34G—\u001b[36Grunning\u001b[44Gthe\u001b[48Gpreamble\u001b[57Gfirst.\r\u001b[1B\u001b[K\r\u001b[2C\u001b[1BRan\u001b[7G1\u001b[9Gshell\u001b[15Gcommand\r\u001b[2C\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GI\u001b[5Ghave\u001b[10Gthe\u001b[14Gpreamble.\u001b[24GThere's\u001b[32Ga\u001b[34Gone-time\u001b[43Grouting\u001b[51Ginstruction\u001b[63Gto\u001b[66Ghandle,\u001b[74Gand\u001b[78GI\u001b[80Gneed\u001b[85Gto\u001b[88Gunderstand\u001b[99Gthe\u001b[103Grepo\u001b[108Gbefore\r\u001b[1B \u001b[3Gproce\u001b[9Gding. L\u001b[17Gt me run context\u001b[34Grecovery\u001b[43Gand explore the repo structure\u001b[74Gi\u001b[76G parallel.\u001b[K\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BListed\u001b[10G1\u001b[12Gdirectory,\u001b[23Gran\u001b[27G2\u001b[29Gshell\u001b[35Gcommands\r\u001b[2C\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GThere's\u001b[11Ga\u001b[13Gplan\u001b[18Gfile\u001b[23Gto\u001b[26Greview.\u001b[34GLet\u001b[38Gme\u001b[41Gread\u001b[46Git\u001b[49Galong\u001b[55Gwith\u001b[60Gthe\u001b[64GREADME.\r\u001b[1B\u001b[K\r\u001b[2C\u001b[1BRead\u001b[8G2\u001b[10Gfiles\r\u001b[2C\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GGood\u001b[8G—\u001b[10Gthere's\u001b[18Ga\u001b[20Gplan\u001b[25Gat\u001b[28G.claude/plans/ui-heavy-feature.md\u001b[62G(User\u001b[68GDashboard\u001b[78Gpage)\u001b[84Gto\u001b[87Greview.\u001b[95GBefore\u001b[102Gstarting\u001b[111Gthe\r\u001b[1B \u001b[3Gpipeline, I need\u001b[20Gto handle the routi\u001b[40Gg-injection onboarding st\u001b[66Gp\u001b[68Gfrom the preamble.\u001b[87G\u001b[K\r\u001b[1B────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\r\u001b[1BPlanning:\r\u001b[1B/tmp/gstack-paid-shard-W6Y9pi/tmp/gstack-hermetic-3071829-QCsZIh/with-skills/.claude/plans/quirky-tumbling-sky.md\r\u001b[1B────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\r\u001b[1C\u001b[1B☐\u001b[4GRouting\u001b[12Grules\r\u001b[2B│\u001b[3Ggstack\u001b[10Gworks\u001b[16Gbest\u001b[21Gwhen\u001b[26Gyour\u001b[31Gproject's\u001b[41GCLAUDE.md\u001b[51Gincludes\u001b[60Gskill\u001b[66Grouting\u001b[74Grules.\u001b[81GWould\u001b[87Gyou\u001b[91Glike\u001b[96Gto\u001b[99Gadd\u001b[103Gthem?\r\u001b[2B❯\u001b[3G1.\u001b[6GAdd\u001b[10Grouting\u001b[18Grules\u001b[24Gto\u001b[27GCLAUDE.md\r\u001b[5C\u001b[1BAppends\u001b[14Gstandard\u001b[23Gskill\u001b[29Grouting\u001b[37Grules\u001b[43Gto\u001b[46GCLAUDE.md\u001b[56G(creating\u001b[66Git\u001b[69Gif\u001b[72Gabsent)\u001b[80Gand\u001b[84Gcommits.\u001b[93GMeans\u001b[99Ggstack\u001b[106Gskills\u001b[113Glike\r\u001b[5C\u001b[1B/autoplan,\u001b[17G/ship,\u001b[24G/qa\u001b[28Getc.\u001b[33Gget\u001b[37Ginvoked\u001b[45Gautomatically\u001b[59Gwhen\u001b[64Gthe\u001b[68Gtask\u001b[73Gmatches.\u001b[82G(recommended)\r\u001b[1B \u001b[3G2. N\u001b[8G thanks, I'll invoke\u001b[29Gskills\u001b[36Gmanually\r\u001b[2C\u001b[1B \u001b[5G Skip\u001b[11Grouting setup. You can re-enabl\u001b[43G later by removing the routing_\u001b[76Gclined flag.\u001b[K\r\u001b[2C\u001b[1B3.\u001b[6GType\u001b[11Gsomething.\u001b[81G\u001b[K\r\u001b[2B 4.\u001b[6GChat\u001b[11Gabout\u001b[17Gthis\r\u001b[1B\u001b[K\r\u001b[1BEnter to\u001b[10Gsel\u001b[14Gct · ↑/↓ \u001b[24Go navigate · Esc\u001b[41Gto cancel\u001b[K\u001b[40;1H\u001b[31;1H", + "expected": [ + " ▐▛███▛█ Claude Code v2.1.257", + "▝▜██████▀ Sonnet 4.6 · API Usage Billing", + " ▝▝ ▝▝ /tmp/gstack-paid-shard-W6Y9pi/tmp/gstack-autoplan-chain-n4IM84", + "", + "", + "❯ /autoplan", + "", + "● Starting the autoplan pipeline — running the preamble first.", + "", + " Ran 1 shell command", + " ", + "● I have the preamble. There's a one-time routing instruction to handle, and I need to understand the repo before", + " proceeding. Let me run context recovery and explore the repo structure in parallel.", + "", + " Listed 1 directory, ran 2 shell commands", + " ", + "● There's a plan file to review. Let me read it along with the README.", + "", + " Read 2 files", + " ", + "● Good — there's a plan at .claude/plans/ui-heavy-feature.md (User Dashboard page) to review. Before starting the", + " pipeline, I need to handle the routing-injection onboarding step from the preamble.", + "────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────", + "Planning:", + "/tmp/gstack-paid-shard-W6Y9pi/tmp/gstack-hermetic-3071829-QCsZIh/with-skills/.claude/plans/quirky-tumbling-sky.md", + "────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────", + " ☐ Routing rules", + "", + "│ gstack works best when your project's CLAUDE.md includes skill routing rules. Would you like to add them?", + "", + "❯ 1. Add routing rules to CLAUDE.md", + " Appends standard skill routing rules to CLAUDE.md (creating it if absent) and commits. Means gstack skills like", + " /autoplan, /ship, /qa etc. get invoked automatically when the task matches. (recommended)", + " 2. No thanks, I'll invoke skills manually", + " Skip routing setup. You can re-enable later by removing the routing_declined flag.", + " 3. Type something.", + "────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────", + " 4. Chat about this", + "", + "Enter to select · ↑/↓ to navigate · Esc to cancel" + ] +} diff --git a/test/fixtures/pty-screen/design.json b/test/fixtures/pty-screen/design.json new file mode 100644 index 000000000..a1e78823e --- /dev/null +++ b/test/fixtures/pty-screen/design.json @@ -0,0 +1,95 @@ +{ + "source": "Captured source-G synthetic eval session,2026-09-08; actual prior viewport plus following VT redraw.", + "cols": 120, + "rows": 40, + "initial": [ + "", + "❯ /plan-design-review ", + " ", + " Ran 1 shell command", + "", + "● Scope gate: plan mode active — auto-selecting B (reviewing PLAN.md in the working directory).", + "", + " Skipping routing injection — this is a review fixture repo. Now running the pre-review audit.", + " ", + " Read 2 files, ran 1 shell command", + "", + "● Now let me read the review sections file as required by the skill.", + "", + " Read 1 file, ran 1 shell command", + " ", + "● No prior learnings found. Cross-project config is not set — will ask once I have the user's attention.", + "", + " Pre-Review System Audit:", + " - DESIGN.md: exists and comprehensive (spacing, typography, color, interaction patterns all specified)", + " - UI scope: APP UI classifier — account-settings form with Profile + Notifications sections", + " - 5 gaps explicitly listed in the plan; all have corresponding spec in DESIGN.md", + " - No prior design reviews in the review log", + " - No TODOS.md exists", + "", + " Step 0 Assessment:", + "", + " Initial score: 5/10. The plan correctly identifies 5 real gaps and preserves strong accessibility and responsive specs", + " inherited from the existing behavior description. A 10/10 would specify HOW each gap is resolved (concrete component", + " changes, token values, before/after layout), not just name the problem. The DESIGN.md already has all correct values;", + " the plan just needs to reference them.", + "", + " Writing the initial plan file now before asking Step 0D.", + "", + "✽ Undulating… (1m 59s · ↓ 5.4k tokens)", + " ⎿  Tip: Use /btw to ask a quick side question without interrupting Claude's current work", + " ✔ Update installed · Restart to update", + "────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────", + "❯ ", + "────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────", + " ⏸ plan mode on (shift+tab to cycle) · esc to interrupt · ← for agents" + ], + "cursor": { + "x": 2, + "y": 37 + }, + "update": "\u001b[H\u001b[3G14\u001b[6G-\u001b[8G**Initial\u001b[18Gscore:**\u001b[27G5/10\u001b[32G—\u001b[34Gplan\u001b[39Gcorrectly\u001b[49Gnames\u001b[55G5\u001b[57Ggaps\u001b[62Gbut\u001b[66Gdoes\u001b[71Gnot\u001b[75Gspecify\u001b[83GHOW\u001b[87Geach\u001b[92Gis\u001b[95Gresolved.\r\u001b[1B \u001b[3G15\u001b[K\r\u001b[2C\u001b[1B16\u001b[6G##\u001b[9GStep\u001b[14G0:\u001b[17GDesign\u001b[24GScope\u001b[30GAssessment\r\u001b[2C\u001b[1B17\u001b[K\r\u001b[2C\u001b[1B18\u001b[6G**Initial\u001b[16Grating:\u001b[24G5/10**\r\u001b[1B \u001b[3G19\u001b[K\r\u001b[2C\u001b[1B20\u001b[6GThe\u001b[10Gplan\u001b[15Gearns\u001b[21Ga\u001b[23G5\u001b[25Gbecause:\r\u001b[2C\u001b[1B21 - It correctly id\u001b[24Gn\u001b[27Gfies\u001b[32G5 real,\u001b[40Gspecific\u001b[49Ggaps (Visual Hiera\u001b[68Gchy, Spacing, Color, Typography,\u001b[101GMotion)\r\u001b[2C\u001b[1B22\u001b[6G-\u001b[8GIt\u001b[11Gpreserves\u001b[21Gstrong\u001b[28Gaccessibility\u001b[42Gand\u001b[46Gresponsive\u001b[57Gspecs\u001b[63Gfrom\u001b[68Gthe\u001b[72Gexisting\u001b[81Gbehavior\u001b[90Gdescription\r\u001b[2C\u001b[1B23 -\u001b[8GDESIGN.md exist\u001b[24G and\u001b[29Gsupplies\u001b[38Gcorrect\u001b[46Gvalues\u001b[53Gfor\u001b[57Gall\u001b[61G5\u001b[63Ggaps\r\u001b[2C\u001b[1B24\r\u001b[1B \u001b[3G25 A 10/10\u001b[14Gwould:\u001b[K\r\u001b[2C\u001b[1B26\u001b[6G-\u001b[8GSpecify\u001b[16Gthe\u001b[20Gexact\u001b[26Gchange\u001b[33Gfor\u001b[37Geach\u001b[42Ggap\u001b[46G(concrete\u001b[56Gtoken,\u001b[63Gbefore\u001b[70G→\u001b[72Gafter)\r\u001b[2C\u001b[1B27 -\u001b[8GAdd inte\u001b[18Gction state table\u001b[36G(loading,\u001b[46Gempty,\u001b[53Gerror,\u001b[60Gsuccess,\u001b[69Gpartial)\r\u001b[2C\u001b[1B28\u001b[6G-\u001b[8GStoryboard\u001b[19Gthe\u001b[23Guser\u001b[28Gjourney\r\u001b[1B \u001b[3G29\u001b[6G- Call out which DESIGN.md tokens each fix applies\u001b[57G\u001b[K\r\u001b[2C\u001b[1B30\u001b[6G-\u001b[8GResolve\u001b[16Gall\u001b[20Gimplementation\u001b[35Gambiguities\u001b[47Gbefore\u001b[54Ga\u001b[56Gdeveloper\u001b[66Gpicks\u001b[72Git\u001b[75Gup\r\u001b[2C\u001b[1B31\u001b[K\r\u001b[2C\u001b[1B32 **DESIGN.md status:** Pres\u001b[33Gnt and detailed. Spacing scale, c\u001b[67Glor system, typography,\u001b[91Ginteract\u001b[100Gon patterns,\u001b[113Gand\u001b[117Gc\r\u001b[2C\u001b[1B \u001b[5G omponent\u001b[15Gbehavior \u001b[25Gre all specified. This review cal\u001b[59Gbrates against it.\u001b[K\r\u001b[2C\u001b[1B33\u001b[K\r\u001b[2C\u001b[1B34 **Ex\u001b[11Gsting design l\u001b[26Gverage:**\u001b[36G\u001b[K\r\u001b[2C\u001b[1B35 -\u001b[8GButton, Field, InlineStatus,\u001b[37GErrorSummary,\u001b[51GConfirmationDialog\u001b[70Gcomponents\u001b[81Greused\u001b[88Gas-is\r\u001b[2C\u001b[1B36\u001b[6G-\u001b[8GFocus\u001b[14Gring:\u001b[20G2px\u001b[24Gsolid\u001b[30G#1d4ed8,\u001b[39Goffset\u001b[46G2px\u001b[50G(already\u001b[59Gspecified)\r\u001b[2C\u001b[1B37 -\u001b[8G8px ba\u001b[15Ge spacing\u001b[25Gscale\u001b[31Galready\u001b[39Gestablished\r\u001b[2C\u001b[1B38\r\u001b[2C\u001b[1B39 ---\u001b[K\r\u001b[2C\u001b[1B40\u001b[K\r\u001b[2C\u001b[1B41 ## Review Passes\u001b[K\r\u001b[2C\u001b[1B42\u001b[K\r\u001b[2C\u001b[1B43\u001b[6G*(Populated\u001b[18Gprogressively\u001b[32Gas\u001b[35Gthe\u001b[39Greview\u001b[46Gproceeds)*\r\u001b[2C\u001b[1B44\u001b[K\r\u001b[1B╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌\r\u001b[1B Do you wa\u001b[12Gt to create gstack-test-plan-design.md?\r\u001b[1C\u001b[1B❯ 1. Yes\u001b[11G\u001b[K\r\u001b[3C\u001b[1B2.\u001b[7GYes,\u001b[12Gand\u001b[16Gswitch\u001b[23Gto\u001b[26Gaccept\u001b[33Gedits\u001b[39G(auto-approve\u001b[53Gfile\u001b[58Gedits\u001b[64Gand\u001b[68Gcommon\u001b[75Gfile\u001b[80Gcommands)\u001b[90Gfor this session; Yes,\u001b[113Gan\u001b[116G\u001b[K\r\u001b[1B always allow access to /tmp/gstack-paid-shard-jPAIoN/tmp/gstack-e2e-plan-design-ga2LJt for this session\u001b[K\r\u001b[1B \u001b[4G3.\u001b[7GNohift+tab)\r\u001b[1B\u001b[K\r\u001b[1C\u001b[1BEsc to canc\u001b[14Gl ·\u001b[18GTab to amend\u001b[K\u001b[40;1H\u001b[35;2H", + "completed": "\u001b(B\u000f\u001b[?1000h\u001b[?1002h\u001b[?1003h\u001b[?1006h\u001b]0;◐ Claude Code\u0007\u001b[H\u001b[3G\u001b[K\r\u001b[2C\u001b[1BRan\u001b[7G1\u001b[9Gshell\u001b[15Gcommand\r\u001b[2C\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GScope\u001b[9Ggate:\u001b[15Gplan\u001b[20Gmode\u001b[25Gactive\u001b[32G—\u001b[34Gauto-selecting\u001b[49GB\u001b[51G(reviewing\u001b[62GPLAN.md\u001b[70Gin\u001b[73Gthe\u001b[77Gworking\u001b[85Gdirectory).\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BSkipping\u001b[12Grouting\u001b[20Ginjection\u001b[30G—\u001b[32Gthis\u001b[37Gis\u001b[40Ga\u001b[42Greview\u001b[49Gfixture\u001b[57Grepo.\u001b[63GNow\u001b[67Grunning\u001b[75Gthe\u001b[79Gpre-review\u001b[90Gaudit.\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BRead\u001b[8G2 files, ran\u001b[21G1 shell command\u001b[K\r\u001b[2C\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GNow let me read the review sect\u001b[35Gons fil\u001b[43G as required by the\u001b[63Gskill.\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BRead\u001b[9G file, ran 1\u001b[22Gshell\u001b[28Gcommand\r\u001b[2C\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GNo\u001b[6Gprior\u001b[12Glearnings found. Cross-project config is\u001b[53Gnot set — will ask once I\u001b[79Ghave\u001b[84Gthe\u001b[88Guser's\u001b[95Gattention.\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BPre\u001b[7GReview System Audit:\u001b[K\r\u001b[2C\u001b[1B- DESIGN.md:\u001b[16Gexists and comprehensive (spacing, typography, color, interaction\u001b[82Gpatterns\u001b[91Gall\u001b[95Gspecified)\r\u001b[2C\u001b[1B- UI\u001b[8Gscope:\u001b[15GAPP\u001b[19GUI\u001b[22Gclassifier\u001b[33G—\u001b[35Gaccount-settings\u001b[52Gform\u001b[57Gwith\u001b[62GProfile\u001b[70G+\u001b[72GNotifications\u001b[86Gsections\r\u001b[2C\u001b[1B- 5 gaps explici\u001b[20Gly listed in the plan; \u001b[44Gl\u001b[46G have corresponding spec\u001b[71Gin DESIGN.md\u001b[K\r\u001b[2C\u001b[1B-\u001b[5GN\u001b[7G \u001b[9Grior design reviews in\u001b[32Gth\u001b[35G review\u001b[43Glog\u001b[K\r\u001b[2C\u001b[1B- No\u001b[8GTODOS.md\u001b[17Gexists\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BStep\u001b[8G0 Assessment:\u001b[K\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BInitial scor\u001b[16G: 5/10. The plan correctly identifies\u001b[54G5\u001b[56Greal\u001b[61Ggaps\u001b[66Gand\u001b[70Gpreserves\u001b[80Gstrong\u001b[87Gaccessibility\u001b[101Gand\u001b[105Gresponsive\u001b[116Gspecs\r\u001b[2C\u001b[1Binherited\u001b[13Gfrom\u001b[18Gthe\u001b[22Gexisting\u001b[31Gbehavior\u001b[40Gdescription.\u001b[53GA\u001b[55G10/10\u001b[61Gwould\u001b[67Gspecify\u001b[75GHOW\u001b[79Geach\u001b[84Ggap\u001b[88Gis\u001b[91Gresolved\u001b[100G(concrete\u001b[110Gcomponent\r\u001b[2C\u001b[1Bchanges,\u001b[12Gtoken\u001b[18Gvalues,\u001b[26Gbefore/after\u001b[39Glayout),\u001b[48Gnot\u001b[52Gjust\u001b[57Gname\u001b[62Gthe\u001b[66Gproblem.\u001b[75GThe\u001b[79GDESIGN.md\u001b[89Galready\u001b[97Ghas\u001b[101Gall\u001b[105Gcorrect\u001b[113Gvalues;\r\u001b[2C\u001b[1Bthe\u001b[7Gplan\u001b[12Gjust\u001b[17Gneeds\u001b[23Gto\u001b[26Greference\u001b[36Gthem.\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BWriting\u001b[11Gthe\u001b[15Ginitial\u001b[23Gplan\u001b[28Gfile\u001b[33Gnow\u001b[37Gbefore\u001b[44Gasking\u001b[51GStep\u001b[56G0D.\r\u001b[2C\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GWrite(/tmp/gstack-paid-shard-jPAIoN/tmp/gstack-e2e-plan-design-ga2LJt/gstack-test-plan-design.md)\r\u001b[1B\u001b[K\r\u001b[1B✢ Undulati\u001b[12Gg… (2m 0s ·\u001b[24G↓ 5.4\u001b[30G \u001b[32Gokens)\u001b[K\r\u001b[1C\u001b[1B ⎿  Tip:\u001b[11GUse\u001b[15G/btw\u001b[20Gto\u001b[23Gask\u001b[27Ga\u001b[29Gquick\u001b[35Gside\u001b[40Gquestion\u001b[49Gwithout\u001b[57Ginterrupting\u001b[70GClaude's\u001b[79Gcurrent\u001b[87Gwork\r\u001b[3C\u001b[1B \u001b[7G \u001b[12G \u001b[16G \u001b[23G \u001b[26G \u001b[33G \u001b[39G \u001b[53G \u001b[58G \u001b[64G \u001b[68G \u001b[75G \u001b[80G \u001b[90G \u001b[94G \u001b[99G ● high · /effort\r\u001b[1B────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\r\u001b[1B❯ \u001b[4G\u001b[K\r\u001b[1B────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\r\u001b[1C\u001b[1B ⏸ plan mod\u001b[14G on\u001b[18G(shift+tab to\u001b[32Gcycle)\u001b[39G·\u001b[41Gesc\u001b[45Gto\u001b[48Ginterrupt\u001b[58G·\u001b[60G←\u001b[62Gfor\u001b[66Gagents\u001b[40;1H\u001b[38;3H\u001b[?25h\u001b[?25l\u001b[H\r\u001b[1B●\u001b[3GNo prior \u001b[13Gearnings found.\u001b[29GCross-project\u001b[43Gconfig\u001b[50Gis\u001b[53Gnot\u001b[57Gset\u001b[61G—\u001b[63Gwill\u001b[68Gask\u001b[72Gonce\u001b[77GI\u001b[79Ghave\u001b[84Gthe\u001b[88Guser's\u001b[95Gattention.\r\u001b[2B \u001b[3GPre-Review System Audit:\u001b[K\r\u001b[2C\u001b[1B-\u001b[5GDESIGN.md:\u001b[16Gexists\u001b[23Gand\u001b[27Gcomprehensive\u001b[41G(spacing,\u001b[51Gtypography,\u001b[63Gcolor,\u001b[70Ginteraction\u001b[82Gpatterns\u001b[91Gall\u001b[95Gspecified)\r\u001b[2C\u001b[1B- UI scope: APP UI classifier — account-settings fo\u001b[55Gm\u001b[57Gwith Pr\u001b[65Gfile + Notifications s\u001b[88Gctions\u001b[K\r\u001b[2C\u001b[1B-\u001b[5G5\u001b[7Ggaps\u001b[12Gexplicitly\u001b[23Glisted\u001b[30Gin\u001b[33Gthe\u001b[37Gplan;\u001b[43Gall\u001b[47Ghave\u001b[52Gcorresponding\u001b[66Gspec\u001b[71Gin\u001b[74GDESIGN.md\r\u001b[2C\u001b[1B- No\u001b[8Gprior desig\u001b[21Grevi\u001b[26Gws\u001b[29Gin the review\u001b[43Glog\r\u001b[2C\u001b[1B-\u001b[5GNo\u001b[8GTODOS.md\u001b[17Gexists\r\u001b[1B\u001b[K\r\u001b[2C\u001b[1BStep\u001b[8G0\u001b[10GAssessment:\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BInitial\u001b[11Gscore:\u001b[18G5/10.\u001b[24GThe\u001b[28Gplan\u001b[33Gcorrectly\u001b[43Gidentifies\u001b[54G5\u001b[56Greal\u001b[61Ggaps\u001b[66Gand\u001b[70Gpreserves\u001b[80Gstrong\u001b[87Gaccessibility\u001b[101Gand\u001b[105Gresponsive\u001b[116Gspecs\r\u001b[1B \u001b[3Ginhe\u001b[9Gted from the\u001b[22Gexisting behavi\u001b[38Gr des\u001b[44Gription.\u001b[53GA 10/10\u001b[61Gwould specify HOW\u001b[79Ge\u001b[81Gch\u001b[84Ggap\u001b[88Gi\u001b[90G \u001b[92Ge\u001b[94Golved (c\u001b[104Gcrete\u001b[110Gcomponent\r\u001b[2C\u001b[1Bchanges,\u001b[12Gtoken\u001b[18Gvalues,\u001b[26Gbefore/after\u001b[39Glayout),\u001b[48Gnot\u001b[52Gjust\u001b[57Gname\u001b[62Gthe\u001b[66Gproblem.\u001b[75GThe\u001b[79GDESIGN.md\u001b[89Galready\u001b[97Ghas\u001b[101Gall\u001b[105Gcorrect\u001b[113Gvalues;\r\u001b[2C\u001b[1Bth\u001b[6G plan just n\u001b[19Geds to reference\u001b[36Gthem.\r\u001b[2C\u001b[1B\u001b[K\r\u001b[2C\u001b[1BWriting the\u001b[15Ginitial plan \u001b[30Gle\u001b[33Gnow before ask\u001b[50G Step 0D.\u001b[K\r\u001b[2C\u001b[1B\u001b[K\r\u001b[1B●\u001b[3GWrite(/tmp/gstack-paid-shard-jPAIoN/tmp/gstack-e2e-plan-design-ga2LJt/gstack-test-plan-design.md)\r\u001b[2C\u001b[1B⎿\u001b[5G Wrote 44 lines \u001b[22Go\u001b[24G../gstack-e2e-plan-design-ga2LJt/gstack-test-plan-design.md\r\u001b[7C\u001b[1B1\u001b[10G#\u001b[12GDesign\u001b[19GReview\u001b[26GPlan:\u001b[32GSettings\u001b[41GPage\u001b[46GUI\u001b[49GRedesign\r\u001b[2C\u001b[1B \u001b[8G2\u001b[10G\u001b[K\r\u001b[7C\u001b[1B4\r\u001b[2C\u001b[1B 5 This is\u001b[18Ga plan-mode design review of \u001b[48Ghe Settings Page UI Redesign plan. The\u001b[87Gplan updates an existing ac\u001b[K\r\u001b[2C\u001b[1B count-se\u001b[19Gtings form against a\u001b[40Gche\u001b[44Gked-\u001b[49Gn DESIGN.md. Five\u001b[67Gimplementation gaps are\u001b[91Gidentified i\u001b[104G the plan;\u001b[K\r\u001b[2C\u001b[1B lementation begins.\u001b[K\r\u001b[2C\u001b[1B \u001b[7G 6\u001b[K\r\u001b[7C\u001b[1B7\u001b[10G##\u001b[13GPre-Review\u001b[24GAudit\r\u001b[2C\u001b[1B 9 - **DESIGN.md:** Ex\u001b[30Gsts and comprehensive. All design\u001b[64Gdecisions\u001b[74Gcalibrate\u001b[84Gagainst\u001b[92Git.\r\u001b[6C\u001b[1B10\u001b[10G-\u001b[12G**UI\u001b[17Gscope:**\u001b[26GAPP\u001b[30GUI\u001b[33G—\u001b[35Gaccount-settings\u001b[52Gform\u001b[57Gwith\u001b[62GProfile\u001b[70Gand\u001b[74GNotifications\u001b[88Gsections.\r\u001b[1B \u001b[3G … +39 lines\u001b[K\u001b[40;1H\u001b[38;3H\u001b[?25h", + "expected": [ + " 14 - **Initial score:** 5/10 — plan correctly names 5 gaps but does not specify HOW each is resolved.", + " 15", + " 16 ## Step 0: Design Scope Assessment", + " 17", + " 18 **Initial rating: 5/10**", + " 19", + " 20 The plan earns a 5 because:", + " 21 - It correctly identifies 5 real, specific gaps (Visual Hierarchy, Spacing, Color, Typography, Motion)", + " 22 - It preserves strong accessibility and responsive specs from the existing behavior description", + " 23 - DESIGN.md exists and supplies correct values for all 5 gaps", + " 24", + " 25 A 10/10 would:", + " 26 - Specify the exact change for each gap (concrete token, before → after)", + " 27 - Add interaction state table (loading, empty, error, success, partial)", + " 28 - Storyboard the user journey", + " 29 - Call out which DESIGN.md tokens each fix applies ", + " 30 - Resolve all implementation ambiguities before a developer picks it up", + " 31", + " 32 **DESIGN.md status:** Present and detailed. Spacing scale, color system, typography, interaction patterns, and c", + " omponent behavior are all specified. This review calibrates against it.", + " 33", + " 34 **Existing design leverage:**", + " 35 - Button, Field, InlineStatus, ErrorSummary, ConfirmationDialog components reused as-is", + " 36 - Focus ring: 2px solid #1d4ed8, offset 2px (already specified)", + " 37 - 8px base spacing scale already established", + " 38", + " 39 ---", + " 40", + " 41 ## Review Passes", + " 42", + " 43 *(Populated progressively as the review proceeds)*", + " 44", + "╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌", + " Do you want to create gstack-test-plan-design.md?", + " ❯ 1. Yes", + " 2. Yes, and switch to accept edits (auto-approve file edits and common file commands) for this session; Yes, and", + " always allow access to /tmp/gstack-paid-shard-jPAIoN/tmp/gstack-e2e-plan-design-ga2LJt for this session", + " 3. Nohift+tab)", + "", + " Esc to cancel · Tab to amend" + ] +} diff --git a/test/helpers/autoplan-setup-question.ts b/test/helpers/autoplan-setup-question.ts index 0f67b1c0b..b30ad1e23 100644 --- a/test/helpers/autoplan-setup-question.ts +++ b/test/helpers/autoplan-setup-question.ts @@ -22,6 +22,12 @@ export function autoplanRoutingSetupInput(visible: string, seen: Set): s // migration, deletion, or unrelated workflow is not the opposed action. const decline = options.filter(option => { const title = option.title.replace(/\(Recommended\)$/i, ''); + // The action can stand alone or follow a short courtesy ('No thanks'). + // Cursor redraws can damage that courtesy while leaving 'invoke skills + // manually' intact. Match the complete action, not the spelling of No; + // arbitrary preceding instructions and extra trailing actions still fail. + const manual = title.replace(/^[a-z]{0,3}thanks[,—–-]/i, ''); + if (/^(?:manualinvocation|(?:I['’]ll)?invoke(?:skills)?manually)$/i.test(manual)) return true; const prefix = /^(?:Nothanks|Skip)(?:[,—–-])?/i.exec(title); if (!prefix) return false; // 'No thanks' can be followed by the same explicit Skip action. Strip diff --git a/test/helpers/ceo-mode-option.ts b/test/helpers/ceo-mode-option.ts index 86fd68529..64bca3e25 100644 --- a/test/helpers/ceo-mode-option.ts +++ b/test/helpers/ceo-mode-option.ts @@ -1,6 +1,10 @@ /** Match CEO mode labels after the PTY capture strips cursor-spacing escapes. */ import { capturePlanCountQuestion, + createPlanCountPermissionGuard, + parseQuestionPrompt, + parseNumberedOptions, + auqFingerprint, classifyPlanCountFrame, isNumberedOptionListVisible, planCountSubmissionInput, @@ -42,14 +46,37 @@ type ModeNavigationAction = | { kind: 'question'; question: AskUserQuestionFingerprint } | { kind: 'mode'; index: number; question: AskUserQuestionFingerprint }; +// Permission state follows the navigation session without entering its AUQ +// dedup set. A file-tool result can reopen an identical permission prompt. +const permissionStates = new WeakMap, { + file: ReturnType; + other: Set; +}>(); +function ceoPermissionAction(visible: string, seenQuestions: Set): 'grant' | 'handled' | null { + let state = permissionStates.get(seenQuestions); + if (!state) { + state = { file: createPlanCountPermissionGuard(), other: new Set() }; + permissionStates.set(seenQuestions, state); + } + const file = state.file(visible); + if (file !== null) return file; + if (classifyPlanCountFrame(visible) !== 'permission') return null; + const signature = auqFingerprint(parseQuestionPrompt(visible), parseNumberedOptions(visible)); + if (state.other.has(signature)) return 'handled'; + state.other.add(signature); + return 'grant'; +} + /** Handle native controls before deduping actual navigation questions. */ export function nextCeoModeNavigation( visible: string, targetMode: CeoMode, seenQuestions: Set, ): ModeNavigationAction { + const permission = ceoPermissionAction(visible, seenQuestions); + if (permission !== null) return permission === 'grant' + ? { kind: 'permission', input: '1\r' } : { kind: 'wait' }; 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' }; @@ -150,9 +177,8 @@ export function nextCeoPostureContinuation( seenQuestions: Set, alreadyContinued: boolean, ): 'permission' | 'question' | null { - if (classifyPlanCountFrame(visible) === 'permission') { - return capturePlanCountQuestion(visible, seenQuestions, 0, true) ? 'permission' : null; - } + const permission = ceoPermissionAction(visible, seenQuestions); + if (permission !== null) return permission === 'grant' ? 'permission' : null; if (alreadyContinued || !nativeCeoModeAnswer(transcript, targetMode, selectionStartedAt)) return null; const action = nextCeoModeNavigation(visible, targetMode, seenQuestions); return action.kind === 'question' ? 'question' : null; diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts index 8ef2fed60..ad96eb051 100644 --- a/test/helpers/claude-pty-runner.ts +++ b/test/helpers/claude-pty-runner.ts @@ -32,6 +32,7 @@ import { createPlanCountFixture } from './plan-count-fixture'; import { createPlanCountSnapshotWriter } from './plan-count-artifacts'; import { readPlanCountTranscript, unresolvedPlanQuestionCalls, type NativePlanQuestionCall, type PlanCountTranscript } from './plan-count-transcript'; import { trustDialogInput } from './pty-trust-dialog'; +import { createPtyScreen } from './pty-screen'; /** Strip ANSI escapes for pattern-matching against visible text. */ export function stripAnsi(s: string): string { @@ -95,6 +96,8 @@ export interface ClaudePtyOptions { /** Terminal size. Default 120x40. Plan-mode UI lays out cleanly at this size. */ cols?: number; rows?: number; + /** Opt in only when input targeting needs the actual VT viewport. */ + observeScreen?: boolean; /** Working directory. Default: process.cwd(). The repo cwd has the gstack * skill registry and trusted-folder cookie, so most tests want this. */ cwd?: string; @@ -113,6 +116,8 @@ export interface ClaudePtySession { rawOutput(): string; /** Visible (ANSI-stripped) output for the entire session. For pattern matching. */ visibleText(): string; + /** Flush the opted-in terminal parser and return only its current viewport. */ + currentScreen(): Promise; /** * Mark the current buffer position. Subsequent waitForAny / visibleSince * calls only look at output AFTER this mark. Use to scope assertions to @@ -1226,6 +1231,82 @@ export function auqFingerprint( return (Bun as any).hash(normalized + '||' + sig).toString(16); } +/** Permission and question capture inspect the same cursor-anchored window. */ +function planCountPermissionMenu(visible: string): { + normalized: string; cursorAt: number; prompt: string; menu: string; +} | null { + const normalized = stripPtyResidue(visible).replace(/\r+\n?/g, '\n'); + const start = Math.max(0, normalized.length - 4096); + const cursor = [...normalized.slice(start).matchAll(/❯\s*1\./g)].at(-1); + if (!cursor) return null; + const cursorAt = start + cursor.index; + const before = normalized.slice(start, cursorAt); + const header = [...before.matchAll(/(?:^|\n)[\t │┃]*[☐□]([^\n│]*)/g)].at(-1); + const menu = normalized.slice(cursorAt); + // The native question panel has its own header and navigation footer. + // Its actual finding may discuss file creation or permission policy. + if (header && !/❯\s*[1-9]\./.test(before.slice(header.index)) && + /Enter\s*to\s*select\s*·\s*↑\/↓\s*to\s*navigate\s*·\s*Esc\s*to\s*cancel/i.test(menu)) return null; + // Anchor redraw identity to its question line; prior tool output and + // the preceding menu's footer must not change a permission signature. + const prompt = [...before.matchAll(/^[^\n]*\?[^\n]*$/gm)].at(-1)?.[0].trim() + ?? parseQuestionPrompt(normalized); + if (!prompt || !isPermissionDialogVisible(prompt + '\n' + menu)) return null; + return { normalized, cursorAt, prompt, menu }; +} + +/** A scrolled question must retain its entire visible suffix, choices and footer. */ +function matchesClippedNativeQuestion(visible: string, call: NativePlanQuestionCall): boolean { + const cursor = [...visible.matchAll(/❯\s*1\./g)].at(-1); + if (!cursor) return false; + const before = visible.slice(0, cursor.index); + // This is the top of the actual viewport, not a selected historical + // snippet. A header, preceding menu or blank top is not clipped identity. + if (!before.split('\n')[0]?.trim() || /[☐□❯]/.test(before)) return false; + const suffix = before.replace(/^[ \t]*[│┃] ?|[│┃][ \t]*$/gm, '').trim(); + const exact = (value: string) => value.replace(/\s+/g, ''); + const question = call.questions[0]!; + const native = exact(question.question); + const displayed = exact(suffix); + // Require substantial positive question text, including all visible + // pre-menu lines. Shared option labels or a generic short tail cannot + // borrow an unrelated pending call's routing policy. + if (suffix.split('\n').filter(line => line.trim()).length < 2 || displayed.length < 160 || + displayed.length > native.length || !native.endsWith(displayed)) return false; + const menu = visible.slice(cursor.index); + const footer = /Enter\s*to\s*select\s*·\s*↑\/↓\s*to\s*navigate\s*·\s*Esc\s*to\s*cancel/i.exec(menu); + if (!footer || !/^[\s│┃─━└┘]*$/.test(menu.slice(footer.index + footer[0].length))) return false; + const options = parseNumberedOptions(visible); + return options.length === question.options.length && options.every((option, index) => + option.index === index + 1 && exact(option.label) === exact(question.options[index]!.label)); +} + +/** Match native question identity before permission text can choose an answer. */ +export function matchesNativePlanQuestion(visible: string, call: NativePlanQuestionCall): boolean { + if (call.failed || call.questions.length !== 1) return false; + const normalized = stripPtyResidue(visible).replace(/\r+\n?/g, '\n'); + const tail = normalized.slice(-4096); + const cursor = [...tail.matchAll(/❯\s*1\./g)].at(-1); + if (!cursor) return false; + const before = tail.slice(0, cursor.index); + const header = [...before.matchAll(/(?:^|\n)[\t │┃]*[☐□]([^\n│]*)/g)].at(-1); + const compact = (value: string) => value.replace(/\s+/g, '').toLowerCase(); + const question = call.questions[0]!; + if (!header) return matchesClippedNativeQuestion(normalized, call); + if (compact(header[1]!) !== compact(question.header) || /❯\s*[1-9]\./.test(before.slice(header.index))) return false; + const identity = question.question.match(/]+>/i)?.[0] ?? question.question; + if (!compact(before.slice(header.index)).includes(compact(identity))) return false; + // Preserve the captured damaged-option path when the native panel's + // complete footer is intact. With a damaged footer, require the full + // question and every offered label instead of guessing from keywords. + if (/Enter\s*to\s*select\s*·\s*↑\/↓\s*to\s*navigate\s*·\s*Esc\s*to\s*cancel/i.test(tail.slice(cursor.index))) return true; + const displayed = before.slice(header.index).replace(/^[\t ─━┄┅┈┉┌┐└┘├┤┬┴┼│┃☐□■]+/gm, ''); + const options = parseNumberedOptions(visible); + return compact(displayed) === compact(`${question.header} ${question.question}`) && + options.length === question.options.length && options.every((option, index) => + option.index === index + 1 && compact(option.label) === compact(question.options[index]!.label)); +} + /** Capture each distinct question once, including questions sharing a menu. */ export function capturePlanCountQuestion( visible: string, @@ -1241,36 +1322,25 @@ export function capturePlanCountQuestion( // parser's still-visible historical question and queue spurious input. if (!cursor) return null; - if (pending && !pending.answered && !pending.failed && pending.questions.length === 1) { - const beforeCursor = tail.slice(0, cursor.index); - const header = [...beforeCursor.matchAll(/(?:^|\n)[\t │┃]*[☐□]([^\n│]*)/g)].at(-1); - const compact = (value: string) => value.replace(/\s+/g, '').toLowerCase(); - const question = pending.questions[0]!; - const questionId = question.question.match(/]+>/i)?.[0]; - const identity = questionId ?? question.question; - const footer = /Enter\s*to\s*select\s*·\s*↑\/↓\s*to\s*navigate\s*·\s*Esc\s*to\s*cancel/i; - // A captured B menu lost the dot in option 2. The native call supplies - // its actual choices, but only after the matching boxed question, - // cursor, and complete navigation footer establish the active UI. - if (header && compact(header[1]!) === compact(question.header) && - !/❯\s*[1-9]\./.test(beforeCursor.slice(header.index)) && - compact(beforeCursor.slice(header.index)).includes(compact(identity)) && - footer.test(tail.slice(cursor.index))) { - const fp = nativePlanCallFingerprint(pending, observedAtMs, preReview); - const renderedOptions = parseNumberedOptions(visible); - const renderedPrompt = parseQuestionPrompt(visible); - const renderedSignature = renderedOptions.length >= 2 && renderedPrompt - ? auqFingerprint(renderedPrompt, renderedOptions) : null; - const alreadyHandled = seen.has(fp.signature) || Boolean(renderedSignature && seen.has(renderedSignature)); - // Once answered, the same menu may linger while the native call is - // no longer pending, or the native record may arrive after the UI - // answer. Bind both identities before testing either direction. - if (renderedSignature) seen.add(renderedSignature); - seen.add(fp.signature); - if (alreadyHandled) return null; - return fp; - } + if (pending && !pending.answered && !pending.failed && matchesNativePlanQuestion(visible, pending)) { + const fp = nativePlanCallFingerprint(pending, observedAtMs, preReview); + const renderedOptions = parseNumberedOptions(visible); + const renderedPrompt = parseQuestionPrompt(visible); + const renderedSignature = renderedOptions.length >= 2 && renderedPrompt + ? auqFingerprint(renderedPrompt, renderedOptions) : null; + const alreadyHandled = seen.has(fp.signature) || Boolean(renderedSignature && seen.has(renderedSignature)); + // Once answered, the same menu may linger while the native call is + // no longer pending, or the native record may arrive after the UI + // answer. Bind both identities before testing either direction. + if (renderedSignature) seen.add(renderedSignature); + seen.add(fp.signature); + if (alreadyHandled) return null; + return fp; } + // A permission cursor can outlive the short frame-classifier window. + // Never turn it into an ordinary prompt answer, even when its question + // was damaged by a redraw or its original grant was not observed. + if (planCountPermissionMenu(visible)) return null; const options = parseNumberedOptions(visible); if (options.length < 2) return null; const promptSnippet = parseQuestionPrompt(visible); @@ -1287,23 +1357,28 @@ export function capturePlanCountQuestion( * Its text is not a new answer request. The same file can ask again after * a successful native Write/Edit result, so content-only dedup is too broad. */ -export function createPlanCountPermissionGuard(): (visible: string) => 'grant' | 'handled' | null { +export function createPlanCountPermissionGuard(): (visible: string, completionHistory?: string) => 'grant' | 'handled' | null { let granted: { signature: string; completedAt: number } | undefined; - return (visible) => { - const normalized = stripPtyResidue(visible).replace(/\r+\n?/g, '\n'); - const start = Math.max(0, normalized.length - 4096); - const cursor = [...normalized.slice(start).matchAll(/❯\s*1\./g)].at(-1); - if (!cursor) return null; - const cursorAt = start + cursor.index; - const question = [...normalized.slice(start, cursorAt).matchAll(/^[\t │]*(Do\s*you\s*want\s*to\s*(?:create|overwrite|edit|make\s*this\s*edit\s*to)[^\n?]*\?)/gim)].at(-1)?.[1]; - if (!question || !isPermissionDialogVisible(question + '\n' + normalized.slice(cursorAt))) return null; - const signature = question.replace(/\s+/g, ''); + return (visible, completionHistory = visible) => { + // Only the current viewport can establish an actionable permission. + // Historical file results release a later identical grant, never a menu. + const candidate = planCountPermissionMenu(visible); + if (!candidate) return null; + const { normalized, cursorAt, prompt, menu } = candidate; + // File permission identity comes from the action or native approval + // choices, not a brittle exact rendering of "Do you want to ...". + // Other permission kinds retain the existing caller policy. + if (!/(?:create|overwrite|edit)/i.test(prompt.replace(/\s+/g, '')) && + !/(?:acceptedits|allowalledits|auto-approvefileedits)/i.test(menu.replace(/\s+/g, ''))) return null; + const signature = prompt.replace(/\s+/g, ''); // Match actual native file-tool results, not proposed diff rows, tool // headers, or tips. A diff repaint after the menu is still pending. - const completed = [...normalized.matchAll(/^[\t ]*⎿[\t \u00a0]*(?:Wrote\s*\d+\s*lines?|Added\s*\d+\s*lines?|Removed\s*\d+\s*lines?|Updated\b|Edited\b)/gm)].at(-1); - const completedAt = completed?.index ?? -1; - if (completedAt > cursorAt) return 'handled'; + const completionPattern = /^[\t ]*⎿[\t \u00a0]*(?:Wrote\s*\d+\s*lines?|Added\s*\d+\s*lines?|Removed\s*\d+\s*lines?|Updated\b|Edited\b)/gm; + const visibleCompletedAt = [...normalized.matchAll(completionPattern)].at(-1)?.index ?? -1; + if (visibleCompletedAt > cursorAt) return 'handled'; + const history = stripPtyResidue(completionHistory).replace(/\r+\n?/g, '\n'); + const completedAt = [...history.matchAll(completionPattern)].at(-1)?.index ?? -1; if (granted?.signature === signature && completedAt <= granted.completedAt) return 'handled'; granted = { signature, completedAt }; return 'grant'; @@ -1348,6 +1423,7 @@ export function classifyPlanCountFrame( const permissionTail = visible.slice(Math.max(visible.length - TAIL_SCAN_BYTES, activeQuestionStart, 0)); if (isNumberedOptionListVisible(visible) && isPermissionDialogVisible(permissionTail) && + planCountPermissionMenu(visible) && (!report || report.index <= menuCursor)) { return 'permission'; } @@ -1462,10 +1538,15 @@ export function hasNativePlanCompletion( lastLine !== `Plan written to: ${expectedPlanPath}`) return false; const prose = text.replace(/```[\s\S]*?```|`[^`]*`/g, ''); if (/\?|\b(?:awaiting|waiting for|please (?:choose|answer|confirm))\b/i.test(prose)) return false; + return hasCompletePlanReport(expectedPlanPath, Math.max(startedAt, ...answerTimes), finishedAt); +} + +function hasCompletePlanReport(expectedPlanPath: string, minimumMtime: number, maximumMtime: number): boolean { + if (!path.isAbsolute(expectedPlanPath)) return false; try { const stat = fs.lstatSync(expectedPlanPath); if (!stat.isFile() || stat.size > 2 * 1024 * 1024 || - stat.mtimeMs < Math.max(startedAt, ...answerTimes) || stat.mtimeMs > finishedAt) return false; + stat.mtimeMs < minimumMtime || stat.mtimeMs > maximumMtime) return false; // A template/example inside Markdown code is not the completed report. // Fences open with 3+ identical backticks or tildes, indented at most // three spaces. A close uses the same marker, at least the opening @@ -1491,11 +1572,101 @@ export function hasNativePlanCompletion( if (!assertReviewReportAtBottom(content).ok) return false; const report = content.slice(content.indexOf('## GSTACK REVIEW REPORT')); // Reject a provisional heading; retain both clean and issues-open reports. - return /^\|[^\n]*Review[^\n]*\|/m.test(report) && - /^(?:\*\*)?VERDICT:/m.test(report) && /(?:NO )?UNRESOLVED DECISIONS/.test(report); + const rows = report.split('\n'); + const table = rows.findIndex(line => /^\|[^\n]*Review[^\n]*\|/.test(line)); + const completeTable = table >= 0 && /^\|[ :|-]+\|$/.test(rows[table + 1] ?? '') && + /^\|.*[a-zA-Z].*\|$/.test(rows[table + 2] ?? ''); + const decisions = rows.findIndex(line => /^\*\*UNRESOLVED DECISIONS:\*\*$/.test(line)); + const trailing = rows.slice(decisions + 1).filter(line => line.trim()); + const closed = rows.filter(line => line.trim()).at(-1) === 'NO UNRESOLVED DECISIONS' || + (decisions >= 0 && trailing.length > 0 && trailing.every(line => /^[-*] \S|^\+ \d+ unresolved from prior reviews$/.test(line))); + return completeTable && /^(?:[-*] )?(?:\*\*)?VERDICT:(?:\*\*)?[ \t]*[A-Za-z]/m.test(report) && closed; } catch { return false; } } +/** Native report completion is independent of the terminal's streamed headings. */ +export function hasNativePlanTerminal( + transcript: PlanCountTranscript, + expectedPlanPath: string, + startedAt: number, + frame: 'completion_summary' | 'plan_ready', + administrativeCalls: ReadonlySet = new Set(), +): boolean { + if (transcript.status !== 'ready' || !transcript.calls.length || + transcript.calls.some(c => (!c.answered && !c.failed) || (c.answered && c.failed)) || + unresolvedPlanQuestionCalls(transcript.calls).length) return false; + const sessions = new Set([...transcript.calls.map(c => c.sessionId), + ...transcript.assistantMessages.map(m => m.sessionId), + ...(transcript.planReadyRequests ?? []).map(r => r.sessionId)]); + if (sessions.size !== 1) return false; + const answered = transcript.calls.filter(c => c.answered); + if (!answered.length) return false; + const answerTimes = answered.map(c => Date.parse(c.answeredAt ?? '')); + if (!answerTimes.every(Number.isFinite)) return false; + const latestAnswer = Math.max(...answerTimes); + const latestReady = [...(transcript.planReadyRequests ?? [])] + .sort((a, b) => Date.parse(a.timestamp) - Date.parse(b.timestamp)).at(-1); + if (latestReady?.failed) return false; + const modifyingAnswers = answered.filter(c => + !administrativeCalls.has(`${c.sessionId}:${c.toolUseId}`) && !isCompletedDxHandoff(c)) + .map(c => Date.parse(c.answeredAt!)); + if (!modifyingAnswers.length || !hasCompletePlanReport(expectedPlanPath, + Math.max(startedAt, ...modifyingAnswers), Date.now())) return false; + + if (frame === 'plan_ready') { + // A real pending ExitPlanMode call is the approval gate. Do not answer + // it or mistake its failed tool result for a completed review. + return Boolean(latestReady && Date.parse(latestReady.timestamp) > latestAnswer && + Date.parse(latestReady.timestamp) <= Date.now()); + } + const final = [...transcript.assistantMessages] + .sort((a, b) => Date.parse(a.timestamp) - Date.parse(b.timestamp)).at(-1); + if (!final || !Number.isFinite(Date.parse(final.timestamp)) || Date.parse(final.timestamp) <= latestAnswer) return false; + const text = final.text.trim(); + // Only finished, fixture-scoped native prose supplies this evidence. A + // rendered heading, quoted example, proposed write, or pending question + // cannot substitute for an actual completion message. + if (/^(?:>|`{3,}|~{3,}|(?:example|sample|template|quoted)\b)/i.test(text) || + /\b(?:cannot|can't|unable to)\s+(?:complete|finish)|\b(?:awaiting|waiting for|please (?:choose|answer|confirm))\b/i.test(text) || + text.endsWith('?')) return false; + if (/^(?:CEO|Eng(?:ineering)?|Design|DX) review complete[.!](?:\s|$)/i.test(text)) return true; + // A full native summary can precede the final report write; the independent + // fresh-file check above prevents the observed heading-before-Write race. + const lines: string[] = []; + let fence: { marker: string; length: number } | undefined; + for (const line of text.split(/\r?\n/)) { + const delimiter = /^ {0,3}(`{3,}|~{3,})(.*)$/.exec(line); + if (delimiter) { + if (!fence) fence = { marker: delimiter[1]![0]!, length: delimiter[1]!.length }; + else if (delimiter[1]![0] === fence.marker && delimiter[1]!.length >= fence.length && !delimiter[2]!.trim()) fence = undefined; + continue; + } + if (!fence && !/^(?: {4}|\t| {0,3}>)/.test(line)) lines.push(line); + } + if (fence) return false; + const summary = lines.findIndex(line => /^(?:#{1,6}\s*)?(?:\*\*)?Completion\s+summary(?:\*\*)?\s*:?[ \t]*$/i.test(line.trim())); + const preceding = lines.slice(0, summary).filter(line => line.trim()).at(-1) ?? ''; + if (summary < 0 || /\b(?:example|sample|template|quote|emit|print)\b.*[:：]\s*$/i.test(preceding)) return false; + const rows = lines.slice(summary + 1).filter(line => /^\s*(?:[-*] |\|)/.test(line)); + return rows.length > 0 && rows.some(line => /\b(?:review|issues?|findings?|gaps?|resolved|scope)\b/i.test(line)); +} + +/** G's post-report DX next-step menu changes orchestration, not plan decisions. */ +function isCompletedDxHandoff(call: NativePlanQuestionCall): boolean { + if (!call.answered || call.failed || call.questions.length !== 1 || + !Array.isArray(call.unansweredQuestionIndices) || call.unansweredQuestionIndices.length) return false; + const q = call.questions[0]!; + if (q.multiSelect || !/^next\s+steps?$/i.test(q.header.trim()) || + !/^DX review (?:done|complete)[.!]/i.test(q.question) || + !//.test(q.question)) return false; + const labels = q.options.map(o => o.label.trim().replace(/\s*\(recommended\)\s*$/i, '')); + const allowed = (label: string) => /^(?:Run \/plan-eng-review|Ready to implement|Skip, handle manually)$/.test(label); + return labels.every(allowed) && labels.includes('Run /plan-eng-review') && labels.includes('Skip, handle manually') && + q.options.some(o => o.label === call.answers?.[q.question]); +} + + + /** * Test helper: if `obs.planFile` was set, read it and assert * `## GSTACK REVIEW REPORT` is the last `## ` section. Throws on @@ -1573,28 +1744,42 @@ export const ceoFirstReviewAUQ: Step0BoundaryPredicate = (fp) => /\bfinding\b|\bmissing\b|\bambiguous\b|\bundefined\b|doesn['’]t\s+(?:define|specify|cover|mention)/i.test(title); }) ?? false; -/** Native setup identity comes from answered question IDs/actions, not display wording. */ +/** Native setup needs an answered scope decision, not a particular model-chosen qid. */ export const engSetupAUQ: Step0BoundaryPredicate = (fp) => { const call = fp.nativeCall; if (!call?.answered || call.failed) return false; const answered = call.questions.filter(q => Boolean(call.answers?.[q.question])); // A mixed packet containing an answered finding is not wholly setup. return answered.length > 0 && answered.every(q => { + if (!q.options.some(option => option.label === call.answers?.[q.question])) return false; + const actions = q.options.map(option => option.label.trim().replace(/^[A-Z][.)]\s+/i, '')); const id = //i.exec(q.question)?.[1]?.toLowerCase(); + const body = q.question.replace(/]*>/gi, '').trim() + .replace(/^D\s*\d+\s*[—–:-]\s*/i, ''); + // A finding about one component or a TODO remains substantive even if + // it cites the plan's size or offers to reduce that individual issue. + if (/\b(?:issue|finding|gap|TODO)\b/i.test(q.header) || + /^(?:(?:architecture|code quality|test|performance|security)\s+)?(?:issue|finding|gap|TODO)\b/i.test(body)) return false; + const learningPremise = /\bcross[- ]project\s+(?:learnings|lessons)\b|\b(?:learnings|lessons)\b[^.!?]{0,100}\b(?:other|all)\s+(?:projects|repositories)\b/i.test(body); const crossProject = id === 'cross-project-learnings' || id === 'preamble-cross-project-learnings' || - (!id && /^cross[- ]project$/i.test(q.header.trim())); + (!id && /^cross[- ]project$/i.test(q.header.trim())) || learningPremise; if (crossProject) { - return q.options.some((enabled, i) => /^enable\s+cross[- ]project\b/i.test(enabled.label.trim()) && - q.options.some((scoped, j) => j !== i && /\bproject[- ]scoped\b/i.test(scoped.label))); + return actions.some((enabled, i) => + (/^enable\s+cross[- ]project\b/i.test(enabled) || + (learningPremise && /^enable(?:\s*\(recommended\))?$/i.test(enabled))) && + actions.some((scoped, j) => j !== i && /\bproject[- ]scoped\b/i.test(scoped))); } - const scopeComplexity = id === 'plan-eng-scope-complexity' || id === 'plan-eng-review-scope-reduce' || - // Older native captures omitted qids. Require whole-plan class/file scope - // and its opposed scope actions, never an individual implementation issue. - (!id && /\bplan\b/i.test(q.question) && /\bclasses\b/i.test(q.question) && /\bfiles\b/i.test(q.question)); + // The skill's complexity gate concerns the whole plan and a concrete + // file/class/service count. Its qid and header can vary across native runs. + const scopeHeading = /^scope(?:\s+(?:challenge|reduction|complexity))?\s*:/i.test(body); + const wholePlanStatement = /(?:^|\n)(?:ELI10:\s*)?(?:this|the|whole|entire)\s+plan\s+(?:touches|spans|covers|changes|introduces|involves)\b/i.test(body); + const wholePlanScope = (/^(?:(?:scope(?:\s+(?:challenge|reduction|complexity))?|complexity\s+check)\s*:\s*)?(?:this|the|whole|entire)\s+plan\s+(?:touches|spans|covers|changes|introduces|involves)\b/i.test(body) || + (scopeHeading && wholePlanStatement)) && /\b\d+\s+(?:new\s+)?(?:files|classes|services)\b/i.test(body); + const scopeComplexity = id === 'plan-eng-scope-complexity' || id === 'plan-eng-review-scope-reduce' || wholePlanScope; return scopeComplexity && - q.options.some((proceed, i) => - (/^proceed\s+as[- ]is\b/i.test(proceed.label.trim()) || /^accept\b.*\bdesign\b.*\bfocus\b.*\bquality\b/i.test(proceed.label.trim())) && - q.options.some((reduce, j) => j !== i && /^(?:reduce\b|flag\s+scope\s+reduction\b)/i.test(reduce.label.trim()))); + actions.some((proceed, i) => + (/^proceed\s+as[- ]is\b/i.test(proceed) || /^accept\b.*\bdesign\b.*\bfocus\b.*\bquality\b/i.test(proceed)) && + actions.some((reduce, j) => j !== i && /^(?:reduce\b|flag\s+scope\s+reduction\b)/i.test(reduce))); }); }; @@ -1711,29 +1896,40 @@ export async function launchClaudePty( } } + // Construction must succeed before any CLI can be spawned. + const screen = opts.observeScreen ? await createPtyScreen(cols, rows) : undefined; + let screenClosing: Promise | undefined; + let screenFailure: unknown; + const disposeScreen = () => screenClosing ??= (screen?.dispose() ?? Promise.resolve()).catch(error => { screenFailure = error; }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any - const proc = (Bun as any).spawn([claudePath, ...args], { + let proc: any; + try { proc = (Bun as any).spawn([claudePath, ...args], { terminal: { cols, rows, data(_t: unknown, chunk: Buffer) { - buffer += chunk.toString('utf-8'); + const text = chunk.toString('utf-8'); + buffer += text; + if (screen && !screenClosing) screen.write(text); }, }, cwd, env: childEnv, - }); + }); } catch (error) { await disposeScreen(); throw error; } // Track exit so waitForAny can fail fast if claude crashes. let exitedPromise: Promise = Promise.resolve(); if (proc.exited && typeof proc.exited.then === 'function') { exitedPromise = proc.exited - .then((code: number | null) => { + .then(async (code: number | null) => { exitCodeCaptured = code; exited = true; + await disposeScreen(); }) - .catch(() => { + .catch(async () => { exited = true; + await disposeScreen(); }); } @@ -1853,7 +2049,7 @@ export async function launchClaudePty( clearTimeout(trustWatcherStop); clearInterval(trustWatcher); for (const timer of trustInputTimers) clearTimeout(timer); - if (exited) return; + if (exited) { await disposeScreen(); return; } try { proc.kill?.('SIGINT'); } catch { @@ -1869,6 +2065,7 @@ export async function launchClaudePty( } await Promise.race([exitedPromise, Bun.sleep(1000)]); } + await disposeScreen(); } return { @@ -1876,6 +2073,12 @@ export async function launchClaudePty( sendKey, rawOutput: () => buffer, visibleText: () => stripAnsi(buffer), + currentScreen: async () => { + if (!screen) throw new Error('PTY screen observation was not enabled for this session.'); + if (screenClosing) await screenClosing; + if (screenFailure) throw new Error('PTY screen observation failed.', { cause: screenFailure }); + return screen.read(); + }, mark, visibleSince, waitForAny, @@ -2389,7 +2592,7 @@ export async function runPlanSkillCounting(opts: { * Native-bound selection uses activeCapture, whose metadata is present only * when capturePlanCountQuestion matched the currently visible native question. */ pickAUQ?: (fp: AskUserQuestionFingerprint, activeCapture: AskUserQuestionFingerprint) => number | null; - /** Opt-in native completion fallback, validated against this caller-owned report path. */ + /** Require native completion plus this caller-owned final report before accepting a soft terminal. */ expectedPlanPath?: string; /** Additional versioned files available in the isolated fixture before the skill starts. */ fixtureFiles?: Record; @@ -2433,6 +2636,7 @@ export async function runPlanSkillCounting(opts: { env: { ...opts.env, ...fixture.env }, model: opts.model, seedSkills: true, + observeScreen: true, }); } catch (error) { fixture.cleanup(); @@ -2443,6 +2647,7 @@ export async function runPlanSkillCounting(opts: { const seen = new Set(); const countedCalls = new Set(); const filePermission = createPlanCountPermissionGuard(); + let lastMatchedNativeQuestion: NativePlanQuestionCall | undefined; let transcript: PlanCountTranscript = { status: 'missing', calls: [], assistantMessages: [] }; let boundaryFired = false; let step0Count = 0; @@ -2451,9 +2656,10 @@ export async function runPlanSkillCounting(opts: { let isFirstAUQ = true; const saveSnapshot = createPlanCountSnapshotWriter(); let lastCheckpointAt = Date.now(); + let viewport = ''; const capture = (observation: object) => saveSnapshot({ - skillName: opts.skillName, observation, raw: session.rawOutput(), visible: session.visibleText(), + skillName: opts.skillName, observation, raw: session.rawOutput(), visible: session.visibleText(), viewport, cwd: fixture.cwd, claudeConfigDir: session.hermeticConfigDir, }); @@ -2488,13 +2694,13 @@ export async function runPlanSkillCounting(opts: { try { await Bun.sleep(8000); // boot grace + auto-trust handler window - const since = session.mark(); + session.mark(); session.send(`${opts.slashCommand}\r`); const budgetStart = Date.now(); while (Date.now() - budgetStart < timeoutMs) { await Bun.sleep(2000); - const visible = session.visibleSince(since); + const visible = viewport = await session.currentScreen(); transcript = session.hermeticConfigDir ? readPlanCountTranscript(session.hermeticConfigDir, fixture.cwd) : { status: 'error', calls: [], assistantMessages: [], error: 'Claude count session has no isolated transcript directory' }; @@ -2552,8 +2758,33 @@ export async function runPlanSkillCounting(opts: { ); } - const frame = classifyPlanCountFrame(visible); - const permission = filePermission(visible); + const pending = transcript.calls.find(c => !c.answered && !c.failed); + const newlyMatched = pending && matchesNativePlanQuestion(visible, pending); + if (newlyMatched) lastMatchedNativeQuestion = pending; + const renderedFrame = classifyPlanCountFrame(visible); + const administrative = new Set(fingerprints.filter(fp => fp.administrative).map(fp => fp.signature)); + // A long completed summary may scroll its heading off the viewport. + // With no active input UI, retain the existing native/report validator; + // neither display text nor a missing heading supplies completion evidence. + const nativeCompletion = opts.expectedPlanPath && hasNativePlanCompletion(transcript, opts.expectedPlanPath, startedAt); + const nativeSummary = !nativeCompletion && renderedFrame === null && opts.expectedPlanPath && + !isNumberedOptionListVisible(visible) && !isPermissionDialogVisible(visible) && !isProseAUQVisible(visible) && + hasNativePlanTerminal(transcript, opts.expectedPlanPath, startedAt, 'completion_summary', administrative); + const terminalFrame = nativeSummary ? 'completion_summary' : renderedFrame; + const isTerminalHint = terminalFrame === 'completion_summary' || terminalFrame === 'plan_ready'; + const verifiedTerminal = nativeSummary || (opts.expectedPlanPath && isTerminalHint && + hasNativePlanTerminal(transcript, opts.expectedPlanPath, startedAt, terminalFrame, administrative)); + // A streamed heading cannot dismiss the last bound native question. + // Only an accepted terminal may supersede its answered redraw; otherwise + // permission wording inside that question could queue a stray answer. + const acceptedTerminal = isTerminalHint && (!opts.expectedPlanPath || verifiedTerminal); + const nativeQuestionVisible = newlyMatched || (!acceptedTerminal && + lastMatchedNativeQuestion && matchesNativePlanQuestion(visible, lastMatchedNativeQuestion)); + const terminalHint = nativeQuestionVisible ? null : terminalFrame; + let frame = terminalHint; + // Clear unverified hints before routing active permissions and Submit. + if (opts.expectedPlanPath && isTerminalHint && !verifiedTerminal) frame = null; + const permission = nativeQuestionVisible ? null : filePermission(visible, session.visibleText()); if (frame === 'permission' || (permission === 'grant' && frame === null)) { if (permission !== 'handled') session.send(`${defaultPick}\r`); await Bun.sleep(1500); @@ -2585,8 +2816,11 @@ 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 (opts.expectedPlanPath && verifiedTerminal && (frame === 'completion_summary' || frame === 'plan_ready')) { + return snapshot(frame, `native ${frame} and final report verified (step0=${step0Count}, review=${reviewCount})`, visible); + } + + // Legacy callers without a report contract retain their terminal policy. if (frame === 'completion_summary') { if (transcript.status !== 'ready' || transcript.calls.some(c => !c.answered && !c.failed) || unresolvedPlanQuestionCalls(transcript.calls).length) { @@ -2610,17 +2844,21 @@ export async function runPlanSkillCounting(opts: { ); } - if (opts.expectedPlanPath && hasNativePlanCompletion(transcript, opts.expectedPlanPath, startedAt)) { + if (opts.expectedPlanPath && !transcript.planReadyRequests?.length && !isNumberedOptionListVisible(visible) && + hasNativePlanCompletion(transcript, opts.expectedPlanPath, startedAt)) { return snapshot('completion_summary', `native review completion and final report verified (step0=${step0Count}, review=${reviewCount})`, visible); } + // Never submit a plan-approval menu while its final report/native gate + // is unverified: choosing option 1 would enter implementation. + if (opts.expectedPlanPath && terminalHint === 'plan_ready') continue; + // A dismissed or repainted permission is never a native question. if (permission === 'handled') continue; // Dedupe the complete question, not just its answer labels: separate // findings often reuse the same Add to plan / Defer / Skip menu. - const pending = transcript.calls.find(c => !c.answered && !c.failed); const fp = capturePlanCountQuestion(visible, seen, Date.now() - startedAt, !boundaryFired, pending); if (!fp) continue; // Press to advance — first AUQ may use the override pick. @@ -2645,7 +2883,7 @@ export async function runPlanSkillCounting(opts: { return snapshot( 'timeout', `no terminal outcome within ${timeoutMs}ms (step0=${step0Count}, review=${reviewCount})`, - session.visibleSince(since), + viewport = await session.currentScreen(), ); } finally { try { diff --git a/test/helpers/claude-pty-runner.unit.test.ts b/test/helpers/claude-pty-runner.unit.test.ts index a58cff738..9a383b1b0 100644 --- a/test/helpers/claude-pty-runner.unit.test.ts +++ b/test/helpers/claude-pty-runner.unit.test.ts @@ -41,6 +41,7 @@ import { COMPLETION_SUMMARY_RE, classifyPlanCountFrame, capturePlanCountQuestion, + matchesNativePlanQuestion, createPlanCountPermissionGuard, planCountPrerequisitePick, planCountSubmissionInput, @@ -1751,7 +1752,7 @@ describe('Step0BoundaryPredicate per-skill', () => { for (const alter of [ (call: any) => { call.answered = false; }, (call: any) => { call.failed = true; }, - (call: any) => { call.questions[0].header = 'Architecture'; }, + (call: any) => { call.questions[0].header = 'Architecture issue'; call.questions[0].question = 'D1 — Architecture issue: should the cross-project feature use shared storage?'; }, (call: any) => { call.questions[0].options = [{ label: 'Enable cross-project search' }, { label: 'Disable all search' }]; }, (call: any) => { call.questions[0].options = [{ label: 'Enable cross-project search with project-scoped storage' }, { label: 'Discuss later' }]; }, (call: any) => { call.questions[0].question = 'How should concurrent AuthCache writes across projects be serialized?'; @@ -1936,7 +1937,7 @@ describe('Step0BoundaryPredicate per-skill', () => { unansweredScope.nativeCall!.answers = { 'Separate answered setup question': 'Continue' }; expect(engStep0Boundary(unansweredScope)).toBe(false); for (const alter of [ - (q: any) => { q.question = q.question.replace('plan-eng-scope-complexity', 'plan-eng-cache-complexity'); }, + (q: any) => { q.question = 'D1 — Architecture issue: the plan touches 12 files and introduces 4 classes, but AuthCache has a race. Reduce cache scope? '; }, (q: any) => { q.options = [{ label: 'Change cache size' }, { label: 'Keep cache size' }]; }, ]) { const unrelated = structuredClone(original); @@ -2388,3 +2389,297 @@ describe('native Eng first packet and registry identities', () => { expect(engFirstReviewAUQ(nativePlanCallFingerprint(callWith([q], { [q.question]: q.options[0]!.label }), 0, true))).toBe(false); }); }); + + +describe('native Eng setup semantics (captured G)', () => { + // Exact native question stems, offered actions and answers. Five findings + // and both substantive TODO decisions must remain in review coverage. + const rows = [ + { + "header": "Cross-project", + "question": "gstack can search learnings from your other projects on this machine to find patterns that might apply to this auth refactor review. This stays local \u2014 no data leaves your machine. Enable cross-project learnings? ", + "options": [ + "Enable (recommended)", + "Project-scoped only" + ], + "answer": "Enable (recommended)" + }, + { + "header": "Scope", + "question": "D1 \u2014 Scope challenge: this plan touches 12 files and introduces 4 new classes. Reduce scope or proceed as-is? ", + "options": [ + "Reduce: phase it (recommended)", + "Proceed as-is", + "Investigate first" + ], + "answer": "Reduce: phase it (recommended)" + }, + { + "header": "Arch: cache wiring", + "question": "D2 \u2014 Architecture issue 1: AuthBroker and SessionMint share a global mutable AuthCache via module-level export. Module-level singletons prevent test isolation and break in multi-process deployments (cluster, serverless, worker threads). How should AuthCache be wired? ", + "options": [ + "Dependency injection (recommended)", + "Keep module-level export" + ], + "answer": "Dependency injection (recommended)" + }, + { + "header": "Arch: concurrency", + "question": "D3 \u2014 Architecture issue 2: the plan explicitly states mutations to AuthCache are not serialized. With two services writing concurrently (e.g., AuthBroker evicting a token while SessionMint reads-then-writes it), you get classic check-then-act races. How should this be handled? ", + "options": [ + "Make mutations idempotent + last-write-wins (recommended)", + "Add an async mutex per cache key", + "Document and defer" + ], + "answer": "Make mutations idempotent + last-write-wins (recommended)" + }, + { + "header": "Code quality", + "question": "D4 \u2014 Code quality issue 1: validateAndDispatch() is 60 lines with three nested try/catch blocks, each silently swallowing a different error class. Swallowed errors mean silent failures in production \u2014 a token validation error looks identical to a dispatch error from the outside. Fix approach? ", + "options": [ + "Extract + typed errors (recommended)", + "Add structured logging before swallowing", + "Proceed as-is" + ], + "answer": "Extract + typed errors (recommended)" + }, + { + "header": "Tests", + "question": "D5 \u2014 Test issue: all 13 new code paths in AuthCache and AuthBroker are untested (0% coverage planned). The plan mentions unit and integration coverage for success/error paths, but specifics are absent. Add explicit test requirements to the plan now? ", + "options": [ + "Add explicit test plan (recommended)", + "Keep plan vague, trust implementation" + ], + "answer": "Add explicit test plan (recommended)" + }, + { + "header": "Performance", + "question": "D6 \u2014 Performance issue: token validation makes 5 sequential IDP API calls. The plan notes they are independent and could be parallelized via Promise.all trivially. Fix in this PR or defer? ", + "options": [ + "Fix in this PR with Promise.all (recommended)", + "Defer to follow-up TODO" + ], + "answer": "Fix in this PR with Promise.all (recommended)" + }, + { + "header": "TODO: PR 2", + "question": "D7 \u2014 TODO: capture PR 2 scope (SessionMint, TokenStore, RequestPolicy) in TODOS.md so it doesn\u2019t get lost after PR 1 ships. Add it? ", + "options": [ + "Add to TODOS.md (recommended)", + "Skip \u2014 not valuable enough" + ], + "answer": "Add to TODOS.md (recommended)" + }, + { + "header": "TODO: IDP retry", + "question": "D8 \u2014 TODO: IDP circuit breaker. Token validation makes 5 IDP calls (now parallelized). If the IDP is degraded, all 5 fail together \u2014 no retry, no fallback, no circuit breaker in the plan. Add a TODO to add circuit breaker / exponential retry logic around IDP calls? ", + "options": [ + "Add to TODOS.md (recommended)", + "Build it now in this PR", + "Skip \u2014 not valuable enough" + ], + "answer": "Add to TODOS.md (recommended)" + } +]; + const callAt = (index: number) => { + const row = rows[index]!; + return { sessionId: 'captured-g-eng', toolUseId: `g-${index}`, answered: true, failed: false, + questions: [{ header: row.header, question: row.question, options: row.options.map(label => ({ label })) }], + answers: { [row.question]: row.answer } }; + }; + const setup = (call: ReturnType) => engSetupAUQ(nativePlanCallFingerprint(call, 0, false)); + const change = (index: number, question: string, labels?: string[]) => { + const call = callAt(index); const q = call.questions[0]!; + q.question = question; + if (labels) q.options = labels.map(label => ({ label })); + call.answers = { [question]: q.options[0]!.label }; + return call; + }; + test('two setup calls precede five findings and two substantive TODO calls', () => { + for (const order of [[0, 1], [1, 0]]) { + let started = false; + const phases = [...order, 2, 3, 4, 5, 6, 7, 8].map(index => { + const phase = planCountQuestionPhase(nativePlanCallFingerprint(callAt(index), 0, !started), + started, engStep0Boundary, engFirstReviewAUQ, engSetupAUQ); + started = phase.reviewStarted; return phase; + }); + expect(phases.map(p => p.preReview)).toEqual([true, true, false, false, false, false, false, false, false]); + expect(phases.filter(p => !p.preReview)).toHaveLength(7); + } + expect(setup(callAt(0))).toBe(true); + expect(setup(callAt(1))).toBe(true); + for (const index of [2, 3, 4, 5, 6, 7, 8]) expect(setup(callAt(index))).toBe(false); + }); + test('whole-plan scope uses its premise and opposed actions, not a model-chosen qid or header', () => { + for (const question of [ + 'D1 — Scope challenge: this plan touches 12 files and introduces 4 new classes. Reduce scope or proceed as-is?', + 'The plan introduces 4 new services. Reduce the scope or proceed as-is? ', + 'Complexity check: the plan spans 12 files. Reduce scope or proceed as-is? ', + ]) { + const call = change(1, question); call.questions[0]!.header = 'Review setup'; + expect(setup(call)).toBe(true); + call.questions[0]!.options.reverse(); + call.answers = { [question]: call.questions[0]!.options[0]!.label }; + expect(setup(call)).toBe(true); + } + }); + test('abbreviated enable requires explicit cross-project learnings and an opposed project-scoped action', () => { + for (const question of [ + 'Should gstack search learnings from your other projects on this machine?', + 'Enable cross-project learnings for this review? ', + ]) { + const call = change(0, question); call.questions[0]!.header = 'Review setup'; + expect(setup(call)).toBe(true); + call.questions[0]!.options.reverse(); + call.answers = { [question]: call.questions[0]!.options[0]!.label }; + expect(setup(call)).toBe(true); + } + for (const question of ['Enable the new feature?', 'Choose setup. ']) { + expect(setup(change(0, question))).toBe(false); + } + expect(setup(change(0, rows[0]!.question, ['Enable (recommended)', 'Discuss later']))).toBe(false); + }); + test('individual issues and TODOs cannot borrow whole-plan scope or cross-project words', () => { + for (const [index, question, header] of [ + [1, 'D1 — Architecture issue: this plan touches 12 files and adds 4 classes, but the AuthCache has a race. Reduce scope or proceed as-is?', 'Architecture issue'], + [1, 'D1 — Scope challenge: this cache spans 12 files and introduces 4 classes. Reduce scope or proceed as-is?', 'Scope'], + [1, 'D1 — Test gap: the plan spans 12 files. Reduce test scope or proceed as-is?', 'Tests'], + [1, 'The plan spans 12 files. Reduce scope or proceed as-is?', 'TODO: deferred implementation'], + [0, 'D1 — Security issue: cross-project learnings leak client data. Enable the feature or use project-scoped storage?', 'Security issue'], + ] as const) { + const call = change(index, question); call.questions[0]!.header = header; + expect(setup(call)).toBe(false); + } + expect(setup(change(1, rows[1]!.question, ['Reduce token scope', 'Investigate']))).toBe(false); + }); + test('the captured retry scope heading and letter-prefixed native actions remain setup', () => { + const retry = { + "header": "Scope", + "question": "D2 \u2014 Scope reduction: 12 files + 4 new classes exceeds the complexity threshold\nProject/branch/task: Multi-tenant Auth Refactor on main\nELI10: The plan introduces 4 new classes (TokenStore, SessionMint, AuthCache, RequestPolicy) across 12 files. That\u2019s a lot of new surface area at once. Auth refactors are already high-risk (broken auth = all users locked out). Adding 4 new abstractions simultaneously makes the blast radius of a mistake much larger. A simpler split \u2014 just AuthBroker + SessionMint, collapsing TokenStore into AuthCache and inlining RequestPolicy \u2014 would achieve the same goal with 2 new classes and fewer files touched.\nStakes if we pick wrong: With 4 new classes landing together, a single bug in any one of them could take down auth for all tenants simultaneously. Fewer classes = smaller blast radius, easier rollback, faster onboarding for the next engineer.\nRecommendation: B (proceed as-is) \u2014 the plan already has the existing cache adapter retained, and the 4-class split may reflect genuine domain separations the plan description doesn\u2019t fully explain. Review the design at full scope, flag individual issues per section.\nCompleteness: Note: options differ in kind, not coverage \u2014 no completeness score.\nPros / cons:\nA) Reduce scope \u2014 collapse TokenStore into AuthCache, inline RequestPolicy\n \u2705 Fewer moving parts: 2 new classes instead of 4, smaller blast radius if auth fails\n \u2705 Easier to review, test, and roll back each piece independently\n \u274c May discard intentional domain separation the plan author had in mind\n \u274c Requires re-planning before implementation can start\nB) Proceed as-is with full review (recommended)\n \u2705 Respects the planned architecture and lets the full review surface real issues per section\n \u2705 Faster path to implementation if the 4-class split turns out to be justified\n \u274c Higher blast radius: 4 simultaneous new classes touching 12 files is more fragile to ship\n \u274c The legacyAuthFlow rewrite without a regression test is a landmine that needs explicit attention\nNet: You\u2019re trading blast-radius safety (fewer classes) against re-planning delay. Recommend B \u2014 proceed at full scope, but treat each class boundary and the missing regression test as explicit issues in the review. ", + "options": [ + "A) Reduce scope", + "B) Proceed as-is (Recommended)" + ], + "answer": "A) Reduce scope" +}; + const call = change(1, retry.question, retry.options); + call.questions[0]!.header = retry.header; + call.answers = { [retry.question]: retry.answer }; + expect(setup(call)).toBe(true); + // A numerical component issue cannot borrow the whole-plan sentence in + // the explanatory body and the same Reduce/Proceed choices. + const issue = change(1, retry.question.replace('Scope reduction:', 'AuthCache issue:'), retry.options); + expect(setup(issue)).toBe(false); + const partial = change(1, retry.question.replace('ELI10: The plan introduces', 'ELI10: AuthCache introduces'), retry.options); + expect(setup(partial)).toBe(false); + const missingOpposition = change(1, retry.question, ['A) Reduce scope', 'B) Investigate']); + expect(setup(missingOpposition)).toBe(false); + }); + test('late setup cannot hide earlier or later substantive findings', () => { + const order = [2, 1, 3, 0, 4, 5, 6, 7, 8]; + let started = false; + const phases = order.map(index => { + const phase = planCountQuestionPhase(nativePlanCallFingerprint(callAt(index), 0, !started), + started, engStep0Boundary, engFirstReviewAUQ, engSetupAUQ); + started = phase.reviewStarted; return phase; + }); + expect(phases.map(p => p.preReview)).toEqual([false, true, false, true, false, false, false, false, false]); + expect(phases.filter(p => !p.preReview)).toHaveLength(7); + }); + test('pending, failed, uncertain and mixed answered calls never become wholly setup', () => { + for (const index of [0, 1]) { + const original = callAt(index); + expect(engSetupAUQ({ ...nativePlanCallFingerprint(original, 0, false), nativeCall: undefined })).toBe(false); + for (const mutate of [ + (c: ReturnType) => { c.answered = false; }, + (c: ReturnType) => { c.failed = true; }, + (c: ReturnType) => { c.answers = {}; }, + (c: ReturnType) => { c.answers = { [c.questions[0]!.question]: 'Uncertain; I have not selected a choice' }; }, + ]) { const c = structuredClone(original); mutate(c); expect(setup(c)).toBe(false); } + const mixed = structuredClone(original); const finding = callAt(2).questions[0]!; + mixed.questions.push(finding); + mixed.answers[finding.question] = finding.options[0]!.label; + expect(setup(mixed)).toBe(false); + expect(planCountQuestionPhase(nativePlanCallFingerprint(mixed, 0, true), false, + engStep0Boundary, engFirstReviewAUQ, engSetupAUQ).preReview).toBe(false); + mixed.answers = { [finding.question]: finding.options[0]!.label }; + expect(setup(mixed)).toBe(false); + } + }); +}); + +describe('completed permission cannot become a queued review answer (captured G)', () => { + // Exact captured post-Write frame; the temporary repository path is sanitized. + // The damaged "wat" came from the CLI redraw, not the underlying question. + const captured = " real,specificgaps (Visual Hierachy, Spacing, Color, Typography,Motion)\r22-Itpreservesstrongaccessibilityandresponsivespecsfromtheexistingbehaviordescription\r23 -DESIGN.md exist andsuppliescorrectvaluesforall5gaps\r24\r 25 A 10/10would:\r26-Specifytheexactchangeforeachgap(concretetoken,before→after)\r27 -Add intection state table(loading,empty,error,success,partial)\r28-Storyboardtheuserjourney\r 29- Call out which DESIGN.md tokens each fix applies\r30-Resolveallimplementationambiguitiesbeforeadeveloperpicksitup\r31\r32 **DESIGN.md status:** Presnt and detailed. Spacing scale, clor system, typography,interacton patterns,andc\r omponentbehavior re all specified. This review calbrates against it.\r33\r34 **Exsting design lverage:**\r35 -Button, Field, InlineStatus,ErrorSummary,ConfirmationDialogcomponentsreusedas-is\r36-Focusring:2pxsolid#1d4ed8,offset2px(alreadyspecified)\r37 -8px bae spacingscalealreadyestablished\r38\r39 ---\r40\r41 ## Review Passes\r42\r43*(Populatedprogressivelyasthereviewproceeds)*\r44\r╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌\r Do you wat to create gstack-test-plan-design.md?\r❯ 1. Yes\r2.Yes,andswitchtoacceptedits(auto-approvefileeditsandcommonfilecommands)for this session; Yes,an\r always allow access to /tmp/fixture for this session\r 3.Nohift+tab)\r\rEsc to cancl ·Tab to amend\u000f\u001b[?1000h\u001b[?1002h\u001b[?1003h\u001b[?1006h\rRan1shellcommand\r\r●Scopegate:planmodeactive—auto-selectingB(reviewingPLAN.mdintheworkingdirectory).\r\rSkippingroutinginjection—thisisareviewfixturerepo.Nowrunningthepre-reviewaudit.\r\rRead2 files, ran1 shell command\r\r●Now let me read the review sectons fil as required by theskill.\r\rRead file, ran 1shellcommand\r\r●Nopriorlearnings found. Cross-project config isnot set — will ask once Ihavetheuser'sattention.\r\rPreReview System Audit:\r- DESIGN.md:exists and comprehensive (spacing, typography, color, interactionpatternsallspecified)\r- UIscope:APPUIclassifier—account-settingsformwithProfile+Notificationssections\r- 5 gaps explicily listed in the plan; l have corresponding specin DESIGN.md\r-N rior design reviews inth reviewlog\r- NoTODOS.mdexists\r\rStep0 Assessment:\r\rInitial scor: 5/10. The plan correctly identifies5realgapsandpreservesstrongaccessibilityandresponsivespecs\rinheritedfromtheexistingbehaviordescription.A10/10wouldspecifyHOWeachgapisresolved(concretecomponent\rchanges,tokenvalues,before/afterlayout),notjustnametheproblem.TheDESIGN.mdalreadyhasallcorrectvalues;\rtheplanjustneedstoreferencethem.\r\rWritingtheinitialplanfilenowbeforeaskingStep0D.\r\r●Write(/tmp/fixture/gstack-test-plan-design.md)\r\r✢ Undulatig… (2m 0s ·↓ 5.4 okens)\r ⎿  Tip:Use/btwtoaskaquicksidequestionwithoutinterruptingClaude'scurrentwork\r ● high · /effort\r────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\r❯ \r────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────\r ⏸ plan mod on(shift+tab tocycle)·esctointerrupt·←foragents\u001b[?25h\u001b[?25l\r●No prior earnings found.Cross-projectconfigisnotset—willaskonceIhavetheuser'sattention.\r Pre-Review System Audit:\r-DESIGN.md:existsandcomprehensive(spacing,typography,color,interactionpatternsallspecified)\r- UI scope: APP UI classifier — account-settings fomwith Prfile + Notifications sctions\r-5gapsexplicitlylistedintheplan;allhavecorrespondingspecinDESIGN.md\r- Noprior desigreviwsin the reviewlog\r-NoTODOS.mdexists\r\rStep0Assessment:\r\rInitialscore:5/10.Theplancorrectlyidentifies5realgapsandpreservesstrongaccessibilityandresponsivespecs\r inheted from theexisting behavir desription.A 10/10would specify HOWechgapi eolved (ccretecomponent\rchanges,tokenvalues,before/afterlayout),notjustnametheproblem.TheDESIGN.mdalreadyhasallcorrectvalues;\rth plan just neds to referencethem.\r\rWriting theinitial plan lenow before ask Step 0D.\r\r●Write(/tmp/fixture/gstack-test-plan-design.md)\r⎿ Wrote 44 lines"; + + test('a missed grant followed by native Write completion sends no stale answer', () => { + const guard = createPlanCountPermissionGuard(); + expect(classifyPlanCountFrame(captured)).toBeNull(); + expect(guard(captured)).toBe('handled'); + expect(capturePlanCountQuestion(captured, new Set(), 0, true)).toBeNull(); + }); + + test('the same damaged active file permission grants once and never counts as a finding', () => { + const menu = captured.slice(captured.indexOf('Do you wat'), captured.indexOf('Esc to cancl')) + 'Esc to cancl ·Tab to amend'; + const guard = createPlanCountPermissionGuard(); + expect(guard(menu)).toBe('grant'); + expect(guard(menu)).toBe('handled'); + expect(capturePlanCountQuestion(menu, new Set(), 0, false)).toBeNull(); + const completed = menu + '\n⎿ Wrote 44 lines'; + expect(guard(completed)).toBe('handled'); + expect(guard(completed + '\n' + menu)).toBe('grant'); + }); + + test('plain legacy file decisions remain questions without native permission controls', () => { + const frame = 'Do you want to create first.md?\n❯1.Create the reviewed file\n2.Keep the current layout'; + expect(classifyPlanCountFrame(frame)).toBeNull(); + expect(createPlanCountPermissionGuard()(frame)).toBeNull(); + expect(capturePlanCountQuestion(frame, new Set(), 0, false)?.options).toHaveLength(2); + }); + + test('a matching native finding can discuss file permissions without being consumed', () => { + const question = { + header: 'File policy', + question: 'Should we create a file that documents always allow access to the project?', + options: [{ label: 'Create it' }, { label: 'Keep current policy' }], + }; + const pending = { sessionId: 'file-policy', toolUseId: 'file-finding', answered: false, questions: [question] }; + const frame = captured + '\n☐ ' + question.header + '\n' + question.question + + '\n❯1.Create it\n2.Keep current policy\nEnter to select · ↑/↓ to navigate · Esc to cancel'; + expect(classifyPlanCountFrame(frame)).toBeNull(); + expect(createPlanCountPermissionGuard()(frame)).toBeNull(); + expect(capturePlanCountQuestion(frame, new Set(), 0, false, pending)?.nativeCall).toBe(pending); + expect(capturePlanCountQuestion(frame, new Set(), 0, false)?.promptSnippet).toContain('File policy'); + }); +}); + + +describe('native question identity outranks permission wording', () => { + const question = { + header: 'File policy', + question: 'D1 — Should we create a file that documents always allow access to the project? ', + options: [{ label: 'Create it' }, { label: 'Keep current policy' }], + }; + const pending = { sessionId: 'file-policy', toolUseId: 'finding', answered: false, questions: [question] }; + const frame = '☐ File policy\n' + question.question + '\n❯1.Create it\n2.Keep current policy\nEnter to select · ↑/↓ to navigte · Esc to cancel'; + test('full native question and every option establish identity despite a damaged footer', () => { + // This is the formerly conflicting pure classifier result. The counting + // loop must consult native identity before taking its permission action. + expect(classifyPlanCountFrame(frame)).toBe('permission'); + expect(matchesNativePlanQuestion(frame, pending)).toBe(true); + const seen = new Set(); + expect(capturePlanCountQuestion(frame, seen, 0, false, pending)?.nativeCall).toBe(pending); + expect(capturePlanCountQuestion(frame, seen, 1, false, pending)).toBeNull(); + }); + test('same header, changed choices, missing identity and an overlaid real permission cannot borrow a native call', () => { + for (const different of [ + frame.replace('Should we create a file', 'Should we delete the file'), + frame.replace('2.Keep current policy', '2.Allow all edits'), + frame.replace(question.question, 'A different question with the same header?'), + frame + '\nDo you want to create actual.md?\n❯1.Yes\n2.Yes, and switch to accept edits\n3.No\nEsc to cancel · Tab to amend', + ]) expect(matchesNativePlanQuestion(different, pending)).toBe(false); + expect(capturePlanCountQuestion(frame, new Set(), 0, false, { ...pending, failed: true })).toBeNull(); + expect(capturePlanCountQuestion(frame, new Set(), 0, false)).toBeNull(); + }); +}); diff --git a/test/helpers/disabled-plan-review-fixture.ts b/test/helpers/disabled-plan-review-fixture.ts index 431fc5244..41bf1808e 100644 --- a/test/helpers/disabled-plan-review-fixture.ts +++ b/test/helpers/disabled-plan-review-fixture.ts @@ -90,7 +90,9 @@ export function disabledPlanReviewEvidence(result: { || result.output.split('\n').some(line => /\b(?:outside(?:\s+(?:voice|review))?|codex(?:\s+review)?)\b/i.test(line) && /\bdisabled\b/i.test(line) && !/\bnot\s+disabled\b/i.test(line)); const falseCompletion = /\b(?:both reviewers agree|outside_status["'`*]*\s*[:=]\s*["'`*]*completed)\b/i.test(result.output); - const agentAvailable = Array.isArray(init?.tools) && init.tools.includes('Agent'); + // Native CLI releases expose the requested subagent as Agent or Task. + // Availability never permits dispatch: fallbackCalls rejects both names. + const agentAvailable = Array.isArray(init?.tools) && init.tools.some((tool: unknown) => tool === 'Agent' || tool === 'Task'); let records: any[] = []; let malformedLog = false; try { records = reviewLog.trim() ? reviewLog.trim().split('\n').map(line => JSON.parse(line)) : []; } diff --git a/test/helpers/plan-count-artifacts.ts b/test/helpers/plan-count-artifacts.ts index 3568fbdb4..506ba3a06 100644 --- a/test/helpers/plan-count-artifacts.ts +++ b/test/helpers/plan-count-artifacts.ts @@ -7,6 +7,7 @@ interface PlanCountSnapshot { observation: object; raw: string; visible: string; + viewport?: string; cwd: string; claudeConfigDir: string | null; } @@ -31,6 +32,7 @@ export function createPlanCountSnapshotWriter(env: NodeJS.ProcessEnv = process.e }; write('terminal.raw.log', input.raw); write('terminal.visible.log', input.visible); + if (input.viewport !== undefined) write('terminal.screen.log', input.viewport); write('observation.json', JSON.stringify({ ...input.observation, artifactDir, capture: { skill: input.skillName, runId: env.EVALS_RUN_ID, cwd: input.cwd, diff --git a/test/helpers/plan-count-transcript.ts b/test/helpers/plan-count-transcript.ts index c1de9670d..07c8e05ed 100644 --- a/test/helpers/plan-count-transcript.ts +++ b/test/helpers/plan-count-transcript.ts @@ -25,6 +25,8 @@ export interface PlanCountTranscript { status: 'missing' | 'ready' | 'error'; calls: NativePlanQuestionCall[]; assistantMessages: Array<{ sessionId: string; text: string; timestamp: string }>; + /** Actual native plan-mode approval requests; pending is the UI gate, never an AUQ. */ + planReadyRequests?: Array<{ sessionId: string; toolUseId: string; timestamp: string; failed: boolean }>; error?: string; } @@ -57,6 +59,7 @@ function validQuestions(value: unknown): value is NativePlanQuestion[] { export function readPlanCountTranscript(configDir: string, cwd: string): PlanCountTranscript { const calls = new Map(); const assistantMessages: PlanCountTranscript['assistantMessages'] = []; + const planReadyRequests = new Map[number]>(); let matched = false; let bytes = 0; let files = 0; @@ -89,6 +92,12 @@ export function readPlanCountTranscript(configDir: string, cwd: string): PlanCou typeof block.text === 'string' && block.text.trim() && validTimestamp(record.timestamp)) { assistantMessages.push({ sessionId: record.sessionId, text: block.text, timestamp: record.timestamp }); } + if (record.message.role === 'assistant' && block.type === 'tool_use' && block.name === 'ExitPlanMode' && + typeof block.id === 'string' && validTimestamp(record.timestamp)) { + const key = `${record.sessionId}:${block.id}`; + if (!planReadyRequests.has(key)) planReadyRequests.set(key, { sessionId: record.sessionId, + toolUseId: block.id, timestamp: record.timestamp, failed: false }); + } if (record.message.role === 'assistant' && block.type === 'tool_use' && block.name === 'AskUserQuestion' && typeof block.id === 'string' && object(block.input) && validQuestions(block.input.questions)) { const key = `${record.sessionId}:${block.id}`; @@ -100,6 +109,8 @@ export function readPlanCountTranscript(configDir: string, cwd: string): PlanCou questions: block.input.questions, answered: false }); } else if (record.message.role === 'user' && block.type === 'tool_result' && typeof block.tool_use_id === 'string') { + const ready = planReadyRequests.get(`${record.sessionId}:${block.tool_use_id}`); + if (ready && block.is_error === true) ready.failed = true; const call = calls.get(`${record.sessionId}:${block.tool_use_id}`); const answers = record.toolUseResult?.answers; const validAnswers = call && object(answers) ? Object.fromEntries(call.questions @@ -124,7 +135,8 @@ export function readPlanCountTranscript(configDir: string, cwd: string): PlanCou } } } - return { status: matched ? 'ready' : 'missing', calls: [...calls.values()], assistantMessages }; + return { status: matched ? 'ready' : 'missing', calls: [...calls.values()], assistantMessages, + ...(planReadyRequests.size ? { planReadyRequests: [...planReadyRequests.values()] } : {}) }; } catch (error) { // A failed read cannot silently turn an incomplete transcript into a // complete review. Keep the diagnostic explicit and return no coverage. diff --git a/test/helpers/pty-screen.ts b/test/helpers/pty-screen.ts new file mode 100644 index 000000000..5618c5035 --- /dev/null +++ b/test/helpers/pty-screen.ts @@ -0,0 +1,82 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { createRequire } from 'node:module'; + +/** The existing xterm package ships this headless source beside its browser bundle. */ +let terminalConstructor: Promise | undefined; +function loadTerminal(): Promise { + return terminalConstructor ??= (async () => { + try { + const source = path.join(path.dirname(createRequire(import.meta.url).resolve('xterm/package.json')), 'src'); + const entry = path.join(source, 'headless/public/Terminal.ts'); + if (!fs.existsSync(entry)) throw new Error(`Installed xterm headless source is missing: ${entry}`); + const transpiler = new Bun.Transpiler({ loader: 'ts', target: 'bun', trimUnusedImports: true, + tsconfig: JSON.stringify({ compilerOptions: { experimentalDecorators: true, useDefineForClassFields: false } }) }); + const build = await Bun.build({ entrypoints: [entry], target: 'bun', format: 'esm', + // xterm5 detects Node by navigator absence. This changes only the + // compiled module; Bun's global navigator and other tests stay intact. + define: { navigator: 'undefined' }, + plugins: [{ name: 'installed-xterm-headless', setup(builder) { + builder.onResolve({ filter: /^(common|headless)\// }, args => { + const stem = path.join(source, args.path); + return { path: fs.existsSync(`${stem}.ts`) ? `${stem}.ts` : `${stem}.d.ts` }; + }); + builder.onLoad({ filter: /[/\\]xterm[/\\]src[/\\].*\.ts$/ }, async args => ({ + contents: transpiler.transformSync(await Bun.file(args.path).text()), loader: 'js', + })); + } }], + }); + if (!build.success) throw new AggregateError(build.logs, 'Installed xterm headless build failed'); + const encoded = Buffer.from(await build.outputs[0].text()).toString('base64'); + return (await import(`data:text/javascript;base64,${encoded}`)).Terminal; + } catch (cause) { + throw new Error('PTY screen unavailable; cannot safely observe terminal input.', { cause }); + } + })(); +} + +export interface PtyScreen { + write(text: string): void; + read(): Promise; + dispose(): Promise; +} + +/** One terminal per session; read only the actual viewport, never scrollback. */ +export async function createPtyScreen(cols: number, rows: number): Promise { + const Terminal = await loadTerminal(); + const terminal = new Terminal({ cols, rows, scrollback: 0, allowProposedApi: true }); + let pending = 0; + let failure: unknown; + let final: string | undefined; + let closing: Promise | undefined; + const waiting = new Set<() => void>(); + const settled = () => { if (pending === 0) { for (const done of waiting) done(); waiting.clear(); } }; + const drain = async () => { + while (pending > 0) await new Promise(resolve => waiting.add(resolve)); + if (failure) throw new Error('PTY screen parse failed.', { cause: failure }); + }; + const viewport = () => { + const buffer = terminal.buffer.active; + return Array.from({ length: rows }, (_, i) => buffer.getLine(buffer.baseY + i)?.translateToString(true) ?? '').join('\n'); + }; + return { + write(text) { + if (closing) throw new Error('Cannot write to a disposed PTY screen.'); + if (!text) return; + pending++; + try { terminal.write(text, () => { pending--; settled(); }); } + catch (error) { failure = error; pending--; settled(); } + }, + async read() { + if (closing) { await closing; return final!; } + await drain(); + return viewport(); + }, + dispose() { + return closing ??= (async () => { + try { await drain(); final = viewport(); } + finally { terminal.dispose(); } + })(); + }, + }; +} diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index d19825cc8..ed818ae72 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -153,17 +153,17 @@ export const E2E_TOUCHFILES: Record = { // devex, office-hours + future PR2 carves). One file iterating CARVE_GUARDS; // the selector sets GSTACK_CARVE_SKILL= to scope cost to the changed // skill (D-CODEX A). Touching the registry/helper or sections.ts runs all. - 'carve-section-loading': ['design-html/**', 'design-shotgun/**', 'qa/**', 'browse/**', 'retro/**', 'autoplan/**', 'spec/**', 'setup-gbrain/**', 'review/**', 'codex/**', 'land-and-deploy/**', 'plan-eng-review/**', 'plan-design-review/**', 'plan-devex-review/**', 'office-hours/**', 'document-release/**', 'design-consultation/**', 'cso/**', 'test/helpers/carve-guards.ts', 'scripts/resolvers/sections.ts', 'scripts/resolvers/redact-doc.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/carve-section-loading.test.ts'], - '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/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/helpers/autoplan-phase-observer.ts', 'test/autoplan-phase-observer.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/skill-e2e-autoplan-chain.test.ts'], + 'carve-section-loading': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'design-html/**', 'design-shotgun/**', 'qa/**', 'browse/**', 'retro/**', 'autoplan/**', 'spec/**', 'setup-gbrain/**', 'review/**', 'codex/**', 'land-and-deploy/**', 'plan-eng-review/**', 'plan-design-review/**', 'plan-devex-review/**', 'office-hours/**', 'document-release/**', 'design-consultation/**', 'cso/**', 'test/helpers/carve-guards.ts', 'scripts/resolvers/sections.ts', 'scripts/resolvers/redact-doc.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/carve-section-loading.test.ts'], + 'autoplan-chain-pty': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/ui-heavy-feature.md', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/helpers/autoplan-phase-observer.ts', 'test/autoplan-phase-observer.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.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-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/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/ceo-completion-handoff.ts', 'test/ceo-completion-handoff.test.ts', 'test/fixtures/ceo-completion-handoff-calls.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-finding-count.test.ts'], - 'plan-eng-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-eng-finding-count.test.ts'], - 'plan-design-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-design-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-design-finding-count.test.ts'], - 'plan-devex-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-devex-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts'], + '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/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/ceo-completion-handoff.ts', 'test/ceo-completion-handoff.test.ts', 'test/fixtures/ceo-completion-handoff-calls.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-finding-count.test.ts'], + 'plan-eng-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-eng-finding-count.test.ts'], + 'plan-design-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-design-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-design-finding-count.test.ts'], + 'plan-devex-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-devex-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts'], // Gate-tier reviewCount-floor counterparts. Catch the May 2026 transcript // bug (model wrote a plan-mode plan and ExitPlanMode'd without firing any @@ -180,8 +180,8 @@ export const E2E_TOUCHFILES: Record = { // a model fires one AUQ then batches the rest into a "## Decisions to // confirm" plan write. runPlanSkillFloorCheck cannot detect that shape // (it exits on first AUQ); runPlanSkillCounting can. - 'plan-eng-multi-finding-batching': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts'], - 'plan-ceo-split-overflow': ['bin/gstack-config', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts'], + '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/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts'], + 'plan-ceo-split-overflow': ['bin/gstack-config', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts'], 'brain-privacy-gate': ['bin/gstack-skill-start', 'bin/gstack-skill-end', 'scripts/resolvers/preamble/generate-brain-sync-block.ts', 'scripts/resolvers/preamble.ts', 'bin/gstack-brain-sync', 'bin/gstack-artifacts-init', 'bin/gstack-config', 'test/helpers/agent-sdk-runner.ts', 'test/skill-e2e-brain-privacy-gate.test.ts'], // /setup-gbrain Path 4 (Remote MCP) — happy + bad-token end-to-end via @@ -361,7 +361,7 @@ export const E2E_TOUCHFILES: Record = { // Autoplan - 'autoplan-dual-voice': ['autoplan/**', 'codex/**', 'bin/gstack-codex-probe', 'scripts/resolvers/review.ts', 'scripts/resolvers/design.ts', 'test/skill-e2e-autoplan-dual-voice.test.ts'], + 'autoplan-dual-voice': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'autoplan/**', 'codex/**', 'bin/gstack-codex-probe', 'scripts/resolvers/review.ts', 'scripts/resolvers/design.ts', 'test/skill-e2e-autoplan-dual-voice.test.ts'], // Multi-provider benchmark adapters — live API smoke against real claude/codex/gemini CLIs 'benchmark-providers-live': ['bin/gstack-model-benchmark', 'test/helpers/providers/**', 'test/helpers/benchmark-runner.ts', 'test/helpers/pricing.ts', 'test/skill-e2e-benchmark-providers.test.ts'], diff --git a/test/plan-count-completion.test.ts b/test/plan-count-completion.test.ts index cca888444..889f3d123 100644 --- a/test/plan-count-completion.test.ts +++ b/test/plan-count-completion.test.ts @@ -4,7 +4,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { pathToFileURL } from 'node:url'; -import { hasNativePlanCompletion, classifyPlanCountFrame } from './helpers/claude-pty-runner'; +import { hasNativePlanCompletion, hasNativePlanTerminal, classifyPlanCountFrame } from './helpers/claude-pty-runner'; import type { PlanCountTranscript } from './helpers/plan-count-transcript'; const CAPTURED_CALL = { @@ -239,3 +239,255 @@ process.stdin.resume(); } finally { clearTimeout(timer); child.kill('SIGKILL'); fs.rmSync(dir, { recursive: true, force: true }); } }, 40000); }); + +// Last answered call and last finalized native text at the exact G soft-terminal race. +const CAPTURED_ENG_STREAM = { + "status": "ready", + "calls": [ + { + "sessionId": "4c3d29fb-2a00-4e5d-b3c6-9b584986e340", + "toolUseId": "toolu_011KrMyU6fjW7RQ7aAVJ4DfA", + "questions": [ + { + "question": "D7 — Performance: 5 sequential IDP calls that could be Promise.all'd\nProject/branch/task: Multi-tenant Auth Refactor on main\nELI10: During token validation, the code makes 5 API calls to the identity provider (IDP) one at a time. But the calls are independent — none uses the result of the previous one. Changing from sequential to Promise.all([…]) takes one line of code and cuts auth latency by ~5x. At 100ms per call, sequential = ~500ms; parallel = ~100ms. This matters most under load: auth is on the critical path of every authenticated request.\nStakes if we pick wrong: Every authenticated request pays a 400ms tax that could be eliminated with one line. Under load, slow auth creates timeout cascades — auth is typically among the first things to degrade.\nRecommendation: A — trivially fix it now. The plan itself says “calls are independent” and “trivially” parallelizable. The only reason not to is if there’s a hidden ordering dependency the plan doesn’t mention.\nCompleteness: Note: options differ in kind, not coverage — no completeness score.\nPros / cons:\nA) Parallelize the 5 IDP calls with Promise.all in this PR (recommended)\n ✅ 5x latency reduction on the auth critical path for free — one line change\n ✅ Under load, parallel calls bound latency to the slowest single call instead of sum of all\n ❌ If any hidden ordering dependency exists (not mentioned in the plan), it will surface as a test failure — easily caught\nB) Defer parallelization to a follow-up performance PR\n ✅ Keeps this PR focused on the refactor\n ❌ The plan already flagged this as trivial; deferring creates a PR that self-documents debt without resolving it\nNet: The plan already identified this as trivial. Doing it now costs 5 minutes and makes the auth path meaningfully faster for all users. ", + "header": "Performance", + "options": [ + { + "label": "A) Parallelize IDP calls now (Recommended)", + "description": "Promise.all the 5 independent calls. ~5 min, 5x auth latency improvement." + }, + { + "label": "B) Defer to a follow-up PR", + "description": "Keeps this PR focused on refactor; acknowledged debt." + } + ], + "multiSelect": false + } + ], + "answered": true, + "failed": false, + "answers": { + "D7 — Performance: 5 sequential IDP calls that could be Promise.all'd\nProject/branch/task: Multi-tenant Auth Refactor on main\nELI10: During token validation, the code makes 5 API calls to the identity provider (IDP) one at a time. But the calls are independent — none uses the result of the previous one. Changing from sequential to Promise.all([…]) takes one line of code and cuts auth latency by ~5x. At 100ms per call, sequential = ~500ms; parallel = ~100ms. This matters most under load: auth is on the critical path of every authenticated request.\nStakes if we pick wrong: Every authenticated request pays a 400ms tax that could be eliminated with one line. Under load, slow auth creates timeout cascades — auth is typically among the first things to degrade.\nRecommendation: A — trivially fix it now. The plan itself says “calls are independent” and “trivially” parallelizable. The only reason not to is if there’s a hidden ordering dependency the plan doesn’t mention.\nCompleteness: Note: options differ in kind, not coverage — no completeness score.\nPros / cons:\nA) Parallelize the 5 IDP calls with Promise.all in this PR (recommended)\n ✅ 5x latency reduction on the auth critical path for free — one line change\n ✅ Under load, parallel calls bound latency to the slowest single call instead of sum of all\n ❌ If any hidden ordering dependency exists (not mentioned in the plan), it will surface as a test failure — easily caught\nB) Defer parallelization to a follow-up performance PR\n ✅ Keeps this PR focused on the refactor\n ❌ The plan already flagged this as trivial; deferring creates a PR that self-documents debt without resolving it\nNet: The plan already identified this as trivial. Doing it now costs 5 minutes and makes the auth path meaningfully faster for all users. ": "A) Parallelize IDP calls now (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-08T18:46:43.934Z" + } + ], + "assistantMessages": [ + { + "sessionId": "4c3d29fb-2a00-4e5d-b3c6-9b584986e340", + "text": "Tests decision: add characterization tests for legacyAuthFlow() before rewriting. Moving to Section 4: Performance Review.\n\n**Finding 4 [P2] (confidence: 9/10):** The plan states: \"Token validation issues 5 sequential API calls to the IDP; they could be parallelized via Promise.all trivially (calls are independent).\"\n\nThis is a free 5x latency improvement being left on the table. At 100ms per IDP call (typical), sequential = ~500ms per auth validation. Parallel via Promise.all = ~100ms.", + "timestamp": "2026-09-08T18:46:30.531Z" + } + ] +}; +const CAPTURED_DX_HANDOFF = { + "sessionId": "cc1b321f-f306-4086-a236-34dd30f457be", + "toolUseId": "toolu_01AGy3uosDrjJJUv1aKGkhRY", + "questions": [ + { + "question": "DX review done. 5 findings resolved, 7 implementation tasks written. What's next?\n\nEvalKit SDK / DX POLISH review / main\nThe DX review found API design issues (argument order, auth error), a broken getting-started path, a TTHW gap, and a cliff-edge upgrade. All resolved with user decisions. The full output is at gstack-test-plan-devex.md. Eng review is the required gate before shipping.\n", + "header": "Next step", + "multiSelect": false, + "options": [ + { + "label": "Run /plan-eng-review (Recommended)", + "description": "Required gate. The arg-order fix and --skip-ci flag both have architectural implications that eng review should validate." + }, + { + "label": "Ready to implement", + "description": "Start building the 7 tasks. Run /devex-review after shipping to measure actual TTHW vs the <2-min target." + }, + { + "label": "Skip, handle manually", + "description": "You'll manage next steps outside this session." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "DX review done. 5 findings resolved, 7 implementation tasks written. What's next?\n\nEvalKit SDK / DX POLISH review / main\nThe DX review found API design issues (argument order, auth error), a broken getting-started path, a TTHW gap, and a cliff-edge upgrade. All resolved with user decisions. The full output is at gstack-test-plan-devex.md. Eng review is the required gate before shipping.\n": "Run /plan-eng-review (Recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-08T18:46:16.771Z" +}; + +describe('guarded native terminal and report lifecycle', () => { + test('G Eng streamed summary cannot finish while native text/file is still pending', () => { + const f = fixture(); + try { + const visible = '● Performance decision: parallelize the 5 IDP calls. All 4 review sections complete.\n\nCompletionSummary:\n- Step 0: Scope reduced\n- Architecture Review:1 issue found\n- Code Quality Review:1 issue found\n'; + expect(classifyPlanCountFrame(visible)).toBe('completion_summary'); + const t: PlanCountTranscript = structuredClone(CAPTURED_ENG_STREAM); + const answerTime = Math.max(...t.calls.map(c => Date.parse(c.answeredAt!))); + fs.utimesSync(f.file, (answerTime + 1) / 1000, (answerTime + 1) / 1000); + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'completion_summary')).toBe(false); + t.assistantMessages.push({ sessionId: t.calls[0]!.sessionId, timestamp: new Date(answerTime + 2).toISOString(), + text: 'Completion Summary:\n- Architecture Review: 1 issue resolved\n- Code Quality Review: 1 issue resolved\n- Test Review: 1 gap resolved\n- Performance Review: 1 issue resolved\n' }); + for (const content of ['', '# Draft\n', '## GSTACK REVIEW REPORT\n', REPORT.replace('NO UNRESOLVED DECISIONS', ''), REPORT.replace('| DX Review | clean | resolved |\n', '')]) { + fs.writeFileSync(f.file, content); fs.utimesSync(f.file, (answerTime + 3) / 1000, (answerTime + 3) / 1000); + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'completion_summary'), content).toBe(false); + } + fs.writeFileSync(f.file, REPORT); fs.utimesSync(f.file, (answerTime + 3) / 1000, (answerTime + 3) / 1000); + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'completion_summary')).toBe(true); + } finally { f.cleanup(); } + }); + + test('the actual post-report DX handoff does not require rewriting an unchanged report', () => { + const f = fixture(); + try { + const t: PlanCountTranscript = structuredClone(f.transcript); + const sessionId = t.calls[0]!.sessionId; + t.calls.push({ ...structuredClone(CAPTURED_DX_HANDOFF), sessionId }); + t.planReadyRequests = [{ sessionId, toolUseId: 'exit', timestamp: '2026-09-08T18:46:27.000Z', failed: false }]; + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'plan_ready')).toBe(true); + // Unknown, free-form, partly unanswered and substantive menus remain + // freshness boundaries, even if they reuse the same next-step header. + for (const mutate of [ + (c: any) => { c.questions[0].question = c.questions[0].question.replace('devex-next-steps', 'new-change'); }, + (c: any) => { c.answers[c.questions[0].question] = 'Also change the SDK auth behavior'; }, + (c: any) => { c.questions[0].options.push({ label: 'Change the error message now' }); }, + (c: any) => { c.unansweredQuestionIndices = [1]; }, + ]) { + const changed = structuredClone(t); mutate(changed.calls[1]); + expect(hasNativePlanTerminal(changed, f.file, f.startedAt, 'plan_ready')).toBe(false); + } + t.planReadyRequests[0]!.failed = true; + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'plan_ready')).toBe(false); + } finally { f.cleanup(); } + }); + + test('pending/refused/mixed calls, stale or incomplete reports and quoted summaries cannot complete', () => { + const f = fixture(); + try { + const t = structuredClone(f.transcript); + t.planReadyRequests = [{ sessionId: t.calls[0]!.sessionId, toolUseId: 'exit', timestamp: CAPTURED_FINAL.timestamp, failed: false }]; + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'plan_ready')).toBe(true); + const retried = structuredClone(t); + retried.calls.unshift({ ...structuredClone(retried.calls[0]!), toolUseId: 'failed-before-retry', answered: false, failed: true, answers: undefined }); + expect(hasNativePlanTerminal(retried, f.file, f.startedAt, 'plan_ready')).toBe(true); + retried.calls[0]!.questions[0]!.question = 'A different unresolved question'; + expect(hasNativePlanTerminal(retried, f.file, f.startedAt, 'plan_ready')).toBe(false); + for (const mutate of [ + (t: any) => { t.calls[0].answered = false; }, + (t: any) => { t.calls[0].failed = true; }, + (t: any) => { t.calls[0].sessionId = 'other'; }, + (t: any) => { t.planReadyRequests[0].failed = true; }, + (t: any) => { t.planReadyRequests[0].timestamp = t.calls[0].answeredAt; }, + (t: any) => { t.planReadyRequests = []; }, + (t: any) => { t.status = 'missing'; }, + ]) { const changed = structuredClone(t); mutate(changed); expect(hasNativePlanTerminal(changed, f.file, f.startedAt, 'plan_ready')).toBe(false); } + const summary = 'Completion Summary:\n- Review: 4 findings resolved\n'; + for (const text of ['> ' + summary, 'Example:\n' + summary, '```text\n' + summary + '```', 'Here is an example:\n\n' + summary, 'Here is a quoted result:\n```text\n' + summary + '```', 'I cannot complete the review.\n' + summary, 'Completion Summary:\n', summary + 'Waiting for your answer.']) { + t.assistantMessages[0]!.text = text; + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'completion_summary'), text).toBe(false); + } + const old = Date.parse('2026-09-08T17:40:00Z') / 1000; fs.utimesSync(f.file, old, old); + expect(hasNativePlanTerminal(t, f.file, f.startedAt, 'plan_ready')).toBe(false); + } finally { f.cleanup(); } + }); +}); + +test.skipIf(process.platform === 'win32')('real PTY waits through streamed summary, another question and partial report writes', async () => { + await Promise.all(['completion_summary', 'plan_ready'].map(async (terminal) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'guarded-terminal-pty-')); + const fake = path.join(dir, 'fake-claude'); + const worker = path.join(dir, 'worker.ts'); + const output = path.join(dir, 'plan.md'); + const record = path.join(dir, 'inputs.jsonl'); + const events = path.join(dir, 'events.jsonl'); + const result = path.join(dir, 'result.json'); + const runner = pathToFileURL(path.join(import.meta.dir, 'helpers/claude-pty-runner.ts')).href; + fs.writeFileSync(fake, `#!${process.execPath}\n` + String.raw` +import * as fs from 'node:fs'; +import * as path from 'node:path'; +const sessionId = 'guarded-terminal-fixture'; +const project = path.join(process.env.CLAUDE_CONFIG_DIR, 'projects', sessionId); +fs.mkdirSync(project, { recursive: true }); +const file = path.join(project, sessionId + '.jsonl'); +const native = (role, content, extra = {}) => fs.appendFileSync(file, JSON.stringify({ + cwd: process.cwd(), sessionId, isSidechain: false, timestamp: new Date().toISOString(), message: { role, content }, ...extra, +}) + '\n'); +const event = name => fs.appendFileSync(process.env.PROBE_EVENTS, JSON.stringify({ name, at: Date.now() }) + '\n'); +const q = { header: 'Tests', question: 'D2 — Test finding: add the missing retry assertion? ', options: [{ label: 'Add retry assertion' }, { label: 'Leave untested' }] }; +const ask = id => native('assistant', [{ type: 'tool_use', id, name: 'AskUserQuestion', input: { questions: [q] } }]); +const answer = id => native('user', [{ type: 'tool_result', tool_use_id: id, content: 'Answered.' }], { toolUseResult: { answers: { [q.question]: q.options[0].label } } }); +const summary = 'Completion Summary:\n- Architecture Review: 1 issue resolved\n- Test Review: 1 missing test resolved\n'; +const ready = () => process.stdout.write('\nClaude has written up a plan and is ready to execute. Would you like to proceed?\n❯ 1. Yes, and use auto mode\n 2. Yes, manually approve edits\n 3. Tell Claude what to change\n'); +let stage = 0; +process.stdin.setRawMode?.(true); +process.stdin.on('data', data => { + fs.appendFileSync(process.env.PROBE_INPUTS, JSON.stringify(data.toString()) + '\n'); + if (stage === 0) { + stage = 1; ask('first'); answer('first'); + event('streamed-heading'); process.stdout.write('● Completion Summary:\n- Architecture Review: 1 issue resolved\n'); + setTimeout(() => { + ask('second'); event('pending-question'); + process.stdout.write('\n← ☐ Tests ✔ Submit →\n' + q.question + '\n❯ 1. Add retry assertion\n 2. Leave untested\nEnter to select · Tab/Arrow keys to navigate · Esc to cancel\n'); + }, 3500); + return; + } + if (stage === 1 && data.toString().includes('1')) { + stage = 2; answer('second'); event('answered-second'); + fs.writeFileSync(process.env.PROBE_PLAN, '# Draft\n'); + native('assistant', [{ type: 'text', text: summary }], { timestamp: new Date(Date.now() + 1).toISOString() }); + process.stdout.write('\n● ' + summary); + setTimeout(() => { + event('partial-report'); fs.writeFileSync(process.env.PROBE_PLAN, process.env.PROBE_REPORT.split('VERDICT:')[0]); + }, 3000); + setTimeout(() => { + event('complete-report'); fs.writeFileSync(process.env.PROBE_PLAN, process.env.PROBE_REPORT); + if (process.env.PROBE_TERMINAL === 'plan_ready') { + native('assistant', [{ type: 'tool_use', id: 'rejected-ready', name: 'ExitPlanMode', input: {} }]); + native('user', [{ type: 'tool_result', tool_use_id: 'rejected-ready', is_error: true, content: 'Gate not ready' }]); + ready(); + setTimeout(() => { + event('valid-native-ready'); native('assistant', [{ type: 'tool_use', id: 'valid-ready', name: 'ExitPlanMode', input: {} }]); ready(); + }, 2500); + } + }, 6000); + } else if (stage === 2) event('unexpected-plan-approval-input'); +}); +process.stdin.resume(); +`); + fs.chmodSync(fake, 0o755); + fs.writeFileSync(worker, `import { runPlanSkillCounting } from ${JSON.stringify(runner)};\n` + + `const result = await runPlanSkillCounting({skillName:'plan-eng-review',slashCommand:'/plan-eng-review',followUpPrompt:'# Completion fixture',expectedPlanPath:${JSON.stringify(output)},isLastStep0AUQ:()=>false,isReviewAUQ:()=>true,reviewCountCeiling:8,timeoutMs:30000,env:${JSON.stringify({PROBE_PLAN:output,PROBE_INPUTS:record,PROBE_EVENTS:events,PROBE_REPORT:REPORT,PROBE_TERMINAL:terminal})}});\n` + + `await Bun.write(${JSON.stringify(result)},JSON.stringify(result));\n`); + const child = Bun.spawn([process.execPath, worker], { env: { ...process.env, + EVALS_HERMETIC: '1', EVALS_RUN_ID: '', BROWSE_TERMINAL_BINARY: fake }, stdout: 'pipe', stderr: 'pipe' }); + const timer = setTimeout(() => child.kill('SIGKILL'), 45000); + try { + const [code, stdout, stderr] = await Promise.all([child.exited, new Response(child.stdout).text(), new Response(child.stderr).text()]); + expect(code, stdout + stderr).toBe(0); + const observation = JSON.parse(fs.readFileSync(result, 'utf8')); + expect(fs.existsSync(output), `D19: report must exist before stopping; ${observation.summary}`).toBe(true); + expect(observation.outcome, JSON.stringify(observation)).toBe(terminal); + expect(observation.reviewCount).toBe(2); + expect(fs.readFileSync(output, 'utf8')).toBe(REPORT); + const recorded = fs.readFileSync(events, 'utf8').trim().split('\n').map(line => JSON.parse(line)); + expect(recorded.map(e => e.name)).toEqual(['streamed-heading', 'pending-question', 'answered-second', 'partial-report', 'complete-report', ...(terminal === 'plan_ready' ? ['valid-native-ready'] : [])]); + expect(fs.readFileSync(record, 'utf8').trim().split('\n').map(line => JSON.parse(line))).toEqual(['/plan-eng-review\r', '1\r']); + if (terminal === 'plan_ready') expect(observation.transcript.planReadyRequests.map(r => r.failed)).toEqual([true, false]); + } finally { clearTimeout(timer); child.kill('SIGKILL'); await child.exited; fs.rmSync(dir, { recursive: true, force: true }); } + })); +}, 50000); + +// Actual finalized CEO prose uses a relative output name, not the DevEx fallback's +// exact “Plan written to:” sentence. Its caller-owned file is checked independently. +test('captured completed CEO prose supports the ordinary summary path', () => { + const f = fixture(); + try { + f.transcript.assistantMessages = [{ + sessionId: f.transcript.calls[0]!.sessionId, + timestamp: "2026-09-08T17:58:52.319Z", + text: "CEO review complete. Here's the summary:\n\n**Plan:** Payment Processing — Test Coverage\n**Mode:** HOLD SCOPE | **Verdict:** CLEARED (with 4 additions)\n\n**4 gaps found and resolved through interactive review:**\n\n1. **CRITICAL** — \"assert correct receipt is generated\" is too vague. Spec: assert `charge_id`, `amount`, `currency` (at minimum) match the charge response.\n2. **WARNING** — \"fails clean\" doesn't name what's raised. Spec: `assert raises ` (resolve exact class from error hierarchy at implementation).\n3. **WARNING** — Retry count is implicit. Spec: `assert len(mock_stripe_call_history) == 2` explicitly in Test 2.\n4. **WARNING** — No test file path. Spec: resolve by convention-matching existing test structure.\n\nEverything else (architecture, security, data flow, performance, observability, deployment, trajectory) is clean. Section 11 skipped — no UI scope. Review output written to `gstack-test-plan-ceo-paired.md`." + }]; + expect(hasNativePlanCompletion(f.transcript, f.file, f.startedAt)).toBe(false); + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary')).toBe(true); + f.transcript.calls = []; + expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary')).toBe(false); + } finally { f.cleanup(); } +}); diff --git a/test/plan-count-fixture.test.ts b/test/plan-count-fixture.test.ts index a15a618db..b5f983a79 100644 --- a/test/plan-count-fixture.test.ts +++ b/test/plan-count-fixture.test.ts @@ -168,6 +168,7 @@ try { { name: 'design-direct', skillName: 'plan-design-review', prompt: PROMPT, mode: 'direct-finding' }, { name: 'design-batched', skillName: 'plan-design-review', prompt: PROMPT, mode: 'batched-finding' }, { name: 'failed-native', skillName: 'plan-design-review', prompt: PROMPT, mode: 'failed-call' }, + { name: 'native-permission-policy', skillName: 'plan-eng-review', prompt: PROMPT, mode: 'native-permission-policy', report: path.join(dir, 'native-policy-report.md') }, { name: 'permission-lifecycle', skillName: 'plan-design-review', prompt: PROMPT, mode: 'permission-lifecycle' }, { name: 'permission', skillName: 'plan-design-review', prompt: PROMPT, mode: 'permission' }, { name: 'missing-transcript', skillName: 'plan-design-review', prompt: PROMPT, mode: 'missing-transcript' }, @@ -189,13 +190,17 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { spawnSync } from 'node:child_process'; const record = (event) => fs.appendFileSync(process.env.FIXTURE_RECORD, JSON.stringify(event) + '\n'); +// Older fake menus used CR as a line separator. Emit actual terminal newlines; +// real cursor/erase behavior is exercised separately in pty-screen-session.test.ts. +const render = text => process.stdout.write(text.replace(/\r(?!\n)/g, '\r\n')); const sessionId = 'fixture-' + process.pid; const project = path.join(process.env.CLAUDE_CONFIG_DIR, 'projects', sessionId); fs.mkdirSync(project, { recursive: true }); const native = (role, content, extra = {}) => { if (process.env.FIXTURE_MODE === 'missing-transcript') return; fs.appendFileSync(path.join(project, sessionId + '.jsonl'), JSON.stringify({ - cwd: process.cwd(), sessionId, isSidechain: false, message: { role, content }, ...extra, + cwd: process.cwd(), sessionId, isSidechain: false, message: { role, content }, + ...(process.env.FIXTURE_MODE === 'native-permission-policy' ? { timestamp: new Date().toISOString() } : {}), ...extra, }) + '\n'); }; native('assistant', [{ type: 'text', text: 'Fixture CLI started.' }]); @@ -240,7 +245,7 @@ record({ ? 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'); +if (process.env.FIXTURE_MODE === 'exit') render('\x1b[?25lSTARTUP_DIAGNOSTIC fixture CLI booted\x1b[?25h\n'); process.stdin.setRawMode?.(true); let firstInput = true; let completion; @@ -254,12 +259,40 @@ process.stdin.on('data', (data) => { const input = data.toString('utf8'); record({ type: 'input', data: input }); if (!firstInput) { + if (process.env.FIXTURE_MODE === 'native-permission-policy') { + if (permissionStage === 'done') { record({ type: 'unexpected-policy-input', input }); return; } + selected += input.replace(/\r/g, ''); + if (input.includes('\r')) { + permissionStage = 'done'; + const label = nativeQuestions[0].options[Number(selected) - 1]?.label; + record({ type: 'native-policy-answer', selected, label }); + native('user', [{ type: 'tool_result', tool_use_id: 'question-' + callId, content: 'Answered.' }], + { toolUseResult: { answers: { [nativeQuestions[0].question]: label } } }); + // The answered menu survives a provisional heading and partial report. + // Neither may turn its permission wording into a fresh default1. + const summary = 'Completion Summary:\n- Architecture Review: file policy issue resolved\n'; + render('\n● ' + summary); + native('assistant', [{ type: 'text', text: summary }], { timestamp: new Date(Date.now() + 1).toISOString() }); + setTimeout(() => { + fs.writeFileSync(process.env.FIXTURE_EXPECTED_REPORT, '## GSTACK REVIEW REPORT\n'); + record({ type: 'partial-policy-report' }); + }, 3000); + completion = setTimeout(() => { + fs.writeFileSync(process.env.FIXTURE_EXPECTED_REPORT, '# Reviewed plan\n\n## GSTACK REVIEW REPORT\n\n' + + '| Review | Status | Findings |\n|---|---|---|\n| Eng Review | clean | policy resolved |\n\n' + + 'VERDICT: ENG CLEARED\n\nNO UNRESOLVED DECISIONS\n'); + record({ type: 'complete-policy-report' }); + render('\nGSTACK REVIEW REPORT\n'); + }, 6500); + } + return; + } if (process.env.FIXTURE_MODE === 'damaged-menu') { if (question === 0) record({ type: 'input-during-prose', input }); else if (input.includes('\r')) { record({ type: 'damaged-menu-answer', input }); answer(); - process.stdout.write('\nGSTACK REVIEW REPORT\n'); + render('\nGSTACK REVIEW REPORT\n'); } return; } @@ -267,14 +300,14 @@ process.stdin.on('data', (data) => { if (question === 0 && input === '\x1b[Z') { question = 1; record({ type: 'returned-to-unanswered' }); - process.stdout.write('\r← ☒ Routing setup ☐ Cross-project ✔ Submit →\r' + + render('\r← ☒ Routing setup ☐ Cross-project ✔ Submit →\r' + '│ Enable cross-project learnings?\r❯1.Enable cross-project\r2.Keep project-scoped\r'); } else if (question === 1) { selected += input.replace(/\r/g, ''); if (input.includes('\r')) { question = 2; record({ type: 'answered-remaining-tab', selected }); - process.stdout.write(submitPanel(true)); + render(submitPanel(true)); } } else if (question === 2 && input === '\r') { // Native JSONL can arrive only after the whole packet is submitted. @@ -285,7 +318,7 @@ process.stdin.on('data', (data) => { [submitQuestions[1].question]: submitQuestions[1].options[Number(selected) - 1]?.label, } } }); record({ type: 'submitted-both-answers' }); - process.stdout.write('\nGSTACK REVIEW REPORT\n'); + render('\nGSTACK REVIEW REPORT\n'); } else record({ type: 'unexpected-submit-input', question, input }); return; } @@ -299,22 +332,22 @@ process.stdin.on('data', (data) => { record({ type: 'mode-answer', selected, label }); native('user', [{ type: 'tool_result', tool_use_id: 'question-' + callId, content: 'Your questions have been answered.' }], { toolUseResult: { answers: { [modeQuestion.question]: label } } }); - process.stdout.write('\nGSTACK REVIEW REPORT\n'); + render('\nGSTACK REVIEW REPORT\n'); } return; } if (['direct-finding', 'failed-call'].includes(process.env.FIXTURE_MODE) && input.includes('\r')) { answer(); - process.stdout.write('\nGSTACK REVIEW REPORT\n'); + render('\nGSTACK REVIEW REPORT\n'); return; } if (process.env.FIXTURE_MODE === 'batched-finding' && input.includes('\r')) { if (question === 1) { question = 2; - process.stdout.write('\r☐Loading\rD2 — Define the loading state \r❯1.Add spinner\r2.Keep blank\r'); + render('\r☐Loading\rD2 — Define the loading state \r❯1.Add spinner\r2.Keep blank\r'); } else { answer(); - process.stdout.write('\nGSTACK REVIEW REPORT\n'); + render('\nGSTACK REVIEW REPORT\n'); } return; } @@ -322,33 +355,33 @@ process.stdin.on('data', (data) => { if (permissionStage === 'first') { record({ type: 'permission-grant', number: 1, input }); permissionStage = 'pending'; - process.stdout.write(filePermission()); // Same pending menu, appended redraw. + render(filePermission()); // Same pending menu, appended redraw. completion = setTimeout(() => { permissionStage = 'completed'; - process.stdout.write('\n⎿ Wrote320linesto../fixture/gstack-test-plan-design.md\n' + '·'.repeat(1600)); + render('\n⎿ Wrote320linesto../fixture/gstack-test-plan-design.md\n' + '·'.repeat(1600)); completion = setTimeout(() => { permissionStage = 'second'; - process.stdout.write(filePermission()); // New request, identical file/text. + render(filePermission()); // New request, identical file/text. }, 4500); }, 4500); } else if (permissionStage === 'second') { record({ type: 'permission-grant', number: 2, input }); permissionStage = 'question'; - process.stdout.write('\n⎿ Added2lines\n'); + render('\n⎿ Added2lines\n'); ask([questionMetadata('File policy', 'Do you want to create gstack-test-plan-design.md?', ['Yes', 'No'])]); - process.stdout.write('\n☐ File policy\nDo you want to create gstack-test-plan-design.md?\n❯1.Yes\n2.No\n' + + render('\n☐ File policy\nDo you want to create gstack-test-plan-design.md?\n❯1.Yes\n2.No\n' + 'Enter to select · ↑/↓ to navigate · Esc to cancel\n'); } else if (permissionStage === 'question') { record({ type: 'file-policy-answer', input }); native('user', [{ type: 'tool_result', tool_use_id: 'question-' + callId, content: 'Your question has been answered.' }], { toolUseResult: { answers: { [nativeQuestions[0].question]: input.startsWith('2') ? 'No' : 'Yes' } } }); permissionStage = 'done'; - process.stdout.write('\nGSTACK REVIEW REPORT\n'); + render('\nGSTACK REVIEW REPORT\n'); } else record({ type: 'unexpected-permission-input', stage: permissionStage, input }); return; } if (process.env.FIXTURE_MODE === 'permission' && input.includes('\r')) { - process.stdout.write('\nGSTACK REVIEW REPORT\n'); + render('\nGSTACK REVIEW REPORT\n'); return; } if (process.env.FIXTURE_MODE !== 'prerequisite') return; @@ -360,40 +393,40 @@ process.stdin.on('data', (data) => { const labels = ['Run /office-hours now', 'Skip — proceed with standard review']; if (process.env.FIXTURE_SKIP_INDEX === '1') labels.reverse(); ask([questionMetadata('Prerequisite', 'No design doc found. Run /office-hours first?', labels)]); - process.stdout.write('\r☐ Prerequisite\rNo design doc found. Run /office-hours first?\r❯1.' + labels[0] + '\r2.' + labels[1] + '\r'); + render('\r☐ Prerequisite\rNo design doc found. Run /office-hours first?\r❯1.' + labels[0] + '\r2.' + labels[1] + '\r'); return; } selected += input.replace(/\r/g, ''); if (input.includes('\r')) { answer(); record({ type: 'prerequisite-answer', selected }); - process.stdout.write(selected === process.env.FIXTURE_SKIP_INDEX ? '\nGSTACK REVIEW REPORT\n' : '\nWRONG_PREREQUISITE_CHOICE\n'); + render(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 === 'damaged-menu') { - process.stdout.write('☐Stripe event types\nWhich event should the handler accept?\n❯1.Specify one canonical event\n2.Accept all events\n' + + render('☐Stripe event types\nWhich event should the handler accept?\n❯1.Specify one canonical event\n2.Accept all events\n' + '·'.repeat(4200) + '\nMinimum required test cases:\n1.Happy path\n2.Email failure\n3.DB timeout\n4.Unknown event\n5.Unknown user\n❯1\n'); completion = setTimeout(() => { question = 1; ask([questionMetadata('Test scope', 'Should the plan include an integration test? ', ['Unit tests only for now (recommended)', 'Unit tests + one integration test', 'Integration test only'])]); - process.stdout.write('\n☐Test scope\nShould the plan include an integration test? \n' + + render('\n☐Test scope\nShould the plan include an integration test? \n' + '❯1.Unittestsonlyfornow(recommended)\n2Uni tsts + one integration test\n3.Integrationtestonly\n' + 'Enter to select · ↑/↓ to navigate · Esc to cancel\n'); }, 4100); return; } if (process.env.FIXTURE_MODE === 'damaged-submit') { - process.stdout.write(submitPanel(false)); + render(submitPanel(false)); return; } if (['late-mode', 'batched-mode'].includes(process.env.FIXTURE_MODE)) { if (process.env.FIXTURE_MODE === 'batched-mode') ask([modeQuestion, questionMetadata('Separate decision', 'Which fixture target should be used?', ['First target', 'Second target'])]); - process.stdout.write('\r☐ Review mode\r' + modeQuestion.question + '\r❯1.DX EXPANSION\r2.DX POLISH\r3.DX TRIAGE\r'); + render('\r☐ Review mode\r' + modeQuestion.question + '\r❯1.DX EXPANSION\r2.DX POLISH\r3.DX TRIAGE\r'); return; } if (['direct-finding', 'batched-finding', 'failed-call'].includes(process.env.FIXTURE_MODE)) { @@ -405,27 +438,33 @@ process.stdin.on('data', (data) => { const questions = [questionMetadata('Button style', 'D1 — How should the four header buttons differ? ', ['Filled primary', 'Ghost buttons'])]; if (process.env.FIXTURE_MODE === 'batched-finding') questions.push(questionMetadata('Loading', 'D2 — Define the loading state ', ['Add spinner', 'Keep blank'])); ask(questions); - process.stdout.write('\r☐Buttonstyle\r│D1—Howshouldthe4headerbuttonsbedifferentiated?\r❯1.Filledprimary\r2.Ghostbuttons\r'); + render('\r☐Buttonstyle\r│D1—Howshouldthe4headerbuttonsbedifferentiated?\r❯1.Filledprimary\r2.Ghostbuttons\r'); return; } if (process.env.FIXTURE_MODE === 'prerequisite') { question = 1; const suffix = process.env.FIXTURE_CUSTOM === 'true' ? ' Context '.repeat(50) + 'routing-proof-after-240' : ''; ask([questionMetadata('Setup', 'Which fixture setup should be used?' + suffix, ['First setup', 'Second setup'])]); - process.stdout.write('\r☐ Setup\rWhich fixture setup should be used?\r❯1.First setup\r2.Second setup\r'); + render('\r☐ Setup\rWhich fixture setup should be used?\r❯1.First setup\r2.Second setup\r'); + return; + } + if (process.env.FIXTURE_MODE === 'native-permission-policy') { + const prompt = 'D1 — Should we create a file that documents always allow access to the project? '; + ask([questionMetadata('File policy', prompt, ['Create it', 'Keep current policy'])]); + render('\n☐ File policy\n' + prompt + '\n❯1.Create it\n2.Keep current policy\nEnter to select · ↑/↓ to navigte · Esc to cancel\n'); return; } if (process.env.FIXTURE_MODE === 'permission-lifecycle') { - process.stdout.write(filePermission()); + render(filePermission()); return; } if (process.env.FIXTURE_MODE === 'permission') { - process.stdout.write('\rDo you want to create PLAN.md?\r❯1.Yes\r2.Yes, and switch to accept edits\r3.No\r'); + render('\rDo you want to create PLAN.md?\r❯1.Yes\r2.Yes, and switch to accept edits\r3.No\r'); return; } // Longer than the old 3 s delayed fixture send: record that regression // even if the helper would otherwise return on our completion marker. - completion = setTimeout(() => process.stdout.write('\nGSTACK REVIEW REPORT\n'), 4100); + completion = setTimeout(() => render('\nGSTACK REVIEW REPORT\n'), 4100); }); process.on('SIGINT', () => { clearTimeout(completion); @@ -456,16 +495,19 @@ const results = await Promise.all(cases.map(async (item) => ({ slashCommand: '/' + item.skillName, followUpPrompt: item.prompt, fixtureFiles: item.files, + expectedPlanPath: item.report, isLastStep0AUQ: () => false, isFirstReviewAUQ: ['direct-finding', 'batched-finding', 'failed-call'].includes(item.mode) ? designFirstReviewAUQ : undefined, isReviewAUQ: item.custom ? fp => fp.promptSnippet.includes('routing-proof-after-240') : undefined, - pickAUQ: ['late-mode', 'batched-mode'].includes(item.mode) ? devexReviewModePick + pickAUQ: item.mode === 'native-permission-policy' ? () => 2 + : ['late-mode', 'batched-mode'].includes(item.mode) ? devexReviewModePick : item.custom ? fp => fp.promptSnippet.includes('routing-proof-after-240') ? 1 : null : undefined, reviewCountCeiling: 8, timeoutMs: item.mode === 'permission-lifecycle' ? 22000 : 15000, firstAUQPick: () => ['late-mode', 'batched-mode'].includes(item.mode) ? 1 : 2, env: { FIXTURE_RECORD: item.record, FIXTURE_SKILL: item.skillName, FIXTURE_MODE: item.mode, + FIXTURE_EXPECTED_REPORT: item.report ?? '', FIXTURE_CUSTOM: String(item.custom ?? false), FIXTURE_SKIP_INDEX: String(item.skipIndex ?? ''), FIXTURE_CONFIG_BIN: ${JSON.stringify(path.join(ROOT, 'bin/gstack-config'))}, GSTACK_HOME: ${JSON.stringify(hostState)}, GSTACK_STATE_ROOT: ${JSON.stringify(hostState)}, @@ -528,7 +570,7 @@ await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify({ results, onboard : item.mode === 'permission-lifecycle' ? '1\r1\r2\r' : item.mode === 'damaged-submit' ? '\x1b[Z2\r\r' : item.mode === 'batched-finding' ? '2\r1\r' : item.mode === 'batched-mode' ? '1\r' - : ['direct-finding', 'failed-call', 'permission', 'late-mode', 'damaged-menu'].includes(item.mode) ? '2\r' : '')); + : ['direct-finding', 'failed-call', 'permission', 'late-mode', 'damaged-menu', 'native-permission-policy'].includes(item.mode) ? '2\r' : '')); expect(fs.existsSync(startup.cwd)).toBe(false); expect(fs.existsSync(startup.stateRoot)).toBe(false); expect(() => process.kill(startup.pid, 0)).toThrow(); @@ -559,6 +601,15 @@ await Bun.write(${JSON.stringify(resultPath)}, JSON.stringify({ results, onboard expect(result.observation.transcript.calls[0].failure).toContain('is_error'); expect(result.observation.transcript.calls[0].answered).toBe(false); } + if (item.mode === 'native-permission-policy') { + expect(events.filter(event => event.type === 'unexpected-policy-input')).toEqual([]); + expect(events.filter(event => event.type.endsWith('-policy-report')).map(event => event.type)) + .toEqual(['partial-policy-report', 'complete-policy-report']); + expect(fs.readFileSync(item.report!, 'utf8')).toContain('NO UNRESOLVED DECISIONS'); + expect(events.filter(event => event.type === 'native-policy-answer').map(event => event.selected)).toEqual(['2']); + expect(result.observation.transcript.calls).toHaveLength(1); + expect(Object.values(result.observation.transcript.calls[0].answers)).toEqual(['Keep current policy']); + } if (item.mode === 'permission-lifecycle') { expect(events.filter(event => event.type === 'unexpected-permission-input')).toEqual([]); expect(events.filter(event => event.type === 'permission-grant').map(event => event.input)).toEqual(['1\r', '1\r']); diff --git a/test/plan-count-transcript.test.ts b/test/plan-count-transcript.test.ts index f4cb7ff84..3cc5a591c 100644 --- a/test/plan-count-transcript.test.ts +++ b/test/plan-count-transcript.test.ts @@ -218,3 +218,35 @@ describe('native plan-count transcripts', () => { expect(f.read().calls).toEqual([]); }); }); + +describe('native plan approval request evidence', () => { + test('captures only a scoped complete ExitPlanMode request and retains error results', () => { + const f = fixture(); + // Exact native tool shape from G Design's pending approval gate (its + // zero-question workflow remains a separate failed count observation). + const ready = f.record('assistant', [{ type: 'tool_use', id: 'plan-ready', name: 'ExitPlanMode', input: {}, caller: { type: 'direct' } }]); + const line = JSON.stringify(ready); + fs.writeFileSync(f.file, line); + expect(f.read().planReadyRequests).toBeUndefined(); + fs.appendFileSync(f.file, '\n'); + expect(f.read().planReadyRequests).toEqual([{ sessionId: 'session-a', toolUseId: 'plan-ready', timestamp: ready.timestamp, failed: false }]); + f.append(f.record('user', [{ type: 'tool_result', tool_use_id: 'plan-ready', is_error: true, content: 'Plan not accepted' }])); + expect(f.read().planReadyRequests![0]!.failed).toBe(true); + f.append(ready); // duplicate request cannot clear a recorded failure + expect(f.read().planReadyRequests![0]!.failed).toBe(true); + }); + test('foreign, sidechain, quoted, lookalike and untimestamped requests add no readiness', () => { + const f = fixture(); + const block = { type: 'tool_use', id: 'ready', name: 'ExitPlanMode', input: {} }; + f.append(f.record('assistant', [block], { cwd: f.cwd + '-other' }), + f.record('assistant', [block], { sessionId: 'foreign' }), + f.record('assistant', [block], { isSidechain: true }), + f.record('assistant', [block], { timestamp: 'invalid' }), + f.record('assistant', [{ ...block, name: 'example_ExitPlanMode' }]), + f.record('assistant', [{ type: 'text', text: JSON.stringify(block) }])); + expect(f.read().planReadyRequests).toBeUndefined(); + fs.appendFileSync(f.file, 'bad JSON\n'); + expect(f.read().status).toBe('error'); + expect(f.read().planReadyRequests).toBeUndefined(); + }); +}); diff --git a/test/pty-screen-session.test.ts b/test/pty-screen-session.test.ts new file mode 100644 index 000000000..b8399dd3e --- /dev/null +++ b/test/pty-screen-session.test.ts @@ -0,0 +1,149 @@ +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 { spawnSync } from 'node:child_process'; +import { pathToFileURL } from 'node:url'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const runner = pathToFileURL(path.join(ROOT, 'test/helpers/claude-pty-runner.ts')).href; + +function fixture() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'pty-screen-session-')); + const fake = path.join(dir, 'fake-claude'); + const dispatched = path.join(dir, 'dispatched'); + fs.writeFileSync(fake, `#!${process.execPath}\n` + ` +import * as fs from 'node:fs'; +fs.writeFileSync(${JSON.stringify(dispatched)}, 'started'); +process.stdin.setRawMode?.(true); +process.stdout.write('\\x1b[?1049h\\x1b[2J\\x1b[HDo you want to create plan.md?\\r\\n❯1.Yes\\r\\n2.No\\r\\nEsc to cancel · Tab to amend'); +process.stdin.on('data', data => { + const input = data.toString(); + if(input.includes('exit')) process.exit(0); + if(input.includes('1')) process.stdout.write('\\x1b[2J\\x1b[H⎿ Wrote 44 lines\\r\\n❯ '); +}); +setInterval(() => {}, 1000); +`, { mode: 0o755 }); + return { dir, fake, dispatched }; +} +function run(dir: string, fake: string, source: string) { + const worker = path.join(dir, 'worker.ts'); + fs.writeFileSync(worker, source); + return spawnSync(process.execPath, [worker], { cwd: ROOT, encoding: 'utf8', timeout: 12_000, + env: { ...process.env, BROWSE_TERMINAL_BINARY: fake, EVALS_HERMETIC: '1' } }); +} + +describe.skipIf(process.platform === 'win32')('real fake-PTY viewport lifecycle', () => { + test('a completed Write clears actionable permission but preserves raw history and final screen after exit', () => { + const f = fixture(); + try { + const result = run(f.dir, f.fake, ` +import { launchClaudePty, capturePlanCountQuestion, isPermissionDialogVisible } from ${JSON.stringify(runner)}; +const session = await launchClaudePty({ cwd:${JSON.stringify(f.dir)}, observeScreen:true, cols:60, rows:10, timeoutMs:5000 }); +const until = async (predicate) => { for(let i=0;i<100;i++){const screen=await session.currentScreen();if(predicate(screen))return screen;await Bun.sleep(20);}throw new Error('fixture screen timeout'); }; +try { + const active=await until(s=>s.includes('Do you want')); + if(!isPermissionDialogVisible(active))throw new Error('active permission lost'); + session.send('1\\r'); + const completed=await until(s=>s.includes('Wrote 44 lines')); + if(completed.includes('Do you want') || capturePlanCountQuestion(completed,new Set(),0,false))throw new Error('stale permission remained actionable'); + if(!session.rawOutput().includes('Do you want') || !session.visibleText().includes('Do you want'))throw new Error('raw history lost'); + if(completed.split('\\n').length!==10)throw new Error('wrong terminal dimensions'); + session.send('exit\\r'); + for(let i=0;i<100 && !session.exited();i++)await Bun.sleep(20); + if(!session.exited())throw new Error('fake did not exit'); + if(await session.currentScreen()!==completed)throw new Error('final screen changed after disposal'); + process.stdout.write('screen-cleared;raw-retained;natural-exit-disposed'); +} finally { await session.close(); await session.close(); } +`); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toBe('screen-cleared;raw-retained;natural-exit-disposed'); + } finally { fs.rmSync(f.dir, { recursive: true, force: true }); } + }, 15_000); + + test('wall timeout disposes its screen and non-opted-in callers retain their existing raw API', () => { + const f = fixture(); + try { + const result = run(f.dir, f.fake, ` +import { launchClaudePty } from ${JSON.stringify(runner)}; +const timed=await launchClaudePty({cwd:${JSON.stringify(f.dir)},observeScreen:true,cols:60,rows:10,timeoutMs:150}); +try { + for(let i=0;i<100 && !timed.exited();i++)await Bun.sleep(20); + if(!timed.exited())throw new Error('wall timeout did not stop child'); + if((await timed.currentScreen()).split('\\n').length!==10)throw new Error('final timeout viewport unavailable'); +} finally {await timed.close();} +const legacy=await launchClaudePty({cwd:${JSON.stringify(f.dir)},timeoutMs:2000}); +try { + let rejected=false;try {await legacy.currentScreen();}catch(error){rejected=String(error).includes('not enabled');} + if(!rejected)throw new Error('legacy opted in unexpectedly'); + await legacy.waitFor('Do you want',{timeoutMs:1500,pollMs:20}); + if(!legacy.visibleText().includes('Do you want'))throw new Error('legacy raw API changed'); + process.stdout.write('timeout-disposed;legacy-unchanged'); +}finally{await legacy.close();} +`); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toBe('timeout-disposed;legacy-unchanged'); + } finally { fs.rmSync(f.dir, { recursive: true, force: true }); } + }, 15_000); + + test('a full native summary still completes after its heading scrolls beyond forty rows', async () => { + const f = fixture(); + const output = path.join(f.dir, 'plan.md'); + const report = '# Reviewed plan\n\n## GSTACK REVIEW REPORT\n\n| Review | Status | Findings |\n|---|---|---|\n| CEO Review | clean | resolved |\n\nVERDICT: CEO CLEARED\n\nNO UNRESOLVED DECISIONS\n'; + fs.writeFileSync(f.fake, `#!${process.execPath}\n` + ` +import * as fs from 'node:fs'; +import * as path from 'node:path'; +const id='long-native-summary'; +const dir=path.join(process.env.CLAUDE_CONFIG_DIR,'projects',id);fs.mkdirSync(dir,{recursive:true}); +const native=(role,content,extra={})=>fs.appendFileSync(path.join(dir,id+'.jsonl'),JSON.stringify({cwd:process.cwd(),sessionId:id,isSidechain:false,timestamp:new Date().toISOString(),message:{role,content},...extra})+'\\n'); +let sent=false;process.stdin.setRawMode?.(true); +process.stdin.on('data',()=>{if(sent)return;sent=true; + const q={header:'Architecture',question:'Fix the independent finding?',options:[{label:'Fix it'},{label:'Defer'}]}; + native('assistant',[{type:'tool_use',id:'finding',name:'AskUserQuestion',input:{questions:[q]}}]); + native('user',[{type:'tool_result',tool_use_id:'finding',content:'Answered.'}],{timestamp:new Date(Date.now()-100).toISOString(),toolUseResult:{answers:{[q.question]:'Fix it'}}}); + fs.writeFileSync(${JSON.stringify(output)},${JSON.stringify(report)}); + const text='## Completion Summary\\n'+Array.from({length:48},(_,i)=>'- Finding '+(i+1)+': examined and resolved.').join('\\n')+'\\nNo unresolved issues.'; + native('assistant',[{type:'text',text}],{timestamp:new Date(Date.now()+5).toISOString()}); + process.stdout.write('\\x1b[2J\\x1b[H'+text.replace(/\\n/g,'\\r\\n')+'\\r\\n❯ '); +});process.stdin.resume(); +`, {mode:0o755}); + const worker = path.join(f.dir, 'long-summary-worker.ts'); + fs.writeFileSync(worker, `import {runPlanSkillCounting} from ${JSON.stringify(runner)}; +const result=await runPlanSkillCounting({skillName:'plan-ceo-review',slashCommand:'/plan-ceo-review',followUpPrompt:'# Long summary fixture',expectedPlanPath:${JSON.stringify(output)},isLastStep0AUQ:()=>false,isReviewAUQ:()=>true,reviewCountCeiling:8,timeoutMs:15000}); +if(result.outcome!=='completion_summary'||result.reviewCount!==1||result.evidence.includes('## Completion Summary'))throw new Error(JSON.stringify(result)); +process.stdout.write('native-summary-complete-after-heading-scrolled');`); + const child = Bun.spawn([process.execPath, worker], {cwd:ROOT,env:{...process.env,EVALS_HERMETIC:'1',EVALS_RUN_ID:'',BROWSE_TERMINAL_BINARY:f.fake},stdout:'pipe',stderr:'pipe'}); + const timer=setTimeout(()=>child.kill('SIGKILL'),30000); + try { + const [code,stdout,stderr]=await Promise.all([child.exited,new Response(child.stdout).text(),new Response(child.stderr).text()]); + expect(code,stderr).toBe(0); + expect(stdout).toBe('native-summary-complete-after-heading-scrolled'); + } finally {clearTimeout(timer);child.kill('SIGKILL');fs.rmSync(f.dir,{recursive:true,force:true});} + }, 32000); + + test('missing installed headless source fails explicitly before the CLI can spawn', () => { + const f = fixture(); + try { + const copied = path.join(f.dir, 'missing-headless.ts'); + fs.copyFileSync(path.join(ROOT, 'test/helpers/pty-screen.ts'), copied); + const packageDir = path.join(f.dir, 'node_modules/xterm'); + fs.mkdirSync(packageDir, { recursive: true }); + fs.writeFileSync(path.join(packageDir, 'package.json'), '{"name":"xterm","version":"fixture"}'); + const clone = path.join(f.dir, 'launch-with-missing-screen.ts'); + const source = fs.readFileSync(path.join(ROOT, 'test/helpers/claude-pty-runner.ts'), 'utf8') + .replace(/from (['"])(\.\.?\/[^'"]+)\1/g, (_match, _quote, relative) => { + const target = relative === './pty-screen' ? copied : path.resolve(ROOT, 'test/helpers', relative + '.ts'); + return 'from ' + JSON.stringify(pathToFileURL(target).href); + }); + fs.writeFileSync(clone, source); + const result = run(f.dir, f.fake, ` +const {launchClaudePty}=await import(${JSON.stringify(pathToFileURL(clone).href)}); +try {const unexpected=await launchClaudePty({cwd:${JSON.stringify(f.dir)},observeScreen:true});await unexpected.close();throw new Error('launch unexpectedly succeeded');} +catch(error){if(!String(error).includes('PTY screen unavailable') || !String(error.cause).includes('headless source is missing'))throw error;process.stdout.write('failed-before-CLI-spawn');} +`); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toBe('failed-before-CLI-spawn'); + expect(fs.existsSync(f.dispatched)).toBe(false); + } finally { fs.rmSync(f.dir, { recursive: true, force: true }); } + }, 15_000); +}); diff --git a/test/pty-screen.test.ts b/test/pty-screen.test.ts new file mode 100644 index 000000000..12569cb21 --- /dev/null +++ b/test/pty-screen.test.ts @@ -0,0 +1,175 @@ +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 { createPtyScreen } from './helpers/pty-screen'; +import { capturePlanCountQuestion, createPlanCountPermissionGuard, isPermissionDialogVisible, matchesNativePlanQuestion, nativePlanCallFingerprint, stripAnsi } from './helpers/claude-pty-runner'; +import { autoplanRoutingSetupInput } from './helpers/autoplan-setup-question'; +import { createPlanCountSnapshotWriter } from './helpers/plan-count-artifacts'; +import { pickCeoCompletionHandoff } from './helpers/ceo-completion-handoff'; +import type { NativePlanQuestionCall } from './helpers/plan-count-transcript'; + +function seed(frame: { initial: string[]; cursor: { x: number; y: number } }): string { + return '\x1b[?1049h\x1b[2J' + frame.initial.map((line, i) => `\x1b[${i + 1};1H${line}\x1b[K`).join('') + + `\x1b[${frame.cursor.y + 1};${frame.cursor.x + 1}H`; +} +const fixture = (name: string) => JSON.parse(fs.readFileSync(path.join(import.meta.dir, 'fixtures/pty-screen', name + '.json'), 'utf8')); + +describe('owned PTY viewport', () => { + test('preserves cursor positioning, carriage return, erase, and alternate-screen restoration', async () => { + const screen = await createPtyScreen(20, 4); + try { + screen.write('normal'); + screen.write('\x1b[?1049h\x1b[2J\x1b[Habcdef\rX\x1b[4G!\x1b[K'); + expect((await screen.read()).split('\n')[0]).toBe('Xbc!'); + screen.write('\x1b[2;1Hsecond\x1b[2J\x1b[Hready'); + expect(await screen.read()).toBe('ready\n\n\n'); + screen.write('\x1b[?1049l'); + expect((await screen.read()).split('\n')[0]).toBe('normal'); + } finally { await screen.dispose(); } + }); + + test('uses the requested dimensions and excludes scrollback', async () => { + const screen = await createPtyScreen(12, 3); + try { + screen.write('old permission\r\n1\r\n2\r\n3\r\n4'); + const current = await screen.read(); + expect(current.split('\n')).toHaveLength(3); + expect(current).not.toContain('old permission'); + expect(current).toBe('2\n3\n4'); + } finally { await screen.dispose(); } + }); + + test('sessions isolate state and dispose flushes pending writes once without changing navigator', async () => { + const navigatorBefore = globalThis.navigator; + const a = await createPtyScreen(30, 4); + const b = await createPtyScreen(30, 4); + a.write('first'); b.write('second'); + const closing = a.dispose(); + await closing; + expect(await a.read()).toBe('first\n\n\n'); + expect(await b.read()).toBe('second\n\n\n'); + expect(a.dispose()).toBe(closing); + expect(() => a.write('late')).toThrow('disposed'); + await b.dispose(); + expect(globalThis.navigator).toBe(navigatorBefore); + }); + + test('retains a long genuine question when its options are visible and its header has scrolled away', async () => { + const screen = await createPtyScreen(120, 40); + try { + screen.write('☐ API consistency\r\n' + 'Existing context for this independent issue.\r\n'.repeat(45) + + 'Which argument order should run_eval/run_batch use?\r\n❯1.Use the same argument order\r\n2.Keep the inconsistent order\r\n' + + 'Enter to select · ↑/↓ to navigate · Esc to cancel'); + const current = await screen.read(); + expect(current).not.toContain('☐ API consistency'); + const question = capturePlanCountQuestion(current, new Set(), 0, false); + expect(question).not.toBeNull(); + expect(question?.options[0].label).toBe('Use the same argument order'); + expect(question?.options).toHaveLength(2); + } finally { await screen.dispose(); } + }); + + test('clipped native handoff preserves its bound manual choice in either option order', async () => { + for (const contextLines of [36, 41]) for (const reverse of [false, true]) { + const options = [{ label: 'Run /plan-eng-review next' }, { label: "Done — I'll handle reviews manually" }]; + if (reverse) options.reverse(); + const call: NativePlanQuestionCall = { sessionId: 'long-handoff', toolUseId: 'pending', answered: false, + questions: [{ header: 'Next review', + question: 'CEO review is complete. Which next review should run? \n' + + Array.from({ length: contextLines }, (_, i) => `Review context line ${i + 1}: the existing requirements remain approved.`).join('\n'), + options }] }; + const q = call.questions[0]!; + const screen = await createPtyScreen(120, 40); + try { + screen.write(`☐ ${q.header}\r\n${q.question.replace(/\n/g, '\r\n')}\r\n` + + `❯ 1. ${options[0]!.label}\r\n 2. ${options[1]!.label}\r\nEnter to select · ↑/↓ to navigate · Esc to cancel`); + const current = await screen.read(); + expect(current).not.toContain('☐ Next review'); + expect(current.includes('(); + const capture = capturePlanCountQuestion(current, seen, 0, false, call); + expect(capture?.nativeCall).toBe(call); + expect(pickCeoCompletionHandoff(nativePlanCallFingerprint(call, 0, false), capture!)).toBe(reverse ? 1 : 2); + expect(capturePlanCountQuestion(current, seen, 1, false, call)).toBeNull(); + for (const unrelated of [ + // Same choices are not identity, even with an intact native footer. + current.slice(current.indexOf('❯ 1.')), + current.replace('Review context line', 'Unrelated context line'), + 'A different question with shared context?\n' + current, + current.replace('2. ' + options[1]!.label, '2. Approve a new implementation task'), + current.replace('↑/↓ to navigate', '↑/↓ to navigte'), + current + '\nDo you want to create another.md?\n❯ 1. Yes\n2. No\nEsc to cancel · Tab to amend', + ]) { + expect(matchesNativePlanQuestion(unrelated, call)).toBe(false); + const other = capturePlanCountQuestion(unrelated, new Set(), 0, false, call); + expect(other?.nativeCall).toBeUndefined(); + if (other) expect(pickCeoCompletionHandoff(nativePlanCallFingerprint(call, 0, false), other)).toBeNull(); + } + expect(capturePlanCountQuestion(current, new Set(), 0, false, { ...call, failed: true })?.nativeCall).toBeUndefined(); + } finally { await screen.dispose(); } + } + }); + + for (const chunkSize of [37, 4096]) { + test(`captured routing redraw recovers the intact manual option (${chunkSize}-character chunks)`, async () => { + const frame = fixture('autoplan'); + const screen = await createPtyScreen(frame.cols, frame.rows); + try { + screen.write(seed(frame)); + for (let i = 0; i < frame.update.length; i += chunkSize) screen.write(frame.update.slice(i, i + chunkSize)); + const current = await screen.read(); + expect(current.split('\n').map(line => line.trimEnd())).toEqual(frame.expected.map((line: string) => line.trimEnd())); + expect(current).toContain("2. No thanks, I'll invoke skills manually"); + expect(stripAnsi(frame.update)).toContain("2. N thanks, I'll invokeskillsmanually"); + expect(autoplanRoutingSetupInput(current, new Set())).toBe('1\r'); + } finally { await screen.dispose(); } + }); + } + + test('historical file results release a new current permission but cannot activate stale scrollback', () => { + const menu = 'Do you want to create plan.md?\n❯1.Yes\n2.No\nEsc to cancel · Tab to amend'; + const completed = menu + '\n⎿ Wrote 44 lines'; + const guard = createPlanCountPermissionGuard(); + expect(guard(menu, menu)).toBe('grant'); + expect(guard(menu, menu)).toBe('handled'); + expect(guard('⎿ Wrote 44 lines\n❯ ', completed)).toBeNull(); + // The actual new menu has no old Write in its viewport. The separate + // history establishes a new completed-file epoch for this same target. + expect(guard(menu, completed + '\n' + menu)).toBe('grant'); + expect(guard(menu, completed + '\n' + menu)).toBe('handled'); + expect(guard('❯ ', completed + '\n' + menu)).toBeNull(); + expect(guard(menu + '\n⎿ Wrote 1 line', completed + '\n' + menu + '\n⎿ Wrote 1 line')).toBe('handled'); + }); + + test('captured successful Write removes the old permission; raw and viewport evidence stay separate', async () => { + const frame = fixture('design'); + const screen = await createPtyScreen(frame.cols, frame.rows); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'screen-evidence-')); + try { + const raw = seed(frame) + frame.update; + screen.write(raw); + const active = await screen.read(); + expect(active.split('\n').map(line => line.trimEnd())).toEqual(frame.expected.map((line: string) => line.trimEnd())); + expect(active).toContain('Do you want to create gstack-test-plan-design.md?'); + expect(isPermissionDialogVisible(active)).toBe(true); + screen.write(frame.completed); + const current = await screen.read(); + expect(current).toContain('Wrote 44 lines'); + expect(current).not.toContain('Do you want'); + expect(isPermissionDialogVisible(current)).toBe(false); + expect(capturePlanCountQuestion(current, new Set(), 0, false)).toBeNull(); + const preservedRaw = raw + frame.completed; + expect(stripAnsi(preservedRaw)).toContain('Do you wat'); + const native = { calls: [{ toolUseId: 'native-question', answered: true }] }; + const save = createPlanCountSnapshotWriter({ EVALS_RUN_ID: 'screen-proof', GSTACK_EVAL_DIR: dir }); + const saved = save({ skillName: 'plan-design-review', cwd: dir, claudeConfigDir: null, + raw: preservedRaw, visible: stripAnsi(preservedRaw), viewport: current, observation: { native } }); + expect(saved.artifactError).toBeUndefined(); + expect(fs.readFileSync(path.join(saved.artifactDir!, 'terminal.raw.log'), 'utf8')).toBe(preservedRaw); + expect(fs.readFileSync(path.join(saved.artifactDir!, 'terminal.screen.log'), 'utf8')).toBe(current); + expect(JSON.parse(fs.readFileSync(path.join(saved.artifactDir!, 'observation.json'), 'utf8')).native).toEqual(native); + } finally { await screen.dispose(); fs.rmSync(dir, { recursive: true, force: true }); } + }); +}); diff --git a/test/skill-e2e-autoplan-chain.test.ts b/test/skill-e2e-autoplan-chain.test.ts index bae782427..030db1e2f 100644 --- a/test/skill-e2e-autoplan-chain.test.ts +++ b/test/skill-e2e-autoplan-chain.test.ts @@ -79,12 +79,14 @@ describeE2E('/autoplan chain ordering (periodic)', () => { cwd: tempDir, timeoutMs: 1_080_000, // 18 min, slightly above test budget seedSkills: true, + observeScreen: true, }); let hits: AutoplanPhaseHit[] = []; let transcript: PlanCountTranscript = { status: 'missing', calls: [], assistantMessages: [] }; let outcome: 'chain_complete' | 'plan_ready' | 'timeout' | 'exited' = 'timeout'; let evidence = ''; + let viewport = ''; let fullSessionEvidence = ''; let exitCode: number | null = null; let commandStartedAt = Date.now(); @@ -99,14 +101,14 @@ describeE2E('/autoplan chain ordering (periodic)', () => { const capture = (state: string) => { artifacts = saveSnapshot({ skillName: 'autoplan', cwd: tempDir, claudeConfigDir: session.hermeticConfigDir, - raw: session.rawOutput(), visible: session.visibleText(), + raw: session.rawOutput(), visible: session.visibleText(), viewport, observation: { state, hits, native: transcript, exitCode: session.exitCode() }, }); }; try { await Bun.sleep(8000); - const since = session.mark(); + session.mark(); commandStartedAt = Date.now(); session.send('/autoplan\r'); @@ -117,6 +119,7 @@ describeE2E('/autoplan chain ordering (periodic)', () => { const seenSetupQuestions = new Set(); while (Date.now() - start < budgetMs) { await Bun.sleep(5000); + viewport = await session.currentScreen(); observe(); if (Date.now() - lastCheckpointAt >= 30_000) { capture('in_progress'); @@ -124,10 +127,10 @@ describeE2E('/autoplan chain ordering (periodic)', () => { } if (session.exited()) { outcome = 'exited'; - evidence = session.visibleSince(since).slice(-3000); + evidence = viewport.slice(-3000); break; } - const visible = session.visibleSince(since); + const visible = viewport; // Auto-grant any permission dialog so autoplan can keep moving // through its phases. The autoplan template auto-decides review @@ -172,11 +175,13 @@ describeE2E('/autoplan chain ordering (periodic)', () => { } finally { // Preserve boot failures omitted by mark(), and observe the real exit // status before close() deliberately terminates a live session. - exitCode = session.exitCode(); - fullSessionEvidence = diagnosticTail(session.visibleText()); - observe(); - capture(outcome); - await session.close(); + try { + exitCode = session.exitCode(); + viewport = await session.currentScreen(); + fullSessionEvidence = diagnosticTail(session.visibleText()); + observe(); + capture(outcome); + } finally { await session.close(); } } if (outcome === 'exited' || outcome === 'timeout') { diff --git a/test/skill-e2e-plan-ceo-finding-count.test.ts b/test/skill-e2e-plan-ceo-finding-count.test.ts index 77cef2483..3931bbfce 100644 --- a/test/skill-e2e-plan-ceo-finding-count.test.ts +++ b/test/skill-e2e-plan-ceo-finding-count.test.ts @@ -152,6 +152,7 @@ describeE2E('/plan-ceo-review per-finding AskUserQuestion count (periodic)', () skillName: 'plan-ceo-review', slashCommand: '/plan-ceo-review', followUpPrompt: planCeo5Findings(planPath), + expectedPlanPath: planPath, isLastStep0AUQ: ceoStep0Boundary, isFirstReviewAUQ: ceoFirstReviewAUQ, isCompletionHandoffAUQ: isCeoCompletionHandoff, @@ -238,6 +239,7 @@ describeE2E('/plan-ceo-review per-finding AskUserQuestion count (periodic)', () skillName: 'plan-ceo-review', slashCommand: '/plan-ceo-review', followUpPrompt: planCeo2PairedFindings(planPath), + expectedPlanPath: planPath, isLastStep0AUQ: ceoStep0Boundary, isFirstReviewAUQ: ceoFirstReviewAUQ, isCompletionHandoffAUQ: isCeoCompletionHandoff, diff --git a/test/skill-e2e-plan-design-finding-count.test.ts b/test/skill-e2e-plan-design-finding-count.test.ts index fcdcc573e..07a90070b 100644 --- a/test/skill-e2e-plan-design-finding-count.test.ts +++ b/test/skill-e2e-plan-design-finding-count.test.ts @@ -171,6 +171,7 @@ describeE2E('/plan-design-review per-finding AskUserQuestion count (periodic)', skillName: 'plan-design-review', slashCommand: '/plan-design-review', followUpPrompt: planDesign5Findings(planPath), + expectedPlanPath: planPath, isLastStep0AUQ: designStep0Boundary, isFirstReviewAUQ: designFirstReviewAUQ, fixtureFiles: { 'DESIGN.md': designSystem }, diff --git a/test/skill-e2e-plan-eng-finding-count.test.ts b/test/skill-e2e-plan-eng-finding-count.test.ts index 517f06e0b..18f9cbd83 100644 --- a/test/skill-e2e-plan-eng-finding-count.test.ts +++ b/test/skill-e2e-plan-eng-finding-count.test.ts @@ -83,6 +83,7 @@ describeE2E('/plan-eng-review per-finding AskUserQuestion count (periodic)', () skillName: 'plan-eng-review', slashCommand: '/plan-eng-review', followUpPrompt: planEng5Findings(planPath), + expectedPlanPath: planPath, isLastStep0AUQ: engStep0Boundary, isSetupAUQ: engSetupAUQ, isFirstReviewAUQ: engFirstReviewAUQ,