Skip to content

fix(ci): green the Windows leg, cap openai<3, surface desktop failures - #831

Merged
agentforce314 merged 2 commits into
mainfrom
fix/ci-windows-and-openai3
Aug 13, 2026
Merged

fix(ci): green the Windows leg, cap openai<3, surface desktop failures#831
agentforce314 merged 2 commits into
mainfrom
fix/ci-windows-and-openai3

Conversation

@agentforce314

Copy link
Copy Markdown
Owner

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).

All Submissions:

New Feature Submissions:

  • Have you added an explanation of what your changes do and why you'd like us to include them?

Description

Fixes # .

Changes proposed in this pull request:

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

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

Test Results

     4 files     966 suites   27m 16s ⏱️
14 608 tests 14 591 ✅ 17 💤 0 ❌
29 894 runs  29 839 ✅ 55 💤 0 ❌

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>
@agentforce314
agentforce314 merged commit d4561d2 into main Aug 13, 2026
7 checks passed
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