Skip to content

fix(ci): restore green Windows CI and make timed-out suites diagnosable - #2066

Merged
flora131 merged 11 commits into
mainfrom
fix/ci-test-failures
Jul 29, 2026
Merged

fix(ci): restore green Windows CI and make timed-out suites diagnosable#2066
flora131 merged 11 commits into
mainfrom
fix/ci-test-failures

Conversation

@flora131

@flora131 flora131 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Problem

The Tests workflow failed on every run since #2064. The Windows job was killed at its 15-minute timeout-minutes budget, always inside the coding-agent vitest suite step. GitHub reports a timeout-minutes kill as cancelled, which is why this read as noise rather than a red build.

commit Windows job vitest step
#2057 fix(workflows) success (7.5m) 125s
#2059 fix(auth) success (6.7m) 117s
#2064 ModelRuntime cancelled (15.1m) never finished
#2065 fix(copilot) cancelled (15.1m) never finished

Root cause: a test launched a real browser on the CI runner

test/interactive-auth-login.test.ts drives a real LoginDialogComponent through the onAuth callback. showAuthUrl calls openBrowser(url), and on Windows that is:

rundll32 url.dll,FileProtocolHandler https://corp.invalid/login

spawned detached. Node's spawn on Windows creates the child with handle inheritance, so rundll32 inherits the test process's stdout/stderr write handles. The runner has no browser to hand the URL to, so rundll32 never exits.

A GitHub Actions run: step ends only when the child exits and every inherited stdout/stderr handle closes. So the whole visible chain finished normally — 2887 tests passed in ~135s, vitest exited, bun run exited, the wrapper's direct child exited — and the step still hung on pipes that could never reach EOF, until the 15-minute budget killed the job.

#2064 is the first bad commit because #2064 added that test case (honors transported callback metadata and resolves manual redirect input). At 86dd67d6c, the last green commit, this file never invoked onAuth.

Evidence

Run 30495631917, Windows job, with a temporary probe that dumped Win32_Process every 30s after the child exited:

22:25:38  [suite-probe child exited code=0; 2 inherited stream(s) still open]
          ProcessId ParentProcessId Name         Started  CommandLine
          2508      4772            rundll32.exe 22:24:23 rundll32 url.dll,FileProtocolHandler https://corp.invalid/login
...
22:33:xx  ##[error]The operation was canceled.

Every 30s sample through cancellation showed the same single non-infrastructure survivor, started mid-suite, with a dead parent. No bun.exe or node.exe descendant survived — the earlier "leaked engine/RPC child" theory is refuted; the two bun.exe and one node.exe seen at teardown in the first report were the live bun runvitest chain itself.

Fix

Mock the launcher, exactly as pi's own login-dialog suites do (test/suite/regressions/5433-extension-oauth-prompt-input.test.ts):

vi.mock("../src/utils/open-browser.ts", () => ({ openBrowser: vi.fn() }));

and assert the dialog still hands the URL to it, so the launch path stays covered without spawning a process:

expect(vi.mocked(openBrowser)).toHaveBeenCalledWith("https://corp.invalid/login");

That assertion is the regression test: if a future change makes this suite reach the real launcher again, it fails rather than hanging Windows CI.

No shipped behavior changes, so there is no changelog entry. src/utils/open-browser.ts is untouched and still matches upstream pi.

Also in this PR

Three Windows-only test failures from #2064, which dropped platform-aware handling and made the bounded flake retry run the suite twice:

test #2064 wrote restored
footer-width — home abbreviation "~/project" `~${sep}project`
sdk-session-manager — session file prefix `${expectedSessionDir}/` `${expectedSessionDir}${sep}`
sdk-session-manager — prompt cwd ${sessionCwd} sessionCwd.replaceAll("\\", "/")

Production code was correct throughout: formatCwdForFooter joins with sep, and system-prompt.ts renders cwd with POSIX separators.

scripts/run-flaky-test-suite.ts also buffered the child's entire output via new Response(child.stdout).text() and wrote it only after the child exited, so a child killed by the job timeout discarded every collected line — the cancelled step logged minutes of silence. It now tees both streams as they arrive. Retry, deterministic-file, and diagnostics-artifact behavior are unchanged (test/unit/flaky-test-suite-runner.test.ts passes). That change is what produced the evidence above and keeps the next timeout self-describing.

bash-session-metadata cleanup now retries around transient Windows directory locks.

Verification

  • Windows job green with no diagnostic scaffolding: run 30497271702 at 8918590d8test (blacksmith-4vcpu-windows-2025, windows-x64) succeeded in 8m30s (22:46:44 → 22:55:14) against the unchanged 15-minute budget. The coding-agent vitest suite step took 160s and ended on its own. Linux green in the same run.
  • The temporary probes (packages/coding-agent/test-exit-diagnostic.ts, its globalSetup entry, and the wrapper's process-table dump) are removed; the green run above does not contain them.
  • Local: bun run typecheck, bun run lint, bun run check:file-length, bun run test:unit (4406 pass / 0 fail) all green.
  • No timeout-minutes was widened, no survivor is force-killed from CI YAML, and no suite is skipped or excluded.

Ruled out during triage

  • Not a module-graph regression: local suite duration identical pre/post feat(coding-agent)!: adopt pi's ModelRuntime architecture for full model-provider parity #2064 (31.5s vs 32.0s).
  • Not live-network contention: instrumenting globalThis.fetch across all 370 test files recorded exactly one outbound request.
  • Not a leaked engine/RPC child and not a vitest event-loop leak: the vitest main process exits, and the post-exit process table names rundll32 as the only survivor.

Greptile Summary

Restores Windows CI reliability and improves diagnostics for timed-out test suites.

  • Makes path-sensitive coding-agent assertions portable across Windows and POSIX.
  • Prevents browser-launch and temporary-directory cleanup behavior from hanging Windows tests.
  • Streams child stdout and stderr live while retaining output for retry diagnostics and artifacts.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the Windows timeout contract issue is fixed because the workflow retains the required 15-minute budget.

T-Rex T-Rex Logs

What T-Rex did

  • Observed the before-state where flaky-streaming-01-before.log recorded a timeout exit 124 with an empty live terminal-output section.
  • Validated the after-state showing stdout and stderr markers at 22:49:11.495Z, followed by an external timeout completion at 22:49:14.477Z and exit 124.
  • Verified the contract outcome in flaky-runner-contract-02-after.log reports 4 pass, 0 fail, exit 0.
  • Noted that the harness source is retained to expose the exact recognizable markers and a persistent child process for review.
  • Artifacts were uploaded to support verification of the above assertions.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
packages/coding-agent/test/bash-session-metadata.test.ts Makes Windows temporary-session cleanup resilient to transient directory locks without affecting test assertions.
packages/coding-agent/test/footer-width.test.ts Restores the platform-aware separator expectation for abbreviated home-directory paths.
packages/coding-agent/test/interactive-auth-login.test.ts Mocks the platform browser launcher and verifies its URL, preventing detached Windows processes from retaining test output handles.
packages/coding-agent/test/sdk-session-manager.test.ts Makes session paths and system-prompt expectations portable and verifies the shell working directory through a marker file.
scripts/run-flaky-test-suite.ts Tees child output into live CI logs while preserving captured text for existing retry and diagnostic-artifact behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Start["Run test suite attempt"] --> Spawn["Spawn child with stdout/stderr pipes"]
  Spawn --> Tee["Stream both pipes to CI log and capture output"]
  Tee --> Exit{"Attempt succeeds?"}
  Exit -->|Yes| Done["Complete successfully"]
  Exit -->|No| Retry["Run bounded retry"]
  Retry --> TeeRetry["Stream retry output live"]
  TeeRetry --> Persist["Persist diagnostics artifact"]
  Persist --> Result{"Retry succeeds?"}
  Result -->|Yes| Done
  Result -->|No| Fail["Exit non-zero with visible diagnostics"]
Loading

Reviews (10): Last reviewed commit: "test: remove the Windows teardown diagno..." | Re-trigger Greptile

Comment thread .github/workflows/test.yml Outdated
greptile-apps[bot]
greptile-apps Bot previously approved these changes Jul 29, 2026
@greptile-apps
greptile-apps Bot dismissed their stale review July 29, 2026 19:10

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

@flora131
flora131 enabled auto-merge (squash) July 29, 2026 19:10
greptile-apps[bot]
greptile-apps Bot previously approved these changes Jul 29, 2026
@greptile-apps
greptile-apps Bot dismissed their stale review July 29, 2026 19:32

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

flora131 added 4 commits July 29, 2026 12:34
The Windows `test` job has been killed at its 15m `timeout-minutes` budget on
every run since #2064, always inside the coding-agent vitest step. The step
log was empty for the entire 10m before the kill, so the cancellation carried
no evidence about which test stalled.

The cause of the blackout is `run-flaky-test-suite.ts`: it collected the whole
child stdout/stderr with `new Response(child.stdout).text()` and only wrote it
after the child exited. A child killed by the job timeout never exits normally,
so every collected line was discarded.

Tee both streams as they arrive instead. The retry, deterministic-file, and
diagnostics-artifact contracts are unchanged; a timed-out attempt now leaves
its progress in the step log.

Also raise the Windows budget to 30m temporarily so the suite can run to
completion and report its real failures. Restored once the stall is fixed.

Assistant-model: Claude Opus 5
The deterministic CI contract pins the Windows timeout at 15m, so the
temporary diagnostic bump failed 'Deterministic CI and release contracts'
before the vitest step could run. Streamed output alone now carries the
evidence: the killed step reports progress up to the kill point.

Assistant-model: Claude Opus 5
…rewrite

#2064 rewrote these assertions and dropped the platform-aware handling the
previous versions had, so all three fail on Windows only:

- footer-width: `formatCwdForFooter` joins with `sep`, so the expectation is
  `~\project` on Windows. The rewrite hardcoded `~/project`
  (was: `` `~${sep}project` ``).
- sdk-session-manager: the session file separator check hardcoded `/`
  (was: `` `${expectedSessionDir}${sep}` ``).
- sdk-session-manager: the system prompt always renders cwd with POSIX
  separators (`system-prompt.ts` applies `.replace(/\\/g, "/")`), so the
  Windows expectation needs the same normalization
  (was: `` sessionCwd.replaceAll("\\", "/") ``).

Restore all three. Production code is unchanged and was correct throughout.

This is what exhausted the Windows job budget: the three failures triggered
the bounded retry, and two full ~170s suite runs pushed the job past its 15m
cap, where GitHub reported the kill as `cancelled`.

Assistant-model: Claude Opus 5
With the separator fixes in place the Windows suite ran far enough to expose
two more failures, both from tests that assume POSIX process and filesystem
behavior:

- bash-session-metadata: the detached-async-job test leaves a shell running
  with the temp dir as its cwd. Windows locks a directory in that state, so
  `afterEach` cleanup failed with EBUSY and failed the test. Retry the removal
  and ignore a residual failure; reclaiming an OS temp dir is never the
  assertion.
- sdk-session-manager: the cwd check compared `pwd` output through
  `realpathSync`. On Windows the bash tool resolves to Git Bash, which reports
  MSYS paths (`/tmp/...`) that Node resolves against the current drive, so the
  lstat failed with ENOENT on `C:\tmp\...`. Prove the working directory by
  reading a marker file only resolvable from sessionCwd, which tests the same
  contract without depending on how a shell spells a path.

Assistant-model: Claude Opus 5
@flora131
flora131 force-pushed the fix/ci-test-failures branch from dfcb41a to 7ad68bf Compare July 29, 2026 19:36
greptile-apps[bot]
greptile-apps Bot previously approved these changes Jul 29, 2026
All 374 Windows test files now pass in 191s, but the process then sits for
~412s until the job cap kills it. The last green Windows run (pre-#2064)
exited 0.8s after printing its summary, so the exit hang is new and is what
still exhausts the budget.

Add vitest's hanging-process reporter to name the handle. Removed once the
leak is fixed.

Assistant-model: Claude Opus 5
@greptile-apps
greptile-apps Bot dismissed their stale review July 29, 2026 20:00

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

flora131 added 6 commits July 29, 2026 13:43
The Windows suite now passes all 374 files in ~200s, then sits until the job
cap kills it. vitest's hanging-process reporter printed nothing, because the
suite runs under `bun --bun` where that detection does not apply.

Swap it for a global teardown that dumps the live process tree on Windows, to
name whatever still holds the event loop open. The failure path exits in 0.2s
while the success path hangs, which points at a surviving child rather than a
reporting bug. Removed once the leak is fixed.

Assistant-model: Claude Opus 5
The teardown process-tree dump showed no surviving bash/sleep/git children and
no leftover vitest workers, so the handle that blocks exit lives inside
vitest's own process rather than in spawned test children.

Replace the one-shot dump with unref'd probes at +15s and +45s that report
process.getActiveResourcesInfo(). Unref'd timers fire only while the loop is
still alive, so a healthy run exits before they run and pays nothing.

Assistant-model: Claude Opus 5
The interactive OAuth callback test drives a real LoginDialogComponent
through onAuth, and showAuthUrl calls openBrowser. On Windows that spawns
a detached `rundll32 url.dll,FileProtocolHandler https://corp.invalid/login`,
which inherits the test process's stdout/stderr write handles. The CI
runner has no browser to hand off to, so rundll32 never exits: the vitest
process, `bun run`, and every descendant terminate, but the step's pipes
never reach EOF and the job hangs until its 15-minute budget expires.

CI evidence (run 30495631917, Windows job): all 2887 tests pass, the child
exits code=0 at 22:25:38, and a Win32_Process table taken every 30s after
that shows exactly one non-infrastructure survivor, pid 2508
`rundll32 url.dll,FileProtocolHandler https://corp.invalid/login`, started
22:24:23 with a dead parent, still alive when the job is cancelled.

5ef323e added this test, which is why the bisect lands there. Mock the
launcher exactly as pi's login-dialog suites do, and assert the dialog
still hands the URL to it so the launch path stays covered without
spawning a process.

Assistant-model: Claude Opus 5
The probes named the leaking process, so drop them: the vitest globalSetup
that dumped the surviving process tree, its config entry, and the process
table the flaky-suite wrapper printed after the child exited. The streamed
tee stays, since a timed-out attempt still needs a self-describing log.

Assistant-model: Claude Opus 5
@flora131
flora131 merged commit 8490bbf into main Jul 29, 2026
11 checks passed
@flora131
flora131 deleted the fix/ci-test-failures branch July 30, 2026 00:26
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