From 8b36ff6b003688955af5d20786d463cbc00066c7 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Fri, 1 May 2026 11:40:14 +0200 Subject: [PATCH] fix: avoid active editor remount on unrelated vault refresh --- docs/ABSTRACTIONS.md | 4 +- package.json | 2 +- src/hooks/useVaultBridge.test.ts | 8 +- src/utils/pulledVaultRefresh.test.ts | 14 +++- src/utils/pulledVaultRefresh.ts | 6 +- tests/smoke/pull-refresh-open-note.spec.ts | 97 ++++++++++++++++++++++ 6 files changed, 118 insertions(+), 13 deletions(-) diff --git a/docs/ABSTRACTIONS.md b/docs/ABSTRACTIONS.md index 9726a2b2..8d53d280 100644 --- a/docs/ABSTRACTIONS.md +++ b/docs/ABSTRACTIONS.md @@ -462,7 +462,7 @@ interface PulseCommit { - Configurable interval (from app settings: `auto_pull_interval_minutes`) - Pulls on interval, pushes after commits - Awaits the post-pull vault refresh so toasts land after note-list state is fresh -- Reopens the clean active tab from disk after a successful pull update so the editor and note list stay aligned +- Reopens the clean active tab from disk only when the pull changed that active note, so unrelated updates do not remount the editor - Detects merge conflicts → opens `ConflictResolverModal` - Tracks remote status (branch, ahead/behind via `git_remote_status`) - Handles push rejection (divergence) → sets `pull_required` status @@ -471,7 +471,7 @@ interface PulseCommit { ### External Vault Refresh -External vault mutations are any disk writes Tolaria did not just perform through its own save path: Git pulls, AI-agent writes, filesystem watcher events, and edits from another app. These changes must route through `refreshPulledVaultState()` rather than calling `reloadVault()` in isolation. The shared refresh abstraction reloads entries, folders, and saved views together, preserves unsaved active-editor content, reopens a clean active note from disk, and closes the active tab if the file disappeared. `useVaultWatcher` supplies changed filesystem paths to this abstraction after debouncing and after filtering recent app-owned saves. +External vault mutations are any disk writes Tolaria did not just perform through its own save path: Git pulls, AI-agent writes, filesystem watcher events, and edits from another app. These changes must route through `refreshPulledVaultState()` rather than calling `reloadVault()` in isolation. The shared refresh abstraction reloads entries, folders, and saved views together, preserves unsaved active-editor content, reopens a clean active note only when the changed-path list includes that note, and closes the active tab if the file disappeared. Unknown or unrelated watcher updates refresh vault-derived state without remounting the active editor. `useVaultWatcher` supplies changed filesystem paths to this abstraction after debouncing and after filtering recent app-owned saves. `useGitRemoteStatus` is the commit-time companion to `useAutoSync`: - Re-checks `git_remote_status` when the Commit dialog opens and right before submit diff --git a/package.json b/package.json index 2403af93..da56ad6a 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "test": "vitest run", "test:watch": "vitest", "test:e2e": "playwright test", - "playwright:smoke": "playwright test --config playwright.smoke.config.ts tests/smoke/autosave-low-end-typing.spec.ts tests/smoke/create-note-backing-file.spec.ts tests/smoke/delete-note-nonblocking.spec.ts tests/smoke/example.spec.ts tests/smoke/fix-ai-chat-empty-body-v3.spec.ts tests/smoke/fix-crash-create-note.spec.ts tests/smoke/fresh-start-telemetry-onboarding.spec.ts tests/smoke/frontmatter-date-picker.spec.ts tests/smoke/getting-started-template.spec.ts tests/smoke/h1-title-decoupled.spec.ts tests/smoke/h1-untitled-auto-rename.spec.ts tests/smoke/keyboard-command-routing.spec.ts tests/smoke/linkify-init-warnings.spec.ts tests/smoke/missing-string-metadata-open-note.spec.ts tests/smoke/multibyte-search-snippet.spec.ts tests/smoke/multi-selection-shortcuts.spec.ts tests/smoke/vault-loading-skeleton.spec.ts tests/smoke/wikilink-path-fix.spec.ts", + "playwright:smoke": "playwright test --config playwright.smoke.config.ts tests/smoke/autosave-low-end-typing.spec.ts tests/smoke/create-note-backing-file.spec.ts tests/smoke/delete-note-nonblocking.spec.ts tests/smoke/example.spec.ts tests/smoke/fix-ai-chat-empty-body-v3.spec.ts tests/smoke/fix-crash-create-note.spec.ts tests/smoke/fresh-start-telemetry-onboarding.spec.ts tests/smoke/frontmatter-date-picker.spec.ts tests/smoke/getting-started-template.spec.ts tests/smoke/h1-title-decoupled.spec.ts tests/smoke/h1-untitled-auto-rename.spec.ts tests/smoke/keyboard-command-routing.spec.ts tests/smoke/linkify-init-warnings.spec.ts tests/smoke/missing-string-metadata-open-note.spec.ts tests/smoke/multibyte-search-snippet.spec.ts tests/smoke/multi-selection-shortcuts.spec.ts tests/smoke/pull-refresh-open-note.spec.ts tests/smoke/vault-loading-skeleton.spec.ts tests/smoke/wikilink-path-fix.spec.ts", "playwright:regression": "playwright test tests/smoke/", "playwright:integration": "playwright test --config playwright.integration.config.ts", "test:coverage": "node scripts/run-vitest-coverage.mjs", diff --git a/src/hooks/useVaultBridge.test.ts b/src/hooks/useVaultBridge.test.ts index 7a8968d3..22c69fb9 100644 --- a/src/hooks/useVaultBridge.test.ts +++ b/src/hooks/useVaultBridge.test.ts @@ -131,7 +131,7 @@ describe('useVaultBridge', () => { expect(replaceActiveTab).toHaveBeenCalledWith(fresh) }) - it('handleAgentFileModified still refreshes vault-derived UI for other notes', async () => { + it('handleAgentFileModified keeps the active tab mounted for other notes', async () => { const active = makeEntry('/vault/other.md', 'Other') reloadVault.mockResolvedValue([active]) const { result } = renderBridge([], '/vault/other.md') @@ -140,7 +140,7 @@ describe('useVaultBridge', () => { expectVaultDerivedStateReloaded({ reloadVault, reloadFolders, reloadViews }) expect(closeAllTabs).not.toHaveBeenCalled() - expect(replaceActiveTab).toHaveBeenCalledWith(active) + expect(replaceActiveTab).not.toHaveBeenCalled() }) it('keeps unsaved active note content intact while reloading agent changes', async () => { @@ -156,7 +156,7 @@ describe('useVaultBridge', () => { expect(replaceActiveTab).not.toHaveBeenCalled() }) - it('handleAgentVaultChanged reloads vault-derived state and refreshes the active note when safe', async () => { + it('handleAgentVaultChanged reloads vault-derived state without remounting the active note', async () => { const fresh = makeEntry('/vault/active.md', 'Fresh active') reloadVault.mockResolvedValue([fresh]) const { result } = renderBridge([], '/vault/active.md') @@ -165,6 +165,6 @@ describe('useVaultBridge', () => { expectVaultDerivedStateReloaded({ reloadVault, reloadFolders, reloadViews }) expect(closeAllTabs).not.toHaveBeenCalled() - expect(replaceActiveTab).toHaveBeenCalledWith(fresh) + expect(replaceActiveTab).not.toHaveBeenCalled() }) }) diff --git a/src/utils/pulledVaultRefresh.test.ts b/src/utils/pulledVaultRefresh.test.ts index 555bd5a4..b2e9b916 100644 --- a/src/utils/pulledVaultRefresh.test.ts +++ b/src/utils/pulledVaultRefresh.test.ts @@ -43,14 +43,24 @@ describe('refreshPulledVaultState', () => { expect(options.replaceActiveTab).toHaveBeenCalledWith(entries[0]) }) - it('reloads the active tab after any successful pull with updates', async () => { + it('keeps the active tab mounted when updates do not include the active note', async () => { const options = makeOptions({ updatedFiles: ['project/plan.md'] }) await refreshPulledVaultState(options) expect(options.reloadVault).toHaveBeenCalledOnce() expect(options.closeAllTabs).not.toHaveBeenCalled() - expect(options.replaceActiveTab).toHaveBeenCalledWith(expect.objectContaining({ path: '/vault/active.md' })) + expect(options.replaceActiveTab).not.toHaveBeenCalled() + }) + + it('keeps the active tab mounted for full watcher refreshes with unknown changed files', async () => { + const options = makeOptions({ updatedFiles: [] }) + + await refreshPulledVaultState(options) + + expect(options.reloadVault).toHaveBeenCalledOnce() + expect(options.closeAllTabs).not.toHaveBeenCalled() + expect(options.replaceActiveTab).not.toHaveBeenCalled() }) it('matches macOS /tmp and /private/tmp aliases when reloading the active tab entry', async () => { diff --git a/src/utils/pulledVaultRefresh.ts b/src/utils/pulledVaultRefresh.ts index b5301a2c..e1da4c34 100644 --- a/src/utils/pulledVaultRefresh.ts +++ b/src/utils/pulledVaultRefresh.ts @@ -64,14 +64,12 @@ export async function refreshPulledVaultState(options: PulledVaultRefreshOptions closeAllTabs() return entries } + if (!didPullUpdateActiveNote(updatedFiles, vaultPath, latestActiveTabPath)) return entries // Native BlockNote can keep rendering the previous document after a pull that // changes the active file in place. Dropping the tab first forces a full // reopen for that specific case without affecting unrelated pull updates. - if (didPullUpdateActiveNote(updatedFiles, vaultPath, latestActiveTabPath)) { - closeAllTabs() - } - + closeAllTabs() await replaceActiveTab(refreshedEntry) return entries } diff --git a/tests/smoke/pull-refresh-open-note.spec.ts b/tests/smoke/pull-refresh-open-note.spec.ts index 500e9054..b8b7291e 100644 --- a/tests/smoke/pull-refresh-open-note.spec.ts +++ b/tests/smoke/pull-refresh-open-note.spec.ts @@ -10,6 +10,10 @@ import { executeCommand, openCommandPalette } from './helpers' let tempVaultDir: string +interface EditorSwapProbeWindow { + __editorSwapEvents?: string[] +} + async function openNote(page: Page, title: string) { const noteList = page.getByTestId('note-list-container') await noteList.getByText(title, { exact: true }).click() @@ -31,6 +35,71 @@ async function pullFromRemote(page: Page) { await executeCommand(page, 'Pull from Remote') } +async function triggerPullCommand(page: Page) { + await page.evaluate(() => { + window.__laputaTest?.dispatchAppCommand?.('vault-pull') + }) +} + +async function installEditorSwapProbe(page: Page) { + await page.evaluate(() => { + const probeWindow = window as typeof window & EditorSwapProbeWindow + probeWindow.__editorSwapEvents = [] + window.addEventListener('laputa:editor-tab-swapped', (event) => { + const customEvent = event as CustomEvent<{ path?: string }> + probeWindow.__editorSwapEvents?.push(customEvent.detail?.path ?? '') + }) + }) +} + +async function readEditorSwapCount(page: Page): Promise { + return page.evaluate(() => { + const probeWindow = window as typeof window & EditorSwapProbeWindow + return probeWindow.__editorSwapEvents?.length ?? 0 + }) +} + +async function placeCaretAtEndOfBlock(page: Page, blockIndex: number) { + const block = page.locator('.bn-block-content').nth(blockIndex) + await expect(block).toBeVisible({ timeout: 5_000 }) + + const placed = await block.evaluate((element) => { + const walker = document.createTreeWalker(element, NodeFilter.SHOW_TEXT) + let lastTextNode: Text | null = null + while (walker.nextNode()) { + if (walker.currentNode.textContent) lastTextNode = walker.currentNode as Text + } + if (!lastTextNode) return false + + const range = document.createRange() + range.setStart(lastTextNode, lastTextNode.textContent?.length ?? 0) + range.collapse(true) + const selection = window.getSelection() + selection?.removeAllRanges() + selection?.addRange(range) + document.dispatchEvent(new Event('selectionchange')) + return true + }) + + expect(placed).toBe(true) +} + +async function expectEditorFocused(page: Page): Promise { + await expect.poll(async () => page.evaluate(() => { + const active = document.activeElement as HTMLElement | null + return Boolean(active?.isContentEditable || active?.closest('[contenteditable="true"]')) + }), { timeout: 5_000 }).toBe(true) +} + +async function activeSelectionBlockType(page: Page): Promise { + return page.evaluate(() => { + const selection = window.getSelection() + const anchorNode = selection?.anchorNode ?? null + const anchorElement = anchorNode instanceof Element ? anchorNode : anchorNode?.parentElement ?? null + return anchorElement?.closest('.bn-block-content')?.getAttribute('data-content-type') ?? null + }) +} + test.describe('Pull refreshes the open note immediately', () => { test.beforeEach(async ({ page }, testInfo) => { testInfo.setTimeout(60_000) @@ -73,4 +142,32 @@ ${pulledBody} await expect(noteList.getByText(pulledTitle, { exact: true })).toBeVisible({ timeout: 5_000 }) await expect(noteList.getByText(originalTitle, { exact: true })).toHaveCount(0) }) + + test('@smoke unrelated pull keeps the active editor mounted with selection in place', async ({ page }) => { + const marker = `typing before unrelated pull ${Date.now()}` + const noteBPath = path.join(tempVaultDir, 'note', 'note-b.md') + const noteCPath = path.join(tempVaultDir, 'note', 'note-c.md') + + await openNote(page, 'Note B') + await expect(page.locator('.bn-editor h1').first()).toHaveText('Note B', { timeout: 5_000 }) + await placeCaretAtEndOfBlock(page, 1) + await page.keyboard.type(` ${marker}`, { delay: 20 }) + await expect(async () => { + expect(fs.readFileSync(noteBPath, 'utf8')).toContain(marker) + }).toPass({ timeout: 10_000 }) + + await installEditorSwapProbe(page) + await expectEditorFocused(page) + + fs.appendFileSync(noteCPath, `\n\nUnrelated pulled change ${Date.now()}\n`, 'utf8') + await stubUpdatedPull(page, noteCPath) + + await triggerPullCommand(page) + + await expect(page.getByText('Pulled 1 update(s) from remote')).toBeVisible({ timeout: 5_000 }) + await page.waitForTimeout(500) + expect(await readEditorSwapCount(page)).toBe(0) + await expect.poll(() => activeSelectionBlockType(page), { timeout: 5_000 }).toBe('paragraph') + await expectEditorFocused(page) + }) })