From dab01b7f2b052f5f857dc5e194c4b8d2af6a579a Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 15:43:20 +0700 Subject: [PATCH] fix(plugin): install the client build's deps with npm ci MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit npm install reuses whatever the cache and an earlier run left behind and still reports success, so a truncated cache entry produced a node_modules that was missing typescript's lib/*.d.ts — the build then died with TS2318, every global type undefined. npm ci starts from an empty tree and verifies each tarball against the lockfile's integrity hash. Falls back to install when the copied client has no usable lockfile. --- scripts/plugin-open.js | 28 ++++++++- server/__tests__/plugin-bootstrap.test.js | 69 +++++++++++++++++++++++ 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/scripts/plugin-open.js b/scripts/plugin-open.js index 383813ff..0a1c0c1f 100644 --- a/scripts/plugin-open.js +++ b/scripts/plugin-open.js @@ -41,9 +41,17 @@ function run(cmd, args, cwd, stdio) { * caller's stdio — used when this runs inside the detached bootstrap worker, * whose own stdout is discarded, so a build failure is still debuggable. * + * `exec` is the command runner, swappable for tests. + * * @returns {"ready"|"built"|"failed"} */ -function buildClient({ rt = boot.runtimeDir(), root = PLUGIN_ROOT, force = false, logPath } = {}) { +function buildClient({ + rt = boot.runtimeDir(), + root = PLUGIN_ROOT, + force = false, + logPath, + exec = run, +} = {}) { const dist = path.join(rt, "client-dist"); if (!force && fs.existsSync(path.join(dist, "index.html"))) return "ready"; @@ -57,8 +65,22 @@ function buildClient({ rt = boot.runtimeDir(), root = PLUGIN_ROOT, force = false filter: (p) => !/[\\/](node_modules|dist)$/.test(p), }); - if (!run(NPM, ["install", "--no-audit", "--no-fund"], src, stdio)) return "failed"; - if (!run(NPM, ["run", "build"], src, stdio)) return "failed"; + // `npm install` reuses whatever the cache and a previous run left behind and + // still reports success, so a truncated cache entry or an interrupted install + // yields a tree that is missing files nobody notices until `tsc` dies with + // TS2318 (every global type gone, because typescript's own lib/*.d.ts never + // landed). `npm ci` starts from an empty node_modules and checks every + // tarball against the lockfile's integrity hash, so a bad cache entry is + // refetched instead of silently reused. It needs a lockfile that matches + // package.json — fall back to `install` when the copy has none or has drifted. + const npmFlags = ["--no-audit", "--no-fund"]; + if ( + !exec(NPM, ["ci", ...npmFlags], src, stdio) && + !exec(NPM, ["install", ...npmFlags], src, stdio) + ) { + return "failed"; + } + if (!exec(NPM, ["run", "build"], src, stdio)) return "failed"; fs.rmSync(dist, { recursive: true, force: true }); fs.cpSync(path.join(src, "dist"), dist, { recursive: true }); diff --git a/server/__tests__/plugin-bootstrap.test.js b/server/__tests__/plugin-bootstrap.test.js index 004f2bce..f8c69d1d 100644 --- a/server/__tests__/plugin-bootstrap.test.js +++ b/server/__tests__/plugin-bootstrap.test.js @@ -339,3 +339,72 @@ describe("plugin-open / plugin-bootstrap require cycle", () => { assert.equal(typeof require("../../scripts/plugin-open").buildClient, "function"); }); }); + +describe("buildClient dependency install", () => { + const { buildClient } = require("../../scripts/plugin-open"); + + // A fake plugin root holding just enough of a client/ to copy. + function fakeRoot() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "agenthub-root-")); + fs.mkdirSync(path.join(root, "client")); + fs.writeFileSync(path.join(root, "client", "package.json"), "{}"); + return root; + } + + // Records every npm invocation; `fails` names the subcommands that exit non-zero. + // A successful "run build" writes the dist/ the real Vite build would produce. + function fakeExec(calls, fails = []) { + return (_cmd, args, cwd) => { + calls.push(args.join(" ")); + if (fails.includes(args[0])) return false; + if (args[0] === "run") fs.mkdirSync(path.join(cwd, "dist"), { recursive: true }); + return true; + }; + } + + it("installs with `npm ci`, which cannot reuse a half-populated node_modules", () => { + // Regression: `npm install` reported success against a truncated cache + // entry, and the build then died with TS2318 — typescript's lib/*.d.ts + // missing, so every global type (Array, Boolean, …) was undefined. + resetRuntime(); + const calls = []; + const result = buildClient({ rt: RT, root: fakeRoot(), exec: fakeExec(calls) }); + + assert.equal(result, "built"); + assert.equal(calls[0].startsWith("ci"), true, `expected npm ci first, got ${calls[0]}`); + assert.equal( + calls.some((c) => c.startsWith("install")), + false, + "npm install must not run once ci succeeded" + ); + }); + + it("falls back to `npm install` when ci refuses the lockfile", () => { + resetRuntime(); + const calls = []; + const result = buildClient({ rt: RT, root: fakeRoot(), exec: fakeExec(calls, ["ci"]) }); + + assert.equal(result, "built"); + assert.deepEqual( + calls.map((c) => c.split(" ")[0]), + ["ci", "install", "run"] + ); + }); + + it("fails without building when neither install path works", () => { + resetRuntime(); + const calls = []; + const result = buildClient({ + rt: RT, + root: fakeRoot(), + exec: fakeExec(calls, ["ci", "install"]), + }); + + assert.equal(result, "failed"); + assert.equal( + calls.some((c) => c.startsWith("run")), + false, + "the build must not run on a broken dependency tree" + ); + }); +});