feat(platform): make Windows a first-class dev platform, prove it in CI#8
Open
JoshKappler wants to merge 1 commit into
Open
feat(platform): make Windows a first-class dev platform, prove it in CI#8JoshKappler wants to merge 1 commit into
JoshKappler wants to merge 1 commit into
Conversation
The suite had never run on Windows (the CI runner is Linux-only). An empirical run on native Windows showed 280/292 with every failure in one of three clusters, all fixed here; the full battery (vitest, eval all, bench) now passes on a real Windows machine. - src/eval/dev-tools.ts (new) + 6 eval suites + test/resilience/hook-degrades.test.ts: spawn dev tools via process.execPath against each tool's JS entry instead of the node_modules/.bin shims. On win32 the shims are .cmd batch files, which execFile refuses without shell:true (CVE-2024-27980), so every suite that shells out died with EINVAL. This was 11 of the 12 failures. The win32 ternaries picking tsx.cmd are gone. - src/mcp/server.ts: serve --socket maps filesystem-style paths into the named-pipe namespace on win32 (exported resolveSocketPath, used by the CLI, tests, and embedders); pipe paths skip the unlink cleanup since pipes have no fs entry. Node has no AF_UNIX support on Windows and binding a plain path failed with EACCES; this was the 12th failure and a broken shipped feature. test/mcp/socket.test.ts now probes readiness by connecting instead of existsSync (pipes never exist on disk); the POSIX socket-file assertion stays, platform-gated. AGENT_USAGE.md + serve help text document the mapping. - HOME -> os.homedir() at all 5 remaining sites (claude-code/install.ts x3, skill/index.ts, cli.ts logError, config/config.ts tilde expansion). Native Windows leaves HOME unset, so --global installs, codex skill writes, and ~ expansion silently produced relative paths. config/defaults.ts and the codex adapter already did this right; now everything does. test/adapters/home-paths.test.ts + test/config/expand-path.test.ts cover it (the global-install case is win32-gated so POSIX runs never touch a real profile). - src/extract/deterministic/generated-markers.ts: emit forward-slash paths (same .split(sep).join("/") idiom repo-pagerank uses) so fact text and path_globs stop drifting across platforms; test added. - .github/workflows/ci.yml: windows-latest job running the same battery (tsc, biome, vitest, eval all, bench). - .gitattributes (new): checkout stays LF everywhere; the default autocrlf=true on windows-latest would CRLF every file and fail the biome format gate before the first test ran. - README.md + docs/STATUS.md: test count 292 -> 297. Verified on native Windows 11 (Node 22): npx tsc --noEmit clean; npx biome check src test clean; npx vitest run 297/297; npx tsx src/cli.ts eval all 19/19 suites PASS (counters check reads 297/297 live); npx tsx src/cli.ts bench p95 13ms PASS. Previously failing on this machine: 6 eval wrapper tests, 5 hook-degrades tests, 1 socket test; all green now.
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.
The suite has never run on Windows (CI is a single Linux runner). On my machine it was 280/292, with every failure in one of three clusters. This PR fixes all three. The full battery now passes on native Windows (vitest 297/297,
eval all19/19 suites, bench p95 13ms), and a windows-latest CI job proves it on every push.The three clusters:
.cmdspawns. The eval suites and the resilience test spawn dev tools through thenode_modules/.binshims. On win32 those are.cmdbatch files, whichexecFilerefuses withoutshell: true(Node's CVE-2024-27980 hardening), so everything that shelled out died with EINVAL. Fix: a sharedsrc/eval/dev-tools.tsthat spawnsprocess.execPathagainst each tool's real JS entry. No shims, noshell: true, and the win32 ternaries pickingtsx.cmdare gone.serve --socket. Node has no AF_UNIX on Windows, so daemon mode failed with EACCES.resolveSocketPath(exported) maps filesystem-style paths into the named-pipe namespace on win32; pipe paths skip the stale-socket unlink since pipes have no fs entry. The socket test probes readiness by connecting instead ofexistsSync, with the POSIX socket-file assertion kept platform-gated. AGENT_USAGE.md and the serve help text document the mapping.process.env.HOME, which native Windows leaves unset, so--globalinstalls, codex skill writes, the crash log, and~expansion in config paths silently produced relative paths. All five now useos.homedir(), matching whatconfig/defaults.tsand the codex adapter already did. The generated-markers extractor also emitted backslash paths into fact text on Windows; it now uses the same forward-slash normalization repo-pagerank had.Also
.gitattributes(* text=auto eol=lf): windows-latest checks out CRLF by default, which would fail the biome format gate before the first test runs.New tests:
test/adapters/home-paths.test.ts,test/config/expand-path.test.ts, and a forward-slash case in the extractor tests. Counters 292 -> 297. The global-install test is win32-gated so POSIX runs never touch a real profile.Verification (Windows 11, Node 22):
Note: all three PRs bump the README/STATUS test counters off the same base (292), so whichever merges second and third will conflict there. I'll rebase the counters as each one lands.