mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 12:33:05 +01:00
fix(sentry): prove a native capacity retry from its successor run (#19051)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server sends unexpected run failures to Sentry as run-failure events > - A native Codex run can fail because the model is at capacity. Paperclip then schedules a bounded automatic retry > - #18295 keeps that scheduled retry out of Sentry. It requires the committed finalization fields on the failed run row > - On staging, the failed run row can lose `finalizationReasonCode` after the decision is committed. The retry run exists, but the report still goes to Sentry > - This pull request uses the successor retry run as the proof, and stops reading the run's own finalization fields > - The benefit is that a capacity failure that has a scheduled retry does not make Sentry noise. Exhausted and held failures still report ## Linked Issues or Issue Description Refs #18295 **What happened?** A native Codex run failed with `native_provider_overloaded`. The status decision scheduled the capacity retry, and the retry run was created. The server still sent a run-failure event to Sentry. The server release contained #18295. The failed run row had `finalizationPhase: "committed"`, but it did not have `finalizationReasonCode`. The retry run was created about two seconds before the failed run's terminal write. **Expected behavior** When the committed capacity decision creates a retry run, the failure stays local. The retry owns recovery. A later failure of the retry reports its own outcome. **Steps to reproduce** 1. Make a native Codex run fail with a typed `codex`/`server_overloaded` diagnostic. 2. Let the status decision schedule the `native_provider_overloaded` retry. 3. Let a late terminal write replace the run's `resultJson` with a version that does not have `finalizationReasonCode`. 4. Report the run failure. Before this change, Sentry gets an event. **Additional context** Only the committed `schedule_retry` effect in `status-decision-committer.ts` creates a successor with `scheduledRetryReason = native_provider_overloaded` (through `materializeNativeProviderCapacityRetry`). Thus the successor row is a stronger proof than the run's own finalization fields. ## What Changed - `server/src/services/run-failure-report.ts`: `isScheduledNativeCapacityRetry` no longer requires `finalizationPhase === "committed"` or `finalizationReasonCode === "native_provider_overloaded"` on the failed run. It still requires a failed native run with `errorCode = native_provider_overloaded`, the typed `codex`/`server_overloaded` diagnostic, a native task, no signal, and a phase that is not setup. It still requires the successor run for the same task with `retryOfRunId = run.id` and `scheduledRetryReason = native_provider_overloaded`. The successor lookup and its index use do not change. - `server/src/services/__tests__/run-failure-report.test.ts`: two new cases. A scheduled retry stays local when the run row has no finalization fields, and when the row has an earlier finalization phase. The "finalization is not committed" must-report case changes to "finalization has no fields and no retry run exists". That case still reports. - `doc/observability.md`: describes the successor-run proof. ## Verification - `cd server && npx vitest run src/services/__tests__/run-failure-report.test.ts`: 284 tests pass. - Red check: with the `master` version of `run-failure-report.ts`, the two new cases fail. Each one captures one Sentry event. All other 282 tests pass. - `npx vitest run src/services/native-runtime/native-provider-failure.test.ts src/services/native-runtime/status-arbiter.test.ts`: 73 tests pass. - I could not load `native-provider-overload.integration.test.ts` in my local sandbox because of a missing environment setting. CI runs it. - Exhausted, held, other-lane, other-task, no-task, legacy, setup, signal and untyped-diagnostic failures still report. The existing must-report cases prove this. ## Risks Low risk. The change removes two field checks. The successor-run check stays. Only the committed capacity decision creates a successor with this retry reason, so a failure without a scheduled retry still reports. No migration. No change to the retry behavior. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. ## 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)
This commit is contained in:
1 parent
0173110ee3
commit
69dea878c0
3 files changed
+27
-16
No files matched your search
@@ -1134,10 +1134,11 @@ untyped capacity text, and Codex CLI transient upstream failures remain reportab
|
|||||||
The run keeps its failed state and its terminal diagnostic.
|
The run keeps its failed state and its terminal diagnostic.
|
||||||
|
|
||||||
A native Codex model capacity failure stays local only while its automatic
|
A native Codex model capacity failure stays local only while its automatic
|
||||||
retry owns recovery. The run must carry `native_provider_overloaded`, the closed
|
retry owns recovery. The run must carry `native_provider_overloaded` and the
|
||||||
`codex`/`server_overloaded` diagnostic, and a committed finalization whose
|
closed `codex`/`server_overloaded` diagnostic. The proof of the retry is the
|
||||||
reason is `native_provider_overloaded`. The report also requires the
|
`native_provider_overloaded` successor run for the same task, which only the
|
||||||
`native_provider_overloaded` retry run that the same status transaction created.
|
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.
|
||||||
Exhausted or held capacity failures, other retry lanes, setup failures, signals,
|
Exhausted or held capacity failures, other retry lanes, setup failures, signals,
|
||||||
and unreadable retry evidence remain reportable. A failed retry reports its own
|
and unreadable retry evidence remain reportable. A failed retry reports its own
|
||||||
outcome.
|
outcome.
|
||||||
|
|||||||
@@ -467,6 +467,17 @@ describeEmbeddedPostgres("reportRunFailure", () => {
|
|||||||
expect(mockCaptureRunFailure).not.toHaveBeenCalled();
|
expect(mockCaptureRunFailure).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
["no finalization fields", { finalizationPhase: undefined, finalizationReasonCode: undefined }],
|
||||||
|
["an earlier finalization phase", { finalizationPhase: "retryable_failure", 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.
|
||||||
|
const run = await nativeCapacityFailure({ resultJson });
|
||||||
|
await reportRunFailure(db, run);
|
||||||
|
expect(mockCaptureRunFailure).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it.each([
|
it.each([
|
||||||
["the retries are exhausted", { resultJson: { finalizationReasonCode: "native_provider_overloaded_exhausted" }, retry: null }],
|
["the retries are exhausted", { resultJson: { finalizationReasonCode: "native_provider_overloaded_exhausted" }, retry: null }],
|
||||||
["the task holds the retry", { resultJson: { finalizationReasonCode: "native_provider_overloaded_held" }, retry: null }],
|
["the task holds the retry", { resultJson: { finalizationReasonCode: "native_provider_overloaded_held" }, retry: null }],
|
||||||
@@ -474,7 +485,7 @@ describeEmbeddedPostgres("reportRunFailure", () => {
|
|||||||
["the successor belongs to another retry lane", { retry: { scheduledRetryReason: "native_safe_replacement" } }],
|
["the successor belongs to another retry lane", { retry: { scheduledRetryReason: "native_safe_replacement" } }],
|
||||||
["the successor belongs to another task", { retry: { contextSnapshot: { issueId: randomUUID() } } }],
|
["the successor belongs to another task", { retry: { contextSnapshot: { issueId: randomUUID() } } }],
|
||||||
["the run has no native task", { run: { nativeIssueId: null } }],
|
["the run has no native task", { run: { nativeIssueId: null } }],
|
||||||
["finalization is not committed", { resultJson: { finalizationPhase: "retryable_failure" } }],
|
["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 not typed capacity evidence", { resultJson: { nativeProviderFailure: { provider: "codex", category: "turn_failed" } } }],
|
||||||
["the diagnostic is missing", { resultJson: { nativeProviderFailure: undefined } }],
|
["the diagnostic is missing", { resultJson: { nativeProviderFailure: undefined } }],
|
||||||
["the run is legacy", { run: { runtimeMode: "legacy" } }],
|
["the run is legacy", { run: { runtimeMode: "legacy" } }],
|
||||||
|
|||||||
@@ -238,20 +238,19 @@ function isCodexAcpModelCapacityFailure(run: HeartbeatRun, options: RunFailureRe
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A committed Codex `serverOverloaded` terminal chose the bounded capacity
|
* A Codex `serverOverloaded` terminal chose the bounded capacity retry. Only
|
||||||
* retry, and the same status transaction created that retry run. The retry
|
* that committed status decision creates a successor with this retry reason,
|
||||||
* owns recovery; a later failure reports itself. Held and exhausted capacity
|
* so the successor row is the proof. The run's own finalization fields are
|
||||||
* failures have no automatic retry and stay reportable. The retry carries the
|
* not: a late terminal write can replace them with an earlier `resultJson`.
|
||||||
* task in its context, so the company/context-issue index bounds the read to
|
* The retry owns recovery; a later failure reports itself. Held and exhausted
|
||||||
* that task's runs created since this one.
|
* 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<boolean> {
|
async function isScheduledNativeCapacityRetry(db: Db, run: HeartbeatRun, options: RunFailureReportOptions): Promise<boolean> {
|
||||||
const result = asRecord(run.resultJson);
|
const diagnostic = asRecord(asRecord(run.resultJson)?.nativeProviderFailure);
|
||||||
const diagnostic = asRecord(result?.nativeProviderFailure);
|
|
||||||
if (run.status !== "failed" || run.runtimeMode !== "native" || run.errorCode !== NATIVE_PROVIDER_OVERLOADED_CODE ||
|
if (run.status !== "failed" || run.runtimeMode !== "native" || run.errorCode !== NATIVE_PROVIDER_OVERLOADED_CODE ||
|
||||||
run.nativeIssueId == null ||
|
run.nativeIssueId == null || options.phase === "setup" || run.signal != null ||
|
||||||
options.phase === "setup" || run.signal != null || result?.finalizationPhase !== "committed" ||
|
|
||||||
result.finalizationReasonCode !== NATIVE_PROVIDER_OVERLOADED_CODE ||
|
|
||||||
diagnostic?.provider !== NATIVE_PROVIDER_OVERLOADED_DIAGNOSTIC.provider ||
|
diagnostic?.provider !== NATIVE_PROVIDER_OVERLOADED_DIAGNOSTIC.provider ||
|
||||||
diagnostic.category !== NATIVE_PROVIDER_OVERLOADED_DIAGNOSTIC.category) return false;
|
diagnostic.category !== NATIVE_PROVIDER_OVERLOADED_DIAGNOSTIC.category) return false;
|
||||||
const [retry] = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and(
|
const [retry] = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and(
|
||||||
|
|||||||
Reference in new issue
Block a user