diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 7e20f54294d..627b348d63b 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -517,3 +517,9 @@ requires an explicit controller rotation event for that exact transition before accepting a changed PID/process fingerprint; other process changes still fail. See [GitHub execution identity](execution-github-identity.md) and the [run-log contract](run-log-events.md#native-process-rotation). + +Process metadata updates apply only while their run is active and unfinished. +Late callbacks from warm-session inspection or maintenance cannot rewrite a +completed run's process identity. Remote native process timestamps come from +the validated remote marker; an unrelated host process with the same PID must +not replace them. Active local execution retains its host process lookup. diff --git a/server/src/__tests__/heartbeat-process-metadata.test.ts b/server/src/__tests__/heartbeat-process-metadata.test.ts new file mode 100644 index 00000000000..141e74e4db6 --- /dev/null +++ b/server/src/__tests__/heartbeat-process-metadata.test.ts @@ -0,0 +1,63 @@ +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { agents, companies, createDb, heartbeatRuns, startEmbeddedPostgresTestDatabase, type Db } from "@paperclipai/db"; +import * as processes from "../services/hot-restart.js"; +import { persistHeartbeatRunProcessMetadata } from "../services/heartbeat.js"; + +describe("heartbeat process identity persistence", () => { + let database: Awaited>; + let db: Db; + const companyId = randomUUID(), agentId = randomUUID(); + beforeAll(async () => { + database = await startEmbeddedPostgresTestDatabase("paperclip-process-metadata-"); + db = createDb(database.connectionString); + await db.insert(companies).values({ id: companyId, name: "Process identity" }); + await db.insert(agents).values({ id: agentId, companyId, name: "Runner", role: "engineer", status: "idle", adapterType: "codex_local" }); + }, 60_000); + afterEach(() => vi.restoreAllMocks()); + afterAll(async () => { await database?.cleanup(); }); + async function running() { + const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status: "running", invocationSource: "on_demand", startedAt: new Date(), + processPid: 123, processGroupId: null, processStartedAt: new Date("2026-09-09T23:19:42Z") }).returning(); + return run!; + } + + it("uses the remote marker even when its PID exists on the host", async () => { + const run = await running(); + const host = vi.spyOn(processes, "readProcessStartedAt"); + const meta = { pid: process.pid, processGroupId: null, startedAt: "2026-09-09T23:19:43.123Z" }; + const updated = await persistHeartbeatRunProcessMetadata(db, run.id, meta, "remote"); + expect(updated?.processPid).toBe(process.pid); + expect(updated?.processStartedAt?.toISOString()).toBe(meta.startedAt); + expect(host).not.toHaveBeenCalled(); + }); + + it("retains observed local-process identity during an active run", async () => { + const run = await running(); + const start = "2026-09-09T23:19:43.456Z"; + vi.spyOn(processes, "readProcessStartedAt").mockResolvedValue(start); + const updated = await persistHeartbeatRunProcessMetadata(db, run.id, { pid: 456, processGroupId: 456, startedAt: "2026-09-09T23:19:44Z" }); + expect(updated).toMatchObject({ processPid: 456, processGroupId: 456, processStartedAt: new Date(start) }); + }); + + it.each(["succeeded", "failed", "cancelled", "timed_out", "running"] as const)("does not change finished %s history from a late spawn callback", async (status) => { + const run = await running(); + const [finished] = await db.update(heartbeatRuns).set({ status, finishedAt: new Date() }).where(eq(heartbeatRuns.id, run.id)).returning(); + expect(await persistHeartbeatRunProcessMetadata(db, run.id, { pid: 6365, processGroupId: null, startedAt: "2026-09-09T23:20:09.344Z" }, "remote")).toBeNull(); + const [actual] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id)); + expect(actual).toEqual(finished); + }); + + it("fences finalization racing an in-flight local identity lookup", async () => { + const run = await running(); + let resume!: (start: string) => void; + vi.spyOn(processes, "readProcessStartedAt").mockImplementation(() => new Promise((resolve) => { resume = resolve; })); + const pending = persistHeartbeatRunProcessMetadata(db, run.id, { pid: 456, processGroupId: 456, startedAt: "2026-09-09T23:19:43Z" }); + const [finished] = await db.update(heartbeatRuns).set({ status: "succeeded", finishedAt: new Date() }).where(eq(heartbeatRuns.id, run.id)).returning(); + resume("2026-09-09T23:19:43Z"); + expect(await pending).toBeNull(); + const [actual] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id)); + expect(actual).toEqual(finished); + }); +}); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 16a6049bc63..59524f275e6 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -7813,10 +7813,13 @@ export async function persistHeartbeatRunProcessMetadata( db: Db, runId: string, meta: { pid: number; processGroupId: number | null; startedAt: string }, + processLocation: "local" | "remote" = "local", ) { - const observedStartedAt = await readProcessStartedAt(meta.pid).catch( - () => null, - ); + // A remote PID can collide with an unrelated control-plane process. Its + // validated remote marker, not the host's process table, owns this identity. + const observedStartedAt = processLocation === "local" + ? await readProcessStartedAt(meta.pid).catch(() => null) + : null; const startedAt = new Date(observedStartedAt ?? meta.startedAt); return db .update(heartbeatRuns) @@ -7828,7 +7831,14 @@ export async function persistHeartbeatRunProcessMetadata( : startedAt, updatedAt: new Date(), }) - .where(eq(heartbeatRuns.id, runId)) + // Warm-session inspection and idle maintenance can invoke a captured spawn + // callback after this run ended. Keep completed process history immutable, + // including when finalization races the asynchronous identity lookup above. + .where(and( + eq(heartbeatRuns.id, runId), + eq(heartbeatRuns.status, "running"), + isNull(heartbeatRuns.finishedAt), + )) .returning() .then((rows) => rows[0] ?? null); } @@ -11923,8 +11933,9 @@ export function heartbeatService( async function persistRunProcessMetadata( runId: string, meta: { pid: number; processGroupId: number | null; startedAt: string }, + processLocation: "local" | "remote" = "local", ) { - return persistHeartbeatRunProcessMetadata(db, runId, meta); + return persistHeartbeatRunProcessMetadata(db, runId, meta, processLocation); } async function clearDetachedRunWarning(runId: string) { @@ -21560,7 +21571,7 @@ export function heartbeatService( enqueueWakeup, onSpawn: async (meta) => { markDispatchStarted(); - await measureSandboxOperation("heartbeat.persist_run_process_metadata", { operationIndex: 141 }, async () => (persistRunProcessMetadata(run.id, meta))); + await measureSandboxOperation("heartbeat.persist_run_process_metadata", { operationIndex: 141 }, async () => (persistRunProcessMetadata(run.id, meta, executionTarget?.kind === "remote" ? "remote" : "local"))); }, }), )));