fix(environments): a consented environment delete releases restore-source protection so it never stays blocked (#20099)

## Thinking Path

- Deleting an environment with consent destroys its kept sandboxes.
#20085 added failed runs' restore-source sandboxes (`retain_on_failure`)
to that teardown, and the delete dialog now tells the operator that the
consented delete destroys those files.
- But every teardown path refuses a lease whose sandbox carries a
`workspaceRestoreRecovery` marker
(`legacyShutdownWorkspaceResourceProtected`). That is correct for
ordinary cleanup, since the files may be the only copy.
- So the consented delete claimed the restore sources for cleanup, every
teardown returned without destroying, and the delete returned 409
`pending_sandbox_cleanup` forever: a refusal the user cannot act on.
- The operator's consent is exactly the decision the protection waits
for, so the delete releases it explicitly and audits who did, then
proceeds.

## Linked Issues or Issue Description

Refs #20048, follow-up to #20085

**What happened?**
In Daytona warm wave w-20261108T192459-s9501, the harness's consented
environment delete returned 409 `pending_sandbox_cleanup` for minutes
(it retried every 3 s). Two restore-source leases (8b26e044, bcb897c9)
had been claimed for cleanup with `environment_deleted`. The activity
log shows a cleanup attempt on every retry, each returning without
destroying. On the kept database copy,
`legacyShutdownWorkspaceResourceProtected` returns `true` for both,
because of their `workspaceRestoreRecovery` marker. Their restore
recovery had already succeeded (workspace sync finalized). Phase 3
deleted the two sandboxes by hand.

**Steps to reproduce**
1. A failed run leaves a `retain_on_failure` lease with
`workspaceRestoreRecovery` metadata (sandbox stopped).
2. `DELETE /api/environments/:id?destroyReusableSandboxLeases=true`.
3. The lease is claimed (`pending_cleanup`), the teardown refuses it,
and every retry of the delete returns 409.

**Expected behavior**
The consented delete destroys the restore source (the operator was told
the files go with it) and the environment is deleted. No 409 the user
can't act on.

## What Changed

- `server/src/services/environment-runtime.ts`: new
`releaseRestoreSourcesForEnvironmentDelete({ environmentId, consentedBy
})`. For each of the environment's restore-source sandboxes
(`retain_on_failure` with `workspaceRestoreRecovery`, any non-terminal
status including an earlier `pending_cleanup` claim), it removes the
marker from every lease row on that sandbox and records
`restoreSourceReleasedByEnvironmentDelete: { consentedBy, releasedAt }`.
`destroyReusableSandboxLeasesForEnvironment` takes `consentedBy` and
releases first.
- `server/src/routes/environments.ts`: a consented delete whose only
blockers are pending cleanup or kept sandboxes releases restore
protection before the pending-cleanup retry, so leases an earlier
attempt left claimed are freed too. It then passes `consentedBy` to the
teardown.
- Tests:
- A restore source now carries the real `workspaceRestoreRecovery`
marker, and the consented teardown destroys it. This failed before the
fix (`{ destroyed: 0, failed: 1 }`), matching the wave.
- The route test asserts the release runs before the pending-cleanup
retry.

## 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 restore-source test fails on master with `failed: 1`.

## Risks

- **Losing restore files without consent.** The release runs only on the
explicit consented delete path (`destroyReusableSandboxLeases=true`),
and only when the remaining blockers are pending cleanup or kept
sandboxes. It is recorded on every released row. Leases with a pending
native export are still skipped by the teardown.
- **Ordinary cleanup.** Unchanged: restore protection still holds
everywhere else.

## 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 <noreply@anthropic.com>
This commit is contained in:
DottaandClaude Opus 5.5 authored and GitHub committed 2026-11-08 14:27:00 -06:00
1 parent 2a16be36ab
commit f789f3e2af
4 files changed
+75 -3

No files matched your search

@@ -28,6 +28,7 @@ const mockProjectService = vi.hoisted(() => ({
const mockEnvironmentRuntimeService = vi.hoisted(() => ({ const mockEnvironmentRuntimeService = vi.hoisted(() => ({
destroyReusableSandboxLeasesForEnvironment: vi.fn(async () => ({ destroyed: 0, failed: 0, skippedLiveRun: 0 })), destroyReusableSandboxLeasesForEnvironment: vi.fn(async () => ({ destroyed: 0, failed: 0, skippedLiveRun: 0 })),
releaseRestoreSourcesForEnvironmentDelete: vi.fn(async () => 0),
})); }));
const mockCloseWarmNativeSessionsForEnvironment = vi.hoisted(() => const mockCloseWarmNativeSessionsForEnvironment = vi.hoisted(() =>
vi.fn(async () => ({ closed: 0, busy: 0, failed: 0 })), vi.fn(async () => ({ closed: 0, busy: 0, failed: 0 })),
@@ -1774,6 +1775,7 @@ describe("environment routes", () => {
).toHaveBeenCalledExactlyOnceWith({ ).toHaveBeenCalledExactlyOnceWith({
environmentId: "env-1", environmentId: "env-1",
failureReason: "environment_deleted", failureReason: "environment_deleted",
consentedBy: "admin-1",
}); });
expect( expect(
mockCloseWarmNativeSessionsForEnvironment, mockCloseWarmNativeSessionsForEnvironment,
@@ -1845,6 +1847,12 @@ describe("environment routes", () => {
expect(res.status).toBe(200); expect(res.status).toBe(200);
expect(retryPendingEnvironmentCleanup).toHaveBeenCalledExactlyOnceWith({ environmentId: "env-1", actorId: "admin-1" }); expect(retryPendingEnvironmentCleanup).toHaveBeenCalledExactlyOnceWith({ environmentId: "env-1", actorId: "admin-1" });
// Consent releases restore-source protection before the cleanup retry, or a
// protected source would refuse every attempt and keep the delete blocked.
expect(mockEnvironmentRuntimeService.releaseRestoreSourcesForEnvironmentDelete)
.toHaveBeenCalledWith({ environmentId: "env-1", consentedBy: "admin-1" });
expect(mockEnvironmentRuntimeService.releaseRestoreSourcesForEnvironmentDelete.mock.invocationCallOrder[0])
.toBeLessThan(retryPendingEnvironmentCleanup.mock.invocationCallOrder[0]!);
expect(mockEnvironmentService.removeIfDeletable).toHaveBeenCalledWith("env-1"); expect(mockEnvironmentService.removeIfDeletable).toHaveBeenCalledWith("env-1");
}); });
@@ -5537,9 +5537,12 @@ describeEmbeddedPostgres("environmentRuntimeService", () => {
it("destroys a retained restore-source sandbox (retain_on_failure) when its environment is deleted", async () => { it("destroys a retained restore-source sandbox (retain_on_failure) when its environment is deleted", async () => {
const { pluginId, runId, reusableLease } = await seedReusablePluginSandboxLease(); const { pluginId, runId, reusableLease } = await seedReusablePluginSandboxLease();
await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, runId)); await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, runId));
// A failed run's sandbox kept, stopped, as a workspace-restore source. // A failed run's sandbox kept, stopped, as a workspace-restore source. The
// restore marker protects it from every ordinary teardown.
await db.update(environmentLeases).set({ leasePolicy: "retain_on_failure", status: "released", cleanupStatus: "success", await db.update(environmentLeases).set({ leasePolicy: "retain_on_failure", status: "released", cleanupStatus: "success",
releasedAt: new Date() }).where(eq(environmentLeases.id, reusableLease.id)); releasedAt: new Date(), metadata: { ...reusableLease.metadata,
workspaceRestoreRecovery: { schema: "paperclip.workspace-restore-recovery.v1", runId } } })
.where(eq(environmentLeases.id, reusableLease.id));
expect(await environmentService(db).getDeleteBlastRadius(reusableLease.environmentId!)) expect(await environmentService(db).getDeleteBlastRadius(reusableLease.environmentId!))
.toMatchObject({ reusableSandboxLeaseCount: 1, canDelete: false, .toMatchObject({ reusableSandboxLeaseCount: 1, canDelete: false,
reusableSandboxLeaseHolders: [expect.objectContaining({ leasePolicy: "retain_on_failure" })] }); reusableSandboxLeaseHolders: [expect.objectContaining({ leasePolicy: "retain_on_failure" })] });
@@ -5553,9 +5556,13 @@ describeEmbeddedPostgres("environmentRuntimeService", () => {
}), }),
getWorker: vi.fn(() => ({ supportedMethods: ["environmentResumeLease", "environmentReleaseLease", "environmentDestroyLease"] })), getWorker: vi.fn(() => ({ supportedMethods: ["environmentResumeLease", "environmentReleaseLease", "environmentDestroyLease"] })),
} as unknown as PluginWorkerManager; } as unknown as PluginWorkerManager;
// The operator consented to deleting the environment and the files only
// this sandbox holds: that consent releases the restore protection.
const result = await environmentRuntimeService(db, { pluginWorkerManager: workerManager }) const result = await environmentRuntimeService(db, { pluginWorkerManager: workerManager })
.destroyReusableSandboxLeasesForEnvironment({ environmentId: reusableLease.environmentId!, failureReason: "environment_deleted" }); .destroyReusableSandboxLeasesForEnvironment({ environmentId: reusableLease.environmentId!, failureReason: "environment_deleted",
consentedBy: "board-user" });
expect(result).toEqual({ destroyed: 1, failed: 0, skippedLiveRun: 0 }); expect(result).toEqual({ destroyed: 1, failed: 0, skippedLiveRun: 0 });
expect(workerManager.call).toHaveBeenCalledWith(pluginId, "environmentDestroyLease", expect.anything(), expect.anything());
await expect(environmentService(db).getLeaseById(reusableLease.id)).resolves.toMatchObject({ await expect(environmentService(db).getLeaseById(reusableLease.id)).resolves.toMatchObject({
status: "expired", failureReason: "environment_deleted", cleanupStatus: "success" }); status: "expired", failureReason: "environment_deleted", cleanupStatus: "success" });
}); });
+13
View File
@@ -1343,6 +1343,18 @@ export function environmentRoutes(
// own sandbox, already queued for teardown. Run one attempt now instead of // own sandbox, already queued for teardown. Run one attempt now instead of
// waiting out the retry cooldown. An attempt in flight elsewhere is not taken // waiting out the retry cooldown. An attempt in flight elsewhere is not taken
// over, and a failed attempt keeps the lease, so the re-check still rejects. // over, and a failed attempt keeps the lease, so the re-check still rejects.
// Consent covers the environment's restore-source sandboxes too: release
// their restore protection first, so neither the pending-cleanup retry nor
// the teardown below refuses them (including ones an earlier attempt left
// claimed for cleanup).
if (
req.query.destroyReusableSandboxLeases === "true"
&& impact.deleteBlockedReasons.every((reason) =>
reason === "pending_sandbox_cleanup" || reason === "reusable_sandbox_lease")
) {
await environmentRuntime.releaseRestoreSourcesForEnvironmentDelete({ environmentId: existing.id,
consentedBy: actor.actorId });
}
if ( if (
req.query.destroyReusableSandboxLeases === "true" req.query.destroyReusableSandboxLeases === "true"
&& impact.pendingCleanupLeaseCount > 0 && impact.pendingCleanupLeaseCount > 0
@@ -1378,6 +1390,7 @@ export function environmentRoutes(
await environmentRuntime.destroyReusableSandboxLeasesForEnvironment({ await environmentRuntime.destroyReusableSandboxLeasesForEnvironment({
environmentId: existing.id, environmentId: existing.id,
failureReason: "environment_deleted", failureReason: "environment_deleted",
consentedBy: actor.actorId,
}); });
destroyedReusableSandboxLeaseCount = destroyResult.destroyed; destroyedReusableSandboxLeaseCount = destroyResult.destroyed;
impact = await svc.getDeleteBlastRadius(existing.id); impact = await svc.getDeleteBlastRadius(existing.id);
@@ -4309,12 +4309,30 @@ export function environmentRuntimeService(
* throw leaves the lease in place — so the caller re-checks the blast * throw leaves the lease in place — so the caller re-checks the blast
* radius instead of trusting these counts for the delete decision. * radius instead of trusting these counts for the delete decision.
*/ */
/**
* The operator consented to deleting this environment and the files only
* its restore sources hold (the delete dialog says so). That consent is the
* decision that releases the restore protection: drop `workspaceRestoreRecovery`
* from every row on each of the environment's restore-source sandboxes, and
* record who released it, so the consented teardown can destroy them.
* Without this, every teardown refuses a protected restore source and the
* delete stays blocked with nothing the operator can do.
*/
async releaseRestoreSourcesForEnvironmentDelete(input: { environmentId: string; consentedBy: string }): Promise<number> {
return await releaseRestoreSourcesForEnvironmentDelete(db, input);
},
async destroyReusableSandboxLeasesForEnvironment(input: { async destroyReusableSandboxLeasesForEnvironment(input: {
environmentId: string; environmentId: string;
failureReason?: string; failureReason?: string;
/** The operator who consented to the delete; releases restore protection. */
consentedBy?: string;
}): Promise<{ destroyed: number; failed: number; skippedLiveRun: number }> { }): Promise<{ destroyed: number; failed: number; skippedLiveRun: number }> {
const environment = await environmentsSvc.getById(input.environmentId); const environment = await environmentsSvc.getById(input.environmentId);
if (!environment) return { destroyed: 0, failed: 0, skippedLiveRun: 0 }; if (!environment) return { destroyed: 0, failed: 0, skippedLiveRun: 0 };
if (input.consentedBy) {
await releaseRestoreSourcesForEnvironmentDelete(db, { environmentId: input.environmentId, consentedBy: input.consentedBy });
}
const leaseRows = await db const leaseRows = await db
.select() .select()
.from(environmentLeases) .from(environmentLeases)
@@ -4513,4 +4531,30 @@ export function environmentRuntimeService(
}; };
} }
async function releaseRestoreSourcesForEnvironmentDelete(db: Db, input: { environmentId: string; consentedBy: string }): Promise<number> {
const sources = await db.select({ provider: environmentLeases.provider, providerLeaseId: environmentLeases.providerLeaseId })
.from(environmentLeases).where(and(
eq(environmentLeases.environmentId, input.environmentId),
eq(environmentLeases.leasePolicy, "retain_on_failure"),
inArray(environmentLeases.status, ["released", "retained", "failed", "pending_cleanup"]),
sql`${environmentLeases.metadata} ? 'workspaceRestoreRecovery'`,
sql`${environmentLeases.providerLeaseId} is not null`,
));
let released = 0;
for (const source of sources) {
const rows = await db.update(environmentLeases).set({ updatedAt: new Date(),
metadata: sql`(${environmentLeases.metadata} - 'workspaceRestoreRecovery') || ${JSON.stringify({
restoreSourceReleasedByEnvironmentDelete: { consentedBy: input.consentedBy, releasedAt: new Date().toISOString() },
})}::jsonb`,
}).where(and(
sql`${environmentLeases.provider} is not distinct from ${source.provider}`,
eq(environmentLeases.providerLeaseId, source.providerLeaseId!),
sql`${environmentLeases.metadata} ? 'workspaceRestoreRecovery'`,
)).returning({ id: environmentLeases.id });
released += rows.length;
}
return released;
}
export type EnvironmentRuntimeService = ReturnType<typeof environmentRuntimeService>; export type EnvironmentRuntimeService = ReturnType<typeof environmentRuntimeService>;