fix(ui): reconcile git state after worktree changes
This commit is contained in:
@@ -494,7 +494,7 @@ interface GitWorktreeAPI {
|
||||
|
||||
export interface GitAPI {
|
||||
checkIsGitRepository(directory: string): Promise<boolean>;
|
||||
getGitStatus(directory: string, options?: { mode?: 'light' }): Promise<GitStatus>;
|
||||
getGitStatus(directory: string, options?: { mode?: 'light'; fresh?: boolean }): Promise<GitStatus>;
|
||||
getGitDiff(directory: string, options: GetGitDiffOptions): Promise<GitDiffResponse>;
|
||||
getGitFileDiff(directory: string, options: GetGitFileDiffOptions): Promise<GitFileDiffResponse>;
|
||||
getGitRangeDiff?(directory: string, options: GetGitRangeDiffOptions): Promise<GitDiffResponse>;
|
||||
|
||||
@@ -84,7 +84,7 @@ export async function checkIsGitRepository(directory: string): Promise<boolean>
|
||||
return gitHttp.checkIsGitRepository(directory);
|
||||
}
|
||||
|
||||
export async function getGitStatus(directory: string, options?: { mode?: 'light' }): Promise<import('./api/types').GitStatus> {
|
||||
export async function getGitStatus(directory: string, options?: { mode?: 'light'; fresh?: boolean }): Promise<import('./api/types').GitStatus> {
|
||||
const runtime = getRuntimeGit();
|
||||
if (runtime) return runtime.getGitStatus(directory, options);
|
||||
return gitHttp.getGitStatus(directory, options);
|
||||
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
unstageGitFiles,
|
||||
} from './gitApiHttp';
|
||||
import type { GitStatus } from './api/types';
|
||||
import { sessionEvents } from './sessionEvents';
|
||||
|
||||
type FetchCall = {
|
||||
input: RequestInfo | URL;
|
||||
@@ -141,6 +142,28 @@ describe('gitApiHttp index mutations', () => {
|
||||
});
|
||||
|
||||
describe('gitApiHttp status cache', () => {
|
||||
test('a Git refresh hint invalidates the cached status before listeners fetch', async () => {
|
||||
installWindowMock();
|
||||
let statusRequestCount = 0;
|
||||
globalThis.fetch = async () => {
|
||||
statusRequestCount += 1;
|
||||
return jsonResponse(statusPayload({ behind: statusRequestCount }));
|
||||
};
|
||||
|
||||
try {
|
||||
const directory = '/repo-cache-tool-mutation';
|
||||
const first = await getGitStatus(directory);
|
||||
sessionEvents.requestGitRefresh({ directory });
|
||||
const afterMutation = await getGitStatus(directory);
|
||||
|
||||
expect(first.behind).toBe(1);
|
||||
expect(afterMutation.behind).toBe(2);
|
||||
expect(statusRequestCount).toBe(2);
|
||||
} finally {
|
||||
restoreMocks();
|
||||
}
|
||||
});
|
||||
|
||||
test('invalidates cached status after fetch', async () => {
|
||||
installWindowMock();
|
||||
const calls: FetchCall[] = [];
|
||||
@@ -188,6 +211,58 @@ describe('gitApiHttp status cache', () => {
|
||||
restoreMocks();
|
||||
}
|
||||
});
|
||||
|
||||
test('fresh status bypasses an unexpired cached snapshot', async () => {
|
||||
installWindowMock();
|
||||
let statusRequestCount = 0;
|
||||
globalThis.fetch = (async () => {
|
||||
statusRequestCount += 1;
|
||||
return jsonResponse(statusPayload({ behind: statusRequestCount }));
|
||||
}) as typeof fetch;
|
||||
|
||||
try {
|
||||
const directory = '/repo-cache-fresh';
|
||||
const first = await getGitStatus(directory);
|
||||
const cached = await getGitStatus(directory);
|
||||
const fresh = await getGitStatus(directory, { fresh: true });
|
||||
|
||||
expect(first.behind).toBe(1);
|
||||
expect(cached.behind).toBe(1);
|
||||
expect(fresh.behind).toBe(2);
|
||||
expect(statusRequestCount).toBe(2);
|
||||
} finally {
|
||||
restoreMocks();
|
||||
}
|
||||
});
|
||||
|
||||
test('fresh status cannot be replaced in cache by an older in-flight response', async () => {
|
||||
installWindowMock();
|
||||
const statusResolvers: Array<(response: Response) => void> = [];
|
||||
// SAFETY: the mock accepts the same arguments as fetch and always returns
|
||||
// a pending Response promise controlled by this test.
|
||||
globalThis.fetch = (async () => new Promise<Response>((resolve) => {
|
||||
statusResolvers.push(resolve);
|
||||
})) as typeof fetch;
|
||||
|
||||
try {
|
||||
const directory = '/repo-cache-fresh-race';
|
||||
const older = getGitStatus(directory);
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
const fresh = getGitStatus(directory, { fresh: true });
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(statusResolvers).toHaveLength(2);
|
||||
statusResolvers[1](jsonResponse(statusPayload({ current: 'fresh' })));
|
||||
statusResolvers[0](jsonResponse(statusPayload({ current: 'stale' })));
|
||||
|
||||
expect((await fresh).current).toBe('fresh');
|
||||
expect((await older).current).toBe('stale');
|
||||
expect((await getGitStatus(directory)).current).toBe('fresh');
|
||||
expect(statusResolvers).toHaveLength(2);
|
||||
} finally {
|
||||
restoreMocks();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
const statusPayload = (overrides: Partial<GitStatus> = {}): GitStatus => ({
|
||||
|
||||
@@ -40,7 +40,7 @@ import { normalizePath } from './pathNormalization';
|
||||
import { runtimeFetch } from './runtime-fetch';
|
||||
import { getRuntimeUrlResolver } from './runtime-url';
|
||||
import { getRuntimeKey } from './runtime-switch';
|
||||
import { notifyGitStatusInvalidated } from './gitStatusInvalidation';
|
||||
import { notifyGitStatusInvalidated, subscribeGitStatusInvalidations } from './gitStatusInvalidation';
|
||||
|
||||
const API_BASE = '/api/git';
|
||||
const GIT_STATUS_CACHE_TTL_MS = 1200;
|
||||
@@ -60,8 +60,7 @@ const getStatusCacheKey = (runtimeKey: string, directory: string, mode?: 'light'
|
||||
const getStatusCacheVersion = (runtimeKey: string, directory: string): number =>
|
||||
gitStatusCacheVersions.get(getDirectoryCacheKey(runtimeKey, directory)) ?? 0;
|
||||
|
||||
const invalidateGitStatusCache = (directory: string): void => {
|
||||
const runtimeKey = getRuntimeKey();
|
||||
const clearGitStatusCache = (runtimeKey: string, directory: string): void => {
|
||||
const key = getDirectoryCacheKey(runtimeKey, directory);
|
||||
gitStatusCacheVersions.set(key, getStatusCacheVersion(runtimeKey, directory) + 1);
|
||||
for (const mode of [undefined, 'light'] as const) {
|
||||
@@ -69,6 +68,13 @@ const invalidateGitStatusCache = (directory: string): void => {
|
||||
gitStatusCache.delete(statusKey);
|
||||
gitStatusInFlight.delete(statusKey);
|
||||
}
|
||||
};
|
||||
|
||||
subscribeGitStatusInvalidations((directory) => {
|
||||
clearGitStatusCache(getRuntimeKey(), directory);
|
||||
});
|
||||
|
||||
const invalidateGitStatusCache = (directory: string): void => {
|
||||
notifyGitStatusInvalidated(directory);
|
||||
};
|
||||
|
||||
@@ -162,9 +168,14 @@ export async function listGitDirectories(root: string): Promise<string[]> {
|
||||
.filter((path): path is string => path !== null);
|
||||
}
|
||||
|
||||
export async function getGitStatus(directory: string, options?: { mode?: 'light' }): Promise<GitStatus> {
|
||||
export async function getGitStatus(directory: string, options?: { mode?: 'light'; fresh?: boolean }): Promise<GitStatus> {
|
||||
const mode = options?.mode;
|
||||
const runtimeKey = getRuntimeKey();
|
||||
if (options?.fresh) {
|
||||
// A forced read must cross the transport cache boundary too. Advancing the
|
||||
// version also prevents an older in-flight response from repopulating it.
|
||||
clearGitStatusCache(runtimeKey, directory);
|
||||
}
|
||||
const key = getStatusCacheKey(runtimeKey, directory, mode);
|
||||
const now = Date.now();
|
||||
const cached = gitStatusCache.get(key);
|
||||
|
||||
@@ -1,18 +1,18 @@
|
||||
/**
|
||||
* Minimal notification channel for git status invalidation.
|
||||
*
|
||||
* Every successful status-affecting git mutation must call
|
||||
* Every confirmed status-affecting mutation must call
|
||||
* `notifyGitStatusInvalidated`. `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.
|
||||
* cannot commit over newer authoritative state. The HTTP adapter also subscribes
|
||||
* and clears its short-lived status cache.
|
||||
*
|
||||
* Runtime parity: this is about the store's in-flight status request, not about
|
||||
* adapter caching, so it applies to every runtime. The HTTP adapter in
|
||||
* `gitApiHttp.ts` emits it where it clears its own cache; runtime adapters (the
|
||||
* VS Code bridge) have no cache of their own, so the dispatch layer in
|
||||
* `gitApi.ts` emits it for them after a successful runtime mutation. Either
|
||||
* path announces a mutation exactly once.
|
||||
* adapter caching, so it applies to every runtime. HTTP mutations emit from
|
||||
* `gitApiHttp.ts`; runtime adapters such as the VS Code bridge emit from the
|
||||
* dispatch layer in `gitApi.ts`. Tool and editor mutations emit through the
|
||||
* shared Git refresh hint. Each path announces a mutation exactly once.
|
||||
*/
|
||||
|
||||
type GitStatusInvalidationListener = (directory: string) => void;
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import type { Session } from '@opencode-ai/sdk/v2';
|
||||
import type { Part } from '@opencode-ai/sdk/v2/client';
|
||||
import { notifyGitStatusInvalidated } from '@/lib/gitStatusInvalidation';
|
||||
import type { WorktreeMetadata } from '@/types/worktree';
|
||||
|
||||
export type SessionDeleteRequest = {
|
||||
@@ -24,6 +26,12 @@ const deleteListeners = new Set<DeleteListener>();
|
||||
const createListeners = new Set<CreateListener>();
|
||||
const directoryListeners = new Set<DirectoryListener>();
|
||||
const gitRefreshListeners = new Set<GitRefreshListener>();
|
||||
const gitMutatingTools = new Set(['bash', 'edit', 'write', 'apply_patch', 'patch']);
|
||||
|
||||
const normalizeToolName = (tool: string): string => {
|
||||
const parts = tool.trim().toLowerCase().split('.').filter(Boolean);
|
||||
return parts[parts.length - 1] ?? '';
|
||||
};
|
||||
|
||||
export const sessionEvents = {
|
||||
onDeleteRequest(listener: DeleteListener) {
|
||||
@@ -67,6 +75,19 @@ export const sessionEvents = {
|
||||
if (!hint.directory.trim()) {
|
||||
return;
|
||||
}
|
||||
notifyGitStatusInvalidated(hint.directory);
|
||||
gitRefreshListeners.forEach((listener) => listener(hint));
|
||||
},
|
||||
requestGitRefreshForToolTransition(directory: string, previousPart: Part | undefined, nextPart: Part) {
|
||||
if (nextPart.type !== 'tool' || nextPart.state.status !== 'completed') {
|
||||
return;
|
||||
}
|
||||
if (previousPart?.type === 'tool' && previousPart.state.status === 'completed') {
|
||||
return;
|
||||
}
|
||||
if (!gitMutatingTools.has(normalizeToolName(nextPart.tool))) {
|
||||
return;
|
||||
}
|
||||
sessionEvents.requestGitRefresh({ directory });
|
||||
},
|
||||
};
|
||||
|
||||
@@ -106,6 +106,7 @@ const {
|
||||
getLatestWorktreeMetadata,
|
||||
listProjectWorktrees,
|
||||
partitionWorktreesByRegisteredProject,
|
||||
removeProjectWorktree,
|
||||
validateWorktreeCreate,
|
||||
worktreeMapsEqual,
|
||||
} = await import('./worktreeManager');
|
||||
@@ -365,6 +366,44 @@ describe('worktreeManager list invalidation', () => {
|
||||
expect(metadata.worktreeStatus).toBe('pending');
|
||||
expect(getLatestWorktreeMetadata(metadata).worktreeStatus).toBe('ready');
|
||||
});
|
||||
|
||||
test('removes a worktree from sidebar topology owned by another registered checkout', async () => {
|
||||
const removed: WorktreeMetadata = {
|
||||
path: '/worktrees/removed',
|
||||
projectDirectory: '/repo',
|
||||
branch: 'removed',
|
||||
label: 'removed',
|
||||
};
|
||||
const sibling: WorktreeMetadata = {
|
||||
path: '/worktrees/sibling',
|
||||
projectDirectory: '/repo',
|
||||
branch: 'sibling',
|
||||
label: 'sibling',
|
||||
};
|
||||
const unrelatedEntries: WorktreeMetadata[] = [{
|
||||
path: '/other/worktree',
|
||||
projectDirectory: '/other',
|
||||
branch: 'other',
|
||||
label: 'other',
|
||||
}];
|
||||
sessionState.availableWorktreesByProject = new Map([
|
||||
['/worktrees/configured', [removed, sibling]],
|
||||
['/other', unrelatedEntries],
|
||||
]);
|
||||
sessionState.availableWorktrees = [removed, sibling, ...unrelatedEntries];
|
||||
sessionState.worktreeMetadata = new Map([
|
||||
['removed-session', removed],
|
||||
['sibling-session', sibling],
|
||||
]);
|
||||
|
||||
await removeProjectWorktree({ id: 'path:/repo', path: '/repo' }, removed);
|
||||
|
||||
expect(sessionState.availableWorktreesByProject.get('/worktrees/configured')).toEqual([sibling]);
|
||||
expect(sessionState.availableWorktreesByProject.get('/other')).toBe(unrelatedEntries);
|
||||
expect(sessionState.availableWorktrees).toEqual([sibling, ...unrelatedEntries]);
|
||||
expect(sessionState.worktreeMetadata.has('removed-session')).toBe(false);
|
||||
expect(sessionState.worktreeMetadata.get('sibling-session')).toBe(sibling);
|
||||
});
|
||||
});
|
||||
|
||||
describe('worktreeMapsEqual', () => {
|
||||
|
||||
@@ -595,14 +595,16 @@ export async function removeProjectWorktree(project: ProjectRef, worktree: Workt
|
||||
|
||||
// Update sidebar store so removed worktree disappears immediately
|
||||
const normalizedWorktreePath = normalizePath(worktree.path);
|
||||
const sidebarProjectKey = projectDirectory;
|
||||
const currentByProject = useSessionUIStore.getState().availableWorktreesByProject;
|
||||
const updatedByProject = new Map(currentByProject);
|
||||
const projectWorktrees = updatedByProject.get(sidebarProjectKey) ?? [];
|
||||
updatedByProject.set(
|
||||
sidebarProjectKey,
|
||||
projectWorktrees.filter((w) => normalizePath(w.path) !== normalizedWorktreePath),
|
||||
);
|
||||
for (const [projectKey, projectWorktrees] of currentByProject) {
|
||||
const remainingWorktrees = projectWorktrees.filter(
|
||||
(candidate) => normalizePath(candidate.path) !== normalizedWorktreePath,
|
||||
);
|
||||
if (remainingWorktrees.length !== projectWorktrees.length) {
|
||||
updatedByProject.set(projectKey, remainingWorktrees);
|
||||
}
|
||||
}
|
||||
|
||||
// Clean up worktreeMetadata for sessions in the removed worktree
|
||||
const currentMetadata = useSessionUIStore.getState().worktreeMetadata;
|
||||
|
||||
Reference in New Issue
Block a user