From 8464a8c9ae7a2463823cd4915f4d8d4ab26e4912 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 5 Nov 2026 11:45:54 -0800 Subject: [PATCH] fix(native): settle a bootstrap retry that lost its task lock (#19307) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run that fails before any provider work gets a bounded same-run bootstrap retry. Restart recovery decides whether that retry can run. > - The retry needs the task's issue lock. A user comment or another wake can move that lock to a new run before the retry is due. > - Restart recovery then returned `issue_execution_lock_changed` without a write. The same check ran on every recovery pass and never ended. The coordinator stayed `retryable_failure`. > - The orphaned-lease sweep waits for a terminal coordinator before it preserves the workspace. So the sandbox lease stayed active, and every later run for the task was cancelled as `workspace_busy`. > - This pull request gives a lock-lost bootstrap retry the same bounded admission that a superseded remote retry already has. > - The benefit is that the retry settles, the board gets a reconciliation action, and the sandbox lease is released instead of blocking the task. ## Linked Issues or Issue Description No public issue exists. Bug description: **What happened?** On a hosted instance, a native agent run failed at startup because the provider rejected `session.open` with a rate limit. Paperclip scheduled a same-run bootstrap retry. Seconds later, a new run for the task took its issue lock, most likely woken by a user comment. From then on, every restart recovery pass for the old run returned `blocked` / `issue_execution_lock_changed` and wrote nothing. The run recorded about 1,000 recovery events over 12 hours. Its coordinator stayed `retryable_failure` with `recoveryMode: bootstrap_retry`. The scoped orphaned-lease sweep found a pending workspace export for the lease. It could not preserve the workspace, because `preserveNativeFailedWorkspace` requires `terminal_failure`. So the lease stayed `active`, and 16 later runs for the task were cancelled as `workspace_busy`. Three more runs on the same instance were stuck in the same state. A board Stop on the failed run moved the coordinator to `terminal_failure`, and the sweep then released the lease within about a minute. **Expected behavior** A bootstrap retry that cannot get its task lock back settles within a bounded number of recovery passes. The coordinator reaches `terminal_failure`, the board gets a reconciliation action, and the existing orphaned-lease sweep can preserve the workspace and release the sandbox lease. **Steps to reproduce** 1. Fail a native remote run before provider work, so its coordinator is `retryable_failure` with `recoveryMode: bootstrap_retry`. 2. Move the task's `executionRunId` to a newer run for the same task, or clear it. 3. Run `claimNativeRestartRecoveries` for the failed run. 4. Before this change, it returns `blocked` with no `retryAt`, and the coordinator stays `retryable_failure` on every pass. The new tests in `native-remote-retry-admission.test.ts` reproduce this. **Paperclip version or commit** Observed on a hosted deployment. Reproduced in tests on `master` at `10a7d039c8e`. Related: #19190 (a `session.open` model rejection no longer schedules a doomed bootstrap retry), #19131 (preserve superseded observed workspaces) and #19281 (release the leases of a cancelled legacy run). None of them changes this admission path. ## What Changed - **Restart recovery (`server/src/services/native-runtime/native-restart-recovery.ts`)**: a new condition, `lockLostBootstrapRetry`, matches a failed run whose coordinator is `retryable_failure` with `recoveryMode: bootstrap_retry` and whose issue lock is not on the run. - A newer run for the task: the existing superseded check now covers this case too. It defers with `native_retry_source_superseded` as permanent, so admission is exhausted at once. - No newer run (the lock was released): recovery defers with `issue_execution_lock_changed` through the existing `deferRemoteRetry`. That path persists a deadline, backs off and exhausts after the system recovery budget. - When admission is exhausted, the existing hold path writes `terminal_failure` / `native_restart_recovery_blocked`, keeps the provider evidence and creates the board reconciliation action. This PR adds no new terminal path. - A bootstrap retry that still holds its own lock is unchanged. The exact-checkpoint remote retry path is unchanged. - **Docs (`doc/execution-semantics.md`)**: the bootstrap retry section now states the lock-lost rule. ## Verification - New test `settles a %s bootstrap retry that lost its task lock instead of re-checking forever` in `server/src/services/native-runtime/native-remote-retry-admission.test.ts`, for the `superseded` and `released` cases. The fixture is a genuine pre-provider failure: a `session.open` rejection with no session checkpoint, no runner checkpoint row and no provider evidence. - On `master` without the fix, both cases fail: recovery returns `blocked` without a deadline, and the coordinator stays `retryable_failure`. - With the fix, `superseded` settles on the first pass, and `released` settles after `SYSTEM_RECOVERY_MAX_RETRIES` deferred passes. Each deferred pass persists its `nextAttemptAt`. Both end in `terminal_failure` with one board recovery action, and later passes open no transaction. - `npx vitest run src/services/native-runtime/native-remote-retry-admission.test.ts` (from `server/`): 92 passed. - Related suites, each run on its own with exit code 0: `native-ingress-bootstrap-retry`, `native-startup-retry`, `native-scheduled-resume-lock`, `native-restart-recovery-isolation`, `native-runner-restart-recovery`, `native-runner-restart-preparation`, `native-runner-restart-diskless`, `native-warm-transition-restart`, `native-controller-recovery`, `native-failed-workspace-recovery`, `native-session-resumption`, `native-startup-cleanup-recovery`, `heartbeat-observed-workspace-preservation`, `heartbeat/recovery`, `runtime-selection`, `remote-workspace-migration` and `native-session-executor` (1,149 tests). Two other suites each had one local failure that this change does not cause: - `heartbeat-process-recovery`: "redacts opaque environment-bound credentials from Sentry diagnostics" fails the same way on this machine without the change. - `chat-channels.integration`: "settles a shared-endpoint accepted-response backlog without retrying its own locks" failed once under full-suite load. It passes alone, with and without the change. - `pnpm run typecheck` in `server/`: passed. ## Risks - Low to medium. The change only affects failed bootstrap retries whose lock is not on the run. Before this change, such a retry could never run, so no working path is removed. - A released lock that comes back to the same run during the backoff still resumes normally, because the condition reads the current lock on each pass. - When the retry settles, the task gets a board reconciliation action. Before this change, the task was silently stuck behind `workspace_busy`. - No migration and no API change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, code search, test runs), extended thinking, and a read-only investigation subagent for the root cause. ## 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 --------- Co-authored-by: Paperclip --- doc/execution-semantics.md | 10 +++++ .../native-remote-retry-admission.test.ts | 44 +++++++++++++++++++ .../native-runtime/native-restart-recovery.ts | 11 ++++- 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 9a07565666d..65be57ed92d 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -2022,6 +2022,16 @@ not grant retry authority or change the budget. When the budget is exhausted, the task is blocked and its recovery action states that provider startup failed before any provider work began. +A failed bootstrap retry cannot take back an issue lock that moved off its run. +When a newer run exists for the task, restart recovery settles the old retry at +once. When the lock was released and no newer run exists, recovery uses the +bounded admission backoff and then settles the retry. In both cases the +coordinator reaches `terminal_failure`, the board receives a reconciliation +action, and the orphaned-lease sweep can preserve the workspace and release the +sandbox lease. Before this rule, the retry re-checked the lock without a +deadline and never settled, so its sandbox lease stayed active and every later +run for the task was cancelled as `workspace_busy`. + The same limit evidence also holds new work for the agent. Before any workspace or provider work, a queued run for an agent with an active provider limit is cancelled as `provider_limit_wait`, records the expected reset and the run that diff --git a/server/src/services/native-runtime/native-remote-retry-admission.test.ts b/server/src/services/native-runtime/native-remote-retry-admission.test.ts index 69227e908e5..c717dcd5ece 100644 --- a/server/src/services/native-runtime/native-remote-retry-admission.test.ts +++ b/server/src/services/native-runtime/native-remote-retry-admission.test.ts @@ -567,6 +567,50 @@ describeEmbeddedPostgres("remote native retry execution admission", () => { } }); + it.each(["superseded", "released"] as const)("settles a %s bootstrap retry that lost its task lock instead of re-checking forever", async lock => { + const f = await failedRemoteRun("remote"); + // A bootstrap retry follows a pre-provider failure: no checkpoint, provider session or provider events. + const [run] = await f.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId)); + const { sessionCheckpoint: _checkpoint, ...profile } = run.runnerProfileJson ?? {}; + await f.db.update(heartbeatRuns).set({ errorCode: "provider_session_open_rejected", runnerProfileJson: profile }) + .where(eq(heartbeatRuns.id, f.runId)); + await f.db.delete(nativeRunnerCheckpoints) + .where(and(eq(nativeRunnerCheckpoints.companyId, f.companyId), eq(nativeRunnerCheckpoints.runId, f.runId))); + await f.db.update(nativeRunFinalizations).set({ failureCode: "provider_session_open_rejected", failureDetail: { + recoveryMode: "bootstrap_retry", providerSessionEstablished: false, checkpointExists: false, providerEventsExist: false, + } }).where(eq(nativeRunFinalizations.runId, f.runId)); + if (lock === "superseded") { + const [newer] = await f.db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, issueId: f.issueId, + status: "running", createdAt: new Date(Date.now() + 1_000) }).returning({ id: heartbeatRuns.id }); + await f.db.update(issues).set({ executionRunId: newer.id }).where(eq(issues.id, f.issueId)); + } + const onClaim = vi.fn(); + const attempt = (now: Date) => claimNativeRestartRecoveries({ db: f.db, controller: environment.current.successor, + restartKind: "hard", runIds: [f.runId], now, onClaim }); + let now = new Date(); + let passes = 0; + for (; passes <= SYSTEM_RECOVERY_MAX_RETRIES + 1; passes++) { + const [disposition] = await attempt(now); + expect(disposition).toMatchObject({ kind: "blocked" }); + const [coordinator] = await f.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId)); + if (coordinator.phase === "terminal_failure") break; + if (!("retryAt" in disposition) || !disposition.retryAt) throw new Error("a deferred bootstrap retry must persist its deadline"); + expect(coordinator.nextAttemptAt).toEqual(disposition.retryAt); + now = disposition.retryAt; + } + expect(passes).toBe(lock === "superseded" ? 0 : SYSTEM_RECOVERY_MAX_RETRIES); + expect(onClaim).not.toHaveBeenCalled(); + expect((await f.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId)))[0]) + .toMatchObject({ phase: "terminal_failure", nextAttemptAt: null, attempt: 1, failureCode: "native_restart_recovery_blocked" }); + expect(await f.db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, f.issueId))) + .toMatchObject([{ ownerType: "board", wakePolicy: null }]); + const transaction = vi.spyOn(f.db, "transaction"); + try { + for (const elapsed of [30_000, 3_600_000]) expect(await attempt(new Date(now.getTime() + elapsed))).toEqual([]); + expect(transaction).not.toHaveBeenCalled(); + } finally { transaction.mockRestore(); } + }); + it.each(["paused", "Stop", "missing-authority"] as const)("bounds %s holds and preserves the provider and own task lock for manual reconciliation", async gate => { const f = await failedRemoteRun("remote"); if (gate === "paused") await f.db.update(agents).set({ status: "paused" }).where(eq(agents.id, f.agentId)); diff --git a/server/src/services/native-runtime/native-restart-recovery.ts b/server/src/services/native-runtime/native-restart-recovery.ts index 4eaf1ef110a..efb32887539 100644 --- a/server/src/services/native-runtime/native-restart-recovery.ts +++ b/server/src/services/native-runtime/native-restart-recovery.ts @@ -658,6 +658,11 @@ export async function claimNativeRestartRecoveries(input: { )).orderBy(desc(environmentLeases.createdAt)).limit(1); const failedRemoteRetry = row.run.status === "failed" && row.coordinator.phase === "retryable_failure" && row.coordinator.failureDetail?.recoveryMode === "exact_checkpoint_resume" && (remoteWorkspace !== null || pluginTask !== undefined); + // A same-run bootstrap retry cannot take back an issue lock that moved + // off it. Without a bounded admission it would re-check forever, never + // settle, and keep its sandbox lease from the orphan-lease sweep. + const lockLostBootstrapRetry = row.run.status === "failed" && row.coordinator.phase === "retryable_failure" + && row.coordinator.failureDetail?.recoveryMode === "bootstrap_retry" && row.issueExecutionRunId !== row.run.id; const releasedRemoteRetry = failedRemoteRetry && row.issueExecutionRunId === null; const exhaustedRemoteRetry = failedRemoteRetry && row.coordinator.attempt >= 3; const needsResumptionAdmission = releasedRemoteRetry || exhaustedRemoteRetry || @@ -706,7 +711,7 @@ export async function claimNativeRestartRecoveries(input: { if (!updated.length) throw new Error("native_retry_deferral_claim_changed"); return { kind: "blocked", runId: row.run.id, reason, retryAt, transitionUnchanged } as const; }; - if (failedRemoteRetry) { + if (failedRemoteRetry || lockLostBootstrapRetry) { const source = alias(heartbeatRuns, "native_retry_source"); const [newer] = await tx.select({ id: heartbeatRuns.id }).from(heartbeatRuns) .innerJoin(source, and(eq(source.companyId, row.run.companyId), eq(source.id, row.run.id))) @@ -730,6 +735,10 @@ export async function claimNativeRestartRecoveries(input: { const deferred = await deferRemoteRetry("native_retry_admission_ineligible", ["done", "cancelled"].includes(issue.status)); if (deferred) return deferred; } + if (lockLostBootstrapRetry && !admission.exhausted) { + const deferred = await deferRemoteRetry("issue_execution_lock_changed"); + if (deferred) return deferred; + } if (row.issueExecutionRunId !== row.run.id && !(stopCleanup && row.issueExecutionRunId === null) && !releasedRemoteRetry && !admission.exhausted) { return { kind: "blocked",