From 015d3d0a7ca6f1d72dbadda334ed2258533a1a1e Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 8 Nov 2026 12:32:38 -0600 Subject: [PATCH] fix(environments): tear down retain-on-failure sandboxes when their environment is deleted (#20085) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path - Remote sandboxes cost money while they exist, so every sandbox Paperclip keeps must have an owner and an end. - Two lease policies keep a sandbox after its run. `reuse_by_environment` keeps a warm sandbox for the next turn. `retain_on_failure` keeps a failed run's sandbox, stopped, as a workspace-restore source. - Environment deletion is the last chance to end both: with the operator's consent it destroys the environment's kept sandboxes, otherwise it refuses. - It only looked at `reuse_by_environment`. A `retain_on_failure` sandbox neither blocked the delete nor got destroyed, so it was orphaned the moment its environment was deleted. ## Linked Issues or Issue Description Refs #20048 **What happened?** In Daytona warm wave w-20261108T170423-s9501, the harness deleted the environment after grading. Three stopped sandboxes (4e20c8b3, abf0980b, 5e49c561) remained afterwards and had to be deleted by hand through the Daytona API. All three belonged to `retain_on_failure` leases: failed runs' sandboxes kept as workspace-restore sources. **This leaks on master today, with per-turn sandboxes too.** The four leases in that wave carried `workspaceRestoreRecovery` from `legacy-workspace-restore-recovery.ts` (no `nativeSourceRetention` marker). That writer turns any failed run's lease with a workspace-restore failure into a `retain_on_failure` restore source, whether the environment is warm or per-turn. Every such sandbox has been surviving environment deletion and costing money until deleted by hand. **Steps to reproduce** 1. A run in a sandbox environment fails, and its lease is released as a `retain_on_failure` restore source (sandbox stopped, not destroyed). 2. Delete the environment with `destroyReusableSandboxLeases=true`. 3. The delete succeeds; the restore-source sandbox is never destroyed and has no lease or environment left to find it by. **Expected behavior** The delete blast radius counts kept restore-source sandboxes like reusable ones. Without consent, the delete is refused; with consent, they are destroyed through the same claimed teardown before the environment row is removed. ## What Changed - `server/src/services/environments.ts`: `getDeleteBlastRadius` counts leases with policy `reuse_by_environment` **or `retain_on_failure`** (status `active`, `released`, `retained` or `failed`) as kept-sandbox holders. The blocked reason is still `reusable_sandbox_lease`. - `server/src/services/environment-runtime.ts`: `destroyReusableSandboxLeasesForEnvironment` selects both policies. It claims through `destroyClaimedSandboxLease` with `anyLeasePolicy` plus a scope condition limited to those two policies, keeping its existing guards: live-run skip, pending export, legacy shutdown protection, and the claim compare-and-swap. - `server/src/__tests__/environment-runtime.test.ts`: a released `retain_on_failure` lease on a failed run now blocks the delete and is destroyed by the consented teardown (`expired`, `environment_deleted`). This fails on master. ## Verification - `cd server && npx vitest run src/__tests__/environment-runtime.test.ts src/__tests__/environment-service.test.ts src/__tests__/environment-routes.test.ts`: 347/347 pass. - The new test fails on master: the blast radius reports no kept sandbox. ## Risks - **A delete now refused that used to succeed.** An environment holding restore-source sandboxes is refused until the operator consents to destroying them. That is the intended behaviour; it was silently leaking them before. - **Losing restore files.** The consented delete destroys restore sources, which is what the operator agreed to by deleting the environment. Leases with a pending export or legacy shutdown protection are still skipped and keep blocking. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via Claude Code. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- packages/shared/src/types/environment.ts | 4 +++ .../src/__tests__/environment-runtime.test.ts | 26 +++++++++++++++++++ .../src/__tests__/environment-service.test.ts | 1 + server/src/routes/environments.ts | 3 +++ server/src/services/environment-runtime.ts | 10 ++++--- server/src/services/environments.ts | 12 +++++++-- ui/src/pages/CompanyEnvironments.tsx | 8 ++++++ 7 files changed, 59 insertions(+), 5 deletions(-) diff --git a/packages/shared/src/types/environment.ts b/packages/shared/src/types/environment.ts index 9f929fcb71b..f28a839dd38 100644 --- a/packages/shared/src/types/environment.ts +++ b/packages/shared/src/types/environment.ts @@ -107,6 +107,10 @@ export type EnvironmentDeleteBlockedReason = */ export interface EnvironmentDeleteReusableLeaseHolder { leaseId: string; + /** `reuse_by_environment`: a warm sandbox for the next turn (its workspace + * provisions a fresh one). `retain_on_failure`: a stopped sandbox kept as the + * only copy of a failed run's workspace files (destroying it loses them). */ + leasePolicy: string; executionWorkspaceId: string | null; executionWorkspaceName: string | null; issueId: string | null; diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index 34c9b18aa09..5cb2daf5bdb 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -5534,6 +5534,32 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { }); }); + it("destroys a retained restore-source sandbox (retain_on_failure) when its environment is deleted", async () => { + const { pluginId, runId, reusableLease } = await seedReusablePluginSandboxLease(); + await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, runId)); + // A failed run's sandbox kept, stopped, as a workspace-restore source. + await db.update(environmentLeases).set({ leasePolicy: "retain_on_failure", status: "released", cleanupStatus: "success", + releasedAt: new Date() }).where(eq(environmentLeases.id, reusableLease.id)); + expect(await environmentService(db).getDeleteBlastRadius(reusableLease.environmentId!)) + .toMatchObject({ reusableSandboxLeaseCount: 1, canDelete: false, + reusableSandboxLeaseHolders: [expect.objectContaining({ leasePolicy: "retain_on_failure" })] }); + // The atomic delete guard refuses too, not only the blast-radius pre-check. + expect(await environmentService(db).removeIfDeletable(reusableLease.environmentId!)).toBeFalsy(); + const workerManager = { + isRunning: vi.fn((id: string) => id === pluginId), + call: vi.fn(async (_pluginId: string, method: string) => { + if (method === "environmentDestroyLease") return { providerLeaseId: reusableLease.providerLeaseId, state: "destroyed" }; + throw new Error(`Unexpected plugin method: ${method}`); + }), + getWorker: vi.fn(() => ({ supportedMethods: ["environmentResumeLease", "environmentReleaseLease", "environmentDestroyLease"] })), + } as unknown as PluginWorkerManager; + const result = await environmentRuntimeService(db, { pluginWorkerManager: workerManager }) + .destroyReusableSandboxLeasesForEnvironment({ environmentId: reusableLease.environmentId!, failureReason: "environment_deleted" }); + expect(result).toEqual({ destroyed: 1, failed: 0, skippedLiveRun: 0 }); + await expect(environmentService(db).getLeaseById(reusableLease.id)).resolves.toMatchObject({ + status: "expired", failureReason: "environment_deleted", cleanupStatus: "success" }); + }); + it("destroys a failed reusable sandbox after successful release before deleting its environment", async () => { const { pluginId, companyId, runId, environment, reusableLease } = await seedReusablePluginSandboxLease(); const workerManager = { diff --git a/server/src/__tests__/environment-service.test.ts b/server/src/__tests__/environment-service.test.ts index 161b748b5c1..4c600fd3dfe 100644 --- a/server/src/__tests__/environment-service.test.ts +++ b/server/src/__tests__/environment-service.test.ts @@ -711,6 +711,7 @@ describeEmbeddedPostgres("environmentService leases", () => { expect(activeImpact?.reusableSandboxLeaseHolders).toEqual([ { leaseId: lease.id, + leasePolicy: "reuse_by_environment", executionWorkspaceId: workspaceId, executionWorkspaceName: "ACME-7-reusable-lease-holder", issueId, diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 879348d21c6..b73d3cb58e0 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -634,6 +634,9 @@ export function environmentRoutes( return "Cannot delete this environment while a sandbox cleanup is pending. Wait for the cleanup sweep to destroy the orphan sandbox, then retry."; } if (impact.reusableSandboxLeaseCount > 0) { + if (impact.reusableSandboxLeaseHolders.some(holder => holder.leasePolicy === "retain_on_failure")) { + return "Cannot delete this environment while it keeps a failed run's workspace copy (a stopped sandbox kept for file recovery). Deleting the environment with consent destroys that sandbox and the files only it holds."; + } return "Cannot delete this environment while it has a reusable sandbox lease. Remove the associated execution workspace or issue so Paperclip can destroy the sandbox, then retry."; } return null; diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index 2a9895ece08..96aba00bbb7 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -60,7 +60,7 @@ import { runWithRuntimeParent, type StartupSpanContext, } from "@paperclipai/adapter-utils/acpx-engine/startup-timing"; -import { environmentService } from "./environments.js"; +import { environmentService, KEPT_SANDBOX_LEASE_POLICIES } from "./environments.js"; import { instanceSettingsService } from "./instance-settings.js"; import { hasSandboxSnapshotQualification, qualifiedSandboxSnapshotId, usesManagedCloudSandboxImage, type SandboxTemplateIdentity } from "./sandbox-snapshot-qualification.js"; import { verifyRecoverableNativeHarnessBackup } from "./native-runtime/native-harness-backup-storage.js"; @@ -4321,7 +4321,10 @@ export function environmentRuntimeService( .where( and( eq(environmentLeases.environmentId, input.environmentId), - eq(environmentLeases.leasePolicy, "reuse_by_environment"), + // Reusable sandboxes, and retain-on-failure sandboxes kept stopped as + // workspace-restore sources: both outlive their run, so a deleted + // environment must tear them down or they are orphaned. + inArray(environmentLeases.leasePolicy, [...KEPT_SANDBOX_LEASE_POLICIES]), // A failed run may have stopped its reusable sandbox without // destroying it; that provider handle still needs scoped teardown. inArray(environmentLeases.status, ["active", "released", "retained", "failed"]), @@ -4360,7 +4363,8 @@ export function environmentRuntimeService( skippedLiveRun += 1; continue; } - const lease = await destroyClaimedSandboxLease({ environment, leaseRow, failureReason }); + const lease = await destroyClaimedSandboxLease({ environment, leaseRow, failureReason, anyLeasePolicy: true, + scopeCondition: inArray(environmentLeases.leasePolicy, [...KEPT_SANDBOX_LEASE_POLICIES]) }); if (!lease) { // Lost to a racing resume or a concurrent terminal transition — the // lease is no longer ours to destroy. diff --git a/server/src/services/environments.ts b/server/src/services/environments.ts index 72590988116..dd49c30b90c 100644 --- a/server/src/services/environments.ts +++ b/server/src/services/environments.ts @@ -60,6 +60,12 @@ import { type ManagedPluginEnvironmentReconcileAction, } from "./managed-plugin-environments.js"; +/** Lease policies whose sandbox outlives its run: a reusable warm sandbox, and a + * retain-on-failure sandbox kept stopped as a workspace-restore source. The + * delete blast radius, the atomic delete guard and the consented teardown must + * all use this one list, or a delete can orphan a sandbox. */ +export const KEPT_SANDBOX_LEASE_POLICIES = ["reuse_by_environment", "retain_on_failure"] as const; + export async function attachSandboxCreation(db: Db, id: string, scope: { companyId: string; environmentId: string; heartbeatRunId?: string | null }, attach: (tx: Db) => Promise): Promise { return db.transaction(async tx => { @@ -1312,7 +1318,7 @@ export function environmentService(db: Db) { sql`not exists ( select 1 from ${environmentLeases} where ${environmentLeases.environmentId} = ${environments.id} - and ${environmentLeases.leasePolicy} = 'reuse_by_environment' + and ${inArray(environmentLeases.leasePolicy, [...KEPT_SANDBOX_LEASE_POLICIES])} and ${environmentLeases.status} in ('active', 'released', 'retained', 'failed') )`, ), @@ -1415,6 +1421,7 @@ export function environmentService(db: Db) { db .select({ leaseId: environmentLeases.id, + leasePolicy: environmentLeases.leasePolicy, executionWorkspaceId: environmentLeases.executionWorkspaceId, executionWorkspaceName: executionWorkspaces.name, issueId: environmentLeases.issueId, @@ -1427,7 +1434,7 @@ export function environmentService(db: Db) { .where( and( eq(environmentLeases.environmentId, id), - eq(environmentLeases.leasePolicy, "reuse_by_environment"), + inArray(environmentLeases.leasePolicy, [...KEPT_SANDBOX_LEASE_POLICIES]), inArray(environmentLeases.status, ["active", "released", "retained", "failed"]), ), ), @@ -1447,6 +1454,7 @@ export function environmentService(db: Db) { const pendingCleanupLeaseCount = countFromRows(pendingCleanupLeaseRows); const reusableSandboxLeaseHolders = reusableSandboxLeaseRows.map((row) => ({ leaseId: row.leaseId, + leasePolicy: row.leasePolicy, executionWorkspaceId: row.executionWorkspaceId, executionWorkspaceName: row.executionWorkspaceName, issueId: row.issueId, diff --git a/ui/src/pages/CompanyEnvironments.tsx b/ui/src/pages/CompanyEnvironments.tsx index e35a6d59162..aba98699f67 100644 --- a/ui/src/pages/CompanyEnvironments.tsx +++ b/ui/src/pages/CompanyEnvironments.tsx @@ -109,6 +109,9 @@ function environmentDeleteBlockMessage(impact: EnvironmentDeleteBlastRadius): st return "Cannot delete this environment while a sandbox cleanup is pending. Wait for the cleanup sweep to destroy the orphan sandbox, then retry."; } if (impact.reusableSandboxLeaseCount > 0) { + if (impact.reusableSandboxLeaseHolders.some(holder => holder.leasePolicy === "retain_on_failure")) { + return "Cannot delete this environment while it keeps a failed run's workspace copy (a stopped sandbox kept for file recovery). Deleting the environment with consent destroys that sandbox and the files only it holds."; + } return "Cannot delete this environment while it has a reusable sandbox lease. Remove the associated execution workspace or issue so Paperclip can destroy the sandbox, then retry."; } return null; @@ -1924,6 +1927,8 @@ export function CompanyEnvironments({ mode = "list" }: CompanyEnvironmentsProps) } // One row per workspace holding blocking sandbox leases (several leases can // share a workspace). A lease whose workspace FK was nulled groups alone. + const retainedFailureSourceCount = (deleteBlastRadius?.reusableSandboxLeaseHolders ?? []) + .filter(holder => holder.leasePolicy === "retain_on_failure").length; const reusableLeaseHolderGroups = (() => { const groups = new Map< string, @@ -2543,6 +2548,9 @@ export function CompanyEnvironments({ mode = "list" }: CompanyEnvironmentsProps) reusableLeaseOnlyBlock && deleteBlastRadius ? `${deleteBlastRadius.reusableSandboxLeaseCount === 1 ? "1 reusable sandbox" : `${deleteBlastRadius.reusableSandboxLeaseCount} reusable sandboxes`} will be destroyed; the workspaces holding them stay open and provision a fresh sandbox on their next run.` : null, + reusableLeaseOnlyBlock && retainedFailureSourceCount > 0 + ? `${retainedFailureSourceCount === 1 ? "1 of them is" : `${retainedFailureSourceCount} of them are`} a failed run's workspace copy kept for file recovery; destroying it loses the files only it holds.` + : null, agentsUsingEnvironment.length > 0 ? `${agentsUsingEnvironment.length === 1 ? "1 agent uses" : `${agentsUsingEnvironment.length} agents use`} this environment as their default. Choose the environment those agents should be reassigned to.` : null,