From 5b8aa8da0d4a5ede8c2d01df033a87a58e6ca003 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Thu, 23 Apr 2026 19:34:15 +0200 Subject: [PATCH] fix: guard pull refresh during unsaved editor edits --- src/App.tsx | 2 +- src/hooks/useAppSave.test.ts | 16 +++ src/hooks/useAppSave.ts | 12 +- .../checklist-pull-refresh-regression.spec.ts | 121 ++++++++++++++++++ 4 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 tests/smoke/checklist-pull-refresh-regression.spec.ts diff --git a/src/App.tsx b/src/App.tsx index a14ca31d..f5004503 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -640,7 +640,7 @@ function App() { const appSave = useAppSave({ updateEntry: vault.updateEntry, setTabs: notes.setTabs, handleSwitchTab: notes.handleSwitchTab, setToastMessage, loadModifiedFiles: vault.loadModifiedFiles, reloadViews: async () => { await vault.reloadViews() }, - clearUnsaved: vault.clearUnsaved, unsavedPaths: vault.unsavedPaths, + trackUnsaved: vault.trackUnsaved, clearUnsaved: vault.clearUnsaved, unsavedPaths: vault.unsavedPaths, tabs: notes.tabs, activeTabPath: notes.activeTabPath, handleRenameNote: notes.handleRenameNote, handleRenameFilename: notes.handleRenameFilename, replaceEntry: vault.replaceEntry, resolvedPath, diff --git a/src/hooks/useAppSave.test.ts b/src/hooks/useAppSave.test.ts index 28700ca4..cd7da4af 100644 --- a/src/hooks/useAppSave.test.ts +++ b/src/hooks/useAppSave.test.ts @@ -38,6 +38,7 @@ describe('useAppSave', () => { handleSwitchTab: vi.fn(), setToastMessage: vi.fn(), loadModifiedFiles: vi.fn(), + trackUnsaved: vi.fn(), clearUnsaved: vi.fn(), unsavedPaths: new Set(), tabs: [] as Array<{ entry: VaultEntry; content: string }>, @@ -56,6 +57,7 @@ describe('useAppSave', () => { deps.unsavedPaths = new Set() deps.tabs = [] deps.activeTabPath = null + deps.trackUnsaved.mockReset() deps.handleRenameNote.mockResolvedValue(undefined) deps.handleRenameFilename.mockResolvedValue(undefined) deps.initialH1AutoRenameEnabled = true @@ -172,6 +174,20 @@ describe('useAppSave', () => { expect(typeof result.current.handleContentChange).toBe('function') }) + it('marks the edited path as unsaved immediately on content change', () => { + const entry = makeEntry('/vault/note.md', 'Note', 'note.md') + const { result } = renderSave({ + tabs: [{ entry, content: '# Note\n\nBefore' }], + activeTabPath: entry.path, + }) + + act(() => { + result.current.handleContentChange(entry.path, '# Note\n\nAfter') + }) + + expect(deps.trackUnsaved).toHaveBeenCalledWith(entry.path) + }) + it('bumps modifiedAt in live entry state after saving', async () => { vi.useFakeTimers() vi.setSystemTime(new Date('2026-04-16T14:45:00Z')) diff --git a/src/hooks/useAppSave.ts b/src/hooks/useAppSave.ts index cb59cbf3..28d63ddc 100644 --- a/src/hooks/useAppSave.ts +++ b/src/hooks/useAppSave.ts @@ -414,6 +414,7 @@ interface AppSaveDeps { setToastMessage: (msg: string | null) => void loadModifiedFiles: () => void reloadViews?: () => Promise + trackUnsaved?: (path: string) => void clearUnsaved: (path: string) => void unsavedPaths: Set tabs: TabState[] @@ -587,6 +588,7 @@ function useEditorPersistence({ setTabs, setToastMessage, loadModifiedFiles, + trackUnsaved, clearUnsaved, reloadViews, scheduleUntitledRename, @@ -597,6 +599,7 @@ function useEditorPersistence({ setTabs: AppSaveDeps['setTabs'] setToastMessage: AppSaveDeps['setToastMessage'] loadModifiedFiles: AppSaveDeps['loadModifiedFiles'] + trackUnsaved?: AppSaveDeps['trackUnsaved'] clearUnsaved: AppSaveDeps['clearUnsaved'] reloadViews: AppSaveDeps['reloadViews'] scheduleUntitledRename: (path: string, content: string) => void @@ -629,8 +632,10 @@ function useEditorPersistence({ }) const handleContentChange = useCallback((path: string, content: string) => { - handleContentChangeRaw(resolveCurrentPath(path), content) - }, [handleContentChangeRaw, resolveCurrentPath]) + const resolvedPath = resolveCurrentPath(path) + trackUnsaved?.(resolvedPath) + handleContentChangeRaw(resolvedPath, content) + }, [handleContentChangeRaw, resolveCurrentPath, trackUnsaved]) const savePendingForPath = useCallback((path: string) => ( savePendingForPathRaw(resolveCurrentPath(path)) @@ -736,7 +741,7 @@ function useAppSaveHandlers({ export function useAppSave({ updateEntry, setTabs, handleSwitchTab, setToastMessage, loadModifiedFiles, reloadViews, - clearUnsaved, unsavedPaths, tabs, activeTabPath, handleRenameNote, + trackUnsaved, clearUnsaved, unsavedPaths, tabs, activeTabPath, handleRenameNote, handleRenameFilename: handleRenameFilenameRaw, replaceEntry, resolvedPath, initialH1AutoRenameEnabled, }: AppSaveDeps) { @@ -760,6 +765,7 @@ export function useAppSave({ setTabs, setToastMessage, loadModifiedFiles, + trackUnsaved, clearUnsaved, reloadViews, scheduleUntitledRename, diff --git a/tests/smoke/checklist-pull-refresh-regression.spec.ts b/tests/smoke/checklist-pull-refresh-regression.spec.ts new file mode 100644 index 00000000..2379efba --- /dev/null +++ b/tests/smoke/checklist-pull-refresh-regression.spec.ts @@ -0,0 +1,121 @@ +import fs from 'fs' +import path from 'path' +import { test, expect, type Page } from '@playwright/test' +import { + createFixtureVaultCopy, + openFixtureVaultDesktopHarness, + removeFixtureVaultCopy, +} from '../helpers/fixtureVault' +import { executeCommand, openCommandPalette } from './helpers' + +let tempVaultDir: string + +function trackUnexpectedErrors(page: Page): string[] { + const errors: string[] = [] + + page.on('pageerror', (error) => { + errors.push(error.message) + }) + + page.on('console', (message) => { + if (message.type() !== 'error') return + + const text = message.text() + if (text.includes('ws://localhost:9711')) return + errors.push(text) + }) + + return errors +} + +function notePath(vaultPath: string): string { + return path.join(vaultPath, 'note', 'note-b.md') +} + +function writeChecklistNote(vaultPath: string, marker: string, checked = false): void { + const checkbox = checked ? 'x' : ' ' + + fs.writeFileSync(notePath(vaultPath), `--- +Is A: Note +Status: Active +--- + +# Note B + +- [${checkbox}] Toggle me +- [ ] Keep me + +${marker} +`, 'utf8') +} + +async function openNote(page: Page, title: string) { + await page.getByTestId('note-list-container').getByText(title, { exact: true }).click() + await expect(page.locator('.bn-editor')).toBeVisible({ timeout: 5_000 }) +} + +async function pullFromRemote(page: Page) { + await openCommandPalette(page) + await executeCommand(page, 'Pull from Remote') +} + +async function stubUpdatedPull(page: Page, updatedFile: string) { + await page.evaluate((filePath) => { + window.__mockHandlers!.git_pull = () => ({ + status: 'updated', + message: 'Pulled 1 update from remote', + updatedFiles: [filePath], + conflictFiles: [], + }) + }, updatedFile) +} + +function checklistCheckbox(page: Page, index: number) { + return page.locator('.bn-block-content[data-content-type="checkListItem"] input[type="checkbox"]').nth(index) +} + +test.describe('checklist pull refresh regression', () => { + test.beforeEach(({ page }, testInfo) => { + void page + testInfo.setTimeout(60_000) + tempVaultDir = createFixtureVaultCopy() + writeChecklistNote(tempVaultDir, 'Initial checklist body.') + }) + + test.afterEach(() => { + removeFixtureVaultCopy(tempVaultDir) + }) + + test('pull does not replace an unsaved checklist note in place', async ({ page }) => { + const errors = trackUnexpectedErrors(page) + const pulledMarker = `Pulled checklist refresh ${Date.now()}` + const filePath = notePath(tempVaultDir) + + await openFixtureVaultDesktopHarness(page, tempVaultDir) + await openNote(page, 'Note B') + + const firstCheckbox = checklistCheckbox(page, 0) + await expect(firstCheckbox).toBeVisible({ timeout: 5_000 }) + await expect(firstCheckbox).not.toBeChecked() + + await firstCheckbox.click() + await expect(firstCheckbox).toBeChecked() + + writeChecklistNote(tempVaultDir, pulledMarker, true) + await stubUpdatedPull(page, filePath) + await pullFromRemote(page) + + await expect(page.getByText('Pulled 1 update(s) from remote')).toBeVisible({ timeout: 5_000 }) + await expect(page.locator('.bn-editor')).not.toContainText(pulledMarker) + await expect(page.locator('.bn-editor')).toContainText('Initial checklist body.') + await expect(checklistCheckbox(page, 0)).toBeChecked() + + await openNote(page, 'Note C') + await openNote(page, 'Note B') + await expect(page.locator('.bn-editor')).not.toContainText(pulledMarker) + await expect(page.locator('.bn-editor')).toContainText('Initial checklist body.') + await expect(checklistCheckbox(page, 0)).toBeChecked() + + expect(errors).toEqual([]) + }) +})