fix(settings): retry section writes against latest disk state (R-LOGIC-01)

lib/unified-settings.ts

@@ -385,7 +385,7 @@ function writeSettingsRecordSync(
  */
 async function writeSettingsRecordAsync(
 	record: JsonRecord,
-	options?: { skipBackupSnapshot?: boolean },
+	options?: { skipBackupSnapshot?: boolean; expectedMtimeMs?: number | null },
 ): Promise<void> {
 	await fs.mkdir(getCodexMultiAuthDir(), { recursive: true });
 	const payload = normalizeForWrite(record);
@@ -397,6 +397,24 @@ async function writeSettingsRecordAsync(
 	await fs.writeFile(tempPath, data, "utf8");
 	let moved = false;
 	try {
+		if (options && "expectedMtimeMs" in options) {
+			let currentMtimeMs: number | null = null;
+			try {
+				currentMtimeMs = (await fs.stat(UNIFIED_SETTINGS_PATH)).mtimeMs;
+			} catch (error) {
+				const code = (error as NodeJS.ErrnoException).code;
+				if (code !== "ENOENT") {
+					throw error;
+				}
+			}
+			if (currentMtimeMs !== options.expectedMtimeMs) {
+				const staleError = new Error(
+					"Unified settings changed on disk during save; retrying with latest state.",
+				) as NodeJS.ErrnoException;
+				staleError.code = "ESTALE";
+				throw staleError;
+			}
+		}
 		for (let attempt = 0; attempt < 5; attempt += 1) {
 			try {
 				await fs.rename(tempPath, UNIFIED_SETTINGS_PATH);
@@ -420,6 +438,18 @@ async function writeSettingsRecordAsync(
 	}
 }
 
+async function getUnifiedSettingsMtimeMs(): Promise<number | null> {
+	try {
+		return (await fs.stat(UNIFIED_SETTINGS_PATH)).mtimeMs;
+	} catch (error) {
+		const code = (error as NodeJS.ErrnoException).code;
+		if (code === "ENOENT") {
+			return null;
+		}
+		throw error;
+	}
+}
+
 async function enqueueSettingsWrite<T>(task: () => Promise<T>): Promise<T> {
 	const run = settingsWriteQueue.catch(() => {}).then(task);
 	settingsWriteQueue = run.then(
@@ -492,12 +522,25 @@ export function saveUnifiedPluginConfigSync(pluginConfig: JsonRecord): void {
  */
 export async function saveUnifiedPluginConfig(pluginConfig: JsonRecord): Promise<void> {
 	await enqueueSettingsWrite(async () => {
-		const { record, usedBackup } = await readSettingsRecordAsyncInternal();
-		const nextRecord = record ?? {};
-		nextRecord.pluginConfig = { ...pluginConfig };
-		await writeSettingsRecordAsync(nextRecord, {
-			skipBackupSnapshot: usedBackup,
-		});
+		let expectedMtimeMs = await getUnifiedSettingsMtimeMs();
+		let { record, usedBackup } = await readSettingsRecordAsyncInternal();
+		for (let attempt = 0; attempt < 3; attempt += 1) {
+			const nextRecord = record ?? {};
+			nextRecord.pluginConfig = { ...pluginConfig };
+			try {
+				await writeSettingsRecordAsync(nextRecord, {
+					skipBackupSnapshot: usedBackup,
+					expectedMtimeMs,
+				});
+				return;
+			} catch (error) {
+				if ((error as NodeJS.ErrnoException).code !== "ESTALE" || attempt >= 2) {
+					throw error;
+				}
+				expectedMtimeMs = await getUnifiedSettingsMtimeMs();
+				({ record, usedBackup } = await readSettingsRecordAsyncInternal());
+			}
+		}
 	});
 }
 
@@ -538,11 +581,24 @@ export async function saveUnifiedDashboardSettings(
 	dashboardDisplaySettings: JsonRecord,
 ): Promise<void> {
 	await enqueueSettingsWrite(async () => {
-		const { record, usedBackup } = await readSettingsRecordAsyncInternal();
-		const nextRecord = record ?? {};
-		nextRecord.dashboardDisplaySettings = { ...dashboardDisplaySettings };
-		await writeSettingsRecordAsync(nextRecord, {
-			skipBackupSnapshot: usedBackup,
-		});
+		let expectedMtimeMs = await getUnifiedSettingsMtimeMs();
+		let { record, usedBackup } = await readSettingsRecordAsyncInternal();
+		for (let attempt = 0; attempt < 3; attempt += 1) {
+			const nextRecord = record ?? {};
+			nextRecord.dashboardDisplaySettings = { ...dashboardDisplaySettings };
+			try {
+				await writeSettingsRecordAsync(nextRecord, {
+					skipBackupSnapshot: usedBackup,
+					expectedMtimeMs,
+				});
+				return;
+			} catch (error) {
+				if ((error as NodeJS.ErrnoException).code !== "ESTALE" || attempt >= 2) {
+					throw error;
+				}
+				expectedMtimeMs = await getUnifiedSettingsMtimeMs();
+				({ record, usedBackup } = await readSettingsRecordAsyncInternal());
+			}
+		}
 	});
 }

test/unified-settings.test.ts

@@ -736,6 +736,93 @@ describe("unified settings", () => {
 		});
 	});
 
+	it("retries plugin section write against newer on-disk record to avoid stale overwrite", async () => {
+		const {
+			getUnifiedSettingsPath,
+			saveUnifiedPluginConfig,
+		} = await import("../lib/unified-settings.js");
+
+		await fs.writeFile(
+			getUnifiedSettingsPath(),
+			JSON.stringify(
+				{
+					version: 1,
+					pluginConfig: { codexMode: false, retries: 9 },
+					dashboardDisplaySettings: {
+						menuShowLastUsed: true,
+						uiThemePreset: "green",
+					},
+					experimentalDraft: {
+						enabled: false,
+						panels: ["future"],
+					},
+				},
+				null,
+				2,
+			),
+			"utf8",
+		);
+
+		const originalWriteFile = fs.writeFile;
+		const writeSpy = vi.spyOn(fs, "writeFile");
+		let injectedConcurrentWrite = false;
+		writeSpy.mockImplementation(async (...args: Parameters<typeof fs.writeFile>) => {
+			const [filePath, data] = args;
+			const result = await originalWriteFile(...args);
+			if (!injectedConcurrentWrite && String(filePath).includes(".tmp")) {
+				injectedConcurrentWrite = true;
+				await originalWriteFile(
+					getUnifiedSettingsPath(),
+					`${JSON.stringify(
+						{
+							version: 1,
+							pluginConfig: { codexMode: false, retries: 9 },
+							dashboardDisplaySettings: {
+								menuShowLastUsed: false,
+								uiThemePreset: "purple",
+							},
+							experimentalDraft: {
+								enabled: true,
+								panels: ["fresh"],
+							},
+						},
+						null,
+						2,
+					)}\n`,
+					"utf8",
+				);
+			}
+			return result;
+		});
+
+		try {
+			await saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 45_000 });
+		} finally {
+			writeSpy.mockRestore();
+		}
+
+		expect(injectedConcurrentWrite).toBe(true);
+		const parsed = JSON.parse(
+			await fs.readFile(getUnifiedSettingsPath(), "utf8"),
+		) as {
+			pluginConfig?: Record<string, unknown>;
+			dashboardDisplaySettings?: Record<string, unknown>;
+			experimentalDraft?: Record<string, unknown>;
+		};
+		expect(parsed.pluginConfig).toEqual({
+			codexMode: true,
+			fetchTimeoutMs: 45_000,
+		});
+		expect(parsed.dashboardDisplaySettings).toEqual({
+			menuShowLastUsed: false,
+			uiThemePreset: "purple",
+		});
+		expect(parsed.experimentalDraft).toEqual({
+			enabled: true,
+			panels: ["fresh"],
+		});
+	});
+
 	it("falls back to backup when the primary settings file is unreadable", async () => {
 		const {
 			saveUnifiedPluginConfig,