From 8a6eca559789cd51ffa82d8b82a2cde6facddbb6 Mon Sep 17 00:00:00 2001 From: Serhii Dziupin Date: Thu, 13 Aug 2026 10:51:40 +0300 Subject: [PATCH] fix(sessions): select the active project using session ownership (#2865) * fix(sessions): select the active project using session ownership Keep a same-project worktree session while its rendered map is stale, but switch to the remembered or fallback session when the current session is known to belong to another project. Fixes #2317 * test(sessions): pin project-switch ownership recovery in the selection hook --- .../src/components/session/SessionSidebar.tsx | 1 + .../hooks/useProjectSessionSelection.test.ts | 409 +++++++++++++++--- .../hooks/useProjectSessionSelection.ts | 114 ++++- 3 files changed, 444 insertions(+), 80 deletions(-) diff --git a/packages/ui/src/components/session/SessionSidebar.tsx b/packages/ui/src/components/session/SessionSidebar.tsx index f7ce5a3b..b42dc1d6 100644 --- a/packages/ui/src/components/session/SessionSidebar.tsx +++ b/packages/ui/src/components/session/SessionSidebar.tsx @@ -1825,6 +1825,7 @@ const SessionSidebarComponent: React.FC = ({ openNewSessionDraft={openNewSessionDraft} setActiveMainTab={setActiveMainTab} setSessionSwitcherOpen={setSessionSwitcherOpen} + sessionOwnerBySessionId={sessionOwnership.bySessionId} /> ({ + useUIStore: Object.assign( + (selector: (state: { isNewWorktreeDialogOpen: boolean }) => unknown) => + selector({ isNewWorktreeDialogOpen }), + { getState: () => ({ isNewWorktreeDialogOpen }) }, + ), +})); + +mock.module('@/sync/session-ui-store', () => ({ + useSessionUIStore: (selector: (state: { + currentSessionId: string | null; + newSessionDraft: { open: boolean }; + }) => unknown) => selector({ + currentSessionId, + newSessionDraft: { open: newSessionDraftOpen }, + }), +})); + +const { + resolveMissingProjectSessionSelection, + ProjectSessionSelectionEffect, +} = await import('./useProjectSessionSelection'); + // --------------------------------------------------------------------------- // Helper: simulate the projectSessionMeta computation from the hook -// (same visitNodes logic as useProjectSessionSelection.ts lines 46-71) +// (same visitNodes logic as useProjectSessionSelection.ts) // --------------------------------------------------------------------------- type ProjectSection = { @@ -182,24 +211,6 @@ describe('useProjectSessionSelection — worktree session click race', () => { expect(projectMap?.has('wt-session-1')).toBe(true); }); - test('guard preserves currentSessionId when projectMap is stale (the bug fix)', () => { - const { metaByProject, firstSessionByProject } = computeProjectMeta(staleSections); - const projectMap = metaByProject.get('project-1')!; - const currentSessionId = 'wt-session-1'; - - // Path A fails: currentSessionId is set but not in stale projectMap - const pathAHit = Boolean(currentSessionId && projectMap?.has(currentSessionId)); - expect(pathAHit).toBe(false); - - // Guard: if (currentSessionId) return; - // This is what prevents the fallthrough to Path C (auto-select wrong session) - // Without the guard, Path C would select firstSessionByProject = root-session-1 - // instead of preserving the user's wt-session-1 selection - const fallback = firstSessionByProject.get('project-1')?.id ?? null; - expect(fallback).toBe('root-session-1'); - expect(fallback).not.toBe(currentSessionId); - }); - test('second click works correctly when projectSections is updated', () => { const { metaByProject } = computeProjectMeta(updatedSections); const projectMap = metaByProject.get('project-1')!; @@ -210,53 +221,327 @@ describe('useProjectSessionSelection — worktree session click race', () => { expect(pathAHit).toBe(true); }); - test('project switch: guard does NOT fire when currentSessionId matches new project', () => { - // Simulates: user clicks a session in project-2 (normal click, not worktree) + test('project switch: Path A succeeds when currentSessionId matches the new project', () => { const { metaByProject } = computeProjectMeta(project2Sections); const projectMap = metaByProject.get('project-2')!; const currentSessionId = 'project-2-session-1'; - // Path A succeeds — the session is in the new project's projectMap const pathAHit = Boolean(currentSessionId && projectMap?.has(currentSessionId)); expect(pathAHit).toBe(true); - - // Guard condition only fires when Path A fails — should not fire here - const guardWouldFire = Boolean(currentSessionId && !(projectMap?.has(currentSessionId))); - expect(guardWouldFire).toBe(false); - }); - - test('guard does NOT fire when currentSessionId is null (deleted/archived session)', () => { - const { metaByProject } = computeProjectMeta(staleSections); - const projectMap = metaByProject.get('project-1')!; - const currentSessionId = null; - - // Path A: currentSessionId is null → skipped - const pathAHit = Boolean(currentSessionId && projectMap?.has(currentSessionId)); - expect(pathAHit).toBe(false); - - // Guard: currentSessionId is null → skipped, falls through to Path B/C - const guardWouldFire = currentSessionId !== null && !pathAHit; - expect(guardWouldFire).toBe(false); - }); - - test('guard does NOT fire for empty projects — falls through to Path B (open draft)', () => { - // Empty project: no groups/sessions in projectSections - const emptySections: ProjectSection[] = [ - { - project: { id: 'empty-project', normalizedPath: '/workspace/empty' }, - groups: [], - }, - ]; - const { metaByProject } = computeProjectMeta(emptySections); - const projectMap = metaByProject.get('empty-project'); - const currentSessionId = 'some-session-id'; - - // projectMap is undefined for empty project - expect(projectMap).toBe(undefined); - - // Guard: projectMap is undefined → skipped, falls through to Path B - // which opens a new session draft for the empty project - const guardWouldFire = Boolean(currentSessionId && projectMap); - expect(guardWouldFire).toBe(false); + }); +}); + +describe('resolveMissingProjectSessionSelection', () => { + test('A → B selects B remembered session when the current session is owned by A', () => { + const projectBMap = new Map([ + ['project-b-first-session', null], + ['project-b-remembered-session', null], + ]); + + expect(resolveMissingProjectSessionSelection({ + activeProjectId: 'project-b', + currentSessionId: 'stale-worktree-session-a', + currentSessionOwnerProjectId: 'project-a', + projectMap: projectBMap, + metaByProject: new Map([['project-b', projectBMap]]), + rememberedSessionId: 'project-b-remembered-session', + fallbackSessionId: 'project-b-first-session', + })).toEqual({ kind: 'select-session', sessionId: 'project-b-remembered-session' }); + }); + + test('A → B falls back to B first session when none is remembered', () => { + const projectAMap = new Map([['project-a-session', null]]); + const projectBMap = new Map([['project-b-first-session', null]]); + const metaByProject = new Map([ + ['project-a', projectAMap], + ['project-b', projectBMap], + ]); + + expect(resolveMissingProjectSessionSelection({ + activeProjectId: 'project-b', + currentSessionId: 'project-a-session', + currentSessionOwnerProjectId: 'project-a', + projectMap: projectBMap, + metaByProject, + rememberedSessionId: undefined, + fallbackSessionId: 'project-b-first-session', + })).toEqual({ kind: 'select-session', sessionId: 'project-b-first-session' }); + }); + + test('A → B opens a B-scoped draft when B is empty', () => { + expect(resolveMissingProjectSessionSelection({ + activeProjectId: 'project-b', + currentSessionId: 'project-a-session', + currentSessionOwnerProjectId: 'project-a', + projectMap: undefined, + metaByProject: new Map([['project-a', new Map([['project-a-session', null]])]]), + rememberedSessionId: undefined, + fallbackSessionId: null, + })).toEqual({ kind: 'open-draft' }); + }); + + test('preserves a same-project worktree session missing from a stale projectMap', () => { + const projectMap = new Map([['root-session-1', null]]); + const metaByProject = new Map([['project-1', projectMap]]); + + expect(resolveMissingProjectSessionSelection({ + activeProjectId: 'project-1', + currentSessionId: 'wt-session-1', + currentSessionOwnerProjectId: 'project-1', + projectMap, + metaByProject, + rememberedSessionId: undefined, + fallbackSessionId: 'root-session-1', + })).toEqual({ kind: 'preserve-current' }); + }); + + test('preserves an unknown session while worktree metadata may still be loading', () => { + const projectMap = new Map([['root-session-1', null]]); + const metaByProject = new Map([['project-1', projectMap]]); + + expect(resolveMissingProjectSessionSelection({ + activeProjectId: 'project-1', + currentSessionId: 'wt-session-1', + currentSessionOwnerProjectId: null, + projectMap, + metaByProject, + rememberedSessionId: undefined, + fallbackSessionId: 'root-session-1', + })).toEqual({ kind: 'preserve-current' }); + }); + + test('unknown ownership still switches when the session already appears under another project', () => { + const projectAMap = new Map([['project-a-session', null]]); + const projectBMap = new Map([ + ['project-b-first-session', null], + ['project-b-remembered-session', null], + ]); + const metaByProject = new Map([ + ['project-a', projectAMap], + ['project-b', projectBMap], + ]); + + expect(resolveMissingProjectSessionSelection({ + activeProjectId: 'project-b', + currentSessionId: 'project-a-session', + currentSessionOwnerProjectId: null, + projectMap: projectBMap, + metaByProject, + rememberedSessionId: 'project-b-remembered-session', + fallbackSessionId: 'project-b-first-session', + })).toEqual({ kind: 'select-session', sessionId: 'project-b-remembered-session' }); + }); + + test('deleted or missing currentSessionId falls through to remembered/fallback selection', () => { + const projectMap = new Map([['root-session-1', null]]); + + expect(resolveMissingProjectSessionSelection({ + activeProjectId: 'project-1', + currentSessionId: null, + currentSessionOwnerProjectId: null, + projectMap, + metaByProject: new Map([['project-1', projectMap]]), + rememberedSessionId: undefined, + fallbackSessionId: 'root-session-1', + })).toEqual({ kind: 'select-session', sessionId: 'root-session-1' }); + }); + + test('empty projects resolve to opening a draft', () => { + expect(resolveMissingProjectSessionSelection({ + activeProjectId: 'empty-project', + currentSessionId: 'some-session-id', + currentSessionOwnerProjectId: null, + projectMap: undefined, + metaByProject: new Map>(), + rememberedSessionId: undefined, + fallbackSessionId: null, + })).toEqual({ kind: 'open-draft' }); + }); +}); + +// --------------------------------------------------------------------------- +// Hook-level: ProjectSessionSelectionEffect recovery / preserve +// --------------------------------------------------------------------------- + +const installMinimalDom = () => { + const descriptors = new Map(); + const setGlobal = (name: string, value: unknown) => { + descriptors.set(name, Object.getOwnPropertyDescriptor(globalThis, name)); + Object.defineProperty(globalThis, name, { configurable: true, writable: true, value }); + }; + class ElementStub {} + const documentStub: Record = { + nodeType: 9, + defaultView: globalThis, + activeElement: null, + addEventListener: () => undefined, + removeEventListener: () => undefined, + }; + const container = { + nodeType: 1, + tagName: 'DIV', + nodeName: 'DIV', + namespaceURI: 'http://www.w3.org/1999/xhtml', + ownerDocument: documentStub, + addEventListener: () => undefined, + removeEventListener: () => undefined, + }; + documentStub.documentElement = container; + documentStub.body = container; + setGlobal('document', documentStub); + setGlobal('window', globalThis); + setGlobal('location', { search: '', protocol: 'http:', hostname: 'localhost' }); + setGlobal('Element', ElementStub); + setGlobal('HTMLElement', ElementStub); + setGlobal('HTMLIFrameElement', ElementStub); + setGlobal('IS_REACT_ACT_ENVIRONMENT', true); + setGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => setTimeout(() => callback(Date.now()), 0)); + setGlobal('cancelAnimationFrame', (id: ReturnType) => clearTimeout(id)); + return { + container: container as unknown as Element, + restore: () => { + for (const [name, descriptor] of descriptors) { + if (descriptor) Object.defineProperty(globalThis, name, descriptor); + else Reflect.deleteProperty(globalThis, name); + } + }, + }; +}; + +type SelectionEffectProps = React.ComponentProps; + +const bothProjectSections: ProjectSection[] = [staleSections[0]!, project2Sections[0]!]; + +function mountSelectionEffect(initial: { + activeProjectId: string; + projectSections: ProjectSection[]; + sessionId: string | null; + sessionOwnerBySessionId?: ReadonlyMap; + rememberedByProject?: Map; +}) { + currentSessionId = initial.sessionId; + newSessionDraftOpen = false; + isNewWorktreeDialogOpen = false; + + const sessionSelectCalls: Array<[string, string | null]> = []; + const draftCalls: Array<{ selectedProjectId?: string | null; directoryOverride?: string | null } | undefined> = []; + const dom = installMinimalDom(); + const root: Root = createRoot(dom.container); + + const props: SelectionEffectProps = { + projectSections: initial.projectSections, + activeProjectId: initial.activeProjectId, + initialActiveSessionByProject: initial.rememberedByProject ?? new Map(), + persistActiveSessionByProject: () => undefined, + handleSessionSelect: (sessionId, sessionDirectory) => { + sessionSelectCalls.push([sessionId, sessionDirectory]); + }, + mobileVariant: false, + openNewSessionDraft: (options) => { + draftCalls.push(options); + }, + setActiveMainTab: () => undefined, + setSessionSwitcherOpen: () => undefined, + sessionOwnerBySessionId: initial.sessionOwnerBySessionId, + }; + + act(() => { + root.render(React.createElement(ProjectSessionSelectionEffect, props)); + }); + + return { + sessionSelectCalls, + draftCalls, + rerender: (next: Partial & { sessionId?: string | null }) => { + const { sessionId, ...effectProps } = next; + if (sessionId !== undefined) currentSessionId = sessionId; + Object.assign(props, effectProps); + act(() => { + root.render(React.createElement(ProjectSessionSelectionEffect, props)); + }); + }, + teardown: () => { + act(() => { + root.unmount(); + }); + dom.restore(); + }, + }; +} + +describe('ProjectSessionSelectionEffect — ownership recovery', () => { + let teardown: (() => void) | null = null; + + afterEach(() => { + teardown?.(); + teardown = null; + currentSessionId = null; + newSessionDraftOpen = false; + isNewWorktreeDialogOpen = false; + }); + + test('A → B with later foreign ownership selects B remembered session', () => { + const missingASessionId = 'session-a-missing-from-maps'; + const mounted = mountSelectionEffect({ + activeProjectId: 'project-1', + projectSections: bothProjectSections, + sessionId: missingASessionId, + sessionOwnerBySessionId: new Map([[missingASessionId, { projectId: 'project-1' }]]), + rememberedByProject: new Map([['project-2', 'project-2-session-2']]), + }); + teardown = mounted.teardown; + + expect(mounted.sessionSelectCalls).toEqual([]); + + mounted.rerender({ + activeProjectId: 'project-2', + sessionOwnerBySessionId: new Map(), + }); + expect(mounted.sessionSelectCalls).toEqual([]); + + mounted.rerender({ + sessionOwnerBySessionId: new Map([[missingASessionId, { projectId: 'project-1' }]]), + }); + expect(mounted.sessionSelectCalls).toEqual([ + ['project-2-session-2', '/workspace/project-2'], + ]); + }); + + test('A → B with known foreign ownership selects B remembered session', () => { + const mounted = mountSelectionEffect({ + activeProjectId: 'project-1', + projectSections: bothProjectSections, + sessionId: 'root-session-1', + sessionOwnerBySessionId: new Map([['root-session-1', { projectId: 'project-1' }]]), + rememberedByProject: new Map([['project-2', 'project-2-session-2']]), + }); + teardown = mounted.teardown; + + expect(mounted.sessionSelectCalls).toEqual([]); + + mounted.rerender({ activeProjectId: 'project-2' }); + expect(mounted.sessionSelectCalls).toEqual([ + ['project-2-session-2', '/workspace/project-2'], + ]); + }); + + test('stale same-project worktree selection stays put when ownership arrives', () => { + const mounted = mountSelectionEffect({ + activeProjectId: 'project-1', + projectSections: staleSections, + sessionId: 'wt-session-1', + sessionOwnerBySessionId: new Map(), + rememberedByProject: new Map([['project-1', 'root-session-1']]), + }); + teardown = mounted.teardown; + + expect(mounted.sessionSelectCalls).toEqual([]); + expect(mounted.draftCalls).toEqual([]); + + mounted.rerender({ + sessionOwnerBySessionId: new Map([['wt-session-1', { projectId: 'project-1' }]]), + }); + expect(mounted.sessionSelectCalls).toEqual([]); + expect(mounted.draftCalls).toEqual([]); }); }); diff --git a/packages/ui/src/components/session/sidebar/hooks/useProjectSessionSelection.ts b/packages/ui/src/components/session/sidebar/hooks/useProjectSessionSelection.ts index 9d099dc1..7c540c7d 100644 --- a/packages/ui/src/components/session/sidebar/hooks/useProjectSessionSelection.ts +++ b/packages/ui/src/components/session/sidebar/hooks/useProjectSessionSelection.ts @@ -17,6 +17,7 @@ type Args = { activeSessionByProject: Map; setActiveSessionByProject: React.Dispatch>>; currentSessionId: string | null; + currentSessionOwnerProjectId?: string | null; handleSessionSelect: (sessionId: string, sessionDirectory: string | null) => void; newSessionDraftOpen: boolean; mobileVariant: boolean; @@ -25,6 +26,69 @@ type Args = { setSessionSwitcherOpen: (open: boolean) => void; }; +export type MissingProjectSessionSelection = + | { kind: 'preserve-current' } + | { kind: 'open-draft' } + | { kind: 'select-session'; sessionId: string } + | { kind: 'none' }; + +/** + * Resolves the active-project action after its rendered session map does not + * contain the current session. + * + * Authoritative ownership wins. If ownership is still unknown, a session that + * already appears under another project's rendered map is treated as foreign, + * while a session missing from every rendered map is preserved so stale + * worktree metadata can catch up. + */ +export function resolveMissingProjectSessionSelection({ + activeProjectId, + currentSessionId, + currentSessionOwnerProjectId, + projectMap, + metaByProject, + rememberedSessionId, + fallbackSessionId, +}: { + activeProjectId: string; + currentSessionId: string | null; + currentSessionOwnerProjectId?: string | null; + projectMap: ReadonlyMap | undefined; + metaByProject: ReadonlyMap>; + rememberedSessionId: string | undefined; + fallbackSessionId: string | null; +}): MissingProjectSessionSelection { + if (currentSessionId && currentSessionOwnerProjectId === activeProjectId) { + return { kind: 'preserve-current' }; + } + + if (currentSessionOwnerProjectId == null) { + const currentSessionBelongsToAnotherProject = Boolean( + currentSessionId + && Array.from(metaByProject.entries()).some( + ([projectId, sessions]) => projectId !== activeProjectId && sessions.has(currentSessionId), + ), + ); + if (currentSessionId && projectMap && !currentSessionBelongsToAnotherProject) { + return { kind: 'preserve-current' }; + } + } + + if (!projectMap || projectMap.size === 0) { + return { kind: 'open-draft' }; + } + + const remembered = rememberedSessionId && projectMap.has(rememberedSessionId) + ? rememberedSessionId + : null; + const targetSessionId = remembered ?? fallbackSessionId; + if (!targetSessionId || targetSessionId === currentSessionId) { + return { kind: 'none' }; + } + + return { kind: 'select-session', sessionId: targetSessionId }; +} + export const useProjectSessionSelection = (args: Args): void => { const { projectSections, @@ -32,6 +96,7 @@ export const useProjectSessionSelection = (args: Args): void => { activeSessionByProject, setActiveSessionByProject, currentSessionId, + currentSessionOwnerProjectId, handleSessionSelect, newSessionDraftOpen, mobileVariant, @@ -103,10 +168,10 @@ export const useProjectSessionSelection = (args: Args): void => { if (!section) { return; } - previousActiveProjectRef.current = activeProjectId; const projectMap = projectSessionMeta.metaByProject.get(activeProjectId); if (currentSessionId && projectMap && projectMap.has(currentSessionId)) { + previousActiveProjectRef.current = activeProjectId; setActiveSessionByProject((prev) => { if (prev.get(activeProjectId) === currentSessionId) { return prev; @@ -118,16 +183,28 @@ export const useProjectSessionSelection = (args: Args): void => { return; } - // Path A' — currentSessionId is set but not in stale projectMap. - // Preserve user's explicit selection when the projectMap exists but - // is missing the session (worktree data not yet loaded). For - // empty projects (projectMap is undefined), fall through to Path B - // so a new session draft is opened. - if (currentSessionId && projectMap) { + const selection = resolveMissingProjectSessionSelection({ + activeProjectId, + currentSessionId, + currentSessionOwnerProjectId, + projectMap, + metaByProject: projectSessionMeta.metaByProject, + rememberedSessionId: activeSessionByProject.get(activeProjectId), + fallbackSessionId: projectSessionMeta.firstSessionByProject.get(activeProjectId)?.id ?? null, + }); + + // Keep the project unprocessed while ownership/maps may still catch up, + // so a later owner of another project can still select B. + if (selection.kind === 'preserve-current') { + if (currentSessionOwnerProjectId === activeProjectId) { + previousActiveProjectRef.current = activeProjectId; + } return; } - if (!projectMap || projectMap.size === 0) { + previousActiveProjectRef.current = activeProjectId; + + if (selection.kind === 'open-draft') { setActiveMainTab('chat'); if (mobileVariant) { setSessionSwitcherOpen(false); @@ -139,21 +216,16 @@ export const useProjectSessionSelection = (args: Args): void => { return; } - const rememberedSessionId = activeSessionByProject.get(activeProjectId); - const remembered = rememberedSessionId && projectMap.has(rememberedSessionId) - ? rememberedSessionId - : null; - const fallback = projectSessionMeta.firstSessionByProject.get(activeProjectId)?.id ?? null; - const targetSessionId = remembered ?? fallback; - if (!targetSessionId || targetSessionId === currentSessionId) { + if (selection.kind !== 'select-session') { return; } - const targetDirectory = projectMap.get(targetSessionId)?.directory ?? null; - handleSessionSelect(targetSessionId, targetDirectory); + const targetDirectory = projectMap?.get(selection.sessionId)?.directory ?? null; + handleSessionSelect(selection.sessionId, targetDirectory); }, [ activeProjectId, activeSessionByProject, currentSessionId, + currentSessionOwnerProjectId, handleSessionSelect, newSessionDraftOpen, mobileVariant, @@ -187,19 +259,24 @@ export const useProjectSessionSelection = (args: Args): void => { type ProjectSessionSelectionEffectProps = Omit< Args, - 'activeSessionByProject' | 'setActiveSessionByProject' | 'currentSessionId' | 'newSessionDraftOpen' + 'activeSessionByProject' | 'setActiveSessionByProject' | 'currentSessionId' | 'newSessionDraftOpen' | 'currentSessionOwnerProjectId' > & { initialActiveSessionByProject: Map; persistActiveSessionByProject: (value: Map) => void; + sessionOwnerBySessionId?: ReadonlyMap; }; export const ProjectSessionSelectionEffect: React.FC = ({ initialActiveSessionByProject, persistActiveSessionByProject, + sessionOwnerBySessionId, ...args }) => { const currentSessionId = useSessionUIStore((state) => state.currentSessionId); const newSessionDraftOpen = useSessionUIStore((state) => Boolean(state.newSessionDraft?.open)); + const currentSessionOwnerProjectId = currentSessionId + ? sessionOwnerBySessionId?.get(currentSessionId)?.projectId ?? null + : null; const [activeSessionByProject, setActiveSessionByProject] = React.useState( () => new Map(initialActiveSessionByProject), ); @@ -208,6 +285,7 @@ export const ProjectSessionSelectionEffect: React.FC {