diff --git a/packages/ui/src/lib/gitApiHttp.test.ts b/packages/ui/src/lib/gitApiHttp.test.ts index 4a58391c..4dc618b3 100644 --- a/packages/ui/src/lib/gitApiHttp.test.ts +++ b/packages/ui/src/lib/gitApiHttp.test.ts @@ -1,10 +1,30 @@ import { describe, expect, test } from 'bun:test'; import { + abortMerge, + abortRebase, + applyGitStash, + checkoutBranch, + checkoutCommit, + cherryPick, + continueMerge, + continueRebase, + createBranch, + deleteGitBranch, + deleteRemoteBranch, + dropGitStash, getGitBranches, getGitStatus, gitFetch, + merge, + popGitStash, + rebase, + removeRemote, + renameBranch, + resetToCommit, + revertCommit, stageGitFile, stageGitFiles, + stashGitChanges, unstageGitFile, unstageGitFiles, } from './gitApiHttp'; @@ -169,6 +189,200 @@ describe('gitApiHttp status cache', () => { }); }); +const statusPayload = (overrides: Record = {}) => ({ + current: 'main', + tracking: null, + ahead: 0, + behind: 0, + files: [], + isClean: true, + ...overrides, +}); + +const jsonResponse = (payload: unknown) => new Response(JSON.stringify(payload), { + status: 200, + headers: { 'Content-Type': 'application/json' }, +}); + +const installStatusMutationFetchMock = () => { + const mock = { + statusUrls: [] as string[], + behind: 0, + }; + globalThis.fetch = (async (input) => { + const url = String(input); + if (url.startsWith('/api/git/status')) { + mock.statusUrls.push(url); + return jsonResponse(statusPayload({ behind: mock.behind })); + } + return jsonResponse({ success: true }); + }) as typeof fetch; + return mock; +}; + +/** + * Seeds the status cache, performs the mutation, and asserts the next status + * read issues a fresh request that observes the post-mutation state instead of + * serving the pre-mutation cache entry. + */ +const expectStatusInvalidatedBy = async ( + directory: string, + mutate: () => Promise +): Promise => { + const mock = installStatusMutationFetchMock(); + + const seeded = await getGitStatus(directory); + expect(seeded.behind).toBe(0); + + mock.behind = 2; + const cached = await getGitStatus(directory); + expect(cached.behind).toBe(0); + expect(mock.statusUrls).toHaveLength(1); + + await mutate(); + + const refreshed = await getGitStatus(directory); + expect(refreshed.behind).toBe(2); + expect(mock.statusUrls).toHaveLength(2); +}; + +describe('gitApiHttp post-mutation status invalidation (#2281)', () => { + test('checkout and branch mutations invalidate cached status', async () => { + installWindowMock(); + try { + await expectStatusInvalidatedBy('/repo-2281-checkout', () => checkoutBranch('/repo-2281-checkout', 'feature')); + await expectStatusInvalidatedBy('/repo-2281-create-branch', () => createBranch('/repo-2281-create-branch', 'feature/new')); + await expectStatusInvalidatedBy('/repo-2281-rename-branch', () => renameBranch('/repo-2281-rename-branch', 'old', 'new')); + await expectStatusInvalidatedBy('/repo-2281-delete-branch', () => deleteGitBranch('/repo-2281-delete-branch', { branch: 'feature/old' })); + } finally { + restoreMocks(); + } + }); + + test('stash lifecycle mutations invalidate cached status', async () => { + installWindowMock(); + try { + await expectStatusInvalidatedBy('/repo-2281-stash', () => stashGitChanges('/repo-2281-stash', { message: 'WIP' })); + await expectStatusInvalidatedBy('/repo-2281-stash-apply', () => applyGitStash('/repo-2281-stash-apply', { ref: 'stash@{0}' })); + await expectStatusInvalidatedBy('/repo-2281-stash-pop', () => popGitStash('/repo-2281-stash-pop', { ref: 'stash@{0}' })); + await expectStatusInvalidatedBy('/repo-2281-stash-drop', () => dropGitStash('/repo-2281-stash-drop', { ref: 'stash@{0}' })); + } finally { + restoreMocks(); + } + }); + + test('merge and rebase lifecycle mutations invalidate cached status', async () => { + installWindowMock(); + try { + await expectStatusInvalidatedBy('/repo-2281-merge', () => merge('/repo-2281-merge', { branch: 'feature' })); + await expectStatusInvalidatedBy('/repo-2281-merge-abort', () => abortMerge('/repo-2281-merge-abort')); + await expectStatusInvalidatedBy('/repo-2281-merge-continue', () => continueMerge('/repo-2281-merge-continue')); + await expectStatusInvalidatedBy('/repo-2281-rebase', () => rebase('/repo-2281-rebase', { onto: 'main' })); + await expectStatusInvalidatedBy('/repo-2281-rebase-abort', () => abortRebase('/repo-2281-rebase-abort')); + await expectStatusInvalidatedBy('/repo-2281-rebase-continue', () => continueRebase('/repo-2281-rebase-continue')); + } finally { + restoreMocks(); + } + }); + + test('history mutations invalidate cached status', async () => { + installWindowMock(); + try { + await expectStatusInvalidatedBy('/repo-2281-checkout-commit', () => checkoutCommit('/repo-2281-checkout-commit', 'abc123')); + await expectStatusInvalidatedBy('/repo-2281-cherry-pick', () => cherryPick('/repo-2281-cherry-pick', 'abc123')); + await expectStatusInvalidatedBy('/repo-2281-revert-commit', () => revertCommit('/repo-2281-revert-commit', 'abc123')); + await expectStatusInvalidatedBy('/repo-2281-reset', () => resetToCommit('/repo-2281-reset', 'abc123', 'mixed')); + } finally { + restoreMocks(); + } + }); + + test('remote-side mutations invalidate cached status', async () => { + installWindowMock(); + try { + await expectStatusInvalidatedBy('/repo-2281-delete-remote-branch', () => deleteRemoteBranch('/repo-2281-delete-remote-branch', { branch: 'feature', remote: 'origin' })); + await expectStatusInvalidatedBy('/repo-2281-remove-remote', () => removeRemote('/repo-2281-remove-remote', { remote: 'origin' })); + } finally { + restoreMocks(); + } + }); + + test('a failed mutation does not invalidate cached status', async () => { + installWindowMock(); + const statusUrls: string[] = []; + globalThis.fetch = (async (input) => { + const url = String(input); + if (url.startsWith('/api/git/status')) { + statusUrls.push(url); + return jsonResponse(statusPayload()); + } + return new Response(JSON.stringify({ error: 'checkout failed' }), { + status: 500, + headers: { 'Content-Type': 'application/json' }, + }); + }) as typeof fetch; + + try { + const directory = '/repo-2281-failed-checkout'; + await getGitStatus(directory); + + const error = await captureError(async () => { + await checkoutBranch(directory, 'feature'); + }); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe('checkout failed'); + + await getGitStatus(directory); + expect(statusUrls).toHaveLength(1); + } finally { + restoreMocks(); + } + }); + + test('a status request admitted before a mutation cannot satisfy the post-mutation refresh', async () => { + installWindowMock(); + const statusResolvers: Array<(response: Response) => void> = []; + const statusUrls: string[] = []; + globalThis.fetch = (async (input) => { + const url = String(input); + if (url.startsWith('/api/git/status')) { + statusUrls.push(url); + return new Promise((resolve) => { + statusResolvers.push(resolve); + }); + } + return jsonResponse({ success: true }); + }) as typeof fetch; + + try { + const directory = '/repo-2281-deferred'; + const preMutationRead = getGitStatus(directory); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(statusUrls).toHaveLength(1); + + await checkoutBranch(directory, 'feature'); + + const postMutationRead = getGitStatus(directory); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(statusUrls).toHaveLength(2); + + statusResolvers[1](jsonResponse(statusPayload({ current: 'feature' }))); + statusResolvers[0](jsonResponse(statusPayload({ current: 'main' }))); + + const [preMutationStatus, postMutationStatus] = await Promise.all([preMutationRead, postMutationRead]); + expect(preMutationStatus.current).toBe('main'); + expect(postMutationStatus.current).toBe('feature'); + + // The late pre-mutation response must not repopulate the cache. + const cachedRead = await getGitStatus(directory); + expect(cachedRead.current).toBe('feature'); + expect(statusUrls).toHaveLength(2); + } finally { + restoreMocks(); + } + }); +}); + describe('gitApiHttp request priority', () => { test('leaves low-level reads outside the background policy', async () => { installWindowMock(); diff --git a/packages/ui/src/lib/gitApiHttp.ts b/packages/ui/src/lib/gitApiHttp.ts index 24317b24..80385dc0 100644 --- a/packages/ui/src/lib/gitApiHttp.ts +++ b/packages/ui/src/lib/gitApiHttp.ts @@ -37,6 +37,7 @@ import type { import { runtimeFetch } from './runtime-fetch'; import { getRuntimeUrlResolver } from './runtime-url'; import { getRuntimeKey } from './runtime-switch'; +import { notifyGitStatusInvalidated } from './gitStatusInvalidation'; const API_BASE = '/api/git'; const GIT_STATUS_CACHE_TTL_MS = 1200; @@ -65,6 +66,16 @@ const invalidateGitStatusCache = (directory: string): void => { gitStatusCache.delete(statusKey); gitStatusInFlight.delete(statusKey); } + notifyGitStatusInvalidated(directory); +}; + +// Shared success path for status-affecting mutations. The payload is parsed +// before invalidating so a failed mutation (non-ok response handled by the +// caller, or a malformed body) cannot publish a false state change. +const completeStatusMutation = async (directory: string, response: Response): Promise => { + const result = await response.json() as T; + invalidateGitStatusCache(directory); + return result; }; function buildUrl( @@ -418,7 +429,7 @@ export async function deleteGitBranch(directory: string, payload: GitDeleteBranc throw new Error(error.error || 'Failed to delete branch'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function deleteRemoteBranch(directory: string, payload: GitDeleteRemoteBranchPayload): Promise<{ success: boolean }> { @@ -437,7 +448,7 @@ export async function deleteRemoteBranch(directory: string, payload: GitDeleteRe throw new Error(error.error || 'Failed to delete remote branch'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function removeRemote(directory: string, payload: GitRemoveRemotePayload): Promise<{ success: boolean }> { @@ -457,7 +468,7 @@ export async function removeRemote(directory: string, payload: GitRemoveRemotePa throw new Error(error.error || 'Failed to remove remote'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function generateCommitMessage( @@ -664,9 +675,7 @@ export async function createGitCommit( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to create commit'); } - const result = await response.json(); - invalidateGitStatusCache(directory); - return result; + return completeStatusMutation(directory, response); } export async function gitPush( @@ -682,9 +691,7 @@ export async function gitPush( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to push'); } - const result = await response.json(); - invalidateGitStatusCache(directory); - return result; + return completeStatusMutation(directory, response); } export async function gitPull( @@ -700,9 +707,7 @@ export async function gitPull( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to pull'); } - const result = await response.json(); - invalidateGitStatusCache(directory); - return result; + return completeStatusMutation(directory, response); } export async function gitFetch( @@ -718,9 +723,7 @@ export async function gitFetch( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to fetch'); } - const result = await response.json(); - invalidateGitStatusCache(directory); - return result; + return completeStatusMutation(directory, response); } export async function listGitStashes(directory: string): Promise<{ stashes: GitStashEntry[] }> { @@ -755,7 +758,7 @@ export async function stashGitChanges(directory: string, options: { message?: st const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to stash changes'); } - return response.json(); + return completeStatusMutation(directory, response); } const postStashRef = async (directory: string, path: string, options: { ref: string }): Promise<{ success: boolean; ref: string }> => { @@ -768,7 +771,7 @@ const postStashRef = async (directory: string, path: string, options: { ref: str const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || `Failed to ${path}`); } - return response.json(); + return completeStatusMutation(directory, response); }; export const applyGitStash = (directory: string, options: { ref: string }) => postStashRef(directory, 'stash/apply', options); @@ -785,7 +788,7 @@ export async function checkoutBranch(directory: string, branch: string): Promise const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to checkout branch'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function createBranch( @@ -802,7 +805,7 @@ export async function createBranch( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to create branch'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function renameBranch( @@ -819,7 +822,7 @@ export async function renameBranch( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to rename branch'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function getGitLog( @@ -1022,7 +1025,7 @@ export async function rebase( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to rebase'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function abortRebase(directory: string): Promise<{ success: boolean }> { @@ -1033,7 +1036,7 @@ export async function abortRebase(directory: string): Promise<{ success: boolean const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to abort rebase'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function merge( @@ -1049,7 +1052,7 @@ export async function merge( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to merge'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function checkoutCommit( @@ -1065,7 +1068,7 @@ export async function checkoutCommit( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to checkout commit'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function cherryPick( @@ -1081,7 +1084,7 @@ export async function cherryPick( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to cherry-pick'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function revertCommit( @@ -1097,7 +1100,7 @@ export async function revertCommit( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to revert commit'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function resetToCommit( @@ -1115,7 +1118,7 @@ export async function resetToCommit( const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to reset'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function abortMerge(directory: string): Promise<{ success: boolean }> { @@ -1126,7 +1129,7 @@ export async function abortMerge(directory: string): Promise<{ success: boolean const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to abort merge'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function continueRebase(directory: string): Promise<{ success: boolean; conflict: boolean; conflictFiles?: string[] }> { @@ -1137,7 +1140,7 @@ export async function continueRebase(directory: string): Promise<{ success: bool const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to continue rebase'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function continueMerge(directory: string): Promise<{ success: boolean; conflict: boolean; conflictFiles?: string[] }> { @@ -1148,7 +1151,7 @@ export async function continueMerge(directory: string): Promise<{ success: boole const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error(error.error || 'Failed to continue merge'); } - return response.json(); + return completeStatusMutation(directory, response); } export async function stash( diff --git a/packages/ui/src/lib/gitStatusInvalidation.ts b/packages/ui/src/lib/gitStatusInvalidation.ts new file mode 100644 index 00000000..1bd672ff --- /dev/null +++ b/packages/ui/src/lib/gitStatusInvalidation.ts @@ -0,0 +1,34 @@ +/** + * Minimal notification channel for git status invalidation. + * + * A runtime adapter that caches git status (currently only the HTTP adapter in + * `gitApiHttp.ts`) must call `notifyGitStatusInvalidated` whenever a successful + * status-affecting mutation invalidates its cache. `useGitStore` subscribes and + * bumps its per-directory status mutation revision so an immediate refresh + * cannot join an in-flight status request admitted before the mutation, and a + * stale response cannot commit over newer authoritative state. + * + * Runtime parity: the VS Code bridge adapter performs no client-side status + * caching (every `getGitStatus` is a fresh bridge request), so it has no cache + * to invalidate and does not emit this signal today. Any adapter that adds + * caching must emit on invalidation. + */ + +type GitStatusInvalidationListener = (directory: string) => void; + +const listeners = new Set(); + +export const subscribeGitStatusInvalidations = ( + listener: GitStatusInvalidationListener +): (() => void) => { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +}; + +export const notifyGitStatusInvalidated = (directory: string): void => { + for (const listener of listeners) { + listener(directory); + } +}; diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index a578d61f..91387fff 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -132,10 +132,12 @@ Important properties: - `directories: Map` is the source of truth - loading state is per-directory, not global - `ensureStatus()` and `ensureAll()` are the preferred entry points for consumers -- in-flight dedupe exists for status and `ensureAll()` +- in-flight dedupe exists for status and `ensureAll()`; status dedupe is scoped to the per-directory status mutation revision, so a refresh requested after a mutation never joins a pre-mutation in-flight request - runtime reset replaces all live entries with that runtime's persisted branch seeds and invalidates old completions - status, branches, log, identity, repository probes, and prefetch diffs commit through runtime and per-channel generations - status mutations advance a revision so older refreshes cannot undo optimistic or confirmed index changes +- a successful status-affecting git mutation also advances that revision: the HTTP adapter's cache invalidation notifies the store through `lib/gitStatusInvalidation.ts` (the VS Code bridge adapter has no client-side status cache, so it emits nothing today) +- `fetchAll({ force: true })` forces the status fetch as well as the log refresh - branch persistence is versioned, bounded, runtime-scoped, and claims the ambiguous legacy cache once - diff data has per-directory and aggregate count/UTF-8-byte limits; oversized single entries are rejected @@ -255,6 +257,7 @@ Expected model: - `GitView` / `DiffView` ensure current-directory Git state when visible - explicit Git actions refresh status/branches/log as needed +- every status-affecting git mutation invalidates the HTTP adapter's status cache on its success path (failed mutations invalidate nothing), so the follow-up refresh is authoritative instead of the pre-mutation cache entry - a mounted file-mutating tool issues a one-shot Git refresh hint when it transitions from active to successfully finalized; remounting historical completed tools does not replay the hint - a successful dirty save from the in-app file editor issues a path-scoped Git refresh hint; clean autosave checks remain no-ops - refresh hints with authoritative file paths invalidate only those cached and currently rendered diffs before status refresh; pathless tools request status reconciliation without broadly remounting DiffView diff --git a/packages/ui/src/stores/useGitStore.test.ts b/packages/ui/src/stores/useGitStore.test.ts index 2512989a..15e22afd 100644 --- a/packages/ui/src/stores/useGitStore.test.ts +++ b/packages/ui/src/stores/useGitStore.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, test } from 'bun:test'; import type { GitStatus } from '@/lib/api/types'; import { useGitStore } from './useGitStore'; import { getRuntimeKey } from '@/lib/runtime-switch'; +import { notifyGitStatusInvalidated } from '@/lib/gitStatusInvalidation'; type Deferred = { promise: Promise; @@ -126,6 +127,85 @@ describe('useGitStore', () => { expect(lightResult).toBe(fullResult); }); + test('deduplicates concurrent status requests when no mutation occurs', async () => { + setDirectoryStatus(createStatus()); + let statusCalls = 0; + const request = createDeferred(); + const git = createGitApi(() => { + statusCalls += 1; + return request.promise; + }); + + const first = useGitStore.getState().fetchStatus('/repo', git, { silent: true }); + const second = useGitStore.getState().fetchStatus('/repo', git, { silent: true }); + await Promise.resolve(); + + expect(statusCalls).toBe(1); + + request.resolve(createStatus()); + await Promise.all([first, second]); + expect(statusCalls).toBe(1); + }); + + test('a refresh after a mutation does not join the pre-mutation in-flight status request', async () => { + setDirectoryStatus(createStatus()); + const requests: Deferred[] = []; + let statusCalls = 0; + const git = createGitApi(() => { + statusCalls += 1; + const request = createDeferred(); + requests.push(request); + return request.promise; + }); + + const preMutation = useGitStore.getState().fetchStatus('/repo', git, { silent: true }); + await Promise.resolve(); + expect(statusCalls).toBe(1); + + // A successful git mutation invalidates the adapter status cache, which + // notifies the store that the in-flight request predates the mutation. + notifyGitStatusInvalidated('/repo'); + + const postMutation = useGitStore.getState().fetchStatus('/repo', git, { silent: true }); + await Promise.resolve(); + expect(statusCalls).toBe(2); + + requests[1].resolve({ ...createStatus(), current: 'feature' }); + await postMutation; + expect(useGitStore.getState().getDirectoryState('/repo')?.status?.current).toBe('feature'); + + // The late pre-mutation response cannot overwrite the newer authoritative one. + requests[0].resolve(createStatus()); + await preMutation; + expect(useGitStore.getState().getDirectoryState('/repo')?.status?.current).toBe('feature'); + }); + + test('fetchAll({ force: true }) forces a fresh status fetch past the in-flight dedup', async () => { + setDirectoryStatus(createStatus()); + const requests: Deferred[] = []; + let statusCalls = 0; + const git = createGitApi(() => { + statusCalls += 1; + const request = createDeferred(); + requests.push(request); + return request.promise; + }); + + const inFlight = useGitStore.getState().fetchStatus('/repo', git, { silent: true }); + await Promise.resolve(); + expect(statusCalls).toBe(1); + + const all = useGitStore.getState().fetchAll('/repo', git, { force: true }); + await Promise.resolve(); + expect(statusCalls).toBe(2); + + requests[1].resolve({ ...createStatus(), current: 'feature' }); + requests[0].resolve(createStatus()); + await Promise.allSettled([inFlight, all]); + + expect(useGitStore.getState().getDirectoryState('/repo')?.status?.current).toBe('feature'); + }); + test('does not let an older status fetch undo an optimistic mutation', async () => { const initial = createStatus(undefined, [{ path: 'src/index.ts', index: ' ', working_dir: 'M' }]); setDirectoryStatus(initial); diff --git a/packages/ui/src/stores/useGitStore.ts b/packages/ui/src/stores/useGitStore.ts index aec942ba..dbffb679 100644 --- a/packages/ui/src/stores/useGitStore.ts +++ b/packages/ui/src/stores/useGitStore.ts @@ -9,6 +9,7 @@ import type { } from '@/lib/api/types'; import { getDeferredSafeStorage } from '@/stores/utils/safeStorage'; import { getRuntimeKey } from '@/lib/runtime-switch'; +import { subscribeGitStatusInvalidations } from '@/lib/gitStatusInvalidation'; const LOG_STALE_THRESHOLD = 10000; const REPO_CHECK_STALE_THRESHOLD = 60_000; @@ -57,7 +58,7 @@ interface GitStore { setActiveDirectory: (directory: string | null) => void; getDirectoryState: (directory: string) => DirectoryGitState | null; - fetchStatus: (directory: string, git: GitAPI, options?: { silent?: boolean; mode?: 'light' }) => Promise; + fetchStatus: (directory: string, git: GitAPI, options?: { silent?: boolean; mode?: 'light'; force?: boolean }) => Promise; fetchBranches: (directory: string, git: GitAPI) => Promise; fetchLog: (directory: string, git: GitAPI, maxCount?: number) => Promise; fetchIdentity: (directory: string, git: GitAPI) => Promise; @@ -99,7 +100,7 @@ interface GitAPI { const inFlightDiffFetchesByDirectory = new Map>(); const diffFetchGenerationByDirectory = new Map(); -const inFlightStatusFetches = new Map>(); +const inFlightStatusFetches = new Map; statusMutationRevision: number }>(); const inFlightEnsureAllByDirectory = new Map>(); const requestGenerationByChannel = new Map(); const statusMutationRevisionByDirectory = new Map(); @@ -150,6 +151,18 @@ const bumpStatusMutationRevision = (runtimeKey: string, directory: string): void statusMutationRevisionByDirectory.set(key, (statusMutationRevisionByDirectory.get(key) ?? 0) + 1); }; +const getStatusMutationRevision = (runtimeKey: string, directory: string): number => + statusMutationRevisionByDirectory.get(runtimeDirectoryKey(runtimeKey, directory)) ?? 0; + +// A successful status-affecting git mutation invalidates the runtime adapter's +// status cache (see lib/gitStatusInvalidation.ts). Bump the per-directory +// mutation revision so a status request admitted before the mutation can +// neither be joined by a post-mutation refresh nor commit its stale payload +// over the refreshed state. +subscribeGitStatusInvalidations((directory) => { + bumpStatusMutationRevision(getRuntimeKey(), directory); +}); + const getDiffFetchGeneration = (directory: string): number => diffFetchGenerationByDirectory.get(runtimeDirectoryKey(getRuntimeKey(), directory)) ?? 0; @@ -590,10 +603,16 @@ export const useGitStore = create()( const statusFetchMode: GitStatusFetchMode = options.mode ?? 'full'; const runtimeKey = getRuntimeKey(); const statusFetchKey = getStatusFetchKey(runtimeKey, directory, statusFetchMode); - const existing = inFlightStatusFetches.get(statusFetchKey) - ?? (statusFetchMode === 'light' ? inFlightStatusFetches.get(getStatusFetchKey(runtimeKey, directory, 'full')) : undefined); - if (existing) { - return existing; + const statusMutationRevision = getStatusMutationRevision(runtimeKey, directory); + if (!options.force) { + const existing = inFlightStatusFetches.get(statusFetchKey) + ?? (statusFetchMode === 'light' ? inFlightStatusFetches.get(getStatusFetchKey(runtimeKey, directory, 'full')) : undefined); + // Join an in-flight request only when it was admitted at the current + // mutation revision; a request that predates a mutation must not + // satisfy the post-mutation refresh. + if (existing && existing.statusMutationRevision === statusMutationRevision) { + return existing.promise; + } } const token = startRequest(directory, 'status', true); @@ -727,12 +746,12 @@ export const useGitStore = create()( return statusChanged; })(); - inFlightStatusFetches.set(statusFetchKey, fetchPromise); + inFlightStatusFetches.set(statusFetchKey, { promise: fetchPromise, statusMutationRevision }); try { return await fetchPromise; } finally { - if (inFlightStatusFetches.get(statusFetchKey) === fetchPromise) { + if (inFlightStatusFetches.get(statusFetchKey)?.promise === fetchPromise) { inFlightStatusFetches.delete(statusFetchKey); } } @@ -936,8 +955,11 @@ export const useGitStore = create()( const { force = false, silentIfCached = false } = options; const now = Date.now(); + // `force` applies to status as well as log: a forced refresh must not + // resolve from an in-flight status request admitted earlier. await get().fetchStatus(directory, git, { silent: silentIfCached && Boolean(dirState?.status), + force, }); const updatedDirState = get().directories.get(directory);