From 91b641757beeb2782df9c5a83edf0e371b4ddea3 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 6 Nov 2026 12:04:51 -0800 Subject: [PATCH] fix(sentry): keep low-trust project denials on the task (#19531) Keep exact pre-dispatch project access denials on their task instead of the unexpected Sentry failure stream. Validation: 301 focused tests and all applicable CI pass. Greptile scored the exact head 5/5, and no review threads remain unresolved. Co-Authored-By: Paperclip --- doc/LOW-TRUST-PRESETS.md | 8 ++ .../run-failure-report-project-access.test.ts | 128 ++++++++++++++++++ server/src/services/run-failure-report.ts | 14 ++ 3 files changed, 150 insertions(+) create mode 100644 server/src/services/__tests__/run-failure-report-project-access.test.ts diff --git a/doc/LOW-TRUST-PRESETS.md b/doc/LOW-TRUST-PRESETS.md index 6dc5a9f2088..9be0a427785 100644 --- a/doc/LOW-TRUST-PRESETS.md +++ b/doc/LOW-TRUST-PRESETS.md @@ -60,6 +60,14 @@ run lacks direct-chat authority or the user lacks editing permission. The user can request the edit in their authorized chat or apply it directly. Creating another task does not authorize the instruction change. +When a low-trust agent's task is in a project outside its boundary, setup fails +the run with the typed `project_access_denied` code and the denied project ID +before provider work starts. The task notice links Manage project access. That +exact pre-dispatch shape is an owner action, so it stays on the task instead of +the unexpected Sentry run-failure stream. The legacy `setup_failed` code and the +denial message alone never suppress a report, and +`PAPERCLIP_SENTRY_RUN_FAILURES=all` still reports it. + ## Direct-chat instruction edits When an authorized user directly asks an agent in their own Agent Chat to edit diff --git a/server/src/services/__tests__/run-failure-report-project-access.test.ts b/server/src/services/__tests__/run-failure-report-project-access.test.ts new file mode 100644 index 00000000000..2553cf7d1b4 --- /dev/null +++ b/server/src/services/__tests__/run-failure-report-project-access.test.ts @@ -0,0 +1,128 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { agents, applyPendingMigrations, closeRegisteredClients, companies, createDb, heartbeatRuns } from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "../../__tests__/helpers/embedded-postgres.js"; + +const mockCaptureRunFailure = vi.hoisted(() => vi.fn()); + +vi.mock("../../sentry.js", () => ({ + captureRunFailure: mockCaptureRunFailure, +})); + +import { reportRunFailure } from "../run-failure-report.js"; +import { PROJECT_ACCESS_DENIED } from "../project-access-denial.js"; + +const testDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL; +const embeddedPostgresSupport = testDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +describeEmbeddedPostgres("reportRunFailure low-trust project denials", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + let companyId!: string; + let agentId!: string; + let inheritedEnv: NodeJS.ProcessEnv; + + beforeAll(async () => { + if (testDatabaseUrl) { + await applyPendingMigrations(testDatabaseUrl); + db = createDb(testDatabaseUrl); + } else { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-run-failure-project-access-"); + db = createDb(tempDb.connectionString); + } + }, 20_000); + + beforeEach(async () => { + inheritedEnv = process.env; + process.env = Object.fromEntries( + ["PATH", "HOME", "USER", "USERNAME", "LOGNAME", "USERPROFILE", "TMPDIR", "TEMP", "TMP"] + .flatMap((key) => inheritedEnv[key] === undefined ? [] : [[key, inheritedEnv[key]]]), + ); + process.env.PAPERCLIP_SENTRY_RUN_FAILURES = "unexpected"; + companyId = randomUUID(); + agentId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + defaultResponsibleUserId: "responsible-user", + }); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "OpenCodeCoder", + role: "engineer", + status: "active", + adapterType: "opencode_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + }); + + afterEach(async () => { + vi.clearAllMocks(); + vi.unstubAllEnvs(); + process.env = inheritedEnv; + await db.delete(heartbeatRuns); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + if (testDatabaseUrl) await closeRegisteredClients(testDatabaseUrl); + await tempDb?.cleanup(); + }); + + function projectAccessDeniedRun( + overrides: Partial = {}, + ): typeof heartbeatRuns.$inferSelect { + return { + id: randomUUID(), + companyId, + agentId, + status: "failed", + error: "Project is outside this low-trust boundary.", + errorCode: PROJECT_ACCESS_DENIED, + executionStage: "preparing", + nativeIssueId: randomUUID(), + contextSnapshot: null, + resultJson: { + projectAccessDenied: { projectId: randomUUID() }, + executionRecovery: { kind: "bootstrap", providerWorkStarted: false }, + }, + ...overrides, + } as unknown as typeof heartbeatRuns.$inferSelect; + } + + it("keeps a low-trust project denial before dispatch on the task", async () => { + await reportRunFailure(db, projectAccessDeniedRun(), { phase: "setup" }); + expect(mockCaptureRunFailure).not.toHaveBeenCalled(); + }); + + it.each([ + { status: "timed_out" }, { errorCode: "setup_failed" }, { executionStage: "executing" }, { exitCode: 1 }, { signal: "SIGTERM" }, + { resultJson: { projectAccessDenied: { projectId: randomUUID() } } }, + { resultJson: { projectAccessDenied: { projectId: randomUUID() }, executionRecovery: { kind: "bootstrap", providerWorkStarted: true } } }, + { resultJson: { projectAccessDenied: {}, executionRecovery: { kind: "bootstrap", providerWorkStarted: false } } }, + ])("reports project denial markers without an exact pre-dispatch outcome: %j", async (overrides) => { + await reportRunFailure(db, projectAccessDeniedRun(overrides), { phase: "setup" }); + expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1); + }); + + it.each([undefined, "execute"] as const)("reports project denial markers outside the setup report: %s", async (phase) => { + await reportRunFailure(db, projectAccessDeniedRun(), { phase }); + expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1); + }); + + it("reports project denials when every run failure is reported", async () => { + vi.stubEnv("PAPERCLIP_SENTRY_RUN_FAILURES", "all"); + await reportRunFailure(db, projectAccessDeniedRun(), { phase: "setup" }); + expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1); + }); +}); diff --git a/server/src/services/run-failure-report.ts b/server/src/services/run-failure-report.ts index dc210833b55..0d5dd0bd487 100644 --- a/server/src/services/run-failure-report.ts +++ b/server/src/services/run-failure-report.ts @@ -17,6 +17,7 @@ import { readRunFailureReportingMode, type RunFailureReportingMode } from "../se import { RESTRICTED_WORKSPACE_SOURCE_REASON } from "./workspace-source-restriction.js"; import { PLUGIN_RUNNER_AI_CONNECTION_REQUIRED } from "./plugin-runner-ai-connection.js"; import { NATIVE_PROVIDER_OVERLOADED_CODE } from "./native-runtime/native-provider-failure.js"; +import { PROJECT_ACCESS_DENIED } from "./project-access-denial.js"; type HeartbeatRun = typeof heartbeatRuns.$inferSelect; @@ -173,6 +174,18 @@ function isPluginRunnerAiConnectionRequiredBlocker(run: HeartbeatRun, options: R recovery?.kind === "bootstrap" && recovery.providerWorkStarted === false; } +function isProjectAccessDenialBlocker(run: HeartbeatRun, options: RunFailureReportOptions): boolean { + if ( + run.status !== "failed" || run.errorCode !== PROJECT_ACCESS_DENIED || + run.executionStage !== "preparing" || options.phase !== "setup" || + run.exitCode != null || run.signal != null + ) return false; + const denial = asRecord(run.resultJson?.projectAccessDenied); + const recovery = asRecord(run.resultJson?.executionRecovery); + return typeof denial?.projectId === "string" && + recovery?.kind === "bootstrap" && recovery.providerWorkStarted === false; +} + /** Only the managed-clone producer can attach this bounded pre-dispatch evidence. */ function isGitConnectionFailure(run: HeartbeatRun, options: RunFailureReportOptions): boolean { const connection = readConnectionFailure(run.resultJson?.connectionFailure); @@ -333,6 +346,7 @@ export function reportRunFailure(db: Db, run: HeartbeatRun, options: RunFailureR isLocalPathWorkspaceConfigurationBlocker(run, options) || isRestrictedWorkspaceSourceBlocker(run, options) || isPluginRunnerAiConnectionRequiredBlocker(run, options) || + isProjectAccessDenialBlocker(run, options) || isGitConnectionFailure(run, options) || isProviderQuotaFailure(run, options) || isCodexAcpModelCapacityFailure(run, options) ||