mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
perf(build): reuse a current Runner build in the server's prepare:runner-vendor step
The server build's prepare:runner-vendor step ran the Runner's full build every time, so inside `pnpm -r build` the Runner was built twice. The second build took 12-16s on the CI Build job's critical path (six master runs, 2026-10-11) and the same again in every other `pnpm build`. The Runner's build now ends by recording a digest of its inputs, and the server's step reuses the build when the digest matches the stamp and every vendored output exists. Any other state runs the same `pnpm --filter @paperclipai/paperclip-runner build` as before, so a standalone server build (the Dockerfile's path) still builds the Runner itself. PAPERCLIP_RUNNER_VENDOR_REBUILD=1 forces the rebuild. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
bfe7c6357d
commit
0575b85473
9 files changed
+934
-6
No files matched your search
@@ -0,0 +1,139 @@
|
||||
// Makes sure packages/paperclip-runner/dist is a current build before the
|
||||
// server build vendors it, rebuilding the Runner only when that is needed.
|
||||
//
|
||||
// The server build (and typecheck) used to run the Runner's full `build`
|
||||
// unconditionally here. That is what lets a standalone
|
||||
// `pnpm --filter @paperclipai/server build` produce a current vendored Runner
|
||||
// on its own, which the Dockerfile relies on. But inside `pnpm -r build` the
|
||||
// Runner has just been built by the same command, and repeating it cost
|
||||
// 12-16s on the CI Build job's critical path (six master runs, 2026-10-11)
|
||||
// and the same again in every other `pnpm build`.
|
||||
//
|
||||
// The Runner's `build` now records a digest of its inputs after it succeeds
|
||||
// (packages/paperclip-runner/scripts/build-inputs.mjs). This step recomputes
|
||||
// the digest and reuses the build when the stamp matches and every output a
|
||||
// consumer reads exists; anything else runs the full Runner build exactly as
|
||||
// before. The standalone guarantee therefore holds: a fresh checkout, a
|
||||
// changed input, or a deleted output always rebuilds. Set
|
||||
// PAPERCLIP_RUNNER_VENDOR_REBUILD=1 to rebuild regardless of the stamp.
|
||||
//
|
||||
// Usage: node scripts/prepare-runner-vendor.mjs [--repo-root <dir>]
|
||||
// The repo root defaults to this checkout; the flag exists for tests that
|
||||
// build a throwaway workspace layout.
|
||||
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { performance } from "node:perf_hooks";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import {
|
||||
assessRunnerBuild,
|
||||
removeBuildInputsStamp,
|
||||
} from "../../packages/paperclip-runner/scripts/build-inputs.mjs";
|
||||
|
||||
/** The command the server build has always run here; the rebuild path is unchanged. */
|
||||
export const RUNNER_VENDOR_BUILD_COMMAND = "pnpm --filter @paperclipai/paperclip-runner build";
|
||||
export const FORCE_REBUILD_VARIABLE = "PAPERCLIP_RUNNER_VENDOR_REBUILD";
|
||||
export const TIMING_SCHEMA = "paperclip.runner-vendor.timing.v1";
|
||||
|
||||
export function decideRunnerVendorAction({ env, assessment }) {
|
||||
if (env[FORCE_REBUILD_VARIABLE] === "1") {
|
||||
return { action: "rebuild", code: "forced", detail: `${FORCE_REBUILD_VARIABLE}=1` };
|
||||
}
|
||||
if (assessment.current) {
|
||||
return { action: "reuse", code: assessment.code, detail: assessment.detail };
|
||||
}
|
||||
return { action: "rebuild", code: assessment.code, detail: assessment.detail };
|
||||
}
|
||||
|
||||
/**
|
||||
* The pnpm that is running this script, the same rule as the CLI's
|
||||
* resolvePnpmInstallInvocation: pnpm sets npm_execpath to its own entry, which
|
||||
* is a JavaScript file for a normal install (run through this Node) or a native
|
||||
* executable for a standalone pnpm (run directly). Outside a pnpm script,
|
||||
* `pnpm` on PATH is the only option. The pnpm entry is not always on PATH in a
|
||||
* script (corepack, for one), so this cannot just spawn `pnpm`.
|
||||
*/
|
||||
export function resolvePnpmInvocation(env = process.env, nodeExecPath = process.execPath) {
|
||||
const npmExecPath = env.npm_execpath?.trim();
|
||||
if (npmExecPath && npmExecPath.toLowerCase().includes("pnpm")) {
|
||||
if (/\.(cjs|mjs|js)$/i.test(npmExecPath)) {
|
||||
return { command: nodeExecPath, argsPrefix: [npmExecPath] };
|
||||
}
|
||||
return { command: npmExecPath, argsPrefix: [] };
|
||||
}
|
||||
return { command: "pnpm", argsPrefix: [] };
|
||||
}
|
||||
|
||||
export function runBuildCommand(command, { cwd, env = process.env }) {
|
||||
const [tool, ...args] = command.split(" ");
|
||||
if (tool !== "pnpm") throw new Error(`unexpected build command ${command}`);
|
||||
const pnpm = resolvePnpmInvocation(env);
|
||||
const result = spawnSync(pnpm.command, [...pnpm.argsPrefix, ...args], {
|
||||
cwd,
|
||||
env,
|
||||
stdio: "inherit",
|
||||
// A bare `pnpm` on Windows is pnpm.cmd, which needs the shell to resolve.
|
||||
shell: pnpm.command === "pnpm" && process.platform === "win32",
|
||||
});
|
||||
if (result.error) throw result.error;
|
||||
return result.status ?? 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the exit status for the step. `run` is injectable so tests can
|
||||
* observe the command and the stamp state at the moment it runs.
|
||||
*/
|
||||
export function prepareRunnerVendor({
|
||||
repoRoot,
|
||||
env = process.env,
|
||||
run = runBuildCommand,
|
||||
log = console.log,
|
||||
now = () => performance.now(),
|
||||
}) {
|
||||
const packageRoot = path.join(repoRoot, "packages", "paperclip-runner");
|
||||
const checkStart = now();
|
||||
const assessment = assessRunnerBuild({ repoRoot, packageRoot });
|
||||
const decision = decideRunnerVendorAction({ env, assessment });
|
||||
const checkMs = Math.round(now() - checkStart);
|
||||
|
||||
if (decision.action === "reuse") {
|
||||
log(`prepare:runner-vendor: reusing the current Runner build (${decision.detail})`);
|
||||
log(`${TIMING_SCHEMA},action:reuse,reason:${decision.code},check_ms:${checkMs},build_ms:0`);
|
||||
return 0;
|
||||
}
|
||||
|
||||
log(`prepare:runner-vendor: rebuilding the Runner (${decision.detail})`);
|
||||
// A build that stops partway must not leave a stamp that vouches for it; the
|
||||
// Runner's own build writes a fresh one as its final step.
|
||||
removeBuildInputsStamp(assessment.stampPath);
|
||||
const buildStart = now();
|
||||
const status = run(RUNNER_VENDOR_BUILD_COMMAND, { cwd: path.join(repoRoot, "server"), env });
|
||||
const buildMs = Math.round(now() - buildStart);
|
||||
log(
|
||||
`${TIMING_SCHEMA},action:rebuild,reason:${decision.code},check_ms:${checkMs},build_ms:${buildMs},status:${status}`,
|
||||
);
|
||||
return status;
|
||||
}
|
||||
|
||||
export function parseArguments(argv) {
|
||||
let repoRoot = null;
|
||||
for (let index = 0; index < argv.length; index += 1) {
|
||||
const argument = argv[index];
|
||||
if (argument === "--repo-root") {
|
||||
repoRoot = argv[index + 1];
|
||||
if (!repoRoot) throw new Error("--repo-root needs a directory");
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
throw new Error(`unknown argument ${argument}`);
|
||||
}
|
||||
return { repoRoot };
|
||||
}
|
||||
|
||||
if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {
|
||||
const { repoRoot: requested } = parseArguments(process.argv.slice(2));
|
||||
const repoRoot = requested
|
||||
? path.resolve(requested)
|
||||
: path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
|
||||
process.exitCode = prepareRunnerVendor({ repoRoot });
|
||||
}
|
||||
@@ -0,0 +1,267 @@
|
||||
import { execFile } from "node:child_process";
|
||||
import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
assessRunnerBuild,
|
||||
buildInputsStampPath,
|
||||
readBuildInputsStamp,
|
||||
recordRunnerBuild,
|
||||
} from "../../packages/paperclip-runner/scripts/build-inputs.mjs";
|
||||
import {
|
||||
FORCE_REBUILD_VARIABLE,
|
||||
RUNNER_VENDOR_BUILD_COMMAND,
|
||||
TIMING_SCHEMA,
|
||||
decideRunnerVendorAction,
|
||||
prepareRunnerVendor,
|
||||
resolvePnpmInvocation,
|
||||
} from "./prepare-runner-vendor.mjs";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
const prepareScript = fileURLToPath(new URL("./prepare-runner-vendor.mjs", import.meta.url));
|
||||
const recordScript = fileURLToPath(
|
||||
new URL("../../packages/paperclip-runner/scripts/record-build-inputs.mjs", import.meta.url),
|
||||
);
|
||||
const executable = process.platform === "win32" ? "paperclip-runnerd.exe" : "paperclip-runnerd";
|
||||
|
||||
const roots = [];
|
||||
|
||||
function writeTree(root, files) {
|
||||
for (const [relative, content] of Object.entries(files)) {
|
||||
const absolute = path.join(root, ...relative.split("/"));
|
||||
mkdirSync(path.dirname(absolute), { recursive: true });
|
||||
writeFileSync(absolute, content);
|
||||
}
|
||||
}
|
||||
|
||||
/** A throwaway workspace whose Runner is already built, with no stamp yet. */
|
||||
function makeRepo() {
|
||||
const repoRoot = mkdtempSync(path.join(tmpdir(), "prepare-runner-vendor-"));
|
||||
roots.push(repoRoot);
|
||||
writeTree(repoRoot, {
|
||||
"package.json": "{}\n",
|
||||
"pnpm-lock.yaml": "lockfileVersion: 9\n",
|
||||
"pnpm-workspace.yaml": "packages:\n - packages/*\n - server\n",
|
||||
"tsconfig.base.json": "{}\n",
|
||||
"server/package.json": '{"name":"@paperclipai/server"}\n',
|
||||
"packages/paperclip-runner/package.json": '{"name":"@paperclipai/paperclip-runner"}\n',
|
||||
"packages/paperclip-runner/src/index.ts": "export const one = 1;\n",
|
||||
"packages/paperclip-runner/dist/index.js": "export const one = 1;\n",
|
||||
"packages/paperclip-runner/dist/live/index.js": "export {};\n",
|
||||
"packages/paperclip-runner/dist/testing.js": "export {};\n",
|
||||
[`packages/paperclip-runner/dist/bin/${executable}`]: "binary\n",
|
||||
"packages/paperclip-eval-kernel/src/index.ts": "export const kernel = 1;\n",
|
||||
});
|
||||
const packageRoot = path.join(repoRoot, "packages", "paperclip-runner");
|
||||
return { repoRoot, packageRoot };
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe("decideRunnerVendorAction", () => {
|
||||
const current = { current: true, code: "inputs_unchanged", detail: "unchanged" };
|
||||
const stale = { current: false, code: "inputs_changed", detail: "changed" };
|
||||
|
||||
it("reuses a current build and rebuilds a stale one, carrying the assessment's reason", () => {
|
||||
expect(decideRunnerVendorAction({ env: {}, assessment: current })).toEqual({
|
||||
action: "reuse",
|
||||
code: "inputs_unchanged",
|
||||
detail: "unchanged",
|
||||
});
|
||||
expect(decideRunnerVendorAction({ env: {}, assessment: stale })).toEqual({
|
||||
action: "rebuild",
|
||||
code: "inputs_changed",
|
||||
detail: "changed",
|
||||
});
|
||||
});
|
||||
|
||||
it("rebuilds a current build when the operator forces it", () => {
|
||||
expect(decideRunnerVendorAction({ env: { [FORCE_REBUILD_VARIABLE]: "1" }, assessment: current })).toEqual({
|
||||
action: "rebuild",
|
||||
code: "forced",
|
||||
detail: `${FORCE_REBUILD_VARIABLE}=1`,
|
||||
});
|
||||
// Only the documented value forces; an unrelated value does not.
|
||||
expect(decideRunnerVendorAction({ env: { [FORCE_REBUILD_VARIABLE]: "0" }, assessment: current }).action).toBe(
|
||||
"reuse",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolvePnpmInvocation", () => {
|
||||
it("runs the pnpm that is running the script, through Node for a JavaScript entry", () => {
|
||||
expect(resolvePnpmInvocation({ npm_execpath: "/corepack/pnpm/9.15.4/bin/pnpm.cjs" }, "/usr/bin/node")).toEqual({
|
||||
command: "/usr/bin/node",
|
||||
argsPrefix: ["/corepack/pnpm/9.15.4/bin/pnpm.cjs"],
|
||||
});
|
||||
expect(resolvePnpmInvocation({ npm_execpath: "/opt/pnpm/pnpm" }, "/usr/bin/node")).toEqual({
|
||||
command: "/opt/pnpm/pnpm",
|
||||
argsPrefix: [],
|
||||
});
|
||||
expect(resolvePnpmInvocation({ npm_execpath: "C:\\pnpm\\pnpm.exe" }, "node.exe")).toEqual({
|
||||
command: "C:\\pnpm\\pnpm.exe",
|
||||
argsPrefix: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to pnpm on PATH outside a pnpm script or under another package manager", () => {
|
||||
expect(resolvePnpmInvocation({}, "/usr/bin/node")).toEqual({ command: "pnpm", argsPrefix: [] });
|
||||
expect(resolvePnpmInvocation({ npm_execpath: "" }, "/usr/bin/node")).toEqual({ command: "pnpm", argsPrefix: [] });
|
||||
expect(resolvePnpmInvocation({ npm_execpath: "/usr/lib/node_modules/npm/bin/npm-cli.js" }, "/usr/bin/node")).toEqual({
|
||||
command: "pnpm",
|
||||
argsPrefix: [],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("prepareRunnerVendor", () => {
|
||||
it("still runs the Runner build the server build has always run", () => {
|
||||
// The Dockerfile's standalone `pnpm --filter @paperclipai/server build`
|
||||
// depends on this exact command producing a current vendored Runner.
|
||||
expect(RUNNER_VENDOR_BUILD_COMMAND).toBe("pnpm --filter @paperclipai/paperclip-runner build");
|
||||
});
|
||||
|
||||
it("skips the rebuild when the recorded build is current and reports the check time", () => {
|
||||
const { repoRoot, packageRoot } = makeRepo();
|
||||
recordRunnerBuild({ repoRoot, packageRoot });
|
||||
const runs = [];
|
||||
const lines = [];
|
||||
|
||||
const status = prepareRunnerVendor({
|
||||
repoRoot,
|
||||
env: {},
|
||||
run: (...args) => {
|
||||
runs.push(args);
|
||||
return 0;
|
||||
},
|
||||
log: (line) => lines.push(line),
|
||||
});
|
||||
|
||||
expect(status).toBe(0);
|
||||
expect(runs).toEqual([]);
|
||||
expect(lines[0]).toMatch(/^prepare:runner-vendor: reusing the current Runner build \(inputs unchanged/);
|
||||
expect(lines[1]).toMatch(
|
||||
new RegExp(`^${TIMING_SCHEMA},action:reuse,reason:inputs_unchanged,check_ms:\\d+,build_ms:0$`),
|
||||
);
|
||||
expect(readBuildInputsStamp(buildInputsStampPath(packageRoot))).not.toBeNull();
|
||||
});
|
||||
|
||||
it("rebuilds when the inputs changed, removing the stale stamp before the build starts", () => {
|
||||
const { repoRoot, packageRoot } = makeRepo();
|
||||
recordRunnerBuild({ repoRoot, packageRoot });
|
||||
writeFileSync(path.join(packageRoot, "src", "index.ts"), "export const one = 2;\n");
|
||||
const stampPath = buildInputsStampPath(packageRoot);
|
||||
const observed = [];
|
||||
const lines = [];
|
||||
const env = { npm_execpath: "/corepack/pnpm.cjs" };
|
||||
|
||||
const status = prepareRunnerVendor({
|
||||
repoRoot,
|
||||
env,
|
||||
run: (command, options) => {
|
||||
observed.push({ command, ...options, stampPresent: readBuildInputsStamp(stampPath) !== null });
|
||||
return 0;
|
||||
},
|
||||
log: (line) => lines.push(line),
|
||||
});
|
||||
|
||||
expect(status).toBe(0);
|
||||
expect(observed).toEqual([
|
||||
{ command: RUNNER_VENDOR_BUILD_COMMAND, cwd: path.join(repoRoot, "server"), env, stampPresent: false },
|
||||
]);
|
||||
expect(lines[0]).toMatch(/^prepare:runner-vendor: rebuilding the Runner \(inputs changed/);
|
||||
expect(lines[1]).toMatch(
|
||||
new RegExp(`^${TIMING_SCHEMA},action:rebuild,reason:inputs_changed,check_ms:\\d+,build_ms:\\d+,status:0$`),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns the build's failure status and leaves no stamp behind", () => {
|
||||
const { repoRoot, packageRoot } = makeRepo();
|
||||
const lines = [];
|
||||
|
||||
const status = prepareRunnerVendor({
|
||||
repoRoot,
|
||||
env: {},
|
||||
run: () => 3,
|
||||
log: (line) => lines.push(line),
|
||||
});
|
||||
|
||||
expect(status).toBe(3);
|
||||
expect(lines[0]).toMatch(/^prepare:runner-vendor: rebuilding the Runner \(no build stamp/);
|
||||
expect(lines[1]).toMatch(/,action:rebuild,reason:no_stamp,.*,status:3$/);
|
||||
expect(readBuildInputsStamp(buildInputsStampPath(packageRoot))).toBeNull();
|
||||
});
|
||||
|
||||
it("rebuilds a current build when PAPERCLIP_RUNNER_VENDOR_REBUILD=1", () => {
|
||||
const { repoRoot, packageRoot } = makeRepo();
|
||||
recordRunnerBuild({ repoRoot, packageRoot });
|
||||
let runs = 0;
|
||||
const lines = [];
|
||||
|
||||
const status = prepareRunnerVendor({
|
||||
repoRoot,
|
||||
env: { [FORCE_REBUILD_VARIABLE]: "1" },
|
||||
run: () => {
|
||||
runs += 1;
|
||||
return 0;
|
||||
},
|
||||
log: (line) => lines.push(line),
|
||||
});
|
||||
|
||||
expect(status).toBe(0);
|
||||
expect(runs).toBe(1);
|
||||
expect(lines[1]).toMatch(/,action:rebuild,reason:forced,/);
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"end to end: the first server build rebuilds the Runner, the second reuses it, a change rebuilds again",
|
||||
async () => {
|
||||
const { repoRoot, packageRoot } = makeRepo();
|
||||
// A stand-in pnpm that behaves like the Runner build: it records how it
|
||||
// was called, then records the build inputs the way the real build does
|
||||
// as its final step.
|
||||
const binDir = path.join(repoRoot, "bin");
|
||||
const argvLog = path.join(repoRoot, "pnpm-argv.log");
|
||||
mkdirSync(binDir);
|
||||
const pnpm = path.join(binDir, "pnpm");
|
||||
writeFileSync(
|
||||
pnpm,
|
||||
`${[
|
||||
"#!/bin/sh",
|
||||
"set -e",
|
||||
`printf '%s\\n' "$*" >> "${argvLog}"`,
|
||||
`"${process.execPath}" "${recordScript}" --package-root "${packageRoot}" > /dev/null`,
|
||||
].join("\n")}\n`,
|
||||
);
|
||||
chmodSync(pnpm, 0o755);
|
||||
const env = { ...process.env, PATH: `${binDir}${path.delimiter}${process.env.PATH ?? ""}` };
|
||||
delete env[FORCE_REBUILD_VARIABLE];
|
||||
// The suite itself may run under pnpm; the script must then take the
|
||||
// PATH fallback so the stand-in, not the real pnpm, receives the build.
|
||||
delete env.npm_execpath;
|
||||
const run = () => execFileAsync(process.execPath, [prepareScript, "--repo-root", repoRoot], { env });
|
||||
const calls = () => readFileSync(argvLog, "utf8").split("\n").filter(Boolean);
|
||||
|
||||
const first = await run();
|
||||
expect(calls()).toEqual(["--filter @paperclipai/paperclip-runner build"]);
|
||||
expect(first.stdout).toMatch(/action:rebuild,reason:no_stamp,/);
|
||||
expect(assessRunnerBuild({ repoRoot, packageRoot }).current).toBe(true);
|
||||
|
||||
const second = await run();
|
||||
expect(calls()).toHaveLength(1);
|
||||
expect(second.stdout).toMatch(/action:reuse,reason:inputs_unchanged,/);
|
||||
|
||||
writeFileSync(path.join(packageRoot, "src", "index.ts"), "export const one = 2;\n");
|
||||
const third = await run();
|
||||
expect(calls()).toHaveLength(2);
|
||||
expect(third.stdout).toMatch(/action:rebuild,reason:inputs_changed,/);
|
||||
},
|
||||
);
|
||||
});
|
||||
Reference in new issue
Block a user