fix: one crash-safe owner for durable state writes

packages/coding-agent/.changes/res-1267-atomic-persistence.md

@@ -0,0 +1 @@
+- Made every durable JSON/JSONL state write crash-safe through one shared atomic-write owner (temp file + rename, Windows rename retry): auth.json is no longer written in place (an interrupted write can no longer log you out everywhere), the auth migration writes its destination before destroying its sources, racing first-time settings writers no longer silently discard each other, and the kernel bootstrap lock can no longer be stolen mid-reclaim. Session files now repair crash damage (torn tails, zero-filled records) at open instead of silently losing the next message, and a session lease whose owner file is momentarily unreadable is no longer treated as stale and destroyed.

packages/coding-agent/src/core/auth-storage.ts

@@ -15,10 +15,11 @@ import {
 	type OAuthProviderId,
 } from "@earendil-works/pi-ai";
 import { getOAuthApiKey, getOAuthProvider, getOAuthProviders } from "@earendil-works/pi-ai/oauth";
-import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "fs";
+import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs";
 import { dirname, join } from "path";
 import lockfile from "proper-lockfile";
 import { getAgentDir } from "../config.js";
+import { realpathIfPresentSync, writeFileAtomicSync } from "../utils/atomic-file.js";
 import {
 	clearPrimeCliCredentials,
 	getPrimeCliConfigPath,
@@ -116,9 +117,14 @@ export class FileAuthStorageBackend implements AuthStorageBackend {
 	}
 
 	private ensureFileExists(): void {
-		if (!existsSync(this.authPath)) {
-			writeFileSync(this.authPath, "{}", "utf-8");
-			chmodSync(this.authPath, 0o600);
+		try {
+			// Exclusive create: a racing initializer must never replace credentials another
+			// process saved between an existence check and this write.
+			writeFileSync(this.authPath, "{}", { flag: "wx", mode: 0o600 });
+		} catch (error) {
+			if ((error as NodeJS.ErrnoException).code !== "EEXIST") {
+				throw error;
+			}
 		}
 	}
 
@@ -171,8 +177,7 @@ export class FileAuthStorageBackend implements AuthStorageBackend {
 			const current = existsSync(this.authPath) ? readFileSync(this.authPath, "utf-8") : undefined;
 			const { result, next } = fn(current);
 			if (next !== undefined) {
-				writeFileSync(this.authPath, next, "utf-8");
-				chmodSync(this.authPath, 0o600);
+				writeFileAtomicSync(realpathIfPresentSync(this.authPath), next, { mode: 0o600 });
 			}
 			return result;
 		} finally {
@@ -216,8 +221,7 @@ export class FileAuthStorageBackend implements AuthStorageBackend {
 			const { result, next } = await fn(current);
 			throwIfCompromised();
 			if (next !== undefined) {
-				writeFileSync(this.authPath, next, "utf-8");
-				chmodSync(this.authPath, 0o600);
+				writeFileAtomicSync(realpathIfPresentSync(this.authPath), next, { mode: 0o600 });
 			}
 			throwIfCompromised();
 			return result;

packages/coding-agent/src/core/cron-jobs.ts

@@ -1,16 +1,8 @@
 import { randomUUID } from "node:crypto";
-import {
-	closeSync,
-	existsSync,
-	fsyncSync,
-	mkdirSync,
-	openSync,
-	readFileSync,
-	renameSync,
-	writeFileSync,
-} from "node:fs";
+import { existsSync, mkdirSync, readFileSync, renameSync } from "node:fs";
 import { dirname, join, resolve } from "node:path";
 import { lockSync } from "proper-lockfile";
+import { writeFileAtomicSync } from "../utils/atomic-file.js";
 import { getSessionArtifactPathForFile } from "./session-manager.js";
 
 export type AgentCronJobStatus = "active" | "paused" | "completed" | "cancelled";
@@ -1558,27 +1550,8 @@ function writeJobsFile(path: string, jobs: readonly AgentCronJob[], mergeCurrent
 }
 
 function writeJobsState(path: string, state: CronJobsState): void {
-	const directory = dirname(path);
-	mkdirSync(directory, { recursive: true, mode: 0o700 });
-	const tempPath = `${path}.${process.pid}.${randomUUID()}.tmp`;
-	const descriptor = openSync(tempPath, "w", 0o600);
-	try {
-		writeFileSync(descriptor, `${JSON.stringify(state, null, 2)}\n`, "utf-8");
-		fsyncSync(descriptor);
-	} finally {
-		closeSync(descriptor);
-	}
-	renameSync(tempPath, path);
-	try {
-		const directoryDescriptor = openSync(directory, "r");
-		try {
-			fsyncSync(directoryDescriptor);
-		} finally {
-			closeSync(directoryDescriptor);
-		}
-	} catch {
-		// Directory fsync is unavailable on some platforms; the atomic rename still protects readers.
-	}
+	mkdirSync(dirname(path), { recursive: true, mode: 0o700 });
+	writeFileAtomicSync(path, `${JSON.stringify(state, null, 2)}\n`, { mode: 0o600, fsync: true, fsyncDir: true });
 }
 
 function claimDueInState(state: CronJobsState, dueAt: Date, claimedAt: Date): AgentCronDispatch[] {

packages/coding-agent/src/core/kernel/bootstrap.ts

@@ -9,6 +9,7 @@ import { createInterface } from "node:readline/promises";
 import { setTimeout as sleep } from "node:timers/promises";
 import { fileURLToPath } from "node:url";
 import { getPackageDir } from "../../config.js";
+import { tryAcquireDirLock } from "../../utils/dir-lock.js";
 import type { PythonSkillRuntimeInfo } from "../skills.js";
 
 const BOOTSTRAP_SCHEMA = 9;
@@ -449,16 +450,6 @@ function processIsRunning(pid: number): boolean {
 	}
 }
 
-async function readLockPid(lockDir: string): Promise<number | null> {
-	try {
-		const raw = await readFile(path.join(lockDir, "pid"), "utf8");
-		const pid = Number.parseInt(raw.trim(), 10);
-		return Number.isInteger(pid) && pid > 0 ? pid : null;
-	} catch {
-		return null;
-	}
-}
-
 async function lockMissingPidIsStale(lockDir: string): Promise<boolean> {
 	try {
 		const lockStat = await stat(lockDir);
@@ -473,19 +464,13 @@ async function acquireBootstrapLock(venv: string): Promise<() => Promise<void>>
 	await mkdir(path.dirname(lockDir), { recursive: true });
 
 	for (;;) {
-		try {
-			await mkdir(lockDir);
-			await writeFile(path.join(lockDir, "pid"), `${process.pid}\n`, "utf8");
+		const attempt = await tryAcquireDirLock(lockDir, async (ownerPid) =>
+			ownerPid === undefined ? !(await lockMissingPidIsStale(lockDir)) : processIsRunning(ownerPid),
+		);
+		if (attempt === "acquired") {
 			return () => rm(lockDir, { recursive: true, force: true });
-		} catch (error) {
-			if (!isNodeError(error, "EEXIST")) throw error;
-
-			const pid = await readLockPid(lockDir);
-			if (pid === null ? await lockMissingPidIsStale(lockDir) : !processIsRunning(pid)) {
-				await rm(lockDir, { recursive: true, force: true });
-				continue;
-			}
-
+		}
+		if (attempt === "held") {
 			await sleep(BOOTSTRAP_LOCK_RETRY_MS);
 		}
 	}

packages/coding-agent/src/core/model-registry.ts

@@ -22,12 +22,13 @@ import {
 } from "@earendil-works/pi-ai";
 import { registerBuiltinMcpOAuthProviders } from "@earendil-works/pi-ai/mcp";
 import { registerOAuthProvider, resetOAuthProviders } from "@earendil-works/pi-ai/oauth";
-import { existsSync, readFileSync, renameSync, writeFileSync } from "fs";
+import { existsSync, readFileSync } from "fs";
 import { dirname, join } from "path";
 import { type Static, type TProperties, Type } from "typebox";
 import type { Validator } from "typebox/compile";
 import type { TLocalizedValidationError } from "typebox/error";
 import { getAgentDir } from "../config.js";
+import { writeFileAtomicSync } from "../utils/atomic-file.js";
 import type { AuthSourceToken, AuthStatus, AuthStorage } from "./auth-storage.js";
 import { PRIME_INFERENCE_PROVIDER_ID } from "./prime-inference-auth.js";
 import {
@@ -926,9 +927,7 @@ export class ModelRegistry {
 			return;
 		}
 		try {
-			const tmpPath = `${cachePath}.${process.pid}.tmp`;
-			writeFileSync(tmpPath, JSON.stringify({ ...cache, modelIds: [...cache.modelIds] }), { mode: 0o600 });
-			renameSync(tmpPath, cachePath);
+			writeFileAtomicSync(cachePath, JSON.stringify({ ...cache, modelIds: [...cache.modelIds] }), { mode: 0o600 });
 		} catch {
 			// A failed cache write only requires a later refetch.
 		}

packages/coding-agent/src/core/refinement/refinement.ts

@@ -1,19 +1,10 @@
-import { randomUUID } from "node:crypto";
-import {
-	appendFileSync,
-	existsSync,
-	mkdirSync,
-	readFileSync,
-	renameSync,
-	statSync,
-	unlinkSync,
-	writeFileSync,
-} from "node:fs";
+import { appendFileSync, existsSync, mkdirSync, readFileSync, statSync } from "node:fs";
 import { join } from "node:path";
 import type { AgentMessage, ThinkingLevel } from "@earendil-works/pi-agent-core";
 import type { Model } from "@earendil-works/pi-ai";
 import { completeSimple } from "@earendil-works/pi-ai";
 import { getAgentDir } from "../../config.js";
+import { realpathIfPresentSync, writeFileAtomicSync } from "../../utils/atomic-file.js";
 import { serializeConversation } from "../compaction/utils.js";
 import { convertToLlm } from "../messages.js";
 import type { CustomEntry } from "../session-manager.js";
@@ -344,17 +335,10 @@ export function mergeHarnessStates(globalState: HarnessState, localState?: Harne
 
 export function saveHarnessState(harnessStateDir: string, state: HarnessState): string {
 	const statePath = getHarnessStatePath(harnessStateDir);
-	const tempPath = `${statePath}.${process.pid}.${randomUUID()}.tmp`;
 	mkdirSync(harnessStateDir, { recursive: true });
-	try {
-		const mode = existsSync(statePath) ? statSync(statePath).mode & 0o777 : 0o600;
-		writeFileSync(tempPath, `${JSON.stringify(state, null, 2)}\n`, { encoding: "utf8", mode });
-		renameSync(tempPath, statePath);
-	} finally {
-		if (existsSync(tempPath)) {
-			unlinkSync(tempPath);
-		}
-	}
+	const targetPath = realpathIfPresentSync(statePath);
+	const mode = existsSync(targetPath) ? statSync(targetPath).mode & 0o777 : 0o600;
+	writeFileAtomicSync(targetPath, `${JSON.stringify(state, null, 2)}\n`, { mode });
 	return statePath;
 }
 

packages/coding-agent/src/core/session-lease.ts

@@ -50,7 +50,7 @@ export class SessionLease {
 		try {
 			withLeaseGuard(this.directory, () => {
 				const owner = readLeaseOwner(this.directory);
-				if (owner?.token === this.token) {
+				if (typeof owner === "object" && owner.token === this.token) {
 					rmSync(this.directory, { recursive: true, force: true });
 				}
 			});
@@ -83,21 +83,28 @@ export function canonicalSessionPath(sessionPath: string): string {
 	}
 }
 
-function readLeaseOwner(directory: string): SessionLeaseOwner | undefined {
+// "absent" (missing/garbage) is safely stale; "unreadable" may be a LIVE lease and must never be reclaimed.
+function readLeaseOwner(directory: string): SessionLeaseOwner | "absent" | "unreadable" {
+	let raw: string;
 	try {
-		const parsed = JSON.parse(readFileSync(join(directory, "owner.json"), "utf8")) as Partial<SessionLeaseOwner>;
+		raw = readFileSync(join(directory, "owner.json"), "utf8");
+	} catch (error) {
+		return (error as NodeJS.ErrnoException).code === "ENOENT" ? "absent" : "unreadable";
+	}
+	try {
+		const parsed = JSON.parse(raw) as Partial<SessionLeaseOwner>;
 		if (
 			parsed.version !== 1 ||
 			typeof parsed.token !== "string" ||
 			typeof parsed.pid !== "number" ||
 			typeof parsed.sessionPath !== "string" ||
 			typeof parsed.createdAt !== "string"
 		) {
-			return undefined;
+			return "absent";
 		}
 		return parsed as SessionLeaseOwner;
 	} catch {
-		return undefined;
+		return "absent";
 	}
 }
 
@@ -298,19 +305,23 @@ export function acquireSessionLease(
 			} catch (error) {
 				rmSync(candidateDirectory, { recursive: true, force: true });
 				const code = (error as NodeJS.ErrnoException).code;
-				if (code !== "EEXIST" && code !== "ENOTEMPTY") {
+				// win32 reports rename-onto-existing-directory as EPERM/EACCES, not EEXIST.
+				if (code !== "EEXIST" && code !== "ENOTEMPTY" && code !== "EPERM" && code !== "EACCES") {
 					throw error;
 				}
 				const existingOwner = readLeaseOwner(directory);
-				if (existingOwner && isLeaseOwnerAlive(existingOwner)) {
+				if (existingOwner === "unreadable") {
+					continue;
+				}
+				if (existingOwner !== "absent" && isLeaseOwnerAlive(existingOwner)) {
 					throw new SessionAlreadyActiveError(canonicalPath, existingOwner.activeSessionId);
 				}
 				reclaimStaleLease(directory);
 			}
 		}
 
 		const owner = existsSync(directory) ? readLeaseOwner(directory) : undefined;
-		if (owner && isLeaseOwnerAlive(owner)) {
+		if (typeof owner === "object" && isLeaseOwnerAlive(owner)) {
 			throw new SessionAlreadyActiveError(canonicalPath, owner.activeSessionId);
 		}
 		throw new Error(`Could not acquire session lease: ${canonicalPath}`);