From 9ae3d8db3dc930f316f29e6e83fe6c325ac8bc06 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:00:42 -0500 Subject: [PATCH] fix: keep pre-dispatch review waits out of execution recovery (#15024) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The queued-run gate can cancel a continuation that must wait for review. > - This cancellation happens before execution authority or a provider starts. > - Recovery currently treats the gate receipt as unknown provider execution. > - That mistake blocks a conversation after a successful reply and hides Retry. > - This pull request recognizes only the recorded, unclaimed review-wait state. > - Review waits keep their normal disposition path, and new user input can recover older false holds. ## Linked Issues or Issue Description Refs #15015, #15020, and #15022. Related #11614 narrows the review posture that causes cancellation; this change corrects recovery after a valid cancellation. **What happened?** After a successful agent reply, the queued-run gate cancelled an automatic continuation with `issue_continuation_waiting_on_review`. The gate retained `timeoutSource: stale_queued_run_gate` and a matching stop reason. No execution authority or provider started. Recovery still created an unknown-action hold, moved the task to Blocked, and hid Retry. **Expected behavior** Use normal review-wait disposition repair for this recorded state. Allow Retry, a new user message, or saved undelivered input to recover an older false hold after ordinary admission checks pass. Preserve the cancelled run and do not replay its input. **Steps to reproduce** 1. Finish an agent turn on an open task that has a real review target. 2. Let the automatic continuation reach the queued-run review gate. 3. Refresh after the cancelled run is checked by recovery. 4. Confirm a review wait is handled as a wait rather than unknown provider work. 5. Reproduce an older false hold for the same receipt, then request Retry or send a new message. 6. Confirm only one fresh turn starts, and contradictory execution or cleanup evidence retains the hold. **Paperclip version or commit** Reproduced on `215586d127`. **Deployment mode** Authenticated private self-hosted server, built from source. ## What Changed - Recognize the exact review-wait dispatch receipt only while all execution claims remain unset. - Exempt recovery only after checking retained launch and provider events, coordinators, and environment cleanup. Keep the synchronous classifier conservative without that database proof. - Apply the verified classification to automatic recovery, heartbeat retries, and stranded-queue release, so saved user input starts once. - Reuse guarded startup admission for Retry, new input, and saved input on older false holds. - Show a precise review-wait notice and keep continuation guidance consistent with Retry availability. - Verify provider events, launch events, coordinators, and cleanup before admission. - Add five initial red regressions, three additional red recovery evidence regressions, concurrent saved-input coverage, and negative evidence checks. - Document the review-wait contract. ## Verification - Red: five regressions fail on unchanged master. Existing review-wait and contradictory-evidence cases still pass. - Workspace `pnpm -r typecheck` passes. - All 752 affected recovery, continuation, queue, classification, and retry-scheduling tests pass. - Three additional review regressions failed before the database proof was added; all 12 focused review-wait cases then pass. - Workspace `pnpm build` passes. - Saved-input promotion and recovery notice regressions fail before their fixes and pass afterward. - Current-head CI has 54 passing checks and 2 skipped optional Storybook checks. Greptile reviewed `0bf1437110e1617ae4544afa41f4319eac763dba` at 5/5 with zero open findings. The complete suite runs in sharded CI. No complete local monolithic pass is claimed; an earlier long run was stopped, and its unrelated failing case passed in isolation. ## Risks The error code alone cannot establish that no provider started. This exception also requires the server gate receipt, matching stop reason, and null execution authority fields. User admission separately checks coordinator, launch and provider events, environment cleanup, pending decisions, ownership, task holds, budget, and active execution. No migration or dependency change. ## Model Used OpenAI Codex, an agent based on GPT-6. The exact runtime model ID and context window are not exposed in this session. Used reasoning, repository tools, code execution, and browser inspection. ## 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 - [x] All Paperclip CI gates are green - [x] 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 | 2 + .../heartbeat-process-recovery.test.ts | 26 ++++++- .../wake-queue/adapters/postgres.test.ts | 40 +++++++++++ .../modules/wake-queue/adapters/postgres.ts | 4 +- .../src/services/cancelled-native-startup.ts | 6 +- server/src/services/execution-blocker.ts | 11 +-- .../explicit-native-continuation.test.ts | 67 +++++++++++++++++++ server/src/services/heartbeat.ts | 14 ++-- .../legacy-execution-recovery.test.ts | 21 ++++++ .../src/services/legacy-execution-recovery.ts | 7 ++ .../src/services/pre-dispatch-review-wait.ts | 51 ++++++++++++++ server/src/services/recovery/service.ts | 6 +- 12 files changed, 235 insertions(+), 20 deletions(-) create mode 100644 server/src/services/pre-dispatch-review-wait.ts diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 72a0cb98324..46b9e87e7f0 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -488,6 +488,8 @@ A new user message can continue a terminal native run whose process fields were The task thread exposes the guarded Retry action for failed or timed-out conversation runs and native preparation cancelled before provider startup when the never-started proof is verified. The server projects this eligibility on the recovery notice and rechecks it on Retry. Cleanup quarantine retains its inspection path, and non-conversation reconciliation gates remain enforced. Pending decisions, active execution, pause, budget, dependency, and ownership gates remain in force. A refused Retry reports its reason inline and remains available for another attempt. +A continuation cancelled by the queued-run gate while waiting for review did not start a provider. Its matching dispatch-gate receipt and unclaimed execution fields keep this deliberate wait out of unknown-action reconciliation. Pending review and decision gates still apply. If an older server already created a recovery hold for this exact state, Retry or a new user message can request one fresh turn after the server verifies there is no provider, coordinator, or unfinished cleanup. Saved user input is reconsidered through the same admission path; the cancelled turn is not replayed. + A valid recovery action must name: - the source issue and company diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 07bef2d6d8c..d88dac7d329 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -6221,14 +6221,28 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]).toMatchObject({ status: "blocked", assigneeAgentId: agentId }); }); - it("converts a continuation parked for review into a dependency wait on its open sub-tasks", async () => { - const { companyId, agentId, issueId } = await seedStrandedIssueFixture({ + it.each([false, "receipt", "launch", "provider", "cleanup"])("converts a continuation parked for review into a dependency wait on its open sub-tasks (dispatch receipt: %s)", async dispatchReceipt => { + const { companyId, agentId, issueId, runId } = await seedStrandedIssueFixture({ status: "in_progress", runStatus: "cancelled", retryReason: "issue_continuation_needed", runErrorCode: "issue_continuation_waiting_on_review", runError: "Continuation parked: issue is waiting on review/approval", }); + if (dispatchReceipt) { + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, companyId)); + await db.update(heartbeatRuns).set({ startedAt: null, + resultJson: { stopReason: "issue_continuation_waiting_on_review", timeoutSource: "stale_queued_run_gate" }, + }).where(eq(heartbeatRuns.id, run.id)); + } + if (dispatchReceipt === "launch" || dispatchReceipt === "provider") { + await db.insert(heartbeatRunEvents).values({ companyId, runId, agentId, seq: 1, + eventType: dispatchReceipt === "launch" ? "native.process_start_requested" : "provider.event", + stream: "system", level: "info", message: "Contradictory execution evidence" }); + } else if (dispatchReceipt === "cleanup") { + await db.insert(environmentLeases).values({ companyId, heartbeatRunId: runId, + provider: "local", status: "pending_cleanup", cleanupStatus: "failed" }); + } const issuePrefix = `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`; const openChildTodoId = randomUUID(); const openChildInProgressId = randomUUID(); @@ -6270,6 +6284,14 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { const heartbeat = heartbeatService(db); const result = await heartbeat.reconcileStrandedAssignedIssues(); + if (dispatchReceipt && dispatchReceipt !== "receipt") { + expect(result.waitingOnReviewResolved).toBe(0); + expect(result.escalated).toBe(1); + expect(await getExecutionBlocker(db, companyId, issueId)).toMatchObject({ + cause: "legacy_execution_requires_reconciliation", runId, canRetry: false, + }); + return; + } expect(result.waitingOnReviewResolved).toBe(1); expect(result.escalated).toBe(0); expect(result.issueIds).toEqual([issueId]); diff --git a/server/src/modules/wake-queue/adapters/postgres.test.ts b/server/src/modules/wake-queue/adapters/postgres.test.ts index 812cdec546b..9bc674d5fac 100644 --- a/server/src/modules/wake-queue/adapters/postgres.test.ts +++ b/server/src/modules/wake-queue/adapters/postgres.test.ts @@ -9,6 +9,8 @@ import { agents, companies, createDb, + environmentLeases, + heartbeatRunEvents, heartbeatRuns, issueComments, issueRecoveryActions, @@ -59,6 +61,8 @@ describeEmbeddedPostgres("wake-queue postgres adapter", () => { afterEach(async () => { await db.delete(activityLog); await db.delete(issueComments); + await db.delete(heartbeatRunEvents); + await db.delete(environmentLeases); // `heartbeat_runs.wakeup_request_id` references `agent_wakeup_requests.id`, // so the run row must go first. await db.delete(heartbeatRuns); @@ -416,6 +420,42 @@ describeEmbeddedPostgres("wake-queue postgres adapter", () => { expect(wake.status).toBe("deferred_issue_execution"); }); + it.each(["clean", "launch", "cleanup"])("promotes saved input only after a verified pre-dispatch review wait (%s)", async evidence => { + const companyId = await seedCompany(), agentId = await seedAgent({ companyId }); + const issueId = await seedIssue({ companyId, assigneeAgentId: agentId }); + const runId = await seedRun({ companyId, agentId, status: "cancelled", contextSnapshot: { issueId }, + errorCode: "issue_continuation_waiting_on_review" }); + await db.update(heartbeatRuns).set({ finishedAt: new Date(Date.now() - 1000), resultJson: { + stopReason: "issue_continuation_waiting_on_review", timeoutSource: "stale_queued_run_gate", + } }).where(eq(heartbeatRuns.id, runId)); + if (evidence === "launch") await db.insert(heartbeatRunEvents).values({ companyId, runId, agentId, seq: 1, + eventType: "native.process_start_requested", stream: "system", level: "info" }); + if (evidence === "cleanup") await db.insert(environmentLeases).values({ companyId, heartbeatRunId: runId, + provider: "local", status: "pending_cleanup", cleanupStatus: "failed" }); + const [comment] = await db.insert(issueComments).values({ companyId, issueId, authorType: "user", + authorUserId: "responsible-user", body: "Continue with this new message." }).returning(); + const wakeId = await seedDeferredWake({ companyId, agentId, issueId, requestedByActorId: "responsible-user", + payload: { commentId: comment.id, _paperclipWakeContext: { issueId, wakeReason: "issue_commented", wakeCommentIds: [comment.id] } } }); + const release = createReleaseIssueExecution({ issueLock: createPostgresWakeQueueAdapter(db, stubDeps), + recovery: { escalateStrandedAssignedIssue: async () => {}, escalateStrandedRecoveryIssueInPlace: async () => {} } }); + const result = await release({ companyId, runId, now: new Date() }); + const runs = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, companyId)); + const [wake] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, wakeId)); + if (evidence !== "clean") { + expect(result.outcome.kind).toBe("released"); + expect(runs).toHaveLength(1); + expect(wake).toMatchObject({ status: "deferred_issue_execution", runId: null }); + return; + } + expect(result.outcome.kind).toBe("promoted"); + expect(runs).toHaveLength(2); + const successor = runs.find(run => run.status === "queued")!; + expect(wake.runId).toBe(successor.id); + expect(successor.contextSnapshot).toMatchObject({ wakeCommentIds: [comment.id] }); + await release({ companyId, runId, now: new Date() }); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, companyId))).toHaveLength(2); + }); + it("leaves deferred work untouched until the effective execution hold clears", async () => { const companyId = await seedCompany(); const agentId = await seedAgent({ companyId }); diff --git a/server/src/modules/wake-queue/adapters/postgres.ts b/server/src/modules/wake-queue/adapters/postgres.ts index 60b0e359b79..57d256e70be 100644 --- a/server/src/modules/wake-queue/adapters/postgres.ts +++ b/server/src/modules/wake-queue/adapters/postgres.ts @@ -19,7 +19,7 @@ import { nativeRunFinalizations, } from "@paperclipai/db"; import { hasConversationContinuationPolicy } from "../../../services/conversation-continuation.js"; -import { legacyExecutionNeedsReconciliation } from "../../../services/legacy-execution-recovery.js"; +import { legacyExecutionNeedsReconciliationWithEvidence } from "../../../services/legacy-execution-recovery.js"; import { authorizeFailedChatRunRetryWake, FailedChatRunRetryAuthorizationError, @@ -1104,7 +1104,7 @@ export function createPostgresWakeQueueAdapter(db: Db, deps: WakeQueuePostgresAd issueStatus: issueRow?.status ?? "", hasAssigneeUser: Boolean(issueRow?.assigneeUserId), assigneeAgentMatchesRunAgent: issueRow?.assigneeAgentId === run.agentId, - legacyExecutionNeedsReconciliation: legacyExecutionNeedsReconciliation(run), + legacyExecutionNeedsReconciliation: await legacyExecutionNeedsReconciliationWithEvidence(tx as unknown as Db, run), // An operator stop never promotes old queued work by itself. The // next explicit wake adopts those messages atomically when it // queues a run. diff --git a/server/src/services/cancelled-native-startup.ts b/server/src/services/cancelled-native-startup.ts index 9bcadce31cf..53a17527f48 100644 --- a/server/src/services/cancelled-native-startup.ts +++ b/server/src/services/cancelled-native-startup.ts @@ -1,3 +1,4 @@ +import { isPreDispatchReviewWait } from "./pre-dispatch-review-wait.js"; import { and, eq, inArray, isNotNull, or } from "drizzle-orm"; import { environmentLeases, heartbeatRunEvents, heartbeatRuns, nativeRunFinalizations, type Db } from "@paperclipai/db"; import { claimedAdapterType } from "./conversation-continuation.js"; @@ -38,7 +39,8 @@ export async function isCancelledNativeStartup(db: Db, run: Run, coordinator: Co (run.resultJson === null || cancellation?.beforeNativeSelection === true) && Boolean(run.controllerBootId && run.controllerBootId !== legacyControllerBootId && run.controllerLeaseExpiresAt && run.controllerLeaseExpiresAt <= new Date()); - const beforeSelection = historicalBeforeSelection || run.runtimeMode === "legacy" && !run.runtimeModeResolvedAt && + const beforeReviewDispatch = isPreDispatchReviewWait(run) && !coordinator; + const beforeSelection = beforeReviewDispatch || historicalBeforeSelection || run.runtimeMode === "legacy" && !run.runtimeModeResolvedAt && !run.nativeSessionId && !coordinator && claimedAdapterType(run) === "paperclip_runner" && cancellation?.beforeNativeSelection === true; const neverClaimed = run.runtimeMode === "native" && coordinator && @@ -47,7 +49,7 @@ export async function isCancelledNativeStartup(db: Db, run: Run, coordinator: Co !coordinator.controllerPid && !coordinator.leaseOwner && !coordinator.leaseExpiresAt && !coordinator.resultId && !coordinator.failureDetail?.successorRunId; if (!beforeSelection && !neverClaimed) return false; - const settled = typeof run.resultJson?.startupPreparationSettledAt === "string"; + const settled = beforeReviewDispatch || typeof run.resultJson?.startupPreparationSettledAt === "string"; // The old preparer can still be unwinding even though the run is terminal. if (!settled && run.controllerLeaseExpiresAt && run.controllerLeaseExpiresAt > new Date()) return false; const leases = await db.select().from(environmentLeases).where(and( diff --git a/server/src/services/execution-blocker.ts b/server/src/services/execution-blocker.ts index 2d70f70add9..14c316b5a00 100644 --- a/server/src/services/execution-blocker.ts +++ b/server/src/services/execution-blocker.ts @@ -1,3 +1,4 @@ +import { isPreDispatchReviewWait } from "./pre-dispatch-review-wait.js"; import { and, desc, eq, gt, inArray, not, or, sql } from "drizzle-orm"; import { conversationRecoveryActionPredicate, getConversationOwnershipBlocker } from "./conversation-continuation.js"; import { z } from "zod"; @@ -79,13 +80,15 @@ export async function getExecutionBlocker(db: Db, companyId: string, issueId: st or consumed.context_snapshot->'wakeCommentIds' @> jsonb_build_array(${issueComments.id}::text)))`, )) : []; const cancellation = run ? readRunCancellation(run.resultJson) : null; - const runError = cancellation?.reason ?? (run?.status === "cancelled" - ? "Execution was cancelled; its source was not recorded." - : run?.error); const eligibleContinuation = Boolean(run && conversation?.assigneeAgentId === run.agentId && !["done", "cancelled"].includes(conversation.status) && canContinueCancelledRun(run)); const canRetry = Boolean(run && conversation?.assigneeAgentId === run.agentId && !["done", "cancelled"].includes(conversation.status) && await canRetryStoppedRun(db, run)); + const runError = run && canRetry && isPreDispatchReviewWait(run) + ? "Waiting for review; this continuation never started." + : cancellation?.reason ?? (run?.status === "cancelled" + ? "Execution was cancelled; its source was not recorded." + : run?.error); const [chatBinding] = eligibleContinuation || canRetry ? await db.select({ state: chatConversations.state, endpointStatus: chatEndpoints.status, connectionStatus: toolConnections.status, connectionEnabled: toolConnections.enabled, @@ -107,7 +110,7 @@ export async function getExecutionBlocker(db: Db, companyId: string, issueId: st } else { nextAction = "Send a new chat message to continue this conversation."; } - } else if (run?.status === "cancelled" && !eligibleContinuation && action.cause === "legacy_execution_requires_reconciliation") { + } else if (run?.status === "cancelled" && !eligibleContinuation && !canRetry && action.cause === "legacy_execution_requires_reconciliation") { nextAction += " Inspect the run before sending a new message to request continuation."; } diff --git a/server/src/services/explicit-native-continuation.test.ts b/server/src/services/explicit-native-continuation.test.ts index 9ddb4a898be..cd0d3ac143a 100644 --- a/server/src/services/explicit-native-continuation.test.ts +++ b/server/src/services/explicit-native-continuation.test.ts @@ -613,6 +613,73 @@ const support = await getEmbeddedPostgresTestSupport(); return f; } + async function seedCancelledReviewWait() { + const f = await seedHistoricalCancelledPreparation(); + await db.update(heartbeatRuns).set({ startedAt: null, executionStage: null, + controllerBootId: null, controllerLeaseExpiresAt: null, runnerProfileJson: null, + errorCode: "issue_continuation_waiting_on_review", + resultJson: { stopReason: "issue_continuation_waiting_on_review", timeoutSource: "stale_queued_run_gate" }, + }).where(eq(heartbeatRuns.id, f.sourceRunId)); + return f; + } + + it.each(["message", "retry"])("recovers a pre-dispatch review wait through an explicit %s", async kind => { + const f = await seedCancelledReviewWait(); + const notice = await getExecutionBlocker(db, f.companyId, f.issueId); + expect(notice).toMatchObject({ canRetry: true, runError: "Waiting for review; this continuation never started." }); + expect(notice?.nextAction).not.toContain("Inspect the run before sending a new message"); + await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, status: "running" }); + const successor = await heartbeatService(db).wakeup(f.agentId, { source: kind === "retry" ? "on_demand" : "automation", triggerDetail: "manual", + reason: kind === "retry" ? "retry_failed_run" : "issue_commented", + ...(kind === "retry" ? { failedRunId: f.sourceRunId } : {}), requestedByActorType: "user", requestedByActorId: "board", + payload: { issueId: f.issueId, ...(kind === "message" ? { commentId: f.commentId } : {}) }, + contextSnapshot: { issueId: f.issueId, ...(kind === "message" ? { wakeCommentId: f.commentId } : {}) }, + }); + expect(successor).toMatchObject({ status: "queued", contextSnapshot: { forceFreshSession: true, + previousRunId: f.sourceRunId, explicitUserContinuation: { commentId: kind === "message" ? f.commentId : null } } }); + expect(await getExecutionBlocker(db, f.companyId, f.issueId)).toBeNull(); + expect((await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.sourceRunId)))[0].status).toBe("cancelled"); + }); + + it("reconsiders saved input after a pre-dispatch review wait exactly once", async () => { + const f = await seedCancelledReviewWait(); + await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, status: "running" }); + const leaseId = randomUUID(); + await db.insert(environmentLeases).values({ id: leaseId, companyId: f.companyId, + heartbeatRunId: f.sourceRunId, provider: "local", status: "pending_cleanup", cleanupStatus: "failed" }); + await heartbeatService(db).wakeup(f.agentId, { source: "automation", reason: "issue_commented", + requestedByActorType: "user", requestedByActorId: "board", payload: { issueId: f.issueId, commentId: f.commentId }, + contextSnapshot: { issueId: f.issueId, wakeCommentId: f.commentId } }); + const [waiting] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, f.companyId)); + expect(waiting.status).toBe("deferred_issue_execution"); + await db.update(environmentLeases).set({ status: "released", releasedAt: new Date(), cleanupStatus: "succeeded" }) + .where(eq(environmentLeases.id, leaseId)); + await db.update(agentWakeupRequests).set({ updatedAt: new Date(0) }).where(eq(agentWakeupRequests.id, waiting.id)); + await Promise.all([heartbeatService(db).resumeExecutionWaitComments(), heartbeatService(db).resumeExecutionWaitComments()]); + const successors = await db.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.companyId, f.companyId), eq(heartbeatRuns.status, "queued"))); + expect(successors).toHaveLength(1); + expect(successors[0].contextSnapshot).toMatchObject({ wakeCommentIds: [f.commentId], + explicitUserContinuation: { commentId: f.commentId }, forceFreshSession: true }); + expect((await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, waiting.id)))[0]) + .toMatchObject({ status: "coalesced", runId: successors[0].id }); + expect(await getExecutionBlocker(db, f.companyId, f.issueId)).toBeNull(); + }); + + it.each(["launch", "provider", "cleanup"])("retains a pre-dispatch review wait with contradictory %s evidence", async kind => { + const f = await seedCancelledReviewWait(); + if (kind === "cleanup") await db.insert(environmentLeases).values({ companyId: f.companyId, + heartbeatRunId: f.sourceRunId, provider: "local", status: "pending_cleanup", cleanupStatus: "failed" }); + else await db.insert(heartbeatRunEvents).values({ companyId: f.companyId, runId: f.sourceRunId, + agentId: f.agentId, seq: 1, eventType: kind === "launch" ? PROCESS_START_REQUESTED : "provider.event", + stream: "system", level: "info", message: "Contradictory execution evidence" }); + try { + expect(await getExecutionBlocker(db, f.companyId, f.issueId)).toMatchObject({ canRetry: false }); + expect(await admit(f)).toBeNull(); + } finally { + await db.delete(environmentLeases).where(eq(environmentLeases.heartbeatRunId, f.sourceRunId)); + } + }); + it.each([false, true].flatMap(receipt => ["message", "retry"].map(kind => ({ receipt, kind }))))( "recovers historical native preparation through an explicit $kind (receipt: $receipt)", async ({ receipt, kind }) => { const f = await seedHistoricalCancelledPreparation(receipt); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 88f73ea4c6d..9ca39eb35cd 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -44,7 +44,7 @@ import { getNativeReviewAssignment, readNativeReviewAssignmentContext } from "./ import { claimQueuedNativeReviewRun } from "./native-runtime/native-review-dispatch.js"; import { buildNativeReviewRequest } from "./native-runtime/native-review-prompt.js"; import { - legacyExecutionNeedsReconciliation, + legacyExecutionNeedsReconciliationWithEvidence, settleInterruptedNativeBootstrap, terminalizeLegacyExecution, } from "./legacy-execution-recovery.js"; @@ -11771,7 +11771,7 @@ export function heartbeatService( "Native execution recovery owns this provider failure; a quota monitor cannot start a replacement.", ); } - if (legacyExecutionNeedsReconciliation(sourceRun)) { + if (await legacyExecutionNeedsReconciliationWithEvidence(db, sourceRun)) { await terminalizeLegacyExecution({ db, run: sourceRun, @@ -12912,7 +12912,7 @@ export function heartbeatService( } const updated = previousStatus && - legacyExecutionNeedsReconciliation({ + await legacyExecutionNeedsReconciliationWithEvidence(db, { ...previousStatus, status, ...patch, @@ -13002,7 +13002,7 @@ export function heartbeatService( } const updated = previousStatus && - legacyExecutionNeedsReconciliation({ + await legacyExecutionNeedsReconciliationWithEvidence(db, { ...previousStatus, status, ...patch, @@ -14519,7 +14519,7 @@ export function heartbeatService( // Native sessions have their own fenced same-run controller. Legacy // bootstrap recovery shares the durable delay and incident counter with // transient retries; process loss must not open a second retry budget. - if (run.runtimeMode === "native" || legacyExecutionNeedsReconciliation(run)) + if (run.runtimeMode === "native" || await legacyExecutionNeedsReconciliationWithEvidence(db, run)) return null; const scheduled = await scheduleBoundedRetryForRun(run, agent, { now }); return scheduled.outcome === "scheduled" ? scheduled.run : null; @@ -15494,7 +15494,7 @@ export function heartbeatService( }; } - if (legacyExecutionNeedsReconciliation(run)) { + if (await legacyExecutionNeedsReconciliationWithEvidence(db, run)) { return { outcome: "not_scheduled" as const, reason: @@ -25812,7 +25812,7 @@ export function heartbeatService( await scheduleBoundedRetryForRun(livenessRun, agent); } else if ( outcome === "failed" && - !legacyExecutionNeedsReconciliation(livenessRun) + !(await legacyExecutionNeedsReconciliationWithEvidence(db, livenessRun)) ) { await scheduleInteractionContinuationInfrastructureRetryIfEligible( livenessRun, diff --git a/server/src/services/legacy-execution-recovery.test.ts b/server/src/services/legacy-execution-recovery.test.ts index 52397bf506c..b1563e07d1d 100644 --- a/server/src/services/legacy-execution-recovery.test.ts +++ b/server/src/services/legacy-execution-recovery.test.ts @@ -1,3 +1,4 @@ +import { isPreDispatchReviewWait } from "./pre-dispatch-review-wait.js"; import { expect, it } from "vitest"; import { legacyExecutionNeedsReconciliation } from "./legacy-execution-recovery.js"; @@ -86,3 +87,23 @@ it("retains conversation retry eligibility for a transient restore lock timeout" workspaceRestoreFailure: "restore_lock_timeout", conversationContinuation: "continue_conversation_v1", } })).toBe(false); }); + + +it("recognizes only an unclaimed pre-dispatch review wait receipt", () => { + const run = { runtimeMode: "legacy", status: "cancelled", errorCode: "issue_continuation_waiting_on_review", + startedAt: null, runtimeModeResolvedAt: null, processPid: null, processGroupId: null, + processStartedAt: null, nativeIssueId: null, nativeSessionId: null, sessionIdAfter: null, + controllerBootId: null, controllerLeaseExpiresAt: null, executionStage: null, + resultJson: { stopReason: "issue_continuation_waiting_on_review", timeoutSource: "stale_queued_run_gate" } }; + expect(isPreDispatchReviewWait(run)).toBe(true); + expect(legacyExecutionNeedsReconciliation(run)).toBe(true); + for (const patch of [ + { startedAt: new Date() }, { processPid: 123 }, { processGroupId: 123 }, + { processStartedAt: new Date() }, { nativeSessionId: "provider-session" }, { nativeIssueId: "native-task" }, + { sessionIdAfter: "provider-session" }, { runtimeModeResolvedAt: new Date() }, + { controllerBootId: "live-controller" }, { controllerLeaseExpiresAt: new Date() }, { executionStage: "preparing" }, + { errorCode: "operator_interrupted" }, { resultJson: {} }, + { resultJson: { ...run.resultJson, stopReason: "cancelled" } }, + { resultJson: { ...run.resultJson, workspaceRestoreFailure: "restore_unsafe_archive" } }, + ]) expect(isPreDispatchReviewWait({ ...run, ...patch })).toBe(false); +}); diff --git a/server/src/services/legacy-execution-recovery.ts b/server/src/services/legacy-execution-recovery.ts index 98c4c03f7e5..6ab86f7ee43 100644 --- a/server/src/services/legacy-execution-recovery.ts +++ b/server/src/services/legacy-execution-recovery.ts @@ -1,3 +1,4 @@ +import { isPreDispatchReviewWaitVerified } from "./pre-dispatch-review-wait.js"; import { hasWorkspaceRestoreFailure } from "@paperclipai/shared"; import { normalizeMaxTurnStopReason } from "./heartbeat-stop-metadata.js"; import { claimedAdapterType, hasConversationContinuationPolicy } from "./conversation-continuation.js"; @@ -53,6 +54,12 @@ export function legacyExecutionNeedsReconciliation( ); } +/** Review-wait receipts are only exempt after retained execution evidence agrees. + * The synchronous classifier stays conservative for callers without a DB proof. */ +export async function legacyExecutionNeedsReconciliationWithEvidence(db: Db, run: Run): Promise { + return legacyExecutionNeedsReconciliation(run) && !(await isPreDispatchReviewWaitVerified(db, run)); +} + /** Persist the failed legacy run, owned lock release and operator decision together. */ export async function terminalizeLegacyExecution(input: { db: Db; diff --git a/server/src/services/pre-dispatch-review-wait.ts b/server/src/services/pre-dispatch-review-wait.ts new file mode 100644 index 00000000000..08ea49b498e --- /dev/null +++ b/server/src/services/pre-dispatch-review-wait.ts @@ -0,0 +1,51 @@ +import { and, eq, inArray, isNotNull, or } from "drizzle-orm"; +import { environmentLeases, heartbeatRunEvents, heartbeatRuns, nativeRunFinalizations, type Db } from "@paperclipai/db"; +import { PROCESS_IDENTITY_RECORDED, PROCESS_START_REQUESTED } from "./native-local-process-stop.js"; +import { hasRemoteTerminationReceipt } from "./remote-execution-termination.js"; + +type Run = typeof heartbeatRuns.$inferSelect; +type DispatchFields = "startedAt" | "runtimeModeResolvedAt" | "processPid" | "processGroupId" | + "processStartedAt" | "nativeIssueId" | "nativeSessionId" | "sessionIdAfter" | + "controllerBootId" | "controllerLeaseExpiresAt" | "executionStage"; + +/** The queued-run gate owns this receipt before execution authority is claimed. + * A deliberate review wait has no provider actions to reconcile. The error code + * alone is not proof; missing or conflicting dispatch evidence retains the hold. */ +export function isPreDispatchReviewWait( + run: Pick & Partial>, +): boolean { + return run.runtimeMode === "legacy" && run.status === "cancelled" && + run.errorCode === "issue_continuation_waiting_on_review" && + run.resultJson?.stopReason === run.errorCode && + run.resultJson?.timeoutSource === "stale_queued_run_gate" && + run.resultJson?.workspaceRestoreFailure !== "restore_unsafe_archive" && + run.startedAt === null && run.runtimeModeResolvedAt === null && + run.processPid === null && run.processGroupId === null && run.processStartedAt === null && + run.nativeIssueId === null && run.nativeSessionId === null && run.sessionIdAfter === null && + run.controllerBootId === null && run.controllerLeaseExpiresAt === null && run.executionStage === null; +} + +/** The row receipt is a candidate, not permission to discard conflicting + * retained execution or cleanup evidence. Historical false holds must be + * checked against the same evidence that gates explicit user admission. */ +export async function isPreDispatchReviewWaitVerified(db: Db, run: Run): Promise { + if (!isPreDispatchReviewWait(run)) return false; + const [coordinator] = await db.select({ id: nativeRunFinalizations.runId }).from(nativeRunFinalizations).where(and( + eq(nativeRunFinalizations.companyId, run.companyId), eq(nativeRunFinalizations.runId, run.id), + )).limit(1); + if (coordinator) return false; + const leases = await db.select().from(environmentLeases).where(and( + eq(environmentLeases.companyId, run.companyId), eq(environmentLeases.heartbeatRunId, run.id), + )); + if (leases.some(lease => lease.provider === "local" + ? !lease.releasedAt || lease.status === "pending_cleanup" || lease.cleanupStatus === "failed" + : !hasRemoteTerminationReceipt(lease))) return false; + const [execution] = await db.select({ id: heartbeatRunEvents.id }).from(heartbeatRunEvents).where(and( + eq(heartbeatRunEvents.companyId, run.companyId), eq(heartbeatRunEvents.runId, run.id), + or(isNotNull(heartbeatRunEvents.sourceEventId), + inArray(heartbeatRunEvents.eventType, ["adapter.invoke", PROCESS_START_REQUESTED, PROCESS_IDENTITY_RECORDED, + "harness.ready", "session.started", "session.resumed", "session.updated", "turn.started", + "provider.event", "provider.rpc_result", "tool.execution.started"])), + )).limit(1); + return !execution; +} diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 113ea5ce1cd..99ed604dee8 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -89,7 +89,7 @@ import { budgetService } from "../budgets.js"; import { unadmittedChatWakeupCondition } from "../durable-chat-wakeup.js"; import { issueRecoveryActionService } from "../issue-recovery-actions.js"; import { - legacyExecutionNeedsReconciliation, + legacyExecutionNeedsReconciliationWithEvidence, terminalizeLegacyExecution, } from "../legacy-execution-recovery.js"; import { issueTreeControlService } from "../issue-tree-control.js"; @@ -1923,7 +1923,7 @@ export function recoveryService( // Failure recovery shares the durable incident budget and delay. It // cannot fall through into the productive-work continuation queue. if (predecessor.runtimeMode === "native") return null; - if (legacyExecutionNeedsReconciliation(predecessor)) { + if (await legacyExecutionNeedsReconciliationWithEvidence(db, predecessor)) { await terminalizeLegacyExecution({ db, run: predecessor, @@ -4603,7 +4603,7 @@ export function recoveryService( eq(heartbeatRuns.id, executionRecoverySource.id), ), ); - if (source && legacyExecutionNeedsReconciliation(source)) { + if (source && await legacyExecutionNeedsReconciliationWithEvidence(db, source)) { await terminalizeLegacyExecution({ db, run: source,