From 75fcb0b3fca427774cff8f4f7ed43de9ab7f626b Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 23 Oct 2026 14:07:19 -0700 Subject: [PATCH] fix(sentry): keep typed ACP provider access failures local (#17366) 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 sends unexpected run failures to Sentry through `reportRunFailure` > - A Claude ACP run can fail because the provider rejects the stored credential ("Invalid API key"). The ACP agent then sends a typed `access` terminal failure. > - The Claude adapter already classifies this failure as `claude_auth_required`. The run is blocked, and the responsible user gets a reconnect request. > - Sentry also records each of these runs as "ACP agent reported a terminal access failure.". This duplicates a signal that the user already gets, and it hides real defects. > - This pull request adds one narrow exclusion for this proven provider auth state, next to the existing quota and Codex capacity exclusions. > - The benefit is less Sentry noise, and all other access-shaped failures stay visible. ## Linked Issues or Issue Description Refs #14886 (related: it routes ACP access failures in recovery, not in Sentry reporting). **What happened?** Staging Sentry recorded a run failure "ACP agent reported a terminal access failure." for 8 events. All events had `agent_adapter=claude_local`, `error_code=claude_auth_required`, and a typed provider access failure ("Invalid API key · Fix external API key"). The failures stopped when the provider accepted the credential again. **Expected behavior** A provider auth failure that the adapter types and classifies is a user action state. The product already shows it to the user. It must not go to Sentry in `unexpected` mode. **Steps to reproduce** 1. Configure a `claude_local` agent with an ACP runtime and a credential that the provider rejects. 2. Start a run. 3. The ACP agent ends the turn with `category: "access"`. The run fails with `claude_auth_required`. 4. See a new Sentry run-failure event. **Agent adapter(s) involved** Claude Code (`claude_local`, ACP lane). ## What Changed - `server/src/services/run-failure-report.ts`: add `isAcpProviderAccessFailure`. It skips a report only when all of these are true: the reporting mode is `unexpected`, the run status is `failed`, the error code is `claude_auth_required` or `acpx_auth_required`, `resultJson.terminalSessionFailure.category` is `access`, the phase is not `setup`, and the run has no signal. - `server/src/services/__tests__/run-failure-report.test.ts`: add regression tests for the staging shape, eight near-miss shapes that must still report, and the setup and `all` mode paths. ## Verification - Red: without the predicate, the two new "keeps ... local" tests fail (`2 failed | 266 passed`). - Green: `cd server && pnpm exec vitest run src/services/__tests__/run-failure-report.test.ts src/services/__tests__/run-failure-diagnostics.test.ts` gives `383 passed`. - `pnpm exec tsc --noEmit -p server` shows no errors in the changed files. ## Risks - Low risk. The change only removes Sentry events. It does not change run status, recovery, or user notices. - A real defect that the ACP agent wrongly types as `access` with a Claude auth code is not reported in `unexpected` mode. Set `PAPERCLIP_SENTRY_RUN_FAILURES=all` to see these events again. - Codex and other ACP adapters do not map `access` to an auth code, so their access failures still report. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), run through Claude Code with tool use (shell, file edit, Sentry and GitHub connectors). ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Devin Foley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- .../__tests__/run-failure-report.test.ts | 36 +++++++++++++++++++ server/src/services/run-failure-report.ts | 14 ++++++++ 2 files changed, 50 insertions(+) diff --git a/server/src/services/__tests__/run-failure-report.test.ts b/server/src/services/__tests__/run-failure-report.test.ts index f2c833557d1..2ce0d6891fe 100644 --- a/server/src/services/__tests__/run-failure-report.test.ts +++ b/server/src/services/__tests__/run-failure-report.test.ts @@ -430,6 +430,42 @@ describeEmbeddedPostgres("reportRunFailure", () => { expect(mockCaptureRunFailure).toHaveBeenCalledTimes(2); }); + function providerAccessRun(overrides: Partial = {}) { + return buildRun({ + error: "ACP agent reported a terminal access failure.\nInvalid API key · Fix external API key", + errorCode: "claude_auth_required", executionStage: "dispatching", exitCode: 1, signal: null, + resultJson: { + stopReason: "adapter_failed", + terminalSessionFailure: { category: "access", title: "Invalid API key · Fix external API key" }, + }, + ...overrides, + }); + } + + it.each(["claude_auth_required", "acpx_auth_required"])("keeps a typed ACP provider access failure (%s) local", async (errorCode) => { + await seedCompanyAndAgent({ adapterType: "claude_local" }); + for (const phase of [undefined, "execute"] as const) await reportRunFailure(db, providerAccessRun({ errorCode }), { phase }); + expect(mockCaptureRunFailure).not.toHaveBeenCalled(); + }); + + it.each([ + { status: "timed_out" }, { signal: "SIGTERM" }, { errorCode: "acpx_turn_failed" }, { errorCode: "adapter_failed" }, + { resultJson: {} }, { resultJson: null }, { resultJson: { terminalSessionFailure: { category: "service" } } }, + { resultJson: { terminalSessionFailure: "access" } }, + ])("reports a provider auth outcome without typed ACP access evidence: %j", async (overrides) => { + await seedCompanyAndAgent({ adapterType: "claude_local" }); + await reportRunFailure(db, providerAccessRun(overrides), { phase: "execute" }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + }); + + it("reports a typed ACP provider access failure during setup or in all mode", async () => { + await seedCompanyAndAgent({ adapterType: "claude_local" }); + await reportRunFailure(db, providerAccessRun(), { phase: "setup" }); + vi.stubEnv("PAPERCLIP_SENTRY_RUN_FAILURES", "all"); + await reportRunFailure(db, providerAccessRun(), { phase: "execute" }); + expect(mockCaptureRunFailure).toHaveBeenCalledTimes(2); + }); + function restartInterruptedSetupRun(overrides: Partial = {}) { return buildRun({ error: "Process lost -- server may have restarted; retrying once", diff --git a/server/src/services/run-failure-report.ts b/server/src/services/run-failure-report.ts index 46f3c090d5c..c912b19a0d3 100644 --- a/server/src/services/run-failure-report.ts +++ b/server/src/services/run-failure-report.ts @@ -236,6 +236,19 @@ function isCodexAcpModelCapacityFailure(run: HeartbeatRun, options: RunFailureRe options.phase !== "setup" && run.signal == null; } +const providerAuthErrorCodes = new Set(["claude_auth_required", "acpx_auth_required"]); + +/** + * The ACP agent typed its terminal failure as `access` (provider auth_required), + * and the adapter kept it on the sign-in recovery path. The run blocks and the + * user gets a reconnect request, so Sentry adds no signal. + */ +function isAcpProviderAccessFailure(run: HeartbeatRun, options: RunFailureReportOptions): boolean { + return run.status === "failed" && run.errorCode != null && providerAuthErrorCodes.has(run.errorCode) && + asRecord(asRecord(run.resultJson)?.terminalSessionFailure)?.category === "access" && + options.phase !== "setup" && run.signal == null; +} + /** * The Grok endpoint refused every work wake with an HTTP 4xx, so the offer * expired unaccepted. Durable delivery receipts prove the external cause. A @@ -284,6 +297,7 @@ export function reportRunFailure(db: Db, run: HeartbeatRun, options: RunFailureR isGitConnectionFailure(run, options) || isProviderQuotaFailure(run, options) || isCodexAcpModelCapacityFailure(run, options) || + isAcpProviderAccessFailure(run, options) || isRestartInterruptedSetup(run) )) return Promise.resolve(); const runStatus = run.status;