5 Commits

Include renames

Author	SHA1	Message	Date
Nicky LeachandClaude Sonnet 5	04a9f89ede	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>	2026-09-09 16:38:17 -07:00
Nicky LeachandPaperclip	5a1ce7aed8	fix(server): stamp built commit into service.version (#11748) ... ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server emits OpenTelemetry spans so operators can trace agent work > - Each span needs a service version that identifies the code that produced it > - The current service version comes from a static environment value and can become stale after a rebuild > - This pull request records the built commit and resolves the service version from the build stamp, runtime Git, the environment, or an unknown fallback > - The benefit is trace data that identifies the correct built commit during development and deployment ## Linked Issues or Issue Description **What happened?** The server used a static `OTEL_SERVICE_VERSION` value for every OpenTelemetry span. Rebuilds could produce traces with an old commit value. **Expected behavior** The server should report the built commit when a build stamp exists. It should use runtime Git, the environment value, or `unknown` as fallback. **Steps to reproduce** 1. Set `OTEL_SERVICE_VERSION` to an old commit value. 2. Build the server at a different commit. 3. Start the server and inspect the OpenTelemetry service version. 4. Confirm that the built commit takes precedence over the old environment value. ## What Changed - Add a build script that writes the short Git commit to `dist/build-info.json`. - Resolve `service.version` from the build stamp, runtime Git, the environment, or `unknown`. - Log the resolved service version once during server startup. - Add tests for the resolution order and safe behavior without Git. - Document the resolution order in `doc/observability.md`. ## Verification - `pnpm --filter @paperclipai/server build` - `npx vitest run server/src/__tests__/service-version.test.ts` - `pnpm --filter @paperclipai/server typecheck` - Confirm that the build stamp contains the short commit. - Confirm that the stamp wins over the environment value. - Confirm that a build without Git exits successfully without a stamp. ## Risks The server now prefers the built commit over `OTEL_SERVICE_VERSION`. A build without Git uses the existing environment value or `unknown`. The change needs no schema migration and has a single-commit rollback path. ## Model Used OpenAI Codex, GPT-5, tool use and code execution. The runtime does not expose the context window size or reasoning 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 - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-08-19 21:20:40 -07:00
vanductai	9a70a4edaa	fix(server): use stable tsx/cli entry point in dev-watch ... The dev-watch script was importing tsx via the internal path 'tsx/dist/cli.mjs', which is an undocumented implementation detail that broke when tsx updated its internal structure. Switched to the stable public export 'tsx/cli' which is the officially supported entry point and won't break across versions.	2026-03-28 06:42:03 +07:00
dottaandPaperclip	9ddf960312	Harden dev-watch excludes for nested UI outputs ... Co-Authored-By: Paperclip <noreply@paperclip.ing>	2026-03-26 12:35:19 -05:00
dottaandPaperclip	ab82e3f022	Fix worktree runtime isolation recovery ... Co-Authored-By: Paperclip <noreply@paperclip.ing>	2026-03-26 11:12:39 -05:00