From 6382f4e8fa4bd6b4750ffcc20952c7b5ef409dd0 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 9 Sep 2026 01:34:47 -0500 Subject: [PATCH] fix: wait for task sandbox release before follow-up acquisition Co-Authored-By: Paperclip --- doc/sandbox-work-folders.md | 7 ++ .../src/__tests__/environment-runtime.test.ts | 78 ++++++++++++++++--- server/src/services/environment-runtime.ts | 35 +++++++++ 3 files changed, 110 insertions(+), 10 deletions(-) diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 4c2086ce4c..f3118eb7f2 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -90,6 +90,13 @@ server builds. The uploaded artifact and the controller's runner identity use the same file. Replacement is atomic and preserves the previous launcher if staging fails; it does not reset the task's working files or provider session. +A follow-up task run waits up to 30 seconds for a terminal predecessor to release +its reusable sandbox. While that handoff is pending, startup cannot allocate a +second workspace or resume the sandbox concurrently. An active predecessor or +an incomplete release reports a retryable resume error and preserves the lease. +This applies to both runner generations and leaves distinct task/user bindings +isolated. + Acceptance must resume representative pre-upgrade legacy and native tasks with committed, staged, unstaged, and untracked work, verify their original paths and usable continuation, and exercise their existing restore mechanism after a diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index 4fc3f48dee..21473df84b 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -5810,9 +5810,61 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { expect(firstMetadata).not.toContain("rotated-provider-key"); }); - it("preserves active reusable sandbox leases held by another running run", async () => { + it.each([["codex_local", "failed"], ["paperclip_runner", "failed"], ["paperclip_runner", "interrupted"]])("waits for a terminal %s %s task run to release its original sandbox", async (adapterType, status) => { + const seeded = await seedReusablePluginSandboxLease(adapterType); + const taskId = randomUUID(); + await db.insert(issues).values({ id: taskId, companyId: seeded.companyId, title: "Handoff race" }); + const workerManager = { + isRunning: vi.fn(() => true), + call: vi.fn(async (_pluginId: string, method: string) => { + if (method !== "environmentResumeLease") throw new Error(`Unexpected replacement: ${method}`); + return { providerLeaseId: seeded.reusableLease.providerLeaseId, metadata: { + provider: "fake-plugin", image: "fake:test", timeoutMs: 1234, reuseLease: true, + } }; + }), + getWorker: vi.fn(() => ({ supportedMethods: ["environmentResumeLease", "environmentReleaseLease", "environmentDestroyLease"] })), + } as unknown as PluginWorkerManager; + const runtime = environmentRuntimeService(db, { pluginWorkerManager: workerManager }); + const acquire = (runId: string, issueId: string | null) => runtime.acquireRunLease({ + companyId: seeded.companyId, environment: seeded.environment, issueId, + agentId: seeded.agentId, heartbeatRunId: runId, adapterType, + persistedExecutionWorkspace: { id: seeded.executionWorkspaceId, mode: "shared_workspace" }, + }); + // Get the actual runtime fingerprint before binding this fixture to a task. + const first = await acquire(seeded.runId, null); + await db.update(environmentLeases).set({ issueId: taskId, metadata: { + ...first.lease.metadata, + reusableSandboxLease: { ...(first.lease.metadata!.reusableSandboxLease as object), issueId: taskId }, + } }).where(eq(environmentLeases.id, first.lease.id)); + await db.update(heartbeatRuns).set({ status, finishedAt: new Date() }).where(eq(heartbeatRuns.id, seeded.runId)); + const nextRunId = randomUUID(); + await db.insert(heartbeatRuns).values({ id: nextRunId, companyId: seeded.companyId, agentId: seeded.agentId, status: "running" }); + vi.mocked(workerManager.call).mockClear(); + const pending = acquire(nextRunId, taskId); + // Attach the handler immediately so a regression cannot produce an + // unhandled rejection while the previous run finishes its release. + const outcome = pending.then((value) => ({ value }), (error: unknown) => ({ error })); + await new Promise((resolve) => setTimeout(resolve, 300)); + const callsBeforeRelease = vi.mocked(workerManager.call).mock.calls.length; + await environmentService(db).releaseLease(first.lease.id, "released", { cleanupStatus: "success" }); + const result = await outcome; + expect(callsBeforeRelease).toBe(0); + expect(result).not.toHaveProperty("error"); + if (!("value" in result)) throw result.error; + expect(result.value.lease.providerLeaseId).toBe(first.lease.providerLeaseId); + expect(result.value.lease.metadata?.sandboxLeaseAcquisition).toEqual({ outcome: "resumed" }); + expect(workerManager.call).toHaveBeenCalledOnce(); + expect((await environmentService(db).getLeaseById(first.lease.id))?.status).toBe("expired"); + }); + + it.each([false, true])("preserves active reusable sandbox leases held by another running run (same task: %s)", async (sameTask) => { const { pluginId, companyId, agentId, environment, executionWorkspaceId, reusableLease } = await seedReusablePluginSandboxLease(); + const taskId = sameTask ? randomUUID() : null; + if (taskId) { + await db.insert(issues).values({ id: taskId, companyId, title: "Active task" }); + await db.update(environmentLeases).set({ issueId: taskId }).where(eq(environmentLeases.id, reusableLease.id)); + } const nextRunId = randomUUID(); await db.insert(heartbeatRuns).values({ id: nextRunId, @@ -5844,10 +5896,10 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { } as unknown as PluginWorkerManager; const runtimeWithPlugin = environmentRuntimeService(db, { pluginWorkerManager: workerManager }); - const acquired = await runtimeWithPlugin.acquireRunLease({ + const pending = runtimeWithPlugin.acquireRunLease({ companyId, environment, - issueId: null, + issueId: taskId, agentId, heartbeatRunId: nextRunId, persistedExecutionWorkspace: { @@ -5856,13 +5908,19 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { }, }); - expect(acquired.lease.providerLeaseId).toBe("fresh-plugin-lease"); - expect(workerManager.call).toHaveBeenCalledOnce(); - expect(workerManager.call).toHaveBeenCalledWith(pluginId, "environmentAcquireLease", expect.objectContaining({ - agentId, - executionWorkspaceId, - runId: nextRunId, - }), 31234); + if (sameTask) { + await expect(pending).rejects.toThrow("the lease was preserved and no replacement was created"); + expect(workerManager.call).not.toHaveBeenCalled(); + } else { + const acquired = await pending; + expect(acquired.lease.providerLeaseId).toBe("fresh-plugin-lease"); + expect(workerManager.call).toHaveBeenCalledOnce(); + expect(workerManager.call).toHaveBeenCalledWith(pluginId, "environmentAcquireLease", expect.objectContaining({ + agentId, + executionWorkspaceId, + runId: nextRunId, + }), 31234); + } await expect(environmentService(db).getLeaseById(reusableLease.id)).resolves.toMatchObject({ status: "active", cleanupStatus: null, diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index 2570e940a7..448aca6b03 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -1776,6 +1776,41 @@ function createSandboxEnvironmentDriver( throw new Error(`Expected sandbox environment config for driver "${input.environment.driver}".`); } + // A run becomes terminal before its final save and provider release have + // finished. A follow-up must wait for that handoff: filtering the active + // lease out below would otherwise create an empty competing workspace. + // Different tasks and responsible users still receive isolated sandboxes. + if (parsed.config.reuseLease && input.issueId && input.heartbeatRunId && input.agentId && input.executionWorkspaceId) { + const handoffDeadline = Date.now() + 30_000; + while (true) { + const [holder] = await db.select({ + providerLeaseId: environmentLeases.providerLeaseId, + runStatus: heartbeatRuns.status, + }).from(environmentLeases).innerJoin(heartbeatRuns, and( + eq(heartbeatRuns.id, environmentLeases.heartbeatRunId), + eq(heartbeatRuns.companyId, environmentLeases.companyId), + )).where(and( + eq(environmentLeases.companyId, input.companyId), + eq(environmentLeases.environmentId, input.environment.id), + eq(environmentLeases.executionWorkspaceId, input.executionWorkspaceId), + eq(environmentLeases.issueId, input.issueId), + eq(environmentLeases.status, "active"), + eq(environmentLeases.leasePolicy, "reuse_by_environment"), + eq(heartbeatRuns.agentId, input.agentId), + sql`${heartbeatRuns.responsibleUserId} is not distinct from ${responsibleUserId}`, + sql`${heartbeatRuns.id} <> ${input.heartbeatRunId}`, + )).limit(1); + if (!holder) break; + if (!["succeeded", "interrupted", "failed", "cancelled", "timed_out"].includes(holder.runStatus) || Date.now() >= handoffDeadline) { + throw new ReusableSandboxResumeError({ + provider: parsed.config.provider, + providerLeaseId: holder.providerLeaseId ?? input.executionWorkspaceId, + }); + } + await delay(250); + } + } + // Check if this provider should be handled by a plugin. if (!isBuiltinSandboxProvider(parsed.config.provider)) { const pluginProvider = await resolveSandboxProviderPlugin({