fix(ci): green the Windows leg, cap openai<3, surface desktop failures - #831
Merged
Conversation
Diagnosed from the actual run artifacts (junit XMLs pulled anonymously for
the last four PR runs; the deterministic windows-latest failure set was
stable across all of them, plus one cross-platform break that appeared this
morning). Three red jobs, three causes:
1) openai 3.0.0 (released 2026-08-12, hours before today's runs) broke BOTH
pytest legs. The repo floats `openai>=1.109.1`, and v3's httpx2 migration
moves requests off the httpx layer we are coupled to on both sides of the
SDK boundary: `http_client=httpx.Client(verify=False)` stops carrying the
transport, and the stream-abort guard closes an httpx `Response` that no
longer exists behind SDK streams. Verified in a fresh floating venv:
`chat()` under a patched `httpx.Client.send` sails straight past the spy
onto the real network — exactly the CI failure in
test_xai_requests_go_to_chat_completions ("no HTTP request was
attempted"). Capped `openai>=1.109.1,<3` (requirements.txt + pyproject,
same precedent as the deliberate mcp<2 cap); with the cap (2.54.0) the
test passes. Lift only with a real httpx2 migration.
2) windows-latest pytest: the stable 8-test failure set plus one flake.
- Bare `["bash", "-lc", ...]` argvs resolve to the WSL launcher on the
runner (exits immediately, no distro): supervisor + reaper tests now
spawn via shell_platform.bash_argv / popen_tree_kwargs, the way
production does (test_bash_timeout_vs_esc, test_kill_shell_for_agent).
- test_bash_timeout_vs_esc keeps the pre-port `< 3.0s` kill bound on
POSIX; on Windows `taskkill /T` cannot reach the MSYS2 sleep grandchild,
so elapsed is bounded by deadline + one _KILL_REAP_TIMEOUT_S drain and
the timed_out/interrupted flags carry the regression guard there.
- activate_conditional_skills_for_paths: os.path.relpath raises
ValueError cross-drive on Windows (runner workspace on D:, TEMP on C:);
a path we cannot express relative to cwd is outside it — skip it like
the ../absolute guards (src/skills/loader.py; the D:-drive condition is
unreachable on single-drive dev boxes, exercised directly).
- test_reaper_terminal_state_is_eligible spawned with cwd=Path("/tmp"),
which only works where C:\tmp happens to exist — the runner throws
NotADirectoryError (WinError 267). Now tempfile.gettempdir(), and the
wait loop requires notified+evict_after (the reaper commits those in a
second registry update; breaking on terminal status alone raced it).
- test_user_migration_preserves_symlinks_as_symlinks compared
os.readlink literally against a target that Windows returns with a
\?\ prefix; compare resolve() on both sides, keep is_symlink().
- test_failure_retries_with_exponential_backoff asserted wall-clock gaps
between real asyncio sleeps; Windows' ~15.6ms timer granularity ate the
75ms-floor-under-80ms margins. Now captures the delays the uploader
schedules (instance-attribute _sleep shadow) and pins the exact
20/40/80 schedule — deterministic on every platform, and unlike a
lower bound it also catches delays that are too long.
- test_stream_watchdog: _on_timeout sets `fired` under the lock but
closes the response after releasing it — deliberate ordering the
consumer relies on — so a fixed sleep then close.assert_called() races
that window under runner load (the observed flake). Added a bounded
_await_fire poll and applied it to the three same-pattern siblings.
3) Desktop (windows-latest) has been red since the job landed and its
failures were invisible: vitest job logs need an authenticated API call
and the job uploaded no artifact. The run step now writes a junit XML
that the existing Test Results publisher picks up automatically (it
globs every artifact), so the next red names its tests. Best-evidence
fix applied alongside: the electron vitest project gets the same
testTimeout: 30_000 the ui project already documents for Windows
cold-start starvation — its tests shell out for real (git clone/worktree
fixtures, spawned helpers), which a 2-core Defender-scanning runner
starves past the 5s default. Not reproducible locally: full vitest is
green here both against the working tree and a pristine worktree +
fresh npm ci.
Verification: full pytest suite on Windows 10,000 passed / 0 failed; all
touched test files re-run green after every edit; xai test verified failing
on 3.0.0 and passing capped in a CI-equivalent venv; desktop typecheck +
full vitest green twice; junit reporter flags validated against vitest
4.1.10. POSIX semantics preserved by construction (relpath never raises
there, bash_argv is `-lc` identical, tight timing bound restored, watchdog
polls assert the same fired→close contract).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Test Results 4 files 966 suites 27m 16s ⏱️ Results for commit 6616d70. ♻️ This comment has been updated with latest results. |
…op worktree asserts Two failures remained on the PR run after d21547e (junit artifacts pulled anonymously from run 31579051653): 1) ubuntu pytest: test_bg_bash_completion_notifies_the_model failed with "no completion notification delivered" after burning its full 5s poll (0.11s in every passing run sampled). The test observed delivery by peek-polling the process-global notification queue - but anything else alive in the process may legitimately drain that queue: the agent-server worker loop (exercised by tests/server/*, which run earlier) consumes exactly this mode (agent_server.py drain_pending_notifications). The byte-identical failure (assert [] after ~5s) reproduces by running the old test alongside a daemon thread draining task-notification entries: the reaper delivered, the poll only ever saw an already-drained queue. The test now records delivery at the enqueue call with a record-and-forward spy (production path untouched), waits on a deadline (15s, vs a flat 5s a loaded 2-core runner can blow through), and additionally asserts the registry notified/status/exit_code state. Passes with and without the simulated drainer; the old version fails under it. 2) Desktop (windows-latest): the two git-worktree-ops tests that compare paths through fs.realpathSync failed with RUNNER~1 vs runneradmin. The runner TEMP is an 8.3 short name; git reports canonical long paths; fs.realpathSync resolves symlinks but preserves short-name components, so the two sides can never meet. fs.realpathSync.native (GetFinalPathNameByHandle) expands both - and is what main.ts already uses for the same reason. Reproduced locally by pointing TMP/TEMP at a short-form temp dir (os.tmpdir() = ...\CL9C14~1): old assertions fail exactly as on CI, new ones pass; full electron+ui vitest (4539 tests) and typecheck green under a normal env too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Diagnosed from the actual run artifacts (junit XMLs pulled anonymously for the last four PR runs; the deterministic windows-latest failure set was stable across all of them, plus one cross-platform break that appeared this morning). Three red jobs, three causes:
openai 3.0.0 (released 2026-08-12, hours before today's runs) broke BOTH
pytest legs. The repo floats
openai>=1.109.1, and v3's httpx2 migrationmoves requests off the httpx layer we are coupled to on both sides of the
SDK boundary:
http_client=httpx.Client(verify=False)stops carrying thetransport, and the stream-abort guard closes an httpx
Responsethat nolonger exists behind SDK streams. Verified in a fresh floating venv:
chat()under a patchedhttpx.Client.sendsails straight past the spyonto the real network — exactly the CI failure in
test_xai_requests_go_to_chat_completions ("no HTTP request was
attempted"). Capped
openai>=1.109.1,<3(requirements.txt + pyproject,same precedent as the deliberate mcp<2 cap); with the cap (2.54.0) the
test passes. Lift only with a real httpx2 migration.
windows-latest pytest: the stable 8-test failure set plus one flake.
["bash", "-lc", ...]argvs resolve to the WSL launcher on the runner (exits immediately, no distro): supervisor + reaper tests now spawn via shell_platform.bash_argv / popen_tree_kwargs, the way production does (test_bash_timeout_vs_esc, test_kill_shell_for_agent).< 3.0skill bound on POSIX; on Windowstaskkill /Tcannot reach the MSYS2 sleep grandchild, so elapsed is bounded by deadline + one _KILL_REAP_TIMEOUT_S drain and the timed_out/interrupted flags carry the regression guard there.firedunder the lock but closes the response after releasing it — deliberate ordering the consumer relies on — so a fixed sleep then close.assert_called() races that window under runner load (the observed flake). Added a bounded _await_fire poll and applied it to the three same-pattern siblings.Desktop (windows-latest) has been red since the job landed and its
failures were invisible: vitest job logs need an authenticated API call
and the job uploaded no artifact. The run step now writes a junit XML
that the existing Test Results publisher picks up automatically (it
globs every artifact), so the next red names its tests. Best-evidence
fix applied alongside: the electron vitest project gets the same
testTimeout: 30_000 the ui project already documents for Windows
cold-start starvation — its tests shell out for real (git clone/worktree
fixtures, spawned helpers), which a 2-core Defender-scanning runner
starves past the 5s default. Not reproducible locally: full vitest is
green here both against the working tree and a pristine worktree +
fresh npm ci.
Verification: full pytest suite on Windows 10,000 passed / 0 failed; all touched test files re-run green after every edit; xai test verified failing on 3.0.0 and passing capped in a CI-equivalent venv; desktop typecheck + full vitest green twice; junit reporter flags validated against vitest 4.1.10. POSIX semantics preserved by construction (relpath never raises there, bash_argv is
-lcidentical, tight timing bound restored, watchdog polls assert the same fired→close contract).All Submissions:
New Feature Submissions:
Description
Fixes # .
Changes proposed in this pull request: