fix(plugin): install the client build's deps with npm ci
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.
This commit is contained in:
+25
-3
@@ -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 });
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user