fix(persistence): fsync state writes so a rename is actually durable (#10631)

* fix(persistence): fsync state writes so a rename is actually durable

`Store` wrote `orca-data.json` to a temp file and renamed it. rename() is
atomic for readers but says nothing about durability: without an fsync the
directory entry can reach disk before the data does. After power loss or a
hard crash the file can come back holding the previous state or, worse,
zero bytes — and `JSON.parse('')` throws, so an empty file takes the
full corrupt-file path rather than degrading.

This is the same empty-file symptom as #1158 from a different cause. That
issue fixed a logic path that persisted empty state and added the .bak ring
as a safety net; the ring also catches this, which is why it went unnoticed.
Recovery costs up to an hour of tabs/layouts/session state (backups are
throttled to >=1h spacing), and a user in their first hour has no backup
slot yet, so they land on defaults indistinguishable from a fresh install.

Both write paths now fsync the temp file *before* the rename, then fsync
the containing directory. Directory fsync is best-effort by design: Windows
cannot open a directory for fsync and some filesystems reject it, so it is
swallowed. The file fsync is the load-bearing part and works everywhere.

Measured cost on a 3 MB payload: ~0.2 ms per write, against a 1s debounce.
The async path does not block the main thread.

The syscall-order test mocks `node:fs` and counts fsync targets at the
module boundary, asserting ['file', 'directory'] — proving the ordering
rather than inferring it from reading the implementation, since a fsync
after the rename would still pass every content assertion.

* test(persistence): make the syscall proof platform-aware and actually prove the order

Two problems, both found from CodeRabbit's Windows observation.

The assertion hardcoded ['file', 'directory']. Directory fsync is
deliberately best-effort — Windows cannot open a directory for fsync and
some filesystems reject it — so on Windows the helper swallows the failure,
only the file fsync is observed, and the test fails. The expectation now
probes the real platform instead of assuming, keeping the guarantee tight
where directory fsync works rather than dropping it everywhere.

Worse, the test did not prove what its name claimed. Moving the fsync to
*after* the rename still passes: the file is fsynced either way, and only
fsyncs were recorded, so the correct and broken orders produced an identical
log. Mutation-testing the "before rename" claim is what surfaced this — the
mutation passed.

The rename is now recorded in the same sequence, since it is the boundary
the ordering is defined against. Re-running the same mutation fails, so the
ordering claim is now backed by the test rather than asserted in a comment.

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

src/main/durable-file-write-syscall-proof.test.ts

@@ -0,0 +1,67 @@
+// Empirical proof that the durable write fsyncs the file, and the directory where the platform
+// allows it. Counted at the module boundary rather than inferred from reading the implementation.
+import { closeSync, fsyncSync, mkdtempSync, openSync, readFileSync, rmSync } from 'node:fs'
+import type * as NodeFs from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { expect, it, vi } from 'vitest'
+
+/** Why the rename is recorded too: an fsync moved after the rename still fsyncs a file, so a
+ *  fsync-only log reads identically for the correct and the broken order. The rename is the boundary
+ *  the ordering is defined against, so it has to appear in the same sequence. */
+const syscalls: ('fsync:file' | 'fsync:directory' | 'rename')[] = []
+
+vi.mock('node:fs', async () => {
+  const actual = await vi.importActual<typeof NodeFs>('node:fs')
+  return {
+    ...actual,
+    fsyncSync: (fd: number) => {
+      syscalls.push(actual.fstatSync(fd).isDirectory() ? 'fsync:directory' : 'fsync:file')
+      return actual.fsyncSync(fd)
+    },
+    renameSync: (from: NodeFs.PathLike, to: NodeFs.PathLike) => {
+      syscalls.push('rename')
+      return actual.renameSync(from, to)
+    }
+  }
+})
+
+/** Windows cannot open a directory for fsync, and some filesystems reject it; probe rather than
+ *  assume, so the expectation tracks the real platform instead of a hardcoded OS list. */
+function directoryFsyncSupported(directory: string): boolean {
+  let fd: number | null = null
+  try {
+    fd = openSync(directory, 'r')
+    fsyncSync(fd)
+    return true
+  } catch {
+    return false
+  } finally {
+    if (fd !== null) {
+      try {
+        closeSync(fd)
+      } catch {
+        // Nothing actionable in a probe.
+      }
+    }
+  }
+}
+
+it('fsyncs the file before rename, and the directory after where supported', async () => {
+  const { writeFileDurableSync } = await import('./durable-file-write')
+  const dir = mkdtempSync(join(tmpdir(), 'orca-fsync-'))
+  try {
+    const supported = directoryFsyncSupported(dir)
+    syscalls.length = 0 // Discard the probe's own fsync.
+    const target = join(dir, 'x.json')
+    writeFileDurableSync(`${target}.tmp`, target, '{"ok":1}')
+    expect(readFileSync(target, 'utf-8')).toBe('{"ok":1}')
+    // The data fsync must precede the rename: that ordering is the entire fix. Publishing the name
+    // first is what lets a crash expose a stale or zero-length file.
+    expect(syscalls).toEqual(
+      supported ? ['fsync:file', 'rename', 'fsync:directory'] : ['fsync:file', 'rename']
+    )
+  } finally {
+    rmSync(dir, { recursive: true, force: true })
+  }
+})

src/main/durable-file-write.test.ts

@@ -0,0 +1,87 @@
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { writeFileDurable, writeFileDurableSync } from './durable-file-write'
+
+describe('durable file write', () => {
+  let dir: string
+
+  beforeEach(() => {
+    dir = mkdtempSync(join(tmpdir(), 'orca-durable-'))
+  })
+
+  afterEach(() => {
+    rmSync(dir, { recursive: true, force: true })
+  })
+
+  for (const [label, write] of [
+    ['async', (t: string, f: string, p: string) => writeFileDurable(t, f, p)],
+    [
+      'sync',
+      (t: string, f: string, p: string) => {
+        writeFileDurableSync(t, f, p)
+        return Promise.resolve()
+      }
+    ]
+  ] as const) {
+    describe(label, () => {
+      it('publishes the payload at the final path', async () => {
+        const final = join(dir, 'state.json')
+        await write(`${final}.tmp`, final, '{"a":1}')
+        expect(readFileSync(final, 'utf-8')).toBe('{"a":1}')
+      })
+
+      it('replaces existing content atomically', async () => {
+        const final = join(dir, 'state.json')
+        writeFileSync(final, 'stale', 'utf-8')
+        await write(`${final}.tmp`, final, 'fresh')
+        expect(readFileSync(final, 'utf-8')).toBe('fresh')
+      })
+
+      it('leaves no temp file behind on success', async () => {
+        const final = join(dir, 'state.json')
+        const tmp = `${final}.tmp`
+        await write(tmp, final, 'x')
+        expect(() => readFileSync(tmp, 'utf-8')).toThrow()
+      })
+
+      it('round-trips a multi-megabyte payload without truncation', async () => {
+        // Why: the real orca-data.json is large; a partial fsync would surface here.
+        const final = join(dir, 'big.json')
+        const payload = JSON.stringify({ blob: 'x'.repeat(4 * 1024 * 1024) })
+        await write(`${final}.tmp`, final, payload)
+        expect(readFileSync(final, 'utf-8')).toHaveLength(payload.length)
+      })
+
+      it('preserves exact bytes for multibyte and escape-sensitive content', async () => {
+        const final = join(dir, 'utf8.json')
+        const payload = JSON.stringify({ s: 'emoji 🚀 + 日本語 + \u0000 + "quotes"' })
+        await write(`${final}.tmp`, final, payload)
+        expect(readFileSync(final, 'utf-8')).toBe(payload)
+      })
+
+      it('surfaces an unwritable temp path instead of silently succeeding', async () => {
+        const final = join(dir, 'state.json')
+        const tmp = join(dir, 'missing-subdir', 'state.json.tmp')
+        // The sync variant throws synchronously and the async one rejects; both must fail loudly
+        // and neither may publish a partial file.
+        let failed = false
+        try {
+          await write(tmp, final, 'x')
+        } catch {
+          failed = true
+        }
+        expect(failed).toBe(true)
+        expect(() => readFileSync(final, 'utf-8')).toThrow()
+      })
+    })
+  }
+
+  it('keeps the last writer when async and sync paths target one file', async () => {
+    const final = join(dir, 'state.json')
+    await writeFileDurable(`${final}.a.tmp`, final, 'from-async')
+    writeFileDurableSync(`${final}.b.tmp`, final, 'from-sync')
+    expect(readFileSync(final, 'utf-8')).toBe('from-sync')
+  })
+})

src/main/durable-file-write.ts

@@ -0,0 +1,83 @@
+// Why: rename() is atomic for readers but not durable. Without fsync on the file and its directory,
+// a power loss after a successful rename can leave the old contents, or an empty inode — the same
+// empty-file symptom as issue #1158, from a different cause. The .bak ring recovers it at up to an
+// hour's loss; fsync stops it from happening.
+
+import { closeSync, fsyncSync, openSync, renameSync, writeFileSync } from 'node:fs'
+import { open, rename } from 'node:fs/promises'
+import { dirname } from 'node:path'
+
+/**
+ * fsync a directory so a rename within it is durable. Best-effort by design: Windows cannot open a
+ * directory for fsync, and some filesystems reject it. The file fsync above it is the load-bearing
+ * part; this closes the "rename recorded but not persisted" window where the platform allows it.
+ */
+async function syncDirectory(directory: string): Promise<void> {
+  let handle: Awaited<ReturnType<typeof open>> | null = null
+  try {
+    handle = await open(directory, 'r')
+    await handle.sync()
+  } catch {
+    // Expected on Windows and on filesystems without directory fsync.
+  } finally {
+    await handle?.close().catch(() => {})
+  }
+}
+
+function syncDirectorySync(directory: string): void {
+  let fd: number | null = null
+  try {
+    fd = openSync(directory, 'r')
+    fsyncSync(fd)
+  } catch {
+    // Same platform caveats as syncDirectory.
+  } finally {
+    if (fd !== null) {
+      try {
+        closeSync(fd)
+      } catch {
+        // Nothing actionable; the fsync already happened or the open failed.
+      }
+    }
+  }
+}
+
+/**
+ * Rename and then fsync the containing directory. For callers that already fsynced the temp file
+ * themselves and need the rename made durable.
+ */
+export async function renameDurable(tmpPath: string, finalPath: string): Promise<void> {
+  await rename(tmpPath, finalPath)
+  await syncDirectory(dirname(finalPath))
+}
+
+/** Write `payload` to `tmpPath`, fsync it, then rename onto `finalPath` and fsync the directory. */
+export async function writeFileDurable(
+  tmpPath: string,
+  finalPath: string,
+  payload: string
+): Promise<void> {
+  const handle = await open(tmpPath, 'w')
+  try {
+    await handle.writeFile(payload, 'utf-8')
+    // Why: fsync BEFORE rename. A rename that lands first can expose a zero-length file.
+    await handle.sync()
+  } finally {
+    await handle.close()
+  }
+  await rename(tmpPath, finalPath)
+  await syncDirectory(dirname(finalPath))
+}
+
+/** Synchronous counterpart for quit and crash paths that cannot await. */
+export function writeFileDurableSync(tmpPath: string, finalPath: string, payload: string): void {
+  writeFileSync(tmpPath, payload, 'utf-8')
+  const fd = openSync(tmpPath, 'r+')
+  try {
+    fsyncSync(fd)
+  } finally {
+    closeSync(fd)
+  }
+  renameSync(tmpPath, finalPath)
+  syncDirectorySync(dirname(finalPath))
+}

src/main/persistence.ts

@@ -11,7 +11,8 @@ import {
   statSync,
   realpathSync
 } from 'node:fs'
-import { writeFile, rename, mkdir, rm, copyFile } from 'node:fs/promises'
+import { rename, mkdir, rm, copyFile, open } from 'node:fs/promises'
+import { renameDurable, writeFileDurableSync } from './durable-file-write'
 import { join, dirname, isAbsolute, resolve, sep } from 'node:path'
 import { homedir } from 'node:os'
 import { createHash, randomUUID } from 'node:crypto'
@@ -3686,12 +3687,19 @@ export class Store {
     // Why: on any write/rename failure, remove the tmp file so it doesn't leave a multi-MB orphan.
     let renamed = false
     try {
-      await writeFile(tmpFile, payload, 'utf-8')
+      // Why: fsync before rename, then fsync the directory; see writeFileDurable.
+      const handle = await open(tmpFile, 'w')
+      try {
+        await handle.writeFile(payload, 'utf-8')
+        await handle.sync()
+      } finally {
+        await handle.close()
+      }
       // Why: if flush() bumped writeGeneration mid-write, it already wrote fresher state; don't overwrite it.
       if (this.writeGeneration !== gen) {
         return
       }
-      await rename(tmpFile, dataFile)
+      await renameDurable(tmpFile, dataFile)
       renamed = true
       // Why re-check gen: a sync flush during the rename await may have written fresher state; don't record a stale hash over it.
       if (this.writeGeneration === gen) {
@@ -3732,8 +3740,9 @@ export class Store {
     // Why: on any write/rename failure, remove the tmp file so shutdown crashes don't leak orphans.
     let renamed = false
     try {
-      writeFileSync(tmpFile, payload, 'utf-8')
-      renameSync(tmpFile, dataFile)
+      // Why: fsync the temp file and the directory; a bare rename can survive as stale or empty
+      // content after power loss, losing projects/tabs back to the newest usable .bak slot.
+      writeFileDurableSync(tmpFile, dataFile, payload)
       renamed = true
       this.lastWrittenStateHash = stateHash
     } finally {