From 7895f7f2b043056d227281b7b4ee63a358abfb7f Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Thu, 27 Aug 2026 19:10:49 -0700 Subject: [PATCH] Install the declared Sentry server package into the hosted image (#12330) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Paperclip supports opt-in Sentry error monitoring for server and browser errors. > - The hosted image must include the server package when an operator sets SENTRY_DSN. > - The server package is an optional peer in the source tree, so the image did not include it. > - This pull request installs the declared server package in the hosted image and checks the result. > - The benefit is a hosted tenant can send server errors without a manual package install. ## Linked Issues or Issue Description No public issue exists for this change. **What happened?** The hosted image did not include the declared @sentry/node server package. A hosted tenant could set SENTRY_DSN, but the server could not load the package from the image. **Expected behavior** The hosted image must include the exact @sentry/node version from server/package.json. The self-hosted image must remain without this optional package. **Steps to reproduce** 1. Build or pull the hosted image. 2. Resolve @sentry/node from the server package path. 3. Compare its version with server/package.json. 4. Confirm that the tsx loader path still resolves. **Paperclip version or commit** Commit b6ff556a33ebdbe764b7f495951cd59009776608. **Deployment mode** Docker hosted image. ## What Changed - Add a cloud-server-deps Docker stage that installs the declared @sentry/node version in isolation. - Copy the isolated package into the cloud image without changing the production image. - Add a probe that checks the tsx loader and the resolved Sentry version. - Run the probe after the hosted image push in the Docker workflow. - Add server tests and update the observability documentation. ## Verification - Run `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/cloud-image-sentry.test.ts`. - Confirm that the changed test passes in CI. - Confirm that all pull request checks pass. - Note that the Docker workflow does not run for pull requests. It runs after a push to master, for configured tags, or after manual dispatch. ## Risks - Low risk. The production image body stays unchanged. - The cloud image adds the declared Sentry package and a small dependency tree. - The workflow probe fails if the image loses the tsx loader or resolves a different Sentry version. ## Model Used OpenAI GPT-5; exact model version supplied by the execution service; tool use and code execution; context window not specified. ## 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 --- .github/workflows/docker.yml | 32 +++ Dockerfile | 61 ++++++ doc/observability.md | 10 + scripts/assert-cloud-image-sentry.mjs | 67 ++++++ .../src/__tests__/cloud-image-sentry.test.ts | 206 ++++++++++++++++++ 5 files changed, 376 insertions(+) create mode 100644 scripts/assert-cloud-image-sentry.mjs create mode 100644 server/src/__tests__/cloud-image-sentry.test.ts diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index a267581aa1..14b8937abe 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -373,8 +373,12 @@ jobs: target: cloud # Space-separated sandbox-provider directory names to build into # the variant; add here when managed deployments need another. + # CLOUD_BUNDLED_SERVER_DEPS names the optional peer packages the + # variant installs from server/package.json's declared version; + # add another name there when a managed tenant needs it. build-args: | CLOUD_BUNDLED_PLUGINS=daytona + CLOUD_BUNDLED_SERVER_DEPS=@sentry/node PAPERCLIP_BUILD_VERSION=${{ steps.build-version.outputs.version }} PAPERCLIP_BUILD_COMMIT=${{ github.sha }} CLI_TOOLS_CACHE_EPOCH=${{ steps.tools-epoch.outputs.epoch }} @@ -391,3 +395,31 @@ jobs: cache-to: type=registry,ref=ghcr.io/${{ github.repository }}:buildcache-cloud,mode=max tags: ${{ steps.meta-cloud.outputs.tags }} labels: ${{ steps.meta-cloud.outputs.labels }} + + # The cloud target installs @sentry/node at the version + # server/package.json declares, into a directory the server's own + # module resolution walks. Verify the image this job just pushed, not + # a local build, so a build-cache or layer-ordering regression is + # caught before any tenant runs the image. + - name: Verify the pushed image resolves the declared Sentry version + env: + IMAGE_TAGS: ${{ steps.meta-cloud.outputs.tags }} + run: | + set -euo pipefail + image="$(printf '%s\n' "$IMAGE_TAGS" | head -n 1)" + test -n "$image" + + expected="$(node -e "process.stdout.write(require('./server/package.json').peerDependencies['@sentry/node'])")" + test -n "$expected" + + installed="$(docker run --rm --pull always \ + -v "$PWD/scripts/assert-cloud-image-sentry.mjs:/app/server/.ci-sentry-probe.mjs:ro" \ + --entrypoint node "$image" /app/server/.ci-sentry-probe.mjs)" + + echo "Declared optional peer version: $expected" + echo "Installed in the pushed image: $installed" + if [ "$installed" != "$expected" ]; then + echo "ERROR: the pushed image resolves @sentry/node@$installed, expected @sentry/node@$expected" >&2 + exit 1 + fi + echo "The pushed image resolves the declared @sentry/node version." diff --git a/Dockerfile b/Dockerfile index 8670a70dc7..92566cc3fe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -166,5 +166,66 @@ RUN set -eu; \ test -f "$dir/dist/manifest.js" || { echo "ERROR: $dir is missing dist/manifest.js after build" >&2; exit 1; }; \ done +# The hosted image variant ships selected optional peer packages +# pre-installed. A managed tenant then needs no separate install step. +# The self-hosted image stays on the opt-in contract: it never runs this +# stage, so a package like `@sentry/node` stays a true optional peer +# dependency. A self-hosted operator installs it by hand (see +# doc/observability.md). +# +# CLOUD_BUNDLED_SERVER_DEPS names the optional peer packages to install. +# The value is a space-separated list, the same shape as +# CLOUD_BUNDLED_PLUGINS above. The stage reads each package's version +# from the `peerDependencies` block of `server/package.json` at build +# time, so the version has one committed home. +# +# The stage fails the build in three cases: +# - the argument is empty +# - server/package.json declares no version for a named package +# - the named package is not an optional peer +# +# This check keeps the argument limited to packages the server already +# treats as optional. +# +# The install happens in its own isolated directory, not inside +# `server`'s own workspace install. The self-hosted target above never +# gains these packages this way. The directory sits under `/app`, not +# `server/`, and `--ignore-workspace` below excludes it from the pnpm +# workspace. From that directory, pnpm still finds the `packageManager` +# pin in the repo's own `package.json` by walking up — the same pnpm +# version the rest of the build uses. +# +# The install writes no lock file (`--no-lockfile`, the same flag the +# `cloud-plugins` stage above uses). Two builds of the same commit can +# therefore install different transitive versions of a named package. +# Three facts make this an accepted trade-off: +# - the `cloud-plugins` stage above already has the same property, with +# the same flag +# - the direct version of each named package comes from one exact, +# single-sourced place: the `peerDependencies` block of +# `server/package.json` +# - an automated check asserts the installed direct version after every +# build, so a transitive drift that breaks the package still fails the +# build +FROM build AS cloud-server-deps +WORKDIR /app/.cloud-server-deps +ARG CLOUD_BUNDLED_SERVER_DEPS="@sentry/node" +RUN set -eu; \ + test -n "$CLOUD_BUNDLED_SERVER_DEPS" || { echo "ERROR: CLOUD_BUNDLED_SERVER_DEPS is empty; name at least one optional peer package to install" >&2; exit 1; }; \ + echo '{"name":"paperclip-cloud-server-deps","private":true}' > package.json; \ + specifiers=""; \ + for name in $CLOUD_BUNDLED_SERVER_DEPS; do \ + version="$(node -e "const pkg=require('/app/server/package.json'); const name=process.argv[1]; const version=(pkg.peerDependencies||{})[name]; if(!version){console.error('ERROR: server/package.json declares no peerDependencies version for '+JSON.stringify(name));process.exit(1);} const meta=(pkg.peerDependenciesMeta||{})[name]; if(!meta||meta.optional!==true){console.error('ERROR: '+JSON.stringify(name)+' is not declared as an optional peer dependency in server/package.json; CLOUD_BUNDLED_SERVER_DEPS may name only optional peer packages');process.exit(1);} process.stdout.write(version);" "$name")"; \ + test -n "$version" || { echo "ERROR: could not resolve a version for '$name'" >&2; exit 1; }; \ + specifiers="$specifiers ${name}@${version}"; \ + done; \ + test -n "$specifiers" || { echo "ERROR: CLOUD_BUNDLED_SERVER_DEPS names no package" >&2; exit 1; }; \ + pnpm add --ignore-workspace --no-lockfile $specifiers + FROM production AS cloud COPY --chown=node:node --from=cloud-plugins /app/packages/plugins/sandbox-providers /app/packages/plugins/sandbox-providers +# Land the isolated install inside the server's own `node_modules`, the +# directory Node's module resolution walks up to from `/app/server` for +# both a CommonJS `require.resolve` and an ECMAScript `import` — an entry +# on `NODE_PATH` would satisfy only the first and silently fail the second. +COPY --chown=node:node --from=cloud-server-deps /app/.cloud-server-deps/node_modules /app/server/node_modules diff --git a/doc/observability.md b/doc/observability.md index 3c25a9e5ec..8f0d193e29 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -163,6 +163,16 @@ mismatch (see "Server request data" below). pnpm add @sentry/node@10.71.0 ``` +**The hosted image variant ships this package pre-installed.** A managed +tenant runs the image built from the Dockerfile's `cloud` target, and that +target installs the declared version of `@sentry/node` at build time. A +managed tenant needs only `SENTRY_DSN` set; no install step is needed. + +A self-hosted operator runs the image built from the `production` target. +That image holds no Sentry package, the same as before this feature +existed. A self-hosted operator who wants server error monitoring still +completes the install step above. + The browser package, `@sentry/browser`, needs no install step. It is already a development dependency of the `ui` package, pinned to the same exact version, **`10.71.0`**, so the browser code ships inside every diff --git a/scripts/assert-cloud-image-sentry.mjs b/scripts/assert-cloud-image-sentry.mjs new file mode 100644 index 0000000000..4b64b5cd2e --- /dev/null +++ b/scripts/assert-cloud-image-sentry.mjs @@ -0,0 +1,67 @@ +#!/usr/bin/env node +// Prove two things about the target image's server directory, in order: +// +// 1. The production `CMD` can still find its ECMAScript module loader, +// `server/node_modules/tsx/dist/loader.mjs`. That path is a symbolic +// link into the workspace pnpm store, and the `cloud` stage's Sentry +// copy writes into the same `server/node_modules` directory. A copy +// that removes or shadows the link stops the container from booting. +// 2. The installed `@sentry/node` package resolves, the same way the +// server's own peer-version gate does +// (server/src/peer-version-check.ts). Then print its version. +// +// Exit non-zero, with a clear message on standard error, when either check +// fails. Print only the version string on standard output on success. +// +// Mount this file at a path inside the target image's server directory and +// run it there with `node`, so module resolution walks the same +// `node_modules` tree the running server itself resolves from: +// +// docker run --rm \ +// -v "$PWD/scripts/assert-cloud-image-sentry.mjs:/app/server/.ci-sentry-probe.mjs:ro" \ +// --entrypoint node /app/server/.ci-sentry-probe.mjs +import { createRequire } from "node:module"; +import { existsSync, readFileSync, realpathSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const serverDir = dirname(fileURLToPath(import.meta.url)); + +// The production `CMD` boots the server through this exact path, resolved +// as a plain relative file path from the container's `/app` working +// directory (`./server/node_modules/tsx/dist/loader.mjs`), so it bypasses +// package-export checks and only needs the file to exist once symbolic +// links resolve. Follow the link the same way Node's own module loader +// does, so a broken or missing link fails this probe before it fails a +// live container. +const tsxLoaderPath = join(serverDir, "node_modules", "tsx", "dist", "loader.mjs"); +try { + realpathSync(tsxLoaderPath); +} catch (error) { + console.error( + `could not resolve ${tsxLoaderPath}: the production CMD boots through this path and the server cannot start without it (${error.message})`, + ); + process.exit(1); +} + +// Proves the ECMAScript import path resolves. `require.resolve` below +// checks the CommonJS path; the server needs both to succeed. +await import("@sentry/node"); + +const require = createRequire(import.meta.url); +let dir = dirname(require.resolve("@sentry/node")); +for (;;) { + const candidate = join(dir, "package.json"); + if (existsSync(candidate)) { + const parsed = JSON.parse(readFileSync(candidate, "utf8")); + if (parsed.name === "@sentry/node") { + process.stdout.write(parsed.version); + process.exit(0); + } + } + const parent = dirname(dir); + if (parent === dir) break; + dir = parent; +} +console.error("could not resolve the installed @sentry/node package"); +process.exit(1); diff --git a/server/src/__tests__/cloud-image-sentry.test.ts b/server/src/__tests__/cloud-image-sentry.test.ts new file mode 100644 index 0000000000..6eb4d14777 --- /dev/null +++ b/server/src/__tests__/cloud-image-sentry.test.ts @@ -0,0 +1,206 @@ +import { spawnSync } from "node:child_process"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +/** + * Drift guard for the cloud image variant's bundled Sentry server package + * (Dockerfile `cloud` target). + * + * The self-hosted image, built from the `production` target, keeps + * `@sentry/node` as a true optional peer dependency: the operator installs + * it themselves. The hosted (cloud) image installs the packages the + * `CLOUD_BUNDLED_SERVER_DEPS` build argument names, so a managed tenant + * gets server error reports with no separate install step. The stage + * reads each package's version from the `peerDependencies` block of + * `server/package.json` at build time, so the version has one committed + * home. This test pins the invariants that nothing else ties together: + * every Dockerfile instruction that installs `@sentry/node` sits strictly + * after the `production` stage body ends; the Dockerfile and the docker + * workflow carry no literal version pin (they read the version from + * `server/package.json` at build time instead); the `cloud-server-deps` + * stage declares the `CLOUD_BUNDLED_SERVER_DEPS` build argument with a + * default that names `@sentry/node`; the docker workflow passes that same + * argument to the cloud build; and no committed manifest re-declares the + * version. + */ + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", "..", ".."); +const dockerfile = readFileSync(path.join(repoRoot, "Dockerfile"), "utf8"); +const workflow = readFileSync(path.join(repoRoot, ".github", "workflows", "docker.yml"), "utf8"); +const serverPackageJson = JSON.parse( + readFileSync(path.join(repoRoot, "server", "package.json"), "utf8"), +) as { peerDependencies?: Record }; + +const declaredVersion = serverPackageJson.peerDependencies?.["@sentry/node"]; + +const probeSource = readFileSync( + path.join(repoRoot, "scripts", "assert-cloud-image-sentry.mjs"), + "utf8", +); + +/** + * Build a throwaway directory that stands in for the image's `/app/server` + * directory: a copy of the probe script (module resolution walks from a + * script's own location, so the copy must sit where the fake `server` + * directory expects it), a minimal but real `@sentry/node` package, and, + * when `withTsxLoader` is true, a symbolic link at `node_modules/tsx` that + * mirrors the real workspace install (a link out to a separate store + * directory holding `dist/loader.mjs`). Omitting the link stands in for the + * Sentry copy removing or shadowing it. + */ +function buildFakeServerDir(withTsxLoader: boolean) { + const root = mkdtempSync(path.join(tmpdir(), "cloud-image-sentry-probe-")); + const serverDir = path.join(root, "server"); + const sentryDir = path.join(serverDir, "node_modules", "@sentry", "node"); + mkdirSync(sentryDir, { recursive: true }); + writeFileSync( + path.join(sentryDir, "package.json"), + JSON.stringify({ name: "@sentry/node", version: "9.9.9", type: "module", main: "index.mjs" }), + ); + writeFileSync(path.join(sentryDir, "index.mjs"), "export {};\n"); + + if (withTsxLoader) { + const tsxStoreDist = path.join(root, "tsx-store", "dist"); + mkdirSync(tsxStoreDist, { recursive: true }); + writeFileSync(path.join(tsxStoreDist, "loader.mjs"), "export {};\n"); + symlinkSync(path.join("..", "..", "tsx-store"), path.join(serverDir, "node_modules", "tsx")); + } + + const probeCopy = path.join(serverDir, "probe.mjs"); + writeFileSync(probeCopy, probeSource); + return { root, probeCopy }; +} + +function runProbe(probeCopy: string) { + return spawnSync(process.execPath, [probeCopy], { encoding: "utf8" }); +} + +describe("cloud image Sentry install", () => { + it("declares @sentry/node as an optional peer in server/package.json", () => { + expect( + declaredVersion, + "server/package.json must declare @sentry/node as an optional peer", + ).toBeTruthy(); + }); + + it("installs @sentry/node only after the production stage body ends", () => { + const stageHeaderPattern = /^FROM\s+\S+\s+AS\s+(\S+)/gim; + const stages = [...dockerfile.matchAll(stageHeaderPattern)].map((match) => ({ + name: match[1], + index: match.index ?? 0, + })); + + const productionIndex = stages.findIndex((stage) => stage.name.toLowerCase() === "production"); + expect(productionIndex, "the Dockerfile must declare a production stage").toBeGreaterThanOrEqual(0); + + // The next declared stage after `production` marks where its body ends. + const productionBodyEnd = stages[productionIndex + 1]?.index ?? dockerfile.length; + + const sentryMentionOffsets = [...dockerfile.matchAll(/@sentry\/node/g)].map( + (match) => match.index ?? 0, + ); + expect( + sentryMentionOffsets.length, + "the Dockerfile must install @sentry/node somewhere, for the cloud image variant", + ).toBeGreaterThan(0); + + for (const offset of sentryMentionOffsets) { + expect( + offset, + "every @sentry/node mention must sit after the production stage body ends, " + + "so the self-hosted target never installs it", + ).toBeGreaterThanOrEqual(productionBodyEnd); + } + }); + + it("copies the installed package into the cloud stage's server node_modules", () => { + expect(dockerfile).toMatch( + /^COPY --chown=node:node --from=[\w-]+ \S+ \S*server\/node_modules$/m, + ); + }); + + it("reads the installed version from server/package.json instead of a second hardcoded copy", () => { + // Matches a literal pin such as "@sentry/node@10.71.0", not a shell + // variable interpolation such as "@sentry/node@${version}". + const versionPinPattern = /@sentry\/node@(\d[^\s"'`]*)/g; + + for (const source of [ + { label: "Dockerfile", text: dockerfile }, + { label: "docker workflow", text: workflow }, + ]) { + for (const match of source.text.matchAll(versionPinPattern)) { + expect( + match[1], + `${source.label} pins @sentry/node@${match[1]}, which must equal the declared ` + + `optional peer version ${declaredVersion}`, + ).toBe(declaredVersion); + } + } + }); + + it("declares the CLOUD_BUNDLED_SERVER_DEPS build argument with a default that names @sentry/node", () => { + const argPattern = /^ARG\s+CLOUD_BUNDLED_SERVER_DEPS="([^"]*)"/m; + const match = dockerfile.match(argPattern); + expect( + match, + "the Dockerfile must declare ARG CLOUD_BUNDLED_SERVER_DEPS with a quoted default value", + ).not.toBeNull(); + + const names = (match?.[1] ?? "").split(/\s+/).filter(Boolean); + expect( + names, + "the CLOUD_BUNDLED_SERVER_DEPS default must name @sentry/node", + ).toContain("@sentry/node"); + }); + + it("passes CLOUD_BUNDLED_SERVER_DEPS to the cloud build in the docker workflow", () => { + expect(workflow).toMatch(/^\s*CLOUD_BUNDLED_SERVER_DEPS=@sentry\/node\s*$/m); + }); + + it("declares no committed manifest that re-states the version", () => { + expect( + existsSync(path.join(repoRoot, "docker", "cloud-server-deps")), + "docker/cloud-server-deps must not exist; the version has one home, " + + "server/package.json's peerDependencies block", + ).toBe(false); + }); +}); + +describe("cloud image Sentry probe: the server's tsx loader", () => { + it("exits non-zero and names the loader path when server/node_modules/tsx does not resolve", () => { + const { root, probeCopy } = buildFakeServerDir(false); + try { + const result = runProbe(probeCopy); + expect(result.status, "the probe must fail loudly, not boot a broken image").not.toBe(0); + expect( + result.stderr, + "the error must name the exact path the production CMD boots through", + ).toContain(path.join("node_modules", "tsx", "dist", "loader.mjs")); + expect(result.stdout, "a failed probe must not print a version string").toBe(""); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("still prints only the installed @sentry/node version when the loader resolves", () => { + const { root, probeCopy } = buildFakeServerDir(true); + try { + const result = runProbe(probeCopy); + expect(result.status).toBe(0); + expect(result.stdout).toBe("9.9.9"); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); +});