From 9199ceaa358b46059b0d849650c23cd08ca0eb0b Mon Sep 17 00:00:00 2001 From: Test Date: Wed, 11 Mar 2026 17:37:53 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20prevent=20data=20corruption=20when=20cha?= =?UTF-8?q?nging=20note=20type=20=E2=80=94=20preserve=20tab=20content=20in?= =?UTF-8?q?stead=20of=20re-reading=20from=20disk?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After runFrontmatterOp updates the frontmatter and sets the tab content, move_note_to_type_folder only changes the file location (not its content). Re-reading via loadNoteContent(result.new_path) was redundant and dangerous: if the path collided or a stale cache intervened, it could load a different note's content into the tab — the root cause of the data-corruption bug. Also fixes stale-closure issue: replaceEntry no longer spreads the captured `entries` array (which could be stale after the await), avoiding reverting the isA field that runFrontmatterOp already updated. Co-Authored-By: Claude Opus 4.6 --- src/hooks/useNoteActions.test.ts | 36 ++++++++++++++++++++++++++++++++ src/hooks/useNoteActions.ts | 25 ++++++++++++---------- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/src/hooks/useNoteActions.test.ts b/src/hooks/useNoteActions.test.ts index 092b441b..4341bac6 100644 --- a/src/hooks/useNoteActions.test.ts +++ b/src/hooks/useNoteActions.test.ts @@ -988,6 +988,42 @@ describe('useNoteActions hook', () => { })) }) + it('preserves note content after type change — never loads another note (regression)', async () => { + // The mock updateMockFrontmatter returns '---\nupdated: true\n---\n' — + // this represents the note's own content after the frontmatter update. + const frontmatterUpdatedContent = '---\nupdated: true\n---\n' + const wrongContent = '---\ntype: Project\n---\n# Feedback for Laputa\n\nCompletely different note.\n' + + const entry = makeEntry({ path: '/test/vault/note/migrate-newsletter.md', filename: 'migrate-newsletter.md', title: 'Migrate newsletter to Beehiiv', isA: 'Note' }) + const replaceEntry = vi.fn() + const config = makeConfig([entry]) + config.replaceEntry = replaceEntry + + vi.mocked(mockInvoke).mockImplementation(async (cmd: string) => { + if (cmd === 'move_note_to_type_folder') return { new_path: '/test/vault/project/migrate-newsletter.md', updated_links: 0, moved: true } + // Simulate the bug: get_note_content returns a DIFFERENT note's content + // (e.g. path collision, stale cache, or filesystem race) + if (cmd === 'get_note_content') return wrongContent + return '' + }) + + const { result } = renderHook(() => useNoteActions(config)) + + // Open the tab first so we have a tab to check + act(() => { result.current.openTabWithContent(entry, '---\ntype: Note\n---\n# Migrate\n') }) + + await act(async () => { + await result.current.handleUpdateFrontmatter('/test/vault/note/migrate-newsletter.md', 'type', 'Project') + }) + + // The tab content must be the note's OWN content (from the frontmatter update), + // NEVER the content of a different note loaded via get_note_content. + const tab = result.current.tabs.find(t => t.entry.path === '/test/vault/project/migrate-newsletter.md') + expect(tab).toBeDefined() + expect(tab!.content).toBe(frontmatterUpdatedContent) + expect(tab!.content).not.toBe(wrongContent) + }) + it('does not move when value is empty or null-like', async () => { const config = makeConfig() vi.mocked(mockInvoke).mockResolvedValue('') diff --git a/src/hooks/useNoteActions.ts b/src/hooks/useNoteActions.ts index 5fb610d7..fffa9719 100644 --- a/src/hooks/useNoteActions.ts +++ b/src/hooks/useNoteActions.ts @@ -476,16 +476,19 @@ export function useNoteActions(config: NoteActionsConfig) { try { const result = await performMoveToTypeFolder(config.vaultPath, path, value) if (result.moved) { - const entry = entries.find(e => e.path === path) - if (entry) { - const newFilename = result.new_path.split('/').pop() ?? entry.filename - const newContent = await loadNoteContent(result.new_path) - config.replaceEntry?.(path, { ...entry, path: result.new_path, filename: newFilename }) - setTabs(prev => prev.map(t => t.entry.path === path - ? { entry: { ...t.entry, path: result.new_path, filename: newFilename }, content: newContent } - : t)) - if (activeTabPathRef.current === path) handleSwitchTab(result.new_path) - } + const newFilename = result.new_path.split('/').pop() ?? '' + // Update the vault entry with the new path. Only pass the changed + // fields — avoid spreading a stale closure entry which would revert + // the isA update that runFrontmatterOp already applied. + config.replaceEntry?.(path, { path: result.new_path, filename: newFilename } as Partial & { path: string }) + // Preserve the tab content already set by runFrontmatterOp. + // Re-reading from disk via loadNoteContent is unnecessary (the move + // does not change content) and dangerous: if the path collides or a + // stale cache intervenes it could return a different note's content. + setTabs(prev => prev.map(t => t.entry.path === path + ? { entry: { ...t.entry, path: result.new_path, filename: newFilename }, content: t.content } + : t)) + if (activeTabPathRef.current === path) handleSwitchTab(result.new_path) const folder = result.new_path.split('/').slice(-2, -1)[0] ?? '' setToastMessage(`Note moved to ${folder}/`) } @@ -493,7 +496,7 @@ export function useNoteActions(config: NoteActionsConfig) { console.error('Failed to move note to type folder:', err) } } - }, [runFrontmatterOp, config.vaultPath, config.replaceEntry, entries, setTabs, activeTabPathRef, handleSwitchTab, setToastMessage]), + }, [runFrontmatterOp, config.vaultPath, config.replaceEntry, setTabs, activeTabPathRef, handleSwitchTab, setToastMessage]), handleDeleteProperty: useCallback((path: string, key: string) => runFrontmatterOp('delete', path, key), [runFrontmatterOp]), handleAddProperty: useCallback((path: string, key: string, value: FrontmatterValue) => runFrontmatterOp('update', path, key, value), [runFrontmatterOp]), handleRenameNote,