fix: keep worktree sessions in the right group

Prevents stale worktree lists from overwriting newly created worktrees
Uses the created worktree path when selecting linked worktree sessions
This commit is contained in:
Bohdan Triapitsyn
2026-06-03 15:16:36 +03:00
parent 2b098d36f5
commit 570ae9dbc8
5 changed files with 191 additions and 57 deletions
@@ -1679,7 +1679,7 @@ export const SessionSidebar: React.FC<SessionSidebarProps> = ({
setSessionSwitcherOpen(false); setSessionSwitcherOpen(false);
} }
if (options?.sessionId) { if (options?.sessionId) {
setCurrentSession(options.sessionId); setCurrentSession(options.sessionId, worktreePath);
return; return;
} }
openNewSessionDraft({ directoryOverride: worktreePath }); openNewSessionDraft({ directoryOverride: worktreePath });
@@ -0,0 +1,106 @@
import { beforeEach, describe, expect, mock, test } from 'bun:test';
import type { WorktreeMetadata } from '@/types/worktree';
type WorktreeListEntry = {
path?: string;
branch?: string;
head?: string;
name?: string;
};
const listCalls: string[] = [];
const listResolvers: Array<(value: WorktreeListEntry[]) => void> = [];
const createdWorktree = {
name: 'feature',
branch: 'feature',
path: '/repo-feature',
};
const sessionState = {
availableWorktreesByProject: new Map<string, WorktreeMetadata[]>(),
availableWorktrees: [] as WorktreeMetadata[],
};
mock.module('@/lib/openchamberConfig', () => ({
substituteCommandVariables: (command: string) => command,
}));
mock.module('@/lib/worktrees/worktreeBootstrap', () => ({
clearWorktreeBootstrapState: mock(),
markWorktreeBootstrapPending: mock(),
}));
mock.module('@/lib/worktrees/worktreeStatus', () => ({
invalidateResolvedProjectRootCache: mock(),
resolveProjectRoot: (directory: string) => Promise.resolve(directory),
}));
mock.module('@/sync/session-ui-store', () => ({
useSessionUIStore: {
getState: () => sessionState,
setState: (patch: Partial<typeof sessionState> | ((state: typeof sessionState) => Partial<typeof sessionState>)) => {
const next = typeof patch === 'function' ? patch(sessionState) : patch;
Object.assign(sessionState, next);
},
},
}));
mock.module('@/lib/gitApi', () => ({
deleteRemoteBranch: mock(),
git: {
worktree: {
list: (directory: string) => {
listCalls.push(directory);
return new Promise<WorktreeListEntry[]>((resolve) => {
listResolvers.push(resolve);
});
},
create: mock(() => Promise.resolve(createdWorktree)),
remove: mock(() => Promise.resolve({ success: true })),
},
},
}));
const { createWorktree, listProjectWorktrees } = await import('./worktreeManager');
const waitForListCallCount = async (count: number): Promise<void> => {
for (let attempt = 0; attempt < 10; attempt += 1) {
if (listCalls.length >= count) {
return;
}
await Promise.resolve();
}
throw new Error(`Expected ${count} worktree list calls, got ${listCalls.length}`);
};
describe('worktreeManager list invalidation', () => {
beforeEach(() => {
listCalls.length = 0;
listResolvers.length = 0;
sessionState.availableWorktreesByProject = new Map();
sessionState.availableWorktrees = [];
});
test('retries an in-flight list when a worktree is created before it resolves', async () => {
const project = { id: 'project-1', path: '/repo' };
const listing = listProjectWorktrees(project);
await waitForListCallCount(1);
await createWorktree(project, {
preferredName: 'feature',
mode: 'new',
branchName: 'feature',
worktreeName: 'feature',
});
listResolvers[0]([]);
await waitForListCallCount(2);
listResolvers[1]([createdWorktree]);
const result = await listing;
expect(listCalls).toEqual(['/repo', '/repo']);
expect(result.map((entry) => entry.path)).toEqual(['/repo-feature']);
});
});
@@ -150,22 +150,19 @@ const toCreatePayload = (args: {
// Cache worktree listings to avoid repeated git worktree list + rev-parse calls // Cache worktree listings to avoid repeated git worktree list + rev-parse calls
const _worktreeListCache = new Map<string, { value: WorktreeMetadata[]; at: number }>(); const _worktreeListCache = new Map<string, { value: WorktreeMetadata[]; at: number }>();
const _worktreeListInflight = new Map<string, Promise<WorktreeMetadata[]>>(); const _worktreeListInflight = new Map<string, Promise<WorktreeMetadata[]>>();
const _worktreeListGeneration = new Map<string, number>();
const WORKTREE_LIST_CACHE_TTL = 30_000; // 30 seconds const WORKTREE_LIST_CACHE_TTL = 30_000; // 30 seconds
export async function listProjectWorktrees(project: ProjectRef): Promise<WorktreeMetadata[]> { const getWorktreeListGeneration = (projectDirectory: string): number => {
const projectDirectory = normalizePath(project.path); return _worktreeListGeneration.get(projectDirectory) ?? 0;
};
// Return cached if fresh const invalidateWorktreeList = (projectDirectory: string): void => {
const cached = _worktreeListCache.get(projectDirectory); _worktreeListGeneration.set(projectDirectory, getWorktreeListGeneration(projectDirectory) + 1);
if (cached && Date.now() - cached.at < WORKTREE_LIST_CACHE_TTL) { _worktreeListCache.delete(projectDirectory);
return cached.value; };
}
// Dedup in-flight requests const readProjectWorktrees = async (projectDirectory: string): Promise<WorktreeMetadata[]> => {
const inflight = _worktreeListInflight.get(projectDirectory);
if (inflight) return inflight;
const promise = (async (): Promise<WorktreeMetadata[]> => {
const metadataProjectDirectory = await resolveProjectRoot(projectDirectory).catch(() => projectDirectory); const metadataProjectDirectory = await resolveProjectRoot(projectDirectory).catch(() => projectDirectory);
const normalizedProjectDirectory = normalizePath(projectDirectory); const normalizedProjectDirectory = normalizePath(projectDirectory);
@@ -195,16 +192,42 @@ export async function listProjectWorktrees(project: ProjectRef): Promise<Worktre
}) })
.filter((entry) => normalizePath(entry.path) !== normalizedProjectDirectory); .filter((entry) => normalizePath(entry.path) !== normalizedProjectDirectory);
const sorted = results.sort((a, b) => { return results.sort((a, b) => {
const aLabel = (a.label || a.branch || a.path).toLowerCase(); const aLabel = (a.label || a.branch || a.path).toLowerCase();
const bLabel = (b.label || b.branch || b.path).toLowerCase(); const bLabel = (b.label || b.branch || b.path).toLowerCase();
return aLabel.localeCompare(bLabel); return aLabel.localeCompare(bLabel);
}); });
};
_worktreeListCache.set(projectDirectory, { value: sorted, at: Date.now() }); const readStableProjectWorktrees = async (projectDirectory: string): Promise<WorktreeMetadata[]> => {
return sorted; while (true) {
})().finally(() => { const generation = getWorktreeListGeneration(projectDirectory);
const worktrees = await readProjectWorktrees(projectDirectory);
if (generation === getWorktreeListGeneration(projectDirectory)) {
_worktreeListCache.set(projectDirectory, { value: worktrees, at: Date.now() });
return worktrees;
}
}
};
export async function listProjectWorktrees(project: ProjectRef): Promise<WorktreeMetadata[]> {
const projectDirectory = normalizePath(project.path);
// Return cached if fresh
const cached = _worktreeListCache.get(projectDirectory);
if (cached && Date.now() - cached.at < WORKTREE_LIST_CACHE_TTL) {
return cached.value;
}
// Dedup in-flight requests
const inflight = _worktreeListInflight.get(projectDirectory);
if (inflight) return inflight;
const promise = readStableProjectWorktrees(projectDirectory).finally(() => {
if (_worktreeListInflight.get(projectDirectory) === promise) {
_worktreeListInflight.delete(projectDirectory); _worktreeListInflight.delete(projectDirectory);
}
}); });
_worktreeListInflight.set(projectDirectory, promise); _worktreeListInflight.set(projectDirectory, promise);
@@ -255,7 +278,7 @@ export async function createWorktree(project: ProjectRef, args: CreateWorktreeAr
markWorktreeBootstrapPending(metadata.path); markWorktreeBootstrapPending(metadata.path);
_worktreeListCache.delete(projectDirectory); invalidateWorktreeList(projectDirectory);
// The new worktree changes the repo's worktree topology; drop cached root // The new worktree changes the repo's worktree topology; drop cached root
// resolutions so root-branch lookups re-resolve against the new layout. // resolutions so root-branch lookups re-resolve against the new layout.
invalidateResolvedProjectRootCache(); invalidateResolvedProjectRootCache();
@@ -300,7 +323,7 @@ export async function removeProjectWorktree(project: ProjectRef, worktree: Workt
clearWorktreeBootstrapState(worktree.path); clearWorktreeBootstrapState(worktree.path);
_worktreeListCache.delete(normalizePath(project.path)); invalidateWorktreeList(normalizePath(project.path));
// Removing a worktree changes the repo's worktree topology; drop cached root // Removing a worktree changes the repo's worktree topology; drop cached root
// resolutions so root-branch lookups re-resolve against the new layout. // resolutions so root-branch lookups re-resolve against the new layout.
invalidateResolvedProjectRootCache(); invalidateResolvedProjectRootCache();
+9 -4
View File
@@ -5,6 +5,7 @@ import type { QuestionRequest } from "@/types/question"
// Mock SDK client that records permission.reply / question.reply calls // Mock SDK client that records permission.reply / question.reply calls
const replyCalls: Array<{ method: string; params: Record<string, unknown> }> = [] const replyCalls: Array<{ method: string; params: Record<string, unknown> }> = []
const scopedClientDirectories: string[] = [] const scopedClientDirectories: string[] = []
const registeredSessionDirectories: Array<{ sessionID: string; directory: string }> = []
let sessionRevertResult: { data?: unknown; error?: unknown; response?: { status?: number } } = {} let sessionRevertResult: { data?: unknown; error?: unknown; response?: { status?: number } } = {}
let questionReplyError: unknown | null = null let questionReplyError: unknown | null = null
@@ -139,14 +140,18 @@ mock.module("./input-store", () => ({
}, },
})) }))
// Mock useGlobalSessionsStore (imported but not used in permission functions)
mock.module("@/stores/useGlobalSessionsStore", () => ({ mock.module("@/stores/useGlobalSessionsStore", () => ({
useGlobalSessionsStore: {}, useGlobalSessionsStore: {
getState: () => ({
upsertSession: () => {},
}),
},
})) }))
// Mock sync-refs (imported but not used in permission functions)
mock.module("./sync-refs", () => ({ mock.module("./sync-refs", () => ({
registerSessionDirectory: () => {}, registerSessionDirectory: (sessionID: string, directory: string) => {
registeredSessionDirectories.push({ sessionID, directory })
},
})) }))
import { create, type StoreApi } from "zustand" import { create, type StoreApi } from "zustand"
+1 -1
View File
@@ -335,7 +335,7 @@ export async function createSession(
parentID: parentID ?? undefined, parentID: parentID ?? undefined,
}, directoryOverride ?? dir()) }, directoryOverride ?? dir())
const sessionDirectory = (session as { directory?: string }).directory ?? directoryOverride ?? null const sessionDirectory = (session as { directory?: string | null }).directory ?? null
// Pre-populate routing index so SSE events arriving before session.created // Pre-populate routing index so SSE events arriving before session.created
// can be routed to the correct child store // can be routed to the correct child store
if (sessionDirectory) { if (sessionDirectory) {