From 8aa28940bfaee1165065be7d2285fb150720af2b Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 5 Nov 2026 12:04:33 -0800 Subject: [PATCH] fix(sentry): keep a live-heartbeat native capacity retry out of Sentry (#19320) 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, and it keeps known, recovered failures local > - A native Codex "model at capacity" failure gets an automatic retry. That failure must stay out of Sentry while its retry owns recovery (#18295, #19051) > - The check reads the `nativeProviderFailure` diagnostic from the failed run's `resultJson`. On the live heartbeat path, the failed-status write replaces `resultJson` with the adapter's copy, and that copy has no diagnostic > - So every live-path capacity failure with a scheduled retry still goes to Sentry. Only the reconciliation path, which the tests cover, kept the failure local > - This pull request removes the diagnostic check. The scheduled retry run is the proof > - The benefit is that recovered capacity failures no longer create Sentry noise on the normal live path ## Linked Issues or Issue Description Refs #18295 Refs #19051 **What happened?** A native Codex run failed with `native_provider_overloaded`. The committed status decision scheduled the automatic capacity retry, and the retry run existed before the report. The run failure still went to Sentry. In a staging sample, every live-path capacity failure since #19051 was reported (1 of 1). **Expected behavior** When the `native_provider_overloaded` retry run exists for the same task, the failed run does not go to Sentry. The retry reports its own outcome. **Steps to reproduce** 1. Run a native Codex agent through the live heartbeat path (`finalizeNativeRun` without `projectRunStatus`). 2. Let the provider return `serverOverloaded`. The status decision schedules the capacity retry. 3. The heartbeat writes the terminal `failed` status with the adapter's `resultJson`, and `emitTerminalAgentTaskRun` calls `reportRunFailure` with that row. 4. The row has no `nativeProviderFailure`, so `isScheduledNativeCapacityRetry` returns false and the event goes to Sentry. **Relevant logs or output** The failed run row later shows `nativeProviderFailure: codex/server_overloaded`, because a later reconciliation write restores it. The row that the heartbeat write reports does not have the field. The same overwrite also explains the missing `finalizationReasonCode` that #19051 fixed. ## What Changed - `server/src/services/run-failure-report.ts`: `isScheduledNativeCapacityRetry` no longer requires `resultJson.nativeProviderFailure`. The run must still be native and failed, carry `native_provider_overloaded`, have a native task, have no signal, and not be in setup. The same-task successor with `scheduledRetryReason: native_provider_overloaded` is still required. - The successor is enough proof. Only the committed capacity status decision creates that successor, and only `forTerminal` in `native-provider-failure.ts` produces `native_provider_overloaded` together with the typed `codex`/`server_overloaded` diagnostic. - `run-failure-report.test.ts`: a new case keeps the failure local when the row has only the adapter's `resultJson` (no diagnostic, no finalization fields) and the successor exists. The "diagnostic is missing" case now reports only when no retry run exists. The case for a non-capacity diagnostic with a capacity successor is removed, because that state cannot occur. - `doc/observability.md`: the native capacity paragraph now says that the successor is the proof and that the run's own `resultJson` is not required. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/services/__tests__/run-failure-report.test.ts`: 284/284 pass with this change. - With the `master` version of `run-failure-report.ts`, the new case fails: 1 failed, 283 passed. So the test catches the bug. - `native-provider-failure.test.ts`: 57/57 pass. - `native-provider-overload.integration.test.ts` did not load in my sandbox, because `@paperclipai/plugin-sdk` was not built. CI must run it. ## Risks - Low risk. The change only removes one condition from a Sentry filter. - These failures still report: no retry run, exhausted or held retries, other retry lanes, a successor for a different task, legacy runs, signals, timeouts, setup failures, and unreadable retry evidence. - The change does not affect run status, retries, recovery, or the `all` reporting mode. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), agentic coding with tool use (shell, repository reads, test runs). ## 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: Claude Opus 5.5 --- doc/observability.md | 11 ++++++----- .../services/__tests__/run-failure-report.test.ts | 9 +++++---- server/src/services/run-failure-report.ts | 12 +++++------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/doc/observability.md b/doc/observability.md index 00d8ea08c0e..e9f5e842ce9 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -1134,11 +1134,12 @@ untyped capacity text, and Codex CLI transient upstream failures remain reportab The run keeps its failed state and its terminal diagnostic. A native Codex model capacity failure stays local only while its automatic -retry owns recovery. The run must carry `native_provider_overloaded` and the -closed `codex`/`server_overloaded` diagnostic. The proof of the retry is the -`native_provider_overloaded` successor run for the same task, which only the -committed capacity status decision creates. The run's own finalization fields -are not required: a late terminal write can replace them with an earlier result. +retry owns recovery. The run must carry `native_provider_overloaded`. The proof +of the retry is the `native_provider_overloaded` successor run for the same +task, which only the committed capacity status decision creates, and only for +the typed `codex`/`server_overloaded` diagnostic. The run's own `resultJson` is +not required: the live heartbeat's failed-status write replaces it with the +adapter's copy, which lacks the finalizer's diagnostic and finalization fields. Exhausted or held capacity failures, other retry lanes, setup failures, signals, and unreadable retry evidence remain reportable. A failed retry reports its own outcome. diff --git a/server/src/services/__tests__/run-failure-report.test.ts b/server/src/services/__tests__/run-failure-report.test.ts index 04a41cb627d..c533e59fee9 100644 --- a/server/src/services/__tests__/run-failure-report.test.ts +++ b/server/src/services/__tests__/run-failure-report.test.ts @@ -470,9 +470,11 @@ describeEmbeddedPostgres("reportRunFailure", () => { it.each([ ["no finalization fields", { finalizationPhase: undefined, finalizationReasonCode: undefined }], ["an earlier finalization phase", { finalizationPhase: "retryable_failure", finalizationReasonCode: undefined }], + ["only the adapter's resultJson", { nativeProviderFailure: undefined, finalizationPhase: undefined, finalizationReasonCode: undefined }], ] as const)("keeps a scheduled native capacity retry local when the run row has %s", async (_label, resultJson) => { - // A late terminal write can replace the finalizer's fields with an earlier - // resultJson. The successor created by the committed decision still proves it. + // The live heartbeat's failed-status write replaces the finalizer's + // resultJson with the adapter's copy. The successor created by the + // committed decision still proves the scheduled retry. const run = await nativeCapacityFailure({ resultJson }); await reportRunFailure(db, run); expect(mockCaptureRunFailure).not.toHaveBeenCalled(); @@ -486,8 +488,7 @@ describeEmbeddedPostgres("reportRunFailure", () => { ["the successor belongs to another task", { retry: { contextSnapshot: { issueId: randomUUID() } } }], ["the run has no native task", { run: { nativeIssueId: null } }], ["finalization has no fields and no retry run exists", { resultJson: { finalizationPhase: undefined, finalizationReasonCode: undefined }, retry: null }], - ["the diagnostic is not typed capacity evidence", { resultJson: { nativeProviderFailure: { provider: "codex", category: "turn_failed" } } }], - ["the diagnostic is missing", { resultJson: { nativeProviderFailure: undefined } }], + ["the diagnostic is missing and no retry run exists", { resultJson: { nativeProviderFailure: undefined }, retry: null }], ["the run is legacy", { run: { runtimeMode: "legacy" } }], ["the run has a signal", { run: { signal: "SIGTERM" } }], ["the run timed out", { run: { status: "timed_out" } }], diff --git a/server/src/services/run-failure-report.ts b/server/src/services/run-failure-report.ts index c9162ba08bb..fcf4baee0a4 100644 --- a/server/src/services/run-failure-report.ts +++ b/server/src/services/run-failure-report.ts @@ -15,7 +15,7 @@ import { readConnectionFailure, type ConnectionFailure } from "@paperclipai/adap import { isAiConnectionConfigurationReason } from "./ai-connection-configuration-failure.js"; import { readRunFailureReportingMode, type RunFailureReportingMode } from "../sentry-run-failure-policy.js"; import { RESTRICTED_WORKSPACE_SOURCE_REASON } from "./workspace-source-restriction.js"; -import { NATIVE_PROVIDER_OVERLOADED_CODE, NATIVE_PROVIDER_OVERLOADED_DIAGNOSTIC } from "./native-runtime/native-provider-failure.js"; +import { NATIVE_PROVIDER_OVERLOADED_CODE } from "./native-runtime/native-provider-failure.js"; type HeartbeatRun = typeof heartbeatRuns.$inferSelect; @@ -240,19 +240,17 @@ function isCodexAcpModelCapacityFailure(run: HeartbeatRun, options: RunFailureRe /** * A Codex `serverOverloaded` terminal chose the bounded capacity retry. Only * that committed status decision creates a successor with this retry reason, - * so the successor row is the proof. The run's own finalization fields are - * not: a late terminal write can replace them with an earlier `resultJson`. + * so the successor row is the proof. The run's own `resultJson` is not: the + * live heartbeat's failed-status write replaces it with the adapter's copy, + * which lacks the finalizer's diagnostic and finalization fields. * The retry owns recovery; a later failure reports itself. Held and exhausted * capacity failures have no automatic retry and stay reportable. The retry * carries the task in its context, so the company/context-issue index bounds * the read to that task's runs created since this one. */ async function isScheduledNativeCapacityRetry(db: Db, run: HeartbeatRun, options: RunFailureReportOptions): Promise { - const diagnostic = asRecord(asRecord(run.resultJson)?.nativeProviderFailure); if (run.status !== "failed" || run.runtimeMode !== "native" || run.errorCode !== NATIVE_PROVIDER_OVERLOADED_CODE || - run.nativeIssueId == null || options.phase === "setup" || run.signal != null || - diagnostic?.provider !== NATIVE_PROVIDER_OVERLOADED_DIAGNOSTIC.provider || - diagnostic.category !== NATIVE_PROVIDER_OVERLOADED_DIAGNOSTIC.category) return false; + run.nativeIssueId == null || options.phase === "setup" || run.signal != null) return false; const [retry] = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and( eq(heartbeatRuns.companyId, run.companyId), sql`${heartbeatRuns.contextSnapshot} ->> 'issueId' = ${run.nativeIssueId}`, gte(heartbeatRuns.createdAt, run.createdAt),