Skip to content

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

Merged
alltomatos merged 1 commit into
devfrom
fix/ci-subprocess-timeouts-dev
Sep 10, 2026
Merged

alltomatos merged 1 commit into
devfrom
fix/ci-subprocess-timeouts-dev

Conversation

@alltomatos

Copy link
Copy Markdown
Owner

Issue for this PR

Ports #162 (opened against the wrong base branch, agentui-telegram-channel, instead of dev) — same fix, correct target.

What does this PR do?

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.

How did you verify your code works?

  • bun test --timeout 60000 test/cli/run/run-process.test.ts test/cli/acp/lifecycle.test.ts — 19 pass, 0 fail.
  • bun run typecheck (root, via pre-push hook, all 30 workspace tasks) passes clean.

Checklist

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

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>
@github-actions

Copy link
Copy Markdown

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

@github-actions

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.

@alltomatos
alltomatos merged commit 7dbd132 into dev Sep 10, 2026
6 of 18 checks passed
@alltomatos
alltomatos deleted the fix/ci-subprocess-timeouts-dev branch September 10, 2026 21:27
alltomatos pushed a commit that referenced this pull request Sep 11, 2026
….spec.ts

The e2e (linux) job on this PR's CI failed with a strict-mode violation:
getByRole('button', { name: 'Close Tab' }) resolves to two elements
whenever a tab is open and showing the not-found fallback, because
Playwright's getByRole name matching is case-insensitive by default and
two different i18n keys produce near-identical accessible names
(common.closeTab -> "Close tab", session.error.notFound.closeTab ->
"Close Tab"). Confirmed this is not caused by this PR's diff (which
doesn't touch packages/app at all): the same failure reproduces
identically on dev HEAD (9ef686e) and on unrelated PRs #190/#191.

A fix already exists as open PR #196 (closes tracking issue #195),
verified there by reproducing the failure and the fix locally. Ported
the same one-line change (adding exact: true) into this PR so CI goes
green here without waiting on #196 to merge first.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AGh7o4NA8jUjwakDzZo12e
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