From 3ded7bbd91ac078a3629d86dd79bd8ab8733176d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 11 Nov 2026 10:44:27 -0600 Subject: [PATCH] fix(native): do not claim a server restart for a held same-run retry (#20832) 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 > - Native runs on a remote sandbox (Daytona) retry provider startup on the same run > - A due remote same-run retry re-enters the restart-recovery classifier > - When that classifier holds the run, the hold note says "Automatic recovery stopped after a server restart". No server restarted, so the user reads a false cause > - This pull request records the trigger of the hold and words a same-run-retry hold as a stopped automatic retry > - The benefit is that a held task names its real cause ## Linked Issues or Issue Description Refs #20808 Refs #20820 **What happened?** Two Codex runs on one Daytona sandbox failed in `session.open` (`bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted`). The executor saved a `bootstrap_retry`. Thirty seconds later, the due retry entered restart recovery and stopped with `ambiguous_remote_recovery_evidence`. The task said "Automatic recovery stopped after a server restart". No server restart happened. The second run was the board's Retry of the first, and it failed the same way. **Expected behavior** A hold that a same-run retry creates must not claim a server restart. **Steps to reproduce** 1. `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "same-run retry"` 2. On master the recovery action has no `trigger`, and its note contains "server restart". **Paperclip version or commit** master at 5f38226290. ## What Changed - Restart recovery records `trigger: "same_run_retry"` in the recovery action evidence when the held run was a saved `retryable_failure`. It reads this from the coordinator, so a deferred check or a check after a real restart records the same trigger. - `executionRecoverySettlementNote` says "Automatic retry of this run stopped" for such a hold. It does not name a cause, because the classifier can also stop it for `execution_recovery_budget_exhausted`. Holds without the field keep the old text. - Tests: one integration case with the staging signature and one unit case for the note. ## Verification - Red on master: the integration case fails because `evidence.trigger` is absent; the unit case fails because the note says "server restart". - Green here: `npx vitest run src/services/execution-recovery-resolution.test.ts src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "server restart|same-run retry|exited remote runner|remote bootstrap retry held"`: 19 pass on the rebased head. - `npx tsc --noEmit -p server/tsconfig.json`: clean. ## Risks - Low risk. The change adds one optional evidence field and one note branch. Recovery decisions do not change. - Saved state: holds that an older release wrote have no `trigger` and keep the old note. A mid-run deploy reads the field only when present. - Not fixed here: the Codex Planning startup fault itself (`bwrap: loopback: Failed RTM_NEWADDR`). #20820 changes the external-sandbox planning profile so Codex no longer uses its network-isolated filesystem helper. > 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 - Claude (Anthropic) Opus 5.5, model ID `claude-opus-5-5`, run in Claude Code with tool use (shell, file edit) and extended reasoning. ## 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 (no doc change needed) - [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 --- .../execution-recovery-resolution.test.ts | 7 +++++++ .../services/execution-recovery-resolution.ts | 7 ++++++- .../native-runtime/native-restart-recovery.ts | 4 ++++ ...unner-restart-recovery.integration.test.ts | 20 +++++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/server/src/services/execution-recovery-resolution.test.ts b/server/src/services/execution-recovery-resolution.test.ts index a7c0fc57143..854b92fc58f 100644 --- a/server/src/services/execution-recovery-resolution.test.ts +++ b/server/src/services/execution-recovery-resolution.test.ts @@ -9,6 +9,13 @@ describe("executionRecoverySettlementNote", () => { expect(note).toContain("after a server restart"); }); + it("does not claim a server restart for a hold a same-run retry created", () => { + const note = executionRecoverySettlementNote({ current: true, cause: "native_restart_recovery_blocked", + workspaceRestoreFailed: false, trigger: "same_run_retry" }); + expect(note).not.toContain("server restart"); + expect(note).toContain("Automatic retry of this run stopped"); + }); + it("keeps the workspace repair note for other holds with a restore failure", () => { expect(executionRecoverySettlementNote({ current: true, cause: "execution_finalization_deadline_exceeded", workspaceRestoreFailed: true })).toContain("Workspace repair required"); diff --git a/server/src/services/execution-recovery-resolution.ts b/server/src/services/execution-recovery-resolution.ts index 2e917f39b69..e59f05c18f5 100644 --- a/server/src/services/execution-recovery-resolution.ts +++ b/server/src/services/execution-recovery-resolution.ts @@ -46,8 +46,13 @@ import { alias } from "drizzle-orm/pg-core"; */ export function executionRecoverySettlementNote(input: { current: boolean; cause: string; workspaceRestoreFailed: boolean; + /** The hold's recorded trigger; a same-run retry is not a server restart. */ + trigger?: unknown; }): string { if (!input.current) return "Recovery closed because the task's owner, execution, or status changed. No work was replayed."; + if (input.cause === "native_restart_recovery_blocked" && input.trigger === "same_run_retry") { + return "Automatic retry of this run stopped. Saved work is preserved; actions with unverified outcomes will not be repeated."; + } if (input.cause === "native_restart_recovery_blocked") { return "Automatic recovery stopped after a server restart because the previous execution could not be verified. Saved work is preserved; actions with unverified outcomes will not be repeated."; } @@ -696,7 +701,7 @@ export async function settleUnrecoverableExecutions( (!task.executionRunId || task.executionRunId === run.id) && (!task.checkoutRunId || task.checkoutRunId === run.id); const note = executionRecoverySettlementNote({ current, cause: action.cause, - workspaceRestoreFailed: hasWorkspaceRestoreFailure(run.resultJson) }); + workspaceRestoreFailed: hasWorkspaceRestoreFailure(run.resultJson), trigger: action.evidence.trigger }); await notifyUnsafeWorkspaceExportRecovery(tx, { companyId: task.companyId, issueId: task.id }); let nativeFailureBlock = action.evidence.nativeFailureBlock; let nativeBootstrapFailureBlock = action.evidence.nativeBootstrapFailureBlock; diff --git a/server/src/services/native-runtime/native-restart-recovery.ts b/server/src/services/native-runtime/native-restart-recovery.ts index bc7a277b294..244d7120c77 100644 --- a/server/src/services/native-runtime/native-restart-recovery.ts +++ b/server/src/services/native-runtime/native-restart-recovery.ts @@ -1807,6 +1807,10 @@ export async function claimNativeRestartRecoveries(input: { evidence: { runId: row.run.id, reason, + // A saved retryable failure is a same-run retry, not a restart + // hold. Read from the coordinator so a deferred or post-restart + // check of that retry records the same trigger. + ...(row.coordinator.phase === "retryable_failure" ? { trigger: "same_run_retry" } : {}), providerAttempt: row.coordinator.attempt, ...(hold ? { automaticAdmissionExhausted: true, executorStopEvidence: "unverified", providerPid: row.run.processPid, checkpointPreserved: hasCheckpoint } : {}), diff --git a/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts b/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts index 348fcb7c29c..63678a59f2a 100644 --- a/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts +++ b/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts @@ -40,6 +40,7 @@ import { } from "@paperclipai/db"; import { beforeAll, describe, expect, it, vi } from "vitest"; import { heartbeatService } from "../heartbeat.js"; +import { executionRecoverySettlementNote } from "../execution-recovery-resolution.js"; import { canonicalNativeRuntimeContextDigest, @@ -3156,6 +3157,25 @@ process.on("SIGTERM", () => child.kill("SIGTERM")); kind: "blocked", runId: fixture.runId, reason })]); }); + it("records that a held same-run retry was not a server restart", async () => { + // Staging run 61d2e06e: a remote session.open rejection saved by an older + // release, held when its due retry re-entered restart recovery. No server + // restarted, so the hold must not say one did. + const fixture = await seedExitedRemoteRunner("SAME-RUN-RETRY-HOLD", { + originalFailureCode: "provider_session_open_rejected" }, 1); + expect(await claimNativeRestartRecoveries({ db: fixture.db, controller: successor, restartKind: "hard", + runIds: [fixture.runId] })).toEqual([expect.objectContaining({ + kind: "blocked", runId: fixture.runId, reason: "ambiguous_remote_recovery_evidence" })]); + const [action] = await fixture.db.select().from(issueRecoveryActions) + .where(eq(issueRecoveryActions.sourceIssueId, fixture.issueId)); + expect(action).toMatchObject({ cause: "native_restart_recovery_blocked", + evidence: expect.objectContaining({ trigger: "same_run_retry" }) }); + const note = executionRecoverySettlementNote({ current: true, cause: action.cause, + workspaceRestoreFailed: false, trigger: action.evidence.trigger }); + expect(note).not.toContain("server restart"); + expect(note).toContain("Automatic retry of this run stopped"); + }); + it.each<[string, Parameters[1], { kind: string; reason: string }]>([ ["a runner process start was requested", { bootstrapRetry: true, event: { eventType: "native.process_start_requested" } }, { kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }],