diff --git a/CHANGELOG.md b/CHANGELOG.md index 351e7cabd..961c1f398 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,40 @@ # Changelog +## [1.60.2.0] - 2026-08-07 + +## **Three free-suite tests fail-proofed against machine drift.** +## **Plus a filed P1: the suite's exit code can lie, and now we know why.** + +A full-suite health check turned up three tests that failed on dev machines while CI stayed green, all test-side drift rather than product bugs. The eval:list CLI test now spawns from a neutral directory, so slug detection cannot route reads away from the fixture store it seeds (the old cwd made it fail on any machine with the dev symlink). The benchmark CLI's remediation-hint check is case-insensitive, matching the reworded Gemini guidance ("Export GEMINI_API_KEY..."). The session-runner observability floor now expects the 5 wrapped I/O sites that actually exist since the shell-free spawn removed the prompt-file unlink. + +### The numbers that matter + +Source: this branch's investigation logs (~/.gstack-dev/logs/free-suite-*.log) and per-file reruns. + +| Check | Before | After | +|-------|--------|-------| +| eval-list-cli on dev machines | 1 fail (reads empty project dir) | 2/2 pass, deterministic everywhere | +| benchmark-cli remediation hint | 1 fail (case-brittle regex) | 15/15 pass | +| observability check 11 floor | expects >= 6 markers, counts 5 | floor matches the 5 real sites | + +One deeper finding got filed instead of rushed: at least five browse test files force-exit the shared bun process with `setTimeout(() => process.exit(0), 500)`, which can exit 0 before the summary prints and mask real failures. That is now a P1 in TODOS.md with receipts, because removing the exits without fixing the handle leaks they paper over would trade silent failure for hangs. + +### What this means for you + +`bun test` gives the same verdict on your laptop as in CI for these three tests, and the exit-code trust problem is documented with a concrete fix path instead of lurking. + +### Itemized changes + +#### Fixed + +- `test/eval-list-cli.test.ts`: spawn from neutral cwd + absolute script path so `getProjectEvalDir()` slug probes fail deterministically and the seeded legacy store is read. +- `test/benchmark-cli.test.ts`: remediation-hint pattern made case-insensitive for the updated Gemini NOT-READY message. +- `test/helpers/observability.test.ts`: check 11 floor 6 → 5 with the surviving wrapped-I/O sites named. + +#### For contributors + +- TODOS.md: new P1 (free-suite exit code masked by in-process force-exits, with repro + receipts) filed under Test infrastructure. + ## [1.60.1.0] - 2026-07-09 ## **The /autoplan dual-voice eval is back on the board, catching real regressions.** diff --git a/TODOS.md b/TODOS.md index 29721e6e5..fc98f0a83 100644 --- a/TODOS.md +++ b/TODOS.md @@ -45,6 +45,37 @@ a silent mistake breaks all 52 skills. High blast radius — needs its own focus ## Test infrastructure +### P1: Free suite exit code is untrustworthy — in-process force-exits mask failures + +**Priority:** P1 + +**What:** At least five browse test files end with `setTimeout(() => process.exit(0), 500)` +(browse/test/commands.test.ts:101, snapshot.test.ts:36, batch.test.ts:47, +handoff.test.ts:31, content-security.test.ts:465). The timer fires inside the SHARED +`bun test` process, exiting 0 before bun prints its final summary — so `bun test` can +report exit 0 while real test failures scrolled by earlier. Remove the force-exits and +fix the underlying handle leaks they paper over (lingering Playwright/daemon handles +that once made the suite hang), or scope the exit to a spawned child process. + +**Why:** Observed 2026-08-07: three genuinely failing tests (eval-list-cli, +benchmark-cli, observability check 11) rode green `bun test` exit codes across +multiple runs; the failures only surfaced by grepping logs for "(fail)" lines. A test +suite that exits 0 on failure is worse than no suite — it manufactures false +confidence at commit time and in any CI job that trusts the exit code. + +**Pros:** Restores the one contract everything (CI, /ship, humans) relies on: exit +code == truth. Also un-hides the missing final summary block. +**Cons:** The force-exits exist because the suite once hung on leaked handles; +removing them without fixing the leaks trades silent failure for hangs. Needs a +focused pass: find each leaked handle (daemon children, PTY, Playwright contexts), +close them in afterAll, then delete the exits one file at a time. + +**Context / where to start:** `grep -rn "process.exit(0)" browse/test/` — the +setTimeout variants are the offenders (server-no-import-side-effects.test.ts:62 is a +spawned-child probe, fine). Repro: run the full free suite and note the log ends at +the browse files with no "Ran N tests" summary. Receipts: +~/.gstack-dev/logs/free-suite-main-check.log (3 masked fails, exit 0). + ### P2: Periodic CI matrix covers 9 of ~66 e2e files — decide the coverage contract **Priority:** P2 diff --git a/VERSION b/VERSION index c4190e004..762f17554 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.60.1.0 +1.60.2.0 diff --git a/package.json b/package.json index 846438a60..78b98a86f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.60.1.0", + "version": "1.60.2.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/test/benchmark-cli.test.ts b/test/benchmark-cli.test.ts index 8edea3b24..7b359601a 100644 --- a/test/benchmark-cli.test.ts +++ b/test/benchmark-cli.test.ts @@ -133,7 +133,7 @@ describe('gstack-model-benchmark --dry-run', () => { const notReadyLines = out.split('\n').filter(l => l.includes('NOT READY')); expect(notReadyLines.length).toBeGreaterThanOrEqual(2); for (const line of notReadyLines) { - expect(line).toMatch(/(install|Install|login|export|Run|Log in)/); + expect(line).toMatch(/(install|login|export|run|log in)/i); } } finally { fs.rmSync(emptyHome, { recursive: true, force: true }); diff --git a/test/eval-list-cli.test.ts b/test/eval-list-cli.test.ts index f742ec706..536e36617 100644 --- a/test/eval-list-cli.test.ts +++ b/test/eval-list-cli.test.ts @@ -51,8 +51,14 @@ function writeEvalRun(evalDir: string, filename: string, timestamp: string, turn } function runEvalList(...args: string[]): { stdout: string; stderr: string; status: number } { - const result = spawnSync('bun', ['run', 'scripts/eval-list.ts', ...args], { - cwd: ROOT, + // cwd is the temp HOME, NOT the repo root: getProjectEvalDir() probes the + // cwd-relative .claude/skills/gstack/bin/gstack-slug, and on dev machines + // with the self-symlink that probe succeeds, routing reads to an (empty) + // project-scoped dir instead of the legacy ~/.gstack-dev/evals this test + // seeds. A neutral cwd makes both slug probes fail deterministically, so + // the CLI always uses the seeded legacy dir — same behavior as CI. + const result = spawnSync('bun', ['run', path.join(ROOT, 'scripts', 'eval-list.ts'), ...args], { + cwd: tmpHome, env: { ...process.env, HOME: tmpHome, diff --git a/test/helpers/observability.test.ts b/test/helpers/observability.test.ts index 67b588fb7..7ca812965 100644 --- a/test/helpers/observability.test.ts +++ b/test/helpers/observability.test.ts @@ -92,9 +92,10 @@ describe('session-runner observability', () => { ); // Count non-fatal comments — should be present for each new I/O path const nonFatalCount = (src.match(/\/\* non-fatal \*\//g) || []).length; - // Original had 2 (promptFile unlink + failure transcript), we added 4 more - // (runDir creation, progress.log, heartbeat, NDJSON append) - expect(nonFatalCount).toBeGreaterThanOrEqual(6); + // Five wrapped I/O sites: runDir creation, progress.log append, heartbeat + // write, per-test NDJSON append, failure-transcript write. (Was 6 until + // the shell-free spawn removed the promptFile unlink and its marker.) + expect(nonFatalCount).toBeGreaterThanOrEqual(5); }); });