From d3624edf9f30aa4d79e86a4980bd0064a349927f Mon Sep 17 00:00:00 2001 From: lucaronin Date: Tue, 12 May 2026 08:59:30 +0200 Subject: [PATCH] refactor: consolidate repository git state --- docs/ABSTRACTIONS.md | 7 +- src/App.tsx | 25 ++--- src/hooks/useCommitFlow.test.ts | 59 +++++----- src/hooks/useCommitFlow.ts | 144 +++++++++---------------- src/hooks/useGitRemoteStatus.test.ts | 49 --------- src/hooks/useGitRemoteStatus.ts | 52 --------- src/hooks/useGitRepositories.test.ts | 39 ++++++- src/hooks/useGitRepositories.ts | 156 ++++++++++++++++++++++----- 8 files changed, 259 insertions(+), 272 deletions(-) delete mode 100644 src/hooks/useGitRemoteStatus.test.ts delete mode 100644 src/hooks/useGitRemoteStatus.ts diff --git a/docs/ABSTRACTIONS.md b/docs/ABSTRACTIONS.md index 2df689a4..c524650f 100644 --- a/docs/ABSTRACTIONS.md +++ b/docs/ABSTRACTIONS.md @@ -517,10 +517,11 @@ interface PulseCommit { 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. Overlapping entry reloads and modified-file polls are coalesced with a single trailing rerun so watcher and sync bursts do not stack native vault scans or Git status processes. -`useGitRemoteStatus` is the commit-time companion to `useAutoSync`: -- Re-checks `git_remote_status` when the Commit dialog opens and right before submit +`useGitRepositories` is the commit-time companion to `useAutoSync`: +- Owns repository picker validation plus `get_modified_files` and `git_remote_status` loading for active Git repositories +- Re-checks the selected repository when the Commit dialog opens and right before submit - Converts `hasRemote: false` into a local-only commit path -- Keeps the normal push path unchanged for vaults that do have a remote +- Keeps the normal push path unchanged for repositories that do have a remote `AddRemoteModal` is the explicit recovery path for those local-only vaults: - Opens from the `No remote` status-bar chip and the command palette diff --git a/src/App.tsx b/src/App.tsx index a176493a..a7c903fa 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -31,7 +31,6 @@ import { useNoteActions } from './hooks/useNoteActions' import { planNewTypeCreation } from './hooks/useNoteCreation' import { useCommitFlow } from './hooks/useCommitFlow' import { useGitRepositories } from './hooks/useGitRepositories' -import { useGitRemoteStatus } from './hooks/useGitRemoteStatus' import { useViewMode, type ViewMode } from './hooks/useViewMode' import { useEntryActions } from './hooks/useEntryActions' import { useAppCommands } from './hooks/useAppCommands' @@ -80,7 +79,7 @@ import { DeleteProgressNotice } from './components/DeleteProgressNotice' import { UpdateBanner } from './components/UpdateBanner' import { invoke } from '@tauri-apps/api/core' import { isTauri, mockInvoke } from './mock-tauri' -import type { SidebarSelection, InboxPeriod, VaultEntry, ViewDefinition, GitRemoteStatus, WorkspaceIdentity } from './types' +import type { SidebarSelection, InboxPeriod, VaultEntry, ViewDefinition, WorkspaceIdentity } from './types' import type { NoteListItem } from './utils/ai-context' import { initializeNoteProperties } from './utils/initializeNoteProperties' import { filterEntries, filterInboxEntries, type NoteListFilter } from './utils/noteListHelpers' @@ -408,11 +407,14 @@ function App() { loadMcpConfigSnippet, copyMcpConfig, } = useMcpStatus(resolvedPath, setToastMessage, appLocale) - const gitRemoteStatus = useGitRemoteStatus(resolvedPath) const loadDefaultVaultModifiedFiles = vault.loadModifiedFiles const loadAllGitModifiedFiles = gitSurfaces.loadAllModifiedFiles const loadModifiedFilesForRepository = gitSurfaces.loadModifiedFilesForRepository - const refreshGitRemoteStatus = gitRemoteStatus.refreshRemoteStatus + const refreshRemoteStatusForRepository = gitSurfaces.refreshRemoteStatusForRepository + const refreshGitRemoteStatus = useCallback( + () => refreshRemoteStatusForRepository(resolvedPath), + [refreshRemoteStatusForRepository, resolvedPath], + ) const refreshGitModifiedFiles = useCallback(async () => { await Promise.all([ loadDefaultVaultModifiedFiles(), @@ -420,17 +422,6 @@ function App() { ]) }, [loadAllGitModifiedFiles, loadDefaultVaultModifiedFiles]) const loadVaultModifiedFiles = refreshGitModifiedFiles - const resolveRemoteStatusForVaultPath = useCallback(async (vaultPath: string): Promise => { - if (vaultPath === resolvedPath) return refreshGitRemoteStatus() - - try { - return await (isTauri() - ? invoke('git_remote_status', { vaultPath }) - : mockInvoke('git_remote_status', { vaultPath })) - } catch { - return null - } - }, [refreshGitRemoteStatus, resolvedPath]) useEffect(() => { if (gitRepoState !== 'ready') return @@ -903,8 +894,8 @@ function App() { const commitFlow = useCommitFlow({ savePending: appSave.savePending, loadModifiedFiles: refreshGitModifiedFiles, - resolveRemoteStatus: gitRemoteStatus.refreshRemoteStatus, - resolveRemoteStatusForVaultPath, + loadModifiedFilesForVaultPath: loadModifiedFilesForRepository, + resolveRemoteStatusForVaultPath: refreshRemoteStatusForRepository, setToastMessage, onPushRejected: autoSync.handlePushRejected, automaticVaultPaths: activeGitRepositoryPaths, diff --git a/src/hooks/useCommitFlow.test.ts b/src/hooks/useCommitFlow.test.ts index 6f85e13f..e3aa9ccf 100644 --- a/src/hooks/useCommitFlow.test.ts +++ b/src/hooks/useCommitFlow.test.ts @@ -21,14 +21,16 @@ vi.mock('../lib/telemetry', () => ({ describe('useCommitFlow', () => { let savePending: vi.Mock let loadModifiedFiles: vi.Mock - let resolveRemoteStatus: vi.Mock + let loadModifiedFilesForVaultPath: vi.Mock + let resolveRemoteStatusForVaultPath: vi.Mock let setToastMessage: vi.Mock let onPushRejected: vi.Mock beforeEach(() => { savePending = vi.fn().mockResolvedValue(undefined) loadModifiedFiles = vi.fn().mockResolvedValue(undefined) - resolveRemoteStatus = vi.fn().mockResolvedValue({ branch: 'main', ahead: 0, behind: 0, hasRemote: true }) + loadModifiedFilesForVaultPath = vi.fn().mockResolvedValue([{ path: '/vault/a.md', relativePath: 'a.md', status: 'modified' }]) + resolveRemoteStatusForVaultPath = vi.fn().mockResolvedValue({ branch: 'main', ahead: 0, behind: 0, hasRemote: true }) setToastMessage = vi.fn() onPushRejected = vi.fn() mockTrackEvent.mockReset() @@ -36,7 +38,6 @@ describe('useCommitFlow', () => { mockInvokeFn.mockImplementation((command: string) => { if (command === 'git_commit') return Promise.resolve('[main abc1234] test commit') if (command === 'git_push') return Promise.resolve({ status: 'ok', message: 'Pushed to remote' }) - if (command === 'get_modified_files') return Promise.resolve([{ path: '/vault/a.md', relativePath: 'a.md', status: 'modified' }]) throw new Error(`Unexpected command: ${command}`) }) }) @@ -45,7 +46,8 @@ describe('useCommitFlow', () => { return renderHook(() => useCommitFlow({ savePending, loadModifiedFiles, - resolveRemoteStatus, + loadModifiedFilesForVaultPath, + resolveRemoteStatusForVaultPath, setToastMessage, onPushRejected, vaultPath: '/vault', @@ -54,7 +56,7 @@ describe('useCommitFlow', () => { } it('openCommitDialog saves pending, refreshes files, and sets local mode when no remote exists', async () => { - resolveRemoteStatus.mockResolvedValueOnce({ branch: 'main', ahead: 0, behind: 0, hasRemote: false }) + resolveRemoteStatusForVaultPath.mockResolvedValueOnce({ branch: 'main', ahead: 0, behind: 0, hasRemote: false }) const { result } = renderCommitFlow() await act(async () => { @@ -63,7 +65,7 @@ describe('useCommitFlow', () => { expect(savePending).toHaveBeenCalledTimes(1) expect(loadModifiedFiles).toHaveBeenCalledTimes(1) - expect(resolveRemoteStatus).toHaveBeenCalledTimes(1) + expect(resolveRemoteStatusForVaultPath).toHaveBeenCalledWith('/vault') expect(result.current.showCommitDialog).toBe(true) expect(result.current.commitMode).toBe('local') }) @@ -80,7 +82,7 @@ describe('useCommitFlow', () => { expect(mockInvokeFn).toHaveBeenNthCalledWith(2, 'git_push', { vaultPath: '/vault' }) expect(setToastMessage).toHaveBeenCalledWith('Committed and pushed') expect(loadModifiedFiles).toHaveBeenCalled() - expect(resolveRemoteStatus).toHaveBeenCalledTimes(2) + expect(resolveRemoteStatusForVaultPath).toHaveBeenCalledTimes(2) expect(mockTrackEvent).toHaveBeenCalledWith('commit_made', undefined) expect(result.current.showCommitDialog).toBe(false) }) @@ -93,9 +95,9 @@ describe('useCommitFlow', () => { }) expect(savePending).toHaveBeenCalledTimes(1) - expect(mockInvokeFn).toHaveBeenNthCalledWith(1, 'get_modified_files', { vaultPath: '/vault' }) - expect(mockInvokeFn).toHaveBeenNthCalledWith(2, 'git_commit', { vaultPath: '/vault', message: 'Updated 1 note' }) - expect(mockInvokeFn).toHaveBeenNthCalledWith(3, 'git_push', { vaultPath: '/vault' }) + expect(loadModifiedFilesForVaultPath).toHaveBeenCalledWith('/vault') + expect(mockInvokeFn).toHaveBeenNthCalledWith(1, 'git_commit', { vaultPath: '/vault', message: 'Updated 1 note' }) + expect(mockInvokeFn).toHaveBeenNthCalledWith(2, 'git_push', { vaultPath: '/vault' }) expect(setToastMessage).toHaveBeenCalledWith('Committed and pushed') }) @@ -106,14 +108,12 @@ describe('useCommitFlow', () => { behind: 0, hasRemote: true, }) - mockInvokeFn.mockImplementation((command: string, args: { vaultPath?: string }) => { - if (command === 'get_modified_files') { - return Promise.resolve([{ - path: `${args.vaultPath}/note.md`, - relativePath: 'note.md', - status: 'modified', - }]) - } + const loadModifiedFilesForVaultPath = vi.fn((vaultPath: string) => Promise.resolve([{ + path: `${vaultPath}/note.md`, + relativePath: 'note.md', + status: 'modified', + }])) + mockInvokeFn.mockImplementation((command: string) => { if (command === 'git_commit') return Promise.resolve('[main abc1234] test commit') if (command === 'git_push') return Promise.resolve({ status: 'ok', message: 'Pushed to remote' }) throw new Error(`Unexpected command: ${command}`) @@ -121,6 +121,7 @@ describe('useCommitFlow', () => { const { result } = renderCommitFlow({ automaticVaultPaths: ['/vault', '/work'], + loadModifiedFilesForVaultPath, resolveRemoteStatusForVaultPath, }) @@ -132,15 +133,18 @@ describe('useCommitFlow', () => { expect(mockInvokeFn).toHaveBeenCalledWith('git_push', { vaultPath: '/vault' }) expect(mockInvokeFn).toHaveBeenCalledWith('git_commit', { vaultPath: '/work', message: 'Updated 1 note' }) expect(mockInvokeFn).toHaveBeenCalledWith('git_push', { vaultPath: '/work' }) + expect(loadModifiedFilesForVaultPath).toHaveBeenCalledWith('/vault') + expect(loadModifiedFilesForVaultPath).toHaveBeenCalledWith('/work') + expect(resolveRemoteStatusForVaultPath).toHaveBeenCalledWith('/vault') expect(resolveRemoteStatusForVaultPath).toHaveBeenCalledWith('/work') expect(loadModifiedFiles).toHaveBeenCalled() expect(setToastMessage).toHaveBeenCalledWith('AutoGit checkpointed 2 repositories') }) it('runAutomaticCheckpoint retries push-only when local commits are already ahead', async () => { - resolveRemoteStatus.mockResolvedValue({ branch: 'main', ahead: 2, behind: 0, hasRemote: true }) + resolveRemoteStatusForVaultPath.mockResolvedValue({ branch: 'main', ahead: 2, behind: 0, hasRemote: true }) + loadModifiedFilesForVaultPath.mockResolvedValue([]) mockInvokeFn.mockImplementation((command: string) => { - if (command === 'get_modified_files') return Promise.resolve([]) if (command === 'git_push') return Promise.resolve({ status: 'ok', message: 'Pushed to remote' }) throw new Error(`Unexpected command: ${command}`) }) @@ -151,18 +155,15 @@ describe('useCommitFlow', () => { await result.current.runAutomaticCheckpoint() }) - expect(mockInvokeFn).toHaveBeenCalledTimes(2) - expect(mockInvokeFn).toHaveBeenNthCalledWith(1, 'get_modified_files', { vaultPath: '/vault' }) - expect(mockInvokeFn).toHaveBeenNthCalledWith(2, 'git_push', { vaultPath: '/vault' }) + expect(loadModifiedFilesForVaultPath).toHaveBeenCalledWith('/vault') + expect(mockInvokeFn).toHaveBeenCalledTimes(1) + expect(mockInvokeFn).toHaveBeenNthCalledWith(1, 'git_push', { vaultPath: '/vault' }) expect(setToastMessage).toHaveBeenCalledWith('Pushed committed changes') expect(mockTrackEvent).not.toHaveBeenCalled() }) it('runAutomaticCheckpoint reports when there is nothing to commit or push', async () => { - mockInvokeFn.mockImplementation((command: string) => { - if (command === 'get_modified_files') return Promise.resolve([]) - throw new Error(`Unexpected command: ${command}`) - }) + loadModifiedFilesForVaultPath.mockResolvedValue([]) const { result } = renderCommitFlow() @@ -175,7 +176,7 @@ describe('useCommitFlow', () => { }) it('handleCommitPush commits locally and skips push when no remote is configured', async () => { - resolveRemoteStatus.mockResolvedValue({ branch: 'main', ahead: 0, behind: 0, hasRemote: false }) + resolveRemoteStatusForVaultPath.mockResolvedValue({ branch: 'main', ahead: 0, behind: 0, hasRemote: false }) mockInvokeFn.mockImplementation((command: string) => { if (command === 'git_commit') return Promise.resolve('[main abc1234] test message') throw new Error(`Unexpected command: ${command}`) @@ -224,7 +225,7 @@ describe('useCommitFlow', () => { ({ manualVaultPath }) => useCommitFlow({ savePending, loadModifiedFiles, - resolveRemoteStatus, + loadModifiedFilesForVaultPath, resolveRemoteStatusForVaultPath, setToastMessage, onPushRejected, diff --git a/src/hooks/useCommitFlow.ts b/src/hooks/useCommitFlow.ts index 0e927d51..4b801ebe 100644 --- a/src/hooks/useCommitFlow.ts +++ b/src/hooks/useCommitFlow.ts @@ -22,8 +22,8 @@ interface AutomaticCheckpointOptions { interface CommitFlowConfig { savePending: () => Promise loadModifiedFiles: () => Promise - resolveRemoteStatus: () => Promise - resolveRemoteStatusForVaultPath?: (vaultPath: string) => Promise + loadModifiedFilesForVaultPath: (vaultPath: string) => Promise + resolveRemoteStatusForVaultPath: (vaultPath: string) => Promise setToastMessage: (msg: string | null) => void onPushRejected?: () => void automaticVaultPaths?: string[] @@ -69,13 +69,24 @@ interface RepositoryCheckpointResult { type AutomaticCheckpointRunConfig = Pick< CommitFlowConfig, | 'loadModifiedFiles' + | 'loadModifiedFilesForVaultPath' | 'onPushRejected' - | 'resolveRemoteStatus' | 'resolveRemoteStatusForVaultPath' | 'setToastMessage' | 'vaultPath' > +type FinalizeCheckpointConfig = Pick< + CommitFlowConfig, + 'loadModifiedFiles' | 'resolveRemoteStatusForVaultPath' | 'setToastMessage' | 'onPushRejected' +> + +interface FinalizeCheckpointArgs extends FinalizeCheckpointConfig { + result: CommitResult + toastMessage: string + vaultPaths: string[] +} + function commitModeFromRemoteStatus(remoteStatus: GitRemoteStatus | null): CommitMode { return remoteStatus?.hasRemote === false ? 'local' : 'push' } @@ -97,22 +108,6 @@ async function pushCommittedChanges({ vaultPath }: VaultPathArgs): Promise('git_push', { vaultPath }) } -async function readModifiedFiles({ vaultPath }: VaultPathArgs): Promise { - if (!isTauri()) { - return mockInvoke('get_modified_files', { vaultPath }) - } - - return invoke('get_modified_files', { vaultPath }) -} - -async function readRemoteStatus({ vaultPath }: VaultPathArgs): Promise { - if (!isTauri()) { - return mockInvoke('git_remote_status', { vaultPath }) - } - - return invoke('git_remote_status', { vaultPath }) -} - async function executeCommitAction({ vaultPath, message, @@ -215,31 +210,12 @@ function checkpointVaultPaths({ return paths.length > 0 ? paths : [vaultPath] } -async function resolveRemoteStatusForPath( - vaultPath: string, - config: Pick, -): Promise { - if (vaultPath === config.vaultPath) { - return config.resolveRemoteStatus() - } - - if (config.resolveRemoteStatusForVaultPath) { - return config.resolveRemoteStatusForVaultPath(vaultPath) - } - - try { - return await readRemoteStatus({ vaultPath }) - } catch { - return null - } -} - async function checkpointRepository( vaultPath: string, - config: Pick, + config: Pick, ): Promise { - const remoteStatus = await resolveRemoteStatusForPath(vaultPath, config) - const modifiedFiles = await readModifiedFiles({ vaultPath }) + const remoteStatus = await config.resolveRemoteStatusForVaultPath(vaultPath) + const modifiedFiles = await config.loadModifiedFilesForVaultPath(vaultPath) const message = generateAutomaticCommitMessage(modifiedFiles) const command = createAutomaticCheckpointCommand({ remoteStatus, vaultPath, message }) @@ -273,37 +249,40 @@ function multiRepositoryCheckpointToast(results: RepositoryCheckpointResult[]): async function runCheckpointRefresh({ loadModifiedFiles, - resolveRemoteStatus, -}: Pick): Promise { + resolveRemoteStatusForVaultPath, + vaultPaths, +}: Pick & { + vaultPaths: string[] +}): Promise { await loadModifiedFiles() - await resolveRemoteStatus() + await Promise.all([...new Set(vaultPaths)].map((vaultPath) => resolveRemoteStatusForVaultPath(vaultPath))) } -async function finalizeCheckpoint({ - result, - toastMessage, - loadModifiedFiles, - resolveRemoteStatus, - setToastMessage, - onPushRejected, -}: Pick & { - result: CommitResult - toastMessage: string -}): Promise { +async function finalizeCheckpoint(args: FinalizeCheckpointArgs): Promise { + const { + result, + toastMessage, + loadModifiedFiles, + resolveRemoteStatusForVaultPath, + setToastMessage, + onPushRejected, + vaultPaths, + } = args + setToastMessage(toastMessage) if (isPushRejected(result)) { onPushRejected?.() } - await runCheckpointRefresh({ loadModifiedFiles, resolveRemoteStatus }) + await runCheckpointRefresh({ loadModifiedFiles, resolveRemoteStatusForVaultPath, vaultPaths }) } async function runSingleRepositoryCheckpoint( targetVaultPath: string, config: AutomaticCheckpointRunConfig, ): Promise { - const remoteStatus = await resolveRemoteStatusForPath(targetVaultPath, config) - const modifiedFiles = await readModifiedFiles({ vaultPath: targetVaultPath }) + const remoteStatus = await config.resolveRemoteStatusForVaultPath(targetVaultPath) + const modifiedFiles = await config.loadModifiedFilesForVaultPath(targetVaultPath) const message = generateAutomaticCommitMessage(modifiedFiles) const command = createAutomaticCheckpointCommand({ remoteStatus, @@ -321,9 +300,10 @@ async function runSingleRepositoryCheckpoint( result, toastMessage: checkpointToastMessage(result, action), loadModifiedFiles: config.loadModifiedFiles, - resolveRemoteStatus: config.resolveRemoteStatus, + resolveRemoteStatusForVaultPath: config.resolveRemoteStatusForVaultPath, setToastMessage: config.setToastMessage, onPushRejected: config.onPushRejected, + vaultPaths: [targetVaultPath], }) return true } @@ -361,7 +341,8 @@ async function runMultipleRepositoryCheckpoint( config.setToastMessage(multiRepositoryCheckpointToast(results)) await runCheckpointRefresh({ loadModifiedFiles: config.loadModifiedFiles, - resolveRemoteStatus: config.resolveRemoteStatus, + resolveRemoteStatusForVaultPath: config.resolveRemoteStatusForVaultPath, + vaultPaths: targetVaultPaths, }) return results.some((result) => result.status === 'executed') } @@ -370,7 +351,7 @@ function useAutomaticCheckpointAction({ checkpointInFlightRef, savePending, loadModifiedFiles, - resolveRemoteStatus, + loadModifiedFilesForVaultPath, resolveRemoteStatusForVaultPath, setToastMessage, onPushRejected, @@ -393,8 +374,8 @@ function useAutomaticCheckpointAction({ const targetVaultPaths = checkpointVaultPaths({ automaticVaultPaths, vaultPath }) const runConfig = { loadModifiedFiles, + loadModifiedFilesForVaultPath, onPushRejected, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, setToastMessage, vaultPath, @@ -413,8 +394,8 @@ function useAutomaticCheckpointAction({ automaticVaultPaths, checkpointInFlightRef, loadModifiedFiles, + loadModifiedFilesForVaultPath, onPushRejected, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, savePending, setToastMessage, @@ -426,7 +407,6 @@ function useManualCommitPushAction({ checkpointInFlightRef, savePending, loadModifiedFiles, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, setToastMessage, onPushRejected, @@ -445,11 +425,7 @@ function useManualCommitPushAction({ try { await savePending() const targetVaultPath = manualVaultPath || vaultPath - const remoteStatus = await resolveRemoteStatusForPath(targetVaultPath, { - resolveRemoteStatus, - resolveRemoteStatusForVaultPath, - vaultPath, - }) + const remoteStatus = await resolveRemoteStatusForVaultPath(targetVaultPath) const result = await executeCommitAction({ vaultPath: targetVaultPath, message, @@ -461,9 +437,10 @@ function useManualCommitPushAction({ result, toastMessage: commitToastMessage(result), loadModifiedFiles, - resolveRemoteStatus, + resolveRemoteStatusForVaultPath, setToastMessage, onPushRejected, + vaultPaths: [targetVaultPath], }) } catch (err) { console.error('Commit failed:', err) @@ -476,7 +453,6 @@ function useManualCommitPushAction({ loadModifiedFiles, manualVaultPath, onPushRejected, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, savePending, setShowCommitDialog, @@ -488,14 +464,13 @@ function useManualCommitPushAction({ function useCommitModeRefresh({ commitModeVaultPathRef, manualVaultPath, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, setCommitMode, showCommitDialog, vaultPath, }: Pick< CommitFlowConfig, - 'manualVaultPath' | 'resolveRemoteStatus' | 'resolveRemoteStatusForVaultPath' | 'vaultPath' + 'manualVaultPath' | 'resolveRemoteStatusForVaultPath' | 'vaultPath' > & { commitModeVaultPathRef: MutableRefObject setCommitMode: (mode: CommitMode) => void @@ -508,11 +483,7 @@ function useCommitModeRefresh({ const targetVaultPath = manualVaultPath || vaultPath if (commitModeVaultPathRef.current === targetVaultPath) return - void resolveRemoteStatusForPath(targetVaultPath, { - resolveRemoteStatus, - resolveRemoteStatusForVaultPath, - vaultPath, - }).then((remoteStatus) => { + void resolveRemoteStatusForVaultPath(targetVaultPath).then((remoteStatus) => { if (cancelled) return commitModeVaultPathRef.current = targetVaultPath setCommitMode(commitModeFromRemoteStatus(remoteStatus)) @@ -524,7 +495,6 @@ function useCommitModeRefresh({ }, [ commitModeVaultPathRef, manualVaultPath, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, setCommitMode, showCommitDialog, @@ -536,7 +506,6 @@ function useOpenCommitDialog({ commitModeVaultPathRef, loadModifiedFiles, manualVaultPath, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, savePending, setCommitMode, @@ -546,7 +515,6 @@ function useOpenCommitDialog({ CommitFlowConfig, | 'loadModifiedFiles' | 'manualVaultPath' - | 'resolveRemoteStatus' | 'resolveRemoteStatusForVaultPath' | 'savePending' | 'vaultPath' @@ -559,11 +527,7 @@ function useOpenCommitDialog({ await savePending() await loadModifiedFiles() const targetVaultPath = manualVaultPath || vaultPath - const remoteStatus = await resolveRemoteStatusForPath(targetVaultPath, { - resolveRemoteStatus, - resolveRemoteStatusForVaultPath, - vaultPath, - }) + const remoteStatus = await resolveRemoteStatusForVaultPath(targetVaultPath) commitModeVaultPathRef.current = targetVaultPath setCommitMode(commitModeFromRemoteStatus(remoteStatus)) setShowCommitDialog(true) @@ -571,7 +535,6 @@ function useOpenCommitDialog({ commitModeVaultPathRef, loadModifiedFiles, manualVaultPath, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, savePending, setCommitMode, @@ -584,7 +547,7 @@ function useOpenCommitDialog({ export function useCommitFlow({ savePending, loadModifiedFiles, - resolveRemoteStatus, + loadModifiedFilesForVaultPath, resolveRemoteStatusForVaultPath, setToastMessage, onPushRejected, @@ -601,7 +564,6 @@ export function useCommitFlow({ commitModeVaultPathRef, loadModifiedFiles, manualVaultPath, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, savePending, setCommitMode, @@ -613,7 +575,7 @@ export function useCommitFlow({ checkpointInFlightRef, savePending, loadModifiedFiles, - resolveRemoteStatus, + loadModifiedFilesForVaultPath, resolveRemoteStatusForVaultPath, setToastMessage, onPushRejected, @@ -625,7 +587,6 @@ export function useCommitFlow({ checkpointInFlightRef, savePending, loadModifiedFiles, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, setToastMessage, onPushRejected, @@ -636,7 +597,6 @@ export function useCommitFlow({ useCommitModeRefresh({ commitModeVaultPathRef, manualVaultPath, - resolveRemoteStatus, resolveRemoteStatusForVaultPath, setCommitMode, showCommitDialog, diff --git a/src/hooks/useGitRemoteStatus.test.ts b/src/hooks/useGitRemoteStatus.test.ts deleted file mode 100644 index af3bcc22..00000000 --- a/src/hooks/useGitRemoteStatus.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { renderHook, waitFor, act } from '@testing-library/react' -import { describe, expect, it, vi, beforeEach } from 'vitest' -import { useGitRemoteStatus } from './useGitRemoteStatus' - -const mockInvokeFn = vi.fn() - -vi.mock('@tauri-apps/api/core', () => ({ - invoke: (command: string, args: Record) => mockInvokeFn(command, args), -})) - -vi.mock('../mock-tauri', () => ({ - isTauri: () => false, - mockInvoke: (command: string, args: Record) => mockInvokeFn(command, args), -})) - -describe('useGitRemoteStatus', () => { - beforeEach(() => { - vi.clearAllMocks() - }) - - it('loads remote status on mount', async () => { - mockInvokeFn.mockResolvedValue({ branch: 'main', ahead: 0, behind: 0, hasRemote: false }) - - const { result } = renderHook(() => useGitRemoteStatus('/vault')) - - await waitFor(() => { - expect(result.current.remoteStatus).toEqual({ branch: 'main', ahead: 0, behind: 0, hasRemote: false }) - }) - expect(mockInvokeFn).toHaveBeenCalledWith('git_remote_status', { vaultPath: '/vault' }) - }) - - it('refreshRemoteStatus updates the current remote state', async () => { - mockInvokeFn - .mockResolvedValueOnce({ branch: 'main', ahead: 0, behind: 0, hasRemote: true }) - .mockResolvedValueOnce({ branch: 'main', ahead: 1, behind: 0, hasRemote: false }) - - const { result } = renderHook(() => useGitRemoteStatus('/vault')) - - await waitFor(() => { - expect(result.current.remoteStatus?.hasRemote).toBe(true) - }) - - await act(async () => { - await result.current.refreshRemoteStatus() - }) - - expect(result.current.remoteStatus).toEqual({ branch: 'main', ahead: 1, behind: 0, hasRemote: false }) - }) -}) diff --git a/src/hooks/useGitRemoteStatus.ts b/src/hooks/useGitRemoteStatus.ts deleted file mode 100644 index 6b0c05c0..00000000 --- a/src/hooks/useGitRemoteStatus.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { useCallback, useEffect, useState } from 'react' -import { invoke } from '@tauri-apps/api/core' -import { isTauri, mockInvoke } from '../mock-tauri' -import type { GitRemoteStatus } from '../types' - -function tauriCall(command: string, args: Record): Promise { - return isTauri() ? invoke(command, args) : mockInvoke(command, args) -} - -export interface GitRemoteState { - remoteStatus: GitRemoteStatus | null - refreshRemoteStatus: () => Promise -} - -async function readRemoteStatus(vaultPath: string): Promise { - return tauriCall('git_remote_status', { vaultPath }) -} - -export function useGitRemoteStatus(vaultPath: string): GitRemoteState { - const [remoteStatus, setRemoteStatus] = useState(null) - - const refreshRemoteStatus = useCallback(async () => { - try { - const status = await readRemoteStatus(vaultPath) - setRemoteStatus(status) - return status - } catch { - setRemoteStatus(null) - return null - } - }, [vaultPath]) - - useEffect(() => { - let cancelled = false - - async function loadRemoteStatus() { - try { - const status = await readRemoteStatus(vaultPath) - if (!cancelled) setRemoteStatus(status) - } catch { - if (!cancelled) setRemoteStatus(null) - } - } - - void loadRemoteStatus() - return () => { - cancelled = true - } - }, [vaultPath]) - - return { remoteStatus, refreshRemoteStatus } -} diff --git a/src/hooks/useGitRepositories.test.ts b/src/hooks/useGitRepositories.test.ts index e323a4dd..d6f775b3 100644 --- a/src/hooks/useGitRepositories.test.ts +++ b/src/hooks/useGitRepositories.test.ts @@ -1,7 +1,7 @@ import { act, renderHook, waitFor } from '@testing-library/react' import { describe, expect, it, vi, beforeEach } from 'vitest' import { mockInvoke } from '../mock-tauri' -import type { ModifiedFile } from '../types' +import type { GitRemoteStatus, ModifiedFile } from '../types' import { useGitRepositories } from './useGitRepositories' vi.mock('@tauri-apps/api/core', () => ({ invoke: vi.fn() })) @@ -26,6 +26,15 @@ function modifiedFile(relativePath: string): ModifiedFile { } } +function remoteStatus(hasRemote: boolean): GitRemoteStatus { + return { + branch: 'main', + ahead: 0, + behind: 0, + hasRemote, + } +} + describe('useGitRepositories', () => { beforeEach(() => { vi.clearAllMocks() @@ -63,4 +72,32 @@ describe('useGitRepositories', () => { }) expect(result.current.changesModifiedFiles.map((file) => file.relativePath)).toEqual(['new.md']) }) + + it('refreshes remote status through the repository state owner', async () => { + const repositories = [ + { path: '/default', label: 'Default', defaultForNewNotes: true }, + { path: '/work', label: 'Work', defaultForNewNotes: false }, + ] + vi.mocked(mockInvoke).mockImplementation((command, args) => { + if (command === 'get_modified_files') return Promise.resolve([]) + if (command === 'git_remote_status') { + return Promise.resolve(remoteStatus(args.vaultPath === '/default')) + } + throw new Error(`Unexpected command: ${command}`) + }) + + const { result } = renderHook(() => useGitRepositories({ + defaultVaultPath: '/default', + repositories, + })) + + await waitFor(() => expect(mockInvoke).toHaveBeenCalledWith('get_modified_files', { vaultPath: '/default' })) + + await act(async () => { + await result.current.refreshRemoteStatusForRepository('/work') + }) + + expect(mockInvoke).toHaveBeenCalledWith('git_remote_status', { vaultPath: '/work' }) + expect(result.current.remoteStatusForRepository('/work')).toEqual(remoteStatus(false)) + }) }) diff --git a/src/hooks/useGitRepositories.ts b/src/hooks/useGitRepositories.ts index 12ef1f6f..b5a3a828 100644 --- a/src/hooks/useGitRepositories.ts +++ b/src/hooks/useGitRepositories.ts @@ -1,7 +1,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { invoke } from '@tauri-apps/api/core' import { isTauri, mockInvoke } from '../mock-tauri' -import type { ModifiedFile } from '../types' +import type { GitRemoteStatus, ModifiedFile } from '../types' import type { GitRepositoryOption } from '../utils/gitRepositories' import { validGitRepositoryPath } from '../utils/gitRepositories' @@ -10,47 +10,83 @@ interface RepositoryModifiedFiles { files: ModifiedFile[] } +interface RepositoryRemoteStatus { + error: string | null + status: GitRemoteStatus | null +} + interface UseGitRepositoriesOptions { defaultVaultPath: string repositories: GitRepositoryOption[] } +interface RepositorySelectionConfig { + fallbackPath: string + repositories: GitRepositoryOption[] +} + +interface RepositoryStateLookup { + byRepository: ReadonlyMap + fallback: T + path: string +} + +interface RepositoryLoadState { + loadIds: RepositoryLoadIds + path: string +} + +interface RepositoryLoadCompletion extends RepositoryLoadState { + id: number +} + +interface RepositoryFilesArgs { + files: ModifiedFile[] + vaultPath: string +} + const EMPTY_MODIFIED_FILES: RepositoryModifiedFiles = { error: null, files: [], } +const EMPTY_REMOTE_STATUS: RepositoryRemoteStatus = { + error: null, + status: null, +} + type RepositoryLoadIds = Map function tauriCall(command: string, args: Record): Promise { return isTauri() ? invoke(command, args) : mockInvoke(command, args) } -function withRepositoryPath(files: ModifiedFile[], vaultPath: string): ModifiedFile[] { +function withRepositoryPath({ files, vaultPath }: RepositoryFilesArgs): ModifiedFile[] { return files.map((file) => ({ ...file, vaultPath })) } -function modifiedFilesState( - byRepository: Record, - path: string, -): RepositoryModifiedFiles { - return byRepository[path] ?? EMPTY_MODIFIED_FILES +function repositoryState({ byRepository, fallback, path }: RepositoryStateLookup): T { + return byRepository.get(path) ?? fallback } -function nextRepositoryLoadId(loadIds: RepositoryLoadIds, path: string): number { +function nextRepositoryLoadId({ loadIds, path }: RepositoryLoadState): number { const nextId = (loadIds.get(path) ?? 0) + 1 loadIds.set(path, nextId) return nextId } -function isLatestRepositoryLoad(loadIds: RepositoryLoadIds, path: string, id: number): boolean { +function isLatestRepositoryLoad({ loadIds, path, id }: RepositoryLoadCompletion): boolean { return loadIds.get(path) === id } -function useValidatedRepositoryPath( - repositories: GitRepositoryOption[], - fallbackPath: string, -) { +function repositoryErrorMessage(error: unknown, fallback: string): string { + return typeof error === 'string' ? error : fallback +} + +function useValidatedRepositoryPath({ + repositories, + fallbackPath, +}: RepositorySelectionConfig) { const [requestedPath, setRequestedPath] = useState(fallbackPath) const selectedPath = validGitRepositoryPath(requestedPath, repositories, fallbackPath) @@ -62,26 +98,26 @@ function useValidatedRepositoryPath( } function useRepositoryModifiedFiles(repositories: GitRepositoryOption[]) { - const [byRepository, setByRepository] = useState>({}) + const [byRepository, setByRepository] = useState>(() => new Map()) const loadIdsRef = useRef(new Map()) const loadModifiedFilesForRepository = useCallback(async (vaultPath: string) => { if (!vaultPath.trim()) return [] as ModifiedFile[] - const loadId = nextRepositoryLoadId(loadIdsRef.current, vaultPath) + const loadId = nextRepositoryLoadId({ loadIds: loadIdsRef.current, path: vaultPath }) try { - const files = withRepositoryPath( - await tauriCall('get_modified_files', { vaultPath }), + const files = withRepositoryPath({ + files: await tauriCall('get_modified_files', { vaultPath }), vaultPath, - ) - if (isLatestRepositoryLoad(loadIdsRef.current, vaultPath, loadId)) { - setByRepository((current) => ({ ...current, [vaultPath]: { error: null, files } })) + }) + if (isLatestRepositoryLoad({ loadIds: loadIdsRef.current, path: vaultPath, id: loadId })) { + setByRepository((current) => new Map(current).set(vaultPath, { error: null, files })) } return files } catch (error) { - const message = typeof error === 'string' ? error : 'Failed to load changes' - if (isLatestRepositoryLoad(loadIdsRef.current, vaultPath, loadId)) { - setByRepository((current) => ({ ...current, [vaultPath]: { error: message, files: [] } })) + const message = repositoryErrorMessage(error, 'Failed to load changes') + if (isLatestRepositoryLoad({ loadIds: loadIdsRef.current, path: vaultPath, id: loadId })) { + setByRepository((current) => new Map(current).set(vaultPath, { error: message, files: [] })) } return [] as ModifiedFile[] } @@ -99,21 +135,79 @@ function useRepositoryModifiedFiles(repositories: GitRepositoryOption[]) { return { byRepository, loadAllModifiedFiles, loadModifiedFilesForRepository } } +function useRepositoryRemoteStatuses(repositories: GitRepositoryOption[]) { + const [byRepository, setByRepository] = useState>(() => new Map()) + const loadIdsRef = useRef(new Map()) + + const refreshRemoteStatusForRepository = useCallback(async (vaultPath: string) => { + if (!vaultPath.trim()) return null + const loadId = nextRepositoryLoadId({ loadIds: loadIdsRef.current, path: vaultPath }) + + try { + const status = await tauriCall('git_remote_status', { vaultPath }) + if (isLatestRepositoryLoad({ loadIds: loadIdsRef.current, path: vaultPath, id: loadId })) { + setByRepository((current) => new Map(current).set(vaultPath, { error: null, status })) + } + return status + } catch (error) { + const message = repositoryErrorMessage(error, 'Failed to load remote status') + if (isLatestRepositoryLoad({ loadIds: loadIdsRef.current, path: vaultPath, id: loadId })) { + setByRepository((current) => new Map(current).set(vaultPath, { error: message, status: null })) + } + return null + } + }, []) + + const refreshAllRemoteStatuses = useCallback(async () => { + await Promise.all(repositories.map((repository) => refreshRemoteStatusForRepository(repository.path))) + }, [refreshRemoteStatusForRepository, repositories]) + + return { byRepository, refreshAllRemoteStatuses, refreshRemoteStatusForRepository } +} + export function useGitRepositories({ defaultVaultPath, repositories, }: UseGitRepositoriesOptions) { - const [changesRepositoryPath, setChangesRepositoryPath] = useValidatedRepositoryPath(repositories, defaultVaultPath) - const [historyRepositoryPath, setHistoryRepositoryPath] = useValidatedRepositoryPath(repositories, defaultVaultPath) - const [commitRepositoryPath, setCommitRepositoryPath] = useValidatedRepositoryPath(repositories, defaultVaultPath) + const selectionConfig = { fallbackPath: defaultVaultPath, repositories } + const [changesRepositoryPath, setChangesRepositoryPath] = useValidatedRepositoryPath(selectionConfig) + const [historyRepositoryPath, setHistoryRepositoryPath] = useValidatedRepositoryPath(selectionConfig) + const [commitRepositoryPath, setCommitRepositoryPath] = useValidatedRepositoryPath(selectionConfig) const { byRepository, loadAllModifiedFiles, loadModifiedFilesForRepository } = useRepositoryModifiedFiles(repositories) + const { + byRepository: remoteStatusByRepository, + refreshAllRemoteStatuses, + refreshRemoteStatusForRepository, + } = useRepositoryRemoteStatuses(repositories) const allModifiedFiles = useMemo( - () => repositories.flatMap((repository) => modifiedFilesState(byRepository, repository.path).files), + () => repositories.flatMap((repository) => repositoryState({ + byRepository, + fallback: EMPTY_MODIFIED_FILES, + path: repository.path, + }).files), [byRepository, repositories], ) - const changesState = modifiedFilesState(byRepository, changesRepositoryPath) - const commitState = modifiedFilesState(byRepository, commitRepositoryPath) + const changesState = repositoryState({ + byRepository, + fallback: EMPTY_MODIFIED_FILES, + path: changesRepositoryPath, + }) + const commitState = repositoryState({ + byRepository, + fallback: EMPTY_MODIFIED_FILES, + path: commitRepositoryPath, + }) + const remoteStatusForRepository = useCallback((path: string) => repositoryState({ + byRepository: remoteStatusByRepository, + fallback: EMPTY_REMOTE_STATUS, + path, + }).status, [remoteStatusByRepository]) + const remoteStatusErrorForRepository = useCallback((path: string) => repositoryState({ + byRepository: remoteStatusByRepository, + fallback: EMPTY_REMOTE_STATUS, + path, + }).error, [remoteStatusByRepository]) return { allModifiedFiles, @@ -125,6 +219,10 @@ export function useGitRepositories({ historyRepositoryPath, loadAllModifiedFiles, loadModifiedFilesForRepository, + refreshAllRemoteStatuses, + refreshRemoteStatusForRepository, + remoteStatusErrorForRepository, + remoteStatusForRepository, setChangesRepositoryPath, setCommitRepositoryPath, setHistoryRepositoryPath,