From 6de50ba594b15efaa3eae6ed869cd39b3a436456 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Mon, 21 Sep 2026 16:40:46 -0700 Subject: [PATCH] fix(sentry): carry the deployment environment to the browser (#13784) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operators can enable Sentry for the server and the signed-in browser. > - The server SDK reads `SENTRY_ENVIRONMENT` from the process environment. > - The browser receives its DSN through the session response, but receives no environment. > - A browser in staging therefore reports errors under the SDK's production default. > - This pull request passes the configured environment through the existing session and monitoring gate. > - Browser errors then identify the deployment environment while preserving the existing privacy settings. ## Linked Issues or Issue Description **What happened?** With `SENTRY_ENVIRONMENT=staging`, browser exceptions are tagged `production`. This can send errors to the wrong environment's alerts and makes deployment follow-up unreliable. **Expected behavior** The browser uses the server's configured Sentry environment. A reused image works in either staging or production. A signed-out browser still sends no events. **Steps to reproduce** 1. Configure a frontend Sentry DSN and `SENTRY_ENVIRONMENT=staging`. 2. Sign in and capture a browser exception. 3. Inspect the event environment. Before this change, it is `production`. **Paperclip version or commit** Reproduced on `a3749aac4680a901fa0fe1cc898907887abc9908` with the real browser SDK and a local test transport. **Deployment mode** Authenticated server and browser with optional Sentry monitoring enabled. No duplicate environment-attribution issue or pull request was found in the targeted GitHub search. ## What Changed - Add `sentryEnvironment` to the authenticated session response and shared schema. The optional field supports a newer browser reading an older server response. - Pass the environment to the browser SDK. An environment change restarts the client through its existing serialized lifecycle. - Cover environment attribution with a real SDK event, session authorization, unchanged-session refetches, environment changes, and legacy responses. - Document configuration and compatibility. Keep the loaded bundle's release identity and existing privacy filters. ## Verification - The regression test emits `production` for a requested staging environment before the fix. - Focused route, schema, browser lifecycle and real-SDK tests: 69 pass. - UI and shared-package typechecks, direct server `tsc --noEmit`, and token gates pass. - Full `pnpm build` and `pnpm -r typecheck` were attempted. Both stop at the Runner Rust step because `cargo` is absent on this machine. - Complete UI suite: 6,540 tests pass in 626 files. - Full `pnpm test:run`: 8,210 passed, 14 failed, 4,753 skipped; 36 files fail due to embedded PostgreSQL startup/cleanup and macOS runtime-cache `EACCES`. These match the existing local baseline; none touch the changed behavior. - Greptile: 5/5, no unresolved review threads. Linux CI has passed Build, Typecheck + Release Registry, and the completed test jobs so far. Remaining jobs are running or queued: the AWS runner provisioner is retrying EC2 CreateFleet `InternalError` responses. Full results will be recorded before merge. ## Risks Low risk. This adds one optional session field and changes Sentry attribution only. No migration or new monitoring opt-in is introduced. Missing settings keep the browser SDK default. Agent and unauthenticated requests still receive 401 without monitoring settings. Existing loaded browser bundles keep their old behavior until refreshed. ## Model Used OpenAI GPT-6 via Codex, with reasoning, repository inspection, code editing, and test execution. The session does not expose an exact model snapshot or context-window size. ## 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 (focused and full UI suites pass; full-root environment failures documented above) - [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 - [ ] 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 --- doc/observability.md | 16 +++++++ packages/shared/src/validators/access.test.ts | 13 ++++++ packages/shared/src/validators/access.ts | 2 + server/src/__tests__/auth-routes.test.ts | 43 ++++++++++++++++++- server/src/routes/auth.ts | 2 + ui/src/components/SentryGate.test.tsx | 41 ++++++++++++++++-- ui/src/components/SentryGate.tsx | 5 ++- ui/src/lib/sentry.test.ts | 31 ++++++++++--- ui/src/lib/sentry.ts | 10 +++-- 9 files changed, 148 insertions(+), 15 deletions(-) diff --git a/doc/observability.md b/doc/observability.md index 501b095e44..15b0266ef9 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -317,6 +317,22 @@ event. These pages run signed out: session response arrives is not captured. The gate opens only after the session query resolves. +### Environment attribution + +Set `SENTRY_ENVIRONMENT` to the deployment environment, such as `staging` +or `production`. The server SDK reads this value from its process environment. +The authenticated session sends the same value in `sentryEnvironment`, and +`SentryGate` passes it to the browser SDK. This is runtime configuration, so the +same built image can report correctly in different environments. It does not +infer an environment from the page URL or include a tenant identifier. + +When the variable is absent or empty, the session sends `null` and the browser +keeps the SDK's default environment. The field is optional in the session +schema so a newer browser can still read a response from an older server. +A session refetch that changes the environment closes and restarts monitoring; +signing out still closes it. The browser release continues to identify the +loaded bundle, even if the server has since deployed another version. + ### Privacy settings The feature uses built-in Sentry options only. diff --git a/packages/shared/src/validators/access.test.ts b/packages/shared/src/validators/access.test.ts index 32ae79444f..e2d7ec3d99 100644 --- a/packages/shared/src/validators/access.test.ts +++ b/packages/shared/src/validators/access.test.ts @@ -160,6 +160,19 @@ describe("authSessionSchema", () => { expect(result.success && result.data.sentryDsn).toBe(null); }); + it.each([undefined, null, "staging", "production"])( + "preserves the optional Sentry environment (%s)", + (environment) => { + const result = authSessionSchema.parse({ + session: { id: "s1", userId: "u1" }, + user: { id: "u1", email: "a@b.com", name: "Jane", image: null }, + sentryDsn: null, + ...(environment === undefined ? {} : { sentryEnvironment: environment }), + }); + expect(result.sentryEnvironment).toBe(environment); + }, + ); + it("accepts a real sentryDsn value", () => { const result = authSessionSchema.safeParse({ session: { id: "s1", userId: "u1" }, diff --git a/packages/shared/src/validators/access.ts b/packages/shared/src/validators/access.ts index a5eddd48e5..c046b84d56 100644 --- a/packages/shared/src/validators/access.ts +++ b/packages/shared/src/validators/access.ts @@ -205,6 +205,8 @@ export const authSessionSchema = z.object({ // monitoring. The browser reads this value to open its own Sentry gate — // see `ui/src/lib/sentry.ts`. sentryDsn: z.string().min(1).nullable(), + // Optional for browser/server version skew; null leaves the SDK default. + sentryEnvironment: z.string().nullable().optional(), }); export type AuthSession = z.infer; diff --git a/server/src/__tests__/auth-routes.test.ts b/server/src/__tests__/auth-routes.test.ts index 3cb3cf6fb0..7b3588ae07 100644 --- a/server/src/__tests__/auth-routes.test.ts +++ b/server/src/__tests__/auth-routes.test.ts @@ -1,6 +1,6 @@ import express from "express"; import request from "supertest"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { errorHandler } from "../middleware/index.js"; import { authRoutes } from "../routes/auth.js"; @@ -63,6 +63,7 @@ describe.sequential("auth routes", () => { const originalSentryDsnBackend = process.env.SENTRY_DSN_BACKEND; afterEach(() => { + vi.unstubAllEnvs(); if (originalSentryDsn === undefined) delete process.env.SENTRY_DSN; else process.env.SENTRY_DSN = originalSentryDsn; if (originalSentryDsnFrontend === undefined) delete process.env.SENTRY_DSN_FRONTEND; @@ -72,6 +73,7 @@ describe.sequential("auth routes", () => { }); it("returns the persisted user profile in the session payload", async () => { + vi.stubEnv("SENTRY_ENVIRONMENT", undefined); delete process.env.SENTRY_DSN; const app = await createApp( { @@ -92,6 +94,7 @@ describe.sequential("auth routes", () => { }, user: baseUser, sentryDsn: null, + sentryEnvironment: null, }); }); @@ -203,6 +206,44 @@ describe.sequential("auth routes", () => { expect(res.body.sentryDsn).toBeUndefined(); }); + it.each(["staging", "production", "preview"])( + "sends the configured Sentry environment %s to board actors", + async (environment) => { + vi.stubEnv("SENTRY_ENVIRONMENT", environment); + const app = createApp({ type: "board", userId: "user-1", source: "session" }, baseUser); + + const res = await request(app).get("/api/auth/get-session"); + + expect(res.status).toBe(200); + expect(res.body.sentryEnvironment).toBe(environment); + }, + ); + + it.each([undefined, ""])("sends null for an unset Sentry environment (%s)", async (environment) => { + vi.stubEnv("SENTRY_ENVIRONMENT", environment); + const app = createApp({ type: "board", userId: "user-1", source: "session" }, baseUser); + + const res = await request(app).get("/api/auth/get-session"); + + expect(res.status).toBe(200); + expect(res.body.sentryEnvironment).toBeNull(); + }); + + it.each([ + { type: "none", source: "none" }, + { type: "agent", agentId: "agent-1", companyId: "company-1", source: "agent_key" }, + ] satisfies Express.Request["actor"][])("withholds Sentry settings from a $type actor", async (actor) => { + vi.stubEnv("SENTRY_ENVIRONMENT", "staging"); + vi.stubEnv("SENTRY_DSN_FRONTEND", "https://public@o0.ingest.sentry.io/1"); + const app = createApp(actor, baseUser); + + const res = await request(app).get("/api/auth/get-session"); + + expect(res.status).toBe(401); + expect(res.body.sentryDsn).toBeUndefined(); + expect(res.body.sentryEnvironment).toBeUndefined(); + }); + it("updates the signed-in profile", async () => { const app = await createApp( { diff --git a/server/src/routes/auth.ts b/server/src/routes/auth.ts index 6455636f82..cfa65e033c 100644 --- a/server/src/routes/auth.ts +++ b/server/src/routes/auth.ts @@ -55,6 +55,8 @@ export function authRoutes(db: Db) { // handler, so no second authorization check runs here. This field // carries the front-end DSN only; it never carries the backend DSN. sentryDsn: resolveSentryDsns().frontend, + // Match the server SDK's runtime environment, including in reused images. + sentryEnvironment: process.env.SENTRY_ENVIRONMENT || null, })); }); diff --git a/ui/src/components/SentryGate.test.tsx b/ui/src/components/SentryGate.test.tsx index 30702bdba0..e10ad420ee 100644 --- a/ui/src/components/SentryGate.test.tsx +++ b/ui/src/components/SentryGate.test.tsx @@ -8,7 +8,7 @@ import { queryKeys } from "@/lib/queryKeys"; import { SentryGate } from "./SentryGate"; const getSessionMock = vi.hoisted(() => vi.fn()); -const initBrowserErrorMonitoringMock = vi.hoisted(() => vi.fn(async (_dsn: string) => {})); +const initBrowserErrorMonitoringMock = vi.hoisted(() => vi.fn(async (_dsn: string, _environment?: string) => {})); const teardownBrowserErrorMonitoringMock = vi.hoisted(() => vi.fn(async () => {})); vi.mock("@/api/auth", () => ({ @@ -16,7 +16,7 @@ vi.mock("@/api/auth", () => ({ })); vi.mock("@/lib/sentry", () => ({ - initBrowserErrorMonitoring: (dsn: string) => initBrowserErrorMonitoringMock(dsn), + initBrowserErrorMonitoring: (dsn: string, environment?: string) => initBrowserErrorMonitoringMock(dsn, environment), teardownBrowserErrorMonitoring: () => teardownBrowserErrorMonitoringMock(), })); @@ -93,7 +93,7 @@ describe("SentryGate", () => { const root = await renderGate(); expect(initBrowserErrorMonitoringMock).toHaveBeenCalledTimes(1); - expect(initBrowserErrorMonitoringMock).toHaveBeenCalledWith("https://public@o0.ingest.sentry.io/1"); + expect(initBrowserErrorMonitoringMock).toHaveBeenCalledWith("https://public@o0.ingest.sentry.io/1", undefined); root.unmount(); }); @@ -113,10 +113,43 @@ describe("SentryGate", () => { await flushReact(); expect(initBrowserErrorMonitoringMock).toHaveBeenCalledTimes(1); - expect(initBrowserErrorMonitoringMock).toHaveBeenCalledWith("https://public@o0.ingest.sentry.io/1"); + expect(initBrowserErrorMonitoringMock).toHaveBeenCalledWith("https://public@o0.ingest.sentry.io/1", undefined); root.unmount(); }); + it("restarts monitoring when the session environment changes with the same DSN", async () => { + const session = { + session: { id: "s1", userId: "u1" }, + user: { id: "u1", email: "a@b.com", name: "Jane", image: null }, + sentryDsn: "https://public@o0.ingest.sentry.io/1", + sentryEnvironment: "staging", + }; + getSessionMock.mockResolvedValue(session); + const root = await renderGate(); + try { + expect(initBrowserErrorMonitoringMock).toHaveBeenLastCalledWith(session.sentryDsn, "staging"); + await act(async () => { + await queryClient.refetchQueries({ queryKey: queryKeys.auth.session }); + }); + await flushReact(); + expect(initBrowserErrorMonitoringMock).toHaveBeenCalledTimes(1); + expect(teardownBrowserErrorMonitoringMock).not.toHaveBeenCalled(); + + getSessionMock.mockResolvedValue({ ...session, sentryEnvironment: "production" }); + await act(async () => { + await queryClient.refetchQueries({ queryKey: queryKeys.auth.session }); + }); + await flushReact(); + expect(teardownBrowserErrorMonitoringMock).toHaveBeenCalledTimes(1); + expect(initBrowserErrorMonitoringMock).toHaveBeenCalledTimes(2); + expect(initBrowserErrorMonitoringMock).toHaveBeenLastCalledWith(session.sentryDsn, "production"); + expect(teardownBrowserErrorMonitoringMock.mock.invocationCallOrder[0]) + .toBeLessThan(initBrowserErrorMonitoringMock.mock.invocationCallOrder[1]); + } finally { + root.unmount(); + } + }); + it("closes browser monitoring when sign-out clears the session's DSN", async () => { getSessionMock.mockResolvedValue({ session: { id: "s1", userId: "u1" }, diff --git a/ui/src/components/SentryGate.tsx b/ui/src/components/SentryGate.tsx index 0fa0d80e1f..3d99695cc1 100644 --- a/ui/src/components/SentryGate.tsx +++ b/ui/src/components/SentryGate.tsx @@ -26,14 +26,15 @@ export function SentryGate() { }); const dsn = session?.sentryDsn; + const environment = session?.sentryEnvironment ?? undefined; useEffect(() => { if (!dsn) return; - void initBrowserErrorMonitoring(dsn); + void initBrowserErrorMonitoring(dsn, environment); return () => { void teardownBrowserErrorMonitoring(); }; - }, [dsn]); + }, [dsn, environment]); return null; } diff --git a/ui/src/lib/sentry.test.ts b/ui/src/lib/sentry.test.ts index e1612a197e..a887cff096 100644 --- a/ui/src/lib/sentry.test.ts +++ b/ui/src/lib/sentry.test.ts @@ -111,11 +111,10 @@ describe("initBrowserErrorMonitoring", () => { const mocks = mockSentryPackage(); const { initBrowserErrorMonitoring } = await importFreshSentry(); - await initBrowserErrorMonitoring(DSN); + await initBrowserErrorMonitoring(DSN, "staging"); expect(mocks.init).toHaveBeenCalledTimes(1); - const initOptions = mocks.init.mock.calls[0][0] as { dsn: string }; - expect(initOptions.dsn).toBe(DSN); + expect(mocks.init.mock.calls[0][0]).toMatchObject({ dsn: DSN, environment: "staging" }); }); it("a second call starts no second client", async () => { @@ -376,11 +375,14 @@ describe("captured event shape against the real @sentry/browser SDK", () => { * adds no `beforeSend` of its own (see the "holds no beforeSend hook" * test above). */ - async function initRealSentryForTest(onEvent: (event: Record) => void) { + async function initRealSentryForTest( + onEvent: (event: Record) => void, + environment?: string | null, + ) { const { buildBrowserSentryInitOptions } = await importFreshSentry(); const Sentry = await import("@sentry/browser"); Sentry.init({ - ...buildBrowserSentryInitOptions(DSN), + ...buildBrowserSentryInitOptions(DSN, environment), transport: () => ({ send: async () => ({}), flush: async () => true }), beforeSend: (event) => { onEvent(event as unknown as Record); @@ -390,6 +392,25 @@ describe("captured event shape against the real @sentry/browser SDK", () => { return Sentry; } + it.each([ + ["staging", "staging"], + ["production", "production"], + [null, "production"], + [undefined, "production"], + ])("emits environment %s as %s without page context", async (environment, expected) => { + let captured: Record | null = null; + const Sentry = await initRealSentryForTest((event) => { captured = event; }, environment); + try { + Sentry.captureException(new Error("environment attribution check")); + await Sentry.flush(2000); + expect(captured).toMatchObject({ environment: expected }); + expect(captured).not.toHaveProperty("request"); + expect((captured as unknown as Record).breadcrumbs).toBeUndefined(); + } finally { + await Sentry.close(); + } + }); + it("attaches the bundle release to an emitted event without page context", async () => { const commit = "0123456789abcdef0123456789abcdef01234567"; vi.stubGlobal("__PAPERCLIP_BUILD_COMMIT__", commit); diff --git a/ui/src/lib/sentry.ts b/ui/src/lib/sentry.ts index bb433c5d2f..0c3a8e6c63 100644 --- a/ui/src/lib/sentry.ts +++ b/ui/src/lib/sentry.ts @@ -65,12 +65,12 @@ let sentry: SentryBrowserModule | null = null; * — the session query can refetch and call this again, and a second call is * a no-op because a client is already started. */ -export function initBrowserErrorMonitoring(dsn: string): Promise { +export function initBrowserErrorMonitoring(dsn: string, environment?: string | null): Promise { return enqueue(async () => { if (sentry) return; try { const Sentry = await import("@sentry/browser"); - Sentry.init(buildBrowserSentryInitOptions(dsn)); + Sentry.init(buildBrowserSentryInitOptions(dsn, environment)); sentry = Sentry; } catch (err) { // The dynamic import or the init call failed. Fall through with a @@ -152,9 +152,13 @@ export function captureBrowserException(error: unknown): void { * `@sentry/browser` module and assert the resolved integration list and the * captured-event shape against the true SDK, not a stand-in. */ -export function buildBrowserSentryInitOptions(dsn: string): BrowserSentryInitOptions { +export function buildBrowserSentryInitOptions( + dsn: string, + environment?: string | null, +): BrowserSentryInitOptions { return { dsn, + environment: environment ?? undefined, // Use the loaded bundle's build, even when the server has since deployed. release: typeof __PAPERCLIP_BUILD_COMMIT__ === "string"