mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
fix: fence finished run process identity against late native callbacks
This commit is contained in:
1 parent
a7108016b3
commit
3f7cbb4f97
3 files changed
+86
-6
No files matched your search
@@ -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.
|
||||
@@ -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<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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")));
|
||||
},
|
||||
}),
|
||||
)));
|
||||
|
||||
Reference in new issue
Block a user