mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-13 16:38:56 +02:00
Merge remote-tracking branch 'origin/main' into garrytan/binding-polarity-wave
This commit is contained in:
Garry Tan
@@ -186,29 +186,6 @@ references — include it in this fix's coverage list.
|
||||
|
||||
## Test infrastructure
|
||||
|
||||
### P2: Wire `design/test/` into CI (all 8 files are invisible to every runner)
|
||||
|
||||
**What:** Add `design/test/` to the `bun test` glob (`package.json:21`) and
|
||||
`TEST_ROOTS` (`scripts/test-free-shards.ts:32`) after auditing its 8 files for
|
||||
server-spawning/flakiness (they were plausibly excluded on purpose). While in
|
||||
there, fix the known timing flake: `variants-retry-after.test.ts` "HTTP-date:
|
||||
honors a future date with no extra leading exponential" fails ~1-2 in 9 runs
|
||||
under parallel suite load (verified pre-existing on v1.58.5.0 during the
|
||||
June 2026 fix wave — wall-clock assertion with a ~2s window).
|
||||
|
||||
**Why:** Every test in `design/test/` runs only when someone types the path by
|
||||
hand — a silent coverage hole, the fix wave's theme at meta-level. The wave's
|
||||
own design tests went into `test/design-flag-utils.test.ts` to dodge this.
|
||||
|
||||
**Pros:** design binary gets CI coverage; kills a latent "we have tests" illusion.
|
||||
**Cons:** unaudited files may spawn servers or flake; audit first, wire second.
|
||||
|
||||
**Context:** Filed from the June 2026 fix-wave eng review (issue 11 + flake
|
||||
receipts). Start with the audit: which of the 8 files are hermetic? Wire the
|
||||
hermetic ones, quarantine or fix the rest.
|
||||
|
||||
**Effort:** S-M (human ~1d, CC ~30min). **Depends on:** None.
|
||||
|
||||
### P2: /context-save worktree-identity hardening (the #2052 residual)
|
||||
|
||||
**What:** Persist a stable worktree identity (path hash or worktree name) into
|
||||
@@ -253,37 +230,6 @@ Trigger condition documented in `lib/gbrain-sources.ts` at the drift log line.
|
||||
|
||||
**Effort:** M (human ~1d, CC ~45min). **Depends on:** drift-log evidence from
|
||||
the wave's `ensureSourceRegistered` logging.
|
||||
### 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
|
||||
@@ -307,6 +253,14 @@ claim true.
|
||||
some orphans are deliberately manual (ios-device, opus-47 overlay harness), so a plain
|
||||
glob is wrong — needs a curated exclude list.
|
||||
|
||||
**Fresh receipts (2026-08-16, v1.66.0.0 re-baseline):** the first full local
|
||||
periodic run in this store gave the never-baselined tail its first results:
|
||||
`skill-e2e-setup-gbrain-{bad-token,path4-local-pglite,remote}` all failed
|
||||
(spawned-process exit 1 — likely live-gbrain interference on a dev box) and
|
||||
`skill-e2e-ship-idempotency` timed out at the 1800s shard wall. None are in
|
||||
the weekly matrix, so these failures are invisible to CI — exactly this
|
||||
item's thesis. Start the burn-down with those four.
|
||||
|
||||
**Context / where to start:** `.github/workflows/evals-periodic.yml:71` (matrix),
|
||||
`test/helpers/touchfiles.ts` E2E_TIERS (tier labels already exist per test), orphan
|
||||
list generated via `comm -23` between `ls test/skill-e2e-*.test.ts` and the file lists
|
||||
@@ -2411,8 +2365,212 @@ Shipped in v0.6.5. TemplateContext in gen-skill-docs.ts bakes skill name into pr
|
||||
|
||||
**Depends on:** v1.47.0.0 ships; gather real false-negative data from the v1 string matcher.
|
||||
|
||||
## Test/evals/CI speedup follow-ups (filed v1.66.0.0 via /ship review army)
|
||||
|
||||
### P2: Free-suite shard balancing — LPT by recorded durations instead of stable hash
|
||||
|
||||
**What:** Full-suite shard assignment is a stable hash; measured shard durations
|
||||
spread 69.5s-168.5s (max 2.4x min), so ~35-40s of every run is idle tail. Local
|
||||
full-suite mode doesn't need deterministic indices (only the CI --shards matrix
|
||||
does) — bin-pack by recorded per-file durations (bun prints them in the logs the
|
||||
runner already captures), keep assignFilesToShards untouched for --shard mode.
|
||||
**Where:** scripts/test-free-shards.ts main() full-suite path.
|
||||
**Effort:** S (human ~4h, CC ~20min).
|
||||
|
||||
### P2: Propagate parent eval selection to shard children (EVALS_SELECTION_JSON)
|
||||
|
||||
**What:** The sharded paid runner computes selection once in the parent, but each
|
||||
shard child re-derives it at e2e-helpers module load (git spawns per shard; plus a
|
||||
bun child evaluating the old touchfiles-data when map-diff is active). Serialize
|
||||
the parent's selection into the child env and honor it in computeDiffSelection,
|
||||
keeping child self-derivation for non-sharded entrypoints. Add a parent/child
|
||||
selection drift test (same fixture through computePaidDiffSelection and
|
||||
computeDiffSelection) while there.
|
||||
**Where:** scripts/test-paid-shards.ts runPaidShards env block; test/helpers/e2e-helpers.ts.
|
||||
**Effort:** S (human ~4h, CC ~20min).
|
||||
|
||||
### P2: evals.yml matrix census tripwire — gate files must appear in the CI matrix
|
||||
|
||||
**What:** The branch's headline incident (two rehomed gate files silently never ran
|
||||
for 48 versions because the monolith's filename missed the hand-listed evals.yml
|
||||
matrix) has no tripwire binding gate-tier skill-e2e files to the matrix.
|
||||
e2e-tier-alignment covers the LOCAL sharded runner's mapper; the CI matrix can
|
||||
still drift. Parse the workflow YAML in a free test and diff against E2E_TIERS
|
||||
gate files (curated exclude list for deliberately-manual files).
|
||||
**Where:** new test beside test/e2e-tier-alignment.test.ts; .github/workflows/evals.yml.
|
||||
**Effort:** S (human ~3h, CC ~15min).
|
||||
|
||||
### P2: E2E dep-list self-registration sweep — 129 of 177 keys omit their own test file
|
||||
|
||||
**What:** Editing only a test's assertions/prompt selects nothing for most keys
|
||||
(the adversarial review measured 129/177), and parent-side shard skipping makes
|
||||
the hole cheaper to hit. This branch fixed the rehomed files' keys; sweep the
|
||||
rest mechanically (each key's dep list appends the file that declares it) and
|
||||
upgrade e2e-tier-alignment's report-only mode to enforce self-registration.
|
||||
**Where:** test/helpers/touchfiles-data.ts; test/e2e-tier-alignment.test.ts.
|
||||
**Effort:** S (human ~3h, CC ~15min).
|
||||
|
||||
### P3: Paid runner spools non-live shard output to disk instead of RAM
|
||||
|
||||
**What:** Non-live shards buffer their entire 30-min stream-json stdout+stderr in
|
||||
memory (Buffer[]), x jobs concurrent shards. Spool to a temp file like the free
|
||||
runner's per-run log.
|
||||
**Where:** scripts/test-paid-shards.ts runPaidShard buffered path.
|
||||
**Effort:** S (human ~2h, CC ~10min).
|
||||
|
||||
### P3: Eval Docker image freshness tripwire
|
||||
|
||||
**What:** The cache-key trio means the image rebuilds only when Dockerfile/bun.lock
|
||||
change; freshness of the baked unpinned claude CLI now rides entirely on
|
||||
ci-image.yml's cron. If the cron silently fails or is disabled, eval CI pins to an
|
||||
ever-older CLI with no signal. Add an image-age check (fail the eval workflow when
|
||||
the image tag's created date exceeds N days) or a cron-liveness alert.
|
||||
**Where:** .github/workflows/ci-image.yml, evals.yml.
|
||||
**Effort:** S (human ~2h, CC ~10min).
|
||||
|
||||
### P3: Detach-floor self-check against runtime knobs (EVALS_JOBS)
|
||||
|
||||
**What:** test/eval-detach-timeout-floor.test.ts computes the worst case from
|
||||
constants; an operator exporting EVALS_JOBS=2 doubles the gate worst case past the
|
||||
25,200s watchdog and healthy tail shards report never-started. Add a runtime
|
||||
self-check in test-paid-shards main(): warn/fail when the computed worst case with
|
||||
LIVE options exceeds a GSTACK_DETACH_TIMEOUT env exported by gstack-detach.
|
||||
**Where:** scripts/test-paid-shards.ts; bin/gstack-detach.
|
||||
**Effort:** S (human ~2h, CC ~10min).
|
||||
|
||||
### P3: Eval store records the effective judge/capture model per run
|
||||
|
||||
**What:** Model defaults moved (capture Opus→Sonnet) and GSTACK_EVAL_MODEL_JUDGE
|
||||
can silently change graders; eval:compare deltas across a model boundary conflate
|
||||
model swap with skill regressions. Record the resolved models in the eval-store
|
||||
record and surface them in eval:compare.
|
||||
**Where:** test/helpers/eval-store.ts, llm-judge.ts, eval-compare.
|
||||
**Effort:** S (human ~2h, CC ~10min).
|
||||
|
||||
### P3: SECURITY_BENCH periodic lane — classifier behavioral coverage runs nowhere
|
||||
|
||||
**What:** Gating the live L4 classifier tests on SECURITY_BENCH=1 fixed local
|
||||
suite speed but left the prompt-injection classifier with no scheduled lane.
|
||||
Add SECURITY_BENCH=1 (with model-cache warmup, 112MB first run) to
|
||||
evals-periodic.yml so behavioral coverage exists weekly.
|
||||
**Where:** .github/workflows/evals-periodic.yml; browse/test/security-live-playwright.test.ts.
|
||||
**Effort:** S (human ~2h, CC ~10min).
|
||||
|
||||
### P3: Shared child-lifecycle helper for the two shard runners
|
||||
|
||||
**What:** runFreeShard and runPaidShard duplicate ~35 lines of spawn/group-kill/
|
||||
wall-timer scaffold verbatim (and the ShardCommand type). Extract into
|
||||
scripts/test-strict-output.ts, which already hosts the shared lifecycle
|
||||
primitives, leaving stream policy per runner.
|
||||
**Where:** scripts/test-free-shards.ts, scripts/test-paid-shards.ts.
|
||||
**Effort:** S (human ~3h, CC ~15min).
|
||||
|
||||
### P3: DI-refactor gstack-gbrain-detect-mcp-mode test (~40s spawn cost, absorbed but real)
|
||||
|
||||
**What:** Plan item 5 of the v1.66.0.0 pass, deferred: the test spawns the real
|
||||
binary repeatedly. Refactor to import the module with a DI-injected exec seam
|
||||
(never env-set-before-import), keep 1-2 spawn smokes. Cost is currently absorbed
|
||||
by shard parallelism; the per-file wall cost remains.
|
||||
**Where:** test/gstack-gbrain-detect-mcp-mode.test.ts.
|
||||
**Effort:** S (human ~2h, CC ~15min).
|
||||
|
||||
### P2: In-shard eval concurrency (40) is the shared root of the timeout-flake family
|
||||
|
||||
**What:** Every timeout-flake member on PR #2593 (document-release 180s->300s,
|
||||
review-dashboard-via 300s->360s after PR #2472's 180s->300s, retro-base-branch
|
||||
240s->360s) shares one story: claude session STARTUP queues behind up to 39
|
||||
siblings under evals.yml's `--max-concurrency 40`, eating the per-test budget
|
||||
before the first turn. Per-test ratchets treat symptoms. Systemic options:
|
||||
(a) drop in-shard concurrency to ~15-20 and measure the wall-clock cost,
|
||||
(b) startup-aware budgets (start the timer at first turn, not spawn),
|
||||
(c) per-row concurrency overrides like the retries field. Receipts: the
|
||||
PR #2593 flake ledger comment.
|
||||
**Where:** .github/workflows/evals.yml:309 (--max-concurrency 40);
|
||||
test/helpers/session-runner.ts (budget start point).
|
||||
**Effort:** M (human ~1d, CC ~45min + measurement rounds).
|
||||
|
||||
### P2: plan-design-review scope-gate detector is marginal under CI contention
|
||||
|
||||
**What:** `plan-design-review reaches a terminal outcome outside plan mode`
|
||||
(test/skill-e2e-plan-mode-no-op.test.ts) intermittently fails ONLY the
|
||||
`scopeGateQuestionObserved` check on unchanged code — PR #2593 CI: failed
|
||||
rounds 3/11 + one rerun, passed rounds 5/6, all attempts reaching a terminal
|
||||
outcome with no plan-mode leak. Hypothesis: the PTY detector anchors on a
|
||||
render shape that scrolls out or gets rephrased under 40-way in-shard
|
||||
contention. The assertion now throws WITH the last-2KB evidence tail, so the
|
||||
next CI failure carries the screen contents; fix the detector (scan full
|
||||
scrollback, or widen the anchored shape) from that data.
|
||||
|
||||
**Where:** test/helpers/claude-pty-runner.ts (scopeGateQuestionObserved
|
||||
detector), test/skill-e2e-plan-mode-no-op.test.ts.
|
||||
**Effort:** S (human ~3h, CC ~20min + one CI round with evidence).
|
||||
|
||||
### P3: Diagnose the browser-manager-unit wedge on windows-latest
|
||||
|
||||
**What:** The expanded Windows lane wedges to its wall deadline inside
|
||||
browse/test/browser-manager-unit.test.ts (in-flight at kill, PR #2593 run
|
||||
31919227507); the file is green on macOS and Linux. Excluded from the Windows
|
||||
curation with a receipt; needs a Windows repro to find which describe hangs
|
||||
(fake-timer/unref semantics under bun-windows are the suspects).
|
||||
**Where:** browse/test/browser-manager-unit.test.ts; scripts/test-free-shards.ts
|
||||
KNOWN_WINDOWS_INCOMPATIBLE (remove the entry once fixed).
|
||||
**Effort:** S (human ~2h with a Windows box, CC ~15min + CI rounds).
|
||||
|
||||
### P3: skill-census Windows compatibility
|
||||
|
||||
**What:** skillCensus() throws at module load on windows-latest
|
||||
(test/helpers/skill-census.ts:63) — the skills-tree symlink layout needs
|
||||
Developer Mode CI runners lack. Either branch the census walk on win32
|
||||
(treat copy-dirs as the setup script's _link_or_copy fallback produces) or
|
||||
keep the exclusion. Consumers (catalog budget, coverage matrix) currently
|
||||
have no Windows signal.
|
||||
**Where:** test/helpers/skill-census.ts; test/skill-census.test.ts.
|
||||
**Effort:** S (human ~3h, CC ~20min + CI rounds).
|
||||
|
||||
### P3: Tighten revived coverage-audit E2E assertions
|
||||
|
||||
**What:** The revived skill-e2e-coverage-audit tests assert hasGap OR hasTested
|
||||
(near-vacuous) and reference skill sections their own DRIFT WARNING says moved.
|
||||
Tighten to conjunctive assertions and retarget the prompts at live sections;
|
||||
needs one paid run to validate, so it didn't ride the ship.
|
||||
**Where:** test/skill-e2e-coverage-audit.test.ts.
|
||||
**Effort:** S (human ~2h, CC ~15min + one paid run).
|
||||
|
||||
## Completed
|
||||
|
||||
### ✅ DONE (v1.66.0.0): 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).
|
||||
|
||||
**Completed:** v1.66.0.0 (2026-08-15) — main's v1.64 removed the force-exits; v1.66.0.0 adds runner-level strict-output classification (a shard without bun's terminal summary FAILS), size-scaled wall deadlines, and the failure-naming epilogue, so exit code == truth is enforced by the runner, not by convention.
|
||||
|
||||
### Slim preamble + real-PTY plan-mode E2E harness (v1.13.1.0)
|
||||
|
||||
- Compressed 18 preamble resolvers; total `SKILL.md` corpus dropped from 3.08 MB to 2.30 MB across 47 outputs (-25.5%, ~196K tokens saved).
|
||||
|
||||
Reference in New Issue
Block a user