fix(windows): run the better-sqlite3 boot-install via node, not npm.cmd+shell (#861 follow-up)#863
Open
ken-jo wants to merge 1 commit into
Open
fix(windows): run the better-sqlite3 boot-install via node, not npm.cmd+shell (#861 follow-up)#863ken-jo wants to merge 1 commit into
ken-jo wants to merge 1 commit into
Conversation
…md+shell (mksglu#861 follow-up) mksglu#861/e918eac fixed start.mjs's background install of the optional fetch deps (turndown/turndown-plugin-gfm/@mixmark-io/domino) by running npm's own CLI through node with shell:false (which honors `cwd`) instead of `npm.cmd` + shell:true (which DROPS `cwd` on Windows → npm targets the inherited cwd C:\Windows → `mkdir C:\Windows\node_modules` → EPERM every boot). The IDENTICAL npm.cmd cwd-drop still affected the native-dep install in hooks/ensure-deps.mjs — the reporter listed better-sqlite3 among the EPERMs, and that path was left unfixed. It runs on Windows + Node whenever better-sqlite3 is missing (codex/ marketplace clones, broken installs) — the same condition that triggers the turndown installs. This change absorbs e918eac's approach for the native-dep install: - hooks/ensure-deps.mjs: resolve npm-cli.js beside process.execPath and run it via execFileSync(process.execPath, [npmCliJs, "install", ...], { shell:false }) (honors cwd) + windowsHide; fall back to the npm.cmd shim only when npm-cli.js isn't beside node (no host regresses); surface failures to stderr (was silently swallowed behind stdio:"pipe" + an empty catch). - tests/hooks/ensure-deps.test.ts: pin the install branch to the node-CLI / shell:false path and the failure surfacing. Verified on Windows (ssh my-window): `node <npm-cli.js> install <pkg>` launched from cwd=C:\Windows installs into the target's node_modules (cwd honored, no EPERM). ensure-deps suite 26 pass; npm run build + typecheck pass. The rebuild paths in ensureNativeCompat use the same execSync(npm.cmd) but don't mkdir node_modules (no EPERM) and are left for a separate change. Bundles via CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
@ken-jo reviewed the Windows install path here. The change matches the #861 invariant: the primary native-dep install now runs I also checked it locally on Windows:
Looks good to me. |
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.
What / Why / How
Follow-up to #861 (
e918eac). That fix stopped the Windows boot-install EPERM for the optional fetch deps instart.mjsby running npm's own CLI through node (shell:falsehonorscwd) instead ofnpm.cmd+shell:true(which DROPScwdon Windows → npm resolvesnode_modulesfrom the inherited cwdC:\Windows→mkdir C:\Windows\node_modules→ EPERM). The identicalnpm.cmdcwd-drop still affected the native-dep install inhooks/ensure-deps.mjs— the reporter explicitly listedbetter-sqlite3among the EPERMs, and that path was left unfixed.With this PR applied, the
better-sqlite3boot-install uses the same cwd-honoring path, so it can no longer EPERM atC:\Windows\node_modules.This PR absorbs
e918eac's approach for the native-dep install:hooks/ensure-deps.mjs— resolvenpm-cli.jsbesideprocess.execPathand run it viaexecFileSync(process.execPath, [npmCliJs, "install", …], { shell:false })(honorscwd) +windowsHide:true; fall back to thenpm.cmdshim only whennpm-cli.jsisn't beside node (so no host regresses); surface failures to stderr (was silently swallowed behindstdio:"pipe"+ an emptycatch).tests/hooks/ensure-deps.test.ts— pin the install branch to the node-CLI /shell:falsepath + the failure surfacing (source guard, matching this file's existing Windows install can leave better-sqlite3 native binding missing in v1.0.107 #408/Codex CLI 0.131.0: MCP startup_timeout_sec=30s default trips when codex prewarm + cold-start stack #634 patterns; the bug is Windows-only and doesn't reproduce on Linux CI).ensureNativeCompatABI-rebuild paths are untouched (they usenpm.cmdtoo, but don'tmkdir node_modules→ no EPERM; left for a separate change).Affected platforms
Agent / CLI scope:
hooks/ensure-deps.mjsruns on every adapter that boots the MCP server viastart.mjs. No MCP/adapter behavior change; only the native-dep bootstrap install.OS scope:
npm.cmddropscwd). Reachable on Windows + Node whenbetter-sqlite3is missing (codex/marketplace clones, broken installs) — the same condition that triggers thestart.mjsturndown installse918eacalready fixed.ensureDeps()returns early on Bun, and POSIX already runs npm withshell:false(cwd honored). The node-CLI path is equivalent there.Scope notes:
Root Cause
e918eac(Windows: boot-time turndown/better-sqlite3 npm install fails EPERM (C:\Windows\node_modules) every session — shell:true drops cwd #861) fixedstart.mjsbut nothooks/ensure-deps.mjs, which still ranexecSync("npm.cmd install better-sqlite3 …", { shell: true }). On Windows,shell:true/npm.cmddoes not honor thecwdoption (verified), so the install targets the inherited cwd →C:\Windows\node_modules→ EPERM.ensureDeps()only early-returns on Bun (no modern-Node gate), so on Windows + Node it does run this install whenbetter-sqlite3is absent.Validation
npm run build+npm run typecheckvitest run tests/hooks/ensure-deps.test.tsssh my-window, Node v24.13)node <npm-cli.js> install is-numberlaunched fromcwd=C:\Windows→ installed into<target>\node_modules(installedInTarget:true,eperm:false);npm-cli.jsfound beside nodeBehavioral evidence:
execSync("npm.cmd install better-sqlite3", { shell:true })— same cwd-drop class as the (now-fixed) turndown installs → EPERM atC:\Windows\node_moduleswhenbetter-sqlite3is missing.node <npm-cli.js> install … (shell:false)installs into the plugin dir regardless of cwd — verified on real Windows fromcwd=C:\Windows.Cross-agent / cross-OS risk
Minimal. One install call site in
hooks/ensure-deps.mjs; no new dependency; no MCP/adapter/hook-routing change. The node-CLI path is the same mechanisme918eacalready shipped and validated forstart.mjs. POSIX behavior is unchanged (alreadyshell:false).Checklist
shell:falseinstall + failure surfacing), integrated into the existingtests/hooks/ensure-deps.test.tsnpm run build+npm run typecheckpasse918eacfor the native-dep install)next