fix(windows): stop boot-install EPERM + cmd-window flash from shell:true dropping cwd (#861)

The background install of the three optional fetch deps (turndown, turndown-plugin-gfm, @mixmark-io/domino) spawned npm with `shell: IS_WIN32`. On Windows + Node, `shell: true` DROPS the `cwd` option, so the spawned cmd.exe runs in an arbitrary working dir (C:\Windows under Claude Code). `npm install` then tries to create `C:\Windows\node_modules` -> EPERM on every MCP boot, and a cmd.exe window flashes each time; the install never persists, so the existsSync guard never short-circuits and it re-fires every session (reported by @lravizzoni with npm-debug-log evidence). Prefer running npm's own CLI through node directly (no `.cmd` shim, no shell): resolve `npm-cli.js` beside `process.execPath` and spawn it with `shell: false` (honors `cwd`) + `windowsHide: true` (no console window). Fall back to the `npm.cmd` shim only when npm-cli.js can't be located, so a working host — e.g. a POSIX layout where npm-cli.js isn't beside node — never regresses (POSIX already used shell:false, so its behavior is unchanged). Also surface spawn failures and non-zero exits to stderr; this EPERM was invisible for months behind stdio:"ignore" + an empty error handler. Tests: structural regression pins the node/shell:false/windowsHide/fallback contract (regex-free, matches the start.mjs test pattern); a portable behavioral test pins the runtime property the fix relies on (shell:false honors cwd). #634 background-install contract preserved. Needs Windows CI / on-device confirmation.
2026-09-19 03:27:16 +08:00 · 2026-06-22 16:53:14 +03:00
parent dfff7b873f
commit e918eac6c1
2 changed files with 151 additions and 12 deletions
@@ -487,21 +487,51 @@ import "./hooks/ensure-deps.mjs";
  const NPM_INSTALL_BG_PKGS = ["turndown", "turndown-plugin-gfm", "@mixmark-io/domino"];
  const IS_WIN32 = process.platform === "win32";
  const NPM_BIN = IS_WIN32 ? "npm.cmd" : "npm";
+  const NPM_FLAGS = ["--no-package-lock", "--no-save", "--silent", "--no-audit", "--no-fund"];
+  // #861: on Windows the npm shim is `npm.cmd`, which needs `shell: true` to
+  // run — but Node DROPS the `cwd` option when `shell: true`, so the spawned
+  // cmd.exe inherits an arbitrary working dir (C:\Windows under Claude Code).
+  // `npm install` then tries to create `C:\Windows\node_modules` → EPERM on
+  // every boot, and a cmd.exe window flashes each time. Prefer running npm's
+  // own CLI through node directly (no `.cmd` shim, no shell): `shell: false`
+  // honors `cwd` and `windowsHide` suppresses the console window. Fall back to
+  // the shim only when npm-cli.js can't be located, so a working host (e.g. a
+  // POSIX layout where npm-cli.js isn't beside node) can never regress.
+  const NPM_CLI_JS = resolve(dirname(process.execPath), "node_modules", "npm", "bin", "npm-cli.js");
+  const useNodeCli = existsSync(NPM_CLI_JS);
  for (const pkg of NPM_INSTALL_BG_PKGS) {
    if (existsSync(resolve(__dirname, "node_modules", pkg))) continue;
    try {
-      const child = spawn(
-        NPM_BIN,
-        ["install", pkg, "--no-package-lock", "--no-save", "--silent", "--no-audit", "--no-fund"],
-        {
-          cwd: __dirname,
-          stdio: "ignore",
-          detached: true,
-          // npm on Windows ships as a `.cmd` shim — must go through cmd.exe.
-          shell: IS_WIN32,
-        },
-      );
-      child.on("error", () => { /* best effort — npm missing, broken cache, etc. */ });
+      const child = useNodeCli
+        ? spawn(process.execPath, [NPM_CLI_JS, "install", pkg, ...NPM_FLAGS], {
+            cwd: __dirname,
+            stdio: "ignore",
+            detached: true,
+            shell: false,
+            windowsHide: true,
+          })
+        : spawn(NPM_BIN, ["install", pkg, ...NPM_FLAGS], {
+            cwd: __dirname,
+            stdio: "ignore",
+            detached: true,
+            // npm on Windows ships as a `.cmd` shim — must go through cmd.exe.
+            shell: IS_WIN32,
+            windowsHide: true,
+          });
+      // #861: this EPERM was invisible for months behind stdio:"ignore" + an
+      // empty error handler. Surface both spawn failures and non-zero exits.
+      child.on("error", (err) => {
+        process.stderr.write(
+          `[context-mode] background install of ${pkg} failed to spawn: ${err?.message ?? err}\n`,
+        );
+      });
+      child.on("exit", (code) => {
+        if (code) {
+          process.stderr.write(
+            `[context-mode] background install of ${pkg} exited with code ${code}\n`,
+          );
+        }
+      });
      child.unref();
    } catch { /* best effort — never block MCP boot */ }
  }
@@ -0,0 +1,109 @@
+/**
+ * start.mjs boot-time fetch-dep install MUST NOT EPERM on Windows — closes #861.
+ *
+ * The background install of the three optional fetch deps (turndown,
+ * turndown-plugin-gfm, @mixmark-io/domino) spawned npm with `shell: IS_WIN32`.
+ * On Windows + Node, `shell: true` DROPS the `cwd` option: the spawned cmd.exe
+ * runs in an arbitrary working dir (C:\Windows under Claude Code), so
+ * `npm install` tries to create `C:\Windows\node_modules` → EPERM on EVERY MCP
+ * boot, and a cmd.exe window flashes each time. Because the install never
+ * persists, the existsSync guard never short-circuits and it re-fires forever
+ * (reported by @lravizzoni with npm-debug-log evidence; @ken-jo to verify on
+ * Windows).
+ *
+ * Fix: prefer invoking npm's own CLI through node directly (no `.cmd` shim, no
+ * shell) — `shell: false` honors `cwd`, `windowsHide` suppresses the console
+ * window — and fall back to the shim only when npm-cli.js can't be located, so
+ * a working host can never regress.
+ *
+ * SEAM NOTE: start.mjs is a side-effecting raw entry script (can't import). The
+ * structural assertions below match the established start.mjs pattern
+ * (start-mjs-mcp-boot.test.ts, #634). The behavioral test pins the actual
+ * runtime property the fix depends on — `shell:false` honors `cwd` — which is
+ * portable (it holds on every OS; the BUG is that `shell:true` violates it on
+ * Windows only).
+ */
+
+import { describe, it, expect } from "vitest";
+import { readFileSync, mkdtempSync, realpathSync } from "node:fs";
+import { resolve, join } from "node:path";
+import { tmpdir } from "node:os";
+import { spawnSync } from "node:child_process";
+
+const REPO_ROOT = resolve(__dirname, "..", "..");
+const START_MJS = readFileSync(resolve(REPO_ROOT, "start.mjs"), "utf8");
+
+/** Remove all ASCII whitespace without regex, so assertions ignore formatting. */
+function stripWs(s: string): string {
+  let out = "";
+  for (const ch of s) {
+    if (ch !== " " && ch !== "\n" && ch !== "\t" && ch !== "\r") out += ch;
+  }
+  return out;
+}
+
+function sliceBetween(src: string, startAnchor: string, endAnchor: string): string {
+  const start = src.indexOf(startAnchor);
+  const end = src.indexOf(endAnchor, start + startAnchor.length);
+  expect(start, `start anchor not found: ${startAnchor}`).toBeGreaterThanOrEqual(0);
+  expect(end, `end anchor not found: ${endAnchor}`).toBeGreaterThan(start);
+  return src.slice(start, end);
+}
+
+describe("start.mjs Windows boot-install cwd handling (#861)", () => {
+  const BLOCK = sliceBetween(
+    START_MJS,
+    "const NPM_INSTALL_BG_PKGS",
+    "// Self-heal: create CLI shim",
+  );
+  const compact = stripWs(BLOCK);
+
+  it("prefers invoking npm's CLI through node with shell:false (cwd honored)", () => {
+    expect(compact).toContain("npm-cli.js");
+    expect(compact).toContain("process.execPath");
+    expect(compact).toContain("shell:false");
+  });
+
+  it("suppresses the Windows console window (windowsHide)", () => {
+    expect(compact).toContain("windowsHide:true");
+  });
+
+  it("keeps a no-regression fallback to the npm shim", () => {
+    // The shim path (spawn(NPM_BIN, …)) is retained for hosts where npm-cli.js
+    // cannot be located — so behavior on a working host is unchanged. This also
+    // preserves the #634 contract asserted by start-mjs-mcp-boot.test.ts.
+    expect(compact).toContain("spawn(NPM_BIN,");
+  });
+
+  it("still pins cwd to the plugin dir and keeps the detached background contract", () => {
+    expect(compact).toContain("cwd:__dirname");
+    expect(compact).toContain("detached:true");
+    expect(compact).toContain(".unref()");
+  });
+
+  it("surfaces install failures to stderr instead of swallowing them", () => {
+    // The EPERM was invisible for months behind stdio:"ignore" + an empty
+    // error handler. Failures must now be written to stderr.
+    expect(compact).toContain("process.stderr.write");
+    expect(compact).not.toContain('child.on("error",()=>{');
+  });
+
+  it("still installs all three fetch deps", () => {
+    for (const pkg of ["turndown", "turndown-plugin-gfm", "@mixmark-io/domino"]) {
+      expect(BLOCK).toContain(`"${pkg}"`);
+    }
+  });
+});
+
+describe("runtime property the #861 fix relies on", () => {
+  it("spawn with shell:false honors the cwd option (the fix's guarantee)", () => {
+    const d = realpathSync(mkdtempSync(join(tmpdir(), "cm861-")));
+    const r = spawnSync(
+      process.execPath,
+      ["-e", "process.stdout.write(process.cwd())"],
+      { cwd: d, shell: false, encoding: "utf8" },
+    );
+    expect(r.status).toBe(0);
+    expect(realpathSync(r.stdout.trim())).toBe(d);
+  });
+});