diff --git a/packages/ui/src/components/session/SessionSidebar.tsx b/packages/ui/src/components/session/SessionSidebar.tsx index f3fc8c15..a2f6fe1d 100644 --- a/packages/ui/src/components/session/SessionSidebar.tsx +++ b/packages/ui/src/components/session/SessionSidebar.tsx @@ -564,6 +564,7 @@ const SessionSidebarComponent: React.FC = ({ : (args.sourceDirectory ? resolveProjectRef(args.sourceDirectory) : null); return startSessionWorktreeMenuLoad(args, { projects, + getCurrentProjects: () => useProjectsStore.getState().projects, rawWorktreesByProjectRef, getPublishedWorktreesByProject: () => useSessionUIStore.getState().availableWorktreesByProject, resolveProject: (directory) => resolveProjectRef(directory), diff --git a/packages/ui/src/components/session/sidebar/sessionWorktreeMenu.test.ts b/packages/ui/src/components/session/sidebar/sessionWorktreeMenu.test.ts index 4b87fbea..1821ee3c 100644 --- a/packages/ui/src/components/session/sidebar/sessionWorktreeMenu.test.ts +++ b/packages/ui/src/components/session/sidebar/sessionWorktreeMenu.test.ts @@ -197,6 +197,10 @@ describe('startSessionWorktreeMenuLoad', () => { { id: 'linked', path: '/repo-linked' }, { id: 'other', path: '/repo-other' }, ], + getCurrentProjects: () => [ + { id: 'linked', path: '/repo-linked' }, + { id: 'other', path: '/repo-other' }, + ], rawWorktreesByProjectRef: rawRef, getPublishedWorktreesByProject: () => new Map(), resolveProject: () => null, @@ -259,6 +263,7 @@ describe('startSessionWorktreeMenuLoad', () => { }, { projects: [{ id: 'linked', path: '/repo-linked' }], + getCurrentProjects: () => [{ id: 'linked', path: '/repo-linked' }], rawWorktreesByProjectRef: rawRef, getPublishedWorktreesByProject: () => new Map([['/repo-linked', [existing]]]), resolveProject: () => null, @@ -303,6 +308,7 @@ describe('startSessionWorktreeMenuLoad', () => { }, { projects: [{ id: 'linked', path: '/repo-linked' }], + getCurrentProjects: () => [{ id: 'linked', path: '/repo-linked' }], rawWorktreesByProjectRef: rawRef, getPublishedWorktreesByProject: () => publishedTopology, resolveProject: () => null, @@ -352,6 +358,10 @@ describe('startSessionWorktreeMenuLoad', () => { { id: 'owner', path: '/repo' }, { id: 'linked', path: '/repo-linked' }, ], + getCurrentProjects: () => [ + { id: 'owner', path: '/repo' }, + { id: 'linked', path: '/repo-linked' }, + ], rawWorktreesByProjectRef: rawRef, getPublishedWorktreesByProject: () => new Map([['/repo', [ownerExisting]]]), resolveProject: () => null, @@ -398,6 +408,7 @@ describe('startSessionWorktreeMenuLoad', () => { }, { projects: [{ id: 'linked', path: '/repo-linked' }], + getCurrentProjects: () => [{ id: 'linked', path: '/repo-linked' }], rawWorktreesByProjectRef: rawRef, getPublishedWorktreesByProject: () => publishedCurrentRuntime, resolveProject: () => null, @@ -433,6 +444,53 @@ describe('startSessionWorktreeMenuLoad', () => { expect(published).toEqual([]); }); + test('rejects a deferred refresh when the owning project is removed before commit', async () => { + const refreshDeferred = createDeferred(); + const existing = worktree({ path: '/repo-existing', branch: 'existing', label: 'existing', name: 'existing' }); + const published: Array<{ availableWorktreesByProject: Map }> = []; + const rawRef = rawScope('runtime-1', [ + ['/repo-linked', [existing]], + ]); + let currentProjects = [{ id: 'linked', path: '/repo-linked' }]; + + const load = startSessionWorktreeMenuLoad( + { + projectId: 'linked', + sourceDirectory: '/repo-current', + currentWorktree: worktree({ path: '/repo-current', branch: 'current', label: 'current' }), + }, + { + projects: currentProjects, + rawWorktreesByProjectRef: rawRef, + getPublishedWorktreesByProject: () => new Map([['/repo-linked', [existing]]]), + resolveProject: () => null, + listProjectWorktrees: async () => refreshDeferred.promise, + partitionWorktreesByRegisteredProject: (_projects, worktreesByProject) => new Map(worktreesByProject), + worktreeMapsEqual: () => false, + recordWorktreesSeen: () => {}, + publishTopology: (next) => { + published.push({ availableWorktreesByProject: next.availableWorktreesByProject }); + }, + getCurrentProjects: () => currentProjects, + getRuntimeKey: () => 'runtime-1', + now: () => 123, + projectRootBranch: null, + }, + ); + + currentProjects = []; + refreshDeferred.resolve([ + worktree({ path: '/repo-new', projectDirectory: '/repo', branch: 'new', label: 'new', name: 'new' }), + ]); + + const refreshError = await load.refreshTargets.catch((error) => error); + + expect(refreshError).toBeInstanceOf(Error); + expect(refreshError.message).toBe('Project removed during worktree refresh'); + expect(rawRef.current.worktreesByProject.get('/repo-linked')).toEqual([existing]); + expect(published).toEqual([]); + }); + test('falls back to resolving the owning configured project from the source directory when projectId is missing', async () => { const calls: string[] = []; const load = startSessionWorktreeMenuLoad( @@ -443,6 +501,7 @@ describe('startSessionWorktreeMenuLoad', () => { }, { projects: [{ id: 'owner', path: '/repo' }], + getCurrentProjects: () => [{ id: 'owner', path: '/repo' }], rawWorktreesByProjectRef: rawScope('runtime-1', []), getPublishedWorktreesByProject: () => new Map(), resolveProject: (directory) => { diff --git a/packages/ui/src/components/session/sidebar/sessionWorktreeMenu.ts b/packages/ui/src/components/session/sidebar/sessionWorktreeMenu.ts index 081cd376..60512ad8 100644 --- a/packages/ui/src/components/session/sidebar/sessionWorktreeMenu.ts +++ b/packages/ui/src/components/session/sidebar/sessionWorktreeMenu.ts @@ -26,6 +26,7 @@ type SessionWorktreeMenuState = { type StartSessionWorktreeMenuLoadDependencies = { projects: ReadonlyArray; + getCurrentProjects: () => ReadonlyArray; rawWorktreesByProjectRef: { current: RawWorktreesByProjectScope }; getPublishedWorktreesByProject: () => Map; resolveProject: (directory: string) => ProjectRef | null; @@ -285,6 +286,12 @@ export const startSessionWorktreeMenuLoad = ( throw new Error('Runtime changed during worktree refresh'); } + const currentProjects = deps.getCurrentProjects(); + const currentProject = currentProjects.find((candidate) => candidate.id === project.id) ?? null; + if (!currentProject || normalizePath(currentProject.path ?? null) !== normalizedProjectPath) { + throw new Error('Project removed during worktree refresh'); + } + const currentRawScope = ensureRawWorktreesByProjectScope({ rawWorktreesByProjectRef: deps.rawWorktreesByProjectRef, publishedWorktreesByProject: deps.getPublishedWorktreesByProject(), @@ -327,7 +334,7 @@ export const startSessionWorktreeMenuLoad = ( worktreesByProject: nextRawTopology, }; - const partitionedWorktreesByProject = deps.partitionWorktreesByRegisteredProject(deps.projects, nextRawTopology); + const partitionedWorktreesByProject = deps.partitionWorktreesByRegisteredProject(currentProjects, nextRawTopology); const allWorktrees = [...partitionedWorktreesByProject.values()].flat(); deps.recordWorktreesSeen(allWorktrees.map((worktree) => worktree.path), deps.now()); diff --git a/packages/ui/src/components/session/sidebar/sessions/SessionNodeItem.tsx b/packages/ui/src/components/session/sidebar/sessions/SessionNodeItem.tsx index 9a3f0240..5cb49006 100644 --- a/packages/ui/src/components/session/sidebar/sessions/SessionNodeItem.tsx +++ b/packages/ui/src/components/session/sidebar/sessions/SessionNodeItem.tsx @@ -26,7 +26,7 @@ import { useGlobalSessionStatus, useSessionPermissions, useSessionQuestionCount import { useSessionMessageRecordsForExport } from '@/sync/use-sync'; import { useViewportStore, viewportSessionKey } from '@/sync/viewport-store'; import { DraggableSessionRow } from '../folders/sessionFolderDnd'; -import { nodeContainsSessionId, nodeHasPinnedMembershipChange, selectQuestionBadgeSessionScopes } from './sessionNodeItemUtils'; +import { getSessionWorktreeMenuDisabled, nodeContainsSessionId, nodeHasPinnedMembershipChange, selectQuestionBadgeSessionScopes } from './sessionNodeItemUtils'; import type { SessionNode } from '../types'; import { formatProjectLabel, formatSessionCompactDateLabel, formatSessionDateLabel, normalizePath, renderHighlightedText } from '../utils'; import { useProjectsStore } from '@/stores/useProjectsStore'; @@ -993,102 +993,106 @@ function SessionNodeItemComponent(props: SessionNodeItemProps): React.ReactNode {t('sessions.sidebar.session.menu.exportMarkdown')} {!isSubtaskSession && !archivedBucket && !isVSCode ? (() => { + const isWorktreeMenuDisabled = getSessionWorktreeMenuDisabled({ + sessionDirectory, + isStreaming, + isMovingToWorktree, + }); const worktreeMenuState = getSessionWorktreeMenuState({ targets: worktreeTargets, isRefreshing: worktreeTargetsLoading, loadFailed: worktreeTargetsLoadFailed, }); return ( - - - - - - - {t('sessions.sidebar.session.menu.moveToWorktreeTargets')} - - - {worktreeTargets.map((target) => { - const targetPath = normalizePath(target.metadata.path ?? null) ?? target.metadata.path; - const itemLabel = target.isPrimary - ? t('sessions.sidebar.session.moveToWorktree.main') - : (target.metadata.label || target.metadata.branch || target.metadata.name || target.metadata.path); - const isDisabled = target.isCurrent || target.metadata.worktreeStatus !== 'ready'; + + + + + + {t('sessions.sidebar.session.menu.moveToWorktreeTargets')} + + + + {isMovingToWorktree + ? t('sessions.sidebar.session.moveToWorktree.tooltipMoving') + : isStreaming + ? t('sessions.sidebar.session.moveToWorktree.tooltipBusy') + : t('sessions.sidebar.session.moveToWorktree.tooltipTargets')} + + + + {worktreeTargets.map((target) => { + const targetPath = normalizePath(target.metadata.path ?? null) ?? target.metadata.path; + const itemLabel = target.isPrimary + ? t('sessions.sidebar.session.moveToWorktree.main') + : (target.metadata.label || target.metadata.branch || target.metadata.name || target.metadata.path); + const isDisabled = target.isCurrent || target.metadata.worktreeStatus !== 'ready'; - return ( - { - if (isDisabled || !sessionDirectory) { - return; - } - startSessionTreeExistingWorktreeMove({ - root: resolvedSession, - descendants: collectNodeDescendantSessions(node), - sourceDirectory: sessionDirectory, - destination: target.metadata, - successMessage: t('sessions.sidebar.session.moveToWorktree.existingSuccess'), - failureMessage: t('sessions.sidebar.session.moveToWorktree.existingFailed'), - }); - }} - > - - {itemLabel} - {target.isCurrent ? {t('sessions.sidebar.session.moveToWorktree.current')} : null} - - {target.isCurrent ? - ); - })} - {worktreeMenuState.refreshState === 'loading' ? ( - - {t('sessions.sidebar.session.moveToWorktree.refreshing')} - - ) : null} - {worktreeMenuState.refreshState === 'error' ? ( - - {t('sessions.sidebar.session.moveToWorktree.loadFailed')} - - ) : null} - - {worktreeMenuState.showNewWorktreeAction ? ( - { - if (!sessionDirectory || isStreaming || isMovingToWorktree) return; - startSessionTreeWorktreeMove({ - root: resolvedSession, - descendants: collectNodeDescendantSessions(node), - sourceDirectory: sessionDirectory, - successMessage: t('sessions.sidebar.session.moveToWorktree.success'), - failureMessage: t('sessions.sidebar.session.moveToWorktree.failed'), - }); - }} - className="[&>svg]:mr-1" - > - - {t('sessions.sidebar.session.menu.newWorktree')} - - ) : null} - - - - - - {isMovingToWorktree - ? t('sessions.sidebar.session.moveToWorktree.tooltipMoving') - : isStreaming - ? t('sessions.sidebar.session.moveToWorktree.tooltipBusy') - : t('sessions.sidebar.session.moveToWorktree.tooltipTargets')} - - + return ( + { + if (isDisabled || !sessionDirectory) { + return; + } + startSessionTreeExistingWorktreeMove({ + root: resolvedSession, + descendants: collectNodeDescendantSessions(node), + sourceDirectory: sessionDirectory, + destination: target.metadata, + successMessage: t('sessions.sidebar.session.moveToWorktree.existingSuccess'), + failureMessage: t('sessions.sidebar.session.moveToWorktree.existingFailed'), + }); + }} + > + + {itemLabel} + {target.isCurrent ? {t('sessions.sidebar.session.moveToWorktree.current')} : null} + + {target.isCurrent ? + ); + })} + {worktreeMenuState.refreshState === 'loading' ? ( + + {t('sessions.sidebar.session.moveToWorktree.refreshing')} + + ) : null} + {worktreeMenuState.refreshState === 'error' ? ( + + {t('sessions.sidebar.session.moveToWorktree.loadFailed')} + + ) : null} + + {worktreeMenuState.showNewWorktreeAction ? ( + { + if (isWorktreeMenuDisabled || !sessionDirectory) return; + startSessionTreeWorktreeMove({ + root: resolvedSession, + descendants: collectNodeDescendantSessions(node), + sourceDirectory: sessionDirectory, + successMessage: t('sessions.sidebar.session.moveToWorktree.success'), + failureMessage: t('sessions.sidebar.session.moveToWorktree.failed'), + }); + }} + className="[&>svg]:mr-1" + > + + {t('sessions.sidebar.session.menu.newWorktree')} + + ) : null} + + ); })() : null} {isMultiRunLikeSession ? ( diff --git a/packages/ui/src/components/session/sidebar/sessions/sessionNodeItemUtils.test.ts b/packages/ui/src/components/session/sidebar/sessions/sessionNodeItemUtils.test.ts index 27ff998b..07c136b5 100644 --- a/packages/ui/src/components/session/sidebar/sessions/sessionNodeItemUtils.test.ts +++ b/packages/ui/src/components/session/sidebar/sessions/sessionNodeItemUtils.test.ts @@ -2,7 +2,13 @@ import { describe, expect, test } from 'bun:test'; import type { Session } from '@opencode-ai/sdk/v2'; import { getRuntimeKey } from '@/lib/runtime-switch'; import { getPinnedSessionKey } from '@/stores/useSessionPinnedStore'; -import { computeNodeStructureKey, nodeHasPinnedMembershipChange, selectFolderRootNodes, selectQuestionBadgeSessionScopes } from './sessionNodeItemUtils'; +import { + computeNodeStructureKey, + getSessionWorktreeMenuDisabled, + nodeHasPinnedMembershipChange, + selectFolderRootNodes, + selectQuestionBadgeSessionScopes, +} from './sessionNodeItemUtils'; import type { SessionNode } from '../types'; const session = (id: string, title: string): Session => ({ @@ -158,3 +164,31 @@ describe('selectFolderRootNodes', () => { expect(selectFolderRootNodes(['missing-root', 'child'], new Map([['child', child]]))).toEqual([child]); }); }); + +describe('getSessionWorktreeMenuDisabled', () => { + test('shares the parent trigger disabled contract with the new worktree action', () => { + expect(getSessionWorktreeMenuDisabled({ + sessionDirectory: '/repo-feature', + isStreaming: false, + isMovingToWorktree: false, + })).toBe(false); + + expect(getSessionWorktreeMenuDisabled({ + sessionDirectory: null, + isStreaming: false, + isMovingToWorktree: false, + })).toBe(true); + + expect(getSessionWorktreeMenuDisabled({ + sessionDirectory: '/repo-feature', + isStreaming: true, + isMovingToWorktree: false, + })).toBe(true); + + expect(getSessionWorktreeMenuDisabled({ + sessionDirectory: '/repo-feature', + isStreaming: false, + isMovingToWorktree: true, + })).toBe(true); + }); +}); diff --git a/packages/ui/src/components/session/sidebar/sessions/sessionNodeItemUtils.ts b/packages/ui/src/components/session/sidebar/sessions/sessionNodeItemUtils.ts index f2c40e5c..631faad3 100644 --- a/packages/ui/src/components/session/sidebar/sessions/sessionNodeItemUtils.ts +++ b/packages/ui/src/components/session/sidebar/sessions/sessionNodeItemUtils.ts @@ -78,6 +78,16 @@ export type QuestionBadgeSessionScope = { sessionIDs: string[]; }; +export const getSessionWorktreeMenuDisabled = ({ + sessionDirectory, + isStreaming, + isMovingToWorktree, +}: { + sessionDirectory: string | null; + isStreaming: boolean; + isMovingToWorktree: boolean; +}): boolean => !sessionDirectory || isStreaming || isMovingToWorktree; + /** * Choose which (directory, sessionIDs) scopes a sidebar row's pending-question * badge should count. An expanded row counts only its own session; a collapsed diff --git a/packages/ui/src/lib/worktrees/sessionWorktreeMove.test.ts b/packages/ui/src/lib/worktrees/sessionWorktreeMove.test.ts index 14abe32e..a3f82e3e 100644 --- a/packages/ui/src/lib/worktrees/sessionWorktreeMove.test.ts +++ b/packages/ui/src/lib/worktrees/sessionWorktreeMove.test.ts @@ -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(); const storedMetadata = new Map(); const originalConsoleWarn = console.warn; +type SessionUIState = { + availableWorktrees: WorktreeMetadata[]; + availableWorktreesByProject: Map; + worktreeMetadata: Map; + getWorktreeMetadata: (sessionId: string) => WorktreeMetadata | null; + setWorktreeMetadata: (sessionId: string, metadata: WorktreeMetadata | null) => void; +}; + +type SessionUIStatePatch = Partial | ((state: SessionUIState) => Partial); + +const sessionUIState: SessionUIState = { + availableWorktrees: [], + availableWorktreesByProject: new Map(), + worktreeMetadata: new Map(), + 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(), - 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; + 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): Promise => { + 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([]); }); diff --git a/packages/ui/src/lib/worktrees/sessionWorktreeMove.ts b/packages/ui/src/lib/worktrees/sessionWorktreeMove.ts index e0b679b1..0aa5499c 100644 --- a/packages/ui/src/lib/worktrees/sessionWorktreeMove.ts +++ b/packages/ui/src/lib/worktrees/sessionWorktreeMove.ts @@ -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, -): Promise => { - const failures: unknown[] = []; +): Promise => { + 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); diff --git a/packages/ui/src/lib/worktrees/worktreeManager.test.ts b/packages/ui/src/lib/worktrees/worktreeManager.test.ts index eeed16c9..6a8698cc 100644 --- a/packages/ui/src/lib/worktrees/worktreeManager.test.ts +++ b/packages/ui/src/lib/worktrees/worktreeManager.test.ts @@ -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) | 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((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 void>(); + let recoveryReadsAllowed = false; + + listImplementation = () => { + const callNumber = listCalls.length; + if (callNumber === 8 && !recoveryReadsAllowed) { + return Promise.reject(new Error('unexpected extra read')); + } + return new Promise((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', diff --git a/packages/ui/src/lib/worktrees/worktreeManager.ts b/packages/ui/src/lib/worktrees/worktreeManager.ts index 50016065..67cf5235 100644 --- a/packages/ui/src/lib/worktrees/worktreeManager.ts +++ b/packages/ui/src/lib/worktrees/worktreeManager.ts @@ -377,6 +377,7 @@ const _worktreeListCache = new Map }>(); const _worktreeListGeneration = new Map(); 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 => { - 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 { 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;