mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
refactor(terminal): extract repair logic into reusable hook and guard ag
Extract the active terminal repair effect from Terminal.tsx into `useActiveTerminalRepair` hook to enable reuse in tests and clarify responsibilities. Replace falsy coercion guards (`obj[id] ?? []`) with explicit `Object.hasOwn()` checks to handle edge cases: empty-string worktree ids (valid but falsy), prototype-named ids like 'toString', and duplicated tab ids across worktrees. Remove the now-unused `isTabInActiveWorktree` helper. Simplify the isActive logic in terminals.ts to rely solely on owner-equality since the repair now uses proper membership checks.
This commit is contained in:
@@ -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<string, number>())
|
||||
|
||||
+82
-34
@@ -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(<RepairEffectHarness onPass={() => (passes += 1) <= MAX_PASSES} />)
|
||||
root.render(<RepairEffectHarness />)
|
||||
})
|
||||
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,
|
||||
|
||||
@@ -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<string, string | null>
|
||||
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
|
||||
])
|
||||
}
|
||||
@@ -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<typeof createTestStore>
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<string, TerminalTab[]>,
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<AppState, [], [], TabsSlice> = (set,
|
||||
|
||||
activateTab: (tabId, opts) => {
|
||||
set((state) => {
|
||||
const found = findTabAndWorktree(state.unifiedTabsByWorktree, tabId)
|
||||
const scopedWorktreeId = opts?.worktreeId
|
||||
let found: ReturnType<typeof findTabAndWorktree>
|
||||
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 {}
|
||||
}
|
||||
|
||||
@@ -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<AppState, [], [], TerminalSlice>
|
||||
})
|
||||
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<AppState, [], [], TerminalSlice>
|
||||
.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<AppState, [], [], TerminalSlice>
|
||||
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<AppState, [], [], TerminalSlice>
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user