From d06f43b14f05da3a9a0a8202f0cc4140b976b327 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 10 Nov 2026 14:39:24 -0600 Subject: [PATCH] fix(workspaces): settle a resolved no-replay hold once a sibling saved its files (#20641) 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 > - When a remote workspace save fails, recovery actions hold the task until the files are saved > - One failed run can have two holds: its export hold, and a no-replay hold that a later "try again" opened for the same run > - The no-replay hold is recorded resolved-but-blocked, and it is the task's execution blocker > - #20569 lets the export hold retry under the new symlink capture policy and save the files > - The sibling settlement that should then release the no-replay hold only looked at active holds, so the task stayed blocked with no working Retry > - This pull request lets the sibling settlement and its discovery include resolved no-replay holds > - The benefit is that the task continues without a person once the files are saved ## Linked Issues or Issue Description Refs #20569 **What happened?** On staging, a native run on a Daytona sandbox failed to save its workspace (`remote_workspace_archive_unsafe_symlink`). Its export hold (`native_workspace_sync_out_retry_exhausted`) was exhausted after 6 attempts and handed to the board. A user comment then opened a second hold for the same run (`native_continuation_requires_reconciliation`, `preserve_without_replay_v1`). That hold also failed 6 times and was recorded `resolved` with outcome `blocked`. The task's execution blocker is this second hold. **Expected behavior** After #20569 is deployed, the worker re-arms the export hold and saves the files. The no-replay hold of the same run then settles, and the task is no longer blocked. **Steps to reproduce** 1. Create both holds with the exact staging evidence (see the new test `stagingIncident`). 2. Run the recovery worker until nothing is due. 3. Before this change, the export hold completes, but the no-replay hold stays `resolved`/`blocked` and `getExecutionBlocker` still returns it with `canRetry: false`. ## What Changed - `workspace-recovery-fallback.ts` (`settleSiblingRecoveredHold`): a resolved no-replay hold (`isRetainedFileHold`) can now settle when a verified receipt of a sibling action saved its run's files. Settling removes `automaticRecovery`, so the hold stops blocking the task. It starts no turn. - `workspace-recovery-fallback-discovery.ts`: the worker discovers such a resolved hold when a sibling of the same run is recovered. - New test in `workspace-recovery-capture-policy.test.ts` with the exact staging shape of both holds: board owner, `active_run_watchdog`, system recovery policy version 1 without a capture policy version, stage `restoring_files`, `export_interrupted` with cause `provider_error`, 6/6 attempts, `historicalWorkspaceSource` and `workspaceRestoreRecovery` lease ids. - `doc/execution-semantics.md`: states that a resolved no-replay hold also settles from a sibling save. ## Verification - Red before the fix: the new test failed. The no-replay hold stayed `resolved`/`blocked`, and it stayed the execution blocker. - Green after the fix: `npx vitest run --root server src/services/workspace-recovery-capture-policy.test.ts` (3 passed). The test drives the worker's own discovery (`nextWorkspaceRecoveryAt` + `reconcileRetainedWorkspaces`) until nothing is due. It checks: the export hold is re-armed once and saves the files; the no-replay hold is `resolved`/`restored` by the sibling; `getExecutionBlocker` returns null; the continuation hook ran when the task was free; no new run was created; the worker goes idle. - Related suites: `workspace-recovery-fallback`, `-admission`, `-stop-proof`, `-source`, `-preparation`, `-preparation-gates`, `execution-blocker-read`, `preserved-native-response-wait.integration`: 409 passed. - `npx tsc --noEmit -p server/tsconfig.json`: clean. - `pnpm verify` exits 2 (global doc input), so CI supplies the broad coverage. ## Risks - A resolved no-replay hold is now released when another action's verified receipt saved the same run's files. The receipt check is unchanged, so an unverified sibling still does not release it. - Settling starts no turn. Waiting input and the run's own result continue the task through normal admission, which still checks every other hold. - Previous-release story: holds saved by earlier releases need no migration. The worker discovers them on its first pass after deploy. > 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 Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through 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) Co-authored-by: Claude Opus 5.5 --- doc/execution-semantics.md | 5 +- .../workspace-recovery-capture-policy.test.ts | 91 ++++++++++++++++++- .../workspace-recovery-fallback-discovery.ts | 3 +- .../services/workspace-recovery-fallback.ts | 9 +- 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index c60dd58e74c..f80fe979282 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -2763,7 +2763,10 @@ reason. Transient reasons are checked again after five minutes, at most twelve times; new input, Retry or a new recovery generation reconsiders the hold. The recovery worker discovers these holds itself, so holds saved by earlier releases take the same path after deploy. An exhausted hold whose run's files a sibling -recovery action already saved settles as recovered. +recovery action already saved settles as recovered. This includes a no-replay +hold recorded resolved-but-blocked: settling it removes its replay block, so it +stops blocking the task, and starts no turn itself; the run's own result and +any waiting input decide what continues. An exhausted save whose last failure was `agent_stop_unconfirmed` is not a refusal of the saved files: its recovery window ran out before the agent's stop diff --git a/server/src/services/workspace-recovery-capture-policy.test.ts b/server/src/services/workspace-recovery-capture-policy.test.ts index 37eef096ca0..146a3896a3d 100644 --- a/server/src/services/workspace-recovery-capture-policy.test.ts +++ b/server/src/services/workspace-recovery-capture-policy.test.ts @@ -3,9 +3,10 @@ import { mkdir, readFile, symlink, writeFile } from "node:fs/promises"; import path from "node:path"; import { eq } from "drizzle-orm"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { activityLog, executionWorkspaces, issueRecoveryActions } from "@paperclipai/db"; +import { activityLog, executionWorkspaces, heartbeatRuns, issueRecoveryActions } from "@paperclipai/db"; import type { AdapterSandboxExecutionTarget } from "@paperclipai/adapter-utils/execution-target"; import { readRemoteWorkspaceAuthority } from "@paperclipai/shared"; +import { getExecutionBlocker } from "./execution-blocker.js"; import { REMOTE_WORKSPACE_CAPTURE_POLICY_VERSION } from "./remote-workspace-snapshot-archive.js"; import { readSystemRecovery } from "./system-recovery-state.js"; import { nextWorkspaceRecoveryAt } from "./system-recovery.js"; @@ -100,6 +101,94 @@ describe("saves exhausted under an older workspace capture policy", () => { details: expect.objectContaining({ reason: "capture_policy_changed" }) })]); }, 60_000); + /** + * The full staging record of the same incident. The run's export hold + * (cause native_workspace_sync_out_retry_exhausted) names its source through + * historicalWorkspaceSource and the run's workspaceRestoreRecovery. A later + * "try again" comment opened a second, no-replay hold for the same run + * (native_continuation_requires_reconciliation, preserve_without_replay_v1). + * Its six attempts failed the same way and it was recorded + * resolved-but-blocked. That second hold is the task's execution blocker. + */ + async function stagingIncident() { + const f = await exhaustedCaptureHold(); + const exportHold = await action(f.actionId); + const recovery = { schema: "paperclip.workspace-restore-recovery.v1", leaseIds: [f.lease.id] }; + const source = (await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId)))[0]!; + await db.update(heartbeatRuns).set({ errorCode: "native_workspace_sync_out_retry_exhausted", + error: "Saving the workspace failed after 3 attempts. The agent's result is kept; follow the run's recovery action to repair the workspace. " + + "Last cause: remote_workspace_archive_unsafe_symlink", + resultJson: { ...source.resultJson, failureCode: "native_workspace_sync_out_retry_exhausted", + failureCause: "remote_workspace_archive_unsafe_symlink", nextAttemptAt: null, finalizationPhase: "terminal_failure", + originalFailureCode: "native_workspace_sync_out_failed", workspaceRestoreFailure: "restore_failed", + workspaceRestoreRecovery: recovery, systemRecoveryAdoption: { actionId: f.actionId, policyVersion: 1 } } }) + .where(eq(heartbeatRuns.id, f.runId)); + await db.update(issueRecoveryActions).set({ kind: "active_run_watchdog", returnOwnerAgentId: source.agentId, maxAttempts: 3, + evidence: { ...exportHold.evidence, coordinatorAttempt: 1, originalFailureCode: "native_workspace_sync_out_failed", + systemRecoveryActiveAt: Date.now() - 50 * 60_000, workspaceFinalizeAttempt: 3, previousWorkspaceRecovery: null, + historicalWorkspaceSource: { schema: "paperclip.historical-workspace-source.v1", runId: f.runId, leaseIds: [f.lease.id] } } }) + .where(eq(issueRecoveryActions.id, f.actionId)); + const exhausted = readSystemRecovery((await action(f.actionId)).evidence)!; + const noReplayHoldId = randomUUID(), resolvedAt = new Date(Date.now() - 30 * 60_000); + await db.insert(issueRecoveryActions).values({ id: noReplayHoldId, companyId: f.companyId, sourceIssueId: f.issueId, + kind: "active_run_watchdog", status: "resolved", outcome: "blocked", ownerType: "board", resolvedAt, + cause: "native_continuation_requires_reconciliation", fingerprint: `native-workspace:${f.runId}`, + nextAction: exportHold.nextAction, evidence: { runId: f.runId, + systemRecovery: { ...exhausted, generationId: randomUUID(), attemptId: randomUUID() }, + automaticRecovery: { runId: f.runId, policy: "preserve_without_replay_v1", replay: "blocked", + recordedAt: resolvedAt.toISOString(), actionOutcome: "unknown" }, + workspaceRecovery: exportHold.evidence.workspaceRecovery, + originalFailureCode: "native_workspace_sync_out_retry_exhausted", continuationDelivery: "invalidated", + systemRecoveryActiveAt: Date.now() - 40 * 60_000, workspaceRestoreFailure: "restore_failed", + workspaceRestoreRecovery: recovery, previousWorkspaceRecovery: null } }); + return { ...f, noReplayHoldId }; + } + + it("settles the staging incident's export hold and its resolved no-replay sibling without a person", async () => { + const f = await stagingIncident(); + // As on staging: the no-replay sibling is the execution blocker, and it + // offers no task Retry. + expect(await getExecutionBlocker(db, f.companyId, f.issueId)).toMatchObject({ recoveryActionId: f.noReplayHoldId, + workspaceRepairRequired: true, canRetry: false }); + + // The recovery worker's own discovery, as after a deploy: it runs while + // work is due and goes idle once nothing is. + const blockersSeenByContinuation: unknown[] = []; + const onRecovered = vi.fn(async () => { blockersSeenByContinuation.push(await getExecutionBlocker(db, f.companyId, f.issueId)); }); + let passes = 0; + for (; passes < 6; passes += 1) { + const due = await nextWorkspaceRecoveryAt(db); + if (due === null || due > Date.now()) break; + await Promise.all(await reconcileRetainedWorkspaces(db, f.environmentRuntime, onRecovered, true) ?? []); + } + expect(passes).toBeGreaterThan(0); + expect(passes).toBeLessThan(6); + + const saved = await action(f.actionId); + expect(readSystemRecovery(saved.evidence)).toMatchObject({ state: "complete", capturePolicyVersion: REMOTE_WORKSPACE_CAPTURE_POLICY_VERSION }); + expect(readWorkspaceRecovery(saved.evidence)?.status).toBe("recovered"); + expect(saved.ownerType).toBe("system"); + + // The no-replay sibling of the same run is settled by that save and no + // longer holds the task. Settling it replays nothing. + const sibling = await action(f.noReplayHoldId); + expect(sibling).toMatchObject({ status: "resolved", outcome: "restored", ownerType: "system", + resolutionNote: "workspace_recovered_by_sibling_action" }); + expect(sibling.evidence.automaticRecovery).toBeUndefined(); + expect(sibling.evidence.siblingWorkspaceRecovery).toMatchObject({ recoveryActionId: f.actionId }); + expect(readWorkspaceRecovery(sibling.evidence)?.status).toBe("recovered"); + expect(String(sibling.nextAction)).not.toMatch(adminText); + expect(await getExecutionBlocker(db, f.companyId, f.issueId)).toBeNull(); + // The continuation hook ran once the task was free, so saved input is dispatched. + expect(blockersSeenByContinuation.at(-1)).toBeNull(); + expect((await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, f.companyId))).map(row => row.id)).toEqual([f.runId]); + // Bounded: one re-arm, and the worker has nothing left to do. + const next = await nextWorkspaceRecoveryAt(db); + expect(next === null || next > Date.now()).toBe(true); + expect((await db.select().from(activityLog).where(eq(activityLog.action, "issue.workspace_recovery_rearmed"))) + .filter(row => row.entityId === f.issueId)).toHaveLength(1); + }, 90_000); + it("does not re-arm a save that the current policy already exhausted", async () => { const f = await exhaustedCaptureHold(REMOTE_WORKSPACE_CAPTURE_POLICY_VERSION); expect((await retainedWorkspaceCandidates(db, undefined, false, true)).map(row => row.action.id)).not.toContain(f.actionId); diff --git a/server/src/services/workspace-recovery-fallback-discovery.ts b/server/src/services/workspace-recovery-fallback-discovery.ts index adfe86eaf31..aaee9cd2031 100644 --- a/server/src/services/workspace-recovery-fallback-discovery.ts +++ b/server/src/services/workspace-recovery-fallback-discovery.ts @@ -160,7 +160,8 @@ export function retainedSourceFallbackPendingCondition() { and exists (select 1 from heartbeat_runs source where source.company_id = ${issueRecoveryActions.companyId} and source.id = ${sourceRunId} and source.runner_profile_json#>>'{remoteWorkspace,state}' = 'prepared')) - or (${issueRecoveryActions.status} in ('active', 'escalated') and exists (select 1 from issue_recovery_actions sibling + or ((${issueRecoveryActions.status} in ('active', 'escalated') or ${retainedFileHoldCondition()}) + and exists (select 1 from issue_recovery_actions sibling where sibling.company_id = ${issueRecoveryActions.companyId} and sibling.source_issue_id = ${issueRecoveryActions.sourceIssueId} and sibling.id <> ${issueRecoveryActions.id} and sibling.evidence->>'runId' = ${issueRecoveryActions.evidence}->>'runId' and sibling.evidence#>>'{systemRecovery,state}' = 'complete' diff --git a/server/src/services/workspace-recovery-fallback.ts b/server/src/services/workspace-recovery-fallback.ts index 82f1a86682f..ffec097f372 100644 --- a/server/src/services/workspace-recovery-fallback.ts +++ b/server/src/services/workspace-recovery-fallback.ts @@ -469,7 +469,10 @@ export async function settleSiblingRecoveredHold(db: Db, input: Source) { if (!issue || !run || !action || action.evidence.runId !== run.id) return null; if (object(action.evidence.siblingWorkspaceRecovery).schema === SIBLING_WORKSPACE_RECOVERY_SCHEMA) return "already" as const; const system = readSystemRecovery(action.evidence); - if (!["active", "escalated"].includes(action.status) || system?.state !== "exhausted" || system.sourceRunId !== run.id) return null; + // A no-replay hold is recorded resolved-but-blocked. It blocks the task + // until its run's files are saved, so a sibling's save settles it too. + if ((!["active", "escalated"].includes(action.status) && !isRetainedFileHold(action)) + || system?.state !== "exhausted" || system.sourceRunId !== run.id) return null; const receiptId = await verifiedWorkspaceRepairReceipt(tx as unknown as Db, run); const [receipt] = receiptId ? await tx.select({ metadata: workspaceOperations.metadata }).from(workspaceOperations).where(and( eq(workspaceOperations.companyId, input.companyId), eq(workspaceOperations.id, receiptId))) : []; @@ -487,7 +490,9 @@ export async function settleSiblingRecoveredHold(db: Db, input: Source) { await tx.update(issueRecoveryActions).set({ status: "resolved", outcome: "restored", resolvedAt: now, updatedAt: now, ownerType: "system", ownerAgentId: null, ownerUserId: null, nextAction: "Saved work recovered.", resolutionNote: "workspace_recovered_by_sibling_action", wakePolicy: null, monitorPolicy: null, - evidence: { ...action.evidence, + // With the files saved, a no-replay hold no longer blocks the task. + // Settling starts no turn; the run's own result decides what continues. + evidence: { ...action.evidence, automaticRecovery: undefined, workspaceRecovery: { schema: "paperclip.workspace-recovery.v1", runId: run.id, status: "recovered", stage: "saving_receipts", attempt: prior?.attempt ?? system.attempts, attemptLimit: prior?.attemptLimit ?? 6, requestIds: prior?.requestIds ?? [], retryable: false }, siblingWorkspaceRecovery: { schema: SIBLING_WORKSPACE_RECOVERY_SCHEMA, receiptId, recoveryActionId: siblingActionId,