diff --git a/doc/plans/2026-10-02-pi-production-readiness.md b/doc/plans/2026-10-02-pi-production-readiness.md index 0589ea48689..ca48a476751 100644 --- a/doc/plans/2026-10-02-pi-production-readiness.md +++ b/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 diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index 1943b9ee392..874bd88dc03 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/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((resolve) => { enterProvider = resolve; }); + const finish = new Promise((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) => 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. diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index 73a11790885..b9d27990f69 100644 --- a/server/src/services/environment-runtime.ts +++ b/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; + failureReason: string; + }): Promise { + 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 }; },