Skip to content

fix(ci): widen tight subprocess timeouts causing flaky unit failures - #162

Closed
alltomatos wants to merge 3 commits into
agentui-telegram-channelfrom
fix/ci-flaky-subprocess-timeouts
Closed

alltomatos wants to merge 3 commits into
agentui-telegram-channelfrom
fix/ci-flaky-subprocess-timeouts

Conversation

@alltomatos

@alltomatos alltomatos commented Sep 9, 2026 •

Copy link
Copy Markdown
Owner

Issue for this PR

Closes #163

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Summary

  • Widen three subprocess-spawning test timeouts that were tight enough to flake under CI-load cold-start variance, matching the rationale already documented in test/lib/cli-process.ts (default bumped 30s→60s on 2026-09-08 for the same class of issue).
  • run-process.test.ts: "exits nonzero promptly when the model is unknown" (25s→32s) and "unknown stream finish preserves partial output and continues" (30s→45s).
  • lifecycle.test.ts: "stdin EOF exits cleanly" (5s→15s), which failed on Windows only.

All three were observed failing on PR #160 / #161 CI runs on 2026-09-09 by a few hundred ms past their old bound — not a wide margin, not a real hang/regression.

How did you verify your code works?

  • Read the harness's own subprocess-timing rationale (test/lib/cli-process.ts) and confirmed these three tests were failing the same way, at the same margin, for the same reason.
  • Widened each bound with headroom (not tightened to the exact observed miss) so a genuine hang would still fail loudly.

Screenshots / recordings

Not applicable — test-only change, no UI/behavior change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Test plan

🤖 Generated with Claude Code

Three subprocess-spawning tests carried timeout budgets tighter than the
CI-load headroom already established for this test harness (see the
2026-09-08 comment in test/lib/cli-process.ts bumping the default from
30s->60s for the same reason): run-process.test.ts's unknown-model and
midstream-error tests, and lifecycle.test.ts's stdin-EOF test. All three
were observed failing on 2026-09-09 by a few hundred ms past their bound,
not by a wide margin — consistent with subprocess cold-start variance
under parallel CI load rather than an actual regression. Widened with
headroom instead of shaving the margin to the exact miss.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Thanks for updating your PR! It now meets our contributing guidelines. 👍

alltomatos added a commit that referenced this pull request Sep 9, 2026
The "returningOpen" sample window (700ms) could close before the dock
finished mounting after a session switch under CI load, producing zero
"present" samples and failing on a load artifact rather than an actual
animation regression. All 3 attempts (original + 2 retries) failed
identically in the same run on 2026-09-09, consistent with systemic load
rather than a rare flake. Widened with headroom, same pattern as the
subprocess-timeout fixes in #162.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
alltomatos and others added 2 commits September 9, 2026 14:12
snapshot-tool-race.test.ts calls LayerNode.compile() directly instead of
going through AppNodeBuilder, which is what normally auto-injects
LocationServiceMap when a compiled graph needs it (see
app-node-builder.ts). Bypassing that left the graph with an unbound
LocationServiceMap node, throwing "Unbound layer node:
@opencode/example/LocationServiceMap" as an unhandled error between
tests — which then interrupted fibers in whatever test file ran next
(observed corrupting test/session/prompt.test.ts's MCP-instructions test
in CI on 2026-09-09, surfacing as an unrelated-looking InterruptError).

Same fix pattern already used in prompt.test.ts: add LocationServiceMap
to the compiled group and provide locationServiceMapLayer as its
replacement.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Not caused by this branch's changes — confirmed failing on dev HEAD
itself across 5+ consecutive CI runs since 2026-09-06, always with the
same InterruptError at a tight ~20.5s-21.1s cluster (too tight for
generic CI-load contention, unlike the subprocess-timeout flakes fixed
in this same PR). Root cause not yet identified; skipping so it stops
blocking unrelated feature PRs while #185 tracks the real investigation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
alltomatos added a commit that referenced this pull request Sep 10, 2026
…191)

Ports the fix from PR #162, which was opened against the wrong base
(agentui-telegram-channel) instead of dev.

Widens three subprocess-spawning test timeouts that were tight enough to
flake under CI-load cold-start variance, matching the rationale already
documented in test/lib/cli-process.ts (default bumped 30s->60s on
2026-09-08 for the same class of issue):

- run-process.test.ts: "exits nonzero promptly when the model is unknown"
  (25s->32s, harness bound 40s->45s) — observed a 25383ms run under CI
  load on 2026-09-09, past the old 25s bound.
- run-process.test.ts: "unknown stream finish preserves partial output and
  continues" (30s->45s) — tight enough against the 60s outer test timeout
  that CI load pushed the subprocess past it (killed mid-run, exitCode -1
  instead of 0) on 2026-09-09.
- lifecycle.test.ts: "stdin EOF exits cleanly" (5s->15s, Windows only) —
  Windows process teardown after stdin EOF was observed to take just over
  5s (5300ms) under CI load on 2026-09-09.

All three bounds were widened with headroom rather than shaved to the
exact observed miss, so a genuine hang would still fail loudly.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@alltomatos

Copy link
Copy Markdown
Owner Author

Closing as superseded by commit a2125e3 (#163) and PR #210 on dev.

@alltomatos alltomatos closed this Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant