Make the test suite runnable under bun test --parallel

[Gustav-Simonsson]

Summary

bun test --parallel on Bun 1.4.x can livelock (upstream oven-sh/bun#36235): a
worker spins at 100% CPU with no output, and since bun test has no run-level
timeout, the hang is permanent. This PR adds a safe local parallel mode and removes
unnecessary fixed sleeps from the tests.

• New bun run test:parallel [N] (default 4 workers) runs the same seeded suite as
  bun run test with --parallel=N, under a watchdog (scripts/test-parallel.ts).
  If the suite produces no output for 90 s, the watchdog kills its process group and
  restarts it (3 attempts total). A run that exits on its own is never retried, so
  real test failures still fail the gate.
• Tests now wait for the condition they depend on instead of sleeping for a fixed
  time, which makes them stable under parallel load and removes dead time.
• Three test-only timeouts (tool-watchdog salvage grace, teardown deadline,
  agents-panel linger) can now be shortened via optional config values that
  production code does not set.

Full-suite wall time: ~21 s with test:parallel vs ~75 s sequential at the
base commit (≈3.5×). Sequential runs are also ~1.3× faster from the removed waits.
CI is unchanged (sharded sequential test:paths).

Unchanged production behavior

• All production changes are new optional parameters/config fields whose defaults
  are the previous hard-coded values, and no production call site sets them.
• Test changes only replace fixed sleeps with condition waits. No assertions were
  added, removed, or modified. The only new tests (+6) cover the watchdog wrapper.

Verification

• bun run typecheck and bun run lint pass at HEAD.
• Full-suite results are identical before and after, apart from the 6 new tests:
  base 6765 pass / 1 skip / 4 fail, HEAD 6771 pass / 1 skip / 4 fail. The 4 failures
  also occur on the unmodified tree in this environment (permission/git sandbox
  issues) and are unrelated to this PR.
• The --parallel livelock reproduces on the base commit, and the watchdog rescued a
  live occurrence during verification (killed the stalled run after 90 s; the retry
  completed in 21.2 s).
• Caveat: under heavy parallel load, 11 tests in src/agent/exa-web-fetch-alias.test.ts
  (untouched by this PR) hit Bun's 5 s per-test timeout in one run. They pass
  sequentially and in isolated parallel runs.

Test plan

• bun run test:parallel — local full-suite run with the stall watchdog.
• bun run test — unchanged sequential mode.
• CI: unchanged (test:paths).

[TheGreatAxios]

Skywalker · Comment

Comment. The split in this branch is the right one: condition-waits and injectable timeouts for test wall-clock, a process-group watchdog for bun test --parallel livelock. Fake timers do not replace the watchdog. I would not merge this as a CI-speed change — CI is still sharded sequential test:paths.

Fake timers vs the watchdog (the question on this PR)

No. Dropping Bun.sleep / installing fake timers does not make the watchdog unnecessary. They are different layers.

The watchdog in scripts/test-parallel.ts kills a hung bun test worker process (upstream oven-sh/bun#36235: coordinator waits forever, no run-level timeout). That hang is native. JS fake time never runs if the worker is wedged.

What does help suite speed — and what this PR already does in the files it touches — is:

1. Wait for the condition (log.flush(), paint poll, injected clock) instead of setTimeout(10).
2. Shorten production timeouts from tests via optional knobs (salvageGraceMs, teardownDeadlineMs, agentsPanelLingerMs) without changing production defaults.

That is the low-overhead path. It is also the one that stays correct under --parallel load.

mock.module("bun") to stub Bun.sleep — do not do this

Honestly: that snippet is a bad fit here.

• Tests call the global Bun.sleep, not import { sleep } from "bun". mock.module("bun") does not intercept the global.
• This repo forbids bare mock.module (CL-6967 / corbits/no-bare-mock-module). Bun runs ./src + ./tests in one process; an unrestored mock leaks into later files.
• Instant-resolve sleep without a clock leaves Date.now() / new Date() on real time. The elapsed-time example in the prompt fails exactly as described.
• Hand-rolling Date.now = () => fakeTime misses performance.now(), setTimeout, Intl, and anything the runtime uses internally.

Bun already has the API that snippet is reinventing:

• setSystemTime — Date.now / new Date() / Intl only. Does not skip Bun.sleep.
• jest.useFakeTimers() + jest.advanceTimersByTime — does intercept Bun.sleep and can advance Date.now together.

Checks (Bun 1.4.0):

bun test /tmp/bun-timer-probe.test.ts
  setSystemTime does not make Bun.sleep instant  [151.24ms]  pass
  Date.now follows setSystemTime                  pass

bun test /tmp/bun-timer-probe2.test.ts
  Bun.sleep stays pending until advanceTimersByTime  [0.16ms]  pass
  useFakeTimers Date.now advances with the fake clock pass

So: if a unit test is asserting timer math, use useFakeTimers locally and restore in finally. Do not mock the bun module. Do not install fake timers suite-wide in this one-process runner.

performance.now() stays real wall time under setSystemTime (same probe). Anything measuring elapsed work with performance.now() will not see the fake clock.

Does this maximize CI speed?

This PR does not change CI. .github/workflows/ci.yml still runs bun run check:projects-dir-guard per shard, Bun 1.3.14. Sequential test:paths is the right CI default while #36235 still reproduces on 1.3.14 (~10% in the upstream report) and residually on 1.4.0 (~2/30 in the follow-up comment).

CI wall-clock from this diff is only the sequential wait cuts (approval-log 10ms sleeps → flush(), linger 4.5s → 800ms, salvage/teardown shortened in subprocess/tests). The claimed 3.5× is local test:parallel only.

If hang rate is really ~50% at 4 workers (commit body) and stall is 90s × 3 attempts, expected local time can exceed sequential ~75s. Healthy silence is claimed at 0.64s (scripts/test-parallel.ts comment). 90s is a lot of margin; tightening stall (still well above one slow file) is the overhead knob, not fake timers.

Remaining Bun.sleep / setTimeout waits on main (untouched by this PR) still add up — src/tui/slash-popup-gate.test.ts especially. Same recipe as this PR: condition wait or inject the clock. Not a global fake-timer preload.

Findings

Docs / upstream mismatch

scripts/test-parallel.ts:4-7 and docs/IMPLEMENTATION.md say #36235 is still open on 1.4.2 and describe a worker spinning at 100% CPU with a zombie git child.

Check: gh api repos/oven-sh/bun/issues/36235 → state: closed, closed_at: 2026-07-31, state_reason: completed. Later comment (2026-08-21) says residual hangs on 1.4.0 stable with all threads parked in condvar waits, no children. Local reproduction may differ; the comment should describe what we actually observed, and should not say the issue is open if GitHub has it closed.

Watchdog tests are themselves wall-clock

scripts/test-parallel.test.ts uses 300ms stall vs 100ms print ticks (output resets the stall timer). Under the --parallel load this branch exists to enable, a delayed tick can look like a stall. I ran bun test ./scripts/test-parallel.test.ts --seed 424242 in the worktree: 6 pass / 0 fail in 1.99s, isolated. That does not prove the 300ms window under a loaded parallel suite.

PR description vs CONTRIBUTING.md

Required sections are ## Summary and ## Verification. Extra ## Unchanged production behavior and ## Test plan are not in the template. No Linear Fixes CL-… / skip CL-…; branch is faster_tests (fork Gustav-Simonsson/corbits-code), so that may be intentional.

What looks solid

• createApprovalLog().flush() instead of 10ms sleeps — waits on the append tail, production path still fire-and-forget.
• Frozen clock in nudge-director.test.ts via the constructor's existing now argument — no global Date patch.
• transcript-anchor.test.ts polls for the painted row instead of a 250ms sleep.
• Optional knobs default to previous constants; production call sites do not set them (teardownDeadlineMs only in tests/fixtures/exec-shutdown-reap/simulate-reap.ts, a subprocess).
• Watchdog does not retry a child that exits on its own (scripts/test-parallel.ts runWithWatchdog: if (!stalled) return). Group kill is covered by the grandchild test.

Checks

• git diff origin/main...HEAD --stat — 17 files, +530/−158, no unexpected binaries.
• git log origin/main..HEAD — 5 commits; subjects ≤60 chars, imperative, no prefixes; bodies wrap ≤72.
• bun test ./scripts/test-parallel.test.ts --seed 424242 — 6 pass, 0 fail, 1.99s (worktree HEAD 7eec056).
• PR CI rollup — sequential shards green (Bun 1.3.14).
• Fake-timer probes above on Bun 1.4.0.

Worktree left at /Users/thegreataxios/abklabs/worktree/faster_tests (fork ref pull/915/head). Remove with git worktree remove when done.

[TheGreatAxios]

Critic · Comment

No production-path break. Knobs do not leak (salvageGraceMs / agentsPanelLingerMs per-instance; teardownDeadlineMs only written from the reap fixture subprocess). Fake timers still cannot replace this watchdog — it wraps a bun-test-runner hang, not JS sleep.

Should-fix (not in the Skywalker comment)

Ctrl+C leaves the suite running. Bun.spawn(..., { detached: true }) at scripts/test-parallel.ts:78-84 puts bun test --parallel in its own session. The import.meta.main path (:164-188) installs no SIGINT/SIGTERM handler and never terminateGroup on wrapper exit. Stall (:118-121) is the only kill path.

Sequence: bun run test:parallel → Ctrl+C → wrapper dies → child group keeps running (piped stdout already dead). That is the opposite of the comment at :81-82 (“kills bun test, its workers, and any grandchild together”).

Permanent test: SIGINT the wrapper while a long bun -e child is alive, then assert the child pid and its grandchild are gone.

Linger test still sleeps. src/tui/product-host.test.ts:328-331 waits TEST_AGENTS_PANEL_LINGER_MS + 500 (800ms) instead of polling heights.agents === 0. Same race class the transcript test already fixed with a condition wait (src/tui/transcript-anchor.test.ts:19-30).

Agree with Skywalker (not re-argued)

• #36235 is closed; comments still say open + 100% CPU + zombie git (scripts/test-parallel.ts:4-7, :146, docs/IMPLEMENTATION.md).
• scripts/test-parallel.test.ts 300ms stall vs 100ms ticks can flake under the load this runner exists to create.
• 90s × 3 retries vs a claimed ~50% hang rate can erase the 21s vs 75s win.

Did not run the full suite or reproduce Ctrl+C / bun livelock in this pass.

[TheGreatAxios]

Proposal: keep the test hygiene, drop the --parallel wrapper

Thanks for this — the timing-race work is real, and sequential is already ~1.3× faster from it. The wrapper is a different product. Please split this PR rather than landing both.

What we want

Tests that are actual units: no shared process state, no setTimeout(10) hoping the log landed, no 4s linger paid in wall clock. Fast enough that a local git hook (lint + unit slice) is rational. CI stays sharded sequential test:paths. --randomize --seed 424242 stays — that is a leak detector, not a speed trick.

bun test --parallel is not how we get there. CI already does not use it. A git hook should not either. oven-sh/bun#36235 is Closed upstream; it still wedges on the 1.4.x we run, which is a Bun-version problem, not a reason to own a retrying process-group killer in this repo.

---

Keep (commits 1–4)

b3d6c36a  Expose salvage grace override in the tool watchdog config
b7809dab  Expose teardown deadline override in process handler options
0497cda0  Expose agents-panel linger override in product host config
51db9b66  Fix test timing races exposed by parallel test runs

These are the right pattern: await the condition, or inject the clock/timeout. Production defaults unchanged.

1. Await the append, don’t sleep

createApprovalLog already chains appends on tail. Tests were doing:

await new Promise((r) => setTimeout(r, 10));
const [record] = readRecords(dir);

Keep flush():

await log.flush();
const [record] = readRecords(dir);

Same change in permission-plugin.test.ts (the DRY approvalGate helper is good too).

2. Poll for paint, don’t sleep 250ms

transcript-anchor.test.ts waiting for the tree-sitter highlighter is correct:

async function frameWith(h: Harness, needle: string, timeoutMs = 3_000) {
  const deadline = Date.now() + timeoutMs;
  for (;;) {
    await h.renderOnce();
    const frame = h.captureCharFrame();
    if (frame.includes(needle) || Date.now() > deadline) return frame;
    await new Promise((resolve) => setTimeout(resolve, 25));
  }
}

3. Inject now, don’t race Date.now()

SubAgentDirector already takes a clock. Stall tests that are not about stall timing should freeze it:

const frozenNow = () => 0;
new SubAgentDirector("system", [], onContinuation, 30, frozenNow);

Tests that are about stall timing should step a fake clock (now += 1500), which you already do in a few cases. That is the house style: inject now / schedule first. Not suite-wide useFakeTimers, not mock.module("bun") (that does not rebind global Bun; bare mock.module is banned — CL-6967 / withMockedModuleDuring).

4. Test-only timeout knobs (production never sets them)

Knob	Default	Test
salvageGraceMs	production salvage grace	tool-execution-watchdog.test.ts
teardownDeadlineMs	2s	simulate-reap.ts
agentsPanelLingerMs	4s	product-host sticky-poll test

Keep these. One leftover in the linger test: the knob is 300ms, then it still sleeps 300 + 500. Please poll the zone height / frame instead of setTimeout, same as frameWith.

---

Drop (commit 5)

7eec0569  Add parallel test runner with stall watchdog

Please delete from this PR:

• scripts/test-parallel.ts
• scripts/test-parallel.test.ts
• package.json → test:parallel
• the Parallel local runs paragraph in docs/IMPLEMENTATION.md

Why this is out of scope for the merge:

• CI is already two sequential test:paths shards. Grow the matrix (split fat ./src) if wall clock matters. Do not add bun --parallel to CI.
• detached: true and no SIGINT/SIGTERM forward means Ctrl+C on bun run test:parallel leaves bun test --parallel running. A local convenience tool that ignores Ctrl+C is worse than sequential.
• Retry-on-stall papers over a Bun livelock. A child that exits is not retried (good) but a wedged worker is indistinguishable from “slow machine / hung test” at 90s of silence.
• Isolating clocks does not retire this wrapper, and we don’t want the wrapper. Speed for hooks comes from not sleeping, not from 4 workers.

If we still want laptop --parallel later, it is a separate optional tool: process-group kill, SIGINT forwarded, no retry of real failures, documented as local-only until the Bun we run does not wedge. Not this PR.

---

How to land it

Cheapest (preferred): drop commit 5 on this branch, retitle, keep the first four.

git revert --no-edit 7eec0569
# or reset the wrapper files and amend if you are rewriting

Suggested title: Stop sleeping in tests; inject timeouts

Suggested summary:

Tests wait for the condition they depend on (log.flush(), paint-poll, injected now) and three production timeouts are overridable in tests only. Sequential suite is faster and less racy. No bun test --parallel wrapper; CI stays sharded sequential.

If reverting on this branch is messy: close this PR and cherry-pick b3d6c36a..51db9b66 onto a new branch. Please don’t re-author flush() / the knobs from scratch — they’re already the right shape.

---

Follow-ups (not this PR)

Same pattern, remaining sleep piles — these are what make a git hook possible:

• slash-popup-gate.test.ts (~16× 60ms)
• agent-fleet.test.ts, gate-wire.test.ts, other TUI tests
• ~7 files still on Bun.sleep, ~30 files on real setTimeout(n > 0)

CI: more sequential test:paths shards if ./src is the long pole.

Hook: lint + unit paths in seconds. Not evals, not worktrees, not test:parallel.

---

Ask

Please strip commit 5 (or cherry-pick 1–4 onto a new PR) and I’ll re-review that slice. Happy to keep discussing the optional local --parallel tool separately once the suite is actually fast.