From d0d8cb2db662d0010ed33820a68a55e45871f557 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 26 May 2026 22:27:51 -0700 Subject: [PATCH] test(e2e): split-overflow regression for /plan-ceo-review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Periodic-tier E2E test that catches the original failure mode the user complained about: 5+ options for ONE decision must split into N sequential AskUserQuestion calls, not drop one to fit Conductor's 4-option cap. Fixture: 5 independent chat-platform integration candidates (Slack/Discord/Teams/Telegram/Mattermost), each carrying its own include/defer/cut decision. Floor = 4 review-phase AUQs (standard [N-1] tolerance band). Pre-fix "drop to 4 + 1 dropped" fails this floor. Wired into test/helpers/touchfiles.ts: tier periodic, depends on plan-ceo-review/**, the new preamble subsection, the question-pref binary (for the carve-out), and the runner helper. touchfiles.test.ts expected count bumped 21 → 22 to account for the new entry. Cost: ~$0.30/run when EVALS_TIER=periodic. Skips silently otherwise. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/fixtures/forcing-finding-seeds.ts | 56 +++++++++ test/helpers/touchfiles.ts | 2 + .../skill-e2e-plan-ceo-split-overflow.test.ts | 108 ++++++++++++++++++ test/touchfiles.test.ts | 8 +- 4 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 test/skill-e2e-plan-ceo-split-overflow.test.ts diff --git a/test/fixtures/forcing-finding-seeds.ts b/test/fixtures/forcing-finding-seeds.ts index 8d52c858d..14a41f72a 100644 --- a/test/fixtures/forcing-finding-seeds.ts +++ b/test/fixtures/forcing-finding-seeds.ts @@ -120,3 +120,59 @@ export const FORCING_BATCHING_ENG = [ 'iterate the payload to recompute the dependency graph. Could cache the', 'graph on the first attempt; not planned.', ].join('\n'); + +/** + * Split-overflow regression seed (periodic tier). + * + * Catches the original failure mode the user complained about: when the + * agent has 5+ options for ONE conceptual decision, it must split into N + * sequential AskUserQuestion calls (or batch into compatible ≤4-groups), + * NOT drop an option arbitrarily to fit Conductor's 4-option cap. + * + * Fixture shape: 5 independent platform-integration candidates for ONE + * scope decision. Each is independent (no dependencies between them) so + * the natural compliant shape is a per-option split chain at parent D. + * + * Used by test/skill-e2e-plan-ceo-split-overflow.test.ts to assert the + * agent fires >= 4 review-phase AUQs (floor uses the standard [N-1] + * tolerance band, accounting for one expected scope-reduction-or-merge + * call before the per-option chain begins). + * + * Pre-fix behavior: agent fires 1 AUQ with 4 options, "trims" the 5th + * via prose ("E5 is the largest lift and a natural follow-up; moving to + * TODOs without asking"). That's the bug. Floor of 4 detects it. + */ +export const FORCING_SPLIT_OVERFLOW_CEO = [ + 'Please review this plan and help me decide scope. Write your plan-mode plan to /tmp/gstack-test-plan-ceo-split-overflow.md (use Edit/Write to that exact path).', + '', + '# Plan: Pick which chat-platform integrations to ship this quarter', + '', + 'We have engineering bandwidth for at most 2-3 integrations this quarter.', + 'I need your help deciding which to prioritize. Below are 5 candidates,', + 'each fully independent of the others (no shared infrastructure, no', + 'dependencies between them). For each, the user can independently decide:', + 'include in this scope, defer to next quarter, or cut entirely.', + '', + '## E1) Slack — DM bot for incident alerts', + 'Build cost: ~2 weeks. Existing Slack auth flow we can reuse. High user', + 'demand (top customer request in Q2 survey, ~40% of asks).', + '', + '## E2) Discord — guild bot for community channels', + 'Build cost: ~3 weeks. Greenfield integration, no existing auth. Medium', + 'demand (~15% of asks, but loud community).', + '', + '## E3) Microsoft Teams — webhook + bot framework', + 'Build cost: ~4 weeks. Enterprise customers specifically asked for this.', + 'Highest revenue impact per user but smallest user count (~5% of asks).', + '', + '## E4) Telegram — bot API integration', + 'Build cost: ~1 week. Simplest API surface. Low strategic value but', + 'cheap win (~8% of asks, mostly from international users).', + '', + '## E5) Mattermost — REST plugin', + 'Build cost: ~2 weeks. Self-hosted enterprise users. Niche but locked-in', + 'segment (~3% of asks but all from high-ARR accounts).', + '', + 'Please walk me through each candidate and help me decide include/defer/cut', + 'per option. I want individual decisions per candidate, not a bundled pick.', +].join('\n'); diff --git a/test/helpers/touchfiles.ts b/test/helpers/touchfiles.ts index e25c58399..cda37a65e 100644 --- a/test/helpers/touchfiles.ts +++ b/test/helpers/touchfiles.ts @@ -149,6 +149,7 @@ export const E2E_TOUCHFILES: Record = { // confirm" plan write. runPlanSkillFloorCheck cannot detect that shape // (it exits on first AUQ); runPlanSkillCounting can. 'plan-eng-multi-finding-batching': ['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/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-eng-multi-finding-batching.test.ts'], + 'plan-ceo-split-overflow': ['plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'bin/gstack-question-preference', 'test/helpers/claude-pty-runner.ts', 'test/fixtures/forcing-finding-seeds.ts', 'test/skill-e2e-plan-ceo-split-overflow.test.ts'], 'brain-privacy-gate': ['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'], // /setup-gbrain Path 4 (Remote MCP) — happy + bad-token end-to-end via @@ -475,6 +476,7 @@ export const E2E_TIERS: Record = { 'plan-design-finding-floor': 'gate', 'plan-devex-finding-floor': 'gate', 'plan-eng-multi-finding-batching': 'periodic', + 'plan-ceo-split-overflow': 'periodic', // Privacy gate for gstack-brain-sync — periodic (non-deterministic LLM call, // costs ~$0.30-$0.50 per run, not needed on every commit) diff --git a/test/skill-e2e-plan-ceo-split-overflow.test.ts b/test/skill-e2e-plan-ceo-split-overflow.test.ts new file mode 100644 index 000000000..05f3c51f2 --- /dev/null +++ b/test/skill-e2e-plan-ceo-split-overflow.test.ts @@ -0,0 +1,108 @@ +/** + * /plan-ceo-review split-overflow regression (periodic, paid, real-PTY). + * + * Catches the original failure mode the user complained about: when the + * agent has 5+ options for ONE conceptual decision, it must split into N + * sequential AskUserQuestion calls (or batch into compatible ≤4-groups), + * NOT drop an option arbitrarily to fit Conductor's 4-option cap. + * + * Pre-fix reasoning trace from the user transcript that motivated this: + * "I'm hitting Conductor's limit of 4 options in the AUQ, so I need + * to cut one. E4 is the largest lift and probably beyond scope... + * Trimming: E4. Moving to TODOs without asking. Re-firing with 4." + * + * The fixture seeds 5 independent scope candidates (chat-platform + * integrations) — each carries an independent include/defer/cut decision. + * With the split rule active, the natural compliant shape is a per-option + * chain at parent D; the test asserts the agent fires at least + * [N-1] review-phase AUQs (standard tolerance band from the existing + * finding-count tests, which accounts for one expected scope-reduction + * call before the per-option chain begins). + * + * Why a separate test from skill-e2e-plan-ceo-finding-count and + * skill-e2e-plan-eng-multi-finding-batching: + * - finding-count tests fire one AUQ per finding (Architecture, Code + * Quality, etc) — they exercise the "one issue per call" rule, not + * the "5+ options for ONE decision" split rule. + * - This test fixtures ONE scope decision with 5 options inside it, + * which is exactly the shape that hits Conductor's 4-option cap and + * triggers the new split-vs-drop guidance. + * + * Tier: periodic (~25 min, ~$0.30-$5.00/run depending on agent path). + * Sequential by default. + */ + +import { describe, test } from 'bun:test'; +import * as fs from 'node:fs'; +import { + runPlanSkillCounting, + ceoStep0Boundary, +} from './helpers/claude-pty-runner'; +import { FORCING_SPLIT_OVERFLOW_CEO } from './fixtures/forcing-finding-seeds'; + +const shouldRun = !!process.env.EVALS && process.env.EVALS_TIER === 'periodic'; +const describeE2E = shouldRun ? describe : describe.skip; + +const N = 5; +const FLOOR = N - 1; // 4 — must fire at least one AUQ per non-dropped option + +const PLAN_PATH = '/tmp/gstack-test-plan-ceo-split-overflow.md'; + +describeE2E('/plan-ceo-review split-overflow regression (periodic)', () => { + test( + `5-option scope decision emits >= ${FLOOR} review-phase AskUserQuestions (no dropping)`, + async () => { + try { + fs.rmSync(PLAN_PATH, { force: true }); + } catch { + /* best-effort */ + } + + const obs = await runPlanSkillCounting({ + skillName: 'plan-ceo-review', + slashCommand: '/plan-ceo-review', + followUpPrompt: FORCING_SPLIT_OVERFLOW_CEO, + isLastStep0AUQ: ceoStep0Boundary, + reviewCountCeiling: N + 3, // hard cap above floor + tolerance + cwd: process.cwd(), + timeoutMs: 1_500_000, // 25 min + env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' }, + }); + + try { + if (!['plan_ready', 'completion_summary', 'ceiling_reached'].includes(obs.outcome)) { + throw new Error( + `split-overflow test FAILED: outcome=${obs.outcome}\n` + + `step0=${obs.step0Count} review=${obs.reviewCount} elapsed=${obs.elapsedMs}ms\n` + + `--- evidence (last 3KB) ---\n${obs.evidence}`, + ); + } + if (obs.reviewCount < FLOOR) { + throw new Error( + `SPLIT-OVERFLOW REGRESSION: reviewCount=${obs.reviewCount} < FLOOR=${FLOOR}.\n` + + `Agent surfaced fewer review-phase AUQs than independent scope options.\n` + + `This is the original drop-to-fit-4-options failure mode:\n` + + ` expected: ${N} per-option calls (or compliant ≤4-group batching with follow-up)\n` + + ` got: ${obs.reviewCount} call(s)\n` + + `Most likely the agent dropped one option to fit Conductor's 4-option\n` + + `cap, the exact bug scripts/resolvers/preamble/generate-ask-user-format.ts\n` + + `"Handling 5+ options — split, never drop" exists to prevent.\n` + + `Review-phase fingerprints:\n` + + obs.fingerprints + .filter((f) => !f.preReview) + .map((f) => ` - "${f.promptSnippet.slice(0, 80)}"`) + .join('\n') + + `\n--- evidence (last 3KB) ---\n${obs.evidence}`, + ); + } + } finally { + try { + fs.rmSync(PLAN_PATH, { force: true }); + } catch { + /* best-effort */ + } + } + }, + 1_700_000, + ); +}); diff --git a/test/touchfiles.test.ts b/test/touchfiles.test.ts index 416ef0576..5e3a8c3e0 100644 --- a/test/touchfiles.test.ts +++ b/test/touchfiles.test.ts @@ -105,8 +105,12 @@ describe('selectTests', () => { expect(result.selected).toContain('auto-decide-preserved'); // v1.27+ gate-tier reviewCount-floor regression for transcript bug expect(result.selected).toContain('plan-ceo-finding-floor'); - expect(result.selected.length).toBe(21); - expect(result.skipped.length).toBe(Object.keys(E2E_TOUCHFILES).length - 21); + // garrytan/askuserquestion-split-on-overflow: split-overflow periodic + // E2E test also depends on plan-ceo-review/** (5-option scope decision + // regression for the "drop to fit 4 options" failure mode). + expect(result.selected).toContain('plan-ceo-split-overflow'); + expect(result.selected.length).toBe(22); + expect(result.skipped.length).toBe(Object.keys(E2E_TOUCHFILES).length - 22); }); test('global touchfile triggers ALL tests', () => {