diff --git a/packages/plugins/sandbox-providers/daytona/README.md b/packages/plugins/sandbox-providers/daytona/README.md index a7e0198f8c9..76edf25e51d 100644 --- a/packages/plugins/sandbox-providers/daytona/README.md +++ b/packages/plugins/sandbox-providers/daytona/README.md @@ -27,6 +27,8 @@ Notes: - The current published Daytona SDK package is `@daytonaio/sdk`. - The driver supports both `snapshot`-based and `image`-based sandbox creation. If both are set, validation rejects the config as ambiguous. +- Each cold create uses a unique provider name and ownership labels. If creation fails after Daytona has allocated a sandbox, the driver looks up that exact name, checks every ownership label, and waits for deletion. Failed, missing, or timed-out lookups and failed deletions report unconfirmed cleanup with the provider name; they never return a usable lease. A missing name lookup after an uncertain create is not proof that a delayed provider request cannot create a resource. +- For plugin-backed sandbox lease acquisition, unresolved creation cleanup crosses the worker RPC as a validated ownership envelope. The host records `pending_cleanup` before retrying deletion. If only a creation name is known, the provider first returns an ownership-verified sandbox ID without deleting it. The host saves that observation before authorizing deletion. Its existing cleanup sweep and durable spool preserve retries across controller restarts and environment deletion. A missing observed ID confirms cleanup after a lost deletion reply or database update; a name that has never been observed remains unresolved. Provider exceptions and resolved credentials are excluded from that envelope. This requires the matching host and plugin SDK update; probe and custom-image interactive-setup calls still report unconfirmed immediate cleanup without this lease-recovery path. - Reusable leases map to Daytona stop/start semantics. Non-reusable leases are deleted on release. A provider-resolved `target` does not change the identity of an existing sandbox. Release closes the same scoped lease that a later sentinel-verified resume reopens. - A sandbox record can survive the loss of its underlying container. Resume treats it as expired only when a fresh provider read confirms the exact missing-container error for that sandbox and marks it unrecoverable. Unknown errors and failed confirmation reads preserve the lease. The host still requires a verified native-runner backup before replacement. diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts index d15bbe9814c..86b4fbfd650 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts @@ -36,6 +36,7 @@ import plugin, { __setDaytonaPluginContextForTest, } from "./plugin.js"; import type { PluginContext } from "@paperclipai/plugin-sdk"; +import { environmentCreationCleanupErrorData, readEnvironmentCreationCleanupError } from "@paperclipai/plugin-sdk"; import manifest from "./manifest.js"; import { parseTarVerboseListingLine, splitLinkEntryOnce } from "./file-sync.js"; @@ -523,6 +524,207 @@ describe("Daytona sandbox provider plugin", () => { }); }); + describe("failed sandbox creation cleanup", () => { + const params = { + driverKey: "daytona", companyId: "company-1", environmentId: "env-1", runId: "run-1", + config: { image: "node:20", timeoutMs: 300_000, reuseLease: false, livenessTimeoutMs: 100 }, + }; + const createError = new MockDaytonaTimeoutError("Failed to create and start sandbox within 300 seconds"); + + beforeEach(() => { + process.env.DAYTONA_API_KEY = "host-key"; + mockCreate.mockRejectedValue(createError); + }); + + function ownedSandbox() { + const requested = mockCreate.mock.calls.at(-1)![0]; + return { ...createMockSandbox(), name: requested.name, labels: { ...requested.labels } }; + } + + async function unresolvedCreation() { + mockGet.mockRejectedValue(new MockDaytonaNotFoundError("not visible yet")); + const error = await plugin.definition.onEnvironmentAcquireLease!(params).catch(error => error); + const cleanup = readEnvironmentCreationCleanupError(error); + expect(cleanup).not.toBeNull(); + return { error, cleanup: cleanup! }; + } + + async function journalObservation(cleanup: NonNullable>) { + const error = await plugin.definition.onEnvironmentDestroyLease!({ ...params, + providerLeaseId: cleanup.providerLeaseId, leaseMetadata: { failedCreateCleanup: cleanup }, + }).catch(error => error); + const observed = readEnvironmentCreationCleanupError(error); + expect(observed?.observedProviderLeaseId).toBeTruthy(); + return observed!; + } + + it("hands non-secret cleanup ownership to the host when creation is uncertain", async () => { + const { error, cleanup } = await unresolvedCreation(); + expect(cleanup).toMatchObject({ companyId: params.companyId, environmentId: params.environmentId, runId: params.runId, + providerLeaseId: mockCreate.mock.calls[0][0].name, labels: mockCreate.mock.calls[0][0].labels }); + expect(cleanup.accountFingerprint).toMatch(/^[a-f0-9]{64}$/); + expect(JSON.stringify(environmentCreationCleanupErrorData(error))).not.toContain("host-key"); + expect(JSON.stringify(environmentCreationCleanupErrorData(error))).not.toContain(createError.message); + }); + + it("preserves ownership when the SDK adds labels to its mutable create input", async () => { + mockCreate.mockImplementation(async (input) => { + // Daytona 0.203.0 writes its default language into params.labels. + input.labels["code-toolbox-language"] = "python"; + throw createError; + }); + const { error, cleanup } = await unresolvedCreation(); + expect(environmentCreationCleanupErrorData(error)).toBeDefined(); + expect(cleanup.labels).not.toHaveProperty("code-toolbox-language"); + const orphan = ownedSandbox(); + mockGet.mockResolvedValue(orphan); + const observed = await journalObservation(cleanup); + expect(orphan.delete).not.toHaveBeenCalled(); + await expect(plugin.definition.onEnvironmentDestroyLease!({ ...params, + providerLeaseId: cleanup.providerLeaseId, leaseMetadata: { failedCreateCleanup: observed }, + })).resolves.toEqual({ providerLeaseId: cleanup.providerLeaseId, state: "destroyed" }); + expect(orphan.delete).toHaveBeenCalledWith(10, true); + }); + + it("retries a late-visible failed creation using its persisted ownership envelope", async () => { + const { cleanup } = await unresolvedCreation(); + const orphan = ownedSandbox(); mockGet.mockResolvedValue(orphan); + const observed = await journalObservation(cleanup); + expect(orphan.delete).not.toHaveBeenCalled(); + await expect(plugin.definition.onEnvironmentDestroyLease!({ ...params, providerLeaseId: cleanup.providerLeaseId, + leaseMetadata: { failedCreateCleanup: observed } })).resolves.toEqual({ providerLeaseId: cleanup.providerLeaseId, state: "destroyed" }); + expect(mockGet).toHaveBeenLastCalledWith(orphan.id); + expect(orphan.delete).toHaveBeenCalledWith(10, true); + expect(orphan.process.executeCommand).not.toHaveBeenCalled(); + }); + + it("keeps a failed-creation retry unresolved while the allocation is still not visible", async () => { + const { cleanup } = await unresolvedCreation(); + await expect(plugin.definition.onEnvironmentDestroyLease!({ ...params, providerLeaseId: cleanup.providerLeaseId, + leaseMetadata: { failedCreateCleanup: cleanup } })).rejects.toThrow(); + }); + + it("accepts absence only after a matching provider ID was observed", async () => { + const { cleanup } = await unresolvedCreation(); + const orphan = ownedSandbox(); mockGet.mockResolvedValue(orphan); + const observed = await journalObservation(cleanup); + expect(orphan.delete).not.toHaveBeenCalled(); + mockGet.mockRejectedValue(new MockDaytonaNotFoundError("already deleted")); + await expect(plugin.definition.onEnvironmentDestroyLease!({ ...params, + providerLeaseId: cleanup.providerLeaseId, leaseMetadata: { failedCreateCleanup: observed }, + })).resolves.toEqual({ providerLeaseId: cleanup.providerLeaseId, state: "destroyed" }); + expect(mockGet).toHaveBeenLastCalledWith(orphan.id); + }); + + it("retains the observed ID when immediate deletion loses its acknowledgement", async () => { + let orphan: ReturnType; + mockGet.mockImplementation(async () => { + orphan = ownedSandbox(); + orphan.delete.mockRejectedValue(new Error("delete response lost")); + return orphan; + }); + const error = await plugin.definition.onEnvironmentAcquireLease!(params).catch(error => error); + const cleanup = readEnvironmentCreationCleanupError(error)!; + expect(cleanup.observedProviderLeaseId).toBe(orphan!.id); + mockGet.mockRejectedValue(new MockDaytonaNotFoundError("already deleted")); + await expect(plugin.definition.onEnvironmentDestroyLease!({ ...params, + providerLeaseId: cleanup.providerLeaseId, leaseMetadata: { failedCreateCleanup: cleanup }, + })).resolves.toEqual({ providerLeaseId: cleanup.providerLeaseId, state: "destroyed" }); + }); + + it.each(["company", "account", "labels"])("fences failed-creation retries by %s", async (mismatch) => { + const { cleanup } = await unresolvedCreation(); + const orphan = ownedSandbox(); mockGet.mockResolvedValue(orphan); + if (mismatch === "labels") orphan.labels["paperclip-run-id"] = "foreign-run"; + const retry = { ...params, providerLeaseId: cleanup.providerLeaseId, leaseMetadata: { failedCreateCleanup: cleanup } }; + if (mismatch === "company") retry.companyId = "foreign-company"; + if (mismatch === "account") process.env.DAYTONA_API_KEY = "another-account-key"; + await expect(plugin.definition.onEnvironmentDestroyLease!(retry)).rejects.toThrow(); + expect(orphan.delete).not.toHaveBeenCalled(); + }); + + it("deletes the exact sandbox left behind when create times out, then preserves the create error", async () => { + let orphan: ReturnType; + mockGet.mockImplementation(async () => (orphan = ownedSandbox())); + await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toBe(createError); + expect(mockCreate.mock.calls[0][0].name).toMatch(/^paperclip-create-[0-9a-f-]{36}$/); + expect(mockGet).toHaveBeenCalledWith(mockCreate.mock.calls[0][0].name); + expect(orphan!.delete).toHaveBeenCalledWith(10, true); + expect(orphan!.process.executeCommand).not.toHaveBeenCalled(); + }); + + it.each(["paperclip-company-id", "paperclip-environment-id", "paperclip-run-id", "paperclip-create-attempt", "paperclip-provider"])("never deletes a sandbox with a mismatched %s", async (label) => { + let orphan: ReturnType; + mockGet.mockImplementation(async () => { + orphan = ownedSandbox(); orphan.labels[label] = "different-owner"; return orphan; + }); + await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed"); + expect(orphan!.delete).not.toHaveBeenCalled(); + }); + + it("never deletes a sandbox returned for a different name", async () => { + const wrong = { ...createMockSandbox(), labels: {} }; + mockGet.mockResolvedValue(wrong); + await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed"); + expect(wrong.delete).not.toHaveBeenCalled(); + }); + + it("does not treat a not-found lookup after uncertain creation as confirmed cleanup", async () => { + mockGet.mockRejectedValue(new MockDaytonaNotFoundError("missing")); + await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed"); + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it("reports unconfirmed cleanup when the provider lookup fails", async () => { + mockGet.mockRejectedValue(new Error("provider unavailable")); + await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed"); + }); + + it("reports unconfirmed cleanup when provider deletion fails", async () => { + mockGet.mockImplementation(async () => { + const orphan = ownedSandbox(); orphan.delete.mockRejectedValue(new Error("delete failed")); return orphan; + }); + await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed"); + }); + + it("bounds a hung provider lookup and preserves both failure causes", async () => { + vi.useFakeTimers(); + try { + mockGet.mockImplementation(() => new Promise(() => {})); + const result = plugin.definition.onEnvironmentAcquireLease!(params).catch((error: unknown) => error); + await vi.advanceTimersByTimeAsync(10_001); + const error = await result; + expect(error).toBeInstanceOf(AggregateError); + expect((error as AggregateError).errors[0]).toBe(createError); + expect((error as Error).message).toContain(mockCreate.mock.calls[0][0].name); + } finally { vi.useRealTimers(); } + }); + + it("bounds a hung provider deletion without returning a lease", async () => { + vi.useFakeTimers(); + try { + mockGet.mockImplementation(async () => { + const orphan = ownedSandbox(); orphan.delete.mockImplementation(() => new Promise(() => {})); return orphan; + }); + const result = plugin.definition.onEnvironmentAcquireLease!(params).catch((error: unknown) => error); + await vi.advanceTimersByTimeAsync(15_001); + expect(await result).toBeInstanceOf(AggregateError); + } finally { vi.useRealTimers(); } + }); + + it("uses distinct creation identities for concurrent attempts on the same run", async () => { + mockGet.mockRejectedValue(new MockDaytonaNotFoundError("missing")); + await Promise.allSettled([ + plugin.definition.onEnvironmentAcquireLease?.(params), + plugin.definition.onEnvironmentAcquireLease?.(params), + ]); + const requests = mockCreate.mock.calls.map(([request]) => request); + expect(requests).toHaveLength(2); + expect(new Set(requests.map((r) => r.name)).size).toBe(2); + expect(new Set(requests.map((r) => r.labels["paperclip-create-attempt"])).size).toBe(2); + }); + }); + it("rejects ambiguous or invalid config", async () => { await expect(plugin.definition.onEnvironmentValidateConfig?.({ driverKey: "daytona", diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.ts index a053af7e7c7..8e1b19866f4 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.ts @@ -9,9 +9,10 @@ import type { Resources, Sandbox, } from "@daytonaio/sdk"; -import { decodeChannelBytes, definePlugin, NOOP_PLUGIN_TRACER } from "@paperclipai/plugin-sdk"; +import { decodeChannelBytes, definePlugin, NOOP_PLUGIN_TRACER, PluginEnvironmentCreationCleanupError, readEnvironmentCreationCleanupError } from "@paperclipai/plugin-sdk"; import type { PluginContext, + PluginEnvironmentCreationCleanup, PluginTracer, PluginEnvironmentAcquireLeaseParams, PluginEnvironmentCancelInteractiveSetupParams, @@ -942,18 +943,84 @@ async function createSandbox( throw new Error(resourceRequestError); } const client = createDaytonaClient(config); - const createParams = buildCreateParams(config, buildSandboxLabels({ + // The SDK can create a sandbox and then throw while waiting for it to start. + // Keep an attempt-specific provider name so that failure cannot lose ownership. + const attemptId = randomUUID(); + const name = `paperclip-create-${attemptId}`; + const labels = { ...buildSandboxLabels({ companyId: params.companyId, environmentId: params.environmentId, runId: "runId" in params ? params.runId : undefined, setupSessionId: "sessionId" in params ? params.sessionId : undefined, purpose: options.purpose, reuseLease: config.reuseLease, - })); - const sandbox = await client.create(createParams, { - timeout: toTimeoutSeconds(config.timeoutMs), - }); - return sandbox; + }), "paperclip-create-attempt": attemptId }; + // The SDK mutates params.labels (for example, code-toolbox-language). + // Preserve our immutable ownership snapshot for validation and retry. + const createParams = { ...buildCreateParams(config, { ...labels }), name }; + try { + return await client.create(createParams, { + timeout: toTimeoutSeconds(config.timeoutMs), + }); + } catch (createError) { + const cleanup: PluginEnvironmentCreationCleanup = { + providerLeaseId: name, companyId: params.companyId, environmentId: params.environmentId, + ...("runId" in params ? { runId: params.runId } : {}), + attemptId, labels, accountFingerprint: sandboxAccountDiscriminator(config), + }; + try { + // A not-found lookup after an uncertain create is not a deletion receipt: + // the provider may still materialize the request. Keep the name in the + // error so cleanup can be reconciled rather than declaring success. + await destroyFailedCreation(config, cleanup); + } catch (cleanupError) { + throw new PluginEnvironmentCreationCleanupError([createError, cleanupError], + `Daytona sandbox creation failed; cleanup could not be confirmed for ${name}`, cleanup); + } + throw createError; + } +} + +// Resolve by the immutable creation name, not through the admitted-lease cache: +// the SDK's returned sandbox ID differs from this provisional name. Persist a +// verified provider ID before host-driven deletion. Its later absence is then +// conclusive, even if the deletion receipt or database update was lost. +async function destroyFailedCreation( + config: DaytonaDriverConfig, cleanup: PluginEnvironmentCreationCleanup, + requireDurableObservation = false, +): Promise { + if (cleanup.providerLeaseId !== `paperclip-create-${cleanup.attemptId}` || + cleanup.accountFingerprint !== sandboxAccountDiscriminator(config) || + cleanup.labels["paperclip-provider"] !== "daytona" || + cleanup.labels["paperclip-create-attempt"] !== cleanup.attemptId || + cleanup.labels["paperclip-company-id"] !== cleanup.companyId || + cleanup.labels["paperclip-environment-id"] !== cleanup.environmentId || + (cleanup.runId !== undefined && cleanup.labels["paperclip-run-id"] !== cleanup.runId)) { + throw new Error("Failed-create sandbox ownership does not match"); + } + const client = createDaytonaClient(config); + let orphan: Sandbox; + try { + orphan = await withLivenessTimeout("sandbox.failedCreateLookup", 10_000, + () => client.get(cleanup.observedProviderLeaseId ?? cleanup.providerLeaseId)); + } catch (error) { + // A name that has never been observed may still materialize. An ID already + // observed with exact ownership cannot materialize as a new allocation. + if (cleanup.observedProviderLeaseId && error instanceof DaytonaNotFoundError) return; + throw error; + } + if ((cleanup.observedProviderLeaseId && orphan.id !== cleanup.observedProviderLeaseId) || + orphan.name !== cleanup.providerLeaseId || Object.entries(cleanup.labels).some(([key, value]) => orphan.labels?.[key] !== value)) { + throw new Error("Failed-create sandbox ownership does not match"); + } + if (!cleanup.observedProviderLeaseId) { + cleanup.observedProviderLeaseId = orphan.id; + if (requireDurableObservation) { + // No destructive call yet: the host journals the observation and retries. + throw new PluginEnvironmentCreationCleanupError([], "Persist observed sandbox identity before cleanup", cleanup); + } + } + await withLivenessTimeout("sandbox.failedCreateDelete", 15_000, () => orphan.delete(10, true)); } // ─── Per-lease started-sandbox handle cache ────────────────────────────────── @@ -2403,6 +2470,17 @@ const plugin = definePlugin({ ): Promise { if (!params.providerLeaseId) return; const config = parseDriverConfig(params.config); + if (params.leaseMetadata?.failedCreateCleanup !== undefined) { + const cleanup = readEnvironmentCreationCleanupError({ data: { + schema: "paperclip/environment-creation-cleanup/v1", cleanup: params.leaseMetadata.failedCreateCleanup, + } }); + if (!cleanup || cleanup.companyId !== params.companyId || cleanup.environmentId !== params.environmentId || + cleanup.providerLeaseId !== params.providerLeaseId) { + throw new Error("Failed-create sandbox cleanup scope does not match"); + } + await destroyFailedCreation(config, cleanup, true); + return { providerLeaseId: params.providerLeaseId, state: "destroyed" }; + } const scope: SandboxScope = { driverKey: params.driverKey, companyId: params.companyId, diff --git a/packages/plugins/sdk/src/environment-creation-cleanup.ts b/packages/plugins/sdk/src/environment-creation-cleanup.ts new file mode 100644 index 00000000000..8dcdc1f768f --- /dev/null +++ b/packages/plugins/sdk/src/environment-creation-cleanup.ts @@ -0,0 +1,62 @@ +/** Non-secret ownership evidence for an allocation whose creation failed. */ +export interface PluginEnvironmentCreationCleanup { + providerLeaseId: string; + /** Provider ID observed with matching ownership, distinct from a provisional creation name. */ + observedProviderLeaseId?: string; + companyId: string; + environmentId: string; + runId?: string; + attemptId: string; + accountFingerprint: string; + labels: Record; +} + +const SCHEMA = "paperclip/environment-creation-cleanup/v1"; + +/** + * Never serialize the provider exceptions: they may contain credentials. + * The host persists only this explicit ownership envelope, then retries teardown. + */ +export class PluginEnvironmentCreationCleanupError extends AggregateError { + readonly cleanup: PluginEnvironmentCreationCleanup; + + constructor(errors: unknown[], message: string, cleanup: PluginEnvironmentCreationCleanup) { + super(errors, message); + this.name = "PluginEnvironmentCreationCleanupError"; + this.cleanup = cleanup; + } +} + +export function environmentCreationCleanupErrorData(error: unknown): unknown { + if (!(error instanceof PluginEnvironmentCreationCleanupError)) return undefined; + const cleanup = readEnvironmentCreationCleanupError(error); + return cleanup ? { schema: SCHEMA, cleanup } : undefined; +} + +export function readEnvironmentCreationCleanupError(error: unknown): PluginEnvironmentCreationCleanup | null { + const data = error instanceof PluginEnvironmentCreationCleanupError + ? { schema: SCHEMA, cleanup: error.cleanup } + : error && typeof error === "object" && "data" in error ? error.data : null; + if (!data || typeof data !== "object" || !("schema" in data) || data.schema !== SCHEMA || !("cleanup" in data)) return null; + const value = data.cleanup; + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const row = value as Record; + const identifier = (value: unknown): value is string => typeof value === "string" && /^[a-zA-Z0-9_-]{1,200}$/.test(value); + if (![row.providerLeaseId, row.companyId, row.environmentId, row.attemptId].every(identifier)) return null; + if (row.runId !== undefined && !identifier(row.runId)) return null; + if (row.observedProviderLeaseId !== undefined && !identifier(row.observedProviderLeaseId)) return null; + if (typeof row.accountFingerprint !== "string" || !/^[a-f0-9]{64}$/.test(row.accountFingerprint)) return null; + if (!row.labels || typeof row.labels !== "object" || Array.isArray(row.labels)) return null; + const labels = row.labels as Record; + if (Object.keys(labels).length > 16 || Object.entries(labels).some(([key, value]) => !/^paperclip-[a-z-]+$/.test(key) || !identifier(value))) return null; + return { + providerLeaseId: row.providerLeaseId as string, + ...(typeof row.observedProviderLeaseId === "string" ? { observedProviderLeaseId: row.observedProviderLeaseId } : {}), + companyId: row.companyId as string, + environmentId: row.environmentId as string, + ...(typeof row.runId === "string" ? { runId: row.runId } : {}), + attemptId: row.attemptId as string, + accountFingerprint: row.accountFingerprint, + labels: { ...labels } as Record, + }; +} diff --git a/packages/plugins/sdk/src/index.ts b/packages/plugins/sdk/src/index.ts index 1cf5adf0dda..788247f39ad 100644 --- a/packages/plugins/sdk/src/index.ts +++ b/packages/plugins/sdk/src/index.ts @@ -448,3 +448,6 @@ export { MEMBERSHIP_STATUSES, PRINCIPAL_TYPES, } from "@paperclipai/shared"; + +export { PluginEnvironmentCreationCleanupError, environmentCreationCleanupErrorData, readEnvironmentCreationCleanupError } from "./environment-creation-cleanup.js"; +export type { PluginEnvironmentCreationCleanup } from "./environment-creation-cleanup.js"; diff --git a/packages/plugins/sdk/src/worker-rpc-host.ts b/packages/plugins/sdk/src/worker-rpc-host.ts index 5e56ef200ca..edf9634901c 100644 --- a/packages/plugins/sdk/src/worker-rpc-host.ts +++ b/packages/plugins/sdk/src/worker-rpc-host.ts @@ -1,3 +1,4 @@ +import { environmentCreationCleanupErrorData } from "./environment-creation-cleanup.js"; /** * Worker-side RPC host — runs inside the child process spawned by the host. * @@ -1571,7 +1572,9 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost ? (err as any).code : PLUGIN_RPC_ERROR_CODES.WORKER_ERROR; - sendMessage(createErrorResponse(id, errorCode, errorMessage)); + sendMessage(createErrorResponse(id, errorCode, errorMessage, + method === "environmentAcquireLease" || method === "environmentDestroyLease" + ? environmentCreationCleanupErrorData(err) : undefined)); } } diff --git a/packages/plugins/sdk/tests/environment-creation-cleanup.test.ts b/packages/plugins/sdk/tests/environment-creation-cleanup.test.ts new file mode 100644 index 00000000000..e75f7877ee0 --- /dev/null +++ b/packages/plugins/sdk/tests/environment-creation-cleanup.test.ts @@ -0,0 +1,75 @@ +import { createInterface } from "node:readline"; +import { PassThrough } from "node:stream"; +import { describe, expect, it } from "vitest"; +import { definePlugin } from "../src/define-plugin.js"; +import { PluginEnvironmentCreationCleanupError, environmentCreationCleanupErrorData, readEnvironmentCreationCleanupError } from "../src/environment-creation-cleanup.js"; +import { createRequest, isJsonRpcResponse, JsonRpcCallError, parseMessage, serializeMessage, type JsonRpcResponse } from "../src/protocol.js"; +import { startWorkerRpcHost } from "../src/worker-rpc-host.js"; + +const ownership = { + providerLeaseId: "paperclip-create-attempt-1", companyId: "company-1", environmentId: "environment-1", + runId: "run-1", attemptId: "attempt-1", accountFingerprint: "a".repeat(64), + labels: { "paperclip-provider": "daytona" }, +}; +const schema = "paperclip/environment-creation-cleanup/v1"; + +async function invoke(method: string, error: Error) { + const stdin = new PassThrough(); + const stdout = new PassThrough(); + const lines = createInterface({ input: stdout }); + const worker = startWorkerRpcHost({ stdin, stdout, plugin: definePlugin({ + async setup() {}, + async onEnvironmentAcquireLease() { throw error; }, + async onEnvironmentProbe() { throw error; }, + async onEnvironmentDestroyLease() { throw error; }, + }) }); + try { + const response = new Promise((resolve) => { + lines.on("line", (line) => { + const message = parseMessage(line); + if (isJsonRpcResponse(message)) resolve(message); + }); + }); + stdin.write(serializeMessage(createRequest(method, {}, "cleanup-test"))); + return await response; + } finally { + worker.stop(); lines.close(); stdin.destroy(); stdout.destroy(); + } +} + +describe("failed environment creation ownership", () => { + it.each(["environmentAcquireLease", "environmentDestroyLease"])("crosses %s RPC as allowlisted ownership only", async (method) => { + const cleanup = { ...ownership, accessToken: "secret-token", config: { apiKey: "secret-key" } }; + const error = new PluginEnvironmentCreationCleanupError([new Error("secret-provider-response")], "Cleanup required", cleanup); + const response = await invoke(method, error); + expect("error" in response && response.error).toMatchObject({ data: { schema, cleanup: ownership } }); + if (!("error" in response) || !response.error) throw new Error("Expected RPC error"); + expect(readEnvironmentCreationCleanupError(new JsonRpcCallError(response.error))).toEqual(ownership); + expect(JSON.stringify(response)).not.toContain("secret-"); + }); + + it.each(["environmentAcquireLease", "environmentDestroyLease", "environmentProbe"])("does not forward arbitrary error data for %s", async (method) => { + const response = await invoke(method, Object.assign(new Error("provider failure"), { data: { apiKey: "secret-key", schema, cleanup: ownership } })); + expect("error" in response && response.error).not.toHaveProperty("data"); + }); + + it("only forwards the typed envelope on supported acquisition calls", async () => { + const response = await invoke("environmentProbe", new PluginEnvironmentCreationCleanupError([], "Cleanup required", ownership)); + expect("error" in response && response.error).not.toHaveProperty("data"); + }); + + it.each([ + { providerLeaseId: "../other" }, { companyId: "" }, { environmentId: null }, { attemptId: "a".repeat(201) }, + { runId: "other/run" }, { observedProviderLeaseId: "../other" }, { accountFingerprint: "secret-key" }, { labels: { apiKey: "secret-key" } }, + { labels: { "paperclip-provider": "Bearer secret-key" } }, { labels: [] }, + { labels: Object.fromEntries(Array.from({ length: 17 }, (_, i) => [`paperclip-${String.fromCharCode(97 + i)}`, "x"])) }, + ])("rejects malformed evidence %j", (invalid) => { + const cleanup = { ...ownership, ...invalid } as typeof ownership; + expect(readEnvironmentCreationCleanupError({ data: { schema, cleanup } })).toBeNull(); + expect(environmentCreationCleanupErrorData(new PluginEnvironmentCreationCleanupError([], "invalid", cleanup))).toBeUndefined(); + }); + + it.each([null, {}, { data: { schema: "unknown", cleanup: ownership } }, { data: { schema, cleanup: [] } }])("rejects malformed envelopes %j", (value) => { + expect(readEnvironmentCreationCleanupError(value)).toBeNull(); + }); +}); diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index 37e8d315254..285d01c4dbe 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -653,6 +653,127 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { expect(call.mock.calls.filter((entry) => entry[1] === "environmentResumeLease")).toHaveLength(shouldResume ? 1 : 0); }); + it.each(["pending", "inline", "foreign scope", "foreign environment", "foreign run", "malformed", "deleted environment"])("retains uncertain plugin creation for durable cleanup: %s", async (mode) => { + const seeded = await seedReusablePluginSandboxLease(); + await environmentService(db).releaseLease(seeded.reusableLease.id, "expired"); + const attemptId = randomUUID(); + const providerLeaseId = `paperclip-create-${attemptId}`; + const cleanup = { + providerLeaseId, attemptId, companyId: mode === "foreign scope" ? randomUUID() : seeded.companyId, + environmentId: mode === "foreign environment" ? randomUUID() : seeded.environment.id, + runId: mode === "foreign run" ? randomUUID() : seeded.runId, + accountFingerprint: mode === "malformed" ? "invalid" : "a".repeat(64), labels: { "paperclip-provider": "fake-plugin" }, + }; + const failure = Object.assign(new Error("Provider creation cleanup required"), { + data: { schema: "paperclip/environment-creation-cleanup/v1", cleanup }, + }); + let available = mode === "inline"; + const call = vi.fn(async (_id: string, method: string, params: Record) => { + if (method === "environmentAcquireLease") throw failure; + if (method === "environmentDestroyLease") { + const pending = await db.select().from(environmentLeases).where(eq(environmentLeases.providerLeaseId, providerLeaseId)); + expect(pending).toHaveLength(1); + expect(pending[0].status).toBe("pending_cleanup"); + expect(params).toMatchObject({ providerLeaseId, companyId: seeded.companyId, environmentId: seeded.environment.id, leaseMetadata: { failedCreateCleanup: cleanup } }); + if (!available) throw new Error("provider not visible yet"); + return { providerLeaseId, state: "destroyed" }; + } + throw new Error(`Unexpected method ${method}`); + }); + const worker = { isRunning: () => true, getWorker: () => ({ supportedMethods: ["environmentDestroyLease"] }), call } as unknown as PluginWorkerManager; + const candidate = environmentRuntimeService(db, { pluginWorkerManager: worker }); + const acquisition = candidate.acquireRunLease({ companyId: seeded.companyId, environment: seeded.environment, + issueId: null, heartbeatRunId: seeded.runId, persistedExecutionWorkspace: null }); + if (mode === "inline") { + await expect(acquisition).rejects.toMatchObject({ + message: "Sandbox creation failed; allocated sandbox cleanup was confirmed.", cause: failure, + }); + } else { + await expect(acquisition).rejects.toBe(failure); + } + const rows = await db.select().from(environmentLeases).where(eq(environmentLeases.providerLeaseId, providerLeaseId)); + if (["foreign scope", "foreign environment", "foreign run", "malformed"].includes(mode)) { + expect(rows).toHaveLength(0); + expect(call.mock.calls.filter((c) => c[1] === "environmentDestroyLease")).toHaveLength(0); + return; + } + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ status: mode === "inline" ? "expired" : "pending_cleanup", + cleanupStatus: mode === "inline" ? "success" : "failed", companyId: seeded.companyId, + metadata: { pluginId: seeded.pluginId, sandboxProviderPlugin: true, failedCreateCleanup: cleanup } }); + if (mode === "pending" || mode === "deleted environment") { + available = true; + if (mode === "deleted environment") { + await db.update(environmentLeases).set({ environmentId: null }).where(eq(environmentLeases.id, rows[0].id)); + } + const restarted = environmentRuntimeService(db, { pluginWorkerManager: worker }); + const persisted = await environmentService(db).getLeaseById(rows[0].id); + await expect(restarted.retryPendingSandboxTeardown({ environment: null, lease: persisted! })).resolves.toEqual({ providerLeaseId, state: "destroyed" }); + } + }); + + it.each(["deleted receipt lost", "observation response lost", "journal write failed", "foreign observation"])( + "journals observed creation identity before deletion and recovers: %s", async (fault) => { + const seeded = await seedReusablePluginSandboxLease(); + await environmentService(db).releaseLease(seeded.reusableLease.id, "expired"); + const cleanup = { + providerLeaseId: `paperclip-create-${randomUUID()}`, attemptId: randomUUID(), + companyId: seeded.companyId, environmentId: seeded.environment.id, runId: seeded.runId, + accountFingerprint: "a".repeat(64), labels: { "paperclip-provider": "fake-plugin" }, + }; + const observed = { ...cleanup, observedProviderLeaseId: "verified-provider-id" }; + const failure = Object.assign(new Error("Creation uncertain"), { + data: { schema: "paperclip/environment-creation-cleanup/v1", cleanup }, + }); + let injected = false; + let deleted = false; + const call = vi.fn(async (_id: string, method: string, params: Record) => { + if (method === "environmentAcquireLease") throw failure; + if (method !== "environmentDestroyLease") throw new Error(`Unexpected ${method}`); + const evidence = (params.leaseMetadata as { failedCreateCleanup: typeof observed }).failedCreateCleanup; + if (!evidence.observedProviderLeaseId) { + expect(deleted).toBe(false); + if (!injected && fault === "observation response lost") { + injected = true; + throw new Error("Lost observation response"); + } + if (!injected && fault === "foreign observation") { + injected = true; + throw Object.assign(new Error("Foreign observation"), { + data: { schema: "paperclip/environment-creation-cleanup/v1", cleanup: { ...observed, companyId: "another-company" } }, + }); + } + if (!injected && fault === "journal write failed") { + injected = true; + vi.spyOn(db, "update").mockImplementationOnce(() => { throw new Error("Database unavailable"); }); + } + throw Object.assign(new Error("Persist observed sandbox identity before cleanup"), { + data: { schema: "paperclip/environment-creation-cleanup/v1", cleanup: observed }, + }); + } + const rows = await db.select().from(environmentLeases).where(eq(environmentLeases.providerLeaseId, cleanup.providerLeaseId)); + expect(rows).toHaveLength(1); + expect(rows[0].metadata).toMatchObject({ failedCreateCleanup: observed }); + if (!deleted) { + deleted = true; + if (fault === "deleted receipt lost") throw new Error("Deletion succeeded but reply lost"); + } + return { providerLeaseId: cleanup.providerLeaseId, state: "destroyed" }; + }); + const worker = { isRunning: () => true, getWorker: () => ({ supportedMethods: ["environmentDestroyLease"] }), call } as unknown as PluginWorkerManager; + const runtime = environmentRuntimeService(db, { pluginWorkerManager: worker }); + await expect(runtime.acquireRunLease({ companyId: seeded.companyId, environment: seeded.environment, + issueId: null, heartbeatRunId: seeded.runId, persistedExecutionWorkspace: null })).rejects.toBe(failure); + const rows = await db.select().from(environmentLeases).where(eq(environmentLeases.providerLeaseId, cleanup.providerLeaseId)); + expect(rows).toHaveLength(1); + expect(rows[0].status).toBe("pending_cleanup"); + expect(deleted).toBe(fault === "deleted receipt lost"); + const restarted = environmentRuntimeService(db, { pluginWorkerManager: worker }); + await expect(restarted.retryPendingSandboxTeardown({ environment: null, lease: rows[0]! })).resolves.toEqual({ providerLeaseId: cleanup.providerLeaseId, state: "destroyed" }); + expect(deleted).toBe(true); + }, + ); + it("resumes the same provider lease on the next per-turn run", async () => { const seeded = await seedReusablePluginSandboxLease(); const workerManager = { diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index 0a8f5ffc301..e5956c1e393 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -1,3 +1,4 @@ +import { readEnvironmentCreationCleanupError } from "@paperclipai/plugin-sdk"; import { remoteTerminationReceipt } from "./remote-execution-termination.js"; import { createHash, randomUUID } from "node:crypto"; import { and, eq, inArray, sql } from "drizzle-orm"; @@ -1555,7 +1556,38 @@ function createSandboxEnvironmentDriver( }); }; - // Clean up an orphan sandbox after the conditional lease insert rejected it. + // Resolve an uncertain creation to a verified provider ID, journal it, then + // authorize deletion. This makes a later missing-ID response conclusive even + // when the destructive RPC reply or the final lease update was lost. + const destroyWithCreationObservation = async (input: { + leaseId: string | null; + metadata: Record; + teardown: (metadata: Record) => Promise; + }): Promise => { + try { + return await input.teardown(input.metadata); + } catch (error) { + const previous = readEnvironmentCreationCleanupError({ data: { + schema: "paperclip/environment-creation-cleanup/v1", cleanup: input.metadata.failedCreateCleanup, + } }); + const observed = readEnvironmentCreationCleanupError(error); + const scopeKeys = ["providerLeaseId", "companyId", "environmentId", "runId", "attemptId", "accountFingerprint"] as const; + if (!input.leaseId || !previous || previous.observedProviderLeaseId || !observed?.observedProviderLeaseId || + scopeKeys.some((key) => previous[key] !== observed[key]) || + Object.keys(previous.labels).length !== Object.keys(observed.labels).length || + Object.entries(previous.labels).some(([key, value]) => observed.labels[key] !== value)) throw error; + const lease = await environmentsSvc.getLeaseById(input.leaseId); + if (!lease || lease.companyId !== previous.companyId || lease.providerLeaseId !== previous.providerLeaseId || + (lease.environmentId !== null && lease.environmentId !== previous.environmentId)) throw error; + const metadata = { ...lease.metadata, failedCreateCleanup: observed }; + // A failed write must stop here; no destructive call is permitted until + // the provider identity can survive a host or worker restart. + if (!await environmentsSvc.updateLeaseMetadata(lease.id, metadata)) throw error; + return await input.teardown(metadata); + } + }; + + // Clean up an orphan after a rejected insert or uncertain provider creation. // The acquire provisioned a remote sandbox, then the insert rejected (a // foreign-company binding), so no lease row tracks the live sandbox. This // handler records the durable `pending_cleanup` row FIRST, before the inline @@ -1578,14 +1610,14 @@ function createSandboxEnvironmentDriver( record: DeferredOrphanCleanupRecord; cause: unknown; canTeardown: boolean; - teardown: () => Promise; - }): Promise => { + teardown: (pendingLeaseId: string | null) => Promise; + }): Promise => { const durable = await tryWriteDurablePendingCleanup(input.record); let teardownFailed = !input.canTeardown; let receipt: unknown; if (!teardownFailed) { try { - receipt = await input.teardown(); + receipt = await input.teardown(durable.leaseId); } catch { teardownFailed = true; } @@ -1595,15 +1627,15 @@ function createSandboxEnvironmentDriver( if (durable.leaseId !== null) { await releaseCleanedUpOrphanRow(durable.leaseId, orphanDiagnosticFields(input.record), receipt); } - return; + return true; } // The teardown failed, so the orphan sandbox is still live. if (durable.leaseId !== null) { // The durable row already tracks the orphan, so a sweep finds and releases // it. The caller rethrows the original insert rejection. - return; + return false; } - await escalateUnwrittenOrphan(input.record, input.cause, durable.lastError); + return await escalateUnwrittenOrphan(input.record, input.cause, durable.lastError); }; // The run-time exec parent context, held per lease id. A plugin sandbox @@ -2019,38 +2051,86 @@ function createSandboxEnvironmentDriver( }); } } - const acquiredLease = providerLease ?? await pluginWorkerManager.call( - pluginProvider.resolved.plugin.id, - "environmentAcquireLease", - { - driverKey: parsed.config.provider, - companyId: input.companyId, - environmentId: input.environment.id, - issueId: input.issueId, - config: workerConfig, - // Plugin SDK requires a string; ad-hoc test leases use a fresh - // UUID so providers that validate or persist the runId still see - // a well-formed identifier. - runId: input.heartbeatRunId ?? randomUUID(), - workspaceMode: input.executionWorkspaceMode ?? undefined, - agentId: input.agentId ?? undefined, - executionWorkspaceId: input.executionWorkspaceId ?? undefined, - // The agent's harness for THIS run, so the plugin picks the matching - // runtime image (per-run adapter, mixed-harness environments). - // NOTE: environment-runtime.ts has TWO drivers calling - // environmentAcquireLease; this plugin-sandbox one is the HEARTBEAT - // path. Omitting adapterType here silently falls back to the - // environment's default adapter image (a pi agent then runs in the - // opencode image and the harness binary is missing at exec time). - adapterType: input.adapterType ?? undefined, - // Forward a caller deadline so the provider configures a provider-side - // expiry at or before it and returns the real provider expiry. - ...(requestedExpiresAtParam(input.requestedExpiresAt) !== undefined - ? { requestedExpiresAt: requestedExpiresAtParam(input.requestedExpiresAt) } - : {}), - }, - resolvePluginSandboxRpcTimeoutMs(workerConfig), - ); + const acquisitionRunId = input.heartbeatRunId ?? randomUUID(); + const acquiredLease = providerLease ?? await (async () => { + try { + return await pluginWorkerManager.call( + pluginProvider.resolved.plugin.id, + "environmentAcquireLease", + { + driverKey: parsed.config.provider, + companyId: input.companyId, + environmentId: input.environment.id, + issueId: input.issueId, + config: workerConfig, + // Plugin SDK requires a string; ad-hoc test leases use a fresh + // UUID so providers that validate or persist the runId still see + // a well-formed identifier. + runId: acquisitionRunId, + workspaceMode: input.executionWorkspaceMode ?? undefined, + agentId: input.agentId ?? undefined, + executionWorkspaceId: input.executionWorkspaceId ?? undefined, + // The agent's harness for THIS run, so the plugin picks the matching + // runtime image (per-run adapter, mixed-harness environments). + // NOTE: environment-runtime.ts has TWO drivers calling + // environmentAcquireLease; this plugin-sandbox one is the HEARTBEAT + // path. Omitting adapterType here silently falls back to the + // environment's default adapter image (a pi agent then runs in the + // opencode image and the harness binary is missing at exec time). + adapterType: input.adapterType ?? undefined, + // Forward a caller deadline so the provider configures a provider-side + // expiry at or before it and returns the real provider expiry. + ...(requestedExpiresAtParam(input.requestedExpiresAt) !== undefined + ? { requestedExpiresAt: requestedExpiresAtParam(input.requestedExpiresAt) } + : {}), + }, + resolvePluginSandboxRpcTimeoutMs(workerConfig), + ); + } catch (error) { + const cleanup = readEnvironmentCreationCleanupError(error); + // The authenticated worker may identify its uncertain allocation, + // but cannot redirect cleanup to another company, environment, or run. + if (cleanup && cleanup.companyId === input.companyId && + cleanup.environmentId === input.environment.id && cleanup.runId === acquisitionRunId) { + const cleanupMetadata = { + ...sandboxConfigForLeaseMetadata(storedConfig), + failedCreateCleanup: cleanup, + driver: input.environment.driver, + pluginId: pluginProvider.resolved.plugin.id, + pluginKey: pluginProvider.resolved.plugin.pluginKey, + sandboxProviderPlugin: true, + }; + // Record before retrying the provider. Existing pending-cleanup + // recovery (including its durable spool) survives controller loss. + const cleanupConfirmed = await cleanUpRejectedOrphanSandbox({ + record: { + companyId: input.companyId, environmentId: input.environment.id, + executionWorkspaceId: input.executionWorkspaceId ?? null, + issueId: input.issueId ?? null, heartbeatRunId: input.heartbeatRunId ?? null, + provider: parsed.config.provider, providerLeaseId: cleanup.providerLeaseId, + metadata: cleanupMetadata, + }, + cause: error, + canTeardown: pluginWorkerManager.isRunning(pluginProvider.resolved.plugin.id), + teardown: (pendingLeaseId) => destroyWithCreationObservation({ + leaseId: pendingLeaseId, metadata: cleanupMetadata, + teardown: (metadata) => pluginWorkerManager.call(pluginProvider.resolved.plugin.id, "environmentDestroyLease", { + driverKey: parsed.config.provider, companyId: input.companyId, + environmentId: input.environment.id, issueId: input.issueId, + config: workerConfig, providerLeaseId: cleanup.providerLeaseId, + leaseMetadata: metadata, + }, resolvePluginSandboxRpcTimeoutMs(workerConfig)), + }), + }); + if (cleanupConfirmed) { + throw new Error("Sandbox creation failed; allocated sandbox cleanup was confirmed.", { cause: error }); + } + } + // A cleanup record is not a usable lease. Keep acquisition failed, + // even when its compensating deletion succeeds on this attempt. + throw error; + } + })(); // Ad-hoc test leases are never publishable for reuse: storing them // as `reuse_by_environment` would let a concurrent heartbeat resume @@ -2524,23 +2604,35 @@ function createSandboxEnvironmentDriver( { issueId: input.lease.issueId, heartbeatRunId: input.lease.heartbeatRunId }, ); const workerConfig = stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig); - return await pluginWorkerManager.call( + const failedCreation = readEnvironmentCreationCleanupError({ data: { + schema: "paperclip/environment-creation-cleanup/v1", + cleanup: input.lease.metadata?.failedCreateCleanup, + } }); + // An environment deletion clears the lease FK. Retain the original + // ownership scope for uncertain allocations without accepting a foreign + // company, provider name, or changed environment from metadata. + if (failedCreation && (failedCreation.companyId !== input.lease.companyId || + failedCreation.providerLeaseId !== input.lease.providerLeaseId || + (input.lease.environmentId !== null && failedCreation.environmentId !== input.lease.environmentId))) { + throw new Error("Pending creation cleanup ownership does not match the lease"); + } + const teardown = (metadata: Record) => pluginWorkerManager.call( pluginProvider.resolved.plugin.id, "environmentDestroyLease", { driverKey: recordedProvider, companyId: input.lease.companyId, - // The provider teardown keys on the provider lease id. The - // environment id is only context, and it is empty when a delete - // removed the environment before this orphan was recorded. - environmentId: input.lease.environmentId ?? input.environment?.id ?? "", + environmentId: failedCreation?.environmentId ?? input.lease.environmentId ?? input.environment?.id ?? "", issueId: input.lease.issueId, config: workerConfig, providerLeaseId: input.lease.providerLeaseId, - leaseMetadata: input.lease.metadata ?? undefined, + leaseMetadata: metadata, }, resolvePluginSandboxRpcTimeoutMs(workerConfig), ); + return failedCreation + ? await destroyWithCreationObservation({ leaseId: input.lease.id, metadata: input.lease.metadata ?? {}, teardown }) + : await teardown(input.lease.metadata ?? {}); } // Built-in provider path. Resolve the recorded config secrets through the