fix(farm): make the ca tools suite pass on Windows CI (#541, #542) - #543
Merged
Conversation
The last two failures keeping #521's coverage-union windows cell red. Both are pre-existing defects that only a second host could surface: this tree had never run on Windows in CI, because the tools job is ubuntu-only. #541 - writeWorktreeFile canonicalized the worktree ROOT via realpath but resolved the caller's ABSOLUTE destination as-is, then compared the two. Those are the same directory and different strings whenever a link or a Windows 8.3 short name sits on the path, so a caller building a destination from the root it had itself passed in was refused for writing inside its own worktree. Same root cause as #539, one module down. Only the ANCESTOR chain is canonicalized; the final component stays literal. Resolving the leaf would follow a symlink AT the destination - precisely what the per-segment lstat checks exist to catch - so pre-resolving it would silently undo this module's contract. Pinned by a test that writes through an inside-pointing alias and asserts the target is untouched. Like #539 this TIGHTENS the guard: a destination whose ancestors traverse a link out of the root is now caught by containment rather than relying on a later check. #542 - every farm launch in farm.test.ts is a real subprocess and none were tracked. A vitest timeout left one running with the temp directory as its cwd, so the afterEach rmSync failed EBUSY and the log reported the TEARDOWN rather than the timeout that caused it. Children are now reaped before the directory is removed, and the removal tolerates Windows releasing handles asynchronously after a kill. The heaviest case gets a budget drawn from MEASUREMENT rather than a nudge: 3553ms isolated on a developer box against the 5000ms default - 29% headroom before any load - and 5331ms under full-suite load on a CI runner. Now 30s, about 6x the slowest observed run and still bounded, so a genuine hang fails rather than sitting until the job timeout. The case itself is untouched: FARM_CONCURRENCY stays 6 and the bounded-at-11 / true-total-12 assertions stand (#515 AC-4, #542 AC-2). Buying headroom by shrinking the six-way path would delete the coverage the case exists for. 4/4 mutants killed on the #541 logic. Two survived the first round and both were real gaps: nothing pinned "do not pre-resolve the leaf", and the fail-closed branch never executed - realpath is now injectable so it can, the same fix #539 needed for the same reason. Also corrects worktree-fs.test.ts's header, which still claimed CI runs this tree on ubuntu ONLY. That stopped being true when #521 added the windows cell, and the stale note is what made a Windows-only refusal easy to not look for. 537 tests pass. ca 2.10.3 -> 2.10.4. Claude-Session: https://claude.ai/code/session_01WJgVfZw7J81PB7mwpHyUxx
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.
Closes #541. Closes #542.
The last two failures keeping #521's
Coverage union <os: windows-latest>cell red. Both are pre-existing defects that only a second host could surface — this tree had never run on Windows in CI, because thetoolsjob is ubuntu-only.#541 — an absolute destination inside your own worktree was refused
writeWorktreeFilecanonicalized the worktree root viarealpath, but resolved the caller's absolute destination as-is, then compared the two. Same directory, different strings, whenever a link or a Windows 8.3 short name is on the path.So a caller building a destination from the root it had itself passed in was refused for writing inside its own worktree. Same root cause as #539, one module down.
Red proof, using two real spellings of one directory (a junction):
Before
After — all four as expected, sentinel untouched.
Only the ancestors are canonicalized
The final component stays literal. Resolving the leaf would follow a symlink at the destination — precisely what the per-segment
lstatchecks exist to catch — so pre-resolving it would silently undo this module's contract. Pinned by a test that targets an inside-pointing alias and asserts the real file is untouched.Verified current behaviour first rather than assuming it:
It tightens, it does not loosen
A destination whose ancestors traverse a link out of the root is now caught by containment rather than relying on a later check. Same direction as #539.
#542 — a timed-out test abandoned a live subprocess
Every farm launch in
farm.test.tsis a real child process and none were tracked. A vitest timeout left one running with the temp directory as its cwd, soafterEach'srmSyncfailed:The teardown error is louder than the timeout that caused it, so the log names the wrong problem. Children are now reaped before the directory is removed, and the removal tolerates Windows releasing handles asynchronously after a kill.
The budget is measured, not nudged
Now 30 s: ~6× the slowest observed run, still bounded, so a genuine hang fails rather than sitting until the job timeout.
The case itself is untouched.
FARM_CONCURRENCYstays at 6 and the bounded-at-11 / true-total-12 assertions stand (#515 AC-4, #542 AC-2). Buying headroom by shrinking the six-way path would delete the coverage the case exists for.Verification
Full tools suite 537 passed. 4/4 mutants killed on the #541 logic.
Two survived the first round, and both were real gaps:
EACCES/ELOOPare not portably inducible, sorealpathis now injectable — the same fix farm's worktree containment refuses a legitimate root when two path spellings disagree (Windows 8.3) #539 needed, for the same reason.Also
worktree-fs.test.ts's header still claimed CI runs this tree on ubuntu only. That stopped being true when #521 added the Windows cell, and a stale note like that is exactly what makes a Windows-only refusal easy to not look for. Corrected.ca2.10.3 → 2.10.4.