fix(config): bind cross-process file locks to their owning host

packages/coding-agent/CHANGELOG.md

@@ -8,6 +8,7 @@
 - Fixture quality gates that complete intermediate Ultragoal stories now write file-backed adversarial artifact proof; skill-state hooks and computer red-team fixtures match the unconditional adversarial path check so #3543 CI stays fail-closed without weakening hydration exactness (#3543).
 - Runtime settings reconciliation now validates every `web_search.fallback` entry against the declared provider enum instead of accepting unsupported or non-string array items (#3601).
 - Ultragoal critic-gate, dogfood, review, durable-completion, and runtime test suites now pin `CI_DEV_CHANGED_PATHS` hermetically in their setup/teardown. Their temp checkpoints live inside the enclosing git work tree, so the CI planner's changed paths (which include computer control surface paths on branches that touch them) previously leaked into the computed change set and falsely triggered the mandatory computer red-team suite (`COMPUTER_REDTEAM_CASE_MISSING: … must include kill-switch-bypass`). The production kill-switch-bypass gate is unchanged; only the test fixtures now isolate their own contract from the host branch's diff (#3533).
+- Cross-process file locks now record the host that owns them, so a state directory on a shared volume (NFS home directories are explicitly supported) no longer lets one host reap another host's freshly-taken lock. Owner liveness is probed against the *local* process table, and a remote owner's pid is almost never a live local pid, so a foreign owner previously read as `dead` and was reclaimed immediately with no stale grace period — two hosts could then hold the same lock and run the same critical section concurrently. A foreign-host owner is now reclaimable only after the `staleMs` elapsed-time heuristic, and host identity is part of owner equality for the guarded lock-dir removal so one host cannot delete another's lock on a pid/timestamp coincidence. Records written before this change carry no host id and keep the previous local-pid semantics.
 
 ### Fixed
 

packages/coding-agent/src/config/file-lock.ts

@@ -1,5 +1,6 @@
 import type { Stats } from "node:fs";
 import * as fs from "node:fs/promises";
+import * as os from "node:os";
 import * as path from "node:path";
 import { hasFsCode, isEnoent } from "@gajae-code/utils/fs-error";
 
@@ -40,6 +41,29 @@ function currentProcessStartTime(): string {
 	return ownProcessStartTime;
 }
 
+let ownHostId: string | undefined;
+
+/**
+ * Stable identity of the host that owns a lock record.
+ *
+ * Liveness below is probed against the *local* process table, which is only
+ * meaningful for a lock written by this host. A state directory on a shared
+ * volume (NFS home directories are explicitly supported) is contended by
+ * several hosts, and a remote owner's pid is almost never a live local pid — so
+ * without this field a foreign host's freshly-taken lock reads as `dead` and is
+ * reclaimed instantly, with no stale grace period at all.
+ */
+function currentHostId(): string {
+	if (ownHostId === undefined) {
+		try {
+			ownHostId = os.hostname() || "unknown";
+		} catch {
+			ownHostId = "unknown";
+		}
+	}
+	return ownHostId;
+}
+
 function cachedProcessStartTime(owner: FileLockOwnerToken, cache?: Map<string, string | null>): string | null {
 	if (!cache) return processStartTime(owner.pid);
 	const key = `${owner.pid}:${owner.start_time ?? ""}`;
@@ -50,15 +74,28 @@ function cachedProcessStartTime(owner: FileLockOwnerToken, cache?: Map<string, s
 	return startTime;
 }
 
+function ownerIsForeignHost(owner: FileLockOwnerToken): boolean {
+	return owner.host_id !== undefined && owner.host_id !== currentHostId();
+}
+
 function ownerIsAlive(owner: FileLockOwnerToken, startTimeCache?: Map<string, string | null>): boolean {
+	// A remote owner's pid indexes a different process table; never claim to know
+	// it is alive, and never let the caller conclude it is dead either (see
+	// `staleLockSnapshot`, which routes foreign owners to elapsed time only).
+	if (ownerIsForeignHost(owner)) return false;
 	if (ownerLiveness(owner.pid) !== "alive") return false;
 	if (!owner.start_time) return true;
 	const currentStartTime = cachedProcessStartTime(owner, startTimeCache);
 	return currentStartTime === null || currentStartTime === owner.start_time;
 }
 
 function writeLockInfo(lockPath: string): Promise<LockInfo> {
-	const info: LockInfo = { pid: process.pid, start_time: currentProcessStartTime(), timestamp: Date.now() };
+	const info: LockInfo = {
+		pid: process.pid,
+		start_time: currentProcessStartTime(),
+		host_id: currentHostId(),
+		timestamp: Date.now(),
+	};
 	return Bun.write(`${lockPath}/info`, JSON.stringify(info)).then(() => info);
 }
 
@@ -72,17 +109,18 @@ async function readLockInfo(lockPath: string): Promise<LockInfo | null> {
 	}
 
 	if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return null;
-	const { pid, start_time, timestamp } = parsed as Partial<LockInfo>;
+	const { pid, start_time, host_id, timestamp } = parsed as Partial<LockInfo>;
 	if (
 		typeof pid !== "number" ||
 		!Number.isInteger(pid) ||
 		pid <= 0 ||
 		typeof timestamp !== "number" ||
 		!Number.isFinite(timestamp) ||
-		(start_time !== undefined && (typeof start_time !== "string" || !start_time))
+		(start_time !== undefined && (typeof start_time !== "string" || !start_time)) ||
+		(host_id !== undefined && (typeof host_id !== "string" || !host_id))
 	)
 		return null;
-	return { pid, start_time, timestamp };
+	return { pid, start_time, host_id, timestamp };
 }
 
 /** @internal */
@@ -94,6 +132,11 @@ export async function readFileLockInfoForGc(lockDir: string): Promise<FileLockOw
 export interface FileLockOwnerToken {
 	pid: number;
 	start_time?: string;
+	/**
+	 * Host that wrote the record. Absent on pre-host-id locks, which keep the
+	 * previous local-pid semantics for backward compatibility.
+	 */
+	host_id?: string;
 
 	timestamp: number;
 }
@@ -143,6 +186,10 @@ export async function removeFileLockDirForGc(
 	if (
 		current.pid !== expected.pid ||
 		(expected.start_time !== undefined && current.start_time !== expected.start_time) ||
+		// Two hosts sharing a volume can coincide on pid and even timestamp, so
+		// host identity is part of owner equality. An absent expectation still
+		// matches an absent record, preserving pre-host-id behaviour.
+		current.host_id !== expected.host_id ||
 		current.timestamp !== expected.timestamp
 	) {
 		return "owner_changed";
@@ -210,6 +257,13 @@ async function staleLockSnapshot(
 	// not have its lock stolen (#652). Reclaim a dead owner immediately. Only when owner
 	// liveness is indeterminate do we fall back to the staleMs elapsed-time heuristic.
 	if (ownerIsAlive(info, startTimeCache)) return { stale: false };
+	// A foreign host's pid says nothing about that host's process table, so the
+	// `dead` fast path must not apply to it. Such an owner is reclaimable only
+	// after the elapsed-time heuristic, which is the same grace period a local
+	// owner of indeterminate liveness receives.
+	if (ownerIsForeignHost(info)) {
+		return Date.now() - info.timestamp > staleMs ? { stale: true, owner: info } : { stale: false };
+	}
 	if (ownerLiveness(info.pid) === "dead" || Date.now() - info.timestamp > staleMs) {
 		return { stale: true, owner: info };
 	}

packages/coding-agent/test/file-lock-cross-host.test.ts

@@ -0,0 +1,161 @@
+import { describe, expect, test } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { readFileLockInfoForGc, removeFileLockDirForGc, withFileLock } from "../src/config/file-lock";
+
+/**
+ * A lock owner's liveness is probed against the *local* process table. That is
+ * only meaningful for a lock this host wrote. When the state directory lives on
+ * a shared volume (NFS home directories are supported), a remote owner's pid is
+ * almost never a live local pid, so without host identity a foreign host's
+ * freshly-taken lock is judged dead and reclaimed instantly — with none of the
+ * stale grace period a local owner of unknown liveness receives.
+ */
+describe("file lock cross-host owner identity", () => {
+	const FOREIGN_HOST = "gjc-foreign-host-fixture";
+
+	async function stageForeignLock(
+		dir: string,
+		file: string,
+		overrides: Record<string, unknown> = {},
+	): Promise<string> {
+		const lockDir = `${path.join(dir, file)}.lock`;
+		await fs.mkdir(lockDir, { recursive: true });
+		await Bun.write(
+			path.join(lockDir, "info"),
+			JSON.stringify({
+				// A pid that is not alive on this host, which is the normal case for a
+				// process owned by a different machine.
+				pid: 999_999,
+				start_time: "Mon Jan  1 00:00:00 2029",
+				host_id: FOREIGN_HOST,
+				timestamp: Date.now(),
+				...overrides,
+			}),
+		);
+		return lockDir;
+	}
+
+	test("refuses to steal another host's freshly-taken lock", async () => {
+		const dir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-lock-xhost-"));
+		try {
+			await stageForeignLock(dir, "state.json");
+			let ran = false;
+			await expect(
+				withFileLock(
+					path.join(dir, "state.json"),
+					async () => {
+						ran = true;
+					},
+					{ retries: 2, retryDelayMs: 5, staleMs: 60_000 },
+				),
+			).rejects.toThrow(/Failed to acquire lock/);
+			expect(ran).toBe(false);
+			// The foreign owner's record must survive untouched.
+			const info = await readFileLockInfoForGc(`${path.join(dir, "state.json")}.lock`);
+			expect(info?.host_id).toBe(FOREIGN_HOST);
+		} finally {
+			await fs.rm(dir, { recursive: true, force: true });
+		}
+	});
+
+	test("still reclaims another host's lock once it exceeds the stale window", async () => {
+		const dir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-lock-xhost-"));
+		try {
+			await stageForeignLock(dir, "state.json", { timestamp: Date.now() - 120_000 });
+			let ran = false;
+			await withFileLock(
+				path.join(dir, "state.json"),
+				async () => {
+					ran = true;
+				},
+				{ retries: 3, retryDelayMs: 5, staleMs: 10_000 },
+			);
+			expect(ran).toBe(true);
+		} finally {
+			await fs.rm(dir, { recursive: true, force: true });
+		}
+	});
+
+	test("a dead local owner is still reclaimed immediately", async () => {
+		const dir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-lock-xhost-"));
+		try {
+			// No host_id at all: a pre-host-id record keeps the previous local-pid
+			// semantics, so a dead pid is reclaimed without waiting out staleMs.
+			const lockDir = `${path.join(dir, "state.json")}.lock`;
+			await fs.mkdir(lockDir, { recursive: true });
+			await Bun.write(path.join(lockDir, "info"), JSON.stringify({ pid: 999_999, timestamp: Date.now() }));
+			let ran = false;
+			await withFileLock(
+				path.join(dir, "state.json"),
+				async () => {
+					ran = true;
+				},
+				{ retries: 3, retryDelayMs: 5, staleMs: 60_000 },
+			);
+			expect(ran).toBe(true);
+		} finally {
+			await fs.rm(dir, { recursive: true, force: true });
+		}
+	});
+
+	test("a live local owner is never reclaimed by elapsed time", async () => {
+		const dir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-lock-xhost-"));
+		try {
+			const lockDir = `${path.join(dir, "state.json")}.lock`;
+			await fs.mkdir(lockDir, { recursive: true });
+			await Bun.write(
+				path.join(lockDir, "info"),
+				JSON.stringify({ pid: process.pid, timestamp: Date.now() - 600_000 }),
+			);
+			await expect(
+				withFileLock(path.join(dir, "state.json"), async () => undefined, {
+					retries: 2,
+					retryDelayMs: 5,
+					staleMs: 1,
+				}),
+			).rejects.toThrow(/Failed to acquire lock/);
+		} finally {
+			await fs.rm(dir, { recursive: true, force: true });
+		}
+	});
+
+	test("guarded removal refuses a token whose host differs", async () => {
+		const dir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-lock-xhost-"));
+		try {
+			const lockDir = await stageForeignLock(dir, "state.json");
+			const onDisk = await readFileLockInfoForGc(lockDir);
+			expect(onDisk).not.toBeNull();
+			// Same pid, start_time and timestamp, different host: two machines sharing
+			// a volume can coincide on all of those, so host identity must gate the
+			// delete rather than being ignored.
+			expect(
+				await removeFileLockDirForGc(lockDir, {
+					pid: onDisk!.pid,
+					start_time: onDisk!.start_time,
+					host_id: "some-other-host",
+					timestamp: onDisk!.timestamp,
+				}),
+			).toBe("owner_changed");
+			expect(await readFileLockInfoForGc(lockDir)).not.toBeNull();
+			// The exact owner token still removes it.
+			expect(await removeFileLockDirForGc(lockDir, onDisk!)).toBe("removed");
+		} finally {
+			await fs.rm(dir, { recursive: true, force: true });
+		}
+	});
+
+	test("a lock this process takes records its own host identity", async () => {
+		const dir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-lock-xhost-"));
+		try {
+			let observed: string | undefined;
+			await withFileLock(path.join(dir, "state.json"), async () => {
+				observed = (await readFileLockInfoForGc(`${path.join(dir, "state.json")}.lock`))?.host_id;
+			});
+			expect(observed).toBe(os.hostname() || "unknown");
+		} finally {
+			await fs.rm(dir, { recursive: true, force: true });
+		}
+	});
+});