mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
fix(recovery): release a handed-off run's saved-files hold for the person it handed the task to (#20941)
## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
0bf3236bc8
commit
b01959cb15
3 files changed
+51
-9
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user