fix(server): bundle the vendored paperclip-runner instead of hand-mirroring its deps (#13121)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The server package is published to npm, but its native-runtime
driver code lives in `packages/paperclip-runner`, a private workspace
package that is never published
> - So the server build vendors the runner's compiled code by copying it
in directly, instead of taking it as a normal npm dependency
> - But `cp -R` only copies code, not `node_modules`, so every npm
package the runner imports has to be re-declared by hand in
`server/package.json` to stay resolvable once vendored
> - That hand mirroring step is silent and easy to forget: it missed
`smol-toml` in #13110, and CI stayed green while production crash-looped
3 seconds into every start (#13116)
> - This pull request keeps the proven `cp -R` vendor step exactly as it
was, and adds a build check that derives the required dependency set
from an esbuild scan of the vendored entry points, failing loudly and
precisely if any package the runner actually needs isn't declared in
`server/package.json`
> - The benefit is the dependency list is now verified against the real
module graph instead of hand-copied, so this exact class of bug cannot
pass a green build again -- without changing how the runner's code is
laid out on disk, which several of its modules depend on for unrelated
filesystem lookups

## Linked Issues or Issue Description

Refs: #13110 (introduced the `smol-toml` import that the vendor step
could not resolve), #13116 (the follow-up fix for a different oversight
in the same PR), #11813 (the same "vendored package installed outside
the monorepo dependency graph loses a runtime dependency" failure shape,
in the Kubernetes plugin installer instead of the server build)

No issue exists yet for this specific incident, so per CONTRIBUTING.md
option (B):

**What happened?**
`packages/paperclip-runner/package.json` added `smol-toml` as a runtime
dependency in #13110. `server/package.json`'s existing convention (see
`acpx`, `ajv`) requires mirroring every runtime dependency the vendored
runner imports into `server/package.json` too, because the server build
copies the runner's compiled `dist/` tree with `cp -R` -- code only, no
`node_modules`. That mirroring step was missed. CI never runs the
compiled server (`node dist/index.js`); it only builds it, type-checks
it, and boots the app in dev mode via `tsx` against source, which never
touches the vendored path. So the PR merged green, and the deployed
server crash-looped in production:
```
Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'smol-toml' imported from
/srv/paperclip/app/server/dist/vendor/paperclip-runner/drivers/codex/codex-startup-trust.js
```

**Expected behavior**
Any npm package the vendored runner code needs at runtime should either
be guaranteed present by construction, or the build should fail with a
clear, actionable error before the change ever reaches a PR -- not
silently pass CI and fail only once deployed.

**Steps to reproduce (the original incident)**
1. Add a new runtime dependency to
`packages/paperclip-runner/package.json` (e.g. a TOML parser) and use it
from a module reachable from the runner's `index.ts` export graph.
2. Do not add the same dependency to `server/package.json`.
3. Run `pnpm build` in `server/` -- it succeeds.
4. Run `node dist/index.js` -- it crashes with `ERR_MODULE_NOT_FOUND`
for the new package.

## What Changed

- **Revision note:** the first version of this PR replaced the `cp -R`
vendor step with an esbuild bundle of the runner's entry points.
Greptile's review correctly caught that this broke packaged
ACPX/OpenCode provider startup: several runner modules resolve sibling
build artifacts via `import.meta.url`-relative filesystem paths (not JS
imports) at whatever depth their source file sits at, and bundling
collapses/rearranges that layout. The current version keeps the file
layout untouched and only adds verification. See the second commit's
message for the full explanation.
- `server/scripts/verify-runner-vendor-dependencies.mjs`: a new build
step that runs esbuild with `write: false` (a pure module-graph scan --
nothing is written to disk) against the runner's two entry points server
actually imports (`index.js`, `testing.js`), with `packages: "external"`
so its metafile reports exactly which npm packages the code needs at
runtime. It fails with a precise, actionable error if any of them isn't
declared in `server/package.json`'s `dependencies`. This is deliberately
more precise than "mirror every dependency the runner declares": running
it against this repo's real manifests shows
`packages/paperclip-runner/package.json` declares dependencies
(`react-markdown`, the codex/opencode CLI packages, ...) that only its
unrelated `./react` and `./browser` export subpaths use -- server never
imports those, so a blanket mirror rule would demand dependencies server
doesn't actually need.
- `server/package.json`: added the new check into the `build` script
(right after the runner is built, before the expensive `tsc`/copy steps,
so it fails fast), and added `smol-toml` (`^1.4.2`, matching
`packages/paperclip-runner/package.json`) to `dependencies` -- the
actual missing piece from #13110. The vendor step (`cp -R
../packages/paperclip-runner/dist/. dist/vendor/paperclip-runner/`) is
unchanged from before this PR.
- Widened `server/vitest.config.ts`'s `include` to also run
`scripts/**/*.test.mjs`, and added
`server/scripts/verify-runner-vendor-dependencies.test.mjs` unit-testing
the pure dependency-diff function (`findMissingVendorDependencies`)
against the exact shape of the `smol-toml` incident, plus a case proving
an unreachable dependency (like `react-markdown`) is correctly never
flagged.
- Updated `server/src/__tests__/server-package-build-script.test.ts`'s
existing build-script assertions to match.

## Verification

- `node --check` on the new script -- syntax OK. `node -e` JSON-parsed
the edited `package.json` files after every edit.
- Unit-verified `findMissingVendorDependencies` directly against:
nothing missing, one missing (the `smol-toml` shape), and multiple
missing with stable sort order.
- Ran the actual check against this repo's real
`packages/paperclip-runner/package.json` and `server/package.json` (via
a standalone `node` invocation, since `pnpm build` needs a Rust
toolchain this sandbox doesn't have -- see below) to see its real
output. It correctly reported `smol-toml`, `acpx`, and `ajv` as already
satisfied, and did **not** flag `react-markdown`, `remark-gfm`,
`json-schema-to-ts`, `opencode-ai`, `@openai/codex`, or the
`@agentclientprotocol/*` packages -- confirming the "reachable from
index.js/testing.js" scoping works as intended and doesn't demand
dependencies server doesn't need.
- Built a fixture tree at a real filesystem location (not just
in-process) mimicking `packages/paperclip-runner`: a manifest declaring
both a reachable dependency (`smol-toml`, actually imported by the
fixture's `dist/index.js`/`testing.js`) and an unreachable one
(`react-markdown`, declared but never imported). Copied the real script
next to a fixture `server/package.json` and ran it as its own process
(`node server/scripts/verify-runner-vendor-dependencies.mjs`), twice:
- `smol-toml` missing from the fixture's server dependencies → the
script throws with the exact intended message and exits 1.
- `smol-toml` present, `react-markdown` absent → the script exits 0,
proving the unreachable dependency is correctly never flagged.
- Not verified locally: the real `packages/paperclip-runner` build, and
therefore the check running end-to-end against its true
`dist/index.js`/`dist/testing.js`. This sandbox has no Rust toolchain
(the runner's own build compiles a Cargo binary) and an incomplete
workspace install. CI's `Build` job (`.github/workflows/pr-trusted.yml`)
runs the real thing; I'll watch it on this PR.

## Risks

- The check's precision (scoping to what's reachable from
`index.js`/`testing.js`, rather than every declared runner dependency)
means a dependency that becomes reachable through some *other* export
subpath server starts importing later would need this check's
entry-point list updated too. That list is a 2-line array in the script
with a comment explaining why, and matches the only two paths server/src
actually imports today (verified by a repo-wide search).
- This only changes a build-time check; the actual vendored file layout
(`cp -R` of the runner's whole compiled tree) is byte-for-byte the same
as before this PR, so there's no behavioral change to the running server
beyond `smol-toml` now being present as intended.
- I could not exercise the real Rust-backed build locally (no Cargo in
this sandbox); see Verification. I am relying on CI's `Build` job to
confirm this end to end and will fix forward if it surfaces something
the fixture-based testing didn't.

## Model Used

Claude Sonnet 5 (`claude-sonnet-5`), via Claude Code. Standard
(non-extended) reasoning mode, with tool use (Bash, Read, Edit/Write,
`gh`) for repository exploration, local esbuild-based verification
against hand-built fixtures, and PR authoring. No extended thinking
mode.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [ ] I have run tests locally and they pass — see Verification: full
local verification was not possible (no Rust toolchain, incomplete
workspace install in this sandbox); watching CI's `Build` job on this PR
to confirm.
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes — no
user-facing docs describe this internal build step; none needed
updating.
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green — pending, will monitor.
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups —
addressed the first review round; watching for re-review.
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Nicky Leach 2026-09-09 16:38:17 -07:00 committed by GitHub
parent 2d45f42e47
commit 04a9f89ede
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 262 additions and 2 deletions

View File

@ -35,7 +35,7 @@
"dev": "tsx src/index.ts",
"dev:watch": "cross-env PAPERCLIP_MIGRATION_PROMPT=never PAPERCLIP_MIGRATION_AUTO_APPLY=true tsx ./scripts/dev-watch.ts",
"prepare:ui-dist": "bash ../scripts/prepare-server-ui-dist.sh",
"build": "pnpm run prepare:runner-vendor && tsc && mkdir -p dist/onboarding-assets dist/built-ins dist/services/scripts dist/vendor/paperclip-runner && cp -R src/onboarding-assets/. dist/onboarding-assets/ && cp -R src/built-ins/. dist/built-ins/ && cp -R src/services/scripts/. dist/services/scripts/ && cp -R ../packages/paperclip-runner/dist/. dist/vendor/paperclip-runner/ && node scripts/write-build-stamp.mjs",
"build": "pnpm run prepare:runner-vendor && node scripts/verify-runner-vendor-dependencies.mjs && tsc && mkdir -p dist/onboarding-assets dist/built-ins dist/services/scripts dist/vendor/paperclip-runner && cp -R src/onboarding-assets/. dist/onboarding-assets/ && cp -R src/built-ins/. dist/built-ins/ && cp -R src/services/scripts/. dist/services/scripts/ && cp -R ../packages/paperclip-runner/dist/. dist/vendor/paperclip-runner/ && node scripts/write-build-stamp.mjs",
"prepack": "pnpm run prepare:ui-dist && pnpm run build",
"postpack": "rm -rf ui-dist",
"clean": "rm -rf dist",
@ -88,6 +88,7 @@
"pino-http": "^11.0.0",
"pino-pretty": "^13.1.3",
"sharp": "^0.35.4",
"smol-toml": "^1.4.2",
"ssh2": "^1.17.0",
"ws": "^8.21.3",
"zod": "^4.4.3"

View File

@ -0,0 +1,145 @@
// Verify every npm package the vendored paperclip-runner actually imports
// at runtime is also declared as a direct dependency of server/package.json.
//
// packages/paperclip-runner is private and never published, so the server
// build vendors its compiled dist/ tree wholesale with `cp -R` (see the
// `build` script's `... dist/vendor/paperclip-runner/` step) -- code only,
// no node_modules alongside it. Every npm package the vendored code
// actually imports is therefore also a runtime dependency of server once
// vendored, and has to be declared there too (see acpx, ajv). That mirror
// step is easy to forget -- it silently missed smol-toml in #13110, and CI
// stayed green while production crash-looped 3 seconds into every start
// (#13116) -- because nothing enforced it.
//
// This derives the required set from esbuild's own module-resolution scan
// of the two entry points server actually imports (index.js, testing.js),
// with `write: false` so nothing is written to disk and `packages:
// "external"` so npm imports are reported, not inlined. That is precise:
// paperclip-runner declares dependencies (react-markdown, the codex/opencode
// CLI packages, ...) that only its unrelated ./react and ./browser export
// subpaths use, which server never imports, so requiring *every* declared
// dependency to be mirrored would be over-broad and demand dependencies
// server does not actually need.
//
// This intentionally only analyzes the module graph; it does not bundle or
// rewrite anything on disk. Several runner modules resolve sibling build
// artifacts -- the native runnerd binary, the ACPX/OpenCode CJS sidecar
// scripts in dist/cli, JSON replay fixtures under a top-level protocol/
// directory -- via `import.meta.url`-relative filesystem paths rather than
// JS imports, each at whatever nesting depth its source file happens to
// sit at. Actually bundling those entry points (an earlier version of this
// fix did) collapses and rearranges that layout, silently breaking those
// lookups. Preserving the original `cp -R` tree 1:1 is what keeps all of
// them working, so this script only verifies; it never restructures.
import { build } from "esbuild";
import { existsSync, readFileSync } from "node:fs";
import { builtinModules } from "node:module";
import { dirname, resolve } from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
const serverRoot = resolve(dirname(fileURLToPath(import.meta.url)), "..");
const runnerRoot = resolve(serverRoot, "../packages/paperclip-runner");
const runnerDist = resolve(runnerRoot, "dist");
// The only entry points server/src actually imports from the vendored
// runner (server/src/**/*.ts import "../vendor/paperclip-runner/index.js"
// or ".../testing.js").
const ENTRY_POINT_NAMES = ["index.js", "testing.js"];
const NODE_BUILTINS = new Set([
...builtinModules,
...builtinModules.map((name) => `node:${name}`),
]);
/** The npm package name a bare import specifier resolves to, honoring scoped packages and subpaths. */
function packageNameFromSpecifier(specifier) {
const segments = specifier.split("/");
return specifier.startsWith("@") ? segments.slice(0, 2).join("/") : segments[0];
}
export function findMissingVendorDependencies(importedPackageNames, declaredDependencyNames) {
return [...importedPackageNames]
.filter((name) => !declaredDependencyNames.has(name))
.sort();
}
export async function findRunnerExternalPackages(entryPoints) {
for (const entryPoint of entryPoints) {
if (!existsSync(entryPoint)) {
throw new Error(
`paperclip-runner vendor check: expected build output at ${entryPoint}. ` +
`Run "pnpm --filter @paperclipai/paperclip-runner build" first.`,
);
}
}
// write: false means this never touches disk -- it's a module-graph scan,
// not a real bundle. Vendoring itself still happens via `cp -R` elsewhere
// in the build script. `outdir` is required by esbuild for multiple entry
// points but nothing is ever written there, so any sibling path will do.
const result = await build({
entryPoints,
outdir: resolve(dirname(entryPoints[0]), ".vendor-dependency-scan"),
bundle: true,
write: false,
platform: "node",
format: "esm",
packages: "external",
metafile: true,
logLevel: "silent",
});
const externalPackageNames = new Set();
for (const output of Object.values(result.metafile.outputs)) {
for (const imported of output.imports) {
if (!imported.external) continue;
if (imported.path.startsWith(".") || NODE_BUILTINS.has(imported.path)) continue;
externalPackageNames.add(packageNameFromSpecifier(imported.path));
}
}
return externalPackageNames;
}
function readDependencyNames(packageJsonPath) {
const packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8"));
return new Map(Object.entries(packageJson.dependencies ?? {}));
}
function explainMissingDependencies(missing, runnerDependencyNames) {
const lines = missing.map((name) => {
const range = runnerDependencyNames.get(name);
return range
? ` "${name}": "${range}" (matches packages/paperclip-runner/package.json)`
: ` "${name}" (not declared as a paperclip-runner dependency either -- check for a missing or mistyped dependency there first)`;
});
return (
`paperclip-runner vendor check: server/package.json is missing the runtime ` +
`${missing.length === 1 ? "dependency" : "dependencies"} the vendored runner ` +
`imports at runtime:\n${lines.join("\n")}\n\n` +
`packages/paperclip-runner/dist is copied into server's own published package ` +
`without its node_modules, so every npm package the runner imports must also be ` +
`a direct dependency of server so it resolves once vendored. Add ` +
`${missing.length === 1 ? "it" : "them"} to server/package.json's "dependencies".`
);
}
async function main() {
const entryPoints = ENTRY_POINT_NAMES.map((name) => resolve(runnerDist, name));
const externalPackageNames = await findRunnerExternalPackages(entryPoints);
const serverDependencyNames = readDependencyNames(resolve(serverRoot, "package.json"));
const missing = findMissingVendorDependencies(
externalPackageNames,
new Set(serverDependencyNames.keys()),
);
if (missing.length > 0) {
const runnerDependencyNames = readDependencyNames(resolve(runnerRoot, "package.json"));
throw new Error(explainMissingDependencies(missing, runnerDependencyNames));
}
}
// Only run when invoked directly (`node scripts/verify-runner-vendor-dependencies.mjs`),
// not when the vitest suite imports findMissingVendorDependencies for a unit test.
const isMain = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href;
if (isMain) await main();

View File

@ -0,0 +1,98 @@
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import {
findMissingVendorDependencies,
findRunnerExternalPackages,
} from "./verify-runner-vendor-dependencies.mjs";
describe("findMissingVendorDependencies", () => {
it("returns nothing when every runner dependency is already declared on server", () => {
const missing = findMissingVendorDependencies(
new Set(["acpx", "ajv", "smol-toml"]),
new Set(["acpx", "ajv", "smol-toml", "express"]),
);
expect(missing).toEqual([]);
});
it("flags a runner dependency that isn't mirrored into server/package.json", () => {
// This is the exact shape of the incident this check exists to catch:
// packages/paperclip-runner/package.json grew a new runtime dependency
// (smol-toml) that never got mirrored into server/package.json, so the
// vendored `cp -R` copy failed to resolve it at runtime (#13110, #13116).
const missing = findMissingVendorDependencies(
new Set(["acpx", "ajv", "smol-toml"]),
new Set(["acpx", "ajv"]),
);
expect(missing).toEqual(["smol-toml"]);
});
it("sorts multiple missing dependencies for a stable error message", () => {
const missing = findMissingVendorDependencies(
new Set(["smol-toml", "ajv-formats", "acpx"]),
new Set(),
);
expect(missing).toEqual(["acpx", "ajv-formats", "smol-toml"]);
});
});
describe("findRunnerExternalPackages", () => {
// Fixture-level coverage for the actual esbuild scan, not just the diff
// function: a real dist/index.js + dist/testing.js on disk, structurally
// matching packages/paperclip-runner's shape (testing.js re-exports
// index.js, which imports another local module that imports a bare npm
// specifier), plus package.json dependency noise that should be ignored
// because nothing reachable from these entry points imports it.
let fixtureDir;
afterEach(() => {
if (fixtureDir) rmSync(fixtureDir, { recursive: true, force: true });
fixtureDir = undefined;
});
function writeFixture() {
fixtureDir = mkdtempSync(join(tmpdir(), "paperclip-runner-vendor-fixture-"));
writeFileSync(
join(fixtureDir, "internal.js"),
'import { parse } from "smol-toml";\n' +
"export function parseSomething(text) { return parse(text); }\n",
);
writeFileSync(
join(fixtureDir, "index.js"),
'export * from "./internal.js";\nexport const marker = "index";\n',
);
writeFileSync(
join(fixtureDir, "testing.js"),
'export * from "./index.js";\nexport const testingMarker = "testing";\n',
);
return [join(fixtureDir, "index.js"), join(fixtureDir, "testing.js")];
}
it("reports only the npm packages actually reachable from the entry points", async () => {
const entryPoints = writeFixture();
const externalPackageNames = await findRunnerExternalPackages(entryPoints);
// smol-toml is reachable through internal.js -> index.js -> testing.js
// and must be reported. Nothing else was imported anywhere in the
// fixture, so this also proves the scan doesn't fall back to "every
// dependency the package declares" (which is what made the check
// over-broad before -- see the module header).
expect(externalPackageNames).toEqual(new Set(["smol-toml"]));
});
it("throws a clear, actionable error when an entry point is missing", async () => {
fixtureDir = mkdtempSync(join(tmpdir(), "paperclip-runner-vendor-fixture-"));
const missingEntryPoint = join(fixtureDir, "index.js");
await expect(findRunnerExternalPackages([missingEntryPoint])).rejects.toThrow(
/expected build output at .*index\.js.*Run "pnpm --filter @paperclipai\/paperclip-runner build" first/s,
);
});
});

View File

@ -62,6 +62,22 @@ describe("server package build script", () => {
);
});
it("verifies vendored runner dependencies are mirrored before building", () => {
const packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")) as {
scripts?: Record<string, string>;
};
// See scripts/verify-runner-vendor-dependencies.mjs: packages/paperclip-runner
// is vendored with a raw `cp -R` of its compiled dist/, so every runtime
// dependency it imports must also be a direct dependency of server. This
// check derives that requirement from an esbuild scan of the vendored
// entry points instead of relying on a human to have kept a hand-copied
// list in sync (the smol-toml incident in #13110/#13116).
expect(packageJson.scripts?.build).toContain(
"node scripts/verify-runner-vendor-dependencies.mjs",
);
});
it("loads runner source when the source server starts before workspace builds", () => {
const shim = readFileSync(runnerShimPath, "utf8");

View File

@ -15,7 +15,7 @@ export default defineConfig({
},
test: {
environment: "node",
include: ["src/**/*.test.ts"],
include: ["src/**/*.test.ts", "scripts/**/*.test.mjs"],
// Each server suite boots + tears down its own embedded Postgres in
// beforeAll/afterAll. Under the loaded serial shard (maxWorkers=1) the
// graceful shutdown can occasionally cross vitest's default 10s hookTimeout,