BuzzbeeSCD/openchamber
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(sessions): harden worktree move recovery

Browse Source
This commit is contained in:
mattv8
2026-08-27 18:43:37 -06:00
parent 415f443a79
commit 52bea6d1f3
10 changed files with 478 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/ui/src/lib/worktrees/sessionWorktreeMove.test.ts
+154 -31
View File
@@ -11,11 +11,10 @@ const moveCalls: Array<{
moveChanges: boolean;
}> = [];
const refreshCalls: string[][] = [];
type RemoveProjectWorktreeOptions = { deleteLocalBranch: boolean };
type RemoveProjectWorktreeCall = {
project: ProjectRef;
worktree: WorktreeMetadata;
options: RemoveProjectWorktreeOptions;
projectDirectory: string;
directory: string;
deleteLocalBranch: boolean;
};
type MoveSessionImplementation = (
session: Session,
@@ -37,15 +36,38 @@ type DeferredVoid = {
resolve: () => void;
reject: (error: Error) => void;
};
type IncompleteRollbackCause = {
moveError: Error;
rollbackFailures: Array<{ sessionId: string; error: Error }>;
};
const removeWorktreeCalls: RemoveProjectWorktreeCall[] = [];
const metadataWrites: Array<{ sessionId: string; metadata: WorktreeMetadata | null }> = [];
const latestMetadataInputs: WorktreeMetadata[] = [];
const toastSuccesses: string[] = [];
const toastErrors: Array<{ title: string; description?: string }> = [];
const directoryStates = new Map<string, DirectoryState>();
const storedMetadata = new Map<string, WorktreeMetadata | null>();
const originalConsoleWarn = console.warn;
type SessionUIState = {
availableWorktrees: WorktreeMetadata[];
availableWorktreesByProject: Map<string, WorktreeMetadata[]>;
worktreeMetadata: Map<string, WorktreeMetadata | null>;
getWorktreeMetadata: (sessionId: string) => WorktreeMetadata | null;
setWorktreeMetadata: (sessionId: string, metadata: WorktreeMetadata | null) => void;
};
type SessionUIStatePatch = Partial<SessionUIState> | ((state: SessionUIState) => Partial<SessionUIState>);
const sessionUIState: SessionUIState = {
availableWorktrees: [],
availableWorktreesByProject: new Map<string, WorktreeMetadata[]>(),
worktreeMetadata: new Map<string, WorktreeMetadata | null>(),
getWorktreeMetadata: (sessionId: string) => storedMetadata.get(sessionId) ?? null,
setWorktreeMetadata: (sessionId: string, metadata: WorktreeMetadata | null) => {
storedMetadata.set(sessionId, metadata);
metadataWrites.push({ sessionId, metadata });
},
};
let moveSessionImplementation: MoveSessionImplementation = async () => {};
let refreshImplementation: RefreshImplementation = async () => {};
@@ -74,6 +96,26 @@ mock.module('@/components/ui', () => ({
mock.module('@/lib/gitApi', () => ({
getGitStatus: mock(() => Promise.resolve({ current: 'feature' })),
deleteRemoteBranch: mock(),
git: {
worktree: {
list: mock(() => Promise.resolve([])),
create: mock(() => Promise.resolve(null)),
validate: mock(() => Promise.resolve({ ok: true, errors: [] })),
remove: mock((projectDirectory: string, options: { directory: string; deleteLocalBranch?: boolean }) => {
removeWorktreeCalls.push({
projectDirectory,
directory: options.directory,
deleteLocalBranch: options.deleteLocalBranch === true,
});
return Promise.resolve({ success: true });
}),
},
},
}));
mock.module('@/lib/openchamberConfig', () => ({
substituteCommandVariables: (command: string) => command,
}));
mock.module('@/lib/worktreeSessionCreator', () => ({
@@ -83,17 +125,15 @@ mock.module('@/lib/worktreeSessionCreator', () => ({
mock.module('@/lib/worktrees/worktreeBootstrap', () => ({
waitForWorktreeGitReady: mock((directory: string) => waitForWorktreeGitReadyImplementation(directory)),
clearWorktreeBootstrapState: mock(),
markWorktreeBootstrapPending: mock(),
setWorktreeBootstrapState: mock(),
startWorktreeBootstrapWatcher: mock(),
}));
mock.module('@/lib/worktrees/worktreeManager', () => ({
getLatestWorktreeMetadata: (metadata: WorktreeMetadata) => {
latestMetadataInputs.push(metadata);
return latestMetadataResult;
},
removeProjectWorktree: (project: ProjectRef, worktree: WorktreeMetadata, options: RemoveProjectWorktreeOptions) => {
removeWorktreeCalls.push({ project, worktree, options });
return Promise.resolve();
},
mock.module('@/lib/worktrees/worktreeStatus', () => ({
invalidateResolvedProjectRootCache: mock(),
resolveProjectRoot: (directory: string) => Promise.resolve(directory),
}));
mock.module('@/stores/useGlobalSessionsStore', () => ({
@@ -112,15 +152,17 @@ mock.module('@/sync/session-actions', () => ({
mock.module('@/sync/session-ui-store', () => ({
useSessionUIStore: {
getState: () => ({
availableWorktrees: [],
availableWorktreesByProject: new Map<string, WorktreeMetadata[]>(),
getWorktreeMetadata: (sessionId: string) => storedMetadata.get(sessionId) ?? null,
setWorktreeMetadata: (sessionId: string, metadata: WorktreeMetadata | null) => {
storedMetadata.set(sessionId, metadata);
metadataWrites.push({ sessionId, metadata });
},
}),
getState: () => sessionUIState,
setState: (patch: SessionUIStatePatch) => {
const next = patch instanceof Function ? patch(sessionUIState) : patch;
Object.assign(sessionUIState, next);
},
},
}));
mock.module('@/sync/session-worktree-store', () => ({
useSessionWorktreeStore: {
setState: mock(),
},
}));
@@ -193,18 +235,54 @@ const deferred = (): DeferredVoid => {
return { promise, resolve, reject };
};
const getIncompleteRollbackCause = (error: Error): IncompleteRollbackCause => {
const cause = error.cause;
if (!cause || !(cause instanceof Object)) {
throw new Error('Expected rollback error cause details');
}
const parsed = cause as Partial<IncompleteRollbackCause>;
if (!(parsed.moveError instanceof Error)) {
throw new Error('Expected rollback moveError cause');
}
if (!Array.isArray(parsed.rollbackFailures)) {
throw new Error('Expected rollback failures in cause');
}
const rollbackFailures = parsed.rollbackFailures.map((entry) => {
if (!entry || !(entry instanceof Object)) {
throw new Error('Expected rollback failure entry');
}
const failure = entry as { sessionId?: unknown; error?: unknown };
if (typeof failure.sessionId !== 'string') {
throw new Error('Expected rollback failure session ID');
}
if (!(failure.error instanceof Error)) {
throw new Error('Expected rollback failure error');
}
return { sessionId: failure.sessionId, error: failure.error };
});
return {
moveError: parsed.moveError,
rollbackFailures,
};
};
describe('moveSessionTreeToExistingWorktree', () => {
beforeEach(() => {
moveCalls.length = 0;
refreshCalls.length = 0;
removeWorktreeCalls.length = 0;
metadataWrites.length = 0;
latestMetadataInputs.length = 0;
toastSuccesses.length = 0;
toastErrors.length = 0;
directoryStates.clear();
storedMetadata.clear();
sessionUIState.worktreeMetadata = new Map();
sessionUIState.availableWorktreesByProject = new Map();
latestMetadataResult = makeWorktreeMetadata({ label: 'Latest destination' });
sessionUIState.availableWorktrees = [latestMetadataResult];
moveSessionImplementation = async () => {};
refreshImplementation = async () => {};
createQuickWorktreeImplementation = async () => makeWorktreeMetadata({ path: '/created-worktree', worktreeSource: 'created-for-session' });
@@ -243,7 +321,6 @@ describe('moveSessionTreeToExistingWorktree', () => {
{ sessionId: 'root', metadata: latestMetadataResult },
{ sessionId: 'child', metadata: latestMetadataResult },
]);
expect(latestMetadataInputs).toEqual([destination, destination]);
expect(refreshCalls).toEqual([['/source', '/destination']]);
expect(removeWorktreeCalls).toEqual([]);
});
@@ -390,6 +467,40 @@ describe('moveSessionTreeToExistingWorktree', () => {
}
};
const error = await moveSessionTreeToExistingWorktree({
root,
descendants: [child],
sourceDirectory: '/source',
destination: makeWorktreeMetadata(),
}).catch((rejection) => rejection);
expect(error).toBeInstanceOf(Error);
if (!(error instanceof Error)) {
throw error;
}
expect(error.message.includes('could not be fully rolled back')).toBe(true);
const cause = getIncompleteRollbackCause(error);
expect(cause.moveError.message).toBe('child failed');
expect(cause.rollbackFailures).toEqual([{ sessionId: 'root', error: new Error('rollback failed') }]);
expect(removeWorktreeCalls).toEqual([]);
});
const expectBusyOrRetryRollbackBlock = async (status: Extract<SessionStatus['type'], 'busy' | 'retry'>): Promise<void> => {
const root = makeSession('root');
const child = makeSession('child');
setStatuses('/source', { root: 'idle', child: 'idle' });
setStatuses('/destination', {});
moveSessionImplementation = async (session, sourceDirectory) => {
if (sourceDirectory === '/source' && session.id === 'root') {
setStatuses('/destination', { root: status });
return;
}
if (sourceDirectory === '/source' && session.id === 'child') {
throw new Error('child failed');
}
};
await expect(moveSessionTreeToExistingWorktree({
root,
descendants: [child],
@@ -397,7 +508,19 @@ describe('moveSessionTreeToExistingWorktree', () => {
destination: makeWorktreeMetadata(),
})).rejects.toThrow('could not be fully rolled back');
expect(moveCalls).toEqual([
{ sessionId: 'root', sourceDirectory: '/source', destinationDirectory: '/destination', moveChanges: true },
{ sessionId: 'child', sourceDirectory: '/source', destinationDirectory: '/destination', moveChanges: false },
]);
expect(removeWorktreeCalls).toEqual([]);
};
test('does not attempt rollback for a moved root that becomes busy in the destination', async () => {
await expectBusyOrRetryRollbackBlock('busy');
});
test('does not attempt rollback for a moved root that becomes retry in the destination', async () => {
await expectBusyOrRetryRollbackBlock('retry');
});
test('keeps the move successful when the post-move refresh fails', async () => {
@@ -435,9 +558,9 @@ describe('moveSessionTreeToExistingWorktree', () => {
await waitFor(() => toastErrors.length === 1);
expect(toastErrors).toEqual([{ title: 'failed', description: 'git-ready failed' }]);
expect(removeWorktreeCalls).toEqual([{
project: { id: 'project-1', path: '/repo' },
worktree: makeWorktreeMetadata({ path: '/created-worktree', worktreeSource: 'created-for-session', label: 'Destination' }),
options: { deleteLocalBranch: true },
projectDirectory: '/repo',
directory: '/created-worktree',
deleteLocalBranch: true,
}]);
expect(moveCalls).toEqual([]);
});
@@ -458,9 +581,9 @@ describe('moveSessionTreeToExistingWorktree', () => {
await waitFor(() => toastErrors.length === 1);
expect(removeWorktreeCalls).toEqual([{
project: { id: 'project-1', path: '/repo' },
worktree: makeWorktreeMetadata({ path: '/created-worktree', worktreeSource: 'created-for-session', label: 'Destination' }),
options: { deleteLocalBranch: true },
projectDirectory: '/repo',
directory: '/created-worktree',
deleteLocalBranch: true,
}]);
expect(moveCalls).toEqual([]);
});
packages/ui/src/lib/worktrees/sessionWorktreeMove.ts
+31 -4
View File
@@ -61,15 +61,39 @@ const assertSessionsIdle = (sessions: Session[], sourceDirectory: string): void
if (hasActiveSession) throw new Error('Session is not idle');
};
type RollbackFailure = {
sessionId: string;
error: Error;
};
const createIncompleteRollbackError = (moveError: Error, rollbackFailures: RollbackFailure[]): Error => {
const rollbackSummary = rollbackFailures
.map(({ sessionId, error }) => `${sessionId}: ${error.message}`)
.join(', ');
return new Error(
`Session move partially failed and could not be fully rolled back: ${moveError.message}. Rollback failures: ${rollbackSummary}`,
{ cause: { moveError, rollbackFailures } },
);
};
const isSessionBusyOrRetrying = (session: Session, directory: string): boolean => {
const status = getDirectoryState(directory)?.session_status[session.id]?.type;
return status === 'busy' || status === 'retry';
};
const rollbackMovedSessions = async (
sessions: Session[],
rootSessionId: string,
sourceDirectory: string,
worktreeDirectory: string,
previousMetadata: ReadonlyMap<string, WorktreeMetadata | undefined>,
): Promise<unknown[]> => {
const failures: unknown[] = [];
): Promise<RollbackFailure[]> => {
const failures: RollbackFailure[] = [];
for (const session of [...sessions].reverse()) {
if (isSessionBusyOrRetrying(session, worktreeDirectory)) {
failures.push({ sessionId: session.id, error: new Error('Session is not idle') });
continue;
}
try {
await moveSessionToDirectory(
session,
@@ -79,7 +103,10 @@ const rollbackMovedSessions = async (
);
useSessionUIStore.getState().setWorktreeMetadata(session.id, previousMetadata.get(session.id) ?? null);
} catch (error) {
failures.push(error);
failures.push({
sessionId: session.id,
error: error instanceof Error ? error : new Error(String(error)),
});
}
}
return failures;
@@ -149,7 +176,7 @@ const moveSessionTreeTransaction = async (
previousMetadata,
);
if (rollbackFailures.length > 0) {
throw new Error(`Session move partially failed and could not be fully rolled back: ${moveError.message}`);
throw createIncompleteRollbackError(moveError, rollbackFailures);
}
if (destination?.onMoveFailure) {
return destination.onMoveFailure(moveError);
packages/ui/src/lib/worktrees/worktreeManager.test.ts
+63
View File
@@ -12,6 +12,7 @@ type WorktreeListEntry = {
const listCalls: string[] = [];
const listResolvers: Array<(value: WorktreeListEntry[]) => void> = [];
const listRejecters: Array<(reason: Error) => void> = [];
let listImplementation: ((directory: string) => Promise<WorktreeListEntry[]>) | undefined;
const createPayloads: unknown[] = [];
const validatePayloads: unknown[] = [];
const createdWorktree = {
@@ -79,6 +80,9 @@ mock.module('@/lib/gitApi', () => ({
worktree: {
list: (directory: string) => {
listCalls.push(directory);
if (listImplementation) {
return listImplementation(directory);
}
return new Promise<WorktreeListEntry[]>((resolve, reject) => {
listResolvers.push(resolve);
listRejecters.push((reason: Error) => reject(reason));
@@ -121,6 +125,7 @@ describe('worktreeManager list invalidation', () => {
listCalls.length = 0;
listResolvers.length = 0;
listRejecters.length = 0;
listImplementation = undefined;
createPayloads.length = 0;
validatePayloads.length = 0;
bootstrapWatcherCalls.length = 0;
@@ -234,6 +239,64 @@ describe('worktreeManager list invalidation', () => {
await expect(listing).rejects.toThrow('git failed');
});
test('rejects sustained invalidation explicitly, preserves the last cached result, and allows a later retry', async () => {
const project = { id: 'project-force-convergence', path: '/repo-force-convergence' };
const oldWorktree = [{ path: '/repo-old', branch: 'old', name: 'old' } satisfies WorktreeListEntry];
const scriptedResolvers = new Map<number, (value: WorktreeListEntry[]) => void>();
let recoveryReadsAllowed = false;
listImplementation = () => {
const callNumber = listCalls.length;
if (callNumber === 8 && !recoveryReadsAllowed) {
return Promise.reject(new Error('unexpected extra read'));
}
return new Promise<WorktreeListEntry[]>((resolve) => {
scriptedResolvers.set(callNumber, resolve);
});
};
const seededListing = listProjectWorktrees(project);
await waitForListCallCount(1);
scriptedResolvers.get(1)?.(oldWorktree);
expect((await seededListing).map((entry) => entry.path)).toEqual(['/repo-old']);
const unstableListing = listProjectWorktrees(project, { force: true });
await waitForListCallCount(2);
const forcedRefreshA = listProjectWorktrees(project, { force: true });
await waitForListCallCount(3);
scriptedResolvers.get(3)?.([createdWorktree]);
expect((await forcedRefreshA).map((entry) => entry.path)).toEqual(['/repo-feature']);
scriptedResolvers.get(2)?.([{ path: '/repo-stale-a', branch: 'stale-a', name: 'stale-a' }]);
await waitForListCallCount(4);
const forcedRefreshB = listProjectWorktrees(project, { force: true });
await waitForListCallCount(5);
scriptedResolvers.get(5)?.([createdWorktree]);
expect((await forcedRefreshB).map((entry) => entry.path)).toEqual(['/repo-feature']);
scriptedResolvers.get(4)?.([{ path: '/repo-stale-b', branch: 'stale-b', name: 'stale-b' }]);
await waitForListCallCount(6);
const forcedRefreshC = listProjectWorktrees(project, { force: true });
await waitForListCallCount(7);
scriptedResolvers.get(7)?.([createdWorktree]);
expect((await forcedRefreshC).map((entry) => entry.path)).toEqual(['/repo-feature']);
scriptedResolvers.get(6)?.([{ path: '/repo-stale-c', branch: 'stale-c', name: 'stale-c' }]);
await expect(unstableListing).rejects.toThrow('Worktree list did not converge');
expect(listCalls).toHaveLength(7);
const cachedResult = await listProjectWorktrees(project);
expect(cachedResult.map((entry) => entry.path)).toEqual(['/repo-feature']);
expect(listCalls).toHaveLength(7);
recoveryReadsAllowed = true;
const recoveredListing = listProjectWorktrees(project, { force: true });
await waitForListCallCount(8);
scriptedResolvers.get(8)?.([createdWorktree]);
expect((await recoveredListing).map((entry) => entry.path)).toEqual(['/repo-feature']);
});
test('marks fast-created worktrees pending until bootstrap settles', async () => {
const metadata = await createWorktree({ id: 'project-1', path: '/repo' }, {
preferredName: 'feature',
packages/ui/src/lib/worktrees/worktreeManager.ts
+23 -6
View File
@@ -377,6 +377,7 @@ const _worktreeListCache = new Map<string, { value: WorktreeMetadata[]; at: numb
const _worktreeListInflight = new Map<string, { generation: number; promise: Promise<WorktreeMetadata[]> }>();
const _worktreeListGeneration = new Map<string, number>();
const WORKTREE_LIST_CACHE_TTL = 30_000; // 30 seconds
const WORKTREE_LIST_MAX_CONVERGENCE_ATTEMPTS = 3;
const getWorktreeListGeneration = (projectDirectory: string): number => {
return _worktreeListGeneration.get(projectDirectory) ?? 0;
@@ -428,7 +429,7 @@ const readStableProjectWorktrees = async (
projectDirectory: string,
minimumGeneration = getWorktreeListGeneration(projectDirectory),
): Promise<WorktreeMetadata[]> => {
while (true) {
for (let attempt = 0; attempt < WORKTREE_LIST_MAX_CONVERGENCE_ATTEMPTS; attempt += 1) {
const generation = getWorktreeListGeneration(projectDirectory);
const worktrees = await readProjectWorktrees(projectDirectory);
@@ -437,11 +438,16 @@ const readStableProjectWorktrees = async (
return worktrees;
}
}
throw new Error(
`Worktree list did not converge after ${WORKTREE_LIST_MAX_CONVERGENCE_ATTEMPTS} attempts`
);
};
export async function listProjectWorktrees(project: ProjectRef, options?: { force?: boolean }): Promise<WorktreeMetadata[]> {
const projectDirectory = normalizePath(project.path);
const force = options?.force === true;
const previousCache = force ? _worktreeListCache.get(projectDirectory) : undefined;
if (force) {
invalidateWorktreeList(projectDirectory);
@@ -459,11 +465,22 @@ export async function listProjectWorktrees(project: ProjectRef, options?: { forc
const inflight = _worktreeListInflight.get(projectDirectory);
if (inflight && inflight.generation === generation) return inflight.promise;
const promise = readStableProjectWorktrees(projectDirectory, generation).finally(() => {
if (_worktreeListInflight.get(projectDirectory)?.promise === promise) {
_worktreeListInflight.delete(projectDirectory);
}
});
const promise = readStableProjectWorktrees(projectDirectory, generation)
.catch((error) => {
if (
previousCache
&& !_worktreeListCache.has(projectDirectory)
&& getWorktreeListGeneration(projectDirectory) === generation
) {
_worktreeListCache.set(projectDirectory, previousCache);
}
throw error;
})
.finally(() => {
if (_worktreeListInflight.get(projectDirectory)?.promise === promise) {
_worktreeListInflight.delete(projectDirectory);
}
});
_worktreeListInflight.set(projectDirectory, { generation, promise });
return promise;