From b63608099ccc8b9afb409d445da7af0c33bae0f2 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 15 Aug 2026 08:03:41 -0700 Subject: [PATCH] evals: split skill-e2e-review into three per-file CI shards Bun runs describe blocks as concurrency barriers, so the e2e-review CI job executed its tests serially: 741s of an 860s PR critical path for tests whose slowest member is 224s. The per-file matrix is the repo's parallelism unit, so the split moves: - Retro E2E + retro-base-branch -> test/skill-e2e-retro.test.ts - review/ship base-branch + Review Dashboard Via Attribution -> test/skill-e2e-review-attribution.test.ts - sql-injection / enum-completeness / design-lite stay in test/skill-e2e-review.test.ts One 741s job becomes three ~180-250s jobs. Locally the worst paid shard drops from 1705s (94.7% of the 1800s kill) to under 700s. Test names, bodies, suite strings, and eval-store collectors are unchanged, so baselines carry over. Matrix rows added to both eval workflows (attribution is gate-only, so no periodic row); the report job's hardcoded runner count is gone (drift-proof). Co-Authored-By: Claude Fable 5 --- .github/workflows/evals-periodic.yml | 5 + .github/workflows/evals.yml | 8 +- test/skill-e2e-retro.test.ts | 181 ++++++++++ test/skill-e2e-review-attribution.test.ts | 282 +++++++++++++++ test/skill-e2e-review.test.ts | 412 +--------------------- 5 files changed, 478 insertions(+), 410 deletions(-) create mode 100644 test/skill-e2e-retro.test.ts create mode 100644 test/skill-e2e-review-attribution.test.ts diff --git a/.github/workflows/evals-periodic.yml b/.github/workflows/evals-periodic.yml index 23c6ff126..5f1b00a7b 100644 --- a/.github/workflows/evals-periodic.yml +++ b/.github/workflows/evals-periodic.yml @@ -84,6 +84,11 @@ jobs: file: test/skill-e2e-qa-workflow.test.ts - name: e2e-review file: test/skill-e2e-review.test.ts + - name: e2e-retro + file: test/skill-e2e-retro.test.ts + # e2e-review-attribution is gate-only (all three of its tests are + # gate-tier) — deliberately absent here; an all-skip shard would just + # burn a container boot weekly. - name: e2e-workflow file: test/skill-e2e-workflow.test.ts - name: e2e-routing diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index 16ee6fc4b..cb96e9419 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -106,6 +106,10 @@ jobs: file: test/skill-e2e-qa-workflow.test.ts - name: e2e-review file: test/skill-e2e-review.test.ts + - name: e2e-retro + file: test/skill-e2e-retro.test.ts + - name: e2e-review-attribution + file: test/skill-e2e-review-attribution.test.ts - name: e2e-workflow file: test/skill-e2e-workflow.test.ts - name: e2e-routing @@ -357,14 +361,14 @@ jobs: BODY="## E2E Evals: ${STATUS} - **${PASSED}/${TOTAL}** tests passed | **\$${COST}** total cost | **13 parallel runners** + **${PASSED}/${TOTAL}** tests passed | **\$${COST}** total cost | Suite | Result | Status | Cost | |-------|--------|--------|------| $(echo -e "$SUITE_LINES") --- - *13x ubicloud-standard-8 (Docker: pre-baked toolchain + deps) | wall clock ≈ slowest suite*" + *ubicloud-standard-8 runners (Docker: pre-baked toolchain + deps) | wall clock ≈ slowest suite*" if [ "$FAILED" -gt 0 ]; then FAILURES="" diff --git a/test/skill-e2e-retro.test.ts b/test/skill-e2e-retro.test.ts new file mode 100644 index 000000000..0ee80b5db --- /dev/null +++ b/test/skill-e2e-retro.test.ts @@ -0,0 +1,181 @@ +import { expect, beforeAll, afterAll } from 'bun:test'; +import { runSkillTest } from './helpers/session-runner'; +import { + ROOT, runId, + describeIfSelected, testConcurrentIfSelected, + logCost, recordE2E, + createEvalCollector, finalizeEvalCollector, +} from './helpers/e2e-helpers'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +const evalCollector = createEvalCollector('e2e-retro'); + +// --- Retro base branch detection smoke test --- + +describeIfSelected('Base branch detection', ['retro-base-branch'], () => { + let baseBranchDir: string; + const run = (cmd: string, args: string[], cwd: string) => + spawnSync(cmd, args, { cwd, stdio: 'pipe', timeout: 5000 }); + + beforeAll(() => { + baseBranchDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-basebranch-')); + }); + + afterAll(() => { + try { fs.rmSync(baseBranchDir, { recursive: true, force: true }); } catch {} + }); + + testConcurrentIfSelected('retro-base-branch', async () => { + const dir = path.join(baseBranchDir, 'retro-base'); + fs.mkdirSync(dir, { recursive: true }); + + // Create git repo with commit history + run('git', ['init'], dir); + run('git', ['config', 'user.email', 'dev@example.com'], dir); + run('git', ['config', 'user.name', 'Dev'], dir); + + fs.writeFileSync(path.join(dir, 'app.ts'), 'console.log("hello");\n'); + run('git', ['add', 'app.ts'], dir); + run('git', ['commit', '-m', 'feat: initial app', '--date', '2026-03-14T09:00:00'], dir); + + fs.writeFileSync(path.join(dir, 'auth.ts'), 'export function login() {}\n'); + run('git', ['add', 'auth.ts'], dir); + run('git', ['commit', '-m', 'feat: add auth', '--date', '2026-03-15T10:00:00'], dir); + + fs.writeFileSync(path.join(dir, 'test.ts'), 'test("it works", () => {});\n'); + run('git', ['add', 'test.ts'], dir); + run('git', ['commit', '-m', 'test: add tests', '--date', '2026-03-16T11:00:00'], dir); + + // Copy retro skill + fs.mkdirSync(path.join(dir, 'retro'), { recursive: true }); + fs.copyFileSync(path.join(ROOT, 'retro', 'SKILL.md'), path.join(dir, 'retro', 'SKILL.md')); + + const result = await runSkillTest({ + prompt: `Read retro/SKILL.md for instructions on how to run a retrospective. + +IMPORTANT: Follow the "Detect default branch" step first. Since there is no remote, gh will fail — fall back to main. +Then use the detected branch name for all git queries. + +Run /retro for the last 7 days of this git repo. Skip any AskUserQuestion calls — this is non-interactive. +This is a local-only repo so use the local branch (main) instead of origin/main for all git log commands. + +Write your retrospective to ${dir}/retro-output.md`, + workingDirectory: dir, + maxTurns: 25, + // 360s, not 240s: same runner-contention class as review-dashboard-via. + // /retro is a long multi-step flow — a clean pass measured 225s and the + // next CI run timed out at the 240s line (exitReason "timeout", 3/3 + // attempts). Outer bun timeout below rises to 480s for headroom. + timeout: 360_000, + testName: 'retro-base-branch', + runId, + }); + + logCost('/retro base-branch', result); + recordE2E(evalCollector, '/retro default branch detection', 'Base branch detection', result, { + passed: ['success', 'error_max_turns'].includes(result.exitReason), + }); + expect(['success', 'error_max_turns']).toContain(result.exitReason); + + // Verify retro output was produced + const retroPath = path.join(dir, 'retro-output.md'); + if (fs.existsSync(retroPath)) { + const content = fs.readFileSync(retroPath, 'utf-8'); + expect(content.length).toBeGreaterThan(100); + } + }, 480_000); +}); + +// --- Retro E2E --- + +describeIfSelected('Retro E2E', ['retro'], () => { + let retroDir: string; + + beforeAll(() => { + retroDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-retro-')); + const run = (cmd: string, args: string[]) => + spawnSync(cmd, args, { cwd: retroDir, stdio: 'pipe', timeout: 5000 }); + + // Create a git repo with varied commit history + run('git', ['init', '-b', 'main']); + run('git', ['config', 'user.email', 'dev@example.com']); + run('git', ['config', 'user.name', 'Dev']); + + // Day 1 commits + fs.writeFileSync(path.join(retroDir, 'app.ts'), 'console.log("hello");\n'); + run('git', ['add', 'app.ts']); + run('git', ['commit', '-m', 'feat: initial app setup', '--date', '2026-03-10T09:00:00']); + + fs.writeFileSync(path.join(retroDir, 'auth.ts'), 'export function login() {}\n'); + run('git', ['add', 'auth.ts']); + run('git', ['commit', '-m', 'feat: add auth module', '--date', '2026-03-10T11:00:00']); + + // Day 2 commits + fs.writeFileSync(path.join(retroDir, 'app.ts'), 'import { login } from "./auth";\nconsole.log("hello");\nlogin();\n'); + run('git', ['add', 'app.ts']); + run('git', ['commit', '-m', 'fix: wire up auth to app', '--date', '2026-03-11T10:00:00']); + + fs.writeFileSync(path.join(retroDir, 'test.ts'), 'import { test } from "bun:test";\ntest("login", () => {});\n'); + run('git', ['add', 'test.ts']); + run('git', ['commit', '-m', 'test: add login test', '--date', '2026-03-11T14:00:00']); + + // Day 3 commits + fs.writeFileSync(path.join(retroDir, 'api.ts'), 'export function getUsers() { return []; }\n'); + run('git', ['add', 'api.ts']); + run('git', ['commit', '-m', 'feat: add users API endpoint', '--date', '2026-03-12T09:30:00']); + + fs.writeFileSync(path.join(retroDir, 'README.md'), '# My App\nA test application.\n'); + run('git', ['add', 'README.md']); + run('git', ['commit', '-m', 'docs: add README', '--date', '2026-03-12T16:00:00']); + + // Copy retro skill + fs.mkdirSync(path.join(retroDir, 'retro'), { recursive: true }); + fs.copyFileSync( + path.join(ROOT, 'retro', 'SKILL.md'), + path.join(retroDir, 'retro', 'SKILL.md'), + ); + }); + + afterAll(() => { + try { fs.rmSync(retroDir, { recursive: true, force: true }); } catch {} + }); + + testConcurrentIfSelected('retro', async () => { + const result = await runSkillTest({ + prompt: `Read retro/SKILL.md for instructions on how to run a retrospective. + +Run /retro for the last 7 days of this git repo. Skip any AskUserQuestion calls — this is non-interactive. +Write your retrospective report to ${retroDir}/retro-output.md + +Analyze the git history and produce the narrative report as described in the SKILL.md.`, + workingDirectory: retroDir, + maxTurns: 30, + timeout: 300_000, + testName: 'retro', + runId, + model: 'claude-opus-4-7', + }); + + logCost('/retro', result); + recordE2E(evalCollector, '/retro', 'Retro E2E', result, { + passed: ['success', 'error_max_turns'].includes(result.exitReason), + }); + // Accept error_max_turns — retro does many git commands to analyze history + expect(['success', 'error_max_turns']).toContain(result.exitReason); + + // Verify the retro was written + const retroPath = path.join(retroDir, 'retro-output.md'); + if (fs.existsSync(retroPath)) { + const retro = fs.readFileSync(retroPath, 'utf-8'); + expect(retro.length).toBeGreaterThan(100); + } + }, 420_000); +}); + +// Module-level afterAll — finalize eval collector after all tests complete +afterAll(async () => { + await finalizeEvalCollector(evalCollector); +}); diff --git a/test/skill-e2e-review-attribution.test.ts b/test/skill-e2e-review-attribution.test.ts new file mode 100644 index 000000000..85c10b8f9 --- /dev/null +++ b/test/skill-e2e-review-attribution.test.ts @@ -0,0 +1,282 @@ +import { expect, beforeAll, afterAll } from 'bun:test'; +import { runSkillTest } from './helpers/session-runner'; +import { + ROOT, runId, + describeIfSelected, testConcurrentIfSelected, + logCost, recordE2E, + createEvalCollector, finalizeEvalCollector, +} from './helpers/e2e-helpers'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +const evalCollector = createEvalCollector('e2e-review-attribution'); + +// --- Base branch detection smoke tests --- + +describeIfSelected('Base branch detection', ['review-base-branch', 'ship-base-branch'], () => { + let baseBranchDir: string; + const run = (cmd: string, args: string[], cwd: string) => + spawnSync(cmd, args, { cwd, stdio: 'pipe', timeout: 5000 }); + + beforeAll(() => { + baseBranchDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-basebranch-')); + }); + + afterAll(() => { + try { fs.rmSync(baseBranchDir, { recursive: true, force: true }); } catch {} + }); + + testConcurrentIfSelected('review-base-branch', async () => { + const dir = path.join(baseBranchDir, 'review-base'); + fs.mkdirSync(dir, { recursive: true }); + + // Create git repo with a feature branch off main + run('git', ['init'], dir); + run('git', ['config', 'user.email', 'test@test.com'], dir); + run('git', ['config', 'user.name', 'Test'], dir); + + fs.writeFileSync(path.join(dir, 'app.rb'), '# clean base\nclass App\nend\n'); + run('git', ['add', 'app.rb'], dir); + run('git', ['commit', '-m', 'initial commit'], dir); + + // Create feature branch with a change + run('git', ['checkout', '-b', 'feature/test-review'], dir); + fs.writeFileSync(path.join(dir, 'app.rb'), '# clean base\nclass App\n def hello; "world"; end\nend\n'); + run('git', ['add', 'app.rb'], dir); + run('git', ['commit', '-m', 'feat: add hello method'], dir); + + // Extract only Step 0 (base branch detection) + minimal review instructions + // Full SKILL.md is ~1500 lines — copying it causes the agent to spend all turns reading + const full = fs.readFileSync(path.join(ROOT, 'review', 'SKILL.md'), 'utf-8'); + const step0Start = full.indexOf('## Step 0: Detect platform and base branch'); + const step1Start = full.indexOf('## Step 1: Check branch'); + const step1End = full.indexOf('---', step1Start + 10); + const extracted = full.slice(step0Start, step1End > step1Start ? step1End : step1Start + 500); + fs.writeFileSync(path.join(dir, 'review-SKILL.md'), extracted); + + const result = await runSkillTest({ + prompt: `You are in a git repo on a feature branch with changes. +Read review-SKILL.md for the base branch detection instructions. + +IMPORTANT: Follow Step 0 to detect the base branch. Since there is no remote, gh commands will fail — fall back to main. +Then run git diff against the detected base branch and write a brief review. +Write your findings to ${dir}/review-output.md`, + workingDirectory: dir, + maxTurns: 15, + timeout: 90_000, + testName: 'review-base-branch', + runId, + }); + + logCost('/review base-branch', result); + recordE2E(evalCollector, '/review base branch detection', 'Base branch detection', result); + expect(result.exitReason).toBe('success'); + + // Verify the review used "base branch" language (from Step 0) + const toolOutputs = result.toolCalls.map(tc => tc.output || '').join('\n'); + const allOutput = (result.output || '') + toolOutputs; + // The agent should have run git diff against main (the fallback) + const usedGitDiff = result.toolCalls.some(tc => { + if (tc.tool !== 'Bash') return false; + const cmd = typeof tc.input === 'string' ? tc.input : tc.input?.command || JSON.stringify(tc.input); + return cmd.includes('git diff'); + }); + expect(usedGitDiff).toBe(true); + }, 120_000); + + testConcurrentIfSelected('ship-base-branch', async () => { + const dir = path.join(baseBranchDir, 'ship-base'); + fs.mkdirSync(dir, { recursive: true }); + + // Create git repo with feature branch + run('git', ['init'], dir); + run('git', ['config', 'user.email', 'test@test.com'], dir); + run('git', ['config', 'user.name', 'Test'], dir); + + fs.writeFileSync(path.join(dir, 'app.ts'), 'console.log("v1");\n'); + run('git', ['add', 'app.ts'], dir); + run('git', ['commit', '-m', 'initial'], dir); + + run('git', ['checkout', '-b', 'feature/ship-test'], dir); + fs.writeFileSync(path.join(dir, 'app.ts'), 'console.log("v2");\n'); + run('git', ['add', 'app.ts'], dir); + run('git', ['commit', '-m', 'feat: update to v2'], dir); + + // Extract only Step 0 (base branch detection) from ship/SKILL.md + // (copying the full 1900-line file causes agent context bloat and flaky timeouts) + const fullShipSkill = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8'); + const step0Start = fullShipSkill.indexOf('## Step 0: Detect platform and base branch'); + const step0End = fullShipSkill.indexOf('## Step 1: Pre-flight'); + const shipSection = fullShipSkill.slice(step0Start, step0End > step0Start ? step0End : undefined); + fs.writeFileSync(path.join(dir, 'ship-SKILL.md'), shipSection); + + const result = await runSkillTest({ + prompt: `Read ship-SKILL.md. It contains Step 0 (Detect base branch) from the ship workflow. + +Run the base branch detection. Since there is no remote, gh commands will fail — fall back to main. + +Then run git diff and git log against the detected base branch. + +Write a summary to ${dir}/ship-preflight.md including: +- The detected base branch name +- The current branch name +- The diff stat against the base branch`, + workingDirectory: dir, + maxTurns: 18, + timeout: 150_000, + testName: 'ship-base-branch', + runId, + }); + + logCost('/ship base-branch', result); + recordE2E(evalCollector, '/ship base branch detection', 'Base branch detection', result); + expect(result.exitReason).toBe('success'); + + // Verify preflight output was written + const preflightPath = path.join(dir, 'ship-preflight.md'); + if (fs.existsSync(preflightPath)) { + const content = fs.readFileSync(preflightPath, 'utf-8'); + expect(content.length).toBeGreaterThan(20); + // Should mention the branch name + expect(content.toLowerCase()).toMatch(/main|base/); + } + + // Verify no destructive actions — no push, no PR creation + const destructiveTools = result.toolCalls.filter(tc => + tc.tool === 'Bash' && typeof tc.input === 'string' && + (tc.input.includes('git push') || tc.input.includes('gh pr create')) + ); + expect(destructiveTools).toHaveLength(0); + }, 180_000); +}); + +// --- Review Dashboard Via Attribution E2E --- + +describeIfSelected('Review Dashboard Via Attribution', ['review-dashboard-via'], () => { + let dashDir: string; + + beforeAll(() => { + dashDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-dashboard-via-')); + const run = (cmd: string, args: string[], cwd = dashDir) => + spawnSync(cmd, args, { cwd, stdio: 'pipe', timeout: 5000 }); + + // Create git repo with feature branch + run('git', ['init', '-b', 'main']); + run('git', ['config', 'user.email', 'test@test.com']); + run('git', ['config', 'user.name', 'Test']); + + fs.writeFileSync(path.join(dashDir, 'app.ts'), 'console.log("v1");\n'); + run('git', ['add', 'app.ts']); + run('git', ['commit', '-m', 'initial']); + + run('git', ['checkout', '-b', 'feature/dashboard-test']); + fs.writeFileSync(path.join(dashDir, 'app.ts'), 'console.log("v2");\n'); + run('git', ['add', 'app.ts']); + run('git', ['commit', '-m', 'feat: update']); + + // Get HEAD commit for review entries + const headResult = spawnSync('git', ['rev-parse', '--short', 'HEAD'], { cwd: dashDir, stdio: 'pipe' }); + const commit = headResult.stdout.toString().trim(); + + // Pre-populate review log with autoplan-sourced entries + // gstack-review-read reads from ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl + // For the test, we'll write a mock gstack-review-read script that returns our test data + const timestamp = new Date().toISOString().replace(/\.\d{3}Z$/, 'Z'); + const reviewData = [ + `{"skill":"plan-eng-review","timestamp":"${timestamp}","status":"clean","unresolved":0,"critical_gaps":0,"issues_found":0,"mode":"FULL_REVIEW","via":"autoplan","commit":"${commit}"}`, + `{"skill":"plan-ceo-review","timestamp":"${timestamp}","status":"clean","unresolved":0,"critical_gaps":0,"mode":"SELECTIVE_EXPANSION","via":"autoplan","commit":"${commit}"}`, + `{"skill":"codex-plan-review","timestamp":"${timestamp}","status":"clean","source":"codex","commit":"${commit}"}`, + ].join('\n'); + + // Write a mock gstack-review-read that returns our test data + const mockBinDir = path.join(dashDir, '.mock-bin'); + fs.mkdirSync(mockBinDir, { recursive: true }); + fs.writeFileSync(path.join(mockBinDir, 'gstack-review-read'), [ + '#!/usr/bin/env bash', + `echo '${reviewData.split('\n').join("'\necho '")}'`, + 'echo "---CONFIG---"', + 'echo "false"', + 'echo "---HEAD---"', + `echo "${commit}"`, + ].join('\n')); + fs.chmodSync(path.join(mockBinDir, 'gstack-review-read'), 0o755); + + // Extract only the Review Readiness Dashboard section from ship/SKILL.md + // (copying the full 1900-line file causes agent context bloat and timeouts) + const fullSkill = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8'); + const dashStart = fullSkill.indexOf('## Review Readiness Dashboard'); + const dashEnd = fullSkill.indexOf('\n---\n', dashStart); + const dashSection = fullSkill.slice(dashStart, dashEnd > dashStart ? dashEnd : undefined); + fs.writeFileSync(path.join(dashDir, 'ship-SKILL.md'), dashSection); + }); + + afterAll(() => { + try { fs.rmSync(dashDir, { recursive: true, force: true }); } catch {} + }); + + testConcurrentIfSelected('review-dashboard-via', async () => { + const mockBinDir = path.join(dashDir, '.mock-bin'); + + const result = await runSkillTest({ + prompt: `Read ship-SKILL.md. You only need to run the Review Readiness Dashboard section. + +Instead of running ~/.claude/skills/gstack/bin/gstack-review-read, run this mock: ${mockBinDir}/gstack-review-read + +Parse the output and display the dashboard table. Pay attention to: +1. The "via" field in entries — show source attribution (e.g., "via /autoplan") +2. The codex-plan-review entry — it should populate the Outside Voice row +3. Since Eng Review IS clear, there should be NO gate blocking — just display the dashboard + +Skip the preamble, lake intro, telemetry, and all other ship steps. +Write the dashboard output to ${dashDir}/dashboard-output.md`, + workingDirectory: dashDir, + maxTurns: 12, + // 300s, not 180s: on a saturated CI runner this file's concurrent + // sessions queue behind each other and session STARTUP can eat the + // whole budget — observed as deterministic timeout at 0 turns/$0.00 + // for exactly 180s across 3 attempts (PR #2472 CI + its baseline), + // while the 240s-budget tests in the same job passed. Outer bun + // timeout below rises to 360s to keep headroom over the inner budget. + timeout: 300_000, + testName: 'review-dashboard-via', + runId, + }); + + logCost('/ship dashboard-via', result); + recordE2E(evalCollector, '/ship review dashboard via attribution', 'Dashboard via field', result); + expect(result.exitReason).toBe('success'); + + // Check dashboard output for via attribution + const dashPath = path.join(dashDir, 'dashboard-output.md'); + const allOutput = [ + result.output || '', + ...result.toolCalls.map(tc => tc.output || ''), + ].join('\n').toLowerCase(); + + // Verify via attribution appears somewhere (conversation or file) + let dashContent = ''; + if (fs.existsSync(dashPath)) { + dashContent = fs.readFileSync(dashPath, 'utf-8').toLowerCase(); + } + const combined = allOutput + dashContent; + + // Should mention autoplan attribution + expect(combined).toMatch(/autoplan/); + // Should show eng review as CLEAR (it has a clean entry) + expect(combined).toMatch(/clear/i); + // Should NOT contain AskUserQuestion gate (no blocking) + const gateQuestions = result.toolCalls.filter(tc => + tc.tool === 'mcp__conductor__AskUserQuestion' || + (tc.tool === 'AskUserQuestion') + ); + // Ship dashboard should not gate when eng review is clear + expect(gateQuestions).toHaveLength(0); + }, 360_000); +}); + +// Module-level afterAll — finalize eval collector after all tests complete +afterAll(async () => { + await finalizeEvalCollector(evalCollector); +}); diff --git a/test/skill-e2e-review.test.ts b/test/skill-e2e-review.test.ts index 5eaf16ca3..660f0ed96 100644 --- a/test/skill-e2e-review.test.ts +++ b/test/skill-e2e-review.test.ts @@ -252,414 +252,10 @@ Important: The design checklist should catch issues like blacklisted fonts, smal }, 300_000); }); -// --- Base branch detection smoke tests --- - -describeIfSelected('Base branch detection', ['review-base-branch', 'ship-base-branch', 'retro-base-branch'], () => { - let baseBranchDir: string; - const run = (cmd: string, args: string[], cwd: string) => - spawnSync(cmd, args, { cwd, stdio: 'pipe', timeout: 5000 }); - - beforeAll(() => { - baseBranchDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-basebranch-')); - }); - - afterAll(() => { - try { fs.rmSync(baseBranchDir, { recursive: true, force: true }); } catch {} - }); - - testConcurrentIfSelected('review-base-branch', async () => { - const dir = path.join(baseBranchDir, 'review-base'); - fs.mkdirSync(dir, { recursive: true }); - - // Create git repo with a feature branch off main - run('git', ['init'], dir); - run('git', ['config', 'user.email', 'test@test.com'], dir); - run('git', ['config', 'user.name', 'Test'], dir); - - fs.writeFileSync(path.join(dir, 'app.rb'), '# clean base\nclass App\nend\n'); - run('git', ['add', 'app.rb'], dir); - run('git', ['commit', '-m', 'initial commit'], dir); - - // Create feature branch with a change - run('git', ['checkout', '-b', 'feature/test-review'], dir); - fs.writeFileSync(path.join(dir, 'app.rb'), '# clean base\nclass App\n def hello; "world"; end\nend\n'); - run('git', ['add', 'app.rb'], dir); - run('git', ['commit', '-m', 'feat: add hello method'], dir); - - // Extract only Step 0 (base branch detection) + minimal review instructions - // Full SKILL.md is ~1500 lines — copying it causes the agent to spend all turns reading - const full = fs.readFileSync(path.join(ROOT, 'review', 'SKILL.md'), 'utf-8'); - const step0Start = full.indexOf('## Step 0: Detect platform and base branch'); - const step1Start = full.indexOf('## Step 1: Check branch'); - const step1End = full.indexOf('---', step1Start + 10); - const extracted = full.slice(step0Start, step1End > step1Start ? step1End : step1Start + 500); - fs.writeFileSync(path.join(dir, 'review-SKILL.md'), extracted); - - const result = await runSkillTest({ - prompt: `You are in a git repo on a feature branch with changes. -Read review-SKILL.md for the base branch detection instructions. - -IMPORTANT: Follow Step 0 to detect the base branch. Since there is no remote, gh commands will fail — fall back to main. -Then run git diff against the detected base branch and write a brief review. -Write your findings to ${dir}/review-output.md`, - workingDirectory: dir, - maxTurns: 15, - timeout: 90_000, - testName: 'review-base-branch', - runId, - }); - - logCost('/review base-branch', result); - recordE2E(evalCollector, '/review base branch detection', 'Base branch detection', result); - expect(result.exitReason).toBe('success'); - - // Verify the review used "base branch" language (from Step 0) - const toolOutputs = result.toolCalls.map(tc => tc.output || '').join('\n'); - const allOutput = (result.output || '') + toolOutputs; - // The agent should have run git diff against main (the fallback) - const usedGitDiff = result.toolCalls.some(tc => { - if (tc.tool !== 'Bash') return false; - const cmd = typeof tc.input === 'string' ? tc.input : tc.input?.command || JSON.stringify(tc.input); - return cmd.includes('git diff'); - }); - expect(usedGitDiff).toBe(true); - }, 120_000); - - testConcurrentIfSelected('ship-base-branch', async () => { - const dir = path.join(baseBranchDir, 'ship-base'); - fs.mkdirSync(dir, { recursive: true }); - - // Create git repo with feature branch - run('git', ['init'], dir); - run('git', ['config', 'user.email', 'test@test.com'], dir); - run('git', ['config', 'user.name', 'Test'], dir); - - fs.writeFileSync(path.join(dir, 'app.ts'), 'console.log("v1");\n'); - run('git', ['add', 'app.ts'], dir); - run('git', ['commit', '-m', 'initial'], dir); - - run('git', ['checkout', '-b', 'feature/ship-test'], dir); - fs.writeFileSync(path.join(dir, 'app.ts'), 'console.log("v2");\n'); - run('git', ['add', 'app.ts'], dir); - run('git', ['commit', '-m', 'feat: update to v2'], dir); - - // Extract only Step 0 (base branch detection) from ship/SKILL.md - // (copying the full 1900-line file causes agent context bloat and flaky timeouts) - const fullShipSkill = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8'); - const step0Start = fullShipSkill.indexOf('## Step 0: Detect platform and base branch'); - const step0End = fullShipSkill.indexOf('## Step 1: Pre-flight'); - const shipSection = fullShipSkill.slice(step0Start, step0End > step0Start ? step0End : undefined); - fs.writeFileSync(path.join(dir, 'ship-SKILL.md'), shipSection); - - const result = await runSkillTest({ - prompt: `Read ship-SKILL.md. It contains Step 0 (Detect base branch) from the ship workflow. - -Run the base branch detection. Since there is no remote, gh commands will fail — fall back to main. - -Then run git diff and git log against the detected base branch. - -Write a summary to ${dir}/ship-preflight.md including: -- The detected base branch name -- The current branch name -- The diff stat against the base branch`, - workingDirectory: dir, - maxTurns: 18, - timeout: 150_000, - testName: 'ship-base-branch', - runId, - }); - - logCost('/ship base-branch', result); - recordE2E(evalCollector, '/ship base branch detection', 'Base branch detection', result); - expect(result.exitReason).toBe('success'); - - // Verify preflight output was written - const preflightPath = path.join(dir, 'ship-preflight.md'); - if (fs.existsSync(preflightPath)) { - const content = fs.readFileSync(preflightPath, 'utf-8'); - expect(content.length).toBeGreaterThan(20); - // Should mention the branch name - expect(content.toLowerCase()).toMatch(/main|base/); - } - - // Verify no destructive actions — no push, no PR creation - const destructiveTools = result.toolCalls.filter(tc => - tc.tool === 'Bash' && typeof tc.input === 'string' && - (tc.input.includes('git push') || tc.input.includes('gh pr create')) - ); - expect(destructiveTools).toHaveLength(0); - }, 180_000); - - testConcurrentIfSelected('retro-base-branch', async () => { - const dir = path.join(baseBranchDir, 'retro-base'); - fs.mkdirSync(dir, { recursive: true }); - - // Create git repo with commit history - run('git', ['init'], dir); - run('git', ['config', 'user.email', 'dev@example.com'], dir); - run('git', ['config', 'user.name', 'Dev'], dir); - - fs.writeFileSync(path.join(dir, 'app.ts'), 'console.log("hello");\n'); - run('git', ['add', 'app.ts'], dir); - run('git', ['commit', '-m', 'feat: initial app', '--date', '2026-03-14T09:00:00'], dir); - - fs.writeFileSync(path.join(dir, 'auth.ts'), 'export function login() {}\n'); - run('git', ['add', 'auth.ts'], dir); - run('git', ['commit', '-m', 'feat: add auth', '--date', '2026-03-15T10:00:00'], dir); - - fs.writeFileSync(path.join(dir, 'test.ts'), 'test("it works", () => {});\n'); - run('git', ['add', 'test.ts'], dir); - run('git', ['commit', '-m', 'test: add tests', '--date', '2026-03-16T11:00:00'], dir); - - // Copy retro skill - fs.mkdirSync(path.join(dir, 'retro'), { recursive: true }); - fs.copyFileSync(path.join(ROOT, 'retro', 'SKILL.md'), path.join(dir, 'retro', 'SKILL.md')); - - const result = await runSkillTest({ - prompt: `Read retro/SKILL.md for instructions on how to run a retrospective. - -IMPORTANT: Follow the "Detect default branch" step first. Since there is no remote, gh will fail — fall back to main. -Then use the detected branch name for all git queries. - -Run /retro for the last 7 days of this git repo. Skip any AskUserQuestion calls — this is non-interactive. -This is a local-only repo so use the local branch (main) instead of origin/main for all git log commands. - -Write your retrospective to ${dir}/retro-output.md`, - workingDirectory: dir, - maxTurns: 25, - // 360s, not 240s: same runner-contention class as review-dashboard-via. - // /retro is a long multi-step flow — a clean pass measured 225s and the - // next CI run timed out at the 240s line (exitReason "timeout", 3/3 - // attempts). Outer bun timeout below rises to 480s for headroom. - timeout: 360_000, - testName: 'retro-base-branch', - runId, - }); - - logCost('/retro base-branch', result); - recordE2E(evalCollector, '/retro default branch detection', 'Base branch detection', result, { - passed: ['success', 'error_max_turns'].includes(result.exitReason), - }); - expect(['success', 'error_max_turns']).toContain(result.exitReason); - - // Verify retro output was produced - const retroPath = path.join(dir, 'retro-output.md'); - if (fs.existsSync(retroPath)) { - const content = fs.readFileSync(retroPath, 'utf-8'); - expect(content.length).toBeGreaterThan(100); - } - }, 480_000); -}); - -// --- Retro E2E --- - -describeIfSelected('Retro E2E', ['retro'], () => { - let retroDir: string; - - beforeAll(() => { - retroDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-retro-')); - const run = (cmd: string, args: string[]) => - spawnSync(cmd, args, { cwd: retroDir, stdio: 'pipe', timeout: 5000 }); - - // Create a git repo with varied commit history - run('git', ['init', '-b', 'main']); - run('git', ['config', 'user.email', 'dev@example.com']); - run('git', ['config', 'user.name', 'Dev']); - - // Day 1 commits - fs.writeFileSync(path.join(retroDir, 'app.ts'), 'console.log("hello");\n'); - run('git', ['add', 'app.ts']); - run('git', ['commit', '-m', 'feat: initial app setup', '--date', '2026-03-10T09:00:00']); - - fs.writeFileSync(path.join(retroDir, 'auth.ts'), 'export function login() {}\n'); - run('git', ['add', 'auth.ts']); - run('git', ['commit', '-m', 'feat: add auth module', '--date', '2026-03-10T11:00:00']); - - // Day 2 commits - fs.writeFileSync(path.join(retroDir, 'app.ts'), 'import { login } from "./auth";\nconsole.log("hello");\nlogin();\n'); - run('git', ['add', 'app.ts']); - run('git', ['commit', '-m', 'fix: wire up auth to app', '--date', '2026-03-11T10:00:00']); - - fs.writeFileSync(path.join(retroDir, 'test.ts'), 'import { test } from "bun:test";\ntest("login", () => {});\n'); - run('git', ['add', 'test.ts']); - run('git', ['commit', '-m', 'test: add login test', '--date', '2026-03-11T14:00:00']); - - // Day 3 commits - fs.writeFileSync(path.join(retroDir, 'api.ts'), 'export function getUsers() { return []; }\n'); - run('git', ['add', 'api.ts']); - run('git', ['commit', '-m', 'feat: add users API endpoint', '--date', '2026-03-12T09:30:00']); - - fs.writeFileSync(path.join(retroDir, 'README.md'), '# My App\nA test application.\n'); - run('git', ['add', 'README.md']); - run('git', ['commit', '-m', 'docs: add README', '--date', '2026-03-12T16:00:00']); - - // Copy retro skill - fs.mkdirSync(path.join(retroDir, 'retro'), { recursive: true }); - fs.copyFileSync( - path.join(ROOT, 'retro', 'SKILL.md'), - path.join(retroDir, 'retro', 'SKILL.md'), - ); - }); - - afterAll(() => { - try { fs.rmSync(retroDir, { recursive: true, force: true }); } catch {} - }); - - testConcurrentIfSelected('retro', async () => { - const result = await runSkillTest({ - prompt: `Read retro/SKILL.md for instructions on how to run a retrospective. - -Run /retro for the last 7 days of this git repo. Skip any AskUserQuestion calls — this is non-interactive. -Write your retrospective report to ${retroDir}/retro-output.md - -Analyze the git history and produce the narrative report as described in the SKILL.md.`, - workingDirectory: retroDir, - maxTurns: 30, - timeout: 300_000, - testName: 'retro', - runId, - model: 'claude-opus-4-7', - }); - - logCost('/retro', result); - recordE2E(evalCollector, '/retro', 'Retro E2E', result, { - passed: ['success', 'error_max_turns'].includes(result.exitReason), - }); - // Accept error_max_turns — retro does many git commands to analyze history - expect(['success', 'error_max_turns']).toContain(result.exitReason); - - // Verify the retro was written - const retroPath = path.join(retroDir, 'retro-output.md'); - if (fs.existsSync(retroPath)) { - const retro = fs.readFileSync(retroPath, 'utf-8'); - expect(retro.length).toBeGreaterThan(100); - } - }, 420_000); -}); - -// --- Review Dashboard Via Attribution E2E --- - -describeIfSelected('Review Dashboard Via Attribution', ['review-dashboard-via'], () => { - let dashDir: string; - - beforeAll(() => { - dashDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-dashboard-via-')); - const run = (cmd: string, args: string[], cwd = dashDir) => - spawnSync(cmd, args, { cwd, stdio: 'pipe', timeout: 5000 }); - - // Create git repo with feature branch - run('git', ['init', '-b', 'main']); - run('git', ['config', 'user.email', 'test@test.com']); - run('git', ['config', 'user.name', 'Test']); - - fs.writeFileSync(path.join(dashDir, 'app.ts'), 'console.log("v1");\n'); - run('git', ['add', 'app.ts']); - run('git', ['commit', '-m', 'initial']); - - run('git', ['checkout', '-b', 'feature/dashboard-test']); - fs.writeFileSync(path.join(dashDir, 'app.ts'), 'console.log("v2");\n'); - run('git', ['add', 'app.ts']); - run('git', ['commit', '-m', 'feat: update']); - - // Get HEAD commit for review entries - const headResult = spawnSync('git', ['rev-parse', '--short', 'HEAD'], { cwd: dashDir, stdio: 'pipe' }); - const commit = headResult.stdout.toString().trim(); - - // Pre-populate review log with autoplan-sourced entries - // gstack-review-read reads from ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl - // For the test, we'll write a mock gstack-review-read script that returns our test data - const timestamp = new Date().toISOString().replace(/\.\d{3}Z$/, 'Z'); - const reviewData = [ - `{"skill":"plan-eng-review","timestamp":"${timestamp}","status":"clean","unresolved":0,"critical_gaps":0,"issues_found":0,"mode":"FULL_REVIEW","via":"autoplan","commit":"${commit}"}`, - `{"skill":"plan-ceo-review","timestamp":"${timestamp}","status":"clean","unresolved":0,"critical_gaps":0,"mode":"SELECTIVE_EXPANSION","via":"autoplan","commit":"${commit}"}`, - `{"skill":"codex-plan-review","timestamp":"${timestamp}","status":"clean","source":"codex","commit":"${commit}"}`, - ].join('\n'); - - // Write a mock gstack-review-read that returns our test data - const mockBinDir = path.join(dashDir, '.mock-bin'); - fs.mkdirSync(mockBinDir, { recursive: true }); - fs.writeFileSync(path.join(mockBinDir, 'gstack-review-read'), [ - '#!/usr/bin/env bash', - `echo '${reviewData.split('\n').join("'\necho '")}'`, - 'echo "---CONFIG---"', - 'echo "false"', - 'echo "---HEAD---"', - `echo "${commit}"`, - ].join('\n')); - fs.chmodSync(path.join(mockBinDir, 'gstack-review-read'), 0o755); - - // Extract only the Review Readiness Dashboard section from ship/SKILL.md - // (copying the full 1900-line file causes agent context bloat and timeouts) - const fullSkill = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8'); - const dashStart = fullSkill.indexOf('## Review Readiness Dashboard'); - const dashEnd = fullSkill.indexOf('\n---\n', dashStart); - const dashSection = fullSkill.slice(dashStart, dashEnd > dashStart ? dashEnd : undefined); - fs.writeFileSync(path.join(dashDir, 'ship-SKILL.md'), dashSection); - }); - - afterAll(() => { - try { fs.rmSync(dashDir, { recursive: true, force: true }); } catch {} - }); - - testConcurrentIfSelected('review-dashboard-via', async () => { - const mockBinDir = path.join(dashDir, '.mock-bin'); - - const result = await runSkillTest({ - prompt: `Read ship-SKILL.md. You only need to run the Review Readiness Dashboard section. - -Instead of running ~/.claude/skills/gstack/bin/gstack-review-read, run this mock: ${mockBinDir}/gstack-review-read - -Parse the output and display the dashboard table. Pay attention to: -1. The "via" field in entries — show source attribution (e.g., "via /autoplan") -2. The codex-plan-review entry — it should populate the Outside Voice row -3. Since Eng Review IS clear, there should be NO gate blocking — just display the dashboard - -Skip the preamble, lake intro, telemetry, and all other ship steps. -Write the dashboard output to ${dashDir}/dashboard-output.md`, - workingDirectory: dashDir, - maxTurns: 12, - // 300s, not 180s: on a saturated CI runner this file's concurrent - // sessions queue behind each other and session STARTUP can eat the - // whole budget — observed as deterministic timeout at 0 turns/$0.00 - // for exactly 180s across 3 attempts (PR #2472 CI + its baseline), - // while the 240s-budget tests in the same job passed. Outer bun - // timeout below rises to 360s to keep headroom over the inner budget. - timeout: 300_000, - testName: 'review-dashboard-via', - runId, - }); - - logCost('/ship dashboard-via', result); - recordE2E(evalCollector, '/ship review dashboard via attribution', 'Dashboard via field', result); - expect(result.exitReason).toBe('success'); - - // Check dashboard output for via attribution - const dashPath = path.join(dashDir, 'dashboard-output.md'); - const allOutput = [ - result.output || '', - ...result.toolCalls.map(tc => tc.output || ''), - ].join('\n').toLowerCase(); - - // Verify via attribution appears somewhere (conversation or file) - let dashContent = ''; - if (fs.existsSync(dashPath)) { - dashContent = fs.readFileSync(dashPath, 'utf-8').toLowerCase(); - } - const combined = allOutput + dashContent; - - // Should mention autoplan attribution - expect(combined).toMatch(/autoplan/); - // Should show eng review as CLEAR (it has a clean entry) - expect(combined).toMatch(/clear/i); - // Should NOT contain AskUserQuestion gate (no blocking) - const gateQuestions = result.toolCalls.filter(tc => - tc.tool === 'mcp__conductor__AskUserQuestion' || - (tc.tool === 'AskUserQuestion') - ); - // Ship dashboard should not gate when eng review is clear - expect(gateQuestions).toHaveLength(0); - }, 360_000); -}); +// Base branch detection tests for review/ship + the Review Dashboard Via +// Attribution describe live in test/skill-e2e-review-attribution.test.ts. +// Retro tests (retro, retro-base-branch) live in test/skill-e2e-retro.test.ts. +// Split so CI's per-file matrix can run them in parallel. // Module-level afterAll — finalize eval collector after all tests complete afterAll(async () => {