From a5fe5deebe13a115e8b0571ae96e8c22d05e5936 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 20:39:34 -0600 Subject: [PATCH] fix(environments): reclaim sandboxes left by a restart during acquire or teardown (#19696) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Remote agent runs execute in provider sandboxes (for example Daytona) that Paperclip creates and must clean up > - Before each provider create, Paperclip saves a durable creation intent (`sandbox_creation_operations`). Recovery uses it to find and destroy a sandbox whose lease was never committed > - Recovery waited 30 minutes after creation before it checked an intent, because a live server could still be waiting for the provider > - After a restart, the creating server is gone, but the intent still waited the full 30 minutes. A short-lived or redeployed instance never reclaimed those sandboxes > - The teardown that follows has the same problem: a teardown claim fences other sweeps for 15 minutes, and a restart during the claim kept the sandbox alive until the fence expired > - This pull request saves the server process identity with each intent and each teardown claim, and ends the wait when same-host proof shows that process exited > - The benefit is that a restart during sandbox acquire or teardown no longer leaves paid provider sandboxes behind ## Linked Issues or Issue Description **What happened?** A control-plane restart during Daytona sandbox acquire left the created sandbox without an `environment_leases` row. In one restart chaos wave, 6 creation intents stayed `pending` with `next_attempt_at` = creation + 30 minutes. The wave ended (and its environment was deleted) before that time, so 4 sandboxes stayed `stopped` in Daytona. Only Daytona's own auto-delete would remove them. Two of the runs did not exist in `heartbeat_runs` because they were non-run acquires; their intents still had the run id. **Expected behavior** After a restart, each sandbox that this instance and company created, with no committed lease and no live creator, is found by its exact creation name and labels and destroyed through the plugin, after the existing ownership checks. Sandboxes from another instance, another host, or a live server stay untouched. Nothing acts org-wide. **Steps to reproduce** 1. Start a Daytona-backed run. 2. Kill the server (SIGKILL) after the creation intent commits and before the lease commits. 3. Restart the server. On master, the intent stays `pending` for 30 minutes and the sandbox stays in Daytona. 4. The regression tests in `server/src/__tests__/sandbox-creation-operations.test.ts` reproduce this with the controlled provider fixture. Refs #18357 (added the durable creation intent this change completes). **Paperclip version or commit** master 07a548073a (live proof at 8d525dba7d + this change, content-matched to the runner image) ## What Changed - New `server/src/services/controller-process-identity.ts`: records the server boot id, PID, a PID-space hash and the process start time. The PID-space hash covers the hostname, the kernel boot id and the PID namespace on Linux (the hostname and boot time elsewhere), so containers that share a hostname but not a PID namespace never judge each other. It proves that a recorded process exited only in the same PID space: the PID is gone, or the PID is live with a definitely different start time. Another PID space, an unreadable identity, an unidentified record, or this server never counts as exited. - `sandbox-creation-operations.ts`: each creation intent stores the creating process and its settle deadline. Each recovery pass first moves intents whose creator exited to "due now". If the early lookup finds no sandbox before the settle deadline, the intent stays `pending` and retries. It does not become manual reconciliation, because the provider can still materialize an aborted request. - `heartbeat/recovery.ts`: each pending-cleanup teardown claim stores the claiming process, bound to its claim attempt id. Before the sweep reads due leases, it ends the 15-minute fence of a claim whose claimer exited. A newer claim from another path (workspace repair, export retry) that kept an older record has a different attempt id, so it is never released. - Both recovery passes page through candidates with a stable id cursor. Live, foreign or unidentified owners therefore cannot hide an abandoned record behind one page. - This covers every `createOwnedSandbox` caller, including non-task probe and qualification acquires. Their intents have no issue and a generated run id, which is why their run ids are missing from `heartbeat_runs`. - Recovery uses only the existing paths: exact `paperclip-create-` name and label inspection in the plugin, `pending_cleanup` lease, and plugin teardown. No org-wide listing or deletion was added. ## Verification - Red, then green (controlled provider fixture, real embedded PostgreSQL): - `sandbox-creation-operations.test.ts` ā†’ "after the creating server exits mid-acquire". Before the fix, the exited-creator case left no cleanup lease. After the fix, the next sweep creates the `pending_cleanup` lease and the teardown destroys the sandbox. Counter-tests: a creation owned by this live server, another live process, another host, or a committed lease is not inspected or destroyed. An early "not found" keeps the intent pending, and a later sweep still finds a late allocation. - `heartbeat-pending-cleanup-sweep.test.ts` ā†’ "a teardown claim whose server stopped mid-attempt". Before the fix, a lost claim blocked the teardown. After the fix, it retries now. Counter-tests: claims from this live server, another host, or without an identity keep their fence. - Review regressions: "keeps the fence of a newer claim that kept an earlier attempt's server record" and "pages past more fenced claims than one sweep page" fail on the previous commit and pass now. "pages past more unprovable creations than one page" guards the creation-side cursor; the old unordered page happened to include the row, so it was not red. - After the review changes, `sandbox-creation-operations`, `heartbeat-pending-cleanup-sweep`, `live-recovery-work` and `environment-cleanup-work` pass (155 tests), and `tsc --noEmit` for `server` passes. - Related suites pass: sandbox-creation-operations (28), environment-cleanup-work, task-creation-recovery, native-startup-cleanup-recovery, sandbox-orphan-cleanup-spool, plugin-warm-tasks, idle-completed-history (187 total), and the pending-cleanup sweep suite. `tsc --noEmit` for `server` passes. - Live Daytona restart chaos wave `w-20261107T001609-s11` (codex, A1, K2+K4, concurrency 2, RC-06 masked, image `sha256:2767e91cā€¦`, first fix only): 4/4 tasks pass, all restart-crossing, R6 pass for every task, `orphanSandboxes: []`. The server log and database show two creation intents killed by K4 being reconciled right after restart and destroyed through Paperclip. A third reclaimed sandbox lost its teardown claim to the next K4 kill in the same second, so its 15-minute fence blocked the environment delete. The second commit fixes that. - Live Daytona wave `w-20261107T012048-s11`, same plan, both commits: **4/4 pass, all restart-crossing, R6 pass for every task, `orphanSandboxes: []`, no held leases.** The environment delete through Paperclip succeeded with `remainingSandboxes: []`. One restart-time sweep destroyed one sandbox through the plugin. ## Risks - Same-host proof relies on PID liveness and process start time, as native controller takeover already does. A false "exited" result is limited to recovery running early. The plugin still checks exact name and labels before any delete. An early not-found result stays pending. - Multi-host deployments sharing one database keep the old deadlines, because a different PID space is never judged. A redeploy into a new host or PID namespace also keeps the old deadlines (fail closed). - The live waves ran before the review commits. The review commits only narrow when a record is judged, and they add paging. - Intents and claims written before this change have no process identity, so they keep the old deadlines. - No schema change. The new fields live in existing JSON metadata. > 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 --- .../heartbeat-pending-cleanup-sweep.test.ts | 69 +++++++++++- .../sandbox-creation-operations.test.ts | 106 +++++++++++++++++- .../services/controller-process-identity.ts | 91 +++++++++++++++ server/src/services/heartbeat/recovery.ts | 49 +++++++- .../services/sandbox-creation-operations.ts | 55 ++++++++- 5 files changed, 364 insertions(+), 6 deletions(-) create mode 100644 server/src/services/controller-process-identity.ts diff --git a/server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts b/server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts index 7bd1c6fb487..70040517547 100644 --- a/server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts +++ b/server/src/__tests__/heartbeat-pending-cleanup-sweep.test.ts @@ -1,4 +1,5 @@ import { randomUUID } from "node:crypto"; +import { spawn } from "node:child_process"; import { eq, sql } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -38,6 +39,7 @@ vi.mock("../middleware/logger.js", () => ({ import { logger } from "../middleware/logger.ts"; import { heartbeatService, type HeartbeatEnvironmentRuntime } from "../services/heartbeat.ts"; import { hasPendingRunCleanup } from "../services/execution-blocker.js"; +import { currentControllerProcess } from "../services/controller-process-identity.js"; import { environmentStopErrorData, JsonRpcCallError, PLUGIN_RPC_ERROR_CODES, PluginEnvironmentStopError } from "@paperclipai/plugin-sdk"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); @@ -147,8 +149,9 @@ describeEmbeddedPostgres("heartbeat sweepPendingCleanupLeases", () => { environmentId: string | null; updatedAt: Date; metadata?: Record; + id?: string; }): Promise { - const id = randomUUID(); + const id = input.id ?? randomUUID(); await db.insert(environmentLeases).values({ id, companyId: input.companyId, @@ -345,6 +348,70 @@ describeEmbeddedPostgres("heartbeat sweepPendingCleanupLeases", () => { expect(teardown).toHaveBeenCalledTimes(expired ? 1 : 0); }); + describe("a teardown claim whose server stopped mid-attempt", () => { + async function exitedPid(): Promise { + const child = spawn(process.execPath, ["-e", ""], { stdio: "ignore" }); + await new Promise(resolve => child.once("exit", resolve)); + return child.pid!; + } + function claimMetadata(controller: Record, attemptId = "lost-claim") { + return { pendingCleanupAttemptId: attemptId, pendingCleanupInFlight: true, [ATTEMPTS_KEY]: 1, + pendingCleanupLeaseExpiresAtMs: Date.now() + 14 * 60_000, pendingCleanupController: controller }; + } + async function sweepClaimedBy(controller: Record, attemptId = "lost-claim", + before?: (scope: { companyId: string; environmentId: string }) => Promise) { + const scope = await seedCompanyAndEnvironment(); + await before?.(scope); + const leaseId = await insertOrphanEphemeralLease({ ...scope, updatedAt: new Date(Date.now() - 60_000), + metadata: claimMetadata(controller, attemptId) }); + const teardown = vi.fn(async () => {}); + const service = heartbeatService(db, { environmentRuntime: { + retryPendingSandboxTeardown: teardown, + } as unknown as HeartbeatEnvironmentRuntime }); + const result = await service.sweepPendingCleanupLeases(); + return { leaseId, teardown, result }; + } + async function exitedClaimer(attemptId = "lost-claim") { + return { ...await currentControllerProcess(), bootId: randomUUID(), pid: await exitedPid(), attemptId }; + } + + it("retries now instead of waiting for the claim to expire", async () => { + const { teardown, result } = await sweepClaimedBy(await exitedClaimer()); + expect(teardown).toHaveBeenCalledTimes(1); + expect(result.destroyed).toBe(1); + }); + it("keeps the fence of a claim held by this live server", async () => { + const { leaseId, teardown } = await sweepClaimedBy({ ...await currentControllerProcess(), attemptId: "lost-claim" }); + expect(teardown).not.toHaveBeenCalled(); + const [lease] = await db.select().from(environmentLeases).where(eq(environmentLeases.id, leaseId)); + expect(lease!.metadata).toMatchObject({ pendingCleanupInFlight: true, pendingCleanupAttemptId: "lost-claim" }); + }); + it("keeps the fence of a newer claim that kept an earlier attempt's server record", async () => { + // Another claim path replaced the attempt but left the exited server's record. + const { teardown } = await sweepClaimedBy(await exitedClaimer("earlier-attempt"), "newer-live-claim"); + expect(teardown).not.toHaveBeenCalled(); + }); + it("keeps the fence of a claim from another host", async () => { + const { teardown } = await sweepClaimedBy({ ...await exitedClaimer(), hostHash: "f".repeat(32) }); + expect(teardown).not.toHaveBeenCalled(); + }); + it("keeps the fence of a claim without a recorded server identity", async () => { + const { teardown } = await sweepClaimedBy({}); + expect(teardown).not.toHaveBeenCalled(); + }); + it("pages past more fenced claims than one sweep page to find a lost one", async () => { + const foreign = { ...await exitedClaimer(), hostHash: "f".repeat(32) }; + const { teardown, result } = await sweepClaimedBy(await exitedClaimer(), "lost-claim", async scope => { + for (let index = 0; index < SWEEP_PAGE_SIZE + 5; index++) { + await insertOrphanEphemeralLease({ ...scope, updatedAt: new Date(Date.now() - 60_000), + id: `00000000-0000-4000-8000-${String(index).padStart(12, "0")}`, metadata: claimMetadata(foreign) }); + } + }); + expect(teardown).toHaveBeenCalledTimes(1); + expect(result.destroyed).toBe(1); + }); + }); + 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 9ba022747b3..e4792ce6ce0 100644 --- a/server/src/__tests__/sandbox-creation-operations.test.ts +++ b/server/src/__tests__/sandbox-creation-operations.test.ts @@ -1,4 +1,4 @@ -import { execFile } from "node:child_process"; +import { execFile, spawn } from "node:child_process"; import { createServer } from "node:http"; import { promisify } from "node:util"; import { fileURLToPath } from "node:url"; @@ -17,6 +17,7 @@ import { HttpError } from "../errors.js"; import * as secretOwner from "../services/secrets.js"; import { readIdleSleepSafety } from "../services/idle-sleep-safety.js"; import { nextEnvironmentCleanupAt } from "../services/environment-cleanup-state.js"; +import { readProcessStartedAt } from "../services/hot-restart.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; const external = resolveExternalTestDatabaseUrl("PAPERCLIP_HISTORY_TEST_DATABASE_URL"); @@ -334,4 +335,107 @@ describe("durable sandbox creation through the runtime entry point", () => { expect(await nextEnvironmentCleanupAt(db)).toBeNull(); await expect(acquire()).rejects.toThrow("requires reconciliation"); expect(creates).toBe(1); }); + + describe("after the creating server exits mid-acquire", () => { + async function exitedPid(): Promise { + const child = spawn(process.execPath, ["-e", ""], { stdio: "ignore" }); + await new Promise(resolve => child.once("exit", resolve)); + return child.pid!; + } + // The provider allocated the sandbox; the server exited before committing the lease. + async function interruptedCreation(controller: (current: Record) => Promise>) { + loseReply = true; + await expect(acquire()).rejects.toThrow("Lost provider reply"); + const [operation] = await db.select().from(sandboxCreationOperations); + expect(operation).toMatchObject({ state: "pending" }); + expect(operation!.nextAttemptAt.getTime()).toBeGreaterThan(Date.now() + 25 * 60_000); + const current = operation!.metadata.creationController as Record; + await db.update(sandboxCreationOperations).set({ metadata: { ...operation!.metadata, creationController: await controller(current) } }) + .where(eq(sandboxCreationOperations.id, operation!.id)); + return operation!; + } + const inspections = () => vi.mocked(workers.call).mock.calls.filter(value => value[1] === "environmentInspectLease"); + + it("destroys the sandbox on the next sweep instead of waiting for the settle deadline", async () => { + const pid = await exitedPid(); + const operation = await interruptedCreation(async current => ({ ...current, bootId: randomUUID(), pid })); + await runtime().flushDeferredOrphanCleanups(); + const [lease] = await db.select().from(environmentLeases); + expect(lease).toMatchObject({ status: "pending_cleanup", companyId, providerLeaseId: resources.get(operation.id), + failureReason: "interrupted_sandbox_creation" }); + expect(await db.select().from(sandboxCreationOperations)).toEqual([expect.objectContaining({ state: "cleanup_pending", leaseId: lease!.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("pages past more unprovable creations than one page to find an abandoned one", async () => { + const pid = await exitedPid(); + const operation = await interruptedCreation(async current => ({ ...current, bootId: randomUUID(), pid })); + const { id: _id, runId: _runId, ...row } = operation; + // Sorted ahead of the abandoned intent: creators on another host cannot be judged. + await db.insert(sandboxCreationOperations).values(Array.from({ length: 105 }, (_, index) => ({ ...row, + id: `00000000-0000-4000-8000-${String(index).padStart(12, "0")}`, runId: randomUUID(), + metadata: { ...row.metadata, creationController: { ...(row.metadata.creationController as object), bootId: randomUUID(), pid, hostHash: "f".repeat(32) } } }))); + await runtime().flushDeferredOrphanCleanups(); + expect(await db.select().from(environmentLeases)).toEqual([expect.objectContaining({ status: "pending_cleanup", + providerLeaseId: resources.get(operation.id) })]); + }); + 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() })); + await runtime().flushDeferredOrphanCleanups(); + expect(await db.select().from(environmentLeases)).toEqual([expect.objectContaining({ status: "pending_cleanup" })]); + }); + it("leaves a creation owned by this live server untouched until its deadline", async () => { + await interruptedCreation(async current => current); + await runtime().flushDeferredOrphanCleanups(); + expect(inspections()).toHaveLength(0); + expect(await db.select().from(environmentLeases)).toEqual([]); + expect(await db.select().from(sandboxCreationOperations)).toEqual([expect.objectContaining({ state: "pending" })]); + expect(resources.size).toBe(1); expect(destroys).toBe(0); + }); + it("leaves a creation owned by another live server process untouched", async () => { + const pid = process.ppid; + const startedAt = await readProcessStartedAt(pid); + await interruptedCreation(async current => ({ ...current, bootId: randomUUID(), pid, processStartedAt: startedAt })); + await runtime().flushDeferredOrphanCleanups(); + expect(inspections()).toHaveLength(0); + expect(await db.select().from(environmentLeases)).toEqual([]); + expect(resources.size).toBe(1); + }); + it("does not judge a creator on another host", async () => { + const pid = await exitedPid(); + await interruptedCreation(async current => ({ ...current, bootId: randomUUID(), pid, hostHash: "f".repeat(32) })); + await runtime().flushDeferredOrphanCleanups(); + expect(inspections()).toHaveLength(0); + expect(await db.select().from(environmentLeases)).toEqual([]); + expect(resources.size).toBe(1); + }); + it("keeps ownership when an early lookup finds no sandbox yet", async () => { + const pid = await exitedPid(); + const operation = await interruptedCreation(async current => ({ ...current, bootId: randomUUID(), pid })); + const resource = resources.get(operation.id)!; + resources.delete(operation.id); // Not materialized at the provider yet. + await runtime().flushDeferredOrphanCleanups(); + expect(inspections()).toHaveLength(1); + const [pending] = await db.select().from(sandboxCreationOperations); + expect(pending).toMatchObject({ state: "pending" }); + expect(pending!.nextAttemptAt.getTime()).toBeGreaterThan(Date.now()); + // A late allocation is still found and removed by the retained owner. + resources.set(operation.id, resource); + await due(); await runtime().flushDeferredOrphanCleanups(); + expect(await db.select().from(environmentLeases)).toEqual([expect.objectContaining({ status: "pending_cleanup", providerLeaseId: resource })]); + }); + it("never touches a sandbox whose lease was committed", async () => { + const { lease } = await acquire(); + const pid = await exitedPid(); + const [operation] = await db.select().from(sandboxCreationOperations); + expect(operation).toMatchObject({ state: "attached" }); + await db.update(sandboxCreationOperations).set({ metadata: { ...operation!.metadata, + creationController: { ...(operation!.metadata.creationController as object), bootId: randomUUID(), pid } } }); + await runtime().flushDeferredOrphanCleanups(); + expect(inspections()).toHaveLength(0); + expect(await environmentService(db).getLeaseById(lease.id)).toMatchObject({ status: "active" }); + expect(resources.size).toBe(1); expect(destroys).toBe(0); + }); + }); }); diff --git a/server/src/services/controller-process-identity.ts b/server/src/services/controller-process-identity.ts new file mode 100644 index 00000000000..f752827934e --- /dev/null +++ b/server/src/services/controller-process-identity.ts @@ -0,0 +1,91 @@ +import { createHash, randomUUID } from "node:crypto"; +import { readFileSync, readlinkSync } from "node:fs"; +import { hostname, uptime } from "node:os"; +import { readProcessStartedAt } from "./hot-restart.js"; + +/** + * Identity of the server process that started a provider side effect, saved + * with its durable record. After a restart, recovery uses it to prove that the + * starter has exited, so the side effect cannot still be in flight there. + */ +export type ControllerProcess = { bootId: string; pid: number; hostHash: string; processStartedAt: string | null }; + +const bootId = randomUUID(); +const UNKNOWN_PROCESS_SPACE = "unknown"; + +/** + * Names the space in which PIDs are comparable: one kernel boot and, on Linux, + * one PID namespace. Containers that share a hostname but not a PID namespace + * get different values. When the space cannot be read, no record is judged. + * A redeploy onto another host or namespace also never matches; such records + * keep their original deadlines. + */ +function processSpaceHash(): string { + try { + const parts = [hostname()]; + if (process.platform === "linux") { + parts.push(readFileSync("/proc/sys/kernel/random/boot_id", "utf8").trim(), readlinkSync("/proc/self/ns/pid")); + } else { + // No PID namespaces here. The boot time keeps a rebooted host distinct; + // rounding can only make a record unmatched, which fails closed. + parts.push(String(Math.round((Date.now() - uptime() * 1000) / 600_000))); + } + if (parts.some(part => !part)) return UNKNOWN_PROCESS_SPACE; + return createHash("sha256").update(parts.join("\0")).digest("hex").slice(0, 32); + } catch { + return UNKNOWN_PROCESS_SPACE; + } +} + +const hostHash = processSpaceHash(); +let processStartedAt: Promise | undefined; + +async function observedProcessStartedAt(pid: number): Promise { + try { + const startedAt = Date.parse(await readProcessStartedAt(pid) ?? ""); + return Number.isFinite(startedAt) ? startedAt : null; + } catch { + return null; + } +} + +export function currentControllerBootId(): string { + return bootId; +} + +export async function currentControllerProcess(): Promise { + processStartedAt ??= observedProcessStartedAt(process.pid) + .then(value => value === null ? null : new Date(value).toISOString()); + return { bootId, pid: process.pid, hostHash, processStartedAt: await processStartedAt }; +} + +export function readControllerProcess(value: unknown): ControllerProcess | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const record = value as Record; + if (typeof record.bootId !== "string" || typeof record.hostHash !== "string" || + typeof record.pid !== "number" || !Number.isInteger(record.pid) || record.pid <= 0) return null; + return { bootId: record.bootId, pid: record.pid, hostHash: record.hostHash, + processStartedAt: typeof record.processStartedAt === "string" ? record.processStartedAt : null }; +} + +function processExists(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException | undefined)?.code === "EPERM"; + } +} + +/** True only with proof, in this process's own PID space, that the recorded + * server process has exited. Another space, an unreadable space, this server, + * or an ambiguous live PID returns false. */ +export async function controllerProcessExited(controller: ControllerProcess): Promise { + if (controller.bootId === bootId || hostHash === UNKNOWN_PROCESS_SPACE || controller.hostHash !== hostHash) return false; + if (!processExists(controller.pid)) return true; + // The PID now belongs to a live process, possibly this restarted server. + // Only a definitely different start time proves the recorded process is gone. + const recorded = Date.parse(controller.processStartedAt ?? ""); + const observed = await observedProcessStartedAt(controller.pid); + return Number.isFinite(recorded) && observed !== null && Math.abs(observed - recorded) >= 1_000; +} diff --git a/server/src/services/heartbeat/recovery.ts b/server/src/services/heartbeat/recovery.ts index cd00e460eb0..04d40f10e29 100644 --- a/server/src/services/heartbeat/recovery.ts +++ b/server/src/services/heartbeat/recovery.ts @@ -11,6 +11,7 @@ import { retainedNativeSessionCleanupWork } from "../native-runtime/retained-nat import { reconcileRetainedNativeSessionCleanups } from "../native-runtime/retained-native-session-cleanup.js"; import { createScheduledRetryCancellationRecovery } from "./scheduled-retry-cancellation.js"; import { pendingCleanupDeadlineSql } from "../environment-cleanup-state.js"; +import { controllerProcessExited, currentControllerBootId, currentControllerProcess, readControllerProcess } from "../controller-process-identity.js"; import { writeEnvironmentCleanup } from "../environment-cleanup-notifications.js"; import { startNativeSessionExecutionLeaseRenewal, type NativeExecutionLeaseRenewal } from "../native-runtime/native-execution-lease.js"; import type { WorkspaceWorkers } from "../plugin-workspaces.js"; @@ -1417,6 +1418,49 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD return Math.min(Math.floor(value), PENDING_CLEANUP_SWEEP_ATTEMPT_CAP); } + // A teardown claim fences other sweeps for 15 minutes. After a restart, a + // claim whose server process has exited cannot still be running, so its + // fence ends now. The saved process is bound to its own attempt: a later + // claim from another path that kept the old record is never released. A live + // or unidentified claimer keeps its fence. Pages pass every skipped claim. + async function releaseLostPendingCleanupClaims(): Promise { + const nowMs = Date.now(); + let after: string | null = null; + for (;;) { + const claimed: Array<{ id: string; metadata: Record | null }> = await db + .select({ id: environmentLeases.id, metadata: environmentLeases.metadata }) + .from(environmentLeases) + .where(and( + eq(environmentLeases.status, "pending_cleanup"), + sql`${environmentLeases.metadata}->>'pendingCleanupInFlight' = 'true'`, + sql`jsonb_typeof(${environmentLeases.metadata}->'pendingCleanupLeaseExpiresAtMs') = 'number'`, + sql`(${environmentLeases.metadata}->>'pendingCleanupLeaseExpiresAtMs')::numeric > ${nowMs}`, + sql`${environmentLeases.metadata}->'pendingCleanupController'->>'attemptId' = ${environmentLeases.metadata}->>'pendingCleanupAttemptId'`, + sql`${environmentLeases.metadata}->'pendingCleanupController'->>'bootId' is distinct from ${currentControllerBootId()}`, + after === null ? undefined : gt(environmentLeases.id, after), + )) + .orderBy(asc(environmentLeases.id)) + .limit(PENDING_CLEANUP_SWEEP_PAGE_SIZE); + for (const row of claimed) { + const metadata = row.metadata ?? {}; + const saved = metadata.pendingCleanupController as Record | undefined; + const controller = readControllerProcess(saved); + const attemptId = saved?.attemptId; + if (!controller || typeof attemptId !== "string" || !await controllerProcessExited(controller)) continue; + await db.update(environmentLeases) + .set({ metadata: sql`jsonb_set(${pendingCleanupMetadataObjectSql()}, '{pendingCleanupLeaseExpiresAtMs}', to_jsonb(${nowMs}::bigint))` }) + .where(and( + eq(environmentLeases.id, row.id), + eq(environmentLeases.status, "pending_cleanup"), + sql`${environmentLeases.metadata}->>'pendingCleanupAttemptId' = ${attemptId}`, + sql`${environmentLeases.metadata}->'pendingCleanupController'->>'attemptId' = ${attemptId}`, + )); + } + if (claimed.length < PENDING_CLEANUP_SWEEP_PAGE_SIZE) return; + after = claimed[claimed.length - 1]!.id; + } + } + // Atomically claim one retry attempt on a pending_cleanup lease. The update // only matches when the lease is still pending_cleanup and its stored attempt // count still equals `expectedAttempts`. Two concurrent sweeps read the same @@ -1436,13 +1480,14 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD ): Promise { const now = new Date(); const attemptId = randomUUID(); + const controller = await currentControllerProcess(); const claimed = await db .update(environmentLeases) .set({ metadata: sql`jsonb_set(${pendingCleanupMetadataObjectSql()}, array[${PENDING_CLEANUP_ATTEMPTS_METADATA_KEY}], to_jsonb(${expectedAttempts + 1}::int), true) || ${JSON.stringify({ ...(manualAttempt ? { pendingCleanupManualAttemptId: randomUUID() } : {}), pendingCleanupAttemptId: attemptId, pendingCleanupInFlight: true, - pendingCleanupLeaseExpiresAtMs: Date.now() + 15 * 60_000, + pendingCleanupLeaseExpiresAtMs: Date.now() + 15 * 60_000, pendingCleanupController: { ...controller, attemptId }, })}::jsonb`, lastUsedAt: now, updatedAt: now, @@ -1691,6 +1736,8 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD deferredFlushFailed = true; } + await releaseLostPendingCleanupClaims(); + const rows = await db .select() .from(environmentLeases) diff --git a/server/src/services/sandbox-creation-operations.ts b/server/src/services/sandbox-creation-operations.ts index d17300c7d4e..0b5b7bee8c7 100644 --- a/server/src/services/sandbox-creation-operations.ts +++ b/server/src/services/sandbox-creation-operations.ts @@ -1,6 +1,7 @@ import { collectSecretRefPaths, isUuidSecretRef, parseSecretRefBindingObject, readConfigValueAtPath } from "./json-schema-secret-refs.js"; import { createHash, randomUUID } from "node:crypto"; -import { and, eq, lte, or, sql } from "drizzle-orm"; +import { and, asc, eq, gt, lte, or, sql } from "drizzle-orm"; +import { controllerProcessExited, currentControllerBootId, currentControllerProcess, readControllerProcess } from "./controller-process-identity.js"; import { companies, environments, sandboxCreationOperations, type Db } from "@paperclipai/db"; import { readEnvironmentCreationCleanupError, type PluginEnvironmentLease, type PluginEnvironmentLeaseObservation } from "@paperclipai/plugin-sdk"; import { assertEnvironmentCompanyBinding, environmentService } from "./environments.js"; @@ -10,6 +11,46 @@ import { notifyEnvironmentCleanup, writeEnvironmentCleanup } from "./environment export type SandboxCreationOperation = typeof sandboxCreationOperations.$inferSelect; +// A live creator may still be waiting for the provider, and a provider may still +// materialize an aborted request. Absence is conclusive only after this delay. +const CREATION_SETTLE_FLOOR_MS = 30 * 60_000; +const CREATION_PROMOTION_PAGE_SIZE = 100; + +/** After a restart, creation intents from an exited server cannot still be in + * flight there. Reconcile them now instead of at the settle deadline. Absence + * stays inconclusive until that deadline; see recoverSandboxCreations. */ +async function promoteAbandonedSandboxCreations(db: Db): Promise { + const now = new Date(); + // Page past live, foreign, and unidentified creators so they cannot hide an + // abandoned intent behind them. + let after: string | null = null; + for (;;) { + const waiting: Array> = await db + .select({ id: sandboxCreationOperations.id, metadata: sandboxCreationOperations.metadata }) + .from(sandboxCreationOperations).where(and( + eq(sandboxCreationOperations.state, "pending"), gt(sandboxCreationOperations.nextAttemptAt, now), + sql`${sandboxCreationOperations.metadata}->'creationController'->>'bootId' is distinct from ${currentControllerBootId()}`, + sql`coalesce(${sandboxCreationOperations.metadata}->>'creationControllerLost', '') <> 'true'`, + after === null ? undefined : gt(sandboxCreationOperations.id, after), + )).orderBy(asc(sandboxCreationOperations.id)).limit(CREATION_PROMOTION_PAGE_SIZE); + for (const operation of waiting) { + const controller = readControllerProcess(operation.metadata.creationController); + if (!controller || !await controllerProcessExited(controller)) continue; + await db.update(sandboxCreationOperations).set({ nextAttemptAt: now, updatedAt: now, + metadata: sql`coalesce(${sandboxCreationOperations.metadata}, '{}'::jsonb) || '{"creationControllerLost":true}'::jsonb` }) + .where(and(eq(sandboxCreationOperations.id, operation.id), eq(sandboxCreationOperations.state, "pending"), + gt(sandboxCreationOperations.nextAttemptAt, now))); + } + if (waiting.length < CREATION_PROMOTION_PAGE_SIZE) return; + after = waiting[waiting.length - 1]!.id; + } +} + +function creationAbsenceInconclusive(operation: SandboxCreationOperation): boolean { + const settleAt = Date.parse(typeof operation.metadata.creationSettleAt === "string" ? operation.metadata.creationSettleAt : ""); + return operation.metadata.creationControllerLost === true && Number.isFinite(settleAt) && Date.now() < settleAt; +} + export function recoverableTaskCreation(operation: Pick): boolean { const metadata = operation.metadata; return metadata.runnerTaskLease === true && metadata.creationOperationVersion === 1 && @@ -52,6 +93,8 @@ export async function createOwnedSandbox( } const id = randomUUID(); const runId = owner.heartbeatRunId ?? randomUUID(); + const settleAt = new Date(Date.now() + Math.max(CREATION_SETTLE_FLOOR_MS, (owner.timeoutMs ?? 0) + 60_000)); + const creationController = await currentControllerProcess(); const operation = await db.transaction(async tx => { await notifyEnvironmentCleanup(tx, { companyId: owner.companyId, runId: owner.heartbeatRunId ?? undefined }); await assertEnvironmentCompanyBinding(tx as unknown as Db, owner.environmentId, owner.companyId); @@ -59,8 +102,9 @@ export async function createOwnedSandbox( id, companyId: owner.companyId, environmentId: owner.environmentId, originalEnvironmentId: owner.environmentId, runId, requestKey: owner.requestKey ?? "run", issueId: owner.issueId, executionWorkspaceId: owner.executionWorkspaceId, provider: owner.provider, pluginId: owner.pluginId, driverKey: owner.driverKey, - config: owner.config, metadata: { ...owner.metadata, creationCredentialHashes: credentialHashes }, - nextAttemptAt: new Date(Date.now() + Math.max(30 * 60_000, (owner.timeoutMs ?? 0) + 60_000)), + config: owner.config, metadata: { ...owner.metadata, creationCredentialHashes: credentialHashes, + creationController, creationSettleAt: settleAt.toISOString() }, + nextAttemptAt: settleAt, }).onConflictDoNothing().returning(); if (inserted) return inserted; const [existing] = await tx.select().from(sandboxCreationOperations).where(and( @@ -152,6 +196,7 @@ export async function recoverSandboxCreations(db: Db, inspect: (operation: SandboxCreationOperation) => Promise, onTaskRecovered?: (operation: SandboxCreationOperation) => void | Promise, ): Promise { + await promoteAbandonedSandboxCreations(db); const due = await db.select().from(sandboxCreationOperations).where(and( recoverableSandboxCreationCondition(), lte(sandboxCreationOperations.nextAttemptAt, new Date()), )).limit(100); @@ -187,6 +232,10 @@ export async function recoverSandboxCreations(db: Db, cleanup.runId !== candidate.runId || cleanup.attemptId !== candidate.id || cleanup.providerLeaseId !== observation!.providerLeaseId)) { throw new Error("Sandbox creation recovery returned foreign ownership"); } + // An early lookup after a lost creator may precede provider materialization. + if (existingCompany && !cleanup && creationAbsenceInconclusive(candidate)) { + throw new Error("Sandbox creation has not settled yet"); + } await db.transaction(async tx => { const [company] = await tx.select({ id: companies.id }).from(companies).where(eq(companies.id, candidate.companyId)).for("key share"); await tx.select({ id: environments.id }).from(environments).where(eq(environments.id, candidate.originalEnvironmentId)).for("update");