From ad0b7105f35f82c9bbdc7b8bf9dcddda40457a93 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 28 Sep 2026 00:39:41 -0500 Subject: [PATCH] fix: complete built-in sandbox stop-only cleanup Give the built-in fake provider an explicit non-destructive stop operation and preserve its recorded identity through generic stop intent and restart recovery. Keep plugin capability enforcement unchanged. Co-Authored-By: Paperclip --- doc/native-workspace-finalization-recovery.md | 6 ++ .../src/__tests__/environment-runtime.test.ts | 58 +++++++++++++++++++ server/src/services/environment-runtime.ts | 17 +++++- .../src/services/sandbox-provider-runtime.ts | 10 ++++ .../src/services/sandbox-stop-and-retain.ts | 26 ++++++--- 5 files changed, 107 insertions(+), 10 deletions(-) diff --git a/doc/native-workspace-finalization-recovery.md b/doc/native-workspace-finalization-recovery.md index ad52162c20f..10e9dc8c861 100644 --- a/doc/native-workspace-finalization-recovery.md +++ b/doc/native-workspace-finalization-recovery.md @@ -156,3 +156,9 @@ A matching stopped receipt clears the pending intent, leaves the lease released and resumable, and saves `sandboxStopAndRetainReceipt`. Its request identity, original plugin and `method: "environmentStopLease"` attest this dispatch route. The accepted-result export intent and its commitment requirements are unchanged. + +The built-in fake provider uses its explicit `stopLease` operation rather than a +plugin RPC. Its intent pins the registered built-in provider and its receipt says +`builtin.stopLease`. This provider owns no real process or filesystem; the receipt +models its lifecycle. Missing built-in stop support or an unconfirmed receipt +also remains pending through restart, without calling release or destroy. diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index 199537cd055..7886155e222 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -1326,6 +1326,64 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { expect(released[0]?.lease.status).toBe("released"); }); + it.each([false, true])("confirms a built-in fake stop without release or destroy fallback: cancel=%s", async cancel => { + const { companyId, environment, runId } = await seedEnvironment({ + driver: "sandbox", config: { provider: "fake", image: "ubuntu:24.04", reuseLease: false }, + }); + const acquired = await runtime.acquireRunLease({ companyId, environment, issueId: null, + heartbeatRunId: runId, persistedExecutionWorkspace: null }); + const provider = sandboxProviderRuntime.requireSandboxProvider("fake"); + const release = vi.spyOn(provider, "releaseLease"), destroy = vi.spyOn(provider, "destroyLease"); + const released = await runtime.releaseRunLeases(runId, "released", undefined, "stop_and_retain", cancel); + expect(released).toHaveLength(1); + expect(released[0]?.lease).toMatchObject({ status: "released", cleanupStatus: "success", failureReason: null, + metadata: { remoteExecutionTermination: { state: "stopped", providerLeaseId: acquired.lease.providerLeaseId }, + sandboxStopAndRetainReceipt: { method: "builtin.stopLease", builtinProvider: "fake", runId } } }); + expect(await remoteExecutionHasStopped(db, companyId, runId)).toBe(true); + expect(release).not.toHaveBeenCalled(); + expect(destroy).not.toHaveBeenCalled(); + }); + + it.each(["stop_failed", "wrong_receipt", "missing_capability", "changed_binding"])( + "preserves a built-in stop through restart without destructive fallback: %s", async outcome => { + const { companyId, environment, runId } = await seedEnvironment({ + driver: "sandbox", config: { provider: "fake", image: "ubuntu:24.04", reuseLease: false }, + }); + const acquired = await runtime.acquireRunLease({ companyId, environment, issueId: null, + heartbeatRunId: runId, persistedExecutionWorkspace: null }); + const provider = sandboxProviderRuntime.requireSandboxProvider("fake"), originalStop = provider.stopLease!; + const release = vi.spyOn(provider, "releaseLease"), destroy = vi.spyOn(provider, "destroyLease"); + try { + provider.stopLease = outcome === "missing_capability" ? undefined : vi.fn(async () => { + if (outcome === "wrong_receipt") return { providerLeaseId: "sandbox://fake/other", state: "stopped" as const }; + throw new Error("injected built-in stop failure"); + }); + await runtime.releaseRunLeases(runId, "released", undefined, "stop_and_retain", true); + expect(await environmentService(db).getLeaseById(acquired.lease.id)).toMatchObject({ status: "pending_cleanup" }); + expect(await remoteExecutionHasStopped(db, companyId, runId)).toBe(false); + if (outcome === "missing_capability") { + await heartbeatService(db, { environmentRuntime: environmentRuntimeService(db) }).sweepPendingCleanupLeases({ backoffMs: 0 }); + expect(await environmentService(db).getLeaseById(acquired.lease.id)).toMatchObject({ status: "pending_cleanup" }); + } + const stop = vi.fn(originalStop.bind(provider)); + provider.stopLease = stop; + if (outcome === "changed_binding") await db.update(environmentLeases).set({ providerLeaseId: "sandbox://fake/replacement" }) + .where(eq(environmentLeases.id, acquired.lease.id)); + await heartbeatService(db, { environmentRuntime: environmentRuntimeService(db) }).sweepPendingCleanupLeases({ backoffMs: 0 }); + if (outcome === "changed_binding") { + expect(stop).not.toHaveBeenCalled(); + expect(await environmentService(db).getLeaseById(acquired.lease.id)).toMatchObject({ status: "pending_cleanup" }); + } else { + expect(stop).toHaveBeenCalledExactlyOnceWith(expect.objectContaining({ providerLeaseId: acquired.lease.providerLeaseId })); + expect(await environmentService(db).getLeaseById(acquired.lease.id)).toMatchObject({ status: "released", cleanupStatus: "success" }); + expect(await remoteExecutionHasStopped(db, companyId, runId)).toBe(true); + } + expect(release).not.toHaveBeenCalled(); + expect(destroy).not.toHaveBeenCalled(); + } finally { provider.stopLease = originalStop; } + }, + ); + it("releases the remote sandbox when the lease insert rejects a foreign-company binding", async () => { const { companyId, environment, runId } = await seedEnvironment({ driver: "sandbox", diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index c799eba7f2a..4105ec6642d 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -2695,13 +2695,24 @@ function createSandboxEnvironmentDriver( // durable orphan record, not the environment binding, for the same reason // as the plugin path above. The teardown targets the recorded provider, // never the current environment provider. - if (exportResume) throw new Error("Workspace export recovery requires verified stop-only cleanup."); const cleanupConfig = await resolveSandboxCleanupConfigSecrets( db, input.lease.companyId, metadataConfig, { issueId: input.lease.issueId, heartbeatRunId: input.lease.heartbeatRunId }, ); + if (exportResume) { + const provider = getBuiltinSandboxProvider(recordedProvider); + if (resumeIntent?.pluginId !== null || !provider?.stopLease) { + throw new Error("Sandbox preservation requires verified built-in stop-only cleanup."); + } + await assertExportResumeOwnership(); + const receipt = await provider.stopLease({ config: cleanupConfig, providerLeaseId: input.lease.providerLeaseId }); + if (remoteTerminationReceipt(input.lease, receipt)?.state !== "stopped") { + throw new Error("Built-in sandbox stop did not confirm the exact retained allocation."); + } + return receipt; + } await destroySandboxProviderLease({ config: cleanupConfig, providerLeaseId: input.lease.providerLeaseId, @@ -2718,7 +2729,9 @@ function createSandboxEnvironmentDriver( // teardown runs, throws, and counts toward the cap. if (!recordedProvider) return true; // A built-in provider has no plugin worker, so it is always ready. - if (isBuiltinSandboxProvider(recordedProvider)) return true; + if (isBuiltinSandboxProvider(recordedProvider)) { + return !hasStopOnlyCleanup(input.lease) || typeof getBuiltinSandboxProvider(recordedProvider)?.stopLease === "function"; + } // No worker manager is a permanent condition here. Report ready, so the // teardown runs, throws its own "no worker manager" error, and counts // toward the cap. diff --git a/server/src/services/sandbox-provider-runtime.ts b/server/src/services/sandbox-provider-runtime.ts index ad0d690f487..955954c6087 100644 --- a/server/src/services/sandbox-provider-runtime.ts +++ b/server/src/services/sandbox-provider-runtime.ts @@ -97,6 +97,8 @@ export interface SandboxProvider { probe(config: SandboxEnvironmentConfig): Promise; acquireLease(input: AcquireSandboxLeaseInput): Promise; resumeLease(input: ResumeSandboxLeaseInput): Promise; + /** Explicit stop-only support. Absence must never fall back to release/destroy. */ + stopLease?(input: DestroySandboxLeaseInput): Promise<{ providerLeaseId: string; state: "stopped" }>; releaseLease(input: ReleaseSandboxLeaseInput): Promise; destroyLease(input: DestroySandboxLeaseInput): Promise; matchesReusableLease(input: { @@ -181,6 +183,14 @@ class FakeSandboxProvider implements SandboxProvider { }; } + async stopLease(input: DestroySandboxLeaseInput): Promise<{ providerLeaseId: string; state: "stopped" }> { + assertProviderConfig(this.provider, input.config); + if (!input.providerLeaseId?.startsWith("sandbox://fake/")) throw new Error("Fake sandbox stop needs its exact allocation."); + // The fake provider owns no process or filesystem; its explicit stop receipt + // models the lifecycle without invoking its ordinary release or destroy. + return { providerLeaseId: input.providerLeaseId, state: "stopped" }; + } + async releaseLease(): Promise { return; } diff --git a/server/src/services/sandbox-stop-and-retain.ts b/server/src/services/sandbox-stop-and-retain.ts index ce4e54acdea..e9823dcabd4 100644 --- a/server/src/services/sandbox-stop-and-retain.ts +++ b/server/src/services/sandbox-stop-and-retain.ts @@ -1,6 +1,7 @@ import { randomUUID } from "node:crypto"; import { and, eq, sql, type SQL } from "drizzle-orm"; import { environmentLeases, type Db } from "@paperclipai/db"; +import { isBuiltinSandboxProvider } from "./sandbox-provider-runtime.js"; import { remoteTerminationReceipt } from "./remote-execution-termination.js"; import { hasNativeWorkspaceExportResume, readNativeWorkspaceExportResume, settleNativeWorkspaceExportResume } from "./native-runtime/native-workspace-export-resume.js"; @@ -26,9 +27,14 @@ export function readStopOnlyCleanup(lease: Lease) { || intent.companyId !== lease.companyId || intent.runId !== lease.heartbeatRunId || !lease.heartbeatRunId || intent.leaseId !== lease.id || intent.provider !== lease.provider || !lease.provider || intent.providerLeaseId !== lease.providerLeaseId || !lease.providerLeaseId - || typeof intent.requestId !== "string" || !intent.requestId - || typeof intent.pluginId !== "string" || !intent.pluginId || intent.pluginId !== lease.metadata?.pluginId) return null; - return { ...intent, requestId: intent.requestId, pluginId: intent.pluginId }; + || typeof intent.requestId !== "string" || !intent.requestId) return null; + if (intent.builtinProvider !== undefined) { + if (intent.builtinProvider !== lease.provider || !isBuiltinSandboxProvider(lease.provider) + || intent.pluginId !== undefined || lease.metadata?.pluginId != null || lease.metadata?.sandboxProviderPlugin) return null; + return { ...intent, requestId: intent.requestId, pluginId: null, builtinProvider: lease.provider }; + } + if (typeof intent.pluginId !== "string" || !intent.pluginId || intent.pluginId !== lease.metadata?.pluginId) return null; + return { ...intent, requestId: intent.requestId, pluginId: intent.pluginId, builtinProvider: null }; } /** Persist before provider dispatch so an older worker or a restart cannot turn @@ -37,7 +43,9 @@ export async function prepareSandboxStopAndRetain(db: Db, lease: Lease) { const requestId = randomUUID(), now = new Date(); const intent = { schema: "paperclip.sandbox-stop-and-retain.v1", requestId, companyId: lease.companyId, runId: lease.heartbeatRunId, leaseId: lease.id, - provider: lease.provider, providerLeaseId: lease.providerLeaseId, pluginId: lease.metadata?.pluginId }; + provider: lease.provider, providerLeaseId: lease.providerLeaseId, + ...(lease.provider && isBuiltinSandboxProvider(lease.provider) && !lease.metadata?.sandboxProviderPlugin && lease.metadata?.pluginId == null + ? { builtinProvider: lease.provider } : { pluginId: lease.metadata?.pluginId }) }; const [updated] = await db.update(environmentLeases).set({ status: "pending_cleanup", cleanupStatus: "failed", failureReason: "sandbox_stop_pending", releasedAt: now, updatedAt: now, metadata: sql`(coalesce(${environmentLeases.metadata}, '{}'::jsonb) - 'remoteExecutionTermination' - 'sandboxStopAndRetainReceipt') || ${JSON.stringify({ @@ -69,8 +77,10 @@ export async function settleStopOnlyCleanup(db: Db, lease: Lease, options: { att ...(stopped ? { remoteExecutionTermination: receipt, sandboxStopAndRetainReceipt: { schema: "paperclip.sandbox-stop-and-retain-receipt.v1", requestId: intent.requestId, companyId: lease.companyId, runId: lease.heartbeatRunId, leaseId: lease.id, - provider: lease.provider, providerLeaseId: lease.providerLeaseId, pluginId: intent.pluginId, - method: "environmentStopLease", confirmedAt: receipt.confirmedAt, + provider: lease.provider, providerLeaseId: lease.providerLeaseId, + ...(intent.pluginId ? { pluginId: intent.pluginId, method: "environmentStopLease" } + : { builtinProvider: lease.provider, method: "builtin.stopLease" }), + confirmedAt: receipt.confirmedAt, } } : {}), pendingCleanupInFlight: false, pendingCleanupLeaseExpiresAtMs: 0, })}::jsonb`, @@ -78,8 +88,8 @@ export async function settleStopOnlyCleanup(db: Db, lease: Lease, options: { att eq(environmentLeases.heartbeatRunId, lease.heartbeatRunId!), eq(environmentLeases.provider, lease.provider!), eq(environmentLeases.providerLeaseId, lease.providerLeaseId!), eq(environmentLeases.status, "pending_cleanup"), sql`${environmentLeases.metadata}->>'pendingCleanupAttemptId' = ${options.attemptId}`, - sql`${environmentLeases.metadata}->'sandboxStopAndRetain'->>'requestId' = ${intent.requestId}`, - sql`${environmentLeases.metadata}->>'pluginId' = ${intent.pluginId}`, + sql`${environmentLeases.metadata}->'sandboxStopAndRetain' = ${JSON.stringify(lease.metadata?.[SANDBOX_STOP_AND_RETAIN_KEY])}::jsonb`, + sql`${environmentLeases.metadata}->>'pluginId' is not distinct from ${intent.pluginId}`, )).returning(); return updated ?? null; }