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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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']]]) });
|
||||
|
||||
@@ -90,6 +90,13 @@ interface GitStore {
|
||||
// route, and absent when discovery has not run yet.
|
||||
nestedReposByRoot: Map<string, NestedRepoDiscovery>;
|
||||
nestedRepoSelection: Map<string, string>;
|
||||
/**
|
||||
* 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<string, Set<string>>;
|
||||
ensureNestedRepos: (root: string, options?: { force?: boolean }) => Promise<void>;
|
||||
selectNestedRepo: (root: string, repository: string) => void;
|
||||
clearNestedRepoSelection: (root: string) => void;
|
||||
@@ -622,6 +629,7 @@ export const useGitStore = create<GitStore>()(
|
||||
activeDirectory: null,
|
||||
nestedReposByRoot: new Map(),
|
||||
nestedRepoSelection: seedNestedRepoSelection(initialGitRuntimeKey),
|
||||
staleClearedSelections: new Map(),
|
||||
|
||||
resetForRuntimeSwitch: (runtimeKey) => {
|
||||
gitRuntimeGeneration += 1;
|
||||
@@ -639,6 +647,7 @@ export const useGitStore = create<GitStore>()(
|
||||
activeDirectory: null,
|
||||
nestedReposByRoot: new Map(),
|
||||
nestedRepoSelection: seedNestedRepoSelection(runtimeKey),
|
||||
staleClearedSelections: new Map(),
|
||||
});
|
||||
},
|
||||
|
||||
@@ -1273,10 +1282,19 @@ export const useGitStore = create<GitStore>()(
|
||||
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user