From 0020ade6d7f711c092d4daa5428c59cd055c20d0 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Mon, 20 Apr 2026 14:43:52 +0200 Subject: [PATCH] fix: tighten pulled note refresh flow --- docs/ABSTRACTIONS.md | 2 + docs/ARCHITECTURE.md | 6 +- src/App.tsx | 44 ++- src/hooks/useAutoSync.test.ts | 37 ++- src/hooks/useAutoSync.ts | 447 ++++++++++++++++++--------- src/hooks/useEditorTabSwap.test.ts | 22 ++ src/hooks/useEditorTabSwap.ts | 17 + src/hooks/useTabManagement.test.ts | 34 +- src/hooks/useTabManagement.ts | 37 ++- src/utils/pulledVaultRefresh.test.ts | 90 ++++++ src/utils/pulledVaultRefresh.ts | 49 +++ 11 files changed, 612 insertions(+), 173 deletions(-) create mode 100644 src/utils/pulledVaultRefresh.test.ts create mode 100644 src/utils/pulledVaultRefresh.ts diff --git a/docs/ABSTRACTIONS.md b/docs/ABSTRACTIONS.md index 1bae831a..2d89716d 100644 --- a/docs/ABSTRACTIONS.md +++ b/docs/ABSTRACTIONS.md @@ -395,6 +395,8 @@ interface PulseCommit { `useAutoSync` hook handles automatic git sync: - 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 - Detects merge conflicts → opens `ConflictResolverModal` - Tracks remote status (branch, ahead/behind via `git_remote_status`) - Handles push rejection (divergence) → sets `pull_required` status diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 85f6edb3..5e0a36d0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -531,7 +531,11 @@ flowchart TD AS["useAutoSync\n(configurable interval)"] --> PULL["invoke('git_pull')"] PULL --> PC{Result?} PC -->|Conflicts| CM["ConflictResolverModal\nor ConflictNoteBanner"] - PC -->|Fast-forward| RV["reload vault"] + PC -->|Fast-forward| RV["reload vault + folders/views"] + RV --> TAB{"clean active tab?"} + TAB -->|Yes| RT["replace active tab\nwith fresh disk content"] + TAB -->|No| DONE["idle"] + RT --> DONE PC -->|Up to date| DONE["idle"] MAN["Manual commit\n(CommitDialog)"] --> RS["useGitRemoteStatus\n(commit-time check)"] diff --git a/src/App.tsx b/src/App.tsx index ba75d5b5..3b634367 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -74,6 +74,7 @@ import type { NoteListItem } from './utils/ai-context' import { initializeNoteProperties } from './utils/initializeNoteProperties' import { filterEntries, filterInboxEntries, type NoteListFilter } from './utils/noteListHelpers' import { openNoteInNewWindow } from './utils/openNoteWindow' +import { refreshPulledVaultState } from './utils/pulledVaultRefresh' import { isNoteWindow, getNoteWindowParams, getNoteWindowPathCandidates, findNoteWindowEntry, type NoteWindowParams } from './utils/windowMode' import { GitRequiredModal } from './components/GitRequiredModal' import { RenameDetectedBanner, type DetectedRename } from './components/RenameDetectedBanner' @@ -404,17 +405,6 @@ function App() { } }, [vault.entries.length, gitRepoState, resolvedPath]) const { mcpStatus, installMcp } = useMcpStatus(resolvedPath, setToastMessage) - - const autoSync = useAutoSync({ - vaultPath: resolvedPath, - intervalMinutes: settings.auto_pull_interval_minutes, - onVaultUpdated: vault.reloadVault, - onConflict: (files) => { - const names = files.map((f) => f.split('/').pop()).join(', ') - setToastMessage(`Conflict in ${names} — click to resolve`) - }, - onToast: (msg) => setToastMessage(msg), - }) const gitRemoteStatus = useGitRemoteStatus(resolvedPath) // Detect external file renames on window focus @@ -487,8 +477,40 @@ function App() { const { handleSelectNote, handleReplaceActiveTab, + closeAllTabs, openTabWithContent, } = notes + const handlePulledVaultUpdate = useCallback((updatedFiles: string[]) => + refreshPulledVaultState({ + activeTabPath: notes.activeTabPath, + closeAllTabs, + hasUnsavedChanges: (path) => vault.unsavedPaths.has(path), + reloadFolders: vault.reloadFolders, + reloadVault: vault.reloadVault, + reloadViews: vault.reloadViews, + replaceActiveTab: handleReplaceActiveTab, + updatedFiles, + vaultPath: resolvedPath, + }), [ + closeAllTabs, + handleReplaceActiveTab, + notes.activeTabPath, + resolvedPath, + vault.reloadFolders, + vault.reloadVault, + vault.reloadViews, + vault.unsavedPaths, + ]) + const autoSync = useAutoSync({ + vaultPath: resolvedPath, + intervalMinutes: settings.auto_pull_interval_minutes, + onVaultUpdated: handlePulledVaultUpdate, + onConflict: (files) => { + const names = files.map((f) => f.split('/').pop()).join(', ') + setToastMessage(`Conflict in ${names} — click to resolve`) + }, + onToast: (msg) => setToastMessage(msg), + }) const pulseCommitDiffRequestIdRef = useRef(0) const [pulseCommitDiffRequest, setPulseCommitDiffRequest] = useState(null) diff --git a/src/hooks/useAutoSync.test.ts b/src/hooks/useAutoSync.test.ts index f54b8a9a..029e0c76 100644 --- a/src/hooks/useAutoSync.test.ts +++ b/src/hooks/useAutoSync.test.ts @@ -75,12 +75,47 @@ describe('useAutoSync', () => { const { result } = renderSync() await waitFor(() => { - expect(onVaultUpdated).toHaveBeenCalled() + expect(onVaultUpdated).toHaveBeenCalledWith(['note.md', 'project/plan.md']) expect(onToast).toHaveBeenCalledWith('Pulled 2 update(s) from remote') expect(result.current.syncStatus).toBe('idle') }) }) + it('waits for vault refresh before showing the updated toast', async () => { + let releaseVaultRefresh: (() => void) | null = null + const asyncVaultRefresh = vi.fn(() => new Promise((resolve) => { + releaseVaultRefresh = resolve + })) + + mockInvokeFn.mockImplementation((cmd: string) => { + if (cmd === 'get_last_commit_info') return Promise.resolve(MOCK_COMMIT_INFO) + return Promise.resolve(updated(['note.md'])) + }) + + renderHook(() => + useAutoSync({ + vaultPath: '/Users/luca/Laputa', + intervalMinutes: 5, + onVaultUpdated: asyncVaultRefresh, + onConflict, + onToast, + }), + ) + + await waitFor(() => { + expect(asyncVaultRefresh).toHaveBeenCalledWith(['note.md']) + }) + expect(onToast).not.toHaveBeenCalledWith('Pulled 1 update(s) from remote') + + await act(async () => { + releaseVaultRefresh?.() + }) + + await waitFor(() => { + expect(onToast).toHaveBeenCalledWith('Pulled 1 update(s) from remote') + }) + }) + it('calls onConflict and sets conflict status when pull has conflicts', async () => { mockInvokeFn.mockImplementation((cmd: string) => { if (cmd === 'get_last_commit_info') return Promise.resolve(MOCK_COMMIT_INFO) diff --git a/src/hooks/useAutoSync.ts b/src/hooks/useAutoSync.ts index 43a9e775..c0d294fd 100644 --- a/src/hooks/useAutoSync.ts +++ b/src/hooks/useAutoSync.ts @@ -1,10 +1,16 @@ import { useCallback, useEffect, useRef, useState } from 'react' +import type { Dispatch, MutableRefObject, SetStateAction } from 'react' import { invoke } from '@tauri-apps/api/core' import { isTauri, mockInvoke } from '../mock-tauri' import type { GitPullResult, GitPushResult, GitRemoteStatus, LastCommitInfo, SyncStatus } from '../types' import { trackEvent } from '../lib/telemetry' const DEFAULT_INTERVAL_MS = 5 * 60_000 +const FOCUS_COOLDOWN_MS = 30_000 + +type MaybePromise = void | Promise + +type SyncCallbacks = Pick function tauriCall(cmd: string, args: Record): Promise { return isTauri() ? invoke(cmd, args) : mockInvoke(cmd, args) @@ -13,8 +19,8 @@ function tauriCall(cmd: string, args: Record): Promise { interface UseAutoSyncOptions { vaultPath: string intervalMinutes: number | null - onVaultUpdated: () => void - onSyncUpdated?: () => void + onVaultUpdated: (updatedFiles: string[]) => MaybePromise + onSyncUpdated?: () => MaybePromise onConflict: (files: string[]) => void onToast: (msg: string) => void } @@ -36,6 +42,214 @@ export interface AutoSyncState { handlePushRejected: () => void } +type SyncSetState = Dispatch> + +interface PullErrorResolution { + checkExistingConflicts: () => Promise + notifyError?: string + callbacksRef: MutableRefObject + setSyncStatus: SyncSetState +} + +interface SyncTaskOptions { + blockWhenPaused: boolean + pauseRef: MutableRefObject + syncingRef: MutableRefObject + setLastSyncTime: SyncSetState + setSyncStatus: SyncSetState + task: () => Promise +} + +function clearConflictState( + setSyncStatus: SyncSetState, + setConflictFiles: SyncSetState, +): void { + setSyncStatus('idle') + setConflictFiles([]) +} + +function setConflictState( + files: string[], + setSyncStatus: SyncSetState, + setConflictFiles: SyncSetState, + callbacksRef: MutableRefObject, +): void { + setSyncStatus('conflict') + setConflictFiles(files) + void callbacksRef.current.onConflict(files) +} + +function markPullTimestamp( + setLastSyncTime: SyncSetState, + refreshCommitInfo: () => void, +): void { + setLastSyncTime(Date.now()) + refreshCommitInfo() +} + +function useRemoteStatusRefresher( + vaultPath: string, + setRemoteStatus: SyncSetState, +) { + return useCallback(async () => { + try { + const status = await tauriCall('git_remote_status', { vaultPath }) + setRemoteStatus(status) + return status + } catch { + return null + } + }, [vaultPath, setRemoteStatus]) +} + +function useConflictChecker( + vaultPath: string, + setSyncStatus: SyncSetState, + setConflictFiles: SyncSetState, + callbacksRef: MutableRefObject, +) { + return useCallback(async (): Promise => { + try { + const files = await tauriCall('get_conflict_files', { vaultPath }) + if (!Array.isArray(files) || files.length === 0) return false + setConflictState(files, setSyncStatus, setConflictFiles, callbacksRef) + return true + } catch { + return false + } + }, [vaultPath, setSyncStatus, setConflictFiles, callbacksRef]) +} + +function useCommitInfoRefresher( + vaultPath: string, + setLastCommitInfo: SyncSetState, +) { + return useCallback(() => { + tauriCall('get_last_commit_info', { vaultPath }) + .then(info => setLastCommitInfo(info)) + .catch(() => {}) + }, [vaultPath, setLastCommitInfo]) +} + +async function handleUpdatedPull(options: { + result: GitPullResult + callbacksRef: MutableRefObject + setConflictFiles: SyncSetState + setSyncStatus: SyncSetState +}): Promise { + const { + result, + callbacksRef, + setConflictFiles, + setSyncStatus, + } = options + clearConflictState(setSyncStatus, setConflictFiles) + await callbacksRef.current.onVaultUpdated(result.updatedFiles) + await callbacksRef.current.onSyncUpdated?.() + await callbacksRef.current.onToast(`Pulled ${result.updatedFiles.length} update(s) from remote`) +} + +async function resolvePullError(options: PullErrorResolution): Promise { + const { + checkExistingConflicts, + notifyError, + callbacksRef, + setSyncStatus, + } = options + const hasConflicts = await checkExistingConflicts() + if (hasConflicts) return + setSyncStatus('error') + if (notifyError) await callbacksRef.current.onToast(notifyError) +} + +function handlePushResult(options: { + pushResult: GitPushResult + callbacksRef: MutableRefObject + setConflictFiles: SyncSetState + setSyncStatus: SyncSetState +}): void { + const { + pushResult, + callbacksRef, + setConflictFiles, + setSyncStatus, + } = options + if (pushResult.status === 'ok') { + clearConflictState(setSyncStatus, setConflictFiles) + void callbacksRef.current.onToast('Pulled and pushed successfully') + return + } + if (pushResult.status === 'rejected') { + setSyncStatus('pull_required') + void callbacksRef.current.onToast('Push still rejected after pull — try again') + return + } + setSyncStatus('error') + void callbacksRef.current.onToast(pushResult.message) +} + +async function runSyncTask(options: SyncTaskOptions): Promise { + const { + blockWhenPaused, + pauseRef, + syncingRef, + setLastSyncTime, + setSyncStatus, + task, + } = options + if (syncingRef.current || (blockWhenPaused && pauseRef.current)) return + syncingRef.current = true + setSyncStatus('syncing') + + try { + await task() + } catch { + setSyncStatus('error') + setLastSyncTime(Date.now()) + } finally { + syncingRef.current = false + } +} + +function useAutoSyncLifecycle(options: { + checkExistingConflicts: () => Promise + intervalMinutes: number | null + performPull: () => Promise + refreshRemoteStatus: () => Promise +}) { + const { + checkExistingConflicts, + intervalMinutes, + performPull, + refreshRemoteStatus, + } = options + + useEffect(() => { + void checkExistingConflicts().then(hasConflicts => { + if (!hasConflicts) void performPull() + }) + void refreshRemoteStatus() + }, [checkExistingConflicts, performPull, refreshRemoteStatus]) + + const lastPullTimeRef = useRef(0) + useEffect(() => { + const handleFocus = () => { + const now = Date.now() + if (now - lastPullTimeRef.current < FOCUS_COOLDOWN_MS) return + lastPullTimeRef.current = now + void performPull() + } + window.addEventListener('focus', handleFocus) + return () => window.removeEventListener('focus', handleFocus) + }, [performPull]) + + useEffect(() => { + const ms = (intervalMinutes ?? 5) * 60_000 || DEFAULT_INTERVAL_MS + const id = setInterval(() => { void performPull() }, ms) + return () => clearInterval(id) + }, [performPull, intervalMinutes]) +} + export function useAutoSync({ vaultPath, intervalMinutes, @@ -51,178 +265,111 @@ export function useAutoSync({ const [remoteStatus, setRemoteStatus] = useState(null) const syncingRef = useRef(false) const pauseRef = useRef(false) - const callbacksRef = useRef({ onVaultUpdated, onSyncUpdated, onConflict, onToast }) - callbacksRef.current = { onVaultUpdated, onSyncUpdated, onConflict, onToast } - - const refreshRemoteStatus = useCallback(async () => { - try { - const status = await tauriCall('git_remote_status', { vaultPath }) - setRemoteStatus(status) - return status - } catch { - return null - } - }, [vaultPath]) - - /** Check for pre-existing conflicts (e.g. from a prior session or interrupted rebase). */ - const checkExistingConflicts = useCallback(async (): Promise => { - try { - const files = await tauriCall('get_conflict_files', { vaultPath }) - if (files.length > 0) { - setSyncStatus('conflict') - setConflictFiles(files) - callbacksRef.current.onConflict(files) - return true - } - } catch { - // If the command doesn't exist (e.g. browser mock), ignore - } - return false - }, [vaultPath]) - - const refreshCommitInfo = useCallback(() => { - tauriCall('get_last_commit_info', { vaultPath }) - .then(info => setLastCommitInfo(info)) - .catch(() => {}) - }, [vaultPath]) + const callbacksRef = useRef({ onVaultUpdated, onSyncUpdated, onConflict, onToast }) + useEffect(() => { + callbacksRef.current = { onVaultUpdated, onSyncUpdated, onConflict, onToast } + }, [onVaultUpdated, onSyncUpdated, onConflict, onToast]) + const refreshRemoteStatus = useRemoteStatusRefresher(vaultPath, setRemoteStatus) + const checkExistingConflicts = useConflictChecker(vaultPath, setSyncStatus, setConflictFiles, callbacksRef) + const refreshCommitInfo = useCommitInfoRefresher(vaultPath, setLastCommitInfo) const performPull = useCallback(async () => { - if (syncingRef.current || pauseRef.current) return - syncingRef.current = true - setSyncStatus('syncing') + await runSyncTask({ + blockWhenPaused: true, + pauseRef, + syncingRef, + setLastSyncTime, + setSyncStatus, + task: async () => { + const result = await tauriCall('git_pull', { vaultPath }) + markPullTimestamp(setLastSyncTime, refreshCommitInfo) - try { - const result = await tauriCall('git_pull', { vaultPath }) - setLastSyncTime(Date.now()) - refreshCommitInfo() - - if (result.status === 'updated') { - setSyncStatus('idle') - setConflictFiles([]) - callbacksRef.current.onVaultUpdated() - callbacksRef.current.onSyncUpdated?.() - callbacksRef.current.onToast(`Pulled ${result.updatedFiles.length} update(s) from remote`) - } else if (result.status === 'conflict') { - setSyncStatus('conflict') - setConflictFiles(result.conflictFiles) - callbacksRef.current.onConflict(result.conflictFiles) - } else if (result.status === 'error') { - // Pull failed — check if there are pre-existing conflicts that caused it - const hasConflicts = await checkExistingConflicts() - if (!hasConflicts) { - setSyncStatus('error') + if (result.status === 'updated') { + await handleUpdatedPull({ + result, + callbacksRef, + setConflictFiles, + setSyncStatus, + }) + } else if (result.status === 'conflict') { + setConflictState(result.conflictFiles, setSyncStatus, setConflictFiles, callbacksRef) + } else if (result.status === 'error') { + await resolvePullError({ + checkExistingConflicts, + callbacksRef, + setSyncStatus, + }) + } else { + clearConflictState(setSyncStatus, setConflictFiles) } - } else { - // up_to_date or no_remote - setSyncStatus('idle') - setConflictFiles([]) - } - // Refresh remote status after pull - refreshRemoteStatus() - } catch { - setSyncStatus('error') - setLastSyncTime(Date.now()) - } finally { - syncingRef.current = false - } - }, [vaultPath, checkExistingConflicts, refreshCommitInfo, refreshRemoteStatus]) + void refreshRemoteStatus() + }, + }) + }, [vaultPath, refreshCommitInfo, checkExistingConflicts, refreshRemoteStatus]) /** Pull from remote, then auto-push if successful. Used for divergence recovery. */ const pullAndPush = useCallback(async () => { - if (syncingRef.current) return - syncingRef.current = true - setSyncStatus('syncing') + await runSyncTask({ + blockWhenPaused: false, + pauseRef, + syncingRef, + setLastSyncTime, + setSyncStatus, + task: async () => { + const pullResult = await tauriCall('git_pull', { vaultPath }) + markPullTimestamp(setLastSyncTime, refreshCommitInfo) - try { - const pullResult = await tauriCall('git_pull', { vaultPath }) - setLastSyncTime(Date.now()) - refreshCommitInfo() - - if (pullResult.status === 'conflict') { - setSyncStatus('conflict') - setConflictFiles(pullResult.conflictFiles) - callbacksRef.current.onConflict(pullResult.conflictFiles) - return - } - - if (pullResult.status === 'error') { - const hasConflicts = await checkExistingConflicts() - if (!hasConflicts) { - setSyncStatus('error') - callbacksRef.current.onToast('Pull failed: ' + pullResult.message) + if (pullResult.status === 'conflict') { + setConflictState(pullResult.conflictFiles, setSyncStatus, setConflictFiles, callbacksRef) + return } - return - } - if (pullResult.status === 'updated') { - callbacksRef.current.onVaultUpdated() - callbacksRef.current.onSyncUpdated?.() - } + if (pullResult.status === 'error') { + await resolvePullError({ + checkExistingConflicts, + notifyError: `Pull failed: ${pullResult.message}`, + callbacksRef, + setSyncStatus, + }) + return + } - // Now push - const pushResult = await tauriCall('git_push', { vaultPath }) - if (pushResult.status === 'ok') { - setSyncStatus('idle') - setConflictFiles([]) - callbacksRef.current.onToast('Pulled and pushed successfully') - } else if (pushResult.status === 'rejected') { - // Still diverged — shouldn't happen after pull but handle gracefully - setSyncStatus('pull_required') - callbacksRef.current.onToast('Push still rejected after pull — try again') - } else { - setSyncStatus('error') - callbacksRef.current.onToast(pushResult.message) - } + if (pullResult.status === 'updated') { + await callbacksRef.current.onVaultUpdated(pullResult.updatedFiles) + await callbacksRef.current.onSyncUpdated?.() + } - refreshRemoteStatus() - } catch { - setSyncStatus('error') - setLastSyncTime(Date.now()) - } finally { - syncingRef.current = false - } - }, [vaultPath, checkExistingConflicts, refreshCommitInfo, refreshRemoteStatus]) + const pushResult = await tauriCall('git_push', { vaultPath }) + handlePushResult({ + pushResult, + callbacksRef, + setConflictFiles, + setSyncStatus, + }) + + void refreshRemoteStatus() + }, + }) + }, [vaultPath, refreshCommitInfo, checkExistingConflicts, refreshRemoteStatus]) const handlePushRejected = useCallback(() => { setSyncStatus('pull_required') }, []) - // Check for pre-existing conflicts on mount, then pull - useEffect(() => { - checkExistingConflicts().then(hasConflicts => { - if (!hasConflicts) performPull() - }) - refreshRemoteStatus() - }, [checkExistingConflicts, performPull, refreshRemoteStatus]) - - // Pull on window focus (app foreground) — with cooldown to avoid repeated pulls - const lastPullTimeRef = useRef(0) - useEffect(() => { - const FOCUS_COOLDOWN_MS = 30_000 - const handleFocus = () => { - const now = Date.now() - if (now - lastPullTimeRef.current < FOCUS_COOLDOWN_MS) return - lastPullTimeRef.current = now - performPull() - } - window.addEventListener('focus', handleFocus) - return () => window.removeEventListener('focus', handleFocus) - }, [performPull]) - - // Periodic pull - useEffect(() => { - const ms = (intervalMinutes ?? 5) * 60_000 || DEFAULT_INTERVAL_MS - const id = setInterval(performPull, ms) - return () => clearInterval(id) - }, [performPull, intervalMinutes]) + useAutoSyncLifecycle({ + checkExistingConflicts, + intervalMinutes, + performPull, + refreshRemoteStatus, + }) const pausePull = useCallback(() => { pauseRef.current = true }, []) const resumePull = useCallback(() => { pauseRef.current = false }, []) const triggerSync = useCallback(() => { trackEvent('sync_triggered') - performPull() + void performPull() }, [performPull]) return { syncStatus, lastSyncTime, conflictFiles, lastCommitInfo, remoteStatus, triggerSync, pullAndPush, pausePull, resumePull, handlePushRejected } diff --git a/src/hooks/useEditorTabSwap.test.ts b/src/hooks/useEditorTabSwap.test.ts index 7715fd67..7f9b8b9a 100644 --- a/src/hooks/useEditorTabSwap.test.ts +++ b/src/hooks/useEditorTabSwap.test.ts @@ -361,6 +361,28 @@ describe('useEditorTabSwap raw mode sync', () => { expect(mockEditor.replaceBlocks).toHaveBeenCalled() }) + it('re-parses when the active tab content changes without a path change', async () => { + const tabA = makeTab('a.md', 'Note A') + const refreshedTabA = { + ...tabA, + content: '---\ntitle: Note A\n---\n\n# Note A\n\nFresh after pull.', + } + + const { mockEditor, rerenderWith } = await createSwapHarness({ + initialProps: { tabs: [tabA], activeTabPath: 'a.md', rawMode: false }, + }) + + mockEditor.tryParseMarkdownToBlocks.mockClear() + mockEditor.replaceBlocks.mockClear() + + await rerenderWith({ tabs: [refreshedTabA], activeTabPath: 'a.md' }) + + expect(mockEditor.tryParseMarkdownToBlocks).toHaveBeenCalledWith( + expect.stringContaining('Fresh after pull.'), + ) + expect(mockEditor.replaceBlocks).toHaveBeenCalled() + }) + it('ignores editor change events before the pending tab swap applies a new untitled note', async () => { vi.spyOn(document, 'querySelector').mockReturnValue({ scrollTop: 0 } as unknown as Element) vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => { cb(0); return 0 }) diff --git a/src/hooks/useEditorTabSwap.ts b/src/hooks/useEditorTabSwap.ts index 52153806..265b10f0 100644 --- a/src/hooks/useEditorTabSwap.ts +++ b/src/hooks/useEditorTabSwap.ts @@ -508,6 +508,7 @@ function handleStableActivePath(options: { rawSwapPendingRef.current = true return false } + if (shouldRefreshStableActivePath({ activeTabPath, activeTab, cache })) return false if (rawSwapPendingRef.current) return true cacheStableActivePath({ @@ -520,6 +521,22 @@ function handleStableActivePath(options: { return true } +function shouldRefreshStableActivePath(options: { + activeTabPath: string | null + activeTab: Tab | undefined + cache: Map +}): boolean { + const { + activeTabPath, + activeTab, + cache, + } = options + + if (!activeTabPath || !activeTab) return false + const cachedState = cache.get(activeTabPath) + return !cachedState || cachedState.sourceContent !== activeTab.content +} + function cacheStableActivePath(options: { cache: Map activeTabPath: string | null diff --git a/src/hooks/useTabManagement.test.ts b/src/hooks/useTabManagement.test.ts index c9d11a0f..148a639e 100644 --- a/src/hooks/useTabManagement.test.ts +++ b/src/hooks/useTabManagement.test.ts @@ -152,15 +152,45 @@ describe('useTabManagement (single-note model)', () => { expectSingleActiveTab(result, '/vault/b.md') }) - it('is a no-op when replacing with the same entry', async () => { + it('treats /tmp and /private/tmp aliases as the same active note', async () => { + const { mockInvoke } = await import('../mock-tauri') + vi.mocked(mockInvoke) + .mockResolvedValueOnce('# Stale before pull') + .mockResolvedValueOnce('# Fresh after pull') + const beforeNavigate = vi.fn().mockResolvedValue(undefined) + + const { result } = renderHook(() => useTabManagement({ beforeNavigate })) + await selectNote(result, { path: '/private/tmp/vault/active.md', title: 'Active' }) + + await act(async () => { + await result.current.handleReplaceActiveTab( + makeEntry({ path: '/tmp/vault/active.md', title: 'Active' }), + ) + }) + + expect(beforeNavigate).not.toHaveBeenCalled() + expect(result.current.activeTabPath).toBe('/tmp/vault/active.md') + expect(result.current.tabs).toHaveLength(1) + expect(result.current.tabs[0].content).toBe('# Fresh after pull') + }) + + it('reloads content when replacing with the same entry', async () => { + const { mockInvoke } = await import('../mock-tauri') + vi.mocked(mockInvoke) + .mockResolvedValueOnce('# Stale before pull') + .mockResolvedValueOnce('# Fresh after pull') + const { result } = renderHook(() => useTabManagement()) - const entry = { path: '/vault/a.md' } + const entry = { path: '/vault/a.md', title: 'A' } await selectNote(result, entry) + await act(async () => { await result.current.handleReplaceActiveTab(makeEntry(entry)) }) expect(result.current.tabs).toHaveLength(1) + expect(result.current.tabs[0].content).toBe('# Fresh after pull') + expect(vi.mocked(mockInvoke)).toHaveBeenCalledTimes(2) }) it('opens a note when no note is active', async () => { diff --git a/src/hooks/useTabManagement.ts b/src/hooks/useTabManagement.ts index 0e9f4b6f..3907b9b3 100644 --- a/src/hooks/useTabManagement.ts +++ b/src/hooks/useTabManagement.ts @@ -93,7 +93,8 @@ function getCachedNoteContent(path: string): string | null { return prefetchCache.get(path)?.value ?? null } -async function loadNoteContent(path: string): Promise { +async function loadNoteContent(path: string, forceFresh = false): Promise { + if (forceFresh) return requestNoteContent({ path }).promise return prefetchCache.get(path)?.promise ?? requestNoteContent({ path }).promise } @@ -112,6 +113,18 @@ function syncActiveTabPath( setActiveTabPath(path) } +function normalizeComparablePath(path: string): string { + return path + .replaceAll('\\', '/') + .replace(/^\/private\/tmp(?=\/|$)/u, '/tmp') + .replace(/\/+$/u, '') +} + +function pathsMatch(leftPath: string | null, rightPath: string | null): boolean { + if (!leftPath || !rightPath) return false + return normalizeComparablePath(leftPath) === normalizeComparablePath(rightPath) +} + function setSingleTab( tabsRef: React.MutableRefObject, setTabs: React.Dispatch>, @@ -126,7 +139,8 @@ function isAlreadyViewingPath( activeTabPathRef: React.MutableRefObject, path: string, ) { - return activeTabPathRef.current === path || tabsRef.current.some((tab) => tab.entry.path === path) + return pathsMatch(activeTabPathRef.current, path) + || tabsRef.current.some((tab) => pathsMatch(tab.entry.path, path)) } function startEntryNavigation(options: { @@ -162,6 +176,7 @@ function shouldApplyLoadedEntry(options: { navSeqRef: React.MutableRefObject cachedContent: string | null content: string + forceReload: boolean activeTabPathRef: React.MutableRefObject path: string }) { @@ -170,12 +185,14 @@ function shouldApplyLoadedEntry(options: { navSeqRef, cachedContent, content, + forceReload, activeTabPathRef, path, } = options if (navSeqRef.current !== seq) return false - return cachedContent !== content || activeTabPathRef.current !== path + if (forceReload) return true + return cachedContent !== content || !pathsMatch(activeTabPathRef.current, path) } function handleEntryLoadFailure(options: { @@ -203,6 +220,7 @@ function handleEntryLoadFailure(options: { async function navigateToEntry(options: { entry: VaultEntry + forceReload?: boolean navSeqRef: React.MutableRefObject tabsRef: React.MutableRefObject activeTabPathRef: React.MutableRefObject @@ -211,6 +229,7 @@ async function navigateToEntry(options: { }) { const { entry, + forceReload = false, navSeqRef, tabsRef, activeTabPathRef, @@ -222,7 +241,7 @@ async function navigateToEntry(options: { failNoteOpenTrace(entry.path, 'binary-entry') return } - if (isAlreadyViewingPath(tabsRef, activeTabPathRef, entry.path)) { + if (!forceReload && isAlreadyViewingPath(tabsRef, activeTabPathRef, entry.path)) { syncActiveTabPath(activeTabPathRef, setActiveTabPath, entry.path) finishNoteOpenTrace(entry.path) return @@ -239,13 +258,14 @@ async function navigateToEntry(options: { try { markNoteOpenTrace(entry.path, 'contentLoadStart') - const content = await loadNoteContent(entry.path) + const content = await loadNoteContent(entry.path, forceReload) markNoteOpenTrace(entry.path, 'contentLoadEnd') if (!shouldApplyLoadedEntry({ seq, navSeqRef, cachedContent, content, + forceReload, activeTabPathRef, path: entry.path, })) return @@ -282,7 +302,7 @@ export function useTabManagement(options: TabManagementOptions = {}) { ) => { const seq = ++beforeNavigateSeqRef.current const currentPath = activeTabPathRef.current - if (beforeNavigate && currentPath && currentPath !== targetPath) { + if (beforeNavigate && currentPath && !pathsMatch(currentPath, targetPath)) { try { markNoteOpenTrace(targetPath, 'beforeNavigateStart') await beforeNavigate(currentPath, targetPath) @@ -299,7 +319,7 @@ export function useTabManagement(options: TabManagementOptions = {}) { /** Open a note — replaces the current note (single-note model). */ const handleSelectNote = useCallback(async (entry: VaultEntry) => { - if (entry.path !== activeTabPathRef.current) { + if (!pathsMatch(entry.path, activeTabPathRef.current)) { beginNoteOpenTrace(entry.path, 'select-note') } await executeNavigationWithBoundary(entry.path, () => navigateToEntry({ @@ -325,11 +345,12 @@ export function useTabManagement(options: TabManagementOptions = {}) { }, [executeNavigationWithBoundary]) const handleReplaceActiveTab = useCallback(async (entry: VaultEntry) => { - if (entry.path !== activeTabPathRef.current) { + if (!pathsMatch(entry.path, activeTabPathRef.current)) { beginNoteOpenTrace(entry.path, 'replace-active-tab') } await executeNavigationWithBoundary(entry.path, () => navigateToEntry({ entry, + forceReload: true, navSeqRef, tabsRef, activeTabPathRef, diff --git a/src/utils/pulledVaultRefresh.test.ts b/src/utils/pulledVaultRefresh.test.ts new file mode 100644 index 00000000..101b7c60 --- /dev/null +++ b/src/utils/pulledVaultRefresh.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it, vi } from 'vitest' +import type { VaultEntry } from '../types' +import { refreshPulledVaultState } from './pulledVaultRefresh' + +function makeEntry(path: string, title = 'Test note'): VaultEntry { + return { + path, + title, + filename: path.split('/').pop() ?? 'note.md', + snippet: '', + wordCount: 0, + outgoingLinks: [], + } as VaultEntry +} + +function makeOptions(overrides: Partial[0]> = {}) { + const activeEntry = makeEntry('/vault/active.md', 'Active') + return { + activeTabPath: activeEntry.path, + closeAllTabs: vi.fn(), + hasUnsavedChanges: vi.fn(() => false), + reloadFolders: vi.fn(), + reloadVault: vi.fn().mockResolvedValue([activeEntry]), + reloadViews: vi.fn(), + replaceActiveTab: vi.fn().mockResolvedValue(undefined), + updatedFiles: ['active.md'], + vaultPath: '/vault', + ...overrides, + } +} + +describe('refreshPulledVaultState', () => { + it('reloads vault-derived data and refreshes the active note when pull updated it', async () => { + const options = makeOptions() + + const entries = await refreshPulledVaultState(options) + + expect(entries).toHaveLength(1) + expect(options.reloadVault).toHaveBeenCalledOnce() + expect(options.reloadFolders).toHaveBeenCalledOnce() + expect(options.reloadViews).toHaveBeenCalledOnce() + expect(options.replaceActiveTab).toHaveBeenCalledWith(entries[0]) + expect(options.closeAllTabs).not.toHaveBeenCalled() + }) + + it('reloads the active tab after any successful pull with updates', async () => { + const options = makeOptions({ updatedFiles: ['project/plan.md'] }) + + await refreshPulledVaultState(options) + + expect(options.reloadVault).toHaveBeenCalledOnce() + expect(options.replaceActiveTab).toHaveBeenCalledWith(expect.objectContaining({ path: '/vault/active.md' })) + expect(options.closeAllTabs).not.toHaveBeenCalled() + }) + + it('matches macOS /tmp and /private/tmp aliases when reloading the active tab entry', async () => { + const activeEntry = makeEntry('/private/tmp/tolaria/active.md', 'Active') + const options = makeOptions({ + activeTabPath: activeEntry.path, + reloadVault: vi.fn().mockResolvedValue([activeEntry]), + vaultPath: '/tmp/tolaria', + }) + + await refreshPulledVaultState(options) + + expect(options.replaceActiveTab).toHaveBeenCalledWith(activeEntry) + }) + + it('skips tab replacement when the active note has unsaved edits', async () => { + const options = makeOptions({ + hasUnsavedChanges: vi.fn((path: string) => path === '/vault/active.md'), + }) + + await refreshPulledVaultState(options) + + expect(options.replaceActiveTab).not.toHaveBeenCalled() + expect(options.closeAllTabs).not.toHaveBeenCalled() + }) + + it('closes the tab when the pulled note disappeared from the reloaded vault', async () => { + const options = makeOptions({ + reloadVault: vi.fn().mockResolvedValue([makeEntry('/vault/other.md', 'Other')]), + }) + + await refreshPulledVaultState(options) + + expect(options.replaceActiveTab).not.toHaveBeenCalled() + expect(options.closeAllTabs).toHaveBeenCalledOnce() + }) +}) diff --git a/src/utils/pulledVaultRefresh.ts b/src/utils/pulledVaultRefresh.ts new file mode 100644 index 00000000..e46de9d3 --- /dev/null +++ b/src/utils/pulledVaultRefresh.ts @@ -0,0 +1,49 @@ +import type { VaultEntry } from '../types' + +interface PulledVaultRefreshOptions { + activeTabPath: string | null + closeAllTabs: () => void + hasUnsavedChanges: (path: string) => boolean + reloadFolders: () => Promise | unknown + reloadVault: () => Promise + reloadViews: () => Promise | unknown + replaceActiveTab: (entry: VaultEntry) => Promise + updatedFiles: string[] + vaultPath: string +} + +function normalizePath(path: string): string { + return path + .replaceAll('\\', '/') + .replace(/^\/private\/tmp(?=\/|$)/u, '/tmp') + .replace(/\/+$/u, '') +} + +export async function refreshPulledVaultState(options: PulledVaultRefreshOptions): Promise { + const { + activeTabPath, + closeAllTabs, + hasUnsavedChanges, + reloadFolders, + reloadVault, + reloadViews, + replaceActiveTab, + } = options + + const [entries] = await Promise.all([ + reloadVault(), + Promise.resolve(reloadFolders()), + Promise.resolve(reloadViews()), + ]) + + if (!activeTabPath || hasUnsavedChanges(activeTabPath)) return entries + + const refreshedEntry = entries.find(entry => normalizePath(entry.path) === normalizePath(activeTabPath)) + if (!refreshedEntry) { + closeAllTabs() + return entries + } + + await replaceActiveTab(refreshedEntry) + return entries +}