fix(desktop): retry a settings write that loses the rename to a Windows lock

packages/desktop/src/main/storage/atomic-write.test.ts

@@ -0,0 +1,105 @@
+import { describe, expect, test } from "bun:test"
+import { dlopen, FFIType, ptr } from "bun:ffi"
+import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"
+import { tmpdir } from "node:os"
+import path from "node:path"
+import { writeFileAtomic } from "./atomic-write"
+
+const dir = () => mkdtempSync(path.join(tmpdir(), "opencode-atomic-write-"))
+
+const writes = [
+  { name: "a new file", existing: undefined },
+  { name: "an existing file", existing: '{"windowIds":["a"]}' },
+]
+
+describe("writeFileAtomic", () => {
+  for (const item of writes) {
+    test(`writes ${item.name}, creating its directory, without leaving a temporary behind`, () => {
+      const file = path.join(dir(), "nested", "opencode.settings")
+
+      if (item.existing !== undefined) {
+        mkdirSync(path.dirname(file), { recursive: true })
+        writeFileSync(file, item.existing)
+      }
+
+      writeFileAtomic(file, '{"pinchZoomEnabled":true}')
+
+      expect(readFileSync(file, "utf8")).toBe('{"pinchZoomEnabled":true}')
+      expect(existsSync(`${file}.tmp`)).toBe(false)
+    })
+  }
+
+  // A write that cannot land must still clean up after itself, so a reader never mistakes a
+  // staging file for state.
+  test("removes the temporary file when the target cannot be replaced", () => {
+    const target = path.join(dir(), "blocked")
+    mkdirSync(target)
+
+    expect(() => writeFileAtomic(target, "{}")).toThrow()
+    expect(existsSync(`${target}.tmp`)).toBe(false)
+  })
+})
+
+// The failure this module fixes only exists on Windows, and node:fs cannot produce it: every handle
+// it opens grants FILE_SHARE_READ|WRITE|DELETE, so two node:fs handles never contend and a rename
+// always wins. Asking for a specific dwShareMode needs Win32, and bun:ffi is the only way to reach
+// it without a native build or a test-only seam in the module under test.
+function holdOpen(file: string, mode: "read+write" | "exclusive") {
+  const GENERIC_READ = 0x80000000
+  const FILE_SHARE_READ = 0x00000001
+  const FILE_SHARE_WRITE = 0x00000002
+  const OPEN_EXISTING = 3
+  const share = mode === "read+write" ? FILE_SHARE_READ | FILE_SHARE_WRITE : 0
+
+  const kernel32 = dlopen("kernel32.dll", {
+    CreateFileW: {
+      args: [FFIType.ptr, FFIType.u32, FFIType.u32, FFIType.ptr, FFIType.u32, FFIType.u32, FFIType.ptr],
+      returns: FFIType.ptr,
+    },
+    CloseHandle: { args: [FFIType.ptr], returns: FFIType.i32 },
+  })
+
+  const wide = Buffer.from(`${file}\0`, "utf16le")
+  const handle = kernel32.symbols.CreateFileW(ptr(wide), GENERIC_READ, share, null, OPEN_EXISTING, 0, null)
+
+  if (handle === null) throw new Error(`could not open a handle on ${file}`)
+
+  return { handle, release: () => kernel32.symbols.CloseHandle(handle) }
+}
+
+const windowsTest = test.skipIf(process.platform !== "win32")
+
+// An antivirus scanner or search indexer reading the file under AppData\Roaming is what produced
+// the reported EPERM: it grants FILE_SHARE_WRITE, which permits writing through the open handle but
+// not replacing the file.
+windowsTest("writes while another handle holds the target open with FILE_SHARE_READ|FILE_SHARE_WRITE", () => {
+  const file = path.join(dir(), "opencode.settings")
+  writeFileSync(file, '{"ready":false}')
+  const holder = holdOpen(file, "read+write")
+
+  try {
+    writeFileAtomic(file, '{"ready":true}')
+  } finally {
+    holder.release()
+  }
+
+  expect(readFileSync(file, "utf8")).toBe('{"ready":true}')
+  expect(existsSync(`${file}.tmp`)).toBe(false)
+})
+
+// A holder that shares nothing blocks the replacement and the write-through alike, so the write has
+// to fail loudly. What it must not do is leave the staging file behind or damage the old contents.
+windowsTest("reports the lock and keeps the old contents when nothing can be written", () => {
+  const file = path.join(dir(), "opencode.settings")
+  writeFileSync(file, '{"ready":false}')
+  const holder = holdOpen(file, "exclusive")
+
+  try {
+    expect(() => writeFileAtomic(file, '{"ready":true}')).toThrow()
+  } finally {
+    holder.release()
+  }
+
+  expect(readFileSync(file, "utf8")).toBe('{"ready":false}')
+  expect(existsSync(`${file}.tmp`)).toBe(false)
+})

packages/desktop/src/main/storage/atomic-write.ts

@@ -0,0 +1,90 @@
+import { mkdirSync, renameSync, rmSync, writeFileSync } from "node:fs"
+import path from "node:path"
+
+// Replacing a file is not as safe as it looks on Windows. `rename` over an existing file fails with
+// EPERM/EBUSY while *any* other handle holds that file open, even one that granted every share bit
+// in `dwShareMode`. Antivirus, search indexers and backup agents open files under AppData\Roaming
+// routinely while they scan, so the failure is transient, real, and reported against this code
+// path: https://github.com/anomalyco/opencode/issues/51470
+//
+// The lock clears on its own, so retry before concluding the write cannot land.
+const attempts = 4
+
+const retryDelayMs = 15
+
+// A sharing violation is the one failure a later attempt can survive; nothing else clears by
+// waiting. The three codes are what Node reports for it, depending on how the holder opened the
+// file: EPERM for a denied replacement, EBUSY for a denied open.
+function isLockedError(cause: unknown): cause is NodeJS.ErrnoException {
+  if (!(cause instanceof Error) || !("code" in cause)) return false
+
+  return cause.code === "EPERM" || cause.code === "EACCES" || cause.code === "EBUSY"
+}
+
+// The settings store is read before the storage layer exists and window state is written as the
+// window closes, so both callers are synchronous by design. Atomics.wait blocks without spinning,
+// and the buffer is per call, so nothing is shared between calls.
+function waitForLockToClear(ms: number) {
+  Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms)
+}
+
+/**
+ * Runs an operation, retrying while the failure keeps looking like a transient Windows lock.
+ * Returns the lock error once the retries run out, and throws anything that waiting cannot fix.
+ */
+function retryWhileLocked(operation: () => void): Error | undefined {
+  let attempt = 0
+
+  while (true) {
+    try {
+      operation()
+
+      return undefined
+    } catch (cause) {
+      if (!isLockedError(cause)) throw cause
+
+      attempt += 1
+
+      if (attempt >= attempts) return cause
+
+      waitForLockToClear(retryDelayMs * attempt)
+    }
+  }
+}
+
+/**
+ * Replaces a file without ever exposing a half-written one. The contents are written beside the
+ * target and renamed over it, retrying the rename while another process holds the target open.
+ */
+export function writeFileAtomic(file: string, contents: string) {
+  const tmp = `${file}.tmp`
+
+  mkdirSync(path.dirname(file), { recursive: true })
+  writeFileSync(tmp, contents)
+
+  let locked: Error | undefined
+
+  try {
+    locked = retryWhileLocked(() => renameSync(tmp, file))
+  } catch (cause) {
+    rmSync(tmp, { force: true })
+
+    throw cause
+  }
+
+  if (locked === undefined) return
+
+  // The target stayed locked past the retries. A replacement needs the file closed, but writing
+  // through the open handle only needs FILE_SHARE_WRITE, which the scanners that cause this grant.
+  // Keeping the update beats dropping it silently, and a write interrupted by a crash is set aside
+  // by the reader instead of blocking startup.
+  try {
+    writeFileSync(file, contents)
+  } catch {
+    rmSync(tmp, { force: true })
+
+    throw locked
+  }
+
+  rmSync(tmp, { force: true })
+}

packages/desktop/src/main/storage/store.ts

@@ -1,7 +1,8 @@
-import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"
+import { existsSync, readFileSync, renameSync } from "node:fs"
 import path from "node:path"
 import electron from "electron"
 
+import { writeFileAtomic } from "./atomic-write"
 import { SETTINGS_STORE } from "./keys"
 
 const cache = new Map<string, SettingsStore>()
@@ -32,11 +33,7 @@ export function getStore(name = SETTINGS_STORE) {
 export function createSettingsStore(file: string) {
   let data = read(file)
 
-  const write = () => {
-    mkdirSync(path.dirname(file), { recursive: true })
-    writeFileSync(`${file}.tmp`, JSON.stringify(data, null, "\t"))
-    renameSync(`${file}.tmp`, file)
-  }
+  const write = () => writeFileAtomic(file, JSON.stringify(data, null, "\t"))
 
   return {
     path: file,

packages/desktop/src/main/windows/window-state.ts

@@ -1,7 +1,8 @@
-import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"
-import path from "node:path"
+import { existsSync, readFileSync } from "node:fs"
 import type { BrowserWindow, Rectangle } from "electron"
 
+import { writeFileAtomic } from "../storage/atomic-write"
+
 // Remembers a window's bounds and maximized / fullscreen flags across launches in the same JSON
 // file electron-window-state wrote, so existing profiles restore where they were.
 
@@ -100,9 +101,7 @@ export function manageWindowState(win: BrowserWindow, file: string, initial: Win
     win.off("move", changed)
     win.off("close", update)
     win.off("closed", closed)
-    mkdirSync(path.dirname(file), { recursive: true })
-    writeFileSync(`${file}.tmp`, JSON.stringify(state))
-    renameSync(`${file}.tmp`, file)
+    writeFileAtomic(file, JSON.stringify(state))
   }
 
   win.on("resize", changed)