Skip to content

fix(test): bound CLI subprocess fan-out so unit (linux) passes on GitHub-hosted runners (LAC-2715) - #37

Merged
lacymorrow merged 2 commits into
LAC-2385/blacksmith-to-github-runnersfrom
LAC-2715/fix-run-subprocess-ci-timeouts
Jul 10, 2026
Merged

fix(test): bound CLI subprocess fan-out so unit (linux) passes on GitHub-hosted runners (LAC-2715)#37
lacymorrow merged 2 commits into
LAC-2385/blacksmith-to-github-runnersfrom
LAC-2715/fix-run-subprocess-ci-timeouts

Conversation

@lacymorrow

Copy link
Copy Markdown
Owner

Paperclip issue

LAC-2715 — Fix unit (linux) CI failures: opencode run (non-interactive subprocess) suite times out at ~30s on GitHub-hosted runners.

Root cause

The suite spawns one cold bun run src/index.ts subprocess per cliIt.concurrent test with unbounded fan-out (~10 at once, some tests spawn 3 sequentially). Each child re-transpiles the entire dependency graph, and the harness's per-test HOME/XDG_CACHE_HOME isolation leaves every child's Bun transpiler cache cold. On a 4-vCPU GitHub-hosted runner the simultaneous cold starts starve each other until every full-session child hits its 30s subprocess timeout with empty stdout (exit -1, Error: Timed out) — exactly the 7 failures on run 29067555482. The tests that survived are the fail-fast ones (unknown model, bad attach) that exit before the heavy boot.

Reproduced locally: fresh install/cold caches → 8 fail; warm second run → 13 pass. Deterministic A/B under 12× CPU-hog stress: unfixed → 5 pass / 8 fail (same signature as CI), fixed → 13 pass.

Fix (no blind timeout raises — root cause + fail-fast diagnostics)

  • spawnGate semaphore in test/lib/cli-process.ts caps concurrent short-lived CLI spawns at max(2, min(4, cores/2)). The per-spawn 30s timer starts only once the permit is held, so queue wait is not charged against timeoutMs/durationMs and a genuinely hung subprocess still dies at 30s.
  • Shared Bun transpiler cache across children via BUN_RUNTIME_TRANSPILER_CACHE_PATH (content-addressed, so per-test isolation is preserved). Only the first spawn pays the transpile cost; local full test/cli/ runtime dropped 305s → 219s.
  • Better timeout diagnostics: AppProcess drops collected stderr on timeout, so the synthesized stderr now includes elapsed time and the budget (Timed out after Xms (timeout 30000ms)) instead of a bare Error: Timed out.
  • Per-test bun timeouts → 120s in run-process.test.ts: with gated spawns a test's wall clock includes queueing behind other tests, so the test-level budget must cover serialized suite throughput. Hang detection lives in the per-spawn timeout, not the test timeout. Duration regression assertions (durationMs < 30_000) are unchanged and now measure pure subprocess time.

Acceptance criteria

  • unit (linux) reaches success on GitHub-hosted runners → verified by this PR's own CI (base: LAC-2385/blacksmith-to-github-runners).

Testing

  • bun test test/cli/run/run-process.test.ts — 13 pass, cold shared cache.
  • Same, under 12 concurrent CPU hogs (starved-runner simulation) — 13 pass (control without fix: 8 fail).
  • bun test test/cli/ (all 53 files using or adjacent to the fixture) — 414 pass / 0 fail.
  • bun run typecheck (packages/opencode) — clean.

…ives small CI runners (LAC-2715)

The 'opencode run (non-interactive subprocess)' suite spawned one cold
'bun run src/index.ts' per cliIt.concurrent test with unbounded fan-out.
Each child re-transpiles the whole dependency graph, and per-test
HOME/XDG_CACHE_HOME isolation left every child's Bun transpiler cache
cold. On 4-vCPU GitHub-hosted runners the ~10 simultaneous cold starts
starve each other until every child blows its 30s timeout with empty
stdout (reproduced locally: cold caches + CPU stress -> 8 fail; with
this fix under the same stress -> 13 pass).

- Gate short-lived spawns behind a semaphore sized to the host cores;
  the per-spawn 30s timer starts once the permit is held, so a genuine
  hang still fails fast with the same tight budget.
- Share one content-addressed Bun transpiler cache across children via
  BUN_RUNTIME_TRANSPILER_CACHE_PATH so only the first spawn pays the
  transpile cost.
- Annotate synthesized timeout stderr with elapsed/budget timing since
  AppProcess drops collected stderr on timeout.
- Raise per-test bun timeouts to cover serialized suite throughput;
  hang detection now lives in the per-spawn timeout, not the test one.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@lacymorrow
lacymorrow force-pushed the LAC-2715/fix-run-subprocess-ci-timeouts branch from fc21187 to d9a6562 Compare July 10, 2026 04:39

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces performance optimizations for the test suite by implementing a concurrency gate (spawnGate) and a shared Bun transpiler cache to prevent resource starvation on CI runners. The changes include adding a semaphore to limit concurrent subprocess spawns and configuring a shared cache directory for transpilation. I have kept the reviewer's suggestion to make the shared cache directory user-specific to avoid potential permission conflicts in multi-tenant environments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +51 to +52
const sharedTranspilerCache = path.join(os.tmpdir(), "opencode-test-bun-transpiler-cache")
fs.mkdirSync(sharedTranspilerCache, { recursive: true })

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a hardcoded directory name like "opencode-test-bun-transpiler-cache" in the shared system temp directory (os.tmpdir()) can lead to permission conflicts (EACCES) in multi-tenant CI environments or on shared local development machines when different users run the tests.

To prevent this, we should make the directory name user-specific by appending the current username.

Suggested change
const sharedTranspilerCache = path.join(os.tmpdir(), "opencode-test-bun-transpiler-cache")
fs.mkdirSync(sharedTranspilerCache, { recursive: true })
const username = os.userInfo?.().username || process.env.USER || "default"
const sharedTranspilerCache = path.join(os.tmpdir(), "opencode-test-bun-transpiler-cache-" + username)
fs.mkdirSync(sharedTranspilerCache, { recursive: true })

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in 9174c6b — the cache dir is now suffixed with the current username. Used a try/catch around os.userInfo() (it can throw ENOENT in containers without a passwd entry) with USER/USERNAME env fallbacks for POSIX/Windows.

@lacymorrow
lacymorrow merged commit cc7f362 into LAC-2385/blacksmith-to-github-runners Jul 10, 2026
6 of 7 checks passed
@lacymorrow
lacymorrow deleted the LAC-2715/fix-run-subprocess-ci-timeouts branch July 10, 2026 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant