Repository navigation
feat: Windows support - #17
Open
BrasaStudios wants to merge 18 commits into
Open
BrasaStudios wants to merge 18 commits into
BrasaStudios wants to merge 18 commits into
Conversation
Git for Windows turns on core.autocrlf system-wide, so a plain clone on Windows checked out CRLF files and tests that read the fixtures failed before any Windows-specific code ran. text=auto leaves binary files as they are.
npm is npm.cmd on Windows, and since Node fixed CVE-2024-27980 (April 2024) a .cmd file starts only through a shell, so the scaffold stopped at npm install. The arguments are fixed tokens, so on Windows they are joined into one command line; that also avoids the DEP0190 warning Node 24 prints when an argument list is passed with shell: true. Linux and macOS keep execFileSync.
copyTemplates compared each template path, which holds backslashes on Windows, with skip names written with forward slashes, so a file meant to be skipped was copied anyway. path.sep is / on Linux and macOS, so nothing changes there.
…Windows The file field is a site path such as assets/js/view.js that later steps join and compare, but on Windows it was cut from a native path and kept its backslashes.
findBinary looked only for the bare name, so gh.exe and npx.cmd were never found on Windows. A .cmd or .bat CLI starts only through a shell, so its probe arguments are checked and joined into one command line. A probe that times out is ended with taskkill /t, because a negative pid (the process group) means nothing on Windows. Linux and macOS behave as before.
--stop reads the command line of the pid in the pid file from ps. Windows has no ps, so --stop reported a running server as already exited and left it running. There the command line comes from WMI, and the background server starts without opening a console window.
A plain absolute path is not a valid import specifier on Windows (C:\ reads as a URL scheme). pathToFileURL, which the production loader already uses, gives the same module on every platform.
A directory symlink needs administrator rights or Developer Mode on Windows; a junction needs neither and serves these links the same way. Three tests and the e2e scaffold make one.
Three tests compared native paths, or messages that print them, with names written with forward slashes. On Linux and macOS path.sep is already /, so the comparisons there are unchanged.
The fake CLIs were shell scripts, which Windows cannot run, and the stray-process check used pgrep. On Windows each fake is a .cmd that runs a small Node script, and the check asks WMI. The shell fakes stay for Linux and macOS.
chmod 0o500 does not stop writes to a directory on Windows. There the test denies the current account write access with icacls and expects EPERM, the error Windows reports.
Like init-site, the runner could not start npm.cmd or npx.cmd without a shell. Its arguments are script names, flags, local paths and URLs, so on Windows they are checked and joined into one command line, and an argument that would need quoting fails the run loudly.
The site's verify:release and verify:proposals scripts set PUBLIC_INDEXABLE and PROPOSALS with POSIX shell syntax, which cmd.exe, npm's script shell on Windows, cannot run. run-all now takes two switches and sets the variables for every gate, so the scripts are plain node commands in any shell. The second is --with-proposals because every gate already has --proposals <file> for the proposals file. A site scaffolded before this, which sets the variables itself, keeps working.
On Windows find is the text-search command, so the check failed wherever the Unix tools from Git Bash were not first on PATH. A recursive readdirSync gives the same answer on every platform.
The sync checks the skills out in a scratch repository, which inherits core.autocrlf. Git for Windows turns that on system-wide, so every copied file came out with CRLF line endings. The checkout now passes -c core.autocrlf=false; the test runs its sync with core.autocrlf=true forced, so the new check fails without the fix on every platform.
fetchPaced paces request starts, so once the first request had finished, the second waited delayMs minus that request's duration. A first local request on Windows often takes more than 10 ms, and the test failed 5 runs in 8 there. Starting the clock before the first request measures what the pacer promises on every platform; a pacer that did not wait still fails it.
windows-latest joins the matrix, so both Node lines run there too and Windows support cannot quietly break later. The e2e cache key already includes runner.os.
The README requirement and the FAQ answer. The skills' references keep their POSIX shell commands, so the FAQ says to run them in Git Bash, with MSYS_NO_PATHCONV=1 so that Git Bash leaves arguments such as /thank-you/ alone.
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 #14. As you asked there: small commits, no change on Linux or macOS, a Windows job in CI, and the
README and FAQ updated.
What changes
.gitattributeswith* text=auto eol=lf. Git for Windows turns oncore.autocrlfsystem-wide, so a plain clone checked out CRLF files.
.cmdCLIs: since CVE-2024-27980 Node starts a.cmdfile only through a shell. OnWindows only,
init-site,preflightand the e2e runner start them through a shell, with their fixed-tokenarguments joined into one command line. That avoids the DEP0190 warning Node 24 prints for an argument list
passed with
shell: true, and an argument that would need quoting fails loudly instead.preflightalso finds.exeand.cmdCLIs on PATH and ends a hung probe withtaskkill /t.assets.mjsrecords forward-slash asset paths, andinit-site's template copy comparesforward-slash names.
review-serve --stopreads the server's command line from WMI, since Windows has nops, and thebackground server opens no console window. The loopback binding is yours from fix(convert): review-serve listens on loopback only #16; this PR leaves it alone.
run-all --releaseand--with-proposals, as you suggested, soverify:releaseandverify:proposalsare plainnodecommands that cmd.exe can run. The second is--with-proposalsbecauseevery gate already has
--proposals <file>(skills/parity-gates/scripts/lib/gate-cli.mjs:12); happy torename it. A site scaffolded before this, which sets
PUBLIC_INDEXABLEorPROPOSALSitself, keeps working.sync-launch-skillschecks out withcore.autocrlf=false: on Windows it wrote every synced file withCRLF line endings. Its test now forces
core.autocrlf=true, so the check fails without the fix on anyplatform.
.cmdfakesand a WMI stray-process check for preflight,
icaclsfor the unwritable-directory test, forward-slashcomparisons,
readdirSyncin place offind(on Windows,findis the text-search command), and thefetchPaced delay test timed from the first request's start, as the pacer counts it (timed after that
request, it failed 5 runs in 8 on Windows, where a first local request often takes over 10 ms).
windows-latestjoins the matrix, so both Node lines run there.(
PUBLIC_INDEXABLE=true npm run build), so the FAQ says to run them in Git Bash, withMSYS_NO_PATHCONV=1so that
/thank-you/reaches Node unchanged.One dependency: ethanplusai/website-launch-skills#8
The e2e test's prelaunch step runs the vendored
pre-launch, which has the same npm problem(ethanplusai/website-launch-skills#7, fixed in ethanplusai/website-launch-skills#8). Until that is synced, that
one step fails on Windows.
A heads-up for that sync: launch-skills
mainalso carries157f873, which restores thePLACEHOLDER_URLSimport the pinned
0fbb707lacks. With it, pre-launch's placeholder check flags the fixture'smailto:...@example.comlinks (16 strings) andprelaunch-reconcileblocks step 10 on every platform, notonly Windows. We found this while testing the sync and left the fixture and the reconcile rules to you.
Tested
npm testfrom PowerShell with no Git Bash tools on PATH: 853 of 853 tests pass(two runs), with the fix(pre-launch): npm run build on Windows website-launch-skills#8 change applied by hand to the vendored
0fbb707pre-launch. No DEP0190 warnings.
(run 41344977350): Linux passes
on Node 22.23.4 and 24.22.1; Windows passes 852 of 853 on both, the e2e prelaunch step being the one failure
until the sync.
npm run hygienepasses on every commit.process.platform === 'win32'or does the same there (path.sepis/). macOS is not in the CI matrix.🤖 Generated with Claude Code