fix: run install lifecycle scripts on Windows - #483
Conversation
`npm ci`/`npm install` fails on Windows before any dependency work
finishes:
npm error command C:\WINDOWS\system32\cmd.exe /d /s /c [ -d src ] && npm run build || true
'[' is not recognized as an internal or external command
'true' is not recognized as an internal or external command
npm runs lifecycle scripts through the platform shell — cmd.exe on
Windows — and the `prepare` script was POSIX sh. cmd.exe has neither `[`
nor `true`, so the script exited 1 and took the install with it, leaving
the tree unbuilt: no `dist/`, and no generated `hosted-contract.json`,
whose absence alone fails ~195 hosted unit tests.
Move `prepare` into `scripts/prepare.mjs`, plain Node like every other
script in this package, keeping both halves of the original intent: skip
the build when there is no source tree to compile (a published tarball
ships only `dist/`), and never fail the install.
The same `|| true` idiom was silently inert on Windows in the other two
install lifecycle scripts, so their guards move into the scripts
themselves: `postinstall` now swallows its own errors, and `preuninstall`
wraps the daemon-shutdown probe in `try`/`catch`. Both already tolerated
failure on POSIX; now they do so everywhere.
Adds `src/lifecycle-scripts.test.ts`, which fails the build if an install
lifecycle script reintroduces POSIX-only syntax, and exercises the
prepare script's skip, build, and failed-build paths against a throwaway
package.
🟢 No documentation gap found — medium confidenceThe automated review found no documentation gap in the supplied changes. This review is advisory and does not block merging. |
|
The failing check here is a pre-existing flake, not something this PR introduces.
Everything else is green — 1536 passed, plus Evidence that it is unrelated:
I tracked down the cause and opened #484 for it: the test overruns vitest's 5s default under parallel load on Windows, vitest aborts it mid- This PR needs a re-run to go green; I do not have the permissions to trigger one. |
Problem
npm ci/npm installfails on Windows before any dependency work finishes:npm runs lifecycle scripts through the platform shell —
cmd.exeon Windows — andpreparewas POSIX sh.cmd.exehas neither[nortrue, so the script exits 1 and takes the install with it. The tree is left unbuilt: nodist/, and no generatedhosted-contract.json.That last artifact is the expensive part. It is gitignored and produced only by
npm run build, so without itreadInstalledHostedContractIdentity()cannot resolve and every hosted surface fails. On this machine the unit suite went from 237 failures to 40 once the build actually ran; the remaining 40 are unrelated and pre-existing (symlinkEPERMwithout Developer Mode,/tmppaths — #476 — and timing flakes).CI does not catch this: GitHub's
windows-latestrunner happens to resolve a POSIX shell for npm scripts, sopreparesucceeds there. It only bites contributors on an ordinary Windows install.Fix
preparemoves intoscripts/prepare.mjs— plain Node, matching every other script in this package (clean-dist.cjs,copy-yaml.cjs,postinstall.js,check-*.mjs). It keeps both halves of the original intent:dist/);The same
|| trueidiom was silently inert on Windows in the other two install lifecycle scripts — ifpostinstall.jshad ever exited non-zero there,'true' is not recognizedwould have failed the install rather than rescuing it. Those guards move into the scripts themselves:postinstallswallows its own errors, andpreuninstallwraps the daemon-shutdown probe intry/catch. Both already tolerated failure on POSIX; now they do so everywhere.build,compileand the other developer scripts are untouched —&&chaining works fine incmd.exe. What breaks is chaining to a shell builtin it does not have.Regression coverage
src/lifecycle-scripts.test.ts(11 tests):preuninstall,postinstall,prepareandprepublishOnlythat rejects POSIX-only syntax — test brackets, chaining totrue/false/:, command substitution, inlineVAR=prefixes. Checked against the three old strings: it flags every one of them and passes all four new ones, so it is not vacuous.src/, build when there is, and warn-but-succeed when the build fails — exercised by copying the real script into a throwaway package so it resolves that directory as its root. No test-only override needed.Verification
npm run typecheck— clean.npx vitest run --project unit src/lifecycle-scripts.test.ts— 11 pass.npx vitest run --project unit— 3437 pass / 40 fail, down from 237 fail; no new failures.npm installwith exit 1 on Windows and succeeds with the new one.npm run preparenow completes the real build on Windows and regeneratescli-manifest.jsonandhosted-contract.json.