diff --git a/packages/plugins/sandbox-providers/daytona/README.md b/packages/plugins/sandbox-providers/daytona/README.md index a7e0198f8c9..e8dbbc436f8 100644 --- a/packages/plugins/sandbox-providers/daytona/README.md +++ b/packages/plugins/sandbox-providers/daytona/README.md @@ -27,6 +27,7 @@ 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 lookup after an uncertain create is not proof that a delayed provider request cannot create a resource. - 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..939ac0066fa 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts @@ -523,6 +523,105 @@ 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 } }; + } + + 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..b7714494536 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.ts @@ -942,18 +942,41 @@ 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 }; + const createParams = { ...buildCreateParams(config, labels), name }; + try { + return await client.create(createParams, { + timeout: toTimeoutSeconds(config.timeoutMs), + }); + } catch (createError) { + 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. + const orphan = await withLivenessTimeout("sandbox.failedCreateLookup", 10_000, () => client.get(name)); + if (orphan.name !== name || Object.entries(labels).some(([key, value]) => orphan.labels?.[key] !== value)) { + throw new Error("Failed-create sandbox ownership does not match"); + } + // Wait for provider deletion, rather than treating a deletion request as + // a cleanup receipt. Never run tools or install credentials in this sandbox. + await withLivenessTimeout("sandbox.failedCreateDelete", 15_000, () => orphan.delete(10, true)); + } catch (cleanupError) { + throw new AggregateError([createError, cleanupError], + `Daytona sandbox creation failed; cleanup could not be confirmed for ${name}`); + } + throw createError; + } } // ─── Per-lease started-sandbox handle cache ──────────────────────────────────