diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 3ff8e8cb97..8c10ed7672 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -434,6 +434,11 @@ cancellation retain their existing authorities. Immediate recovery honors the same operator-cancellation attribution as periodic recovery, so cancelling a run does not synthesize a continuation that restarts its sandbox. Explicitly queued work can still run through normal promotion. +Native failure recovery also checks the durable cancellation intent under the run +lock before scheduling a retry. An interrupted turn without a semantic result +must preserve cancellation instead of reporting a provider failure. Terminal +cancellation clears stale retry retention flags so final flushing and sandbox +release still run. Automated tests do not qualify a deployed runner image. Before merging, use a new pinned staging stack with the branch's Cloud image and matching migrator. diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index a83ecc2eb4..5609104621 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -21802,6 +21802,7 @@ export function heartbeatService( } } } catch (adapterErr) { + if (adapterErr instanceof NativeCancellationPendingRecoveryError) throw adapterErr; const nativeResumeScheduled = nativeRuntimeResolution.kind === "native" ? await measureSandboxOperation("heartbeat.db.select.from.where.limit.then", { operationIndex: 158 }, async () => (db @@ -22994,6 +22995,12 @@ export function heartbeatService( catch (error) { workFolderSaveFailed = true; logger.error({ err: error, runId: run.id }, "Work folder save failed; retaining sandbox for recovery"); } } let latestRun = await measureSandboxOperation("heartbeat.get_run.catch", { operationIndex: 246 }, async () => (getRun(run.id).catch(() => null))); + // Cancellation can arrive after the executor scheduled a same-run retry. + // Once terminal, that stale local flag must not retain its provider lease. + if (latestRun?.status === "cancelled") { + nativeSessionResumeScheduled = false; + nativeWorkspaceFinalizeScheduled = false; + } // Trace capture is debug-only and must settle independently of every // provider outcome. Adapter/setup failures used to skip the success-path // finalizer, leaving metadata permanently stuck at `capturing` even when diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 02244a6705..9cc816d7e1 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -2534,6 +2534,7 @@ function leaseDb( boundExecution: NativeExecutionInputV1 = execution, coordinatorOverrides: Partial = {}, runResultJson: Record = {}, + writes: Array<{ table: unknown; values: Record }> = [], ): Db { const coordinator: LeaseCoordinator = { runId: boundExecution.binding.runId, @@ -2546,17 +2547,20 @@ function leaseDb( resultId: null, ...coordinatorOverrides, }; - const update = () => ({ - set: () => ({ - where: () => { - const result = Promise.resolve([]) as unknown as Promise & { - returning: () => Promise>; - }; - result.returning = () => - Promise.resolve([{ runId: coordinator.runId }]); - return result; - }, - }), + const update = (table: unknown) => ({ + set: (values: Record) => { + writes.push({ table, values }); + return { + where: () => { + const result = Promise.resolve([]) as unknown as Promise & { + returning: () => Promise>; + }; + result.returning = () => + Promise.resolve([{ runId: coordinator.runId }]); + return result; + }, + }; + }, }); const tx = { select: () => ({ @@ -2585,6 +2589,13 @@ function leaseDb( transaction: async (operation: (transaction: Db) => Promise) => operation(tx as unknown as Db), update, + select: () => ({ + from: (table: unknown) => ({ where: () => ({ limit: async () => + table === heartbeatRuns + ? [{ runnerProfileJson: { sessionCheckpoint: { providerSessionId: "provider" } } }] + : [], + }) }), + }), } as unknown as Db; } @@ -2742,6 +2753,42 @@ describe("native session cancellation", () => { ).resolves.toBe(false); }); + it.each(["pending", "acknowledged"])( + "does not schedule recovery when cancellation becomes %s during a provider turn", + async (dispatchState) => { + const resultJson: Record = {}; + const writes: Array<{ table: unknown; values: Record }> = []; + state.execute.mockImplementationOnce(async (options) => { + options.onSession?.({ cancel: state.cancel }); + // The claim saw no cancellation. The durable intent arrives while the + // provider is running, before its interruption surfaces as a failure. + resultJson.nativeCancellation = { + schema: "paperclip.native-cancellation.v1", + scope: "run", + companyId: execution.binding.companyId, + runId: execution.binding.runId, + issueId: execution.binding.issueId, + dispatchState, + }; + options.onSession?.(null); + throw new Error("native_finalization_missing: session returned no semantic result"); + }); + const failure = await executePaperclipNativeSession({ + db: leaseDb(execution, {}, resultJson, writes), + execution, + runnerInstanceId: "runner", + }).catch((error: unknown) => error); + expect(writes.some(({ values }) => values.phase === "retryable_failure")).toBe(false); + expect(writes.some(({ values }) => values.errorCode === "native_session_interrupted")).toBe(false); + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toBe("native_cancellation_pending_recovery"); + expect(writes).toContainEqual({ + table: nativeRunFinalizations, + values: expect.objectContaining({ leaseOwner: null, leaseExpiresAt: null, nextAttemptAt: null }), + }); + }, + ); + it("allows cancellation to be retried when the session dispatch fails", async () => { state.cancel.mockImplementationOnce(() => { throw new Error("transport unavailable"); diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index c2be96961a..456ca1f997 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -4725,7 +4725,63 @@ async function executePaperclipNativeSessionWithinScope( ? error.message.slice(0, 2_000) : String(error).slice(0, 2_000); const sanitizedStderrTail = redactSensitiveText(message).slice(-4_096); - await input.db.transaction(async (tx) => { + const cancellationWon = await input.db.transaction(async (tx) => { + // Match the execution claim's coordinator -> run lock order. Cancellation + // publishes its intent under the run lock before interrupting the provider. + // A result-less interrupted turn must not overwrite that intent's outcome + // or create recovery work that keeps its sandbox running. + await tx + .select({ runId: nativeRunFinalizations.runId }) + .from(nativeRunFinalizations) + .where(eq(nativeRunFinalizations.runId, input.execution.binding.runId)) + .for("update") + .limit(1); + const boundRun = await tx + .select({ + companyId: heartbeatRuns.companyId, + agentId: heartbeatRuns.agentId, + nativeIssueId: heartbeatRuns.nativeIssueId, + resultJson: heartbeatRuns.resultJson, + }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, input.execution.binding.runId)) + .for("update") + .limit(1) + .then((rows) => rows[0] ?? null); + const cancellation = record(record(boundRun?.resultJson).nativeCancellation); + if ( + cancellation.scope === "run" && + (cancellation.dispatchState === "pending" || cancellation.dispatchState === "acknowledged") + ) { + if ( + boundRun?.companyId !== input.execution.binding.companyId || + boundRun?.agentId !== input.execution.binding.agentId || + boundRun?.nativeIssueId !== input.execution.binding.issueId || + cancellation.schema !== "paperclip.native-cancellation.v1" || + cancellation.companyId !== input.execution.binding.companyId || + cancellation.runId !== input.execution.binding.runId || + cancellation.issueId !== input.execution.binding.issueId + ) { + throw new Error("native_cancellation_intent_conflict"); + } + await tx + .update(nativeRunFinalizations) + .set({ + leaseOwner: null, + leaseExpiresAt: null, + nextAttemptAt: null, + recoveryState: null, + updatedAt: now, + }) + .where(and( + eq(nativeRunFinalizations.runId, input.execution.binding.runId), + eq(nativeRunFinalizations.companyId, input.execution.binding.companyId), + eq(nativeRunFinalizations.issueId, input.execution.binding.issueId), + eq(nativeRunFinalizations.leaseOwner, leaseOwner), + eq(nativeRunFinalizations.attempt, attempt), + )); + return true; + } const updated = await tx .update(nativeRunFinalizations) .set({ @@ -4876,6 +4932,11 @@ async function executePaperclipNativeSessionWithinScope( supersedeOnIdentityChange: recoveryProjection.supersedeOnIdentityChange, }); }); + if (cancellationWon) { + if (taskSettleScope) await trace.end(taskSettleScope, { outcome: "ok" }); + await trace.finish("ok"); + throw new NativeCancellationPendingRecoveryError(); + } if (taskSettleScope) { await trace.end(taskSettleScope, { outcome: "failed" }); }