From b01959cb155a49535acc197e4e7fbd55e13fdf12 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 11 Nov 2026 15:52:35 -0600 Subject: [PATCH] fix(recovery): release a handed-off run's saved-files hold for the person it handed the task to (#20941) 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. > - An agent can hand its own task to a person for review. With #20884, a native run that does this keeps running and finishes its turn. > - A restart can land between that hand-off and the end of the same turn. Restart recovery then settles the run as failed and keeps its unsaved sandbox files, so no work is lost. > - Automatic workspace recovery saves those files. But the workspace-repair hold then stayed on the person's task, because the hold only released for a newer agent owner. > - This pull request releases that hold for the person who received the task through the stopped run's own recorded hand-off, once the files are saved. > - The benefit is that the person's task rests in review with no stale "Saved work recovered" hold, and the agent's files are still saved first. ## Linked Issues or Issue Description Follow-up to #20884 (merged). It was reviewed there as a known limit: "Restart blocks the person's task". It uses the `resultJson.humanSelfHandoff` record that #20884 added. Related: #19184, which released the same hold for a newer agent owner. **What happened?** A native run handed its own task to a person, and a restart landed before the run finished its turn. Restart recovery settled the run and retained its files. After automatic recovery saved them, the task still had a workspace-repair hold ("Saved work recovered. Send a message to continue."). The person had nothing to do with that run's work, and nothing released the hold. **Expected behavior** The hold protects the workspace until the files are saved. After that, the task rests with the person and has no execution blocker. **Steps to reproduce** 1. A native run records a hand-off to a person, and the task is now `in_review` with that person. 2. A restart settles the run as failed, with its prepared remote workspace retained. 3. Automatic workspace recovery completes. 4. Before this fix, `getExecutionBlocker` still returns the run's workspace-repair hold. ## What Changed - `server/src/services/execution-blocker.ts`: `restoreHoldReleasedForCurrentOwnerPredicate` now also releases a recovered file-preservation hold in one more case. The task must be assigned to a person, and the stopped run's `resultJson.humanSelfHandoff` must name that task and that person. The existing proof is still required: the completed-recovery receipt bound to the action, and a copyback rather than a snapshot-only save. Until the files are saved, the hold stays. - `server/src/services/workspace-recovery-admission.test.ts`: two embedded-Postgres tests. - The hold blocks the person's task before recovery and releases after it. - A hand-off recorded for a different person keeps the hold. - `doc/execution-semantics.md`: one sentence on the restart case. ## Verification - Red: with the `execution-blocker.ts` change removed, `releases a handed-off run's recovered files hold for the person it handed the task to` fails. `getExecutionBlocker` still returns the hold after `recoverRetainedWorkspace`. - Green: `cd server && npx vitest run src/services/workspace-recovery-admission.test.ts src/__tests__/interrupted-preparation-hold-settle.test.ts src/services/explicit-native-continuation.test.ts src/services/native-runtime/native-human-self-handoff.test.ts`. All 617 tests pass. The server typecheck passes. - Chaos: the H2 workload from #20884 covers the hand-off with a K1 restart after it. Locally the runs had no unsaved files, so no hold appeared. This PR's case needs a remote sandbox with unsaved files. **No Daytona wave was run.** ## Risks - The release applies only when the stopped run's own record names the current person and task, and only after the completed-recovery receipt. Other holds, snapshot-only saves and unsafe archives are unchanged. - No schema change. A run without the record behaves as before. - This branch is rebased onto master after #20884 merged. The only conflict was an adjacent import line. > 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), model ID `claude-opus-5-5`, through Claude Code with tool use and code execution. The session does not expose the context window or reasoning mode. ## 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 Fable 5.2 --- doc/execution-semantics.md | 5 +++- server/src/services/execution-blocker.ts | 25 +++++++++++----- .../workspace-recovery-admission.test.ts | 30 +++++++++++++++++++ 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 65ebcda93af..e12aa309131 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -83,7 +83,10 @@ the task. Its later task mutations are rejected, including under the update transaction's task and run locks. Read and comment access follows normal policy. A native run's task tools stop acting for the task once it is handed off, and the run's result is kept as history: its finalizer preserves the person's task status -and adds no continuation, blocker, or recovery to it. +and adds no continuation, blocker, or recovery to it. If a restart interrupts the +outgoing turn, the run settles and its unsaved files are retained. Once automatic +recovery saves them, the workspace hold releases for the person who received the +task through that run's own recorded hand-off. Board-initiated reassignments, agent API keys, and handoffs to another agent still require the outgoing execution to stop before ownership changes. diff --git a/server/src/services/execution-blocker.ts b/server/src/services/execution-blocker.ts index 35229f1f2a3..379a76d7a82 100644 --- a/server/src/services/execution-blocker.ts +++ b/server/src/services/execution-blocker.ts @@ -19,6 +19,7 @@ import { nativeManualAdmissionHoldsExecution } from "./native-runtime/native-run import { PROCESS_IDENTITY_RECORDED, PROCESS_START_REQUESTED } from "./native-local-process-stop.js"; import { NATIVE_CLEANUP_SETTLING_MESSAGE, NATIVE_SESSION_CLEANUP_QUARANTINED } from "./native-runtime/completed-turn-cleanup.js"; import { EXECUTION_FINALIZATION_DEADLINE_EXCEEDED, EXECUTION_FINALIZATION_DEADLINE_MESSAGE } from "./execution-control-deadline.js"; +import { NATIVE_HUMAN_SELF_HANDOFF_KEY } from "./native-runtime/native-human-self-handoff.js"; const blockerContextKeys = ["issueId", "taskId", "source", "connectionIntentResolution", "interactionKind", "interactionStatus", "mutation", "interactionId"] as const; @@ -254,7 +255,8 @@ export function interruptedPreparationBeforeProviderWorkCondition() { * unknown outcome, so its hold releases for the same agent too. The proof is * the durable completed-recovery receipt bound to this action, so later reuse * of the workspace by the current owner (new epoch, writer or snapshot) cannot - * re-arm the hold. + * re-arm the hold. A person who received the task through the stopped run's + * own recorded hand-off is released the same way. */ export function restoreHoldReleasedForCurrentOwnerPredicate() { const sourceRunId = sql`coalesce(${issueRecoveryActions.evidence}->>'runId', ${issueRecoveryActions.evidence}->>'sourceRunId')`; @@ -271,13 +273,20 @@ export function restoreHoldReleasedForCurrentOwnerPredicate() { and stopped.id::text = ${sourceRunId} and stopped.status in ('failed', 'timed_out', 'interrupted', 'cancelled') and stopped.finished_at is not null - and task.assignee_agent_id is not null - and ((task.assignee_agent_id <> stopped.agent_id - and not coalesce(task.status = 'in_review' - and task.execution_state->>'status' = 'pending' - and task.execution_state->'currentParticipant'->>'type' = 'agent' - and task.execution_state->'currentParticipant'->>'agentId' = stopped.agent_id::text, false)) - or ${stoppedBeforeProviderTurnCondition()} or ${interruptedPreparationBeforeProviderWorkCondition()}))`, + and ((task.assignee_agent_id is not null + and ((task.assignee_agent_id <> stopped.agent_id + and not coalesce(task.status = 'in_review' + and task.execution_state->>'status' = 'pending' + and task.execution_state->'currentParticipant'->>'type' = 'agent' + and task.execution_state->'currentParticipant'->>'agentId' = stopped.agent_id::text, false)) + or ${stoppedBeforeProviderTurnCondition()} or ${interruptedPreparationBeforeProviderWorkCondition()})) + -- The stopped run handed this task to the person who owns it now + -- (its own recorded hand-off). Its remaining outcomes were never + -- that person's work; once its files are saved, the hold releases. + or (task.assignee_agent_id is null and task.assignee_user_id is not null + and jsonb_typeof(stopped.result_json->'${sql.raw(NATIVE_HUMAN_SELF_HANDOFF_KEY)}') = 'object' + and stopped.result_json->'${sql.raw(NATIVE_HUMAN_SELF_HANDOFF_KEY)}'->>'issueId' = task.id::text + and stopped.result_json->'${sql.raw(NATIVE_HUMAN_SELF_HANDOFF_KEY)}'->>'assigneeUserId' = task.assignee_user_id)))`, completedWorkspaceRecoveryCondition(sourceRunId), // Copyback put the files back into the task workspace that the new owner // reuses. A snapshot-only save needs an explicit fresh-workspace request diff --git a/server/src/services/workspace-recovery-admission.test.ts b/server/src/services/workspace-recovery-admission.test.ts index 9b8ea3fd142..0017992e1b6 100644 --- a/server/src/services/workspace-recovery-admission.test.ts +++ b/server/src/services/workspace-recovery-admission.test.ts @@ -434,6 +434,36 @@ describe("automatic retained workspace recovery", () => { expect(wake.payload?.taskRetry).toMatchObject({ strategy: "continue" }); }, 30_000); + it("releases a handed-off run's recovered files hold for the person it handed the task to", async () => { + // Shape of a restart that lands after a native run handed its own task to a + // person (staging class H): the run recorded the hand-off, the person owns + // the task in review, and restart recovery settled the run as failed with + // its unsaved files retained. + const f = await seed(); + const person = "handoff-reviewer"; + await db.update(heartbeatRuns).set({ errorCode: "native_restart_recovery_blocked", + resultJson: { ...(await run(f.runId)).resultJson, humanSelfHandoff: { issueId: f.issueId, assigneeUserId: person, + recordedAt: new Date().toISOString() } } }).where(eq(heartbeatRuns.id, f.runId)); + await db.update(issues).set({ assigneeAgentId: null, assigneeUserId: person, status: "in_review" }).where(eq(issues.id, f.issueId)); + + // Until its files are saved, the outgoing run's hold stays: nothing may reuse the workspace. + expect(await getExecutionBlocker(db, f.companyId, f.issueId)).toMatchObject({ runId: f.runId, workspaceRepairRequired: true }); + + await recoverRetainedWorkspace(f.input); + expect(await hasVerifiedWorkspaceRepair(db, await run(f.runId))).toBe(true); + // Once they are saved, the person's task carries no workspace-repair hold. + expect(await getExecutionBlocker(db, f.companyId, f.issueId)).toBeNull(); + }); + + it("keeps the recovered files hold when the person did not receive the task from that run's own hand-off", async () => { + const f = await seed(); + await db.update(heartbeatRuns).set({ resultJson: { ...(await run(f.runId)).resultJson, humanSelfHandoff: { + issueId: f.issueId, assigneeUserId: "someone-else", recordedAt: new Date().toISOString() } } }).where(eq(heartbeatRuns.id, f.runId)); + await db.update(issues).set({ assigneeAgentId: null, assigneeUserId: "handoff-reviewer", status: "in_review" }).where(eq(issues.id, f.issueId)); + await recoverRetainedWorkspace(f.input); + expect(await getExecutionBlocker(db, f.companyId, f.issueId)).toMatchObject({ runId: f.runId }); + }); + it("hands a reassigned task to its new assignee once the previous owner's files are recovered", async () => { // Shape of a mid-run reassignment: the old owner's run was cancelled for // the reassignment, its copyback was retained, and its no-replay hold was