diff --git a/src/infra/stale-lock-file.test.ts b/src/infra/stale-lock-file.test.ts index b792895fe7cb..8b7e3d7ac31e 100644 --- a/src/infra/stale-lock-file.test.ts +++ b/src/infra/stale-lock-file.test.ts @@ -2,19 +2,52 @@ import { describe, expect, it } from "vitest"; import { shouldRemoveDeadOwnerOrExpiredLock } from "./stale-lock-file.js"; describe("stale lock file ownership", () => { - it("treats permission-denied process probes as not definitely dead", () => { + it("keeps expired locks when a pid owner is not definitely dead", () => { expect( shouldRemoveDeadOwnerOrExpiredLock({ payload: { pid: 123, - createdAt: new Date(Date.now() - 60_000).toISOString(), + createdAt: "2026-05-23T00:00:00.000Z", }, staleMs: 10, + nowMs: Date.parse("2026-05-23T00:00:11.000Z"), isPidDefinitelyDead: () => false, }), ).toBe(false); }); + it("removes locks when the owner pid starttime changed", () => { + expect( + shouldRemoveDeadOwnerOrExpiredLock({ + payload: { + pid: 123, + createdAt: "2026-05-23T00:00:00.000Z", + starttime: 111, + }, + staleMs: 60_000, + nowMs: Date.parse("2026-05-23T00:00:10.000Z"), + isPidDefinitelyDead: () => false, + getProcessStartTime: () => 222, + }), + ).toBe(true); + }); + + it("does not remove locks when the owner pid starttime still matches", () => { + expect( + shouldRemoveDeadOwnerOrExpiredLock({ + payload: { + pid: 123, + createdAt: "2026-05-23T00:00:00.000Z", + starttime: 111, + }, + staleMs: 10, + nowMs: Date.parse("2026-05-23T00:00:11.000Z"), + isPidDefinitelyDead: () => false, + getProcessStartTime: () => 111, + }), + ).toBe(false); + }); + it("only removes pid-owned locks when the owner is definitely dead", () => { expect( shouldRemoveDeadOwnerOrExpiredLock({ @@ -27,4 +60,16 @@ describe("stale lock file ownership", () => { }), ).toBe(true); }); + + it("removes expired pidless locks", () => { + expect( + shouldRemoveDeadOwnerOrExpiredLock({ + payload: { + createdAt: "2026-05-23T00:00:00.000Z", + }, + staleMs: 10, + nowMs: Date.parse("2026-05-23T00:00:11.000Z"), + }), + ).toBe(true); + }); }); diff --git a/src/infra/stale-lock-file.ts b/src/infra/stale-lock-file.ts index 79ea94cc3e19..222a6d9a310a 100644 --- a/src/infra/stale-lock-file.ts +++ b/src/infra/stale-lock-file.ts @@ -1,8 +1,12 @@ -import { isPidDefinitelyDead as defaultIsPidDefinitelyDead } from "../shared/pid-alive.js"; +import { + getProcessStartTime as defaultGetProcessStartTime, + isPidDefinitelyDead as defaultIsPidDefinitelyDead, +} from "../shared/pid-alive.js"; export type LockFileOwnerPayload = { pid?: number; createdAt?: string; + starttime?: number; }; export function readLockFileOwnerPayload( @@ -14,6 +18,7 @@ export function readLockFileOwnerPayload( return { pid: typeof payload.pid === "number" ? payload.pid : undefined, createdAt: typeof payload.createdAt === "string" ? payload.createdAt : undefined, + starttime: typeof payload.starttime === "number" ? payload.starttime : undefined, }; } @@ -22,14 +27,27 @@ export function shouldRemoveDeadOwnerOrExpiredLock(params: { staleMs: number; nowMs?: number; isPidDefinitelyDead?: (pid: number) => boolean; + getProcessStartTime?: (pid: number) => number | null; }): boolean { const payload = readLockFileOwnerPayload(params.payload); if (payload?.pid) { + // Timestamp age alone cannot prove the owner stopped writing. Only a + // mismatched process start time proves PID reuse while the PID is alive. + if (payload.starttime !== undefined) { + const currentStarttime = (params.getProcessStartTime ?? defaultGetProcessStartTime)( + payload.pid, + ); + if (currentStarttime !== null && currentStarttime !== payload.starttime) { + return true; + } + } return (params.isPidDefinitelyDead ?? defaultIsPidDefinitelyDead)(payload.pid); } if (payload?.createdAt) { const createdAt = Date.parse(payload.createdAt); - return !Number.isFinite(createdAt) || (params.nowMs ?? Date.now()) - createdAt > params.staleMs; + if (!Number.isFinite(createdAt) || (params.nowMs ?? Date.now()) - createdAt > params.staleMs) { + return true; + } } return false; } diff --git a/src/plugin-sdk/file-lock.test.ts b/src/plugin-sdk/file-lock.test.ts index 921cbbd9a11f..ffc8728ac715 100644 --- a/src/plugin-sdk/file-lock.test.ts +++ b/src/plugin-sdk/file-lock.test.ts @@ -117,7 +117,7 @@ describe("acquireFileLock", () => { await expect(fs.readFile(lockPath, "utf8")).resolves.toBe("{"); }); - it("keeps a reported stale lock when its owner pid is alive", async () => { + it("keeps an expired lock when its live owner has no starttime proof", async () => { const filePath = path.join(tempDir, "live-owner"); const lockPath = `${filePath}.lock`; const options = { diff --git a/src/plugin-sdk/file-lock.ts b/src/plugin-sdk/file-lock.ts index 06b5208dd623..0ab2f2d9f87a 100644 --- a/src/plugin-sdk/file-lock.ts +++ b/src/plugin-sdk/file-lock.ts @@ -5,6 +5,7 @@ import { resetFileLockManagerForTest, } from "@openclaw/fs-safe/file-lock"; import { shouldRemoveDeadOwnerOrExpiredLock } from "../infra/stale-lock-file.js"; +import { getProcessStartTime } from "../shared/pid-alive.js"; export type FileLockOptions = { retries: { @@ -86,7 +87,17 @@ export async function acquireFileLock( retry: options.retries, staleRecovery: "remove-if-unchanged", allowReentrant: true, - payload: () => ({ pid: process.pid, createdAt: new Date().toISOString() }), + payload: () => { + const payload: Record = { + pid: process.pid, + createdAt: new Date().toISOString(), + }; + const starttime = getProcessStartTime(process.pid); + if (starttime !== null) { + payload.starttime = starttime; + } + return payload; + }, shouldReclaim: shouldReclaimPluginLock, shouldRemoveStaleLock: (snapshot) => shouldRemoveDeadOwnerOrExpiredLock({