mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
ea10ee8994
commit
ad0b7105f3
5 files changed
+107
-10
No files matched your search
@@ -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.
|
||||
@@ -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",
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -97,6 +97,8 @@ export interface SandboxProvider {
|
||||
probe(config: SandboxEnvironmentConfig): Promise<EnvironmentProbeResult>;
|
||||
acquireLease(input: AcquireSandboxLeaseInput): Promise<SandboxLeaseHandle>;
|
||||
resumeLease(input: ResumeSandboxLeaseInput): Promise<SandboxLeaseHandle | null>;
|
||||
/** Explicit stop-only support. Absence must never fall back to release/destroy. */
|
||||
stopLease?(input: DestroySandboxLeaseInput): Promise<{ providerLeaseId: string; state: "stopped" }>;
|
||||
releaseLease(input: ReleaseSandboxLeaseInput): Promise<void>;
|
||||
destroyLease(input: DestroySandboxLeaseInput): Promise<void>;
|
||||
matchesReusableLease(input: {
|
||||
@@ -181,6 +183,14 @@ class FakeSandboxProvider implements SandboxProvider {
|
||||
};
|
||||
}
|
||||
|
||||
async stopLease(input: DestroySandboxLeaseInput): Promise<{ providerLeaseId: string; state: "stopped" }> {
|
||||
assertProviderConfig<FakeSandboxEnvironmentConfig>(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<void> {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
Reference in new issue
Block a user