From 17ddb8961858cbbbd773312433431530588a1e33 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 9 Sep 2026 02:31:27 -0500 Subject: [PATCH] Preserve reusable sandboxes through failed resume and stop --- doc/sandbox-work-folders.md | 6 ++ .../daytona/src/plugin.test.ts | 44 ++++++++++--- .../sandbox-providers/daytona/src/plugin.ts | 19 +++--- .../src/__tests__/environment-runtime.test.ts | 50 +++++++++++++-- server/src/services/environment-runtime.ts | 62 ++++++++++++++++--- 5 files changed, 145 insertions(+), 36 deletions(-) diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index a06ca20c56..728f9d078d 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -96,6 +96,12 @@ second workspace or resume the sandbox concurrently. An active predecessor or an incomplete release reports a retryable resume error and preserves the lease. The next run claims the reusable lease in Postgres before resuming the provider; competing server processes cannot both resume the same released sandbox. +An incomplete resume keeps a provisional lease marker until provider verification +succeeds. Failed-run cleanup retains that exact lease without stopping or deleting +its sandbox, so a retry cannot silently create a replacement. Daytona refreshes +the live sandbox state on explicit resume, including externally stopped resources +whose cached handles still say running. Failure to stop a reusable sandbox is +reported and retained for retry; it never falls back to deletion or orphan cleanup. This applies to both runner generations and leaves distinct task/user bindings isolated. The new scoped-shell startup setting is not added to an existing unscoped Codex diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts index 84787dff01..a89d833f94 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts @@ -1250,6 +1250,32 @@ describe("Daytona sandbox provider plugin", () => { }); }); + it("refreshes a recently cached handle before resuming an externally stopped sandbox", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "sandbox-reuse", state: "started" }); + sandbox.process.executeCommand.mockImplementation(async (command: string) => ({ + exitCode: 0, result: command.includes("reusable-sandbox-lease.json") ? JSON.stringify({ token: "sentinel-token" }) : "bash", + })); + mockGet.mockResolvedValue(sandbox); + const params = { + driverKey: "daytona", companyId: "company-1", environmentId: "env-1", providerLeaseId: sandbox.id, + config: { timeoutMs: 300000, reuseLease: true }, + leaseMetadata: { workspaceSentinel: { + path: "/home/daytona/paperclip-workspace/.paperclip-runtime/reusable-sandbox-lease.json", + token: "sentinel-token", result: "written", + } }, + }; + await plugin.definition.onEnvironmentResumeLease!(params); + expect(sandbox.start).not.toHaveBeenCalled(); + sandbox.refreshData.mockImplementation(async () => { sandbox.state = "stopped"; }); + const resumed = await plugin.definition.onEnvironmentResumeLease!(params); + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.refreshData).toHaveBeenCalledTimes(2); + expect(sandbox.start).toHaveBeenCalledWith(300); + expect(resumed).toMatchObject({ providerLeaseId: sandbox.id, metadata: { resumedFromState: "stopped" } }); + expect(sandbox.delete).not.toHaveBeenCalled(); + }); + it("expires a reusable lease when the workspace sentinel does not match", async () => { process.env.DAYTONA_API_KEY = "host-key"; const sandbox = createMockSandbox({ id: "sandbox-reuse", state: "stopped" }); @@ -1371,13 +1397,13 @@ describe("Daytona sandbox provider plugin", () => { expect(warnSpy).toHaveBeenCalled(); }); - it("falls back to delete when stopping a reusable lease from an error state fails", async () => { + it("preserves a reusable lease when stopping it from an error state fails", async () => { process.env.DAYTONA_API_KEY = "host-key"; const errored = createMockSandbox({ id: "sandbox-error", state: "error" }); errored.stop.mockRejectedValueOnce(new Error("stop failed")); mockGet.mockResolvedValue(errored); - await plugin.definition.onEnvironmentReleaseLease?.({ + await expect(plugin.definition.onEnvironmentReleaseLease!({ driverKey: "daytona", companyId: "company-1", environmentId: "env-1", @@ -1386,20 +1412,19 @@ describe("Daytona sandbox provider plugin", () => { timeoutMs: 300000, reuseLease: true, }, - }); + })).rejects.toThrow("stop failed"); expect(errored.stop).toHaveBeenCalledWith(300); - expect(errored.delete).toHaveBeenCalledWith(300); + expect(errored.delete).not.toHaveBeenCalled(); }); - it("falls back to delete when stopping a healthy reusable lease fails mid-call", async () => { + it("preserves a healthy reusable lease when stopping it fails mid-call", async () => { process.env.DAYTONA_API_KEY = "host-key"; const sandbox = createMockSandbox({ id: "sandbox-running", state: "started" }); sandbox.stop.mockRejectedValueOnce(new Error("api timeout")); mockGet.mockResolvedValue(sandbox); - const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); - await plugin.definition.onEnvironmentReleaseLease?.({ + await expect(plugin.definition.onEnvironmentReleaseLease!({ driverKey: "daytona", companyId: "company-1", environmentId: "env-1", @@ -1408,11 +1433,10 @@ describe("Daytona sandbox provider plugin", () => { timeoutMs: 300000, reuseLease: true, }, - }); + })).rejects.toThrow("api timeout"); expect(sandbox.stop).toHaveBeenCalledWith(300); - expect(sandbox.delete).toHaveBeenCalledWith(300); - expect(warnSpy).toHaveBeenCalled(); + expect(sandbox.delete).not.toHaveBeenCalled(); }); describe("session model lifecycle (per-lease session store)", () => { diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.ts index 3a33b12c11..1167d244ce 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.ts @@ -2192,6 +2192,10 @@ const plugin = definePlugin({ return { providerLeaseId: null, metadata: { expired: true } }; } + // Explicit resume must observe external stops even when the warm handle + // cache is younger than the auto-stop refresh threshold. + await withLivenessTimeout("sandbox.refreshData", config.livenessTimeoutMs, () => sandbox.refreshData()); + // A stopped sandbox loses its session shell, so the stored session id is // stale after a real restart. Clear the id only when the sandbox is not // already running, and clear it before the restart. A stopped sandbox has @@ -2291,18 +2295,9 @@ const plugin = definePlugin({ if (config.reuseLease) { if (sandbox.state !== "stopped") { - try { - await sandbox.stop(toTimeoutSeconds(config.timeoutMs)); - } catch (error) { - console.warn( - `Failed to stop Daytona sandbox during lease release: ${formatErrorMessage(error)}. Attempting delete instead.`, - ); - await sandbox.delete(toTimeoutSeconds(config.timeoutMs)).catch((deleteError) => { - console.warn( - `Failed to delete Daytona sandbox after stop failure: ${formatErrorMessage(deleteError)}`, - ); - }); - } + // A failed stop says nothing about the safety of deleting the working + // copy. Surface the failure so the host retains the lease for retry. + await sandbox.stop(toTimeoutSeconds(config.timeoutMs)); } return; } diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index d2466039af..e74b44abd7 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -470,6 +470,48 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { return { pluginId, companyId, agentId, environment, runId, executionWorkspaceId, reusableLease }; } + it.each(["codex_local", "paperclip_runner"])("retains a failed %s resume through run cleanup and retries the original sandbox", async (adapterType) => { + const seeded = await seedReusablePluginSandboxLease(adapterType); + let failResume = false; + const workerManager = { + isRunning: vi.fn(() => true), + getWorker: vi.fn(() => ({ supportedMethods: ["environmentResumeLease", "environmentReleaseLease", "environmentDestroyLease"] })), + call: vi.fn(async (_pluginId: string, method: string) => { + if (method !== "environmentResumeLease") throw new Error(`Unexpected disposal or replacement: ${method}`); + if (failResume) throw new Error("Sandbox is not in a startable state"); + return { providerLeaseId: seeded.reusableLease.providerLeaseId, metadata: { + provider: "fake-plugin", image: "fake:test", timeoutMs: 1234, reuseLease: true, + } }; + }), + } as unknown as PluginWorkerManager; + const runtime = environmentRuntimeService(db, { pluginWorkerManager: workerManager }); + const acquire = (runId: string) => runtime.acquireRunLease({ + companyId: seeded.companyId, environment: seeded.environment, agentId: seeded.agentId, + heartbeatRunId: runId, issueId: null, adapterType, + persistedExecutionWorkspace: { id: seeded.executionWorkspaceId, mode: "shared_workspace" }, + }); + const first = await acquire(seeded.runId); + await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, seeded.runId)); + await environmentService(db).releaseLease(first.lease.id, "released"); + const failedRun = randomUUID(); + await db.insert(heartbeatRuns).values({ id: failedRun, companyId: seeded.companyId, agentId: seeded.agentId, status: "running" }); + failResume = true; + await expect(acquire(failedRun)).rejects.toThrow("the lease was preserved"); + await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, failedRun)); + const released = await runtime.releaseRunLeases(failedRun, "failed"); + expect(released).toHaveLength(1); + expect(released[0]!.lease).toMatchObject({ status: "retained", expiresAt: null, + providerLeaseId: first.lease.providerLeaseId, metadata: { sandboxResumePending: true } }); + expect(vi.mocked(workerManager.call).mock.calls.every((call) => call[1] === "environmentResumeLease")).toBe(true); + const retryRun = randomUUID(); + await db.insert(heartbeatRuns).values({ id: retryRun, companyId: seeded.companyId, agentId: seeded.agentId, status: "running" }); + failResume = false; + const retry = await acquire(retryRun); + expect(retry.lease.providerLeaseId).toBe(first.lease.providerLeaseId); + expect(retry.lease.metadata?.sandboxResumePending).toBe(false); + expect(retry.lease.metadata?.sandboxLeaseAcquisition).toEqual({ outcome: "resumed" }); + }); + it("keeps an existing task's legacy sync contract after its sandbox expires, without affecting new tasks", async () => { const seeded = await seedReusablePluginSandboxLease("codex_local"); const taskId = randomUUID(); @@ -5405,7 +5447,7 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { return { pluginId, environment, runId, lease, workerManager, runtimeWithPlugin }; } - it("routes release to pending_cleanup when the worker no longer advertises the release lifecycle method", async () => { + it("retains reusable work when the worker no longer advertises the release lifecycle method", async () => { const { pluginId, lease, workerManager, runtimeWithPlugin } = await seedStaleLifecycleReusableLease(); const released = await runtimeWithPlugin.releaseRunLeases(lease.heartbeatRunId!); @@ -5417,11 +5459,9 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { expect.anything(), expect.anything(), ); - // The failed release verification must enter the pending-cleanup retry flow. - // The reaper sweeps only `pending_cleanup` leases, so a `released` status - // here would strand the still-active provider resource. + // The destructive orphan sweep must not discard reusable work. await expect(environmentService(db).getLeaseById(lease.id)).resolves.toMatchObject({ - status: "pending_cleanup", + status: "retained", cleanupStatus: "failed", failureReason: "release_cleanup_failed", }); diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index 9e6c07e742..6452046793 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -696,6 +696,27 @@ export class SandboxOrphanCleanupWriteError extends Error { } } +async function retainIncompleteSandboxResume(db: Db, lease: EnvironmentLease): Promise { + if (lease.metadata?.sandboxResumePending !== true || !lease.heartbeatRunId) return null; + // A resume failure is not proof that the existing workspace is disposable. + // Keep the claimed reference eligible for retry, without a provider release + // RPC or the destructive pending-cleanup sweep. Successful acquisition + // replaces this provisional metadata with the verified provider metadata. + const now = new Date(); + const [retained] = await db.update(environmentLeases).set({ + status: "retained", expiresAt: null, releasedAt: null, + failureReason: "sandbox_resume_incomplete", cleanupStatus: "failed", + lastUsedAt: now, updatedAt: now, + }).where(and( + eq(environmentLeases.id, lease.id), + eq(environmentLeases.companyId, lease.companyId), + eq(environmentLeases.heartbeatRunId, lease.heartbeatRunId), + inArray(environmentLeases.status, ["active", "retained"]), + sql`${environmentLeases.metadata}->'sandboxResumePending' = 'true'::jsonb`, + )).returning(); + return retained ? toEnvironmentLeaseSnapshot(retained) : null; +} + /** A reusable sandbox could not be resumed, but has not been proven lost. */ export class ReusableSandboxResumeError extends Error { readonly provider: string; @@ -1299,7 +1320,13 @@ function createSandboxEnvironmentDriver( lease: EnvironmentLease, input: Parameters[0], ): Promise { - if (!input.heartbeatRunId || lease.heartbeatRunId === input.heartbeatRunId) return lease; + if (!input.heartbeatRunId) return lease; + const metadata = { ...lease.metadata, sandboxResumePending: true }; + if (lease.heartbeatRunId === input.heartbeatRunId) { + const claimed = await environmentsSvc.updateLeaseMetadata(lease.id, metadata); + if (!claimed) throw new Error("Reusable sandbox claim disappeared before resume"); + return claimed; + } // Transfer ownership atomically before touching the provider. Two server // processes can observe the same released lease; only the winner of this // conditional update/insert may resume its sandbox. A failed resume leaves @@ -1317,7 +1344,7 @@ function createSandboxEnvironmentDriver( // The retained provider's old expiry may already be past. The resume RPC // will attest its current expiry; until then use only this run's deadline. expiresAt: input.requestedExpiresAt ?? null, - metadata: lease.metadata, + metadata, replacesReusableLeaseId: lease.id, }); } @@ -2156,6 +2183,7 @@ function createSandboxEnvironmentDriver( sandboxProviderPlugin: true, ...sandboxConfigForLeaseMetadata(storedConfig), ...sanitizedProviderMetadata, + sandboxResumePending: false, workFolderLayout: legacyWorkFolderLayout || (reusableLease && hasLegacySandboxWorkspace(reusableLease)) ? "legacy" : "scoped", sandboxLeaseAcquisition: providerLease ? { @@ -2407,6 +2435,7 @@ function createSandboxEnvironmentDriver( driver: input.environment.driver, executionWorkspaceMode: input.executionWorkspaceMode, ...providerLease.metadata, + sandboxResumePending: false, workFolderLayout: legacyWorkFolderLayout || (reusableLease && hasLegacySandboxWorkspace(reusableLease)) ? "legacy" : "scoped", sandboxLeaseAcquisition: reusableLease && providerLease.providerLeaseId === reusableLease.providerLeaseId @@ -2502,6 +2531,8 @@ function createSandboxEnvironmentDriver( }, async releaseRunLease(input) { + const pendingResume = await retainIncompleteSandboxResume(db, input.lease); + if (pendingResume) return pendingResume; if (await retainUnsavedWorkFolderLease(db, input.lease)) return { ...input.lease, status: "retained", expiresAt: null, failureReason: "work_folder_save_required" }; if (input.status === "expired" && input.lease.leasePolicy === "reuse_by_environment") { return await destroyReusableSandboxLease({ @@ -2547,7 +2578,7 @@ function createSandboxEnvironmentDriver( } catch { cleanupStatus = "failed"; } - const releaseStatus = input.lease.leasePolicy === "retain_on_failure" && input.status === "failed" + const releaseStatus = (input.lease.leasePolicy === "retain_on_failure" || input.lease.leasePolicy === "reuse_by_environment") && input.status === "failed" ? "retained" as const : input.status; return await environmentsSvc.releaseLease(input.lease.id, releaseStatus, { @@ -3155,13 +3186,12 @@ function createSandboxEnvironmentDriver( cleanupStatus = "failed"; } - // A failed release verification leaves the provider resource active. The - // cleanup reaper retries only `pending_cleanup` leases, so route a failed - // release into that retry flow. A `retain_on_failure` lease keeps the - // resource on purpose for reuse, so it stays `retained` and never enters the - // reaper, which would destroy the resource the retain policy wants to keep. + // Reusable work must survive a failed stop. Never send that resource to the + // destructive pending-cleanup sweep; the next exact-lease resume can retry. + // Ephemeral orphan cleanup still uses the existing sweep. const retained = - input.lease.leasePolicy === "retain_on_failure" && input.status === "failed"; + (input.lease.leasePolicy === "retain_on_failure" && input.status === "failed") || + (input.lease.leasePolicy === "reuse_by_environment" && (input.status === "failed" || cleanupStatus === "failed")); const releaseStatus = retained ? ("retained" as const) : cleanupStatus === "failed" @@ -3318,6 +3348,7 @@ const INTERNAL_PLUGIN_SANDBOX_CONFIG_KEYS = new Set([ "shellCommand", "sandboxProviderPlugin", "sandboxLeaseAcquisition", + "sandboxResumePending", "nativeHarnessBackup", "nativeWorkspaceSync", ]); @@ -3837,6 +3868,19 @@ export function environmentRuntimeService( if (!environment) continue; const leaseSnapshot = toEnvironmentLeaseSnapshot(leaseRow); + const pendingResume = await retainIncompleteSandboxResume(db, leaseSnapshot); + if (pendingResume) { + released.push({ + environment, + lease: pendingResume, + leaseContext: { + executionWorkspaceId: pendingResume.executionWorkspaceId, + executionWorkspaceMode: + (pendingResume.metadata?.executionWorkspaceMode as ExecutionWorkspace["mode"] | null | undefined) ?? null, + }, + }); + continue; + } if (await retainUnsavedWorkFolderLease(db, leaseSnapshot)) continue; if ( providerResourceDisposition === "keep_running" &&