ci: make the Windows desktop-install job blocking - #176
Merged
Conversation
The lane is green — run 31450975252, job 93655123817, all eight steps success: npm ci in 84s, Electron unpacked, the VC++ guard satisfied, and the four-file test:desktop:win-install set passing. That is the first fully green run, after two rounds of narrowing. Listed it in the `needs` of the existing `All required checks pass` aggregator. That is this repo's own gate, so this is a one-line file change rather than a branch-protection setting. The conditional-job trap was checked rather than assumed: a required check that never runs blocks PRs forever, and this job is gated on the `frontend` lane. The aggregator counts only `result == 'failure'` and renders `skipped` as passing, so the job blocks on frontend PRs and is a no-op on Python-only ones — the same property that lets installer-tests and the disabled e2e-desktop already sit in that list. All 16 needs entries were verified to resolve to real job ids. Sequenced on purpose: green first, then blocking. Making a red job required would have blocked every PR — and this job had been red on main through three merges precisely because nothing was waiting on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
This was referenced Aug 11, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 11, 2026
The last open item from the Windows-lane work. Seven files now. Inspected before adding rather than adding and hoping: over-including unverified files caused rounds #174 and #175, and since #176 the lane is a required check, so being wrong now blocks everyone's PRs rather than just mine. The sweep covered Windows-hostile syscalls, separator-dependent assertions and shell assumptions. It found a third instance of the defect class behind #177: `assert.match(a, /\/[0-9a-f]{16}\.sock$/)` against controlSocketPath, which builds with path.join — a leading-slash regex can only match on POSIX. Now asserted on path.basename(a), which is separator-independent and is what the sun_path limit is actually about. Checked under both path flavours. Everything else in the file is safe for stated reasons rather than by assumption: the /var/folders/ check is a negative assertion; ControlPath passes through baseSshOptions verbatim, which does no path joining; the `cd '/home/me/project'` assertion is a remote shell command and remotes are always POSIX; and the path.join calls create directories inside the two tests that already carry win32 guards. Also corrects the workflow header, which #179 had made false — it still said ssh-connection was POSIX-only and that making it cross-platform was tracked as follow-up, which is precisely what #179 did. Remaining exposure is unknown-unknowns across ~40 tests that cannot be executed from this sandbox. If the lane goes red on this, the fix is to drop ssh-connection back out of the list — one word, and the test repairs stand on their own. Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8 Co-authored-by: Claude <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
2 participants
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.
The Windows lane is green, so it can now block.
It's green
Run 31450975252, job
93655123817— all eight stepssuccess:npm ciwith postinstalltest:desktop:win-install(4 files)First fully green run, after the two rounds of narrowing in #174 and #175.
How "required" works here
Not branch protection —
ci.ymlalready has anAll required checks passaggregator withif: always()over aneedslist, and that's the real gate. So this is a one-line file change, no admin rights or repo settings.The trap I checked rather than assumed
A required check that never runs blocks PRs forever, and this job is gated on the
frontendlane — so it isskippedon Python-only PRs. The aggregator's logic:skippedcounts as passing. So the job blocks on frontend PRs and is a no-op on Python-only ones. That's the same property that already letsinstaller-testsand the hard-disablede2e-desktopsit safely in that list.Also verified all 16
needsentries resolve to real job ids.Sequencing, on purpose
Green first, then blocking. Making a red job required would have blocked every PR in the repo — and this job had been red on
mainthrough three merges (#173, #174, #175) precisely because nothing was waiting on it. That gap is what this closes.Follow-ups, unchanged
electron-buildercan't be imported under vitest on Windows (likely upstream).update-relaunch.test.ts:54andwindows-hermes-path.test.ts:177are Linux-only by accident, not design.Generated by Claude Code