Skip to content

CI: type-check Windows in the queue, in parallel (#1385) - #1386

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-ci-windows-compile-check
Oct 11, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-ci-windows-compile-check

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1385

Problem

#1378 put e2e-build-windows in the lean merge queue to catch cfg(unix) slips (#1324), because lean PR runs skip Windows. The job reuses the full *e2e-build-steps: a cargo test --no-run that codegens and links every CLI test binary (~7.2 min on a hosted 4-vCPU runner), then a 41 MB bundle upload that no lean job downloads. It also waits on clippy, only to read outputs.reuse, which is never true on a merge_group run. #1385 found it was the last job before ci-ok in 27 of 30 queue runs (p50 9.45 of 9.6 min).

Fix (.github/workflows/ci.yml)

  • New job windows-compile-check runs on lean merge_group runs (vars.CI_SCOPE != 'full'). It has no needs, so it starts at t≈0. It runs cargo check --locked -p socket-patch-cli --all-features --lib --bins --tests, so the lib, the shipped bin (including cfg(not(test)) code) and every CLI test target are type-checked on Windows. There's no bundle and no upload.
  • e2e-build-windows now runs only on full scope, schedule and dispatch, where e2e-windows and cargo-vex-matrix-windows consume its artifact. In the queue, exactly one of the two jobs runs.
  • Cache: a build-mode cache can't satisfy cargo check, and a merge-queue ref can't share a cache it saves. So the check uses its own check-windows-latest key, and the nightly also runs the job only so that main saves that key (save-if: github.ref == 'refs/heads/main').
  • ci-ok needs the new job. The required check names (ci-ok, clippy) and the aggregation are unchanged. The Linux Depot mapping and kill switch are untouched; this job is hosted Windows, like the one it replaces.
  • scripts/tests/test_ci_scheduling.py: a new test pins the split (if-conditions, no needs, check-not-build, the cache key and save-if, no upload). The preflight test exempts this job only from the clippy-ancestor rule. The no-always() rule still applies to it.

Runner usage

jobs job-min
Lean PR run, before and after 29 ~30.5 (unchanged: neither Windows job runs on pull_request)
Lean merge_group, before (run 38099894100, e2e-build-windows 8.2 min) 30 ~38.2
Lean merge_group, after (estimated) 30 ~33–34 (Windows check est. 3–4 min, warm cache)
Nightly +1 Windows job +~4 (the cache writer)

Merge-queue critical path: the Windows leg should now finish around minute 4–5, before coverage (~7.1). merge_group created → ci-ok should drop from p50 ~9.6 to ~7.1–8 min. The Windows timing is an estimate. Lean CI can't exercise this job before it enters the queue (a dispatch runs the full-scope e2e-build-windows instead), so the first queue run of this PR is the measurement. I'll post it on #1385.

Trade-offs (from /code-review high, kept on purpose)

  • No linking on Windows in the queue. A Windows-only link error, or a post-monomorphization const-eval error, now shows up in the nightly instead of the queue. The crates are pure Rust, and CI perf: e2e-build-windows — full Windows test build is the merge-queue critical path in 27/30 runs (~2.5 min off every merge) #1385 proposed exactly this. Running the binaries on Windows was already nightly-only.
  • Starts before clippy passes. An entry that fails clippy (~1.3 min) also holds a Windows runner until merge-queue-fail-fast.py cancels the run, usually within one 30 s poll. Gating on clippy would put 1.3 min back on every merge, and that is the latency this PR removes.

Fixed from the same review in 7ca55aa: a restore-only reuse of the nightly build cache (it couldn't satisfy cargo check, so every dependency was re-checked), the bin not being checked in non-test mode, the over-broad test exemption, and the hard-coded coupling to the build job's cache key.

Validation

  • python3 -m pytest scripts/tests -q: 323 passed, 4 skipped. The old preflight test failed on the new job before the exemption was added.
  • cargo check --locked -p socket-patch-cli --all-features --lib --bins --tests passes locally (Linux, cold: 2m48s on 4 cores).
  • actionlint v1.7.7: the same kinds of finding as on main, plus one more instance of its existing YAML-anchor limitation (env: *e2e-build-env, the same line as e2e-build-windows).
  • zizmor --offline: 23 low, 0 medium/high, the same as main. There is one more suppressed finding.
  • The workflow loads as YAML, every fromJSON matrix is unchanged, and ci-ok.needs equals every other job.
  • No Rust or cfg code changed, so there's no full-scope dispatch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XkkCjPuNE3yz1M8n4EB44N


Generated by Claude Code


Note

Low Risk
Workflow and scheduling tests only; merge-queue Windows coverage drops linking in lean scope, so link-only failures may surface in nightly/full runs instead of the queue.

Overview
Lean merge queue no longer runs e2e-build-windows (link ~60 test binaries + artifact upload). It runs a new windows-compile-check job that **cargo check**s socket-patch-cli (lib, bins, tests) on Windows without waiting on clippy, so Windows validation can start immediately and shorten the queue critical path.

e2e-build-windows is limited to CI_SCOPE=full, nightly, and workflow_dispatch—where e2e-windows / cargo-vex-matrix-windows still need built binaries. Nightly also runs windows-compile-check so main can write the separate check-windows-latest cache (build caches do not satisfy cargo check).

ci-ok now depends on windows-compile-check. test_ci_scheduling.py pins the split (conditions, no artifact, parallel-start exemption) and documents that lean queue Windows coverage is type-check only, not link/run.

Reviewed by Cursor Bugbot for commit 9471a78. Configure here.

Assisted-by: Claude Code:claude-opus-5-5
The lean merge queue's only Windows job built and linked every CLI
test binary after waiting for clippy, then uploaded a bundle nothing
in lean scope downloads. It set the queue's critical path in 27 of 30
runs, about 9.5 minutes of a 9.6-minute run.

A new windows-compile-check job now runs on lean merge_group runs
instead. It starts at once and runs cargo check over the CLI and every
CLI test target, which still catches a cfg(unix) slip (#1324). Full
scope, nightly and dispatch keep the full e2e-build-windows build that
their Windows legs consume. Pull requests are unchanged.

Fixes #1385

Assisted-by: Claude Code:claude-opus-5-5
A cargo build cache can't satisfy cargo check, so restoring the nightly
build cache left the queue check re-checking every dependency. The
nightly run now also runs windows-compile-check under its own key, and
only main saves it, so queue entries restore a cache that matches.

The check now also covers the lib and the socket-patch bin in non-test
mode, so code under cfg(not(test)) is type-checked on Windows too.

The scheduling test now exempts the job only from the clippy-ancestor
rule. The rule against always() still applies to it.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 11, 2026 13:01
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 11, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Ready for review at 7ca55aa.

  • CI: 37/37 non-skipped checks green on the head; no main-wide failures.
  • Bugbot: reviewed 7ca55aa, no findings.
  • Mergeable: no conflicts with main; no CHANGELOG.md changes.

Slack announcement pending (no Slack send tool in this run; the next run retries).


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. In the lean merge queue, replaces the Windows full test build (e2e-build-windows: link ~60 test binaries + 41 MB upload nobody downloads, gated on clippy) with a new windows-compile-check job that runs cargo check -p socket-patch-cli --all-features --lib --bins --tests in parallel with clippy. Full scope, nightly and dispatch still build and run everything on Windows. Aim: cut the queue's critical path from ~9.6 to ~7–8 min (#1385).

Risk: low–medium. CI-only, no Rust changes. Trade-off accepted in the description: a Windows-only link error now surfaces in the nightly instead of the queue. The new job can't run before this PR's own merge_group run, so that first queue run is its real test.

Look here

  • ci.yml:1017-1051 — new job: if: (lean merge_group + schedule), no needs, own cache key saved only on main
  • ci.yml:1052-1056 — e2e-build-windows drops merge_group; its consumers (e2e-windows, cargo-vex-matrix-windows) were already full-scope only
  • ci.yml:2707 — ci-ok needs the new job
  • test_ci_scheduling.py:44-51, :89 — exemption from the clippy-ancestor rule and the new pinning test

Verified. Read the full diff; confirmed the &e2e-build-env anchor precedes its use, only full-scope jobs consume e2e-build-windows, and --lib --bins --tests checks both test and shipped (non-test) configurations. Ran python3 -m unittest discover -s scripts/tests: 326 OK (5 skipped). CI 87/87 (37 success, 50 skipped, 0 failed); Cursor Bugbot success.

Changes I made. None.

Open questions. None blocking. Heads-up: this PR and #1383 both edit ci.yml and test_ci_scheduling.py and conflict textually; whichever merges second will need a main merge (the burn-down handles that).

Auto-merge is armed: approving sends it straight to the merge queue.


Generated by Claude Code

e2e-build-windows keeps this branch's full-scope-only condition and
gains main's pr_unchanged gate. The scheduling test pins the combined
condition.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 9471a78. Configure here.

Merged via the queue into main with commit 41659c1 Oct 11, 2026
55 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-windows-compile-check branch October 11, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-perf CI / merge-queue performance finding (profiler routine) Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI perf: e2e-build-windows — full Windows test build is the merge-queue critical path in 27/30 runs (~2.5 min off every merge)

3 participants