From 3d23c3b2c30179402200924bb2c8c6fd8145adb8 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Mon, 27 Jul 2026 19:14:53 -0700 Subject: [PATCH] perf(plugin-daytona): cache the started sandbox handle per lease (#10335) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Daytona sandbox adapter spends time on repeated per-exec sandbox lookups during a lease > - That repeated lookup is pure overhead once the started sandbox handle is already known and trusted for the lease > - The adapter still needs strict isolation and fail-closed behavior because the handle is an authenticated compute object, not an inert value > - This pull request memoizes the started sandbox handle per lease in a process-scoped cache, and now advances freshness only after successful reuse so failed commands cannot suppress stale-handle refreshes > - The benefit is lower provider-get latency on the hot path while keeping resume safety, teardown safety, and observability intact ## Linked Issues or Issue Description This is a Daytona performance fix, not a standalone public GitHub issue. ### Problem / Motivation - Repeated `client.get(sandboxId)` calls on the sandbox hot path re-fetch a handle that is already started and trusted for the current lease. - The extra provider round-trip is pure overhead on repeated exec, sync, resume, and interactive-cancel flows. - Cache freshness also has to be tied to successful reuse, or a failed command can make a stale sandbox look freshly used. ### Proposed Solution - Cache the started `Sandbox` handle in process memory, keyed by a non-secret composite lease scope. - Enforce strict identity checks and eviction on release, destroy, interactive cancel, and resume-sentinel mismatch. - Block teardown cleanup until active lease operations finish so delete/stop cannot race in-flight execute or sync work. - Advance freshness only after successful execute, sync, or resume reuse, so failed operations do not mask an auto-stopped sandbox. - Preserve `getDurationMs` in exec metadata so provider-get latency remains observable. ### Alternatives Considered - Keep fetching the sandbox on every exec path. - Cache only by bare lease id. ### Roadmap Alignment - This change is part of the Daytona performance work and narrows per-call overhead without changing the public adapter contract. ## What Changed - Memoized the started Daytona `Sandbox` handle in a process-scoped cache keyed by a composite lease scope. - Added fail-closed identity checks so cache hits and single-flight populate paths reject mismatched sandbox ids. - Evicted cached handles on release, destroy, interactive cancel, and resume sentinel mismatch. - Blocked release, destroy, and interactive cancel teardown cleanup until active lease operations drain. - Advanced cache freshness only after successful execute, sync, and resume reuse. - Kept `getDurationMs` in exec metadata so provider-get latency remains observable. - Expanded the Daytona plugin tests to cover same-lease reuse, cross-scope isolation, eviction paths, rejected populate handling, concurrent single-flight behavior, cached-resume sentinel revalidation, teardown cancellation safety, and failed-execute freshness handling. ## Verification - `pnpm exec vitest run --config vitest.config.ts` in `packages/plugins/sandbox-providers/daytona` — 81/81 passing, including the teardown-cancel, snapshot-capture, syncIn-cancel, and failed-execute freshness regressions. - `git rev-parse origin/perf/daytona-sandbox-handle-cache` matched the authorized submit SHA `528158f998fa88bb0748300f156b38d86d6589cd` before the fixup commits. - `git log --oneline origin/master..origin/perf/daytona-sandbox-handle-cache` shows the expected focused Daytona changes. - GitHub duplicate/related PR search and ROADMAP review were completed before opening the PR. - Remote CI and Greptile completed successfully after this description was updated; the PR is now ready for board handoff. ## Risks - The cache is process-scoped, so correctness depends on the eviction paths staying complete. - A bug in the scope key or identity checks could leak reuse across the wrong lease boundaries, but the implementation fails closed on id mismatches. - Teardown now waits for active operations to drain, so any missed activity bookkeeping could delay cleanup instead of racing it. - Freshness updates now happen after success, which is safer, but it means any missed success-path call would trigger an extra refresh rather than silently masking staleness. ## Model Used OpenAI GPT-5 via Codex, tool-using coding agent; exact context window not surfaced in the workspace. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../daytona/src/plugin.test.ts | 776 ++++++++++++++++- .../sandbox-providers/daytona/src/plugin.ts | 796 +++++++++++++++--- 2 files changed, 1442 insertions(+), 130 deletions(-) diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts index 4fb9a13a2f..fe33be90cb 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts @@ -28,7 +28,11 @@ vi.mock("@daytonaio/sdk", () => ({ DaytonaTimeoutError: MockDaytonaTimeoutError, })); -import plugin, { setDaytonaTimingClockForTest } from "./plugin.js"; +import plugin, { + setDaytonaTimingClockForTest, + setDaytonaHandleFreshnessClockForTest, + __resetDaytonaSandboxHandleCacheForTest, +} from "./plugin.js"; import manifest from "./manifest.js"; function createMockSandbox(overrides: { @@ -50,6 +54,10 @@ function createMockSandbox(overrides: { start: vi.fn().mockResolvedValue(undefined), stop: vi.fn().mockResolvedValue(undefined), recover: vi.fn().mockResolvedValue(undefined), + // Real `refreshData` re-reads live provider state and mutates `state` in + // place; the default mock leaves state untouched, and tests that exercise a + // provider-initiated auto-stop override it to flip `state` to "stopped". + refreshData: vi.fn().mockResolvedValue(undefined), resize: vi.fn().mockResolvedValue(undefined), delete: vi.fn().mockResolvedValue(undefined), archive: vi.fn().mockResolvedValue(undefined), @@ -87,6 +95,9 @@ describe("Daytona sandbox provider plugin", () => { mockSnapshotDelete.mockReset(); vi.restoreAllMocks(); delete process.env.DAYTONA_API_KEY; + // The started-sandbox handle cache is process-scoped; clear it between tests + // so a handle memoized under a reused composite key never leaks forward. + __resetDaytonaSandboxHandleCacheForTest(); }); it("declares environment lifecycle handlers", async () => { @@ -1295,6 +1306,767 @@ describe("Daytona sandbox provider plugin", () => { expect(result).toMatchObject({ exitCode: null, timedOut: true }); expect(result?.stderr).toMatch(/unreachable|credentials/i); }); + + // ─── Per-lease started-sandbox handle cache ──────────────────────────────── + // These prove the security conditions: single-fetch-per-lease, strict + // composite-key isolation (no cross-lease / cross-company / cross-env reuse), + // eviction at every teardown, no caching of failed populates, single-flight + // concurrency, and sentinel re-verification on a cached resume — plus that a + // handle left idle past the provider auto-stop window is refreshed before + // reuse so a provider-initiated stop is not hidden behind a stale snapshot. + describe("started-sandbox handle cache", () => { + function execParams( + providerLeaseId: string, + overrides: { companyId?: string; environmentId?: string; driverKey?: string } = {}, + ) { + return { + driverKey: overrides.driverKey ?? "daytona", + companyId: overrides.companyId ?? "company-1", + environmentId: overrides.environmentId ?? "env-1", + config: { timeoutMs: 300000, reuseLease: false }, + lease: { providerLeaseId, metadata: {} }, + command: "printf", + args: ["hi"], + timeoutMs: 1000, + }; + } + + it("reuses the cached handle across execs on one lease (single client.get)", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + + // Second exec is served from the cache: no second REST re-fetch. + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(2); + }); + + it("keeps getDurationMs present (≈0) on a cache hit", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + const hit = await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + + // The observability contract holds even when the fetch is elided. + expect(typeof (hit!.metadata as Record)?.getDurationMs).toBe("number"); + }); + + it("never serves lease A's handle to lease B (distinct fetch per lease)", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandboxA = createMockSandbox({ id: "lease-a" }); + const sandboxB = createMockSandbox({ id: "lease-b" }); + mockGet.mockImplementation(async (id: string) => (id === "lease-a" ? sandboxA : sandboxB)); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await plugin.definition.onEnvironmentExecute?.(execParams("lease-b")); + + expect(mockGet).toHaveBeenCalledTimes(2); + expect(sandboxA.process.executeCommand).toHaveBeenCalledTimes(1); + expect(sandboxB.process.executeCommand).toHaveBeenCalledTimes(1); + }); + + it("does not share a handle across companies or environments for the same providerLeaseId", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + mockGet.mockImplementation(async () => createMockSandbox({ id: "sandbox-x" })); + + await plugin.definition.onEnvironmentExecute?.(execParams("sandbox-x", { companyId: "company-1", environmentId: "env-1" })); + await plugin.definition.onEnvironmentExecute?.(execParams("sandbox-x", { companyId: "company-2", environmentId: "env-1" })); + await plugin.definition.onEnvironmentExecute?.(execParams("sandbox-x", { companyId: "company-1", environmentId: "env-2" })); + + // Three distinct composite keys → three independent fetches; the bare + // providerLeaseId is never a shared cache slot. + expect(mockGet).toHaveBeenCalledTimes(3); + }); + + it("rejects a queued execute after release teardown closes the lease", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + mockGet.mockImplementation(async () => createMockSandbox({ id: "lease-a" })); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); // miss → get #1 (cached) + const releasePromise = plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: false }, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const queuedExecute = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await expect(queuedExecute).rejects.toThrow(/no longer active/); + await releasePromise; + + // The tombstone closes the lease, so the queued execute never reacquires + // the sandbox after teardown. + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it("rejects an overlapping execute after release teardown closes the lease", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + let stopStarted = false; + let resolveRelease!: () => void; + const releaseGate = new Promise((resolve) => { + resolveRelease = resolve; + }); + sandbox.stop.mockImplementation(() => { + stopStarted = true; + return releaseGate; + }); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + const releasePromise = plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: true }, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(stopStarted).toBe(true); + + const overlappingExec = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await expect(overlappingExec).rejects.toThrow(/no longer active/); + resolveRelease(); + await releasePromise; + + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.stop).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(1); + }); + + it("rejects a late execute after the release tombstone is set", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + let stopStarted = false; + let resolveRelease!: () => void; + const releaseGate = new Promise((resolve) => { + resolveRelease = resolve; + }); + sandbox.stop.mockImplementation(() => { + stopStarted = true; + return releaseGate; + }); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + const releasePromise = plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: true }, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(stopStarted).toBe(true); + + const overlappingExec = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await expect(overlappingExec).rejects.toThrow(/no longer active/); + + resolveRelease(); + await releasePromise; + + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.stop).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(1); + }); + + it("waits for an in-flight execute before teardown cleanup starts", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + let resolveExecute!: () => void; + sandbox.process.executeCommand.mockImplementation(async () => { + await new Promise((resolve) => { + resolveExecute = resolve; + }); + return { + exitCode: 0, + result: "bash", + artifacts: { stdout: "bash" }, + }; + }); + mockGet.mockResolvedValue(sandbox); + + const executePromise = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const releasePromise = plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: true }, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(sandbox.stop).not.toHaveBeenCalled(); + + resolveExecute(); + await Promise.all([executePromise, releasePromise]); + + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(1); + expect(sandbox.stop).toHaveBeenCalledTimes(1); + }); + + it("keeps the teardown gate closed until overlapping teardowns both finish", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + let stopStarted = false; + let deleteStarted = false; + let resolveStop!: () => void; + let resolveDelete!: () => void; + const stopGate = new Promise((resolve) => { + resolveStop = resolve; + }); + const deleteGate = new Promise((resolve) => { + resolveDelete = resolve; + }); + sandbox.stop.mockImplementation(() => { + stopStarted = true; + return stopGate; + }); + sandbox.delete.mockImplementation(() => { + deleteStarted = true; + return deleteGate; + }); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + const releasePromise = plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: true }, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(stopStarted).toBe(true); + + const destroyPromise = plugin.definition.onEnvironmentDestroyLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: true }, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(deleteStarted).toBe(true); + + resolveDelete(); + await destroyPromise; + + const overlappingExec = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await expect(overlappingExec).rejects.toThrow(/no longer active/); + + resolveStop(); + await releasePromise; + + expect(mockGet).toHaveBeenCalledTimes(2); + expect(sandbox.stop).toHaveBeenCalledTimes(1); + expect(sandbox.delete).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(1); + }); + + it("rejects a queued execute after destroy teardown closes the lease", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + mockGet.mockImplementation(async () => createMockSandbox({ id: "lease-a" })); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + const destroyPromise = plugin.definition.onEnvironmentDestroyLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: false }, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const queuedExecute = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await expect(queuedExecute).rejects.toThrow(/no longer active/); + await destroyPromise; + + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it("rejects a queued execute after interactive cancel closes the lease", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + mockGet.mockImplementation(async () => createMockSandbox({ id: "lease-a" })); + + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + const cancelPromise = plugin.definition.onEnvironmentCancelInteractiveSetup?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: false }, + reason: "cancelled", + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const queuedExecute = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await expect(queuedExecute).rejects.toThrow(/no longer active/); + await cancelPromise; + + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it("waits for an in-flight execute before interactive cancel cleanup starts", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + let resolveExecute!: () => void; + sandbox.process.executeCommand.mockImplementation(async () => { + await new Promise((resolve) => { + resolveExecute = resolve; + }); + return { + exitCode: 0, + result: "bash", + artifacts: { stdout: "bash" }, + }; + }); + mockGet.mockResolvedValue(sandbox); + + const executePromise = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const cancelPromise = plugin.definition.onEnvironmentCancelInteractiveSetup?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: false }, + reason: "cancelled", + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(sandbox.delete).not.toHaveBeenCalled(); + + resolveExecute(); + await Promise.all([executePromise, cancelPromise]); + + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(1); + expect(sandbox.delete).toHaveBeenCalledTimes(1); + }); + + it("waits for an in-flight syncIn before interactive cancel cleanup starts", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const hostDir = await fs.mkdtemp(path.join(os.tmpdir(), "daytona-cancel-sync-")); + const source = path.join(hostDir, "payload.txt"); + await fs.writeFile(source, "payload"); + const remoteDir = "/home/daytona/paperclip-workspace"; + + const sandbox = createMockSandbox({ id: "lease-a" }); + let resolveUpload!: () => void; + sandbox.fs.uploadFiles.mockImplementation(async () => { + await new Promise((resolve) => { + resolveUpload = resolve; + }); + }); + mockGet.mockResolvedValue(sandbox); + + const syncPromise = plugin.definition.onEnvironmentSyncIn?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + config: { timeoutMs: 300000, reuseLease: false }, + lease: { providerLeaseId: "lease-a", metadata: { remoteCwd: remoteDir } }, + operations: [ + { + operationId: "sync-op-1", + files: [{ sourcePath: source, targetPath: `${remoteDir}/payload.txt`, kind: "file" }], + }, + ], + }); + // Let syncIn register on the activity gate and reach the hung upload. + await new Promise((resolve) => setTimeout(resolve, 0)); + + const cancelPromise = plugin.definition.onEnvironmentCancelInteractiveSetup?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: false }, + reason: "cancelled", + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + // Cancel must drain the active sync before deleting the sandbox out from + // under it — the same activity-gate contract the execute path relies on. + expect(sandbox.delete).not.toHaveBeenCalled(); + + resolveUpload(); + await Promise.all([syncPromise, cancelPromise]); + + expect(sandbox.fs.uploadFiles).toHaveBeenCalledTimes(1); + expect(sandbox.delete).toHaveBeenCalledTimes(1); + + await fs.rm(hostDir, { recursive: true, force: true }); + }); + + it("rejects a queued execute once interactive cancel tombstones the lease", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + let resolveFirstExecute!: () => void; + let cancelResolved = false; + let queuedExecuteRejected = false; + sandbox.process.executeCommand.mockImplementation(async () => { + await new Promise((resolve) => { + resolveFirstExecute = resolve; + }); + return { + exitCode: 0, + result: "bash", + artifacts: { stdout: "bash" }, + }; + }); + mockGet.mockResolvedValue(sandbox); + + const firstExecutePromise = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const cancelPromise = plugin.definition.onEnvironmentCancelInteractiveSetup?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: false }, + reason: "cancelled", + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const queuedExecutePromise = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + queuedExecutePromise?.catch(() => { + queuedExecuteRejected = true; + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.delete).not.toHaveBeenCalled(); + + resolveFirstExecute(); + await cancelPromise.then(() => { + cancelResolved = true; + }); + await expect(queuedExecutePromise).rejects.toThrow(/no longer active/); + await firstExecutePromise; + + expect(cancelResolved).toBe(true); + expect(queuedExecuteRejected).toBe(true); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(1); + expect(sandbox.delete).toHaveBeenCalledTimes(1); + }); + + it("waits for an in-flight snapshot capture before destroy cleanup starts", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + let resolveSnapshot!: () => void; + sandbox._experimental_createSnapshot.mockImplementation(async () => { + await new Promise((resolve) => { + resolveSnapshot = resolve; + }); + }); + mockGet.mockResolvedValue(sandbox); + + const capturePromise = plugin.definition.onEnvironmentCaptureTemplate?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: false }, + templateLabel: "snapshot-check", + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const destroyPromise = plugin.definition.onEnvironmentDestroyLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: false }, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(sandbox.delete).not.toHaveBeenCalled(); + + resolveSnapshot(); + await Promise.all([capturePromise, destroyPromise]); + + expect(sandbox._experimental_createSnapshot).toHaveBeenCalledTimes(1); + expect(sandbox.delete).toHaveBeenCalledTimes(1); + }); + + it("does not cache a failed populate (NotFound) — the next lookup re-fetches", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + mockGet + .mockRejectedValueOnce(new MockDaytonaNotFoundError("missing")) + .mockResolvedValue(sandbox); + + const first = await plugin.definition.onEnvironmentResumeLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: true }, + }); + expect(first).toEqual({ providerLeaseId: null, metadata: { expired: true } }); + + // The rejected populate must not linger; the exec re-fetches successfully. + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + expect(mockGet).toHaveBeenCalledTimes(2); + }); + + it("single-flights concurrent misses on one lease into a single client.get", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + let resolveGet: ((value: unknown) => void) | undefined; + mockGet.mockImplementation( + () => new Promise((resolve) => { resolveGet = resolve; }), + ); + + const p1 = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + const p2 = plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + // Let both execs reach the shared in-flight populate before it resolves. + await Promise.resolve(); + resolveGet?.(sandbox); + await Promise.all([p1, p2]); + + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(2); + }); + + it("keeps concurrent different-lease populates isolated (no promise crossing)", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandboxA = createMockSandbox({ id: "lease-a" }); + const sandboxB = createMockSandbox({ id: "lease-b" }); + mockGet.mockImplementation(async (id: string) => (id === "lease-a" ? sandboxA : sandboxB)); + + await Promise.all([ + plugin.definition.onEnvironmentExecute?.(execParams("lease-a")), + plugin.definition.onEnvironmentExecute?.(execParams("lease-b")), + ]); + + expect(mockGet).toHaveBeenCalledTimes(2); + expect(mockGet).toHaveBeenCalledWith("lease-a"); + expect(mockGet).toHaveBeenCalledWith("lease-b"); + // Each lease executed in its OWN sandbox, never the other's handle. + expect(sandboxA.process.executeCommand).toHaveBeenCalledTimes(1); + expect(sandboxB.process.executeCommand).toHaveBeenCalledTimes(1); + }); + + it("re-verifies the workspace sentinel on a cached resume and evicts on mismatch", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a", state: "started" }); + // Every executeCommand (exec body + sentinel `cat`) returns a NON-matching token. + sandbox.process.executeCommand.mockResolvedValue({ + exitCode: 0, + result: JSON.stringify({ token: "other-token" }), + artifacts: { stdout: JSON.stringify({ token: "other-token" }) }, + }); + mockGet.mockImplementation(async () => sandbox); + + // Prime the cache with a successful exec on this lease. + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + expect(mockGet).toHaveBeenCalledTimes(1); + const sentinelCallsBefore = sandbox.process.executeCommand.mock.calls.length; + + // Resume hits the cache but MUST still verify the sentinel; mismatch expires. + const resumed = await plugin.definition.onEnvironmentResumeLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "lease-a", + config: { timeoutMs: 300000, reuseLease: true }, + leaseMetadata: { + workspaceSentinel: { + path: "/home/daytona/paperclip-workspace/.paperclip-runtime/reusable-sandbox-lease.json", + token: "expected-token", + result: "written", + }, + }, + }); + expect(resumed).toMatchObject({ + providerLeaseId: null, + metadata: { expired: true, workspaceSentinel: { result: "mismatch" } }, + }); + // The sentinel `cat` ran on the cached handle — verification was not skipped. + expect(sandbox.process.executeCommand.mock.calls.length).toBeGreaterThan(sentinelCallsBefore); + + // The mismatched entry was evicted, so the next exec re-fetches. + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + expect(mockGet).toHaveBeenCalledTimes(2); + }); + + it("refreshes a handle left idle past the auto-stop window and restarts a provider-stopped sandbox", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a", state: "started" }); + // Daytona auto-stopped the sandbox while our cached handle sat idle: a live + // refresh reveals the true "stopped" state that the cached snapshot hid. + sandbox.refreshData.mockImplementation(async () => { + sandbox.state = "stopped"; + }); + mockGet.mockResolvedValue(sandbox); + + let nowMs = 1_000_000; + const restoreFreshness = setDaytonaHandleFreshnessClockForTest(() => nowMs); + try { + // Prime the cache (single fetch, snapshot "started"). + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + expect(sandbox.refreshData).not.toHaveBeenCalled(); + expect(sandbox.start).not.toHaveBeenCalled(); + + // Idle past half of the default 15-min auto-stop interval (> 7.5 min). + nowMs += 8 * 60_000; + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + } finally { + restoreFreshness(); + } + + // The stale handle was refreshed in place (no second REST fetch — the same + // authenticated handle), the refresh exposed the stopped state, and the + // sandbox was restarted before the exec instead of running against a + // stopped sandbox. + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.refreshData).toHaveBeenCalledTimes(1); + expect(sandbox.start).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(2); + }); + + it("does not refresh a handle reused within the auto-stop window", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + mockGet.mockResolvedValue(sandbox); + + let nowMs = 5_000_000; + const restoreFreshness = setDaytonaHandleFreshnessClockForTest(() => nowMs); + try { + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + // Two more execs, each 6 min after the previous — always inside the + // 7.5-min window measured from the last reuse. + nowMs += 6 * 60_000; + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + nowMs += 6 * 60_000; + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + } finally { + restoreFreshness(); + } + + // Each reuse resets the freshness marker (an operation follows, resetting + // the provider idle clock), so an actively-used lease never pays a refresh. + expect(sandbox.refreshData).not.toHaveBeenCalled(); + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it("does not advance freshness when an execute fails before succeeding", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + sandbox.process.executeCommand + .mockRejectedValueOnce(new Error("command failed")) + .mockResolvedValue({ + exitCode: 0, + result: "bash", + artifacts: { stdout: "bash" }, + }); + mockGet.mockResolvedValue(sandbox); + + let nowMs = 7_000_000; + const restoreFreshness = setDaytonaHandleFreshnessClockForTest(() => nowMs); + try { + await expect(plugin.definition.onEnvironmentExecute?.(execParams("lease-a"))).rejects.toThrow( + "command failed", + ); + nowMs += 8 * 60_000; + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + } finally { + restoreFreshness(); + } + + expect(sandbox.refreshData).toHaveBeenCalledTimes(1); + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(2); + }); + + it("does not advance freshness when an execute times out before succeeding", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + sandbox.process.executeCommand + .mockRejectedValueOnce(new MockDaytonaTimeoutError("timed out")) + .mockResolvedValue({ + exitCode: 0, + result: "bash", + artifacts: { stdout: "bash" }, + }); + mockGet.mockResolvedValue(sandbox); + + let nowMs = 8_000_000; + const restoreFreshness = setDaytonaHandleFreshnessClockForTest(() => nowMs); + try { + const first = await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + expect(first).toMatchObject({ timedOut: true, exitCode: null }); + nowMs += 8 * 60_000; + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + } finally { + restoreFreshness(); + } + + expect(sandbox.refreshData).toHaveBeenCalledTimes(1); + expect(mockGet).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(2); + }); + + it("never refreshes when auto-stop is disabled, even after a long idle gap", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox({ id: "lease-a" }); + mockGet.mockResolvedValue(sandbox); + const disabledAutoStop = { timeoutMs: 300000, reuseLease: false, autoStopInterval: 0 }; + + let nowMs = 2_000_000; + const restoreFreshness = setDaytonaHandleFreshnessClockForTest(() => nowMs); + try { + await plugin.definition.onEnvironmentExecute?.({ ...execParams("lease-a"), config: disabledAutoStop }); + nowMs += 60 * 60_000; // an hour idle + await plugin.definition.onEnvironmentExecute?.({ ...execParams("lease-a"), config: disabledAutoStop }); + } finally { + restoreFreshness(); + } + + // Auto-stop off → the provider never stops the sandbox out from under the + // handle, so the cached started snapshot is trusted without a refresh. + expect(sandbox.refreshData).not.toHaveBeenCalled(); + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it("evicts the handle when a freshness refresh fails so the next lookup re-fetches", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const first = createMockSandbox({ id: "lease-a" }); + first.refreshData.mockRejectedValue(new MockDaytonaNotFoundError("sandbox vanished")); + const second = createMockSandbox({ id: "lease-a" }); + mockGet.mockResolvedValueOnce(first).mockResolvedValue(second); + + let nowMs = 3_000_000; + const restoreFreshness = setDaytonaHandleFreshnessClockForTest(() => nowMs); + try { + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); // fetch #1 → first + nowMs += 8 * 60_000; // idle past the refresh window + // The refresh rejects; execute surfaces it (fail closed) and the bad + // entry is evicted. + await expect( + plugin.definition.onEnvironmentExecute?.(execParams("lease-a")), + ).rejects.toThrow("sandbox vanished"); + // Evicted → the following exec re-fetches a fresh handle. + await plugin.definition.onEnvironmentExecute?.(execParams("lease-a")); + } finally { + restoreFreshness(); + } + + expect(mockGet).toHaveBeenCalledTimes(2); + expect(second.process.executeCommand).toHaveBeenCalledTimes(1); + }); + }); }); describe("daytona native file-sync hooks", () => { @@ -1315,7 +2087,9 @@ describe("daytona native file-sync hooks", () => { } beforeEach(() => { + mockGet.mockReset(); process.env.DAYTONA_API_KEY = "host-key"; + __resetDaytonaSandboxHandleCacheForTest(); }); afterEach(async () => { diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.ts index a2a0edd7b5..2a0a240d6b 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.ts @@ -57,6 +57,24 @@ export function setDaytonaTimingClockForTest(now: () => number): () => void { }; } +// Injectable clock for the handle cache's freshness bookkeeping, deliberately +// kept separate from the provider-timing clock so tests can advance virtual time +// past a lease's auto-stop interval without perturbing the `getDurationMs` / +// `durationMs` measurements that ride on `timingNow`. +let handleFreshnessNow: () => number = () => Date.now(); + +/** + * Test seam: override the handle-cache freshness clock and return a restore + * function. Not used in production, where the default wall clock always applies. + */ +export function setDaytonaHandleFreshnessClockForTest(now: () => number): () => void { + const previous = handleFreshnessNow; + handleFreshnessNow = now; + return () => { + handleFreshnessNow = previous; + }; +} + interface DaytonaDriverConfig { apiKey: string | null; apiUrl: string | null; @@ -723,14 +741,365 @@ async function createSandbox( return sandbox; } -async function getSandbox(config: DaytonaDriverConfig, sandboxId: string): Promise { - const client = createDaytonaClient(config); - return await client.get(sandboxId); +// ─── Per-lease started-sandbox handle cache ────────────────────────────────── +// Memoize the started Daytona `Sandbox` handle so repeated exec/sync/resume/ +// teardown calls on one lease skip the per-call `client.get(sandboxId)` REST +// re-fetch (measured ~4,938 ms on `stage.sync`) and the client construction it +// implies. The cache is process-memory only — no handle, API key, or credential +// is ever persisted or logged (Stage-1 security review C6). +// +// Isolation is the whole game here: the cached object is an authenticated +// compute handle, so a mis-keyed or un-evicted entry could run one lease/tenant's +// commands inside another's sandbox. The guarantees below map 1:1 to the Stage-1 +// required-fix conditions: +// C1 Key by a NON-SECRET composite scope, never the bare providerLeaseId: +// {driverKey, companyId, environmentId, providerLeaseId, account}. The +// account discriminator is a hash of the resolved endpoint + credentials +// so two environments pointing at different Daytona accounts (or a rotated +// key) never collide — without storing the secret in the key. +// C2 Every read (cache hit AND resolved single-flight populate) asserts the +// handle's `sandbox.id === providerLeaseId`; a mismatch evicts and throws +// (fail closed) rather than serving the wrong sandbox. +// C4 Callers evict at every teardown hook. Populate rejections (NotFound, +// network, id mismatch) are never cached — they drop from the map so the +// next call re-fetches. +// C5 In-flight populate promises live under the composite key only; there is +// no fallback lookup by bare providerLeaseId, so lease A's in-flight +// promise can never be awaited for lease B. +type SandboxScope = { + driverKey: string; + companyId: string; + environmentId: string; + providerLeaseId: string; + config: DaytonaDriverConfig; +}; + +// Non-secret provider/account fingerprint. Uses the *resolved* key (config or +// DAYTONA_API_KEY env fallback) so an env-provided credential is still scoped, +// but only its sha256 digest — never the key itself — enters the cache key (C1/C6). +function sandboxAccountDiscriminator(config: DaytonaDriverConfig): string { + const resolvedApiKey = config.apiKey ?? process.env.DAYTONA_API_KEY?.trim() ?? null; + return createHash("sha256") + .update(stableStringify({ + apiUrl: config.apiUrl, + target: config.target, + apiKey: resolvedApiKey, + })) + .digest("hex"); } -async function getSandboxOrNull(config: DaytonaDriverConfig, sandboxId: string): Promise { +function sandboxHandleCacheKey(scope: SandboxScope): string { + return stableStringify({ + driverKey: scope.driverKey, + companyId: scope.companyId, + environmentId: scope.environmentId, + providerLeaseId: scope.providerLeaseId, + account: sandboxAccountDiscriminator(scope.config), + }); +} + +function assertHandleMatchesLease(sandbox: Sandbox, providerLeaseId: string): void { + // C2: a handle must never stand in for a different sandbox than the lease + // asked for. Belt-and-suspenders against a provider that returns a renamed or + // substituted sandbox, and against any future key collision. + if (sandbox.id !== providerLeaseId) { + throw new Error( + `Daytona sandbox handle mismatch: handle ${sandbox.id} does not belong to lease ${providerLeaseId}.`, + ); + } +} + +// A cached `Sandbox` carries the provider state captured when it was last +// fetched/refreshed. Daytona auto-stops an idle sandbox after `autoStopInterval` +// minutes, at which point that snapshot ("started") no longer matches reality +// and `ensureSandboxStarted` would wrongly skip the restart, sending every +// subsequent exec/sync at a stopped sandbox. Before reusing a handle that has +// gone untouched for this fraction of the auto-stop interval we re-read the live +// state so the restart decision is made against the truth. Reusing a handle for +// an operation resets Daytona's idle clock, so an actively-used lease stays well +// inside the window and never pays the refresh — only a lease resumed after an +// idle gap does. +const STALE_HANDLE_REFRESH_SAFETY_FRACTION = 0.5; + +function staleHandleRefreshThresholdMs(autoStopIntervalMinutes: number | null): number | null { + // Auto-stop disabled (0 / null): the provider never stops the sandbox out from + // under a live handle, so the started snapshot stays valid until we evict it + // and no refresh is warranted. + if (autoStopIntervalMinutes == null || autoStopIntervalMinutes <= 0) return null; + return Math.floor(autoStopIntervalMinutes * 60_000 * STALE_HANDLE_REFRESH_SAFETY_FRACTION); +} + +type SandboxHandleCacheEntry = { + sandbox: Promise; + // Last time we know the live state was accurate: set when the handle is + // fetched/refreshed and on every reuse (an operation follows, resetting the + // provider idle clock). + verifiedAtMs: number; +}; + +type SandboxLookupOptions = { + bypassTeardownGate?: boolean; +}; + +type SandboxHandleTeardownGate = { + promise: Promise; + release: () => void; + refCount: number; +}; + +const sandboxHandleTeardownGates = (() => { + const gates = new Map(); + + function begin(scope: SandboxScope): SandboxHandleTeardownGate { + const key = sandboxHandleCacheKey(scope); + const existing = gates.get(key); + if (existing) { + existing.refCount += 1; + return existing; + } + let release!: () => void; + const gate: SandboxHandleTeardownGate = { + promise: new Promise((resolve) => { + release = resolve; + }), + release: () => release(), + refCount: 1, + }; + gates.set(key, gate); + return gate; + } + + function current(scope: SandboxScope): SandboxHandleTeardownGate | null { + return gates.get(sandboxHandleCacheKey(scope)) ?? null; + } + + function end(scope: SandboxScope, gate: SandboxHandleTeardownGate): void { + const key = sandboxHandleCacheKey(scope); + gate.refCount -= 1; + if (gate.refCount > 0) return; + if (gates.get(key) === gate) { + gates.delete(key); + } + gate.release(); + } + + function reset(): void { + gates.clear(); + } + + return { begin, current, end, reset }; +})(); + +type SandboxHandleActivityGate = { + promise: Promise; + release: () => void; + refCount: number; +}; + +const sandboxHandleActivityGates = (() => { + const gates = new Map(); + + async function begin(scope: SandboxScope): Promise { + const key = sandboxHandleCacheKey(scope); + const existing = gates.get(key); + if (existing) { + existing.refCount += 1; + return existing; + } + let release!: () => void; + const gate: SandboxHandleActivityGate = { + promise: new Promise((resolve) => { + release = resolve; + }), + release: () => release(), + refCount: 1, + }; + gates.set(key, gate); + return gate; + } + + async function waitForIdle(scope: SandboxScope): Promise { + const gate = gates.get(sandboxHandleCacheKey(scope)); + if (!gate) return; + await gate.promise; + } + + function end(scope: SandboxScope, gate: SandboxHandleActivityGate): void { + const key = sandboxHandleCacheKey(scope); + gate.refCount -= 1; + if (gate.refCount > 0) return; + if (gates.get(key) === gate) { + gates.delete(key); + } + gate.release(); + } + + function reset(): void { + gates.clear(); + } + + return { begin, waitForIdle, end, reset }; +})(); + +type SandboxLeaseAdmissionOptions = { + allowClosed?: boolean; +}; + +const sandboxHandleLeaseAdmissionStates = (() => { + const states = new Map(); + + function key(scope: SandboxScope): string { + return sandboxHandleCacheKey(scope); + } + + function open(scope: SandboxScope): void { + states.set(key(scope), false); + } + + function close(scope: SandboxScope): void { + states.set(key(scope), true); + } + + function isClosed(scope: SandboxScope): boolean { + return states.get(key(scope)) === true; + } + + function reset(): void { + states.clear(); + } + + return { open, close, isClosed, reset }; +})(); + +async function withSandboxActivityGate( + scope: SandboxScope, + fn: () => Promise, + options: SandboxLeaseAdmissionOptions = {}, +): Promise { + while (true) { + if (!options.allowClosed && sandboxHandleLeaseAdmissionStates.isClosed(scope)) { + throw new Error(`Daytona sandbox lease ${scope.providerLeaseId} is no longer active.`); + } + + const teardownGate = sandboxHandleTeardownGates.current(scope); + if (teardownGate) { + await teardownGate.promise; + if (!options.allowClosed && sandboxHandleLeaseAdmissionStates.isClosed(scope)) { + throw new Error(`Daytona sandbox lease ${scope.providerLeaseId} is no longer active.`); + } + continue; + } + + const activityGate = await sandboxHandleActivityGates.begin(scope); + try { + // A teardown can still begin between the initial check above and the + // activity-gate admission. If that happens, back out and wait for the + // teardown to finish instead of proceeding into a race with cleanup. + if (sandboxHandleTeardownGates.current(scope)) { + continue; + } + if (!options.allowClosed && sandboxHandleLeaseAdmissionStates.isClosed(scope)) { + throw new Error(`Daytona sandbox lease ${scope.providerLeaseId} is no longer active.`); + } + return await fn(); + } finally { + sandboxHandleActivityGates.end(scope, activityGate); + } + } +} + +const sandboxHandleCache = (() => { + const entries = new Map(); + + function markFresh(scope: SandboxScope): void { + const entry = entries.get(sandboxHandleCacheKey(scope)); + if (entry) { + entry.verifiedAtMs = handleFreshnessNow(); + } + } + + async function get(scope: SandboxScope, options: SandboxLookupOptions = {}): Promise { + const key = sandboxHandleCacheKey(scope); + + const entry = entries.get(key); + if (entry) { + const sandbox = await entry.sandbox; + // Re-assert on every hit; evict + fail closed on any mismatch (C2). + try { + assertHandleMatchesLease(sandbox, scope.providerLeaseId); + } catch (error) { + entries.delete(key); + throw error; + } + // Refresh the live provider state if the handle may have been auto-stopped + // since we last confirmed it, so the cached `state` snapshot can't hide a + // provider-initiated stop from `ensureSandboxStarted`. A failed refresh + // means the handle is no longer trustworthy — evict and fail closed. + const thresholdMs = staleHandleRefreshThresholdMs(scope.config.autoStopInterval); + if (thresholdMs != null && handleFreshnessNow() - entry.verifiedAtMs >= thresholdMs) { + try { + await sandbox.refreshData(); + } catch (error) { + entries.delete(key); + throw error; + } + } + return sandbox; + } + // Single-flight: the first miss stores the in-flight promise under the + // composite key so concurrent misses on the same lease share one `client.get` + // instead of double-fetching. The promise lives only under this key (C5). + const populate = (async () => { + const client = createDaytonaClient(scope.config); + const sandbox = await client.get(scope.providerLeaseId); + assertHandleMatchesLease(sandbox, scope.providerLeaseId); + return sandbox; + })(); + const populated: SandboxHandleCacheEntry = { sandbox: populate, verifiedAtMs: handleFreshnessNow() }; + entries.set(key, populated); + try { + const sandbox = await populate; + return sandbox; + } catch (error) { + // A rejected populate (NotFound, network, id mismatch) must never remain + // cached (C4/C5). Guard against clobbering a newer entry under the key. + if (entries.get(key) === populated) { + entries.delete(key); + } + throw error; + } + } + + function clear(scope: SandboxScope): void { + entries.delete(sandboxHandleCacheKey(scope)); + } + + function reset(): void { + entries.clear(); + } + + return { get, clear, reset, markFresh }; +})(); + +/** + * Test seam: clear the process-scoped handle cache between tests so a handle + * memoized under a reused composite key in one test never leaks into the next. + * Not used in production. + */ +export function __resetDaytonaSandboxHandleCacheForTest(): void { + sandboxHandleCache.reset(); + sandboxHandleTeardownGates.reset(); + sandboxHandleActivityGates.reset(); + sandboxHandleLeaseAdmissionStates.reset(); +} + +async function getSandbox(scope: SandboxScope, options: SandboxLookupOptions = {}): Promise { + return await sandboxHandleCache.get(scope, options); +} + +async function getSandboxOrNull(scope: SandboxScope, options: SandboxLookupOptions = {}): Promise { try { - return await getSandbox(config, sandboxId); + return await getSandbox(scope, options); } catch (error) { if (error instanceof DaytonaNotFoundError) { return null; @@ -739,6 +1108,10 @@ async function getSandboxOrNull(config: DaytonaDriverConfig, sandboxId: string): } } +function evictSandboxHandle(scope: SandboxScope): void { + sandboxHandleCache.clear(scope); +} + // One-shot command execution via Daytona's `process.executeCommand`. The // session-based API (`createSession` + `executeSessionCommand` with // `runAsync: false`) hangs indefinitely when the supplied command ends with @@ -951,6 +1324,13 @@ const plugin = definePlugin({ config, timeoutSeconds: toTimeoutSeconds(config.timeoutMs), }); + sandboxHandleLeaseAdmissionStates.open({ + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: sandbox.id, + config, + }); return { providerLeaseId: sandbox.id, metadata: leaseMetadata({ @@ -972,14 +1352,26 @@ const plugin = definePlugin({ params: PluginEnvironmentResumeLeaseParams, ): Promise { const config = parseDriverConfig(params.config); - const sandbox = await getSandboxOrNull(config, params.providerLeaseId); - if (!sandbox) { - return { providerLeaseId: null, metadata: { expired: true } }; - } + const scope: SandboxScope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.providerLeaseId, + config, + }; + return await withSandboxActivityGate(scope, async () => { + const sandbox = await getSandboxOrNull(scope, { bypassTeardownGate: true }); + if (!sandbox) { + return { providerLeaseId: null, metadata: { expired: true } }; + } - await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs)); - try { + await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs)); + try { const remoteCwd = await resolveSandboxWorkingDirectory(sandbox); + // C3: a resumed lease must clear the workspace sentinel before it is + // trusted, even when the handle came from the cache. On any non-match we + // evict the cached handle and expire the lease so a stale/foreign sandbox + // is never reused on the subsequent (sentinel-skipping) exec path. const workspaceSentinel = await verifyWorkspaceSentinel({ sandbox, remoteCwd, @@ -987,9 +1379,12 @@ const plugin = definePlugin({ timeoutSeconds: toTimeoutSeconds(config.timeoutMs), }); if (workspaceSentinel.result !== "matched") { + evictSandboxHandle(scope); return { providerLeaseId: null, metadata: { expired: true, workspaceSentinel } }; } const shellCommand = await detectSandboxShellCommand(sandbox, toTimeoutSeconds(config.timeoutMs)); + sandboxHandleCache.markFresh(scope); + sandboxHandleLeaseAdmissionStates.open(scope); return { providerLeaseId: sandbox.id, metadata: leaseMetadata({ @@ -1001,10 +1396,12 @@ const plugin = definePlugin({ workspaceSentinel, }), }; - } catch (error) { - await sandbox.delete(toTimeoutSeconds(config.timeoutMs)).catch(() => undefined); - throw error; - } + } catch (error) { + evictSandboxHandle(scope); + await sandbox.delete(toTimeoutSeconds(config.timeoutMs)).catch(() => undefined); + throw error; + } + }, { allowClosed: true }); }, async onEnvironmentReleaseLease( @@ -1012,43 +1409,63 @@ const plugin = definePlugin({ ): Promise { if (!params.providerLeaseId) return; const config = parseDriverConfig(params.config); - const sandbox = await getSandboxOrNull(config, params.providerLeaseId); - if (!sandbox) return; + const scope: SandboxScope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.providerLeaseId, + config, + }; + // C4: the lease's handle must not outlive its teardown. A teardown gate + // blocks fresh cache reads while cleanup is in flight so overlapping + // exec/sync calls cannot reacquire the same sandbox mid-stop/delete. + const teardownGate = sandboxHandleTeardownGates.begin(scope); + sandboxHandleLeaseAdmissionStates.close(scope); + try { + const sandbox = await getSandboxOrNull(scope, { bypassTeardownGate: true }); + if (!sandbox) return; - if (config.reuseLease) { - if (sandbox.state !== "stopped") { + evictSandboxHandle(scope); + await sandboxHandleActivityGates.waitForIdle(scope); + + 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)}`, + ); + }); + } + } + return; + } + + if (config.archiveOnRelease) { try { - await sandbox.stop(toTimeoutSeconds(config.timeoutMs)); + if (sandbox.state !== "stopped") { + await sandbox.stop(toTimeoutSeconds(config.timeoutMs)); + } + await sandbox.setAutoDeleteInterval(ARCHIVE_ON_RELEASE_AUTO_DELETE_MINUTES); + await sandbox.archive(); + return; } catch (error) { console.warn( - `Failed to stop Daytona sandbox during lease release: ${formatErrorMessage(error)}. Attempting delete instead.`, + `Failed to archive Daytona sandbox during lease release: ${formatErrorMessage(error)}. Falling back to delete.`, ); - await sandbox.delete(toTimeoutSeconds(config.timeoutMs)).catch((deleteError) => { - console.warn( - `Failed to delete Daytona sandbox after stop failure: ${formatErrorMessage(deleteError)}`, - ); - }); } } - return; - } - if (config.archiveOnRelease) { - try { - if (sandbox.state !== "stopped") { - await sandbox.stop(toTimeoutSeconds(config.timeoutMs)); - } - await sandbox.setAutoDeleteInterval(ARCHIVE_ON_RELEASE_AUTO_DELETE_MINUTES); - await sandbox.archive(); - return; - } catch (error) { - console.warn( - `Failed to archive Daytona sandbox during lease release: ${formatErrorMessage(error)}. Falling back to delete.`, - ); - } + await sandbox.delete(toTimeoutSeconds(config.timeoutMs)); + } finally { + sandboxHandleTeardownGates.end(scope, teardownGate); + evictSandboxHandle(scope); } - - await sandbox.delete(toTimeoutSeconds(config.timeoutMs)); }, async onEnvironmentDestroyLease( @@ -1056,9 +1473,28 @@ const plugin = definePlugin({ ): Promise { if (!params.providerLeaseId) return; const config = parseDriverConfig(params.config); - const sandbox = await getSandboxOrNull(config, params.providerLeaseId); - if (!sandbox) return; - await sandbox.delete(toTimeoutSeconds(config.timeoutMs)); + const scope: SandboxScope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.providerLeaseId, + config, + }; + // C4: the teardown gate blocks fresh cache reads while delete is in flight + // so overlapping exec/sync calls cannot reacquire the same sandbox mid-teardown. + const teardownGate = sandboxHandleTeardownGates.begin(scope); + sandboxHandleLeaseAdmissionStates.close(scope); + try { + const sandbox = await getSandboxOrNull(scope, { bypassTeardownGate: true }); + if (!sandbox) return; + + evictSandboxHandle(scope); + await sandboxHandleActivityGates.waitForIdle(scope); + await sandbox.delete(toTimeoutSeconds(config.timeoutMs)); + } finally { + sandboxHandleTeardownGates.end(scope, teardownGate); + evictSandboxHandle(scope); + } }, async onEnvironmentRealizeWorkspace( @@ -1072,9 +1508,18 @@ const plugin = definePlugin({ : params.workspace.remotePath ?? params.workspace.localPath ?? "/paperclip-workspace"; if (params.lease.providerLeaseId) { - const sandbox = await getSandbox(config, params.lease.providerLeaseId); - await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs)); - await sandbox.fs.createFolder(remoteCwd, "755"); + const scope: SandboxScope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.lease.providerLeaseId, + config, + }; + await withSandboxActivityGate(scope, async () => { + const sandbox = await getSandbox(scope, { bypassTeardownGate: true }); + await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs)); + await sandbox.fs.createFolder(remoteCwd, "755"); + }); } return { @@ -1099,6 +1544,13 @@ const plugin = definePlugin({ sandbox, resolveConnectionExpiresInMinutes(params.connectionExpiresInMinutes), ); + sandboxHandleLeaseAdmissionStates.open({ + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: sandbox.id, + config, + }); return { providerLeaseId: sandbox.id, status: "waiting_for_user", @@ -1134,22 +1586,30 @@ const plugin = definePlugin({ }, }; } - const sandbox = await getSandboxOrNull(config, params.providerLeaseId); - if (!sandbox) { - return { - providerLeaseId: null, - status: "missing", - connectionSummary: null, - connectionPayload: null, - metadata: { - provider: "daytona", - missing: true, - }, - }; - } + const scope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.providerLeaseId, + config, + }; + return await withSandboxActivityGate(scope, async () => { + const sandbox = await getSandboxOrNull(scope, { bypassTeardownGate: true }); + if (!sandbox) { + return { + providerLeaseId: null, + status: "missing", + connectionSummary: null, + connectionPayload: null, + metadata: { + provider: "daytona", + missing: true, + }, + }; + } - await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs)); - const remoteCwd = await resolveSandboxWorkingDirectory(sandbox); + await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs)); + const remoteCwd = await resolveSandboxWorkingDirectory(sandbox); const shellCommand = await detectSandboxShellCommand(sandbox, toTimeoutSeconds(config.timeoutMs)); const connection = params.includeConnectionPayload === true ? await createSshConnection(sandbox, resolveConnectionExpiresInMinutes(params.connectionExpiresInMinutes)) @@ -1167,17 +1627,18 @@ const plugin = definePlugin({ connectionPayload: null, }; - return { - providerLeaseId: sandbox.id, - status: "waiting_for_user", - ...connection, - metadata: interactiveSetupMetadata({ - config, - sandbox, - shellCommand, - remoteCwd, - }), - }; + return { + providerLeaseId: sandbox.id, + status: "waiting_for_user", + ...connection, + metadata: interactiveSetupMetadata({ + config, + sandbox, + shellCommand, + remoteCwd, + }), + }; + }); }, async onEnvironmentCaptureTemplate( @@ -1187,7 +1648,15 @@ const plugin = definePlugin({ if (!params.providerLeaseId) { throw new Error("Cannot capture a Daytona template without a setup sandbox lease."); } - const sandbox = await getSandbox(config, params.providerLeaseId); + const scope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.providerLeaseId, + config, + }; + return await withSandboxActivityGate(scope, async () => { + const sandbox = await getSandbox(scope, { bypassTeardownGate: true }); const createSnapshot = (sandbox as DaytonaInteractiveSandbox)._experimental_createSnapshot; if (typeof createSnapshot !== "function") { throw new Error( @@ -1202,20 +1671,21 @@ const plugin = definePlugin({ ? Math.trunc(params.timeoutMs) : config.timeoutMs; - await createSnapshot.call(sandbox, templateRef, toTimeoutSeconds(timeoutMs)); + await createSnapshot.call(sandbox, templateRef, toTimeoutSeconds(timeoutMs)); - return { - templateKind: "snapshot", - templateRef, - metadata: { - provider: "daytona", - sandboxId: sandbox.id, - capturedAt: new Date().toISOString(), - sourceTemplateRefRedacted: Boolean(params.sourceTemplateRef), - previousTemplateRefRedacted: Boolean(params.previousTemplateRef), - timeoutMs, - }, - }; + return { + templateKind: "snapshot", + templateRef, + metadata: { + provider: "daytona", + sandboxId: sandbox.id, + capturedAt: new Date().toISOString(), + sourceTemplateRefRedacted: Boolean(params.sourceTemplateRef), + previousTemplateRefRedacted: Boolean(params.previousTemplateRef), + timeoutMs, + }, + }; + }); }, async onEnvironmentCancelInteractiveSetup( @@ -1232,26 +1702,44 @@ const plugin = definePlugin({ }, }; } - const sandbox = await getSandboxOrNull(config, params.providerLeaseId); - if (!sandbox) { + const scope: SandboxScope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.providerLeaseId, + config, + }; + // C4: cancelling an interactive-setup lease deletes the sandbox, so the + // teardown gate blocks fresh cache reads while delete is in flight. + const teardownGate = sandboxHandleTeardownGates.begin(scope); + sandboxHandleLeaseAdmissionStates.close(scope); + try { + const sandbox = await getSandboxOrNull(scope, { bypassTeardownGate: true }); + if (!sandbox) { + return { + status: "missing", + metadata: { + provider: "daytona", + missing: true, + reason: params.reason ?? null, + }, + }; + } + evictSandboxHandle(scope); + await sandboxHandleActivityGates.waitForIdle(scope); + await sandbox.delete(toTimeoutSeconds(config.timeoutMs)); return { - status: "missing", + status: params.reason === "timed_out" ? "timed_out" : "cancelled", metadata: { provider: "daytona", - missing: true, + sandboxId: sandbox.id, reason: params.reason ?? null, }, }; + } finally { + sandboxHandleTeardownGates.end(scope, teardownGate); + evictSandboxHandle(scope); } - await sandbox.delete(toTimeoutSeconds(config.timeoutMs)); - return { - status: params.reason === "timed_out" ? "timed_out" : "cancelled", - metadata: { - provider: "daytona", - sandboxId: sandbox.id, - reason: params.reason ?? null, - }, - }; }, async onEnvironmentDeleteTemplate( @@ -1293,19 +1781,47 @@ const plugin = definePlugin({ } const config = parseDriverConfig(params.config); - // Time the `client.get` sandbox re-fetch (Open Q1) separately from the - // `executeCommand` round-trip so telemetry can split the per-call REST-get - // cost from the exec cost. `ensureSandboxStarted` is a no-op for an - // already-started sandbox, so it is excluded from the get measurement. - const getStart = timingNow(); - const sandbox = await getSandbox(config, params.lease.providerLeaseId); - const getDurationMs = timingNow() - getStart; - await ensureSandboxStarted(sandbox, toTimeoutSeconds(resolveTimeoutMs(params.timeoutMs, config))); - const result = await executeOneShot(sandbox, params, config); - return { - ...result, - metadata: { ...(result.metadata ?? {}), getDurationMs }, - }; + const providerLeaseId = params.lease.providerLeaseId; + return await withSandboxActivityGate({ + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId, + config, + }, async () => { + // Time the sandbox handle lookup (Open Q1) separately from the + // `executeCommand` round-trip so telemetry can split the per-call get cost + // from the exec cost. With the per-lease handle cache this collapses to ~0 + // on a hit (no `client.get` REST round-trip), but the field stays present so + // `providerGetMs` remains observable — and still captures the occasional + // freshness refresh the cache issues after an idle gap. `ensureSandboxStarted` + // is a no-op for an already-started sandbox, so it is excluded from the get + // measurement. + const getStart = timingNow(); + const sandbox = await getSandbox({ + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId, + config, + }, { bypassTeardownGate: true }); + const getDurationMs = timingNow() - getStart; + await ensureSandboxStarted(sandbox, toTimeoutSeconds(resolveTimeoutMs(params.timeoutMs, config))); + const result = await executeOneShot(sandbox, params, config); + if (!result.timedOut) { + sandboxHandleCache.markFresh({ + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId, + config, + }); + } + return { + ...result, + metadata: { ...(result.metadata ?? {}), getDurationMs }, + }; + }); }, // Opt-in native inbound transfer. Defining this hook (with onEnvironmentSyncOut) @@ -1323,13 +1839,24 @@ const plugin = definePlugin({ const config = parseDriverConfig(params.config); const remoteDir = resolveSyncRemoteDir(params.lease); const timeoutSeconds = toTimeoutSeconds(config.timeoutMs); - const sandbox = await getSandbox(config, params.lease.providerLeaseId); - await ensureSandboxStarted(sandbox, timeoutSeconds); - return await performSyncIn({ - sandbox, - operations: params.operations, - remoteDir, - timeoutSeconds, + const scope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.lease.providerLeaseId, + config, + }; + return await withSandboxActivityGate(scope, async () => { + const sandbox = await getSandbox(scope, { bypassTeardownGate: true }); + await ensureSandboxStarted(sandbox, timeoutSeconds); + const result = await performSyncIn({ + sandbox, + operations: params.operations, + remoteDir, + timeoutSeconds, + }); + sandboxHandleCache.markFresh(scope); + return result; }); }, @@ -1343,13 +1870,24 @@ const plugin = definePlugin({ const config = parseDriverConfig(params.config); const remoteDir = resolveSyncRemoteDir(params.lease); const timeoutSeconds = toTimeoutSeconds(config.timeoutMs); - const sandbox = await getSandbox(config, params.lease.providerLeaseId); - await ensureSandboxStarted(sandbox, timeoutSeconds); - return await performSyncOut({ - sandbox, - operations: params.operations, - remoteDir, - timeoutSeconds, + const scope = { + driverKey: params.driverKey, + companyId: params.companyId, + environmentId: params.environmentId, + providerLeaseId: params.lease.providerLeaseId, + config, + }; + return await withSandboxActivityGate(scope, async () => { + const sandbox = await getSandbox(scope, { bypassTeardownGate: true }); + await ensureSandboxStarted(sandbox, timeoutSeconds); + const result = await performSyncOut({ + sandbox, + operations: params.operations, + remoteDir, + timeoutSeconds, + }); + sandboxHandleCache.markFresh(scope); + return result; }); }, });