From 174e35a1443de27090a2ff9a6efa3b9e9624dfc2 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 3 Sep 2026 10:02:42 -0700 Subject: [PATCH] fix(server): stop paging Sentry for supervised boot races in managed cloud (#12772) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server refuses to boot when its database is not migrated, or when an authenticated public deployment has no `DATABASE_URL`. These refusals are deliberate and correct. > - In managed cloud, a supervisor creates each stack, migrates its fresh database, applies configuration, and restarts the app. The app container often boots before those steps finish. > - Each early boot hits one of the two refusals, exits, and captures the refusal to Sentry. One fleet build batch produces hundreds of identical expected events. Real errors get buried. > - This pull request classifies exactly those two refusals as expected transients when `PAPERCLIP_CLOUD_API_ORIGIN` marks a supervised deployment, and skips only the Sentry capture for them. > - The benefit is a clean error signal: expected provisioning noise stops, and every real failure still reports. ## Linked Issues or Issue Description **What happened?** A managed-cloud stack boots its app container before the supervisor migrates the empty database or finishes applying configuration. The container refuses to start, crash-loops briefly, and converges after the supervisor restarts it. Every refused boot sends an error event to Sentry. A batch of new stacks produces hundreds of these expected events. **Expected behavior** The refusal logs and exits nonzero, so the supervisor can act. Sentry receives no event for an expected provisioning transient. Sentry still receives events for real failures: schema drift, malformed configuration, and every refusal outside managed cloud. **Steps to reproduce** 1. Set `PAPERCLIP_MIGRATION_AUTO_APPLY=false`, `PAPERCLIP_MIGRATION_PROMPT=never`, `SENTRY_DSN`, and `PAPERCLIP_CLOUD_API_ORIGIN`. 2. Point `DATABASE_URL` at an empty database and start the server. 3. The server refuses to start. Before this change it also captures the refusal to Sentry on every boot. **Deployment mode** Authenticated public (managed cloud). ## What Changed - New `server/src/startup-refusals.ts`: a `StartupRefusalError` class for refusals whose remedy belongs to the deployment supervisor, `migrationRefusalError()` to classify a pending-migrations refusal (zero applied migrations = never migrated = supervised transient; any applied history = drift = plain always-reported `Error`), and `shouldReportStartupFailure()` for the capture decision. - `server/src/index.ts`: the pending-migrations refusal uses the classifier; the missing-`DATABASE_URL` refusal under the authenticated-public contract becomes a `StartupRefusalError` (the malformed-URL refusal stays a plain `Error`); the startup crash handler consults `shouldReportStartupFailure()` before `captureException`. Logging and the nonzero exit are unchanged. - New `server/src/__tests__/startup-refusals.test.ts` covering the classification and decision matrix, including the unchanged self-hosted paths. ## Verification - `pnpm vitest run src/__tests__/startup-refusals.test.ts` — 7 passed. - Review the decision matrix in the test file: refusals report when `PAPERCLIP_CLOUD_API_ORIGIN` is absent or blank; non-refusal errors and non-`Error` throwables always report; drift always reports. ## Risks - Low risk. The change only skips a Sentry capture in one narrow, marker-gated case. Boot behavior, logging, and the exit code do not change. - Self-hosted deployments do not set `PAPERCLIP_CLOUD_API_ORIGIN`, so their reporting is unchanged, and the tests pin that. - A supervised deployment with a genuinely stuck migration runner loses per-boot Sentry events for that stack. The supervisor's own health checks and monitoring own that signal, and the container logs still carry the refusal. ## Model Used Claude Fable 5 (claude-fable-5) via Claude Code, extended thinking with tool use. ## 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 (none found for startup Sentry suppression) - [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 (module doc comment; no user-facing docs affected) - [x] I have considered and documented any risks above --- server/src/__tests__/startup-refusals.test.ts | 75 +++++++++++++++++++ server/src/index.ts | 32 +++++++- server/src/startup-refusals.ts | 73 ++++++++++++++++++ 3 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 server/src/__tests__/startup-refusals.test.ts create mode 100644 server/src/startup-refusals.ts diff --git a/server/src/__tests__/startup-refusals.test.ts b/server/src/__tests__/startup-refusals.test.ts new file mode 100644 index 0000000000..763b91ac32 --- /dev/null +++ b/server/src/__tests__/startup-refusals.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from "vitest"; + +import { + StartupRefusalError, + migrationRefusalError, + shouldReportStartupFailure, +} from "../startup-refusals.ts"; + +describe("migrationRefusalError", () => { + const message = "PostgreSQL has pending migrations (…). Refusing to start."; + + it("classifies a never-migrated database as a supervised-transient refusal", () => { + const error = migrationRefusalError({ appliedMigrations: [], tableCount: 0 }, message); + expect(error).toBeInstanceOf(StartupRefusalError); + expect((error as StartupRefusalError).kind).toBe("schema-not-yet-migrated"); + expect(error.message).toContain("Refusing to start"); + }); + + it("keeps pending migrations on a migrated database as a plain, always-reported error", () => { + const error = migrationRefusalError( + { appliedMigrations: ["0000_init.sql"], tableCount: 41 }, + message, + ); + expect(error).toBeInstanceOf(Error); + expect(error).not.toBeInstanceOf(StartupRefusalError); + }); + + it("treats an empty journal beside existing tables as drift, not a fresh database", () => { + // A wiped or never-populated migration journal next to real tables is + // a persistent failure; it must keep reporting. + const error = migrationRefusalError({ appliedMigrations: [], tableCount: 17 }, message); + expect(error).toBeInstanceOf(Error); + expect(error).not.toBeInstanceOf(StartupRefusalError); + }); +}); + +describe("shouldReportStartupFailure", () => { + const refusal = new StartupRefusalError( + "database-contract-unmet", + "authenticated public deployments require DATABASE_URL", + ); + + it("always reports non-refusal startup failures, managed cloud or not", () => { + expect(shouldReportStartupFailure(new Error("boom"), {})).toBe(true); + expect( + shouldReportStartupFailure(new Error("boom"), { + PAPERCLIP_CLOUD_API_ORIGIN: "https://cloud.example.com", + }), + ).toBe(true); + }); + + it("reports supervised-transient refusals outside managed-cloud deployments", () => { + expect(shouldReportStartupFailure(refusal, {})).toBe(true); + }); + + it("suppresses supervised-transient refusals when a cloud supervisor owns the deployment", () => { + expect( + shouldReportStartupFailure(refusal, { + PAPERCLIP_CLOUD_API_ORIGIN: "https://cloud.example.com", + }), + ).toBe(false); + }); + + it("treats a blank cloud origin as unset", () => { + expect(shouldReportStartupFailure(refusal, { PAPERCLIP_CLOUD_API_ORIGIN: " " })).toBe(true); + }); + + it("reports non-Error throwables unconditionally", () => { + expect( + shouldReportStartupFailure("string failure", { + PAPERCLIP_CLOUD_API_ORIGIN: "https://cloud.example.com", + }), + ).toBe(true); + }); +}); diff --git a/server/src/index.ts b/server/src/index.ts index 7d12d95cfa..d35563665a 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -35,6 +35,11 @@ import detectPort from "detect-port"; import { createApp } from "./app.js"; import { loadConfig } from "./config.js"; import { logger } from "./middleware/logger.js"; +import { + StartupRefusalError, + migrationRefusalError, + shouldReportStartupFailure, +} from "./startup-refusals.js"; import { getManagedInstanceConfig, type ManagedInstanceConfig, @@ -243,12 +248,20 @@ export async function startServer(): Promise { const apply = autoApply ? true : await promptApplyMigrations(state.pendingMigrations); if (!apply) { - throw new Error( + // A database with zero applied migrations and zero tables has + // never been migrated: under a managed-cloud supervisor that is + // the expected first-boot race (the harness migrates and + // restarts), so the refusal carries the supervised-transient + // class. Applied history — or pre-existing tables beside an empty + // journal — means drift and keeps the plain, always-reported + // Error. + throw migrationRefusalError( + state, `${label} has pending migrations (${formatPendingMigrationSummary(state.pendingMigrations)}). ` + "Refusing to start against a stale schema. Run pnpm db:migrate or set PAPERCLIP_MIGRATION_AUTO_APPLY=true.", ); } - + logger.info({ pendingMigrations: state.pendingMigrations }, `Applying ${state.pendingMigrations.length} pending migrations for ${label}`); await applyPendingMigrations(connectionString); return "applied (pending migrations)"; @@ -268,7 +281,13 @@ export async function startServer(): Promise { return; } if (!config.databaseUrl) { - throw new Error( + // Under a managed-cloud supervisor a missing DATABASE_URL on boot + // is the config-application race (the container can start before + // the staged variables land), not operator error — the supervisor + // restarts once the config holds. A malformed value below is a + // real misconfiguration and stays an always-reported Error. + throw new StartupRefusalError( + "database-contract-unmet", "authenticated public deployments require DATABASE_URL or config.database.connectionString; refusing embedded PostgreSQL fallback", ); } @@ -1817,7 +1836,12 @@ function isMainModule(metaUrl: string): boolean { if (isMainModule(import.meta.url)) { void startServer().catch(async (err) => { logger.error({ err }, "Paperclip server failed to start"); - captureException(err); + // Supervised-transient refusals in managed-cloud deployments are an + // expected provisioning phase (see startup-refusals.ts) — they log + // and exit nonzero but do not page Sentry. + if (shouldReportStartupFailure(err)) { + captureException(err); + } await shutdownSentry(); process.exit(1); }); diff --git a/server/src/startup-refusals.ts b/server/src/startup-refusals.ts new file mode 100644 index 0000000000..08c3153ffd --- /dev/null +++ b/server/src/startup-refusals.ts @@ -0,0 +1,73 @@ +/** + * Deliberate boot refusals and whether they should page Sentry. + * + * Some startup preconditions the server must not repair itself: an + * unmigrated schema when auto-apply is off (the operator's migration + * runner owns schema), or an unmet database contract for authenticated + * public deployments. The server logs the refusal and exits nonzero so + * whatever supervises the deployment can act. + * + * In supervised managed-cloud deployments (`PAPERCLIP_CLOUD_API_ORIGIN` + * set), two of these refusals are a routine provisioning phase rather + * than an incident: a freshly created stack's app container boots + * before the harness has migrated the empty database or finished + * applying its committed configuration, crash-loops briefly, and is + * restarted by the harness once the precondition holds. Reporting every + * such boot to Sentry buries real errors under hundreds of expected + * events per fleet build batch, so the crash handler skips the capture + * for exactly this class — the refusal still logs and still exits + * nonzero. Everywhere else (self-hosted, local dev) reporting is + * unchanged. + */ + +export type StartupRefusalKind = + | "schema-not-yet-migrated" + | "database-contract-unmet"; + +/** + * A boot refusal whose remedy belongs to the deployment's supervisor. + * Only refusals that are *expected transients* under managed-cloud + * provisioning use this class; refusals that always indicate operator + * error (schema drift, a malformed DATABASE_URL) stay plain `Error`s. + */ +export class StartupRefusalError extends Error { + readonly kind: StartupRefusalKind; + + constructor(kind: StartupRefusalKind, message: string) { + super(message); + this.name = "StartupRefusalError"; + this.kind = kind; + } +} + +/** + * Chooses the error class for a pending-migrations refusal. A database + * with zero applied migrations AND zero tables is not stale — it has + * never been migrated at all, which under a supervisor means "not yet" + * rather than "drifted". Any applied history, or any pre-existing + * tables beside an empty or wiped migration journal, makes pending + * migrations a drift signal that must keep reporting. + */ +export function migrationRefusalError( + state: { appliedMigrations: string[]; tableCount: number }, + message: string, +): Error { + const neverMigrated = state.appliedMigrations.length === 0 && state.tableCount === 0; + return neverMigrated + ? new StartupRefusalError("schema-not-yet-migrated", message) + : new Error(message); +} + +/** + * Whether a startup failure should be captured to Sentry. Everything + * reports except a supervised-transient refusal in a managed-cloud + * deployment. + */ +export function shouldReportStartupFailure( + error: unknown, + env: NodeJS.ProcessEnv = process.env, +): boolean { + if (!(error instanceof StartupRefusalError)) return true; + const cloudOrigin = env.PAPERCLIP_CLOUD_API_ORIGIN?.trim(); + return !cloudOrigin; +}