fix(environments): run pending sandbox cleanup when an environment is deleted (#19716)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Operators delete a sandbox environment, optionally with consent to
destroy its sandboxes (`DELETE
/api/environments/:id?destroyReusableSandboxLeases=true`)
> - A sandbox whose teardown failed or was interrupted is kept as a
`pending_cleanup` lease, and it blocks the delete
> - The cleanup sweep retries such a lease only after a cooldown of up
to 5 minutes
> - A consented delete therefore returned 409 `pending_sandbox_cleanup`
for minutes. A caller with a shorter deadline gave up, and the
environment and its sandbox stayed behind
> - This pull request makes the consented delete run one teardown
attempt for the environment's pending cleanups now, then re-check
> - The benefit is that a consented environment delete completes as soon
as the provider teardown succeeds

## Linked Issues or Issue Description

**What happened?**
After a restart chaos wave, the harness deleted the wave's Daytona
environment with `destroyReusableSandboxLeases=true`. The server
returned 409 `pending_sandbox_cleanup` every 3 seconds for the whole
120-second client deadline. A run lease had entered `pending_cleanup`
after its run failed, and the sweep's retry cooldown had not passed. The
environment was not deleted.

**Expected behavior**
With explicit consent, the delete tears down the environment's own
pending sandbox cleanups and completes when that teardown succeeds. A
teardown that fails, or one already in flight elsewhere, still blocks
the delete.

**Steps to reproduce**
1. Leave a sandbox lease of an environment in `pending_cleanup` with a
future retry time.
2. `DELETE /api/environments/:id?destroyReusableSandboxLeases=true`.
3. On master, the response is 409 `pending_sandbox_cleanup` until the
cooldown ends.

Refs #19696 (restart recovery for creation and teardown claims; this
builds on it).

## What Changed

- `heartbeat/recovery.ts`: `sweepPendingCleanupLeases` accepts
`environmentRetry: { environmentId, actorId }`. Like the existing
run-scoped explicit retry, it bypasses the cooldown for one attempt,
never takes over an in-flight attempt, and logs
`environment_lease.cleanup_retried` with reason `environment_deleted`.
It touches only that environment's leases.
- `routes/environments.ts`: when the delete has consent and the only
blockers are pending cleanup and reusable leases, it runs that retry
first, then re-reads the blast radius. The existing reusable-lease
destroy and the final guard are unchanged. The new dependency is a
narrow, optional `retryPendingEnvironmentCleanup` option; without it,
behavior is unchanged.
- `app.ts`: wires the option to the existing heartbeat service instance.

## Verification

- Red, then green:
- `environment-routes.test.ts`: "runs the environment's pending sandbox
cleanup now and deletes with explicit consent" and "keeps rejecting when
the consented pending cleanup attempt does not finish" fail before and
pass after. "does not run pending sandbox cleanup without consent or
past another blocker" passes in both.
- `heartbeat-pending-cleanup-sweep.test.ts`: "runs one attempt for a
deleted environment's cleanup during its cooldown, scoped to that
environment" fails before and passes after. It checks that another
environment's lease and an in-flight claim are untouched, and that the
activity log is written.
- `sandbox-creation-operations.test.ts`: "reclaims an issue-bound
creation after its run failed and the issue finished on a later run"
covers another chaos finding. A creation intent bound to an issue was
interrupted by SIGKILL 0.5 s after it committed, its run ended
`process_lost`, and a later run finished the issue. The test fails on
the code before #19696 and passes on master, so #19696 already covers
this case. The test is kept as a regression.
- After rebasing on master, `environment-routes`,
`heartbeat-pending-cleanup-sweep`, `agent-runner-qualification-routes`
and `sandbox-creation-operations` pass (192 tests). `tsc --noEmit`
reports no errors in the changed files.

## Risks

- A consented delete now makes provider teardown calls for pending
cleanups. Those sandboxes were already queued for destruction, and the
operator consented to destroy the environment's sandboxes.
- One explicit attempt counts toward the lease's attempt cap, the same
as a user Retry.
- The delete request now waits for those teardown attempts. Each attempt
uses the existing plugin timeouts.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file editing, test execution). Extended reasoning
enabled.

## 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-06 21:28:54 -06:00
1 parent 51eb4ff705
commit aed053633e
6 files changed
+161 -5

No files matched your search

@@ -1830,6 +1830,54 @@ describe("environment routes", () => {
expect(mockEnvironmentService.removeIfDeletable).not.toHaveBeenCalled();
});
it("runs the environment's pending sandbox cleanup now and deletes with explicit consent", async () => {
const environment = createEnvironment();
mockEnvironmentService.getById.mockResolvedValue(environment);
mockEnvironmentService.getDeleteBlastRadius
.mockResolvedValueOnce(createDeleteBlastRadius({ pendingCleanupLeaseCount: 1 }))
.mockResolvedValueOnce(createDeleteBlastRadius());
mockEnvironmentService.removeIfDeletable.mockResolvedValue(environment);
mockInstanceSettingsService.listCompanyIds.mockResolvedValue(["company-1"]);
const retryPendingEnvironmentCleanup = vi.fn(async () => ({ swept: 1, destroyed: 1, capped: 0 }));
const app = createApp(localBoardActor("admin-1"), { retryPendingEnvironmentCleanup });
const res = await request(app).delete("/api/environments/env-1?destroyReusableSandboxLeases=true");
expect(res.status).toBe(200);
expect(retryPendingEnvironmentCleanup).toHaveBeenCalledExactlyOnceWith({ environmentId: "env-1", actorId: "admin-1" });
expect(mockEnvironmentService.removeIfDeletable).toHaveBeenCalledWith("env-1");
});
it("keeps rejecting when the consented pending cleanup attempt does not finish", async () => {
const environment = createEnvironment();
mockEnvironmentService.getById.mockResolvedValue(environment);
mockEnvironmentService.getDeleteBlastRadius.mockResolvedValue(createDeleteBlastRadius({ pendingCleanupLeaseCount: 1 }));
const retryPendingEnvironmentCleanup = vi.fn(async () => ({ swept: 1, destroyed: 0, capped: 0 }));
const app = createApp(localBoardActor("admin-1"), { retryPendingEnvironmentCleanup });
const res = await request(app).delete("/api/environments/env-1?destroyReusableSandboxLeases=true");
expect(res.status).toBe(409);
expect(res.body.details).toEqual({ deleteBlockedReasons: ["pending_sandbox_cleanup"] });
expect(retryPendingEnvironmentCleanup).toHaveBeenCalledOnce();
expect(mockEnvironmentService.removeIfDeletable).not.toHaveBeenCalled();
});
it("does not run pending sandbox cleanup without consent or past another blocker", async () => {
const environment = createEnvironment();
mockEnvironmentService.getById.mockResolvedValue(environment);
const retryPendingEnvironmentCleanup = vi.fn(async () => ({ swept: 0, destroyed: 0, capped: 0 }));
const app = createApp(localBoardActor("admin-1"), { retryPendingEnvironmentCleanup });
mockEnvironmentService.getDeleteBlastRadius.mockResolvedValue(createDeleteBlastRadius({ pendingCleanupLeaseCount: 1 }));
expect((await request(app).delete("/api/environments/env-1")).status).toBe(409);
mockEnvironmentService.getDeleteBlastRadius.mockResolvedValue(createDeleteBlastRadius({
pendingCleanupLeaseCount: 1, isInstanceDefault: true,
}));
expect((await request(app).delete("/api/environments/env-1?destroyReusableSandboxLeases=true")).status).toBe(409);
expect(retryPendingEnvironmentCleanup).not.toHaveBeenCalled();
expect(mockEnvironmentService.removeIfDeletable).not.toHaveBeenCalled();
});
it("rejects a driver or provider config change while a sandbox cleanup is pending", async () => {
const environment = {
...createEnvironment(),
@@ -412,6 +412,53 @@ describeEmbeddedPostgres("heartbeat sweepPendingCleanupLeases", () => {
});
});
it("runs one attempt for a deleted environment's cleanup during its cooldown, scoped to that environment", async () => {
const { companyId, environmentId } = await seedCompanyAndEnvironment();
const otherEnvironmentId = randomUUID();
await db.insert(environments).values({ id: otherEnvironmentId, name: `Other ${otherEnvironmentId}`, driver: "sandbox",
status: "active", config: { provider: "fake", image: "ubuntu:24.04" } });
const cooling = { [ATTEMPTS_KEY]: 1, pendingCleanupRetryAfterMs: Date.now() + 4 * 60_000 };
const target = await insertOrphanEphemeralLease({ companyId, environmentId, updatedAt: new Date(), metadata: cooling });
const other = await insertOrphanEphemeralLease({ companyId, environmentId: otherEnvironmentId, updatedAt: new Date(), metadata: cooling });
const inFlight = await insertOrphanEphemeralLease({ companyId, environmentId, updatedAt: new Date(), metadata: {
...cooling, pendingCleanupAttemptId: "live-claim", pendingCleanupInFlight: true,
pendingCleanupLeaseExpiresAtMs: Date.now() + 10 * 60_000,
} });
const teardown = vi.fn(async () => {});
const service = heartbeatService(db, { environmentRuntime: {
retryPendingSandboxTeardown: teardown,
} as unknown as HeartbeatEnvironmentRuntime });
expect((await service.sweepPendingCleanupLeases()).swept).toBe(0);
const result = await service.sweepPendingCleanupLeases({ environmentRetry: { environmentId, actorId: "admin-1" } });
expect(result).toMatchObject({ swept: 1, destroyed: 1 });
expect(teardown).toHaveBeenCalledOnce();
const statuses = Object.fromEntries((await db.select().from(environmentLeases)).map(lease => [lease.id, lease.status]));
expect(statuses).toEqual({ [target]: "expired", [other]: "pending_cleanup", [inFlight]: "pending_cleanup" });
const [activity] = await db.select().from(activityLog).where(eq(activityLog.entityId, target));
expect(activity).toMatchObject({ action: "environment_lease.cleanup_retried", actorId: "admin-1",
details: expect.objectContaining({ reason: "environment_deleted" }) });
});
it("tries every cleanup of a deleted environment once, past one sweep page", async () => {
const { companyId, environmentId } = await seedCompanyAndEnvironment();
const cooling = { [ATTEMPTS_KEY]: 1, pendingCleanupRetryAfterMs: Date.now() + 4 * 60_000 };
for (let index = 0; index < SWEEP_PAGE_SIZE + 5; index++) {
await insertOrphanEphemeralLease({ companyId, environmentId, updatedAt: new Date(), metadata: cooling });
}
const teardown = vi.fn(async () => {});
const service = heartbeatService(db, { environmentRuntime: {
retryPendingSandboxTeardown: teardown,
} as unknown as HeartbeatEnvironmentRuntime });
const result = await service.sweepPendingCleanupLeases({ environmentRetry: { environmentId, actorId: "admin-1" } });
expect(result.destroyed).toBe(SWEEP_PAGE_SIZE + 5);
expect(teardown).toHaveBeenCalledTimes(SWEEP_PAGE_SIZE + 5);
expect((await db.select().from(environmentLeases)).every(lease => lease.status === "expired")).toBe(true);
});
it("starts escalation and the long cooldown on the fifth failed attempt", async () => {
const { companyId, environmentId } = await seedCompanyAndEnvironment();
const leaseId = await insertPendingCleanupLease({ companyId, environmentId,
@@ -8,7 +8,7 @@ import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { eq, sql } from "drizzle-orm";
import { agents, builtInManagedResources, companies, companySecretVersions, createDb, environmentLeases, environments, heartbeatRuns, plugins, sandboxCreationOperations } from "@paperclipai/db";
import { agents, builtInManagedResources, companies, companySecretVersions, createDb, environmentLeases, environments, heartbeatRuns, issues, plugins, sandboxCreationOperations } from "@paperclipai/db";
import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
import { environmentRuntimeService } from "../services/environment-runtime.js";
@@ -379,6 +379,30 @@ describe("durable sandbox creation through the runtime entry point", () => {
expect(await db.select().from(environmentLeases)).toEqual([expect.objectContaining({ status: "pending_cleanup",
providerLeaseId: resources.get(operation.id) })]);
});
it("reclaims an issue-bound creation after its run failed and the issue finished on a later run", async () => {
// Mirrors a kept chaos database: the server was SIGKILLed 0.5 s after the
// intent committed, the run ended process_lost, and a later run finished
// the task. The intent stayed pending with no lease.
const pid = await exitedPid();
const operation = await interruptedCreation(async current => ({ ...current, bootId: randomUUID(), pid }));
const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId));
const issueId = randomUUID();
await db.insert(issues).values({ id: issueId, companyId, title: "Restart-crossing task", status: "done" });
await db.update(sandboxCreationOperations).set({ issueId }).where(eq(sandboxCreationOperations.id, operation.id));
await db.update(heartbeatRuns).set({ status: "failed", errorCode: "process_lost", finishedAt: new Date() })
.where(eq(heartbeatRuns.id, runId));
await db.insert(heartbeatRuns).values({ ...run!, id: randomUUID(), status: "succeeded", finishedAt: new Date() });
expect(await db.select().from(sandboxCreationOperations)).toEqual([
expect.objectContaining({ state: "pending", leaseId: null, issueId, runId }),
]);
await runtime().flushDeferredOrphanCleanups();
const [lease] = await db.select().from(environmentLeases);
expect(lease).toMatchObject({ status: "pending_cleanup", companyId, providerLeaseId: resources.get(operation.id) });
await runtime().retryPendingSandboxTeardown({ environment: null, lease: (await environmentService(db).getLeaseById(lease!.id))! });
expect(resources.size).toBe(0); expect(destroys).toBe(1); expect(creates).toBe(1);
});
it("treats a restarted server that reused the creator PID as a different process", async () => {
await interruptedCreation(async current => ({ ...current, bootId: randomUUID(),
processStartedAt: new Date(Date.parse(String(current.processStartedAt)) - 60_000).toISOString() }));
+1
View File
@@ -796,6 +796,7 @@ export async function createApp(
api.use(
environmentRoutes(db, {
pluginWorkerManager: workerManager,
retryPendingEnvironmentCleanup: input => connectionIntentHeartbeat.sweepPendingCleanupLeases({ environmentRetry: input }),
recoverMissingPluginWorker: recoverManagedBundledPluginWorker
? {
pluginKeys: managedBundledPluginKeys,
+22
View File
@@ -360,6 +360,10 @@ export function environmentRoutes(
options: {
pluginWorkerManager?: PluginWorkerManager;
recoverMissingPluginWorker?: ReadyPluginWorkerRecovery;
/** Runs one teardown attempt now for each pending cleanup of this
* environment, without the retry cooldown. Absent, a delete waits for the
* scheduled cleanup sweep. */
retryPendingEnvironmentCleanup?: (input: { environmentId: string; actorId: string }) => Promise<unknown>;
} = {},
) {
const router = Router();
@@ -1332,6 +1336,24 @@ export function environmentRoutes(
// sandboxes belonged to the environment the operator consented to
// destroy; the rejection reports the count so nothing is silent.
let destroyedReusableSandboxLeaseCount = 0;
// The same consent covers a pending sandbox cleanup: it is the environment's
// 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
// over, and a failed attempt keeps the lease, so the re-check still rejects.
if (
req.query.destroyReusableSandboxLeases === "true"
&& impact.pendingCleanupLeaseCount > 0
&& options.retryPendingEnvironmentCleanup
&& impact.deleteBlockedReasons.every((reason) =>
reason === "pending_sandbox_cleanup" || reason === "reusable_sandbox_lease")
) {
await options.retryPendingEnvironmentCleanup({ environmentId: existing.id, actorId: actor.actorId });
impact = await svc.getDeleteBlastRadius(existing.id);
if (!impact) {
res.status(404).json({ error: "Environment not found" });
return;
}
}
if (
req.query.destroyReusableSandboxLeases === "true"
&& impact.reusableSandboxLeaseCount > 0
+18 -4
View File
@@ -188,6 +188,9 @@ const NATIVE_OWNERSHIP_UNVERIFIED_MESSAGE =
// The reaper sweeps at most this many pending_cleanup leases per tick.
const PENDING_CLEANUP_SWEEP_PAGE_SIZE = 20;
// An environment delete tries each of that environment's cleanups once, from
// one snapshot, so the request does not stop after a sweep page.
const ENVIRONMENT_CLEANUP_RETRY_LIMIT = 1_000;
const pendingCleanupAttemptsInFlight = new Set<string>();
@@ -1717,6 +1720,10 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD
* but cannot take over an in-flight cleanup attempt.
*/
explicitRetry?: { companyId: string; runId: string; actorId: string; reason?: "retry_failed_run" | "queued_comment_interrupt" };
/** One cleanup attempt per lease of this environment, for an operator who
* asked to delete it with its sandboxes. It bypasses the cooldown in the
* same way as `explicitRetry`, and never takes over an in-flight attempt. */
environmentRetry?: { environmentId: string; actorId: string };
}): Promise<{
swept: number;
destroyed: number;
@@ -1725,6 +1732,7 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD
const backoffMs = opts?.backoffMs ?? 0;
const now = new Date();
const cutoff = new Date(now.getTime() - backoffMs);
const manualRetry = Boolean(opts?.explicitRetry || opts?.environmentRetry);
let deferredFlushFailed = false;
// Flush the in-process orphan-cleanup buffer first. A failed acquire buffers
@@ -1733,7 +1741,7 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD
// `pending_cleanup` row lands once the database recovers. The flush runs
// before the read below, so this same tick tears down a freshly-landed row.
try {
const flushed = opts?.explicitRetry ? null : await environmentRuntime.flushDeferredOrphanCleanups?.();
const flushed = manualRetry ? null : await environmentRuntime.flushDeferredOrphanCleanups?.();
if (flushed && (flushed.recovered > 0 || flushed.pending > 0)) {
logger.info(
{ recovered: flushed.recovered, pending: flushed.pending },
@@ -1759,12 +1767,13 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD
eq(environmentLeases.status, "pending_cleanup"),
opts?.explicitRetry ? eq(environmentLeases.companyId, opts.explicitRetry.companyId) : undefined,
opts?.explicitRetry ? eq(environmentLeases.heartbeatRunId, opts.explicitRetry.runId) : undefined,
pendingCleanupRetryDueSql(Boolean(opts?.explicitRetry)),
opts?.environmentRetry ? eq(environmentLeases.environmentId, opts.environmentRetry.environmentId) : undefined,
pendingCleanupRetryDueSql(manualRetry),
backoffMs > 0 ? lte(environmentLeases.updatedAt, cutoff) : undefined,
),
)
.orderBy(asc(environmentLeases.updatedAt))
.limit(PENDING_CLEANUP_SWEEP_PAGE_SIZE);
.limit(opts?.environmentRetry ? ENVIRONMENT_CLEANUP_RETRY_LIMIT : PENDING_CLEANUP_SWEEP_PAGE_SIZE);
let destroyed = 0;
let capped = 0;
@@ -1837,7 +1846,7 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD
// The deadline survives a server restart; the counter saturates at the
// escalation threshold while attempt identities remain unique.
const claimed = await claimPendingCleanupRetryAttempt(row.id, attempts,
opts?.explicitRetry ? { previousId: metadata.pendingCleanupManualAttemptId } : undefined);
manualRetry ? { previousId: metadata.pendingCleanupManualAttemptId } : undefined);
if (!claimed) continue;
pendingCleanupAttemptsInFlight.add(row.id);
lease.metadata = { ...lease.metadata, pendingCleanupAttemptId: claimed };
@@ -1862,6 +1871,11 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD
action: "environment_lease.cleanup_retried", entityType: "environment_lease", entityId: row.id,
runId: opts.explicitRetry.runId, details: { attempt: attempts + 1, reason: opts.explicitRetry.reason ?? "retry_failed_run" },
});
else if (opts?.environmentRetry) await logActivity(db, {
companyId: row.companyId, actorType: "user", actorId: opts.environmentRetry.actorId,
action: "environment_lease.cleanup_retried", entityType: "environment_lease", entityId: row.id,
runId: row.heartbeatRunId ?? undefined, details: { attempt: attempts + 1, reason: "environment_deleted" },
});
if (useRecordedTeardown) {
// Tear the sandbox down from the recorded provider config and the
// cleanup-authorized secret versions. Preserve any provider receipt;