diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 561dd4e8c40..4189b0e44a1 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -64,7 +64,7 @@ import { import TabGroupSplitLayout from './tab-group/TabGroupSplitLayout' import AiVaultSessionDropLayer from './tab-group/AiVaultSessionDropLayer' import { shouldAutoCreateInitialTerminal } from './terminal/initial-terminal' -import { resolveRepairedActiveTerminalTabId } from './terminal/active-terminal-repair' +import { useActiveTerminalRepair } from './terminal/use-active-terminal-repair' import { scheduleBackgroundTerminalWorktreeMeasure } from './terminal/background-terminal-worktree-visibility' import { applyBackgroundMountTabRestriction, @@ -390,7 +390,10 @@ function Terminal(): React.JSX.Element | null { }, [foregroundTerminalTabIds]) const tabs = useMemo( - () => (renderedActiveWorktreeId ? (tabsByWorktree[renderedActiveWorktreeId] ?? []) : []), + () => + renderedActiveWorktreeId !== null && Object.hasOwn(tabsByWorktree, renderedActiveWorktreeId) + ? tabsByWorktree[renderedActiveWorktreeId] + : [], [renderedActiveWorktreeId, tabsByWorktree] ) useTerminalProviderSnapshotCapability(workspaceSessionReady && hydrationSucceeded) @@ -749,32 +752,15 @@ function Terminal(): React.JSX.Element | null { ) }, [queueEditorCloseRequests]) - useEffect(() => { - const rememberedTabId = renderedActiveWorktreeId - ? (activeTabIdByWorktree[renderedActiveWorktreeId] ?? null) - : null - // Why: prefer the remembered active tab so a repair on a transient switch render doesn't reset selection to Terminal 1. - const repairedTabId = resolveRepairedActiveTerminalTabId({ - activeTabType, - activeTabId, - rememberedTabId, - tabs - }) - if (!repairedTabId) { - return - } - // Why: run in an effect (Zustand mutation during render trips React's cross-component update warning); keep terminal-only so inactive CLI-created tabs can't steal editor/browser focus. - setActiveTab(repairedTabId) - // Why: `tabs` is the dependency so the repair reacts to tab-order/content changes, not just scalar IDs. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ + // Why: repair after render so Zustand mutation cannot trip React's cross-component update warning. + useActiveTerminalRepair({ activeTabId, activeTabType, setActiveTab, tabs, activeTabIdByWorktree, renderedActiveWorktreeId - ]) + }) // Why: only mount TerminalPanes for visited worktrees, else restoring many saved tabs mass-spawns PTYs. const measurableBackgroundWorktreeTimersRef = useRef(new Map()) diff --git a/src/renderer/src/components/terminal/active-terminal-repair-loop.react185.test.tsx b/src/renderer/src/components/terminal/active-terminal-repair-loop.react185.test.tsx index 573ca6111e6..ab3ff3ac45e 100644 --- a/src/renderer/src/components/terminal/active-terminal-repair-loop.react185.test.tsx +++ b/src/renderer/src/components/terminal/active-terminal-repair-loop.react185.test.tsx @@ -1,10 +1,10 @@ /** @vitest-environment happy-dom */ -import { act, useEffect, useMemo } from 'react' +import { act, useMemo } from 'react' import { createRoot } from 'react-dom/client' import { afterEach, describe, expect, it } from 'vitest' import { useAppStore } from '@/store' import type { Tab, TabGroup, TerminalTab } from '../../../../shared/types' -import { resolveRepairedActiveTerminalTabId } from './active-terminal-repair' +import { useActiveTerminalRepair } from './use-active-terminal-repair' ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true @@ -44,8 +44,7 @@ function tabGroup( return { id, worktreeId, activeTabId, tabOrder, recentTabIds: [activeTabId] } } -/** Mirrors the active-terminal repair effect in Terminal.tsx (same deps, same store call). */ -function RepairEffectHarness({ onPass }: { onPass: () => boolean }): null { +function RepairEffectHarness(): null { const activeTabId = useAppStore((s) => s.activeTabId) const activeTabIdByWorktree = useAppStore((s) => s.activeTabIdByWorktree) const activeTabType = useAppStore((s) => s.activeTabType) @@ -53,36 +52,21 @@ function RepairEffectHarness({ onPass }: { onPass: () => boolean }): null { const renderedActiveWorktreeId = useAppStore((s) => s.activeWorktreeId) const setActiveTab = useAppStore((s) => s.setActiveTab) const tabs = useMemo( - () => (renderedActiveWorktreeId ? (tabsByWorktree[renderedActiveWorktreeId] ?? []) : []), + () => + renderedActiveWorktreeId !== null && Object.hasOwn(tabsByWorktree, renderedActiveWorktreeId) + ? tabsByWorktree[renderedActiveWorktreeId] + : [], [renderedActiveWorktreeId, tabsByWorktree] ) - useEffect(() => { - if (!onPass()) { - return - } - const rememberedTabId = renderedActiveWorktreeId - ? (activeTabIdByWorktree[renderedActiveWorktreeId] ?? null) - : null - const repairedTabId = resolveRepairedActiveTerminalTabId({ - activeTabType, - activeTabId, - rememberedTabId, - tabs - }) - if (!repairedTabId) { - return - } - setActiveTab(repairedTabId) - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ + useActiveTerminalRepair({ activeTabId, activeTabType, setActiveTab, tabs, activeTabIdByWorktree, renderedActiveWorktreeId - ]) + }) return null } @@ -95,15 +79,25 @@ afterEach(() => { function measureRepairPasses(): number { let passes = 0 + const setActiveTab = useAppStore.getState().setActiveTab + useAppStore.setState({ + setActiveTab: (tabId) => { + passes += 1 + if (passes <= MAX_PASSES) { + setActiveTab(tabId) + } + } + }) const container = document.createElement('div') document.body.appendChild(container) const root = createRoot(container) cleanup = () => { act(() => root.unmount()) + useAppStore.setState({ setActiveTab }) container.remove() } act(() => { - root.render( (passes += 1) <= MAX_PASSES} />) + root.render() }) return passes } @@ -123,9 +117,8 @@ describe('active-terminal repair effect cannot drive a React #185 update loop', }) it('settles when another worktree reuses the tab id and is scanned first', () => { - // Why: setActiveTab attributes a tab to the first worktree whose array holds - // the id, so a reused id makes it skip the activeTabId write while still - // reallocating activeTabIdByWorktree — the repair effect's own dependency. + // Why regression: first-match ownership skipped activeTabId while reallocating + // activeTabIdByWorktree, retriggering the repair effect indefinitely. useAppStore.setState({ activeWorktreeId: 'wt-active', activeTabType: 'terminal', @@ -145,8 +138,14 @@ describe('active-terminal repair effect cannot drive a React #185 update loop', }) it('activates the active worktree unified tab when another worktree reuses the entity id', () => { - const otherTab = unifiedTerminalTab('u-other', 't1', 'wt-other', 'g-other') - const activeTab = unifiedTerminalTab('u-active', 't1', 'wt-active', 'g-active') + const otherTab = unifiedTerminalTab('t1', 't1', 'wt-other', 'g-other') + const otherPreviousTab = unifiedTerminalTab( + 'other-previous', + 'other-previous', + 'wt-other', + 'g-other' + ) + const activeTab = unifiedTerminalTab('t1', 't1', 'wt-active', 'g-active') const previousActiveTab = unifiedTerminalTab('u-previous', 't2', 'wt-active', 'g-active') useAppStore.setState({ activeWorktreeId: 'wt-active', @@ -157,11 +156,13 @@ describe('active-terminal repair effect cannot drive a React #185 update loop', 'wt-active': [terminalTab('t1', 'wt-active'), terminalTab('t2', 'wt-active')] }, unifiedTabsByWorktree: { - 'wt-other': [otherTab], + 'wt-other': [otherTab, otherPreviousTab], 'wt-active': [activeTab, previousActiveTab] }, groupsByWorktree: { - 'wt-other': [tabGroup('g-other', 'wt-other', otherTab.id, [otherTab.id])], + 'wt-other': [ + tabGroup('g-other', 'wt-other', otherPreviousTab.id, [otherTab.id, otherPreviousTab.id]) + ], 'wt-active': [ tabGroup('g-active', 'wt-active', previousActiveTab.id, [ activeTab.id, @@ -177,7 +178,9 @@ describe('active-terminal repair effect cannot drive a React #185 update loop', }) expect(useAppStore.getState().groupsByWorktree['wt-active'][0].activeTabId).toBe(activeTab.id) - expect(useAppStore.getState().groupsByWorktree['wt-other'][0].activeTabId).toBe(otherTab.id) + expect(useAppStore.getState().groupsByWorktree['wt-other'][0].activeTabId).toBe( + otherPreviousTab.id + ) }) it('keeps unified-only terminal activation as a fallback', () => { @@ -260,6 +263,51 @@ describe('active-terminal repair effect cannot drive a React #185 update loop', expect(useAppStore.getState().activeTabIdByWorktree['']).toBe('t1') }) + it('repairs a falsy-but-valid active worktree id through the production hook', () => { + useAppStore.setState({ + activeWorktreeId: '', + activeTabType: 'terminal', + activeTabId: 'stale-tab', + activeTabIdByWorktree: { '': 't1' }, + tabsByWorktree: { '': [terminalTab('t1', '')] }, + unifiedTabsByWorktree: {} + }) + expect(measureRepairPasses()).toBeLessThan(10) + expect(useAppStore.getState().activeTabId).toBe('t1') + }) + + it('does not read inherited unified tabs for a prototype-named owner', () => { + useAppStore.setState({ + activeWorktreeId: 'toString', + activeTabId: null, + activeTabIdByWorktree: {}, + tabsByWorktree: { toString: [terminalTab('t1', 'toString')] }, + unifiedTabsByWorktree: {} + }) + expect(() => useAppStore.getState().setActiveTab('t1')).not.toThrow() + expect(useAppStore.getState().activeTabId).toBe('t1') + }) + + it('activates own unified tabs for a prototype-named owner', () => { + const target = unifiedTerminalTab('t1', 't1', 'toString', 'g-target') + const previous = unifiedTerminalTab('t2', 't2', 'toString', 'g-target') + useAppStore.setState({ + activeWorktreeId: 'toString', + activeTabId: 't2', + activeTabIdByWorktree: { toString: 't2' }, + tabsByWorktree: { + toString: [terminalTab('t1', 'toString'), terminalTab('t2', 'toString')] + }, + unifiedTabsByWorktree: { toString: [target, previous] }, + groupsByWorktree: { + toString: [tabGroup('g-target', 'toString', previous.id, [target.id, previous.id])] + }, + activeGroupIdByWorktree: { toString: 'g-target' } + }) + act(() => useAppStore.getState().setActiveTab('t1')) + expect(useAppStore.getState().groupsByWorktree.toString[0].activeTabId).toBe(target.id) + }) + it('does not activate a tab with no owner when no worktree is active', () => { useAppStore.setState({ activeWorktreeId: null, diff --git a/src/renderer/src/components/terminal/use-active-terminal-repair.ts b/src/renderer/src/components/terminal/use-active-terminal-repair.ts new file mode 100644 index 00000000000..d495c80c330 --- /dev/null +++ b/src/renderer/src/components/terminal/use-active-terminal-repair.ts @@ -0,0 +1,61 @@ +import { useEffect } from 'react' +import type { TerminalTab, WorkspaceVisibleTabType } from '../../../../shared/types' +import { resolveRepairedActiveTerminalTabId } from './active-terminal-repair' + +type ActiveTerminalRepairInput = { + activeTabType: WorkspaceVisibleTabType + activeTabId: string | null + activeTabIdByWorktree: Record + renderedActiveWorktreeId: string | null + setActiveTab: (tabId: string) => void + tabs: TerminalTab[] +} + +export function repairActiveTerminalTab({ + activeTabType, + activeTabId, + activeTabIdByWorktree, + renderedActiveWorktreeId, + setActiveTab, + tabs +}: ActiveTerminalRepairInput): boolean { + const rememberedTabId = + renderedActiveWorktreeId !== null && + Object.hasOwn(activeTabIdByWorktree, renderedActiveWorktreeId) + ? (activeTabIdByWorktree[renderedActiveWorktreeId] ?? null) + : null + const repairedTabId = resolveRepairedActiveTerminalTabId({ + activeTabType, + activeTabId, + rememberedTabId, + tabs + }) + if (!repairedTabId) { + return false + } + setActiveTab(repairedTabId) + return true +} + +export function useActiveTerminalRepair(input: ActiveTerminalRepairInput): void { + const { + activeTabId, + activeTabIdByWorktree, + activeTabType, + renderedActiveWorktreeId, + setActiveTab, + tabs + } = input + useEffect(() => { + repairActiveTerminalTab(input) + // Why: `tabs` is the dependency so repair reacts to order/content changes, not just scalar ids. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + activeTabId, + activeTabType, + setActiveTab, + tabs, + activeTabIdByWorktree, + renderedActiveWorktreeId + ]) +} diff --git a/src/renderer/src/hooks/remote-workspace-snapshot-duplicate-tab-repair.test.ts b/src/renderer/src/hooks/remote-workspace-snapshot-duplicate-tab-repair.test.ts index 13ca9a54238..2664475db9e 100644 --- a/src/renderer/src/hooks/remote-workspace-snapshot-duplicate-tab-repair.test.ts +++ b/src/renderer/src/hooks/remote-workspace-snapshot-duplicate-tab-repair.test.ts @@ -7,11 +7,11 @@ import { describe, expect, it, vi } from 'vitest' import type * as AgentStatusModule from '@/lib/agent-status' import type { RemoteWorkspaceSnapshot } from '../../../shared/remote-workspace-types' -import type { DirectSshAuthority } from '../../../shared/ssh-types' +import type { DirectSshAuthority, SshProviderEpoch } from '../../../shared/ssh-types' import { createTestStore, makeWorktree } from '../store/slices/store-test-helpers' import { applyDirectSshRemoteWorkspaceSnapshot } from './remote-workspace-snapshot-apply' import type { DirectSshSnapshotApplyToken } from './direct-ssh-reconnect-coordinator-types' -import { resolveRepairedActiveTerminalTabId } from '../components/terminal/active-terminal-repair' +import { repairActiveTerminalTab } from '../components/terminal/use-active-terminal-repair' vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) vi.mock('@/lib/agent-status', async (importOriginal) => { @@ -29,7 +29,7 @@ const MAX_REPAIR_PASSES = 200 const authority: DirectSshAuthority = { targetId: TARGET_ID, - providerEpoch: { generation: 1, restartCount: 0 } as never, + providerEpoch: 'provider-epoch-1' as SshProviderEpoch, connectionGeneration: 1 } @@ -38,7 +38,7 @@ function token(snapshotRevision: number): DirectSshSnapshotApplyToken { authority, catalogRevision: 0, repoFingerprint: 'fp', - authorityRequirement: 'required' as never, + authorityRequirement: 'required', snapshotRevision, outcome: 'complete' } @@ -51,8 +51,10 @@ function snapshot( activeTabId: string | null ): RemoteWorkspaceSnapshot { return { + namespace: 'workspace', revision, updatedAt: revision, + schemaVersion: 1, session: { activeWorktreePath: worktreePath, activeTabId, @@ -75,7 +77,7 @@ function snapshot( lastVisitedAtByWorktreePath: { [worktreePath]: revision }, defaultTerminalTabsAppliedByWorktreePath: { [worktreePath]: true } } - } as unknown as RemoteWorkspaceSnapshot + } satisfies RemoteWorkspaceSnapshot } type TestStore = ReturnType @@ -129,20 +131,18 @@ function runRepairCycle(store: TestStore): { let depIdentityChurn = 0 for (; passes < MAX_REPAIR_PASSES; passes += 1) { const live = store.getState() - const activeWorktreeId = live.activeWorktreeId - const repairedTabId = resolveRepairedActiveTerminalTabId({ + const depsBefore = live.activeTabIdByWorktree + const repaired = repairActiveTerminalTab({ activeTabType: 'terminal', activeTabId: live.activeTabId, - rememberedTabId: activeWorktreeId - ? (live.activeTabIdByWorktree[activeWorktreeId] ?? null) - : null, - tabs: activeWorktreeId ? (live.tabsByWorktree[activeWorktreeId] ?? []) : [] + activeTabIdByWorktree: live.activeTabIdByWorktree, + renderedActiveWorktreeId: live.activeWorktreeId, + setActiveTab: live.setActiveTab, + tabs: live.activeWorktreeId ? (live.tabsByWorktree[live.activeWorktreeId] ?? []) : [] }) - if (!repairedTabId) { + if (!repaired) { return { converged: true, passes, depIdentityChurn } } - const depsBefore = live.activeTabIdByWorktree - live.setActiveTab(repairedTabId) if (store.getState().activeTabIdByWorktree !== depsBefore) { depIdentityChurn += 1 } @@ -209,6 +209,11 @@ describe('direct-SSH snapshot apply, tab id owned by two worktrees', () => { expect(repair.converged).toBe(true) expect(repair.passes).toBeLessThanOrEqual(store.getState().tabsByWorktree[NEW_ID].length) expect(store.getState().activeTabId).toBe('tab-1') + const activeGroupId = store.getState().activeGroupIdByWorktree[NEW_ID] + expect( + store.getState().groupsByWorktree[NEW_ID].find((group) => group.id === activeGroupId) + ?.activeTabId + ).toBe('tab-1') // The dep identity settles: re-running the effect body after convergence // reallocates nothing, so the effect does not schedule itself again. diff --git a/src/renderer/src/store/slices/active-tab-owner-worktree.test.ts b/src/renderer/src/store/slices/active-tab-owner-worktree.test.ts index a033cca5aeb..7e8c092a6c7 100644 --- a/src/renderer/src/store/slices/active-tab-owner-worktree.test.ts +++ b/src/renderer/src/store/slices/active-tab-owner-worktree.test.ts @@ -6,11 +6,8 @@ vi.mock('../../lib/crash-breadcrumb-recorder', () => ({ recordRendererCrashBreadcrumb: (...args: unknown[]) => recordRendererCrashBreadcrumb(...args) })) -const { - resolveActiveTabOwnerWorktreeId, - isTabInActiveWorktree, - _resetDuplicateTabOwnerBreadcrumbsForTests -} = await import('./active-tab-owner-worktree') +const { resolveActiveTabOwnerWorktreeId, _resetDuplicateTabOwnerBreadcrumbsForTests } = + await import('./active-tab-owner-worktree') function tab(id: string, worktreeId: string): TerminalTab { return { id, worktreeId, title: id, createdAt: 0, sortOrder: 0 } as unknown as TerminalTab @@ -126,39 +123,3 @@ describe('resolveActiveTabOwnerWorktreeId', () => { expect(recordRendererCrashBreadcrumb).toHaveBeenCalledTimes(256) }) }) - -describe('isTabInActiveWorktree', () => { - it('sees a duplicated tab id in the active worktree whichever key is scanned last', () => { - const maps = { 'wt-active': [tab('t1', 'wt-active')], 'wt-other': [tab('t1', 'wt-other')] } - expect(isTabInActiveWorktree(maps, 'wt-active', 't1')).toBe(true) - expect(isTabInActiveWorktree(maps, 'wt-other', 't1')).toBe(true) - expect(isTabInActiveWorktree(maps, 'wt-third', 't1')).toBe(false) - }) - - it('is false with no active worktree', () => { - expect(isTabInActiveWorktree({ 'wt-a': [tab('t1', 'wt-a')] }, null, 't1')).toBe(false) - }) - - // Why: a key lookup coerces null to the string 'null', so dropping the guard - // would let a worktree literally named 'null' answer for "no active worktree". - it('does not let a null active worktree match a worktree keyed null', () => { - expect(isTabInActiveWorktree({ null: [tab('t1', 'null')] }, null, 't1')).toBe(false) - }) - - // Why: a plain index would resolve the inherited member and `.some` would throw. - it('does not reach an inherited member for a prototype-named active worktree id', () => { - expect(isTabInActiveWorktree({ 'wt-a': [tab('t1', 'wt-a')] }, 'toString', 't1')).toBe(false) - }) - - it('reads an own key that shadows a prototype name', () => { - expect(isTabInActiveWorktree({ toString: [tab('t1', 'toString')] }, 'toString', 't1')).toBe( - true - ) - }) - - // Why: it must not silently answer "yes" for a falsy-but-present worktree id. - it('reads an empty-string active worktree id', () => { - expect(isTabInActiveWorktree({ '': [tab('t1', '')] }, '', 't1')).toBe(true) - expect(isTabInActiveWorktree({ '': [tab('t2', '')] }, '', 't1')).toBe(false) - }) -}) diff --git a/src/renderer/src/store/slices/active-tab-owner-worktree.ts b/src/renderer/src/store/slices/active-tab-owner-worktree.ts index 1d13a63ec9b..270c5d44f1c 100644 --- a/src/renderer/src/store/slices/active-tab-owner-worktree.ts +++ b/src/renderer/src/store/slices/active-tab-owner-worktree.ts @@ -80,29 +80,3 @@ export function resolveActiveTabOwnerWorktreeId( } return firstOwnerId } - -/** - * Whether the active worktree's own tab list holds this tab. - * - * Why this and not a cached owner id: the tab-owner cache in terminals.ts is - * last-writer-wins, so a duplicated tab id names whichever worktree it saw last - * — which can be a background one for a pane that really is in the active - * worktree. Callers that gate on "is this pane visible" need the active - * worktree's own answer, not a tie-break. - * - * Why not resolveActiveTabOwnerWorktreeId: its full scan runs per OSC title - * frame, and its breadcrumb would report a second caller under one verdict. - */ -export function isTabInActiveWorktree( - tabsByWorktree: Record, - activeWorktreeId: string | null, - tabId: string -): boolean { - // Why hasOwnProperty and not a plain index: ids like `toString` resolve an - // inherited member, and `.some` would then throw. Why not Object.keys: this - // runs per classified title frame, so it must not allocate a key array. - if (activeWorktreeId === null || !Object.hasOwn(tabsByWorktree, activeWorktreeId)) { - return false - } - return tabsByWorktree[activeWorktreeId].some((tab) => tab.id === tabId) -} diff --git a/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts b/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts index 0ce480558aa..b6c10e014ed 100644 --- a/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts +++ b/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts @@ -343,74 +343,4 @@ describe('runtimePaneTitle → sortEpoch', () => { store.getState().clearRuntimePaneTitle('tab-1', 1) expect(store.getState().sortEpoch).toBe(baseline) }) - - // Why: the tab-owner cache is last-writer-wins, so a tab id held under two - // worktrees names a background one for a pane that is in the active worktree - // — and every classified title frame then re-ranks the sidebar. - it('does not bump sortEpoch when a duplicated tab id is also in the active worktree', () => { - const store = createTestStore() - seedStore(store, { - worktreesByRepo: { - repo1: [ - makeWorktree({ id: 'wt-a', repoId: 'repo1', path: '/path/wt-a' }), - makeWorktree({ id: 'wt-b', repoId: 'repo1', path: '/path/wt-b' }) - ] - }, - tabsByWorktree: { - 'wt-a': [makeTab({ id: 'tab-1', worktreeId: 'wt-a' })], - 'wt-b': [makeTab({ id: 'tab-1', worktreeId: 'wt-b' })] - }, - activeWorktreeId: 'wt-a' - }) - const baseline = store.getState().sortEpoch - store.getState().setRuntimePaneTitle('tab-1', 1, '⠋ Claude') - store.getState().setRuntimePaneTitle('tab-1', 1, '✋ Gemini CLI') - expect(store.getState().sortEpoch).toBe(baseline) - }) - - it('does not bump sortEpoch clearing a duplicated tab id held by the active worktree', () => { - const store = createTestStore() - seedStore(store, { - worktreesByRepo: { - repo1: [ - makeWorktree({ id: 'wt-a', repoId: 'repo1', path: '/path/wt-a' }), - makeWorktree({ id: 'wt-b', repoId: 'repo1', path: '/path/wt-b' }) - ] - }, - tabsByWorktree: { - 'wt-a': [makeTab({ id: 'tab-1', worktreeId: 'wt-a' })], - 'wt-b': [makeTab({ id: 'tab-1', worktreeId: 'wt-b' })] - }, - activeWorktreeId: 'wt-b' - }) - store.getState().setRuntimePaneTitle('tab-1', 1, '✋ Gemini CLI') - store.setState({ activeWorktreeId: 'wt-a' }) - const baseline = store.getState().sortEpoch - store.getState().clearRuntimePaneTitle('tab-1', 1) - expect(store.getState().sortEpoch).toBe(baseline) - }) - - // Why: the duplicate must not flip the gate the other way either — a pane in - // no active worktree still has to re-rank. - it('still bumps sortEpoch when neither owner of a duplicated tab id is active', () => { - const store = createTestStore() - seedStore(store, { - worktreesByRepo: { - repo1: [ - makeWorktree({ id: 'wt-a', repoId: 'repo1', path: '/path/wt-a' }), - makeWorktree({ id: 'wt-b', repoId: 'repo1', path: '/path/wt-b' }), - makeWorktree({ id: 'wt-c', repoId: 'repo1', path: '/path/wt-c' }) - ] - }, - tabsByWorktree: { - 'wt-a': [makeTab({ id: 'tab-1', worktreeId: 'wt-a' })], - 'wt-b': [makeTab({ id: 'tab-1', worktreeId: 'wt-b' })], - 'wt-c': [makeTab({ id: 'tab-2', worktreeId: 'wt-c' })] - }, - activeWorktreeId: 'wt-c' - }) - const baseline = store.getState().sortEpoch - store.getState().setRuntimePaneTitle('tab-1', 1, '⠋ Claude') - expect(store.getState().sortEpoch).toBeGreaterThan(baseline) - }) }) diff --git a/src/renderer/src/store/slices/tabs.ts b/src/renderer/src/store/slices/tabs.ts index 9574cd3a6d1..72e22adc136 100644 --- a/src/renderer/src/store/slices/tabs.ts +++ b/src/renderer/src/store/slices/tabs.ts @@ -120,7 +120,7 @@ export type TabsSlice = { entityId: string, contentType?: TabContentType ) => Tab | null - activateTab: (tabId: string, opts?: { preservePreview?: boolean }) => void + activateTab: (tabId: string, opts?: { preservePreview?: boolean; worktreeId?: string }) => void closeUnifiedTab: ( tabId: string, opts?: { recordInteraction?: boolean; terminalRetirementHandled?: boolean } @@ -826,7 +826,17 @@ export const createTabsSlice: StateCreator = (set, activateTab: (tabId, opts) => { set((state) => { - const found = findTabAndWorktree(state.unifiedTabsByWorktree, tabId) + const scopedWorktreeId = opts?.worktreeId + let found: ReturnType + if (scopedWorktreeId !== undefined) { + const scopedTabs = Object.hasOwn(state.unifiedTabsByWorktree, scopedWorktreeId) + ? state.unifiedTabsByWorktree[scopedWorktreeId] + : [] + const scopedTab = scopedTabs.find((tab) => tab.id === tabId) + found = scopedTab ? { tab: scopedTab, worktreeId: scopedWorktreeId } : null + } else { + found = findTabAndWorktree(state.unifiedTabsByWorktree, tabId) + } if (!found) { return {} } diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 9cb47f77fb0..cb1b5809018 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -38,7 +38,7 @@ import { } from '../../../../shared/stable-pane-id' import { isValidHostTerminalTabId, isValidTerminalTabId } from '../../../../shared/terminal-tab-id' import { buildByIdIndex, buildWorktreeByIdIndex } from './worktree-by-id-index' -import { isTabInActiveWorktree, resolveActiveTabOwnerWorktreeId } from './active-tab-owner-worktree' +import { resolveActiveTabOwnerWorktreeId } from './active-tab-owner-worktree' import { isSameCodexRestartNoticeAccount } from './codex-restart-notice-account-identity' import { getRepoIdFromWorktreeId, @@ -1938,7 +1938,9 @@ export const createTerminalSlice: StateCreator }) const state = get() const ownerUnifiedTabs = - tabOwnerWorktreeId !== null ? (state.unifiedTabsByWorktree[tabOwnerWorktreeId] ?? []) : [] + tabOwnerWorktreeId !== null && Object.hasOwn(state.unifiedTabsByWorktree, tabOwnerWorktreeId) + ? state.unifiedTabsByWorktree[tabOwnerWorktreeId] + : [] // Why: a duplicated entity id must activate the same owner chosen above. const item = ownerUnifiedTabs.find( @@ -1948,7 +1950,10 @@ export const createTerminalSlice: StateCreator .flat() .find((entry) => entry.contentType === 'terminal' && entry.entityId === tabId) if (item) { - state.activateTab(item.id) + state.activateTab( + item.id, + tabOwnerWorktreeId !== null ? { worktreeId: tabOwnerWorktreeId } : undefined + ) } }, @@ -2150,13 +2155,7 @@ export const createTerminalSlice: StateCreator const ownerWorktreeId = classificationChanged ? getTerminalTabOwnerWorktreeId(s.tabsByWorktree, tabId) : null - // Why the second test: the owner cache is last-writer-wins, so a tab id - // held under two worktrees can name a background one for a pane that is - // in the active worktree — reinstating the click-driven re-sort #209 removed. - const isActive = - ownerWorktreeId !== null && - (ownerWorktreeId === s.activeWorktreeId || - isTabInActiveWorktree(s.tabsByWorktree, s.activeWorktreeId, tabId)) + const isActive = ownerWorktreeId !== null && ownerWorktreeId === s.activeWorktreeId const shouldBump = classificationChanged && ownerWorktreeId !== null && !isActive return { runtimePaneTitlesByTabId: { @@ -2191,12 +2190,7 @@ export const createTerminalSlice: StateCreator const ownerWorktreeId = hadClassification ? getTerminalTabOwnerWorktreeId(s.tabsByWorktree, tabId) : null - // Why the second test: see setRuntimePaneTitle — a duplicated tab id makes - // the last-writer-wins owner cache name a background worktree. - const isActive = - ownerWorktreeId !== null && - (ownerWorktreeId === s.activeWorktreeId || - isTabInActiveWorktree(s.tabsByWorktree, s.activeWorktreeId, tabId)) + const isActive = ownerWorktreeId !== null && ownerWorktreeId === s.activeWorktreeId const shouldBump = hadClassification && ownerWorktreeId !== null && !isActive return { runtimePaneTitlesByTabId: next,