test(windows): unblock advisory by fixing 8 fixture incompatibilities#429
Merged
Conversation
Replace the hardcoded forward-slash SELF_PATH string with `path.resolve(__filename)` and compare absolute resolved paths. The previous comparison treated `packages/opencode/test/tool/agent-rename.test.ts` (forward slashes) as the canonical self path, but `path.relative` returns backslashes on Windows so the self-exclusion never matched and the sweep flagged its own pattern table. Refs #426
Two of the failing tests rely on a shell `sleep 0.2` command that does not behave the same across cmd / pwsh / powershell / gitbash, so gate them unix-only via the existing `unix(...)` helper: - "loop waits while shell runs and starts after shell exits" - "shell completion resumes queued loop callers" The other two tests do not touch any shell. They timed out at 3000ms purely because the Windows runner is slower for DB writes, fiber scheduling, and the inner polling loops. Add a `slowIOTimeout` constant (10s on Windows, 3s elsewhere) and apply it so Windows runs keep the same coverage: - "prompt submitted during an active run is included in the next LLM input" - "assertNotBusy throws BusyError when loop running" Refs #426
PowerShell 5.1 and cmd reparse embedded double quotes inside an inline `bun -e '<code>'` argument before forwarding it to the native exe, which mangled the JS source enough that bun exited 1 before any env-redaction assertion ran. PowerShell 7 (pwsh) avoided this through `\$PSNativeCommandArgumentPassing`, which is why the [pwsh] variant kept passing while [powershell] and [cmd] both failed at `expect(result.metadata.exit).toBe(0)`. Write the probe to a temp file and run `bun <script>` instead, which reduces the user command to a single quoted path argument that all four shells forward verbatim. Refs #426
… to LF The fixture compared the on-disk snapshot byte-for-byte against an LF-only prefix string. On a Windows runner with `core.autocrlf` enabled (Git for Windows default), the checked-out file has CRLF line endings, so `startsWith(prefix)` returned false in the fixture setup. Both `build:embedded-server emits the runtime entrypoint and wasm sidecars` and `built node server skill bootstrap` failed at this prefix check in tens of milliseconds, never reaching the actual build step. Fix at two layers: - Normalize CRLF to LF in `writeCurrentModelsFixture` and `expectModelsSnapshotUnchanged` so the fixture is platform-agnostic. - Add `.gitattributes` pinning `packages/opencode/src/provider/models-snapshot.js` to `text eol=lf`, so future Windows checkouts also keep LF. Refs #426
`prefs.preload` is built with `path.join`, which produces backslashes on
Windows. The previous `toEndWith("/preload/index.js")` assertion only
matched POSIX separators, so the test failed on Windows even though the
actual preload path was correct.
Use `path.join("preload", "index.js")` so the suffix matches whichever
separator the platform produces. Sandbox / contextIsolation / nodeIntegration
assertions stay unchanged.
Refs #426
`resolveRendererFile` runs the root through `path.resolve` internally, which on Windows prepends the CWD's drive letter to a drive-less posix path (`/Applications/...` -> `D:\\Applications\\...`). The test's expected value came from `join(root, ...)`, which only flips separators and never adds a drive, so received and expected diverged on Windows. Pre-resolve the fixture root once so both sides agree on the canonical form on every platform without changing what the test verifies. Refs #426
NTFS does not expose POSIX permission bits, so `fs.stat().mode` returns a fixed 0o666/0o444 mask on Windows regardless of `chmod 0o600`. The exact 0600-preservation assertion is meaningful only on POSIX. Gate the test with `test.skipIf(process.platform === "win32")` and keep the rest of `migrateDefaultAgent`'s coverage running on Windows. Refs #426
…ly ci-smoke Two unrelated test fixtures that both keep the windows-advisory advisory red: - `packages/app/src/no-mode-picker.test.ts`: the allowlist stores entries with POSIX-style paths but `path.relative` returns backslashes on Windows, so the allowlist never matched and the legitimate `context/global-sync/utils.ts` reference flagged as an offender. Normalize `relPath` to forward slashes before comparing, mirroring the same fix applied earlier to `agent-rename.test.ts`. - `packages/desktop-electron/scripts/ci-smoke.test.ts`: the "packaged smoke reports spawn failures with launch context" fixture relies on an empty file + chmod 0o755 to hit ENOEXEC and exercise the `Failed to launch desktop app:` branch. Windows has no equivalent (empty files via cmd exit cleanly through a different branch), and the assertion targets the spawn error format rather than Windows-specific launch semantics. Gate the test with `test.skipIf(process.platform === "win32")`. Refs #426
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR addresses Windows compatibility issues across the test suite and configuration. Changes include normalizing line endings in snapshots, handling platform-specific path separators, skipping Windows-incompatible tests, adjusting timeouts for slower I/O on Windows, and refactoring test execution to avoid shell quoting issues. ChangesWindows Test Compatibility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 7 minutes and 55 seconds. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements various fixes to ensure the test suite and build processes are compatible with Windows. Changes include normalizing line endings for generated files, using platform-agnostic path separators, and handling case-insensitive drive letters in path comparisons. It also skips POSIX-specific tests on Windows, increases timeouts for slow I/O, and refactors shell command tests to use temporary files to avoid quoting issues in PowerShell and cmd. I have no feedback to provide.
10 tasks
This was referenced May 11, 2026
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
1 participant
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.
Summary
Unblock the
windows-advisoryworkflow by repairing eight Windows-only testfixture failures. All eight commits touch test code, fixtures, or
.gitattributesonly; no product code is modified. The advisory has beenred on every run since it was added on 2026-05-03 (9 / 9 failures), and
roughly thirty individual test failures all trace back to fixtures that
assumed POSIX path separators, LF line endings, POSIX file modes, or POSIX
shell quoting.
Each commit is independently revertable and addresses one fixture surface:
test(windows): exclude self by absolute path in agent-rename sweep— replaces a hardcoded forward-slashSELF_PATHwithpath.resolve(__filename)(lowercased on win32 to absorb NTFS drive-letter casing drift). Fixes 17agent rename literal sweepfailures where the sweep was finding its own pattern table.test(windows): unblock 4 prompt-effect timeouts under runner load— adds aslowIOTimeoutconstant (10s on Windows, 3s elsewhere) for two no-shell tests that need more headroom on Windows runners, and switches two shell-dependent tests to the existingunix(...)helper because they rely onsleep 0.2(a Unix-only command).test(windows): load env-probe script from a file in bash env-filter test— writes the env-probe JS to a tmp file and runsbun <script>instead ofbun -e '<inline JS with embedded double quotes>'. PowerShell 5.1 and cmd reparse the embedded quotes before forwarding to native exes (PS7 avoids this via$PSNativeCommandArgumentPassing), which is why the[pwsh]variant kept passing while[powershell]and[cmd]failed atexpect(result.metadata.exit).toBe(0).test(windows): normalize CRLF in models-snapshot fixture and pin file to LF— normalizes CRLF to LF before the prefix comparison and pinspackages/opencode/src/provider/models-snapshot.jstotext eol=lfvia.gitattributes. Without this, Git for Windows' defaultcore.autocrlfflips line endings and the LF-only prefix string never matches, which made both embedded-server tests fail in 16-47ms before any build ran.test(windows): make preload bridge assertion platform-aware— replacestoEndWith("/preload/index.js")withpath.join("preload", "index.js")so the suffix matches both POSIX and Windows separators.test(windows): canonicalize the renderer-protocol fixture root— runs the fixture root throughpath.resolveonce so received and expected paths agree on the drive letter thatresolveRendererFileadds internally on Windows.test(windows): skip POSIX file mode assertion in migrateDefaultAgent— gates the0600 stays 0600assertion behindtest.skipIf(process.platform === "win32")because NTFS reports a fixed mode mask regardless ofchmod.test(windows): unblock primary-mode allowlist match and skip POSIX-only ci-smoke— same path-separator class as the agent-rename fix (allowlist stored in POSIX form vs. backslashpath.relative), plus awin32skip for a packaged-smoke fixture whose ENOEXEC trigger has no Windows equivalent and is testing the spawn error format itself.Why
windows-advisoryis non-blocking by design but supplies an early Windowssignal for desktop work. With every run red, the signal is unreadable; any
real future Windows regression would be lost in the noise. None of these
failures are recent regressions, they are fixture incompatibilities that
became visible only when the workflow itself landed.
Related Issue
Closes #426.
The two real Windows runtime concerns remaining (path canonicalization in
permission boundaries) live in #427 and are intentionally out of scope here.
Human Review Status
Pending. A human should make the final merge decision after reviewing the
final diff and verification evidence.
Review Focus
selfKeyhelper) sound across bothcase-sensitive POSIX volumes and case-insensitive NTFS volumes? The
lowercase fold is gated on
process.platform === "win32"so it should notalter POSIX behavior.
unix(...)skip in commit 2 acceptable for the twoloop waits while shell runstests? They lose Windows coverage butdepend on
sleep 0.2which is not portable; an alternative would be across-platform delay command, but adding that introduces its own quoting
questions.
bun <scriptfile>redesign in commit 3 still exercising theintended env-redaction surface? Behavior is identical from the bash tool's
point of view; only the user command argument format changed.
Risk Notes
.gitattributes. No product behavior changes..gitattributesrule is scoped to a single generated source file(
packages/opencode/src/provider/models-snapshot.js) so it should notaffect any other working-tree files. Existing developer checkouts on
Windows that already have CRLF on disk will continue to work because the
fixture also normalizes at runtime as a belt-and-suspenders.
prompt-effecttests (theshell-
sleepones) and themigrateDefaultAgentPOSIX-mode case. Theunderlying behavior is still covered on Linux/macOS; the Windows skip is
intentional because the assertion is platform-conditional or
Unix-shell-dependent.
How To Verify
The Windows runtime effect is verified only by the next
windows-advisoryrun on this PR's CI.
Screenshots or Recordings
None. No visible UI changes.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishMaintainer: please add labels
ci,windows,test, and a priority label.Summary by CodeRabbit
Bug Fixes
Tests
Chores