From 5b86b0b50d6f8075c2ed99649e999ea24cc83e10 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Wed, 29 Apr 2026 11:42:37 +0200 Subject: [PATCH] fix: validate active note cache refresh --- src/hooks/useNoteActions.hook.test.ts | 11 +++++--- src/hooks/useNoteActions.ts | 4 ++- src/hooks/useTabManagement.test.ts | 39 ++++++++++++++++++++++----- src/hooks/useTabManagement.ts | 20 +++++++++++--- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/hooks/useNoteActions.hook.test.ts b/src/hooks/useNoteActions.hook.test.ts index 4be507f4..307fe045 100644 --- a/src/hooks/useNoteActions.hook.test.ts +++ b/src/hooks/useNoteActions.hook.test.ts @@ -497,10 +497,14 @@ describe('useNoteActions hook', () => { }) describe('note open is read-only', () => { - it('does not sync title or reload entry when opening or reopening a note', async () => { + it('does not sync title or reload entry when opening or freshness-validating a note', async () => { vi.mocked(isTauri).mockReturnValue(true) const entry = makeEntry({ path: '/test/vault/qa-test.md', filename: 'qa-test.md', title: 'Qa Test' }) - vi.mocked(invoke).mockResolvedValueOnce('# Qa Test\n') + vi.mocked(invoke).mockImplementation(async (command) => { + if (command === 'validate_note_content') return true + if (command === 'get_note_content') return '# Qa Test\n' + return null + }) const { result } = renderHook(() => useNoteActions(makeConfig([entry]))) @@ -510,9 +514,10 @@ describe('useNoteActions hook', () => { const desyncedEntry = { ...entry, title: 'Wrong Title Desynced' } await act(async () => { await result.current.handleSelectNote(desyncedEntry) }) - expect(vi.mocked(invoke)).toHaveBeenCalledTimes(callCountAfterFirstOpen) + expect(vi.mocked(invoke)).toHaveBeenCalledTimes(callCountAfterFirstOpen + 1) expect(vi.mocked(invoke).mock.calls).toEqual([ ['get_note_content', { path: '/test/vault/qa-test.md' }], + ['validate_note_content', { path: '/test/vault/qa-test.md', content: '# Qa Test\n' }], ]) expect(result.current.tabs[0].entry.title).toBe('Qa Test') }) diff --git a/src/hooks/useNoteActions.ts b/src/hooks/useNoteActions.ts index 277023e8..bc6de776 100644 --- a/src/hooks/useNoteActions.ts +++ b/src/hooks/useNoteActions.ts @@ -194,13 +194,15 @@ async function updateFrontmatterAndMaybeRename({ } function buildTabManagementOptions( - config: Pick, + config: Pick, ) { const options: { beforeNavigate?: (fromPath: string, toPath: string) => Promise + hasUnsavedChanges: (path: string) => boolean onMissingNotePath: (entry: VaultEntry) => void onUnreadableNoteContent: (entry: VaultEntry) => void } = { + hasUnsavedChanges: (path) => config.unsavedPaths?.has(path) ?? false, onMissingNotePath: (entry) => { const label = entry.title.trim() || entry.filename config.setToastMessage(`"${label}" could not be opened because its file is missing or moved.`) diff --git a/src/hooks/useTabManagement.test.ts b/src/hooks/useTabManagement.test.ts index 8803b343..4637b90f 100644 --- a/src/hooks/useTabManagement.test.ts +++ b/src/hooks/useTabManagement.test.ts @@ -161,15 +161,24 @@ describe('useTabManagement (single-note model)', () => { expectSingleActiveTab(result, '/vault/b.md') }) - it('is a no-op when selecting the already-open note', async () => { - const { result } = renderHook(() => useTabManagement()) + it('keeps a dirty already-open note in place when selecting it again', async () => { const entry = { path: '/vault/a.md' } - await selectNote(result, entry) - await act(async () => { - await result.current.handleSelectNote(makeEntry(entry)) + const { result: dirtyResult } = renderHook(() => useTabManagement({ + hasUnsavedChanges: (path) => path === '/vault/a.md', + })) + await selectNote(dirtyResult, entry) + act(() => { + dirtyResult.current.setTabs(prev => prev.map(tab => + tab.entry.path === entry.path ? { ...tab, content: '# Local draft' } : tab + )) }) - expect(result.current.tabs).toHaveLength(1) + await act(async () => { + await dirtyResult.current.handleSelectNote(makeEntry(entry)) + }) + + expect(dirtyResult.current.tabs).toHaveLength(1) + expect(dirtyResult.current.tabs[0].content).toBe('# Local draft') }) it('handles load content failure gracefully', async () => { @@ -605,6 +614,24 @@ describe('useTabManagement (single-note model)', () => { expect(vi.mocked(mockInvoke)).toHaveBeenCalledTimes(3) }) + it('refreshes an already-open clean note when cached content is stale on disk', async () => { + mockNoteContent({ '/vault/a.md': '# Original content' }) + + const { result } = renderHook(() => useTabManagement()) + await selectNote(result, { path: '/vault/a.md', title: 'A' }) + + mockNoteContent({ '/vault/a.md': '# External edit' }) + await selectNote(result, { path: '/vault/a.md', title: 'A' }) + + expect(result.current.tabs[0].entry.path).toBe('/vault/a.md') + expect(result.current.tabs[0].content).toBe('# External edit') + expect(vi.mocked(mockInvoke)).toHaveBeenCalledTimes(3) + expect(vi.mocked(mockInvoke)).toHaveBeenNthCalledWith(2, 'validate_note_content', { + path: '/vault/a.md', + content: '# Original content', + }) + }) + it('falls back instead of reopening cached content when the note file disappeared', async () => { vi.mocked(mockInvoke) .mockResolvedValueOnce('# Other note') diff --git a/src/hooks/useTabManagement.ts b/src/hooks/useTabManagement.ts index da26795f..33ebbccc 100644 --- a/src/hooks/useTabManagement.ts +++ b/src/hooks/useTabManagement.ts @@ -224,6 +224,7 @@ export type { Tab } interface TabManagementOptions { beforeNavigate?: (fromPath: string, toPath: string) => Promise + hasUnsavedChanges?: (path: string) => boolean onMissingNotePath?: (entry: VaultEntry, error: unknown) => void | Promise onUnreadableNoteContent?: (entry: VaultEntry, error: unknown) => void | Promise } @@ -236,6 +237,7 @@ interface NavigateToEntryOptions { activeTabPathRef: React.MutableRefObject setTabs: React.Dispatch> setActiveTabPath: React.Dispatch> + hasUnsavedChanges?: (path: string) => boolean onMissingNotePath?: (entry: VaultEntry, error: unknown) => void | Promise onUnreadableNoteContent?: (entry: VaultEntry, error: unknown) => void | Promise } @@ -363,6 +365,7 @@ function isUnreadableNoteContentError(error: unknown): boolean { function shouldApplyLoadedEntry(options: { seq: number navSeqRef: React.MutableRefObject + content: string forceReload: boolean activeTabPathRef: React.MutableRefObject tabsRef: React.MutableRefObject @@ -371,6 +374,7 @@ function shouldApplyLoadedEntry(options: { const { seq, navSeqRef, + content, forceReload, activeTabPathRef, tabsRef, @@ -380,7 +384,8 @@ function shouldApplyLoadedEntry(options: { if (navSeqRef.current !== seq) return false if (forceReload) return true if (!pathsMatch(activeTabPathRef.current, path)) return true - return !tabsRef.current.some((tab) => pathsMatch(tab.entry.path, path)) + const openTab = tabsRef.current.find((tab) => pathsMatch(tab.entry.path, path)) + return !openTab || openTab.content !== content } type EntryLoadFailureKind = @@ -529,8 +534,10 @@ function reopenAlreadyViewingEntry({ tabsRef, activeTabPathRef, setActiveTabPath, -}: Pick): boolean { + hasUnsavedChanges, +}: Pick): boolean { if (!isAlreadyViewingPath(tabsRef, activeTabPathRef, entry.path)) return false + if (!hasUnsavedChanges?.(entry.path)) return false syncActiveTabPath(activeTabPathRef, setActiveTabPath, entry.path) finishNoteOpenTrace(entry.path) return true @@ -567,6 +574,7 @@ async function loadTextEntry(options: Required { - if (!pathsMatch(entry.path, activeTabPathRef.current)) { + const alreadyViewingDirtyEntry = pathsMatch(entry.path, activeTabPathRef.current) + && !!hasUnsavedChanges?.(entry.path) + if (!alreadyViewingDirtyEntry) { beginNoteOpenTrace(entry.path, 'select-note') } await executeNavigationWithBoundary(entry.path, () => navigateToEntry({ @@ -651,10 +662,11 @@ export function useTabManagement(options: TabManagementOptions = {}) { activeTabPathRef, setTabs, setActiveTabPath, + hasUnsavedChanges, onMissingNotePath, onUnreadableNoteContent, })) - }, [executeNavigationWithBoundary, onMissingNotePath, onUnreadableNoteContent]) + }, [executeNavigationWithBoundary, hasUnsavedChanges, onMissingNotePath, onUnreadableNoteContent]) const handleSwitchTab = useCallback((path: string) => { syncActiveTabPath(activeTabPathRef, setActiveTabPath, path)