mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(workspaces): settle a resolved no-replay hold once a sibling saved its files (#20641)
## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
e3bc4ca798
commit
d06f43b14f
4 files changed
+103
-5
No files matched your search
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user