Skip to content

ci(test): fix the 6 flaky unit failures on GitHub-hosted runners - #1

Merged
rodsnyder merged 6 commits into
devfrom
ci-fork-flaky-unit-tests
Sep 13, 2026
Merged

rodsnyder merged 6 commits into
devfrom
ci-fork-flaky-unit-tests

Conversation

@rodsnyder

@rodsnyder rodsnyder commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Result

Run 34749489714 on e3f2367e: unit (linux) ✅ · unit (windows) ✅ · e2e (linux) ✅ · e2e (windows) ❌ (Playwright flake, see below). First green unit matrix on the fork since the Blacksmith→GitHub-hosted runner sweep. Linux unit green 4 runs running; Windows unit went 7 → 5 → 2 → 0 failures over the iterations.

Why

After the runner sweep, test on dev failed on every push. None of it was random — every failure had a runner-shaped mechanism.

What (6 commits)

  1. Windows, 5× Ripgrep > …windows-latest has no rg; first-use download + Expand-Archive blew the 5 s per-test budget. choco install ripgrep before the suite so which("rg.exe") short-circuits. Linux, 1× run-process regression test — 15 s bound eaten by cold CLI spawn under concurrent siblings; harness default 30 s. (d9b5972)
  2. Linux run-process lottery — 11 CPU-bound CLI spawns concurrent on 4 vCPUs; a different test lost each run. --max-concurrency 4 on packages/opencode only. Windows opencode:test had never run here (turbo stops at the first failing package); it's serial on win32 by harness design and takes ~20 min, so the Windows step gets 60. (21915a4)
  3. e2e session-todo-dock-navigation failed 3/3 on Linux — fixed 700 ms sampling window raced the navigation back. sampleDock gains untilPresent. Green every run since. (22e9326)
  4. Windows path tests (2 → 5 → 2 → 0) — they strip the drive letter and expect the file back, which needs temp on the checkout's drive (D:, while %TEMP% is C:) and not under a single-letter dir (/a/_temp parses as A:). Temp at D:\tmp via GITHUB_ENV, Windows only. (f1758f3, e893afe) The last two hardcoded C:../outside.txt; they now use the cwd's drive — still drive-relative, which is the point of the test. (e3f2367)

Not fixed — your call

The app e2e suite is timing-marginal on GitHub-hosted runners: e2e (windows) failed 3 of 6 runs on two toBeVisible timeouts (open-file-expansion … trailing Windows separator, file-browser … switching file tabs); e2e (linux) failed once on review-line-comment hover with 4 others flaky-then-passed. Options: accept and re-run; raise Playwright retries for CI in packages/app; or stop gating the fork on e2e. I'd take the retries bump, but it's a posture choice.

Backlog a4ef43f9.

🤖 Generated with Claude Code

Windows (5x Ripgrep > ...): windows-latest ships no rg, so RipgrepBinary
downloaded and Expand-Archive'd ripgrep on first use, which alone blew the
5s per-test timeout and SIGTERM'd every Ripgrep test behind it. Install
ripgrep via choco before the suite so which("rg.exe") short-circuits the
download. Linux was unaffected: tar extraction fits inside the budget.

Linux (1x run-process "exits nonzero promptly when the model is unknown"):
the 15s wall-clock bound was tighter than the harness default and was
being consumed by cold `bun src/index.ts` startup under concurrent
siblings on ubuntu-latest, not by the anomalyco#27371 hang it guards against.
Use the harness default 30s bound (outer 60s, matching sibling tests);
a real hang still fails via the harness kill + duration assertion.

Backlog a4ef43f9.

Co-Authored-By: Claude Opus 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

Hey! Your PR title ci(test): fix the 6 flaky unit failures on GitHub-hosted runners doesn't follow conventional commit format.

Please update it to start with one of:

  • feat: or feat(scope): new feature
  • fix: or fix(scope): bug fix
  • docs: or docs(scope): documentation changes
  • chore: or chore(scope): maintenance tasks
  • refactor: or refactor(scope): code refactoring
  • test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

@github-actions

Copy link
Copy Markdown

The following comment was made by an LLM, it may be inaccurate:

…tep 60 min

First CI pass on this branch moved the failures rather than removing them:

Linux: the regression test now passes, but 4 sibling run-process tests hit
the 30s bound instead. run-process.test.ts runs 11 CLI spawns concurrently
(bun's default cap is 20) and each is a cold `bun run src/index.ts` - CPU
bound, not I/O bound - so on a 4-vCPU ubuntu-latest they starve each other
and which ones lose is a lottery. --max-concurrency 4 on packages/opencode's
test script (the only package with CLI-subprocess tests; a turbo `--`
passthrough is unsafe because app's script is compound) matches the width
to the cores.

Windows: core:test now passes all 1099 (the ripgrep install worked), which
unmasked opencode:test - turbo had never reached it here because it stops at
the first failing package. The harness runs its CLI tests serially on win32
by design, windows-latest spawns slowly, and the package alone blew the
20-minute step timeout. 60 minutes for the Windows matrix leg only.

Backlog a4ef43f9.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rodsnyder and others added 3 commits September 12, 2026 06:26
e2e/regression/session-todo-dock-navigation failed 3/3 attempts on
GitHub-hosted ubuntu-latest at line 92 (`openSamples.length > 0`). The
"returning to the source session" step opened a fixed 700ms rAF sampling
window and then navigated back; when navigation + first paint took longer
than 700ms the window closed before the dock existed, so there were zero
present samples. The assertion is about the FIRST present frame being
already fully open (no replayed animation), which needs "sample until
present", not a clock. sampleDock gains an `untilPresent` option that stops
five frames after the dock is first seen, capped at 10s; the other three
call sites are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With core:test and the run-process lottery fixed, opencode:test completes
on windows-latest (21 min, 3652 tests) with exactly 2 failures, both
deterministic: external-directory "normalizes Windows path variants to one
glob" and read "normalizes read permission paths on Windows". Each strips
the drive letter from a temp path and expects it to resolve back to the
same file - an assumption that temp and the checkout share a drive. On
windows-latest the checkout is D:\a\opencode and %TEMP% is C:\Users\...,
so the drive-less path resolves against the current drive to
D:\users\runneradmin\appdata\... and the file is not there. Blacksmith kept
both on C:. Point TMP/TEMP at runner.temp (D:\a\_temp) for the Windows leg
so the tests' premise holds; Linux is untouched (empty value, reads TMPDIR).

Backlog a4ef43f9.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The runner.temp override (f1758f3) traded two failures for five. The
Windows path tests strip the drive letter and expect the file to resolve
back; that needs temp on the checkout's drive (drive-less paths resolve
against the current drive), which runner.temp gave - but the normalizer
also reads Git-Bash style /x/... as drive X:, so /a/_temp from D:\a\_temp
became A:\_temp and three shell-permission tests joined the two originals.
Temp must be on D: and NOT under D:\a. A small step derives the drive from
GITHUB_WORKSPACE, creates <drive>:\tmp, and exports TMP/TEMP via
GITHUB_ENV; Linux is untouched.

Backlog a4ef43f9.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…not C:

With temp on the checkout's drive (e893afe) the Windows leg is down to
these two: "asks for external_directory permission for drive-relative
PowerShell paths [pwsh|powershell]". They hardcode `C:../outside.txt`, a
path relative to C:'s current directory, and expect it to resolve against
the test cwd - true only when the cwd is on C:. The other five Windows path
tests need temp on the checkout's drive, which on windows-latest is D:, so
the two constraints cannot both hold with a literal C:. Take the drive
letter from tmp; the test still exercises drive-relative resolution.

Backlog a4ef43f9.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@rodsnyder
rodsnyder merged commit 70279ac into dev Sep 13, 2026
9 of 10 checks passed
@rodsnyder
rodsnyder deleted the ci-fork-flaky-unit-tests branch September 13, 2026 10:04
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