fix: make repair tests portable on Windows#376
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 27, 2026, 5:21 PM ET / 21:21 UTC. Summary Reproducibility: yes. from source inspection and contributor Windows proof, but I did not run current main on Windows in this Linux review environment. Current main lacks the Windows command-resolution and batch-launch path, while the PR body shows the repaired .cmd path passing. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge after maintainer review accepts the cmd.exe validation-launcher boundary and the automation-sensitive helper changes. Do we have a high-confidence way to reproduce the issue? Yes from source inspection and contributor Windows proof, but I did not run current main on Windows in this Linux review environment. Current main lacks the Windows command-resolution and batch-launch path, while the PR body shows the repaired .cmd path passing. Is this the best way to solve the issue? Yes, with maintainer review: the patch follows the existing adjacent Windows launcher pattern, adds focused coverage, and keeps the fix scoped to repair portability. The remaining question is maintainer acceptance of the cmd.exe shell boundary, not a concrete code defect. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ae63b16d6c74. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
a183094 to
ca6ff16
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
ca2f607 to
3f76fb9
Compare
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
What Problem This Solves
ClawSweeper's repair validation path and several test fixtures assumed POSIX command lookup, LF-only source reads, executable fake
ghshims, and/-only path containment. That made Windows local validation fail even when the runtime behavior being tested was otherwise portable.Why This Change Was Made
This updates repair command execution and GitHub CLI helpers to support explicit tool shims on Windows, makes
.cmd/.batvalidation commands run throughcmd.exewith double-escaped batch arguments, fixes state-root publish containment with platform-native path checks, and normalizes source-fixture reads where assertions inspect tracked files. The test fixtures now use explicit Node-backed fake tools instead of depending on extensionless executables being discoverable throughPATH.The follow-up quoting hardening replaces regex-based Windows argument quoting with an explicit scanner that doubles trailing backslashes and backslashes before embedded quotes before applying the existing
cmd.exemetacharacter escaping.User Impact
Maintainers can run the ClawSweeper repair and unit suites from Windows without patching local tool shims or line endings. The state publish path also no longer rejects valid state-root paths on Windows, and Windows batch validation launchers preserve spaces, metacharacters, embedded quotes, and trailing backslashes in arguments.
Evidence
Head proofed:
3f76fb935334e82131ce043c360bcca9903895f9.Exact-head note: the top commit is the session-only empty ClawSweeper re-review trigger.
git diff --exit-code HEAD~1 HEAD --statproduced no tree diff, so the proofed code tree is the PR head tree. The code fix commit isc9b398bd2071b958bf8d8d2f48006f2c2d5bdb0c.pnpm install --frozen-lockfilepassed earlier on this branch; dependencies did not change in the follow-up.pnpm exec oxfmt --write src/repair/command-runner.ts test/repair/command-runner.test.tspassed.node --test test\repair\command-runner.test.tspassed on Windows after the quoting hardening: 4 tests, 4 pass..cmdlauncher probe round-tripped arguments containing embedded quotes, trailing backslashes, double trailing backslashes, spaces plus trailing backslashes,^,%PATH%, and]withequal: true.pnpm run checkpassed after the quoting hardening, includingcheck:active-surface,check:limits,build:all,lint,test:unit,test:repair, changed/full coverage, andformat:check. Final summary: 1118 tests, 1117 pass, 1 skipped; coverage 100% lines, branches, and functions for the covered report.codex review -c service_tier='fast' --uncommittedpassed with no actionable correctness issues. The review also reranpnpm run build:repair,node --test test/repair/command-runner.test.ts, and a 116-case real.cmdargument probe with 0 mismatches.Redacted Windows terminal excerpt for the focused batch launcher path:
Additional
.cmdedge-case probe after the fix:{ "equal": true, "args": [ "quote\"x", "trailing\\", "double\\\\", "space trailing\\", "a b\\", "a b\\\"", "caret^x", "percent%PATH%", "brack]" ] }Full check closeout: