From e7a3d0e38deee365c8762f11ad75b729806d5f0e Mon Sep 17 00:00:00 2001 From: jaygupta17 Date: Wed, 26 Aug 2026 09:04:50 +0530 Subject: [PATCH] fix(git): stop auto-select re-picking a repository that failed its probe A corrupt nested repository can be discovered (its .git entry exists) but fail its status probe. The stale-selection recovery cleared the pick and forced a re-scan, and auto-select immediately re-picked the same path, looping discovery walk + probe for as long as the surface stayed visible. clearNestedRepoSelection now remembers the dropped repository per root (session-only set, cleared on runtime switch), and auto-select skips remembered paths. When every candidate has failed, no selection is made and surfaces settle into their unresolved state instead of churning requests. A manual picker pick remains possible and is probed like any other. --- .../ui/src/hooks/useNestedGitDirectory.ts | 27 +++++++++++++++-- packages/ui/src/stores/DOCUMENTATION.md | 2 +- packages/ui/src/stores/useGitStore.test.ts | 24 +++++++++++++++ packages/ui/src/stores/useGitStore.ts | 30 ++++++++++++++++++- 4 files changed, 78 insertions(+), 5 deletions(-) diff --git a/packages/ui/src/hooks/useNestedGitDirectory.ts b/packages/ui/src/hooks/useNestedGitDirectory.ts index c0855805..a43bce53 100644 --- a/packages/ui/src/hooks/useNestedGitDirectory.ts +++ b/packages/ui/src/hooks/useNestedGitDirectory.ts @@ -8,6 +8,7 @@ import { useIsGitRepo, useNestedRepoSelection, useNestedRepos, + useStaleClearedSelections, } from '@/stores/useGitStore'; type UseNestedGitDirectoryOptions = { @@ -35,6 +36,7 @@ export const useNestedGitDirectory = ( const gitDirectory = useEffectiveGitDirectory(root); const nestedRepos = useNestedRepos(root); const nestedRepoSelection = useNestedRepoSelection(root); + const staleClearedSelections = useStaleClearedSelections(root); // Probe of the resolved repository, used to detect a stale selection. Null // when there is nothing selected to probe. @@ -68,13 +70,32 @@ export const useNestedGitDirectory = ( // Auto-select the first nested repository so the surface opens straight // into repository data; a picker (where rendered) switches between them. + // Repositories whose selection already failed a probe are skipped: without + // this, a corrupt repository (discovered via its .git entry but failing + // git status) would be re-picked right after every stale-clear and loop + // discovery + probe while the surface is visible. When every candidate has + // failed, no selection is made — surfaces settle into their unresolved + // state instead of churning requests. A manual picker pick is still free + // to select anything; it gets probed like any other. React.useEffect(() => { if (!enabled || !root) return; if (rootIsGitRepo !== false) return; - if (!nestedRepos || nestedRepos.length === 0) return; + if (!Array.isArray(nestedRepos) || nestedRepos.length === 0) return; if (nestedRepoSelection) return; - selectNestedRepo(root, nestedRepos[0]); - }, [enabled, nestedRepos, nestedRepoSelection, root, rootIsGitRepo, selectNestedRepo]); + const candidates = staleClearedSelections + ? nestedRepos.filter((repository) => !staleClearedSelections.has(repository)) + : nestedRepos; + if (candidates.length === 0) return; + selectNestedRepo(root, candidates[0]); + }, [ + enabled, + nestedRepos, + nestedRepoSelection, + root, + rootIsGitRepo, + selectNestedRepo, + staleClearedSelections, + ]); // A selected repository that is no longer a git repository is stale: drop // the selection and re-scan so resolution reflects the current tree. diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index 56d19aef..0e10fd10 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -148,7 +148,7 @@ Important properties: - 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()` -- nested repository discovery (`nestedReposByRoot`, `nestedRepoSelection`, `ensureNestedRepos`) is per-root state for roots that are not themselves git repositories; discovery failure is a `null` marker (never a valid empty result), a runtime without the discovery route (VS Code) commits an `'unsupported'` marker, and an in-flight discovery whose runtime switched is discarded at commit time instead of repopulating the cleared map. Selections are persisted per runtime + root, and `useEffectiveGitDirectory(root)` resolves the directory git surfaces operate on (`root` when the root is a repository, the selected nested repository otherwise). `hooks/useNestedGitDirectory.ts` owns the resolution flow (root probe, discovery, auto-select, stale-selection recovery) for every consuming surface (Git tab, diff view, pull-request view, walkthrough view, mobile changes), and `git/NestedRepoResolutionStates.tsx` renders the shared pending/failed/unsupported/empty states +- nested repository discovery (`nestedReposByRoot`, `nestedRepoSelection`, `ensureNestedRepos`) is per-root state for roots that are not themselves git repositories; discovery failure is a `null` marker (never a valid empty result), a runtime without the discovery route (VS Code) commits an `'unsupported'` marker, and an in-flight discovery whose runtime switched is discarded at commit time instead of repopulating the cleared map. Selections are persisted per runtime + root, and `useEffectiveGitDirectory(root)` resolves the directory git surfaces operate on (`root` when the root is a repository, the selected nested repository otherwise). A selection whose repository fails its probe is dropped and remembered session-only (`staleClearedSelections`) so auto-select does not re-pick it and loop walk+probe; manual picker picks bypass the memory. `hooks/useNestedGitDirectory.ts` owns the resolution flow (root probe, discovery, auto-select, stale-selection recovery) for every consuming surface (Git tab, diff view, pull-request view, walkthrough view, mobile changes), and `git/NestedRepoResolutionStates.tsx` renders the shared pending/failed/unsupported/empty states - 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 diff --git a/packages/ui/src/stores/useGitStore.test.ts b/packages/ui/src/stores/useGitStore.test.ts index 20296dba..280a1483 100644 --- a/packages/ui/src/stores/useGitStore.test.ts +++ b/packages/ui/src/stores/useGitStore.test.ts @@ -386,6 +386,30 @@ describe('useGitStore nested repository discovery', () => { expect(useGitStore.getState().nestedRepoSelection.get('/root-b')).toBe('/root-b/two'); }); + test('remembers a stale-cleared repository so auto-select can skip it', () => { + useGitStore.getState().selectNestedRepo('/root-a', '/root-a/one'); + useGitStore.getState().selectNestedRepo('/root-b', '/root-b/two'); + + useGitStore.getState().clearNestedRepoSelection('/root-a'); + useGitStore.getState().clearNestedRepoSelection('/root-b'); + useGitStore.getState().clearNestedRepoSelection('/root-b'); + + const clearedA = useGitStore.getState().staleClearedSelections.get('/root-a'); + const clearedB = useGitStore.getState().staleClearedSelections.get('/root-b'); + expect(clearedA).toEqual(new Set(['/root-a/one'])); + // Repeated clears of the same path stay a set, not an ever-growing list. + expect(clearedB).toEqual(new Set(['/root-b/two'])); + }); + + test('runtime switch clears stale-cleared memory with the rest', () => { + useGitStore.getState().selectNestedRepo('/root-a', '/root-a/one'); + useGitStore.getState().clearNestedRepoSelection('/root-a'); + + useGitStore.getState().resetForRuntimeSwitch('runtime-b'); + + expect(useGitStore.getState().staleClearedSelections.size).toBe(0); + }); + test('runtime switch does not leak selections or discovery across runtimes', () => { useGitStore.getState().selectNestedRepo('/root-a', '/root-a/one'); useGitStore.setState({ nestedReposByRoot: new Map([['/root-a', ['/root-a/one']]]) }); diff --git a/packages/ui/src/stores/useGitStore.ts b/packages/ui/src/stores/useGitStore.ts index 0d1dcf2e..a26690a9 100644 --- a/packages/ui/src/stores/useGitStore.ts +++ b/packages/ui/src/stores/useGitStore.ts @@ -90,6 +90,13 @@ interface GitStore { // route, and absent when discovery has not run yet. nestedReposByRoot: Map; nestedRepoSelection: Map; + /** + * Repositories whose selection was dropped because their probe reported + * them as no longer a repository (corrupt or missing gitdir). Session-only + * memory so auto-select does not immediately re-pick the same broken path + * and loop walk+probe. Not persisted: the next launch re-probes honestly. + */ + staleClearedSelections: Map>; ensureNestedRepos: (root: string, options?: { force?: boolean }) => Promise; selectNestedRepo: (root: string, repository: string) => void; clearNestedRepoSelection: (root: string) => void; @@ -622,6 +629,7 @@ export const useGitStore = create()( activeDirectory: null, nestedReposByRoot: new Map(), nestedRepoSelection: seedNestedRepoSelection(initialGitRuntimeKey), + staleClearedSelections: new Map(), resetForRuntimeSwitch: (runtimeKey) => { gitRuntimeGeneration += 1; @@ -639,6 +647,7 @@ export const useGitStore = create()( activeDirectory: null, nestedReposByRoot: new Map(), nestedRepoSelection: seedNestedRepoSelection(runtimeKey), + staleClearedSelections: new Map(), }); }, @@ -1273,10 +1282,19 @@ export const useGitStore = create()( clearNestedRepoSelection: (root) => { if (!root) return; - if (!get().nestedRepoSelection.has(root)) return; + const cleared = get().nestedRepoSelection.get(root); + if (cleared === undefined) return; const next = new Map(get().nestedRepoSelection); next.delete(root); set({ nestedRepoSelection: next }); + // Remember the drop so auto-select does not re-pick the same path + // before its probe can tell the difference. Only stale-probe + // recovery clears, so every clear here is a failed selection. + const nextStale = new Map(get().staleClearedSelections); + const forRoot = new Set(nextStale.get(root)); + forRoot.add(cleared); + nextStale.set(root, forRoot); + set({ staleClearedSelections: nextStale }); writeCachedNestedRepoSelection(getRuntimeKey(), Object.fromEntries(next)); }, @@ -1406,6 +1424,16 @@ export const useNestedRepoSelection = (root: string | null) => { }); }; +// Repositories of this root whose selection already failed its probe. Auto- +// select skips them; the picker does not (a manual re-pick is a user decision +// and gets probed like any other). +export const useStaleClearedSelections = (root: string | null) => { + return useGitStore((state) => { + if (!root) return null; + return state.staleClearedSelections.get(root) ?? null; + }); +}; + export const useGitBranchLabel = (directory: string | null) => { return useGitStore((state) => { if (!directory) return null;