Persist scoped reusable sandbox cleanup before provider teardown

doc/plans/2026-10-02-pi-production-readiness.md

@@ -7,6 +7,39 @@ qualification. The existing draft stack must be reviewed in dependency order.
 
 ## Failed reusable-lease deletion correction — 2026-10-06
 
+Review of `fc661a364` identifies a further crash window: issue/workspace
+closure calls provider destruction before recording `pending_cleanup`. The
+correction now shares one durable, scoped cleanup claim with environment
+deletion. The atomic claim re-checks company, environment, issue/workspace,
+lease policy, current status and holding-run liveness. It records an attempt
+identity and renewable cleanup ownership before provider work, preventing
+concurrent closures or sweeps from destroying the same resource. A failed
+provider call or settlement retains the handle for recovery. Controller loss
+leaves bounded ownership which the sweep can reclaim after expiry.
+
+All 288 environment tests and the complete server typecheck pass with this
+correction, without provider credentials or changed test deadlines. Six new
+regressions fail semantically on the preceding shipping source: two closure
+paths lack the durable record, environment teardown lacks exclusive cleanup
+ownership, and queued, scheduled-retry and running transitions bypass the stale
+closure live-run check. The failed log hash is
+`3e87d913a7ecfadf5a5e8988526b6f8236d602d18edc32d28aeb5406e370b203`;
+the corrected suite hash is
+`9a21949740b6cdc6055950bb847eae546a071dc7e4ce9dc5c410a3cf0d88f30b`.
+The first free regression also contains two test-spy restoration failures;
+that output remains preserved. Correcting only spy restoration permits the
+six semantic failures above; no assertion or deadline changes.
+
+The preceding `fc661a364` revision completes full build, recursive typecheck,
+Product harness typecheck and all 1,847 Product unit tests. All 18 freshly
+packed archives install with verified integrity and its Darwin x64 binary
+compiles. Its intermediate image is
+`ghcr.io/paperclipai/paperclip-daytona-runner@sha256:4238d570faac69149ab718bc0a336876b2afbef6924ab2c1eccbaddb2a347d65`.
+That exact head has 52 successful checks, two skips and one failed Greptile
+cleanup review. These artifacts and checks stay historical for the new claim
+correction; do not admit or qualify them as the final shipping set. No paid
+attempt runs during these corrections.
+
 An independent check finds the Daytona pending-permission Stop child still
 started after the canonical case passes and environment deletion returns 200.
 The fixture environment and child labels match the actual run. Its canonical
@@ -71,6 +104,11 @@ No new paid attempts run in either correction. Latest-head CI and review remain
 required. The first cleanup head fails its connection-intent browser scenario:
 its continuation reaches `cancelled` where the unchanged test requires
 `succeeded`. Keep that failure and inspect the actual run before changing code.
+The first local diagnosis cannot launch Chromium from its isolated home.
+Supplying the exact cached Chromium revision passes the unchanged scenario
+with a full trace: both runs succeed, the provider is called once more, and
+the task is done. This does not regrade the failed CI run or prove its cause.
+The fresh `fc661a364` CI browser shard and aggregate subsequently pass.
 
 
 ## Linux process-birth correction — 2026-10-06

server/src/__tests__/environment-runtime.test.ts

@@ -6753,6 +6753,103 @@ describeEmbeddedPostgres("environmentRuntimeService", () => {
     ).resolves.toMatchObject({ status });
   });
 
+  it.each(["issue", "workspace", "environment"] as const)(
+    "durably claims failed reusable cleanup before %s teardown and recovers a provider failure",
+    async (scope) => {
+      const seeded = await seedReusablePluginSandboxLease();
+      await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, seeded.runId));
+      await environmentService(db).releaseLease(seeded.reusableLease.id, "failed");
+      const issueId = randomUUID();
+      if (scope === "issue") {
+        await db.insert(issues).values({ id: issueId, companyId: seeded.companyId, title: "Cleanup crash window", status: "done" });
+        await db.update(environmentLeases).set({ issueId }).where(eq(environmentLeases.id, seeded.reusableLease.id));
+      }
+      let enterProvider!: () => void;
+      let finishProvider!: () => void;
+      const entered = new Promise<void>((resolve) => { enterProvider = resolve; });
+      const finish = new Promise<void>((resolve) => { finishProvider = resolve; });
+      let unavailable = true;
+      const call = vi.fn(async (_id: string, method: string) => {
+        if (method !== "environmentDestroyLease") throw new Error(`Unexpected method ${method}`);
+        if (unavailable) {
+          enterProvider();
+          await finish;
+          throw new Error("Provider response lost");
+        }
+        return { providerLeaseId: seeded.reusableLease.providerLeaseId, state: "destroyed" };
+      });
+      const makeRuntime = () => environmentRuntimeService(db, { pluginWorkerManager: {
+        isRunning: () => true, call,
+        getWorker: () => ({ supportedMethods: ["environmentDestroyLease"] }),
+      } as unknown as PluginWorkerManager });
+      const runtime = makeRuntime();
+      const close = (service: ReturnType<typeof makeRuntime>) => scope === "environment"
+        ? service.destroyReusableSandboxLeasesForEnvironment({ environmentId: seeded.environment.id })
+        : service.destroyReusableSandboxLeases({ companyId: seeded.companyId,
+          ...(scope === "issue" ? { issueId } : { executionWorkspaceId: seeded.executionWorkspaceId }) });
+      const closing = close(runtime);
+      try {
+        await entered;
+        // This independently persisted row is what survives controller loss
+        // during the provider call. No provider receipt has arrived yet.
+        expect(await environmentService(db).getLeaseById(seeded.reusableLease.id)).toMatchObject({
+          status: "pending_cleanup", cleanupStatus: "failed", companyId: seeded.companyId,
+          providerLeaseId: seeded.reusableLease.providerLeaseId,
+          metadata: { pendingCleanupAttemptId: expect.any(String), pendingCleanupInFlight: true,
+            pendingCleanupLeaseExpiresAtMs: expect.any(Number) },
+        });
+        await close(makeRuntime());
+        expect(await heartbeatService(db, { environmentRuntime: makeRuntime() }).sweepPendingCleanupLeases({ backoffMs: 0 }))
+          .toEqual({ swept: 0, destroyed: 0, capped: 0 });
+        expect(call).toHaveBeenCalledOnce();
+      } finally {
+        finishProvider();
+        await closing;
+      }
+      expect(await environmentService(db).getLeaseById(seeded.reusableLease.id)).toMatchObject({
+        status: "pending_cleanup", metadata: { pendingCleanupInFlight: false },
+      });
+      unavailable = false;
+      expect(await heartbeatService(db, { environmentRuntime: makeRuntime() }).sweepPendingCleanupLeases({ backoffMs: 0 }))
+        .toEqual({ swept: 1, destroyed: 1, capped: 0 });
+      expect(call).toHaveBeenCalledTimes(2);
+      expect(await environmentService(db).getLeaseById(seeded.reusableLease.id)).toMatchObject({
+        status: "expired", cleanupStatus: "success", metadata: { remoteExecutionTermination: { state: "destroyed" } },
+      });
+    },
+  );
+
+  it.each(["queued", "scheduled_retry", "running"] as const)(
+    "fences scoped cleanup when its holding run becomes %s after selection", async (status) => {
+      const seeded = await seedReusablePluginSandboxLease();
+      await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, seeded.runId));
+      await environmentService(db).releaseLease(seeded.reusableLease.id, "failed");
+      const originalService = environmentsModule.environmentService;
+      const serviceSpy = vi.spyOn(environmentsModule, "environmentService").mockImplementation((client) => {
+        const service = originalService(client);
+        return { ...service, getById: async (id: string) => {
+          const environment = await service.getById(id);
+          if (id === seeded.environment.id) {
+            await db.update(heartbeatRuns).set({ status }).where(eq(heartbeatRuns.id, seeded.runId));
+          }
+          return environment;
+        } };
+      });
+      const call = vi.fn(async () => undefined);
+      const runtime = environmentRuntimeService(db, { pluginWorkerManager: {
+        isRunning: () => true, call, getWorker: () => ({ supportedMethods: ["environmentDestroyLease"] }),
+      } as unknown as PluginWorkerManager });
+      try {
+        expect(await runtime.destroyReusableSandboxLeases({ companyId: seeded.companyId,
+          executionWorkspaceId: seeded.executionWorkspaceId })).toEqual([]);
+        expect(call).not.toHaveBeenCalled();
+        expect(await originalService(db).getLeaseById(seeded.reusableLease.id)).toMatchObject({ status: "failed" });
+      } finally {
+        serviceSpy.mockRestore();
+      }
+    },
+  );
+
   it("destroys reusable plugin-backed sandbox leases scoped to an environment", async () => {
     const { pluginId, runId, reusableLease } = await seedReusablePluginSandboxLease();
     // The holding run is finished, so the reservation is stale and destroyable.

server/src/services/environment-runtime.ts

@@ -3856,6 +3856,86 @@ export function environmentRuntimeService(
     return driver;
   }
 
+  async function destroyScopedReusableSandboxLease(input: {
+    environment: Environment;
+    leaseRow: typeof environmentLeases.$inferSelect;
+    scopeCondition?: ReturnType<typeof and>;
+    failureReason: string;
+  }): Promise<EnvironmentLease | null> {
+    const now = new Date();
+    const attemptId = randomUUID();
+    // Persist recovery ownership before any provider work. Re-check scope and
+    // run liveness in this write: the earlier selection cannot fence a resume.
+    // The in-flight lease also excludes concurrent closures and cleanup sweeps.
+    const [claimed] = await db.update(environmentLeases).set({
+      status: "pending_cleanup",
+      failureReason: input.failureReason,
+      cleanupStatus: "failed",
+      releasedAt: now,
+      lastUsedAt: now,
+      updatedAt: now,
+      metadata: sql`(case when jsonb_typeof(${environmentLeases.metadata}) = 'object'
+        then ${environmentLeases.metadata} else '{}'::jsonb end - 'remoteExecutionTermination')
+        || ${JSON.stringify({ pendingCleanupAttemptId: attemptId, pendingCleanupInFlight: true,
+          pendingCleanupLeaseExpiresAtMs: Date.now() + 15 * 60_000 })}::jsonb`,
+    }).where(and(
+      eq(environmentLeases.id, input.leaseRow.id),
+      eq(environmentLeases.companyId, input.leaseRow.companyId),
+      eq(environmentLeases.environmentId, input.environment.id),
+      eq(environmentLeases.leasePolicy, "reuse_by_environment"),
+      eq(environmentLeases.status, input.leaseRow.status),
+      input.scopeCondition,
+      sql`coalesce(${environmentLeases.metadata}->>'pendingCleanupInFlight', 'false') != 'true'`,
+      sql`NOT EXISTS (
+        SELECT 1 FROM ${heartbeatRuns}
+        WHERE ${heartbeatRuns.id} = ${environmentLeases.heartbeatRunId}
+          AND ${heartbeatRuns.status} IN ('queued', 'scheduled_retry', 'running')
+      )`,
+    )).returning();
+    if (!claimed) return null;
+    const lease = toEnvironmentLeaseSnapshot(claimed);
+    let renewalInFlight = false;
+    const renewal = setInterval(() => {
+      if (renewalInFlight) return;
+      renewalInFlight = true;
+      void db.update(environmentLeases).set({
+        metadata: sql`${environmentLeases.metadata} || ${JSON.stringify({
+          pendingCleanupLeaseExpiresAtMs: Date.now() + 15 * 60_000,
+        })}::jsonb`,
+      }).where(and(eq(environmentLeases.id, lease.id), eq(environmentLeases.status, "pending_cleanup"),
+        sql`${environmentLeases.metadata}->>'pendingCleanupAttemptId' = ${attemptId}`,
+        sql`${environmentLeases.metadata}->>'pendingCleanupInFlight' = 'true'`,
+      )).then(() => {}).catch(() => {
+        logger.warn({ leaseId: lease.id }, "scoped cleanup ownership renewal failed");
+      }).finally(() => { renewalInFlight = false; });
+    }, 30_000);
+    renewal.unref();
+    try {
+      const driver = getDriver(getLeaseDriverKey(lease, input.environment));
+      if (!driver?.destroyRunLease) return lease;
+      return await driver.destroyRunLease({
+        environment: input.environment,
+        lease,
+        failureReason: input.failureReason,
+      }) ?? lease;
+    } catch {
+      // Even a failed settlement write leaves the durable provider handle for
+      // the sweep. Continue with later leases without logging provider errors.
+      return lease;
+    } finally {
+      clearInterval(renewal);
+      // A crash skips this write; the bounded ownership lease lets the sweep
+      // recover. A completed failure becomes eligible for a later explicit retry.
+      await db.update(environmentLeases).set({
+        metadata: sql`${environmentLeases.metadata} || '{"pendingCleanupInFlight":false}'::jsonb`,
+      }).where(and(eq(environmentLeases.id, lease.id),
+        sql`${environmentLeases.metadata}->>'pendingCleanupAttemptId' = ${attemptId}`,
+      )).catch(() => {
+        logger.warn({ leaseId: lease.id }, "scoped cleanup ownership settlement failed");
+      });
+    }
+  }
+
   return {
     getDriver,
 
@@ -4203,18 +4283,12 @@ export function environmentRuntimeService(
           ? await environmentsSvc.getById(leaseRow.environmentId)
           : null;
         if (!environment) continue;
-        const leaseSnapshot = toEnvironmentLeaseSnapshot(leaseRow);
-        const driver = getDriver(getLeaseDriverKey(leaseSnapshot, environment));
-        const lease = driver?.destroyRunLease
-          ? await driver.destroyRunLease({
-              environment,
-              lease: leaseSnapshot,
-              failureReason: input.failureReason ?? "reusable_lease_destroyed",
-            })
-          : await environmentsSvc.releaseLease(leaseSnapshot.id, "pending_cleanup", {
-              failureReason: input.failureReason ?? "reusable_lease_destroyed",
-              cleanupStatus: "failed",
-            });
+        const lease = await destroyScopedReusableSandboxLease({
+          environment,
+          leaseRow,
+          scopeCondition: and(...scopeConditions),
+          failureReason: input.failureReason ?? "reusable_lease_destroyed",
+        });
         if (!lease) continue;
         destroyed.push({
           environment,
@@ -4285,73 +4359,20 @@ export function environmentRuntimeService(
       let failed = 0;
       let skippedLiveRun = 0;
       const failureReason = input.failureReason ?? "environment_delete_requested";
-      const now = new Date();
       for (const leaseRow of leaseRows) {
         if (leaseRow.heartbeatRunId && liveRunIds.has(leaseRow.heartbeatRunId)) {
           skippedLiveRun += 1;
           continue;
         }
-        // Claim the row BEFORE the provider call, mirroring the inline-teardown
-        // invariant used elsewhere in this file: no provider destroy without a
-        // durable `pending_cleanup` reference already on disk. The claim is one
-        // conditional UPDATE, so it is the fence against a racing resume: a
-        // resume that re-activates the lease first makes the status predicate
-        // (or the run-liveness predicate) fail and the claim loses — the live
-        // run keeps its sandbox. A claim that wins parks the lease where the
-        // cleanup sweep owns it, so a crash or thrown destroy after this point
-        // is recovered by the sweep's idempotent teardown, and a double write
-        // failure cannot strand the lease in a reusable status.
-        const claimedRow = await db
-          .update(environmentLeases)
-          .set({
-            status: "pending_cleanup",
-            failureReason,
-            cleanupStatus: "failed",
-            releasedAt: now,
-            lastUsedAt: now,
-            updatedAt: now,
-          })
-          .where(
-            and(
-              eq(environmentLeases.id, leaseRow.id),
-              inArray(environmentLeases.status, ["active", "released", "retained", "failed"]),
-              sql`NOT EXISTS (
-                SELECT 1 FROM ${heartbeatRuns}
-                WHERE ${heartbeatRuns.id} = ${environmentLeases.heartbeatRunId}
-                  AND ${heartbeatRuns.status} IN ('queued', 'scheduled_retry', 'running')
-              )`,
-            ),
-          )
-          .returning()
-          .then((rows) => rows[0] ?? null);
-        if (!claimedRow) {
+        const lease = await destroyScopedReusableSandboxLease({ environment, leaseRow, failureReason });
+        if (!lease) {
           // Lost to a racing resume or a concurrent terminal transition — the
           // lease is no longer ours to destroy.
           skippedLiveRun += 1;
           continue;
         }
-        const leaseSnapshot = toEnvironmentLeaseSnapshot(claimedRow);
-        try {
-          const driver = getDriver(getLeaseDriverKey(leaseSnapshot, environment));
-          if (!driver?.destroyRunLease) {
-            // No driver available: the claim already parked the lease for the
-            // sweep, which retries once the driver's plugin is back.
-            failed += 1;
-            continue;
-          }
-          const lease = await driver.destroyRunLease({
-            environment,
-            lease: leaseSnapshot,
-            failureReason,
-          });
-          if (lease && lease.status !== "pending_cleanup") destroyed += 1;
-          else failed += 1;
-        } catch {
-          // The claim above already parked the lease in `pending_cleanup`, so
-          // the sweep owns the retry; its teardown is idempotent, so a destroy
-          // that reached the provider before the throw resolves as success.
-          failed += 1;
-        }
+        if (lease.status !== "pending_cleanup") destroyed += 1;
+        else failed += 1;
       }
       return { destroyed, failed, skippedLiveRun };
     },