feat: add optimistic error recovery for note creation (#69)

Note creation was already optimistic (UI updates before disk write) but
errors were silently swallowed. Now if the disk write fails, the
optimistic entry is reverted (tab closed, entry removed) and the user
sees an error toast. Each note creation is independent, so one failure
in rapid Cmd+N presses doesn't affect the others.

- Add removeEntry to useVaultLoader for reverting optimistic adds
- Refactor persistNewNote to return a Promise for error handling
- Extract navigateWikilink, persistOptimistic, createAndPersist,
  and runFrontmatterAndApply helpers to reduce hook complexity
- Add tests for error recovery: single note, type, success path,
  and rapid creation with partial failure
- Code health: 8.93→9.01 (CC fixed: 10→7, primitive obsession improved)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

src/App.tsx

@@ -67,7 +67,7 @@ function App() {
 
   useEffect(() => { setApiKey(settings.anthropic_key ?? '') }, [settings.anthropic_key])
 
-  const notes = useNoteActions({ addEntry: vault.addEntry, updateContent: vault.updateContent, entries: vault.entries, setToastMessage, updateEntry: vault.updateEntry })
+  const notes = useNoteActions({ addEntry: vault.addEntry, removeEntry: vault.removeEntry, updateContent: vault.updateContent, entries: vault.entries, setToastMessage, updateEntry: vault.updateEntry })
 
   const { handleSave, handleContentChange, savePendingForPath, savePending } = useEditorSave({
     updateVaultContent: vault.updateContent,

src/hooks/useNoteActions.test.ts

@@ -1,5 +1,7 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest'
 import { renderHook, act } from '@testing-library/react'
+import { invoke } from '@tauri-apps/api/core'
+import { isTauri } from '../mock-tauri'
 import type { VaultEntry } from '../types'
 import {
   slugify,
@@ -17,7 +19,7 @@ import type { NoteActionsConfig } from './useNoteActions'
 // Mock dependencies
 vi.mock('@tauri-apps/api/core', () => ({ invoke: vi.fn() }))
 vi.mock('../mock-tauri', () => ({
-  isTauri: () => false,
+  isTauri: vi.fn(() => false),
   addMockEntry: vi.fn(),
   updateMockContent: vi.fn(),
   mockInvoke: vi.fn().mockResolvedValue(''),
@@ -280,16 +282,18 @@ describe('frontmatterToEntryPatch', () => {
 
 describe('useNoteActions hook', () => {
   const addEntry = vi.fn()
+  const removeEntry = vi.fn()
   const updateContent = vi.fn()
   const updateEntry = vi.fn()
   const setToastMessage = vi.fn()
 
   const makeConfig = (entries: VaultEntry[] = []): NoteActionsConfig => ({
-    addEntry, updateContent, entries, setToastMessage, updateEntry,
+    addEntry, removeEntry, updateContent, entries, setToastMessage, updateEntry,
   })
 
   beforeEach(() => {
     vi.clearAllMocks()
+    vi.mocked(isTauri).mockReturnValue(false)
   })
 
   it('handleCreateNote calls addEntry and creates correct entry', () => {
@@ -436,4 +440,61 @@ describe('useNoteActions hook', () => {
     expect(updateEntry).not.toHaveBeenCalled()
     expect(setToastMessage).toHaveBeenCalledWith('Property updated')
   })
+
+  describe('optimistic error recovery (Tauri mode)', () => {
+    beforeEach(() => {
+      vi.mocked(isTauri).mockReturnValue(true)
+    })
+
+    it.each([
+      ['handleCreateNote', 'Failing Note', 'Note', 'note/failing-note.md'],
+      ['handleCreateType', 'Recipe', 'Type', 'type/recipe.md'],
+    ])('reverts optimistic creation via %s when disk write fails', async (method, title, type, pathFragment) => {
+      vi.mocked(invoke).mockRejectedValueOnce(new Error('disk full'))
+      const { result } = renderHook(() => useNoteActions(makeConfig()))
+
+      await act(async () => {
+        if (method === 'handleCreateNote') result.current.handleCreateNote(title, type)
+        else result.current.handleCreateType(title)
+        await new Promise((r) => setTimeout(r, 0))
+      })
+
+      expect(addEntry).toHaveBeenCalledTimes(1)
+      expect(removeEntry).toHaveBeenCalledWith(expect.stringContaining(pathFragment))
+      expect(setToastMessage).toHaveBeenCalledWith('Failed to create note — disk write error')
+    })
+
+    it('does not revert when disk write succeeds', async () => {
+      vi.mocked(invoke).mockResolvedValueOnce(undefined)
+      const { result } = renderHook(() => useNoteActions(makeConfig()))
+
+      await act(async () => {
+        result.current.handleCreateNote('Good Note', 'Note')
+        await new Promise((r) => setTimeout(r, 0))
+      })
+
+      expect(removeEntry).not.toHaveBeenCalled()
+      expect(setToastMessage).not.toHaveBeenCalled()
+    })
+
+    it('handles rapid creation with one failure independently', async () => {
+      vi.mocked(invoke)
+        .mockResolvedValueOnce(undefined)
+        .mockRejectedValueOnce(new Error('disk full'))
+        .mockResolvedValueOnce(undefined)
+
+      const { result } = renderHook(() => useNoteActions(makeConfig()))
+
+      await act(async () => {
+        result.current.handleCreateNoteImmediate()
+        result.current.handleCreateNoteImmediate()
+        result.current.handleCreateNoteImmediate()
+        await new Promise((r) => setTimeout(r, 0))
+      })
+
+      expect(addEntry).toHaveBeenCalledTimes(3)
+      expect(removeEntry).toHaveBeenCalledTimes(1)
+      expect(removeEntry).toHaveBeenCalledWith(expect.stringContaining('untitled-note-2.md'))
+    })
+  })
 })

src/hooks/useNoteActions.ts

@@ -21,6 +21,7 @@ interface RenameResult {
 
 export interface NoteActionsConfig {
   addEntry: (entry: VaultEntry, content: string) => void
+  removeEntry: (path: string) => void
   updateContent: (path: string, content: string) => void
   entries: VaultEntry[]
   setToastMessage: (msg: string | null) => void
@@ -49,13 +50,10 @@ async function loadNoteContent(path: string): Promise<string> {
     : mockInvoke<string>('get_note_content', { path })
 }
 
-/** Fire-and-forget: persist a newly created note to disk. */
-function persistNewNote(path: string, content: string): void {
-  if (isTauri()) {
-    invoke('save_note_content', { path, content }).catch((err) =>
-      console.error('Failed to persist new note:', err),
-    )
-  }
+/** Persist a newly created note to disk. Returns a Promise for error handling. */
+function persistNewNote(path: string, content: string): Promise<void> {
+  if (!isTauri()) return Promise.resolve()
+  return invoke<void>('save_note_content', { path, content }).then(() => {})
 }
 
 export function buildNewEntry({ path, slug, title, type, status }: NewEntryParams): VaultEntry {
@@ -178,6 +176,30 @@ function findWikilinkTarget(entries: VaultEntry[], target: string): VaultEntry |
   return entries.find((e) => entryMatchesTarget(e, targetLower, targetAsWords))
 }
 
+/** Navigate to a wikilink target, logging a warning if not found. */
+function navigateWikilink(entries: VaultEntry[], target: string, selectNote: (e: VaultEntry) => void): void {
+  const found = findWikilinkTarget(entries, target)
+  if (found) selectNote(found)
+  else console.warn(`Navigation target not found: ${target}`)
+}
+
+/** Persist to disk; on failure, call the revert handler. */
+function persistOptimistic(path: string, content: string, onFail: (p: string) => void): void {
+  persistNewNote(path, content).catch(() => onFail(path))
+}
+
+/** Optimistically add entry to UI, open tab, and persist to disk. */
+function createAndPersist(
+  resolved: { entry: VaultEntry; content: string },
+  addFn: (e: VaultEntry, c: string) => void,
+  openTab: (e: VaultEntry, c: string) => void,
+  onFail: (p: string) => void,
+): void {
+  addEntryWithMock(resolved.entry, resolved.content, addFn)
+  openTab(resolved.entry, resolved.content)
+  persistOptimistic(resolved.entry.path, resolved.content, onFail)
+}
+
 async function executeFrontmatterOp(op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue): Promise<string> {
   if (op === 'update') {
     return isTauri() ? invokeFrontmatter('update_frontmatter', { path, key, value }) : applyMockFrontmatterUpdate(path, key, value!)
@@ -202,10 +224,26 @@ async function reloadTabsAfterRename(
   }
 }
 
+/** Run a frontmatter update/delete and apply the result to state. */
+async function runFrontmatterAndApply(
+  op: 'update' | 'delete', path: string, key: string, value: FrontmatterValue | undefined,
+  callbacks: { updateTab: (p: string, c: string) => void; updateEntry: (p: string, patch: Partial<VaultEntry>) => void; toast: (m: string | null) => void },
+): Promise<void> {
+  try {
+    callbacks.updateTab(path, await executeFrontmatterOp(op, path, key, value))
+    const patch = frontmatterToEntryPatch(op, key, value)
+    if (Object.keys(patch).length > 0) callbacks.updateEntry(path, patch)
+    callbacks.toast(op === 'update' ? 'Property updated' : 'Property deleted')
+  } catch (err) {
+    console.error(`Failed to ${op} frontmatter:`, err)
+    callbacks.toast(`Failed to ${op} property`)
+  }
+}
+
 export function useNoteActions(config: NoteActionsConfig) {
-  const { addEntry, updateContent, entries, setToastMessage, updateEntry } = config
+  const { addEntry, removeEntry, updateContent, entries, setToastMessage, updateEntry } = config
   const tabMgmt = useTabManagement()
-  const { setTabs, handleSelectNote, openTabWithContent, activeTabPathRef, handleSwitchTab } = tabMgmt
+  const { setTabs, handleSelectNote, openTabWithContent, handleCloseTab, activeTabPathRef, handleSwitchTab } = tabMgmt
   const tabsRef = useRef(tabMgmt.tabs)
   // eslint-disable-next-line react-hooks/refs
   tabsRef.current = tabMgmt.tabs
@@ -215,22 +253,23 @@ export function useNoteActions(config: NoteActionsConfig) {
     updateContent(path, newContent)
   }, [setTabs, updateContent])
 
-  const handleNavigateWikilink = useCallback((target: string) => {
-    const found = findWikilinkTarget(entries, target)
-    if (found) handleSelectNote(found)
-    else console.warn(`Navigation target not found: ${target}`)
-  }, [entries, handleSelectNote])
+  const handleNavigateWikilink = useCallback(
+    (target: string) => navigateWikilink(entries, target, handleSelectNote),
+    [entries, handleSelectNote],
+  )
+
+  const revertOptimisticNote = useCallback((path: string) => {
+    handleCloseTab(path)
+    removeEntry(path)
+    setToastMessage('Failed to create note — disk write error')
+  }, [handleCloseTab, removeEntry, setToastMessage])
 
   const pendingNamesRef = useRef<Set<string>>(new Set())
 
   const handleCreateNote = useCallback((title: string, type: string) => {
-    const { entry, content } = resolveNewNote(title, type)
-    addEntryWithMock(entry, content, addEntry)
-    openTabWithContent(entry, content)
-    persistNewNote(entry.path, content)
-  }, [openTabWithContent, addEntry])
+    createAndPersist(resolveNewNote(title, type), addEntry, openTabWithContent, revertOptimisticNote)
+  }, [openTabWithContent, addEntry, revertOptimisticNote])
 
-  /** Create a note immediately with an auto-generated unique title. Dedup-safe for rapid calls. */
   const handleCreateNoteImmediate = useCallback((type?: string) => {
     const noteType = type || 'Note'
     const title = generateUntitledName(entries, noteType, pendingNamesRef.current)
@@ -241,28 +280,19 @@ export function useNoteActions(config: NoteActionsConfig) {
   }, [entries, handleCreateNote])
 
   const handleCreateType = useCallback((typeName: string) => {
-    const { entry, content } = resolveNewType(typeName)
-    addEntryWithMock(entry, content, addEntry)
-    openTabWithContent(entry, content)
-    persistNewNote(entry.path, content)
-  }, [openTabWithContent, addEntry])
+    createAndPersist(resolveNewType(typeName), addEntry, openTabWithContent, revertOptimisticNote)
+  }, [openTabWithContent, addEntry, revertOptimisticNote])
 
-  const runFrontmatterOp = useCallback(async (op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue) => {
-    try {
-      updateTabContent(path, await executeFrontmatterOp(op, path, key, value))
-      const patch = frontmatterToEntryPatch(op, key, value)
-      if (Object.keys(patch).length > 0) updateEntry(path, patch)
-      setToastMessage(op === 'update' ? 'Property updated' : 'Property deleted')
-    } catch (err) {
-      console.error(`Failed to ${op} frontmatter:`, err)
-      setToastMessage(`Failed to ${op} property`)
-    }
-  }, [updateTabContent, updateEntry, setToastMessage])
+  const fmCallbacks = { updateTab: updateTabContent, updateEntry, toast: setToastMessage }
+
+  const runFrontmatterOp = useCallback(
+    (op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue) =>
+      runFrontmatterAndApply(op, path, key, value, fmCallbacks),
+    [updateTabContent, updateEntry, setToastMessage], // eslint-disable-line react-hooks/exhaustive-deps -- fmCallbacks is stable when deps are
+  )
 
   const handleRenameNote = useCallback(async (
-    path: string,
-    newTitle: string,
-    vaultPath: string,
+    path: string, newTitle: string, vaultPath: string,
     onEntryRenamed: (oldPath: string, newEntry: Partial<VaultEntry> & { path: string }, newContent: string) => void,
   ) => {
     try {
@@ -270,15 +300,10 @@ export function useNoteActions(config: NoteActionsConfig) {
       const newContent = await loadNoteContent(result.new_path)
       const entry = entries.find((e) => e.path === path)
       const newEntry = buildRenamedEntry(entry ?? {} as VaultEntry, newTitle, result.new_path)
-
-      const otherTabPaths = tabsRef.current
-        .filter(t => t.entry.path !== path)
-        .map(t => t.entry.path)
-
+      const otherTabPaths = tabsRef.current.filter(t => t.entry.path !== path).map(t => t.entry.path)
       setTabs((prev) => prev.map((t) => t.entry.path === path ? { entry: newEntry, content: newContent } : t))
       if (activeTabPathRef.current === path) handleSwitchTab(result.new_path)
       onEntryRenamed(path, newEntry, newContent)
-
       await reloadTabsAfterRename(otherTabPaths, updateTabContent)
       setToastMessage(renameToastMessage(result.updated_files))
     } catch (err) {

src/hooks/useVaultLoader.test.ts

@@ -49,17 +49,20 @@ vi.mock('../mock-tauri', () => ({
   mockInvoke: (cmd: string, args?: Record<string, unknown>) => mockInvokeFn(cmd, args),
 }))
 
+/** Render the vault loader hook and wait for initial data to load. */
+async function renderVaultLoader() {
+  const hook = renderHook(() => useVaultLoader('/vault'))
+  await waitFor(() => { expect(hook.result.current.entries).toHaveLength(1) })
+  return hook
+}
+
 describe('useVaultLoader', () => {
   beforeEach(() => {
     mockInvokeFn.mockImplementation(defaultMockInvoke)
   })
 
   it('loads entries and content on mount', async () => {
-    const { result } = renderHook(() => useVaultLoader('/vault'))
-
-    await waitFor(() => {
-      expect(result.current.entries).toHaveLength(1)
-    })
+    const { result } = await renderVaultLoader()
 
     expect(result.current.entries[0].title).toBe('Hello')
     expect(result.current.allContent['/vault/note/hello.md']).toContain('# Hello')
@@ -77,22 +80,10 @@ describe('useVaultLoader', () => {
 
   describe('addEntry', () => {
     it('prepends new entry and adds content', async () => {
-      const { result } = renderHook(() => useVaultLoader('/vault'))
+      const { result } = await renderVaultLoader()
+      const newEntry: VaultEntry = { ...mockEntries[0], path: '/vault/note/new.md', filename: 'new.md', title: 'New Note' }
 
-      await waitFor(() => {
-        expect(result.current.entries).toHaveLength(1)
-      })
-
-      const newEntry: VaultEntry = {
-        ...mockEntries[0],
-        path: '/vault/note/new.md',
-        filename: 'new.md',
-        title: 'New Note',
-      }
-
-      act(() => {
-        result.current.addEntry(newEntry, '# New Note')
-      })
+      act(() => { result.current.addEntry(newEntry, '# New Note') })
 
       expect(result.current.entries).toHaveLength(2)
       expect(result.current.entries[0].title).toBe('New Note')
@@ -100,18 +91,8 @@ describe('useVaultLoader', () => {
     })
 
     it('ignores duplicate entry with same path', async () => {
-      const { result } = renderHook(() => useVaultLoader('/vault'))
-
-      await waitFor(() => {
-        expect(result.current.entries).toHaveLength(1)
-      })
-
-      const newEntry: VaultEntry = {
-        ...mockEntries[0],
-        path: '/vault/note/new.md',
-        filename: 'new.md',
-        title: 'New Note',
-      }
+      const { result } = await renderVaultLoader()
+      const newEntry: VaultEntry = { ...mockEntries[0], path: '/vault/note/new.md', filename: 'new.md', title: 'New Note' }
 
       act(() => {
         result.current.addEntry(newEntry, '# New Note')
@@ -124,31 +105,38 @@ describe('useVaultLoader', () => {
 
   describe('updateContent', () => {
     it('updates content for an existing path', async () => {
-      const { result } = renderHook(() => useVaultLoader('/vault'))
+      const { result } = await renderVaultLoader()
 
-      await waitFor(() => {
-        expect(result.current.allContent['/vault/note/hello.md']).toBeDefined()
-      })
-
-      act(() => {
-        result.current.updateContent('/vault/note/hello.md', '# Updated')
-      })
+      act(() => { result.current.updateContent('/vault/note/hello.md', '# Updated') })
 
       expect(result.current.allContent['/vault/note/hello.md']).toBe('# Updated')
     })
   })
 
+  describe('removeEntry', () => {
+    it('removes entry and content by path', async () => {
+      const { result } = await renderVaultLoader()
+
+      act(() => { result.current.removeEntry('/vault/note/hello.md') })
+
+      expect(result.current.entries).toHaveLength(0)
+      expect(result.current.allContent['/vault/note/hello.md']).toBeUndefined()
+    })
+
+    it('is a no-op for non-existent paths', async () => {
+      const { result } = await renderVaultLoader()
+
+      act(() => { result.current.removeEntry('/vault/note/nonexistent.md') })
+
+      expect(result.current.entries).toHaveLength(1)
+    })
+  })
+
   describe('updateEntry', () => {
     it('patches an existing entry by path', async () => {
-      const { result } = renderHook(() => useVaultLoader('/vault'))
+      const { result } = await renderVaultLoader()
 
-      await waitFor(() => {
-        expect(result.current.entries).toHaveLength(1)
-      })
-
-      act(() => {
-        result.current.updateEntry('/vault/note/hello.md', { archived: true, status: 'Done' })
-      })
+      act(() => { result.current.updateEntry('/vault/note/hello.md', { archived: true, status: 'Done' }) })
 
       expect(result.current.entries[0].archived).toBe(true)
       expect(result.current.entries[0].status).toBe('Done')
@@ -168,22 +156,10 @@ describe('useVaultLoader', () => {
     })
 
     it('returns new for freshly added entries', async () => {
-      const { result } = renderHook(() => useVaultLoader('/vault'))
+      const { result } = await renderVaultLoader()
+      const newEntry: VaultEntry = { ...mockEntries[0], path: '/vault/note/brand-new.md', filename: 'brand-new.md', title: 'Brand New' }
 
-      await waitFor(() => {
-        expect(result.current.entries).toHaveLength(1)
-      })
-
-      const newEntry: VaultEntry = {
-        ...mockEntries[0],
-        path: '/vault/note/brand-new.md',
-        filename: 'brand-new.md',
-        title: 'Brand New',
-      }
-
-      act(() => {
-        result.current.addEntry(newEntry, '# Brand New')
-      })
+      act(() => { result.current.addEntry(newEntry, '# Brand New') })
 
       expect(result.current.getNoteStatus('/vault/note/brand-new.md')).toBe('new')
     })

src/hooks/useVaultLoader.ts

@@ -91,6 +91,11 @@ export function useVaultLoader(vaultPath: string) {
   const updateEntry = useCallback((path: string, patch: Partial<VaultEntry>) =>
     setEntries((prev) => prev.map((e) => e.path === path ? { ...e, ...patch } : e)), [])
 
+  const removeEntry = useCallback((path: string) => {
+    setEntries((prev) => prev.filter((e) => e.path !== path))
+    setAllContent((prev) => { const next = { ...prev }; delete next[path]; return next })
+  }, [])
+
   const replaceEntry = useCallback((oldPath: string, patch: Partial<VaultEntry> & { path: string }, newContent: string) => {
     setEntries((prev) => prev.map((e) => e.path === oldPath ? { ...e, ...patch } : e))
     setAllContent((prev) => { const next = { ...prev }; delete next[oldPath]; next[patch.path] = newContent; return next })
@@ -115,7 +120,7 @@ export function useVaultLoader(vaultPath: string) {
 
   return {
     entries, allContent, modifiedFiles,
-    addEntry, updateEntry, replaceEntry, updateContent,
+    addEntry, updateEntry, removeEntry, replaceEntry, updateContent,
     loadModifiedFiles, loadGitHistory, loadDiff, loadDiffAtCommit,
     getNoteStatus, commitAndPush,
   }