diff --git a/server/src/__tests__/environment-routes.test.ts b/server/src/__tests__/environment-routes.test.ts index 826e7517858..e7ed84da754 100644 --- a/server/src/__tests__/environment-routes.test.ts +++ b/server/src/__tests__/environment-routes.test.ts @@ -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(), diff --git a/server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts b/server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts index 70040517547..a637dc89bec 100644 --- a/server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts +++ b/server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts @@ -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, diff --git a/server/src/__tests__/sandbox-creation-operations.test.ts b/server/src/__tests__/sandbox-creation-operations.test.ts index e4792ce6ce0..d66102567f0 100644 --- a/server/src/__tests__/sandbox-creation-operations.test.ts +++ b/server/src/__tests__/sandbox-creation-operations.test.ts @@ -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() })); diff --git a/server/src/app.ts b/server/src/app.ts index 394e5e05504..f978a50459b 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -796,6 +796,7 @@ export async function createApp( api.use( environmentRoutes(db, { pluginWorkerManager: workerManager, + retryPendingEnvironmentCleanup: input => connectionIntentHeartbeat.sweepPendingCleanupLeases({ environmentRetry: input }), recoverMissingPluginWorker: recoverManagedBundledPluginWorker ? { pluginKeys: managedBundledPluginKeys, diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 3cafcc7f7fa..879348d21c6 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -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; } = {}, ) { 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 diff --git a/server/src/services/heartbeat/recovery.ts b/server/src/services/heartbeat/recovery.ts index bce456589bd..83f533df2c8 100644 --- a/server/src/services/heartbeat/recovery.ts +++ b/server/src/services/heartbeat/recovery.ts @@ -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(); @@ -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;