fix(git): harden nested repository discovery
- discard an in-flight discovery when the runtime switches, mirroring the isRequestCurrent pattern, so a late completion cannot repopulate the cleared map and suppress a fresh scan - treat a 501 from /api/fs/git-dirs as an explicit 'unsupported' marker instead of a generic failure; the VS Code webview now answers the route with unsupportedWebRouteResponse so non-repo roots show the honest not-a-repository state without a futile Retry
This commit is contained in:
@@ -116,8 +116,18 @@ export async function checkIsGitRepository(directory: string): Promise<boolean>
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export class GitDirectoriesUnsupportedError extends Error {
|
||||||
|
constructor() {
|
||||||
|
super('Nested git repository discovery is not supported by this runtime');
|
||||||
|
this.name = 'GitDirectoriesUnsupportedError';
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export async function listGitDirectories(root: string): Promise<string[]> {
|
export async function listGitDirectories(root: string): Promise<string[]> {
|
||||||
const response = await runtimeFetch('/api/fs/git-dirs', { query: { path: root } });
|
const response = await runtimeFetch('/api/fs/git-dirs', { query: { path: root } });
|
||||||
|
if (response.status === 501) {
|
||||||
|
throw new GitDirectoriesUnsupportedError();
|
||||||
|
}
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
throw new Error(`Failed to list git directories: ${response.statusText}`);
|
throw new Error(`Failed to list git directories: ${response.statusText}`);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,8 +1,22 @@
|
|||||||
import { beforeEach, describe, expect, test } from 'bun:test';
|
import { beforeEach, describe, expect, mock, test } from 'bun:test';
|
||||||
import type { GitStatus } from '@/lib/api/types';
|
import type { GitStatus } from '@/lib/api/types';
|
||||||
import { useGitStore } from './useGitStore';
|
import { useGitStore } from './useGitStore';
|
||||||
import { getRuntimeKey } from '@/lib/runtime-switch';
|
import { getRuntimeKey } from '@/lib/runtime-switch';
|
||||||
|
|
||||||
|
// The real transport has no server in tests and fails as a generic error.
|
||||||
|
// Tests that exercise other failure modes swap this implementation; the
|
||||||
|
// default keeps every pre-existing expectation (generic failure → null).
|
||||||
|
const listGitDirectoriesControl: { impl: (root: string) => Promise<string[]> } = {
|
||||||
|
impl: async () => {
|
||||||
|
throw new Error('network unavailable');
|
||||||
|
},
|
||||||
|
};
|
||||||
|
class TestGitDirectoriesUnsupportedError extends Error {}
|
||||||
|
mock.module('@/lib/gitApiHttp', () => ({
|
||||||
|
GitDirectoriesUnsupportedError: TestGitDirectoriesUnsupportedError,
|
||||||
|
listGitDirectories: (root: string) => listGitDirectoriesControl.impl(root),
|
||||||
|
}));
|
||||||
|
|
||||||
type Deferred<T> = {
|
type Deferred<T> = {
|
||||||
promise: Promise<T>;
|
promise: Promise<T>;
|
||||||
resolve: (value: T) => void;
|
resolve: (value: T) => void;
|
||||||
@@ -338,6 +352,9 @@ describe('useGitStore', () => {
|
|||||||
|
|
||||||
describe('useGitStore nested repository discovery', () => {
|
describe('useGitStore nested repository discovery', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
|
listGitDirectoriesControl.impl = async () => {
|
||||||
|
throw new Error('network unavailable');
|
||||||
|
};
|
||||||
useGitStore.getState().resetForRuntimeSwitch(getRuntimeKey());
|
useGitStore.getState().resetForRuntimeSwitch(getRuntimeKey());
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -379,12 +396,47 @@ describe('useGitStore nested repository discovery', () => {
|
|||||||
expect(useGitStore.getState().nestedReposByRoot.size).toBe(0);
|
expect(useGitStore.getState().nestedReposByRoot.size).toBe(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('discards an in-flight discovery result when the runtime switches', async () => {
|
||||||
|
const stale = useGitStore.getState().ensureNestedRepos('/root-a');
|
||||||
|
useGitStore.getState().resetForRuntimeSwitch('runtime-b');
|
||||||
|
await stale;
|
||||||
|
|
||||||
|
// The old runtime's late completion must not repopulate the cleared map.
|
||||||
|
expect(useGitStore.getState().nestedReposByRoot.has('/root-a')).toBe(false);
|
||||||
|
|
||||||
|
// Discovery started under the new runtime still commits normally.
|
||||||
|
await useGitStore.getState().ensureNestedRepos('/root-a');
|
||||||
|
expect(useGitStore.getState().nestedReposByRoot.get('/root-a')).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
test('marks discovery failure as a failed marker, not an empty success', async () => {
|
test('marks discovery failure as a failed marker, not an empty success', async () => {
|
||||||
await useGitStore.getState().ensureNestedRepos('/root-a');
|
await useGitStore.getState().ensureNestedRepos('/root-a');
|
||||||
|
|
||||||
expect(useGitStore.getState().nestedReposByRoot.get('/root-a')).toBeNull();
|
expect(useGitStore.getState().nestedReposByRoot.get('/root-a')).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('marks a 501 runtime as unsupported instead of failed', async () => {
|
||||||
|
listGitDirectoriesControl.impl = async () => {
|
||||||
|
throw new TestGitDirectoriesUnsupportedError();
|
||||||
|
};
|
||||||
|
|
||||||
|
await useGitStore.getState().ensureNestedRepos('/root-a');
|
||||||
|
|
||||||
|
expect(useGitStore.getState().nestedReposByRoot.get('/root-a')).toBe('unsupported');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('unsupported does not clobber a previous successful discovery', async () => {
|
||||||
|
listGitDirectoriesControl.impl = async () => ['/root-a/one'];
|
||||||
|
await useGitStore.getState().ensureNestedRepos('/root-a');
|
||||||
|
|
||||||
|
listGitDirectoriesControl.impl = async () => {
|
||||||
|
throw new TestGitDirectoriesUnsupportedError();
|
||||||
|
};
|
||||||
|
await useGitStore.getState().ensureNestedRepos('/root-a', { force: true });
|
||||||
|
|
||||||
|
expect(useGitStore.getState().nestedReposByRoot.get('/root-a')).toEqual(['/root-a/one']);
|
||||||
|
});
|
||||||
|
|
||||||
test('dedupes concurrent discovery runs for the same root', async () => {
|
test('dedupes concurrent discovery runs for the same root', async () => {
|
||||||
const first = useGitStore.getState().ensureNestedRepos('/root-a');
|
const first = useGitStore.getState().ensureNestedRepos('/root-a');
|
||||||
const second = useGitStore.getState().ensureNestedRepos('/root-a');
|
const second = useGitStore.getState().ensureNestedRepos('/root-a');
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ import type {
|
|||||||
} from '@/lib/api/types';
|
} from '@/lib/api/types';
|
||||||
import { getDeferredSafeStorage } from '@/stores/utils/safeStorage';
|
import { getDeferredSafeStorage } from '@/stores/utils/safeStorage';
|
||||||
import { getRuntimeKey } from '@/lib/runtime-switch';
|
import { getRuntimeKey } from '@/lib/runtime-switch';
|
||||||
import { listGitDirectories } from '@/lib/gitApiHttp';
|
import { GitDirectoriesUnsupportedError, listGitDirectories } from '@/lib/gitApiHttp';
|
||||||
|
|
||||||
const LOG_STALE_THRESHOLD = 10000;
|
const LOG_STALE_THRESHOLD = 10000;
|
||||||
const REPO_CHECK_STALE_THRESHOLD = 60_000;
|
const REPO_CHECK_STALE_THRESHOLD = 60_000;
|
||||||
@@ -28,6 +28,11 @@ const DIFF_CACHE_MAX_TOTAL_SIZE_BYTES = 20 * 1024 * 1024; // 20MB
|
|||||||
const DIFF_CACHE_MAX_GLOBAL_ENTRIES = 200;
|
const DIFF_CACHE_MAX_GLOBAL_ENTRIES = 200;
|
||||||
type GitStatusFetchMode = 'full' | 'light';
|
type GitStatusFetchMode = 'full' | 'light';
|
||||||
|
|
||||||
|
// Discovery outcome for a root that is not itself a git repository. The three
|
||||||
|
// states are mutually exclusive: a repository list (possibly empty), a failed
|
||||||
|
// scan (`null`), or a runtime without the discovery route (`'unsupported'`).
|
||||||
|
export type NestedRepoDiscovery = string[] | null | 'unsupported';
|
||||||
|
|
||||||
interface DirectoryGitState {
|
interface DirectoryGitState {
|
||||||
isGitRepo: boolean | null;
|
isGitRepo: boolean | null;
|
||||||
status: GitStatus | null;
|
status: GitStatus | null;
|
||||||
@@ -81,8 +86,9 @@ interface GitStore {
|
|||||||
// Nested repository discovery: when the root directory is not itself a git
|
// Nested repository discovery: when the root directory is not itself a git
|
||||||
// repository, these hold the discovered repositories and the user's pick.
|
// repository, these hold the discovered repositories and the user's pick.
|
||||||
// `nestedReposByRoot` values are `null` when discovery failed — never a
|
// `nestedReposByRoot` values are `null` when discovery failed — never a
|
||||||
// valid empty result — and absent when discovery has not run yet.
|
// valid empty result — `'unsupported'` when the runtime has no discovery
|
||||||
nestedReposByRoot: Map<string, string[] | null>;
|
// route, and absent when discovery has not run yet.
|
||||||
|
nestedReposByRoot: Map<string, NestedRepoDiscovery>;
|
||||||
nestedRepoSelection: Map<string, string>;
|
nestedRepoSelection: Map<string, string>;
|
||||||
ensureNestedRepos: (root: string, options?: { force?: boolean }) => Promise<void>;
|
ensureNestedRepos: (root: string, options?: { force?: boolean }) => Promise<void>;
|
||||||
selectNestedRepo: (root: string, repository: string) => void;
|
selectNestedRepo: (root: string, repository: string) => void;
|
||||||
@@ -1203,6 +1209,7 @@ export const useGitStore = create<GitStore>()(
|
|||||||
if (!root) return;
|
if (!root) return;
|
||||||
const { force = false } = options;
|
const { force = false } = options;
|
||||||
const runtimeKey = getRuntimeKey();
|
const runtimeKey = getRuntimeKey();
|
||||||
|
const runtimeGeneration = gitRuntimeGeneration;
|
||||||
const key = runtimeDirectoryKey(runtimeKey, root);
|
const key = runtimeDirectoryKey(runtimeKey, root);
|
||||||
const current = get().nestedReposByRoot.get(root);
|
const current = get().nestedReposByRoot.get(root);
|
||||||
if (!force && (current !== undefined || inFlightNestedRepoDiscovery.has(key))) {
|
if (!force && (current !== undefined || inFlightNestedRepoDiscovery.has(key))) {
|
||||||
@@ -1217,16 +1224,30 @@ export const useGitStore = create<GitStore>()(
|
|||||||
|
|
||||||
const discovery = (async () => {
|
const discovery = (async () => {
|
||||||
let repositories: string[] | null = null;
|
let repositories: string[] | null = null;
|
||||||
|
let unsupported = false;
|
||||||
try {
|
try {
|
||||||
repositories = await listGitDirectories(root);
|
repositories = await listGitDirectories(root);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error('Failed to discover nested git repositories:', error);
|
if (error instanceof GitDirectoriesUnsupportedError) {
|
||||||
|
unsupported = true;
|
||||||
|
} else {
|
||||||
|
console.error('Failed to discover nested git repositories:', error);
|
||||||
|
}
|
||||||
repositories = null;
|
repositories = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
// A failed retry must not clobber an earlier successful discovery.
|
// A runtime switch invalidates the discovery: resetForRuntimeSwitch
|
||||||
|
// already cleared the map, and committing old-runtime data here would
|
||||||
|
// both leak it and suppress a fresh scan for this root.
|
||||||
|
if (runtimeKey !== getRuntimeKey() || runtimeGeneration !== gitRuntimeGeneration) return;
|
||||||
|
|
||||||
const previous = get().nestedReposByRoot.get(root);
|
const previous = get().nestedReposByRoot.get(root);
|
||||||
const nextValue = repositories ?? previous ?? null;
|
// An authoritative "unsupported" answer replaces only unknown or
|
||||||
|
// failed state; like a failed retry, it must not clobber an earlier
|
||||||
|
// successful discovery.
|
||||||
|
const nextValue: NestedRepoDiscovery = unsupported
|
||||||
|
? (previous ?? 'unsupported')
|
||||||
|
: (repositories ?? previous ?? null);
|
||||||
const next = new Map(get().nestedReposByRoot);
|
const next = new Map(get().nestedReposByRoot);
|
||||||
next.set(root, nextValue);
|
next.set(root, nextValue);
|
||||||
set({ nestedReposByRoot: next });
|
set({ nestedReposByRoot: next });
|
||||||
@@ -1368,8 +1389,9 @@ export const useEffectiveGitDirectory = (root: string | null) => {
|
|||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
// `undefined` = discovery not run yet, `null` = discovery failed, otherwise
|
// `undefined` = discovery not run yet, `null` = discovery failed,
|
||||||
// the discovered nested repository paths (possibly empty).
|
// `'unsupported'` = the runtime has no discovery route, otherwise the
|
||||||
|
// discovered nested repository paths (possibly empty).
|
||||||
export const useNestedRepos = (root: string | null) => {
|
export const useNestedRepos = (root: string | null) => {
|
||||||
return useGitStore((state) => {
|
return useGitStore((state) => {
|
||||||
if (!root) return undefined;
|
if (!root) return undefined;
|
||||||
|
|||||||
@@ -388,6 +388,10 @@ const handleLocalApiRequest = async (input: RequestInfo | URL, url: URL, init: R
|
|||||||
return unsupportedWebRouteResponse('Scheduled tasks');
|
return unsupportedWebRouteResponse('Scheduled tasks');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (normalizedPathname === '/api/fs/git-dirs') {
|
||||||
|
return unsupportedWebRouteResponse('Nested git repository discovery');
|
||||||
|
}
|
||||||
|
|
||||||
if (normalizedPathname === '/api/sessions/snapshot' && method === 'GET') {
|
if (normalizedPathname === '/api/sessions/snapshot' && method === 'GET') {
|
||||||
const activity = await sendBridgeMessage<Record<string, { type: 'idle' | 'busy' | 'cooldown' }>>('api:session-activity:get')
|
const activity = await sendBridgeMessage<Record<string, { type: 'idle' | 'busy' | 'cooldown' }>>('api:session-activity:get')
|
||||||
.catch(() => ({}));
|
.catch(() => ({}));
|
||||||
|
|||||||
Reference in New Issue
Block a user