Add fsync-durability and Windows rename-retry coverage to atomic writes

Companion regression tests for the fsync-before-rename durability fix: the
fsync must precede the rename (power loss between them leaves the old file
intact), and the async rename retries EPERM/EBUSY/EACCES with bounded backoff
while rethrowing anything else on the first attempt. Recovery loader, backup
rotation, and UI notice split out to a follow-up PR per review.

Author: nordicnode

cli/src/hooks/use-send-message.ts

@@ -187,21 +187,7 @@ export const useSendMessage = ({
       if (loadedState) {
         previousRunStateRef.current = loadedState.runState
         setRunState(loadedState.runState)
-        const restoredMessages = sanitizeRestoredMessages(loadedState.messages)
-        if (loadedState.runStateRestored) {
-          setMessages(restoredMessages)
-        } else {
-          // The agent's context was lost (torn run-state.json, nothing
-          // recoverable) while the transcript survived. Surface it: without
-          // this the model just answers as if the earlier turns never
-          // happened, which reads as the assistant being broken.
-          setMessages([
-            createErrorChatMessage(
-              'The saved agent context could not be restored, so the assistant starts this chat without memory of earlier turns. The transcript below is intact.',
-            ),
-            ...restoredMessages,
-          ])
-        }
+        setMessages(sanitizeRestoredMessages(loadedState.messages))
         if (loadedState.chatId) {
           setCurrentChatId(loadedState.chatId)
         }

cli/src/utils/__tests__/run-state-storage.test.ts

@@ -917,146 +917,3 @@ describe('poisoned payload persistence', () => {
     expect(block.outputRaw.self).toBe('[Circular]')
   })
 })
-
-describe('run state recovery', () => {
-  // Point persistence at a temp dir via the explicit test override.
-  const chatDir = path.join(TEST_ROOT, 'codebuff-test-recovery')
-
-  const runStateWithSession = (marker: string): RunState =>
-    ({
-      sessionState: {
-        mainAgentState: {
-          messageHistory: [{ role: 'user', content: marker }],
-        },
-      },
-      output: { type: 'lastMessage', value: marker },
-      traceSessionId: 'trace-1',
-    }) as unknown as RunState
-
-  const runStatePath = path.join(chatDir, 'run-state.json')
-  const bakPath = runStatePath + '.bak'
-  const messagesPath = path.join(chatDir, 'chat-messages.json')
-
-  const writePrimary = (contents: string) =>
-    fs.writeFileSync(runStatePath, contents)
-  const validMessages = JSON.stringify([
-    {
-      id: 'msg-1',
-      variant: 'user',
-      content: 'the prompt',
-      timestamp: new Date().toISOString(),
-    },
-  ] as ChatMessage[])
-
-  beforeEach(() => {
-    fs.rmSync(chatDir, { recursive: true, force: true })
-    fs.mkdirSync(chatDir, { recursive: true })
-    setChatDirOverrideForTesting(chatDir)
-  })
-
-  afterEach(() => {
-    setChatDirOverrideForTesting(undefined)
-  })
-
-  test('recovers agent context from the .bak when the primary is torn', () => {
-    writePrimary('{"sessionState": {"main"') // torn: power loss after rename
-    fs.writeFileSync(bakPath, JSON.stringify(runStateWithSession('from-bak')))
-    fs.writeFileSync(messagesPath, validMessages)
-
-    const loaded = loadMostRecentChatState()
-    expect(loaded).not.toBeNull()
-    // Agent context survived — the model is NOT amnesiac next turn.
-    expect((loaded!.runState as any).sessionState).toBeDefined()
-    expect(loaded!.runStateRestored).toBe(true)
-    // Self-healed: the primary is the recovered generation again.
-    expect(
-      JSON.parse(fs.readFileSync(runStatePath, 'utf8')).output.value,
-    ).toBe('from-bak')
-  })
-
-  test('recovers from the newest complete checkpoint temp when bak is absent', () => {
-    writePrimary('{ torn')
-    fs.writeFileSync(messagesPath, validMessages)
-    // Two temps: an older torn one and a newer complete one (SIGKILL between
-    // write and rename leaves the latter behind).
-    fs.writeFileSync(
-      runStatePath + '.999.oldest.tmp',
-      '{"half":',
-    )
-    fs.writeFileSync(
-      runStatePath + '.1234.newest.tmp',
-      JSON.stringify(runStateWithSession('from-tmp')),
-    )
-
-    const loaded = loadMostRecentChatState()
-    expect(loaded).not.toBeNull()
-    expect((loaded!.runState as any).sessionState).toBeDefined()
-    expect(loaded!.runStateRestored).toBe(true)
-    // Self-healed into the primary.
-    expect(
-      JSON.parse(fs.readFileSync(runStatePath, 'utf8')).output.value,
-    ).toBe('from-tmp')
-  })
-
-  test('flags a healthy primary as fully restored', () => {
-    writePrimary(JSON.stringify(runStateWithSession('healthy')))
-    fs.writeFileSync(messagesPath, validMessages)
-
-    const loaded = loadMostRecentChatState()
-    expect(loaded!.runStateRestored).toBe(true)
-    expect((loaded!.runState as any).sessionState).toBeDefined()
-  })
-
-  test('falls back to a context-less placeholder with the loss flagged when nothing recovers', () => {
-    writePrimary('{ torn')
-    fs.writeFileSync(messagesPath, validMessages)
-
-    const loaded = loadMostRecentChatState()
-    expect(loaded).not.toBeNull()
-    // The amnesia carrier: no sessionState — the SDK will start a fresh
-    // session next turn. runStateRestored=false is what makes the UI say so
-    // instead of the model silently forgetting every earlier turn.
-    expect((loaded!.runState as any).sessionState).toBeUndefined()
-    expect(loaded!.runStateRestored).toBe(false)
-    // The transcript still survives.
-    expect(loaded!.messages.length).toBe(1)
-  })
-
-  test('saveChatState rotates the previous primary into .bak', () => {
-    saveChatState(runStateWithSession('generation-1'), [
-      {
-        id: 'msg-1',
-        variant: 'user',
-        content: 'first',
-        timestamp: new Date().toISOString(),
-      },
-    ])
-    expect(fs.existsSync(bakPath)).toBe(false)
-
-    saveChatState(runStateWithSession('generation-2'), [
-      {
-        id: 'msg-2',
-        variant: 'user',
-        content: 'second',
-        timestamp: new Date().toISOString(),
-      },
-    ])
-
-    expect(JSON.parse(fs.readFileSync(runStatePath, 'utf8')).output.value).toBe(
-      'generation-2',
-    )
-    expect(JSON.parse(fs.readFileSync(bakPath, 'utf8')).output.value).toBe(
-      'generation-1',
-    )
-  })
-
-  test('clearChatState removes the backup too', () => {
-    writePrimary(JSON.stringify(runStateWithSession('x')))
-    fs.writeFileSync(bakPath, JSON.stringify(runStateWithSession('bak')))
-    fs.writeFileSync(messagesPath, validMessages)
-
-    clearChatState()
-    expect(fs.existsSync(runStatePath)).toBe(false)
-    expect(fs.existsSync(bakPath)).toBe(false)
-  })
-})

cli/src/utils/__tests__/write-file-atomic.test.ts

@@ -1,4 +1,4 @@
-import { describe, test, expect, beforeEach, afterEach } from 'bun:test'
+import { describe, test, expect, beforeEach, afterEach, spyOn } from 'bun:test'
 import * as fs from 'fs'
 import * as os from 'os'
 import * as path from 'path'
@@ -124,4 +124,75 @@ describe('writeFileAtomicAsync', () => {
     )
     expect(fs.readdirSync(tempDir)).toEqual(['out.json'])
   })
+
+  test('retries a transient Windows rename lock and succeeds', async () => {
+    const target = path.join(tempDir, 'out.json')
+    let attempts = 0
+    const realRename = fs.promises.rename.bind(fs.promises)
+    const spy = spyOn(fs.promises, 'rename').mockImplementation(
+      async (from, to) => {
+        attempts++
+        if (attempts <= 2) {
+          throw Object.assign(new Error('locked'), { code: 'EPERM' })
+        }
+        return realRename(from, to)
+      },
+    )
+    try {
+      await writeFileAtomicAsync(target, 'recovered')
+    } finally {
+      spy.mockRestore()
+    }
+
+    expect(attempts).toBe(3)
+    expect(fs.readFileSync(target, 'utf8')).toBe('recovered')
+  })
+
+  test('rethrows a non-transient rename error immediately', async () => {
+    const target = path.join(tempDir, 'out.json')
+    let attempts = 0
+    const spy = spyOn(fs.promises, 'rename').mockImplementation(async () => {
+      attempts++
+      throw Object.assign(new Error('missing'), { code: 'ENOENT' })
+    })
+    try {
+      await expect(writeFileAtomicAsync(target, 'data')).rejects.toThrow()
+    } finally {
+      spy.mockRestore()
+    }
+
+    expect(attempts).toBe(1)
+  })
+})
+
+describe('writeFileAtomic durability ordering', () => {
+  beforeEach(() => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codebuff-atomic-'))
+  })
+
+  afterEach(() => {
+    fs.rmSync(tempDir, { recursive: true, force: true })
+  })
+
+  test('fsyncs the temp before the rename goes live', () => {
+    const target = path.join(tempDir, 'out.json')
+    const order: string[] = []
+    const fsyncSpy = spyOn(fs, 'fsyncSync').mockImplementation(() => {
+      order.push('fsync')
+    })
+    const realRename = fs.renameSync.bind(fs)
+    const renameSpy = spyOn(fs, 'renameSync').mockImplementation((from, to) => {
+      order.push('rename')
+      return realRename(from, to)
+    })
+    try {
+      writeFileAtomic(target, '{"a":1}')
+    } finally {
+      fsyncSpy.mockRestore()
+      renameSpy.mockRestore()
+    }
+
+    expect(order).toEqual(['fsync', 'rename'])
+    expect(fs.readFileSync(target, 'utf8')).toBe('{"a":1}')
+  })
 })