* fix(terminals): make redundant tab activation idempotent (React #185) setActiveTab always reallocated activeTabIdByWorktree, even when the tab was already active for that worktree. Terminal's active-terminal repair effect depends on that map, so when the repair cannot converge activeTabId -- which happens when an earlier-scanned worktree reuses the tab id -- the effect re-triggers itself every commit until React throws #185. Crash cluster A: 12 reports, boundary terminal.workbench, 1.4.162/1.4.163. Co-authored-by: Orca <help@stably.ai> * fix(terminals): converge activeTabId when a tab id is owned by two worktrees Prefer the active worktree when resolving a terminal tab's owner. First-match ownership left activeTabId permanently unconvergeable under a duplicated tab id, so the active-terminal repair effect re-triggered itself into React #185. Breadcrumb the duplicate-ownership state (once per tab id) so a crash bundle can prove or kill the production origin of the precondition. Co-authored-by: Orca <help@stably.ai> * fix(crash-reporting): coalesce the duplicate-tab-owner breadcrumb Its renderer guard is once-per-tab-id, so the stale worktree map it exists to diagnose duplicates every tab id at once and could evict the whole 30-entry ring. Also drops two keyed re-reads of tabsByWorktree that would throw for a prototype-named worktree id, and pins the activeTabIdByWorktree guard with a test that fails without it. Co-authored-by: Orca <help@stably.ai> * fix(crash-reporting): key the duplicate-tab-owner crumb on its convergence flag Name-only coalescing keeps only the newest payload, so a resolvedToActiveWorktree false sample — the one value saying the activation still could not converge — was erased by any later benign true in the same 30s window. Keys on the flag instead, mirroring the WebGL name:kind branch; two keys still bound the burst. Also: the previous coalescing commit had no test at all (removing the name from both sets broke zero of 2701 tests), the resolver's activeWorktreeId truthiness check was a hole rather than a guard for a '' active id, and the resolver test file failed oxfmt --check. Co-authored-by: Orca <help@stably.ai> * fix(crash-reporting): keep the non-converging duplicate-tab verdict The two earlier commits contradicted each other. Splitting the coalesce key existed so a `false` verdict could not be erased by a later benign `true` — but the renderer guard was keyed on the tab id alone, so for any one id only the first verdict was ever emitted. A duplicated id that first resolves benignly, then stops converging when the user switches worktrees, dropped the `false` sample at the source. That sample is the whole reason the breadcrumb exists: it is the only value saying the activation could not converge activeTabId. Key the guard on id plus verdict. At most two crumbs per tab id, and the main process still folds each verdict into its own ring entry, so the flood bound is unchanged. * fix(crash): correct the duplicate-tab verdict rationale, pin and cap the guard Three comments said `false` is the verdict that matters because it is the only one showing the activation could not converge. That is backwards. The repair effect activates a tab drawn from tabsByWorktree[active], so the React #185 path can only ever emit `true`; `false` is what a deliberate jump-to-agent into a background worktree emits from a fully converged state. A reader of the next bundle would have discarded the exact sample the breadcrumb exists to capture. The mechanism was right, only its stated reason was wrong: the real justification for keying on the verdict is symmetric, since coalescing keeps only the newest payload and either verdict would erase the other. The suite also did not pin the "at most 2 per tab id" bound - a guard keyed on `${tabId}:${activeWorktreeId}` passed all 11 tests while emitting once per worktree, the storm the guard exists to prevent. Adds a count-pinning test that kills it. Caps the never-pruned guard set at 256 distinct verdict keys (~85KB), mirroring MAX_COALESCE_KEYS. Measured 330 B/entry; a realistic thousand duplicated tab ids is ~0.6MB, negligible but unbounded in principle. * perf(crash): scan worktree tabs by key, and soften the verdict rationale Round 6 corrected my own round-5 comment. I had written that `true` is the repair-loop signature and `false` covers a deliberate background activation. The repair effect can emit `false` too: its closure holds the worktree from its render while the guard runs against live state, so a worktree switch landing in between reattributes the tab. The verdict hints at the caller; it does not prove it, and neither value should be discarded. Comment-only. Also take the free scan win the perf review measured: Object.entries allocates a pair array per worktree on a path that runs per tab activation. Own keys are safe to index by, so Object.keys plus an indexed read is behaviour-identical (16.1us -> 5.0us at 170 worktrees x 10 tabs). * fix(terminal): keep a duplicated tab id from re-sorting the active worktree setActiveTab now prefers the active worktree when a tab id is held by more than one, but terminals.ts has a second, older owner resolver: getTerminalTabOwnerWorktreeId, a memoized map built last-writer-wins. Two of its callers — setRuntimePaneTitle and clearRuntimePaneTitle — use the result for the same "is this pane in the active worktree" gate, so under a duplicate the two resolvers disagree: the cache names whichever worktree it saw last, which can be a background one for a pane the user is looking at. The gate then fails open and every classified OSC title frame bumps sortEpoch, reinstating the click-driven sidebar re-sort #209 removed — 20 title frames measured 20 bumps, each one a store write that re-renders every sortEpoch subscriber. isTabInActiveWorktree answers from the active worktree's own tab list instead of a tie-break. It stays behind the cheap id equality so the common non-duplicated path is unchanged, and it is a hasOwn lookup plus one scan of that worktree's tabs rather than resolveActiveTabOwnerWorktreeId, whose full scan would run per title frame and whose breadcrumb would fold a second caller into one verdict. Leaves updateTabTitle and clearTabLaunchAgent on the cache: they pick which copy of a duplicated tab to mutate, where no answer is defensible until the duplication itself is fixed. * test(terminal): pin the SSH-hydration origin of the duplicate tab id Drives the duplicate from real hydration rather than constructing it: a direct-SSH snapshot is keyed by worktree path, so renaming the worktree on the host (or re-adding the repo, which mints a fresh id) re-resolves it to a new worktree id while replaceHydratedRecordKeys retains the old key verbatim. Nothing de-dupes across keys. Fails on unfixed origin/main with converged=false after 200 passes; the two precondition assertions pass on both sides, so the red is the non-convergence itself and not a setup divergence. The reconnectPersistedTerminals stub is load-bearing and marked as such: with no registered PTY the orphan sweep cleans the duplicate up before the repair effect sees it. * docs(test): record the end-to-end #185 reproduction method on the regression test Co-authored-by: Orca <help@stably.ai> * test(terminal): pin the null-active-worktree guard in isTabInActiveWorktree Dropping the `activeWorktreeId === null` early return was killed by nothing: `Object.hasOwn(map, null)` coerces to the string key 'null', so a worktree literally named 'null' would answer for "no active worktree". An untested guard reads as dead code and gets deleted. * rm triage context * rm triage context * rm context files * 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. --------- Co-authored-by: Orca <help@stably.ai> Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
This commit is contained in:
parent
036b1e78ba
commit
36cc8495ef
|
|
@ -298,6 +298,49 @@ describe('renderer breadcrumb IPC routing', () => {
|
|||
])
|
||||
})
|
||||
|
||||
// Why: the renderer guard is once per tab-id/verdict, so one stale worktree
|
||||
// map can emit enough crumbs to evict the pre-crash trail.
|
||||
it('coalesces duplicate-tab-owner notices across tabs', () => {
|
||||
emitRendererBreadcrumb({
|
||||
name: 'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
data: { ownerCount: 2, resolvedToActiveWorktree: true }
|
||||
})
|
||||
emitRendererBreadcrumb({
|
||||
name: 'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
data: { ownerCount: 3, resolvedToActiveWorktree: true }
|
||||
})
|
||||
|
||||
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
|
||||
expect(recordCoalescedCrashBreadcrumbMock).toHaveBeenCalledTimes(2)
|
||||
for (const call of recordCoalescedCrashBreadcrumbMock.mock.calls) {
|
||||
expect(call[0]).toMatchObject({
|
||||
coalesceKey: 'terminal_tab_id_owned_by_multiple_worktrees:true'
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
// Why flag-scoped: coalescing keeps only the newest payload, and the verdict
|
||||
// flips under a persisting duplicate, so one would erase the other.
|
||||
it('keeps a non-converging duplicate-tab-owner notice out of the converging one', () => {
|
||||
emitRendererBreadcrumb({
|
||||
name: 'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
data: { ownerCount: 2, resolvedToActiveWorktree: false }
|
||||
})
|
||||
emitRendererBreadcrumb({
|
||||
name: 'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
data: { ownerCount: 2, resolvedToActiveWorktree: true }
|
||||
})
|
||||
|
||||
expect(
|
||||
recordCoalescedCrashBreadcrumbMock.mock.calls.map(
|
||||
(call) => (call[0] as { coalesceKey: string }).coalesceKey
|
||||
)
|
||||
).toEqual([
|
||||
'terminal_tab_id_owned_by_multiple_worktrees:false',
|
||||
'terminal_tab_id_owned_by_multiple_worktrees:true'
|
||||
])
|
||||
})
|
||||
|
||||
it('records non-error renderer breadcrumbs without coalescing', () => {
|
||||
emitRendererBreadcrumb({ name: 'renderer_bootstrap_started', data: { dev: true } })
|
||||
|
||||
|
|
|
|||
|
|
@ -326,11 +326,13 @@ function buildUncapturedCrashReportText(
|
|||
// storm, #8260) can flush the whole fixed-size breadcrumb ring in seconds,
|
||||
// erasing the pre-crash trail. Coalesce repeats into one entry that carries a
|
||||
// suppressed count instead.
|
||||
const DUPLICATE_TAB_OWNER_BREADCRUMB = 'terminal_tab_id_owned_by_multiple_worktrees'
|
||||
const COALESCED_RENDERER_BREADCRUMB_NAMES = new Set([
|
||||
'renderer_error',
|
||||
'renderer_unhandled_rejection',
|
||||
'terminal_park_verdict_churn',
|
||||
'terminal_safe_fit_retry_exhausted',
|
||||
DUPLICATE_TAB_OWNER_BREADCRUMB,
|
||||
TERMINAL_WEBGL_DIAGNOSTIC_BREADCRUMB
|
||||
])
|
||||
const RENDERER_BREADCRUMB_COALESCE_MS = 30_000
|
||||
|
|
@ -362,6 +364,11 @@ function rendererBreadcrumbCoalesceKey(
|
|||
if (name === TERMINAL_WEBGL_DIAGNOSTIC_BREADCRUMB) {
|
||||
return `${name}:${String(data?.kind ?? '')}`
|
||||
}
|
||||
// Why: a stale map can emit once per tab-id/verdict; key by verdict so
|
||||
// last-write coalescing cannot erase the other signal while remaining bounded.
|
||||
if (name === DUPLICATE_TAB_OWNER_BREADCRUMB) {
|
||||
return `${name}:${String(data?.resolvedToActiveWorktree ?? '')}`
|
||||
}
|
||||
const primaryMessage = name === 'renderer_error' ? data?.message : data?.reasonMessage
|
||||
const fallbackMessage = name === 'renderer_error' ? data?.errorMessage : undefined
|
||||
const message =
|
||||
|
|
|
|||
|
|
@ -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>())
|
||||
|
|
|
|||
|
|
@ -0,0 +1,327 @@
|
|||
/** @vitest-environment happy-dom */
|
||||
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 { useActiveTerminalRepair } from './use-active-terminal-repair'
|
||||
|
||||
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
|
||||
|
||||
// Why: React throws #185 at 51 nested commits; 400 proves divergence, not slowness.
|
||||
const MAX_PASSES = 400
|
||||
|
||||
function terminalTab(id: string, worktreeId: string): TerminalTab {
|
||||
return { id, worktreeId, title: id, createdAt: 0, sortOrder: 0 } as unknown as TerminalTab
|
||||
}
|
||||
|
||||
function unifiedTerminalTab(
|
||||
id: string,
|
||||
entityId: string,
|
||||
worktreeId: string,
|
||||
groupId: string
|
||||
): Tab {
|
||||
return {
|
||||
id,
|
||||
entityId,
|
||||
worktreeId,
|
||||
groupId,
|
||||
contentType: 'terminal',
|
||||
label: id,
|
||||
customLabel: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 0
|
||||
}
|
||||
}
|
||||
|
||||
function tabGroup(
|
||||
id: string,
|
||||
worktreeId: string,
|
||||
activeTabId: string,
|
||||
tabOrder: string[]
|
||||
): TabGroup {
|
||||
return { id, worktreeId, activeTabId, tabOrder, recentTabIds: [activeTabId] }
|
||||
}
|
||||
|
||||
function RepairEffectHarness(): null {
|
||||
const activeTabId = useAppStore((s) => s.activeTabId)
|
||||
const activeTabIdByWorktree = useAppStore((s) => s.activeTabIdByWorktree)
|
||||
const activeTabType = useAppStore((s) => s.activeTabType)
|
||||
const tabsByWorktree = useAppStore((s) => s.tabsByWorktree)
|
||||
const renderedActiveWorktreeId = useAppStore((s) => s.activeWorktreeId)
|
||||
const setActiveTab = useAppStore((s) => s.setActiveTab)
|
||||
const tabs = useMemo(
|
||||
() =>
|
||||
renderedActiveWorktreeId !== null && Object.hasOwn(tabsByWorktree, renderedActiveWorktreeId)
|
||||
? tabsByWorktree[renderedActiveWorktreeId]
|
||||
: [],
|
||||
[renderedActiveWorktreeId, tabsByWorktree]
|
||||
)
|
||||
|
||||
useActiveTerminalRepair({
|
||||
activeTabId,
|
||||
activeTabType,
|
||||
setActiveTab,
|
||||
tabs,
|
||||
activeTabIdByWorktree,
|
||||
renderedActiveWorktreeId
|
||||
})
|
||||
return null
|
||||
}
|
||||
|
||||
let cleanup: (() => void) | null = null
|
||||
|
||||
afterEach(() => {
|
||||
cleanup?.()
|
||||
cleanup = null
|
||||
})
|
||||
|
||||
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 />)
|
||||
})
|
||||
return passes
|
||||
}
|
||||
|
||||
describe('active-terminal repair effect cannot drive a React #185 update loop', () => {
|
||||
it('settles when the repaired tab is owned by the active worktree', () => {
|
||||
useAppStore.setState({
|
||||
activeWorktreeId: 'wt-active',
|
||||
activeTabType: 'terminal',
|
||||
activeTabId: 'stale-tab',
|
||||
activeTabIdByWorktree: {},
|
||||
tabsByWorktree: { 'wt-active': [terminalTab('t1', 'wt-active')] },
|
||||
unifiedTabsByWorktree: {}
|
||||
})
|
||||
expect(measureRepairPasses()).toBeLessThan(10)
|
||||
expect(useAppStore.getState().activeTabId).toBe('t1')
|
||||
})
|
||||
|
||||
it('settles when another worktree reuses the tab id and is scanned first', () => {
|
||||
// Why regression: first-match ownership skipped activeTabId while reallocating
|
||||
// activeTabIdByWorktree, retriggering the repair effect indefinitely.
|
||||
useAppStore.setState({
|
||||
activeWorktreeId: 'wt-active',
|
||||
activeTabType: 'terminal',
|
||||
activeTabId: 'stale-tab',
|
||||
activeTabIdByWorktree: {},
|
||||
tabsByWorktree: {
|
||||
'wt-other': [terminalTab('t1', 'wt-other')],
|
||||
'wt-active': [terminalTab('t1', 'wt-active')]
|
||||
},
|
||||
unifiedTabsByWorktree: {}
|
||||
})
|
||||
expect(measureRepairPasses()).toBeLessThan(10)
|
||||
// Why: settling by refusing to write would leave the repair permanently
|
||||
// unsatisfied — quiet, but with activeTabId stuck on a tab that is gone.
|
||||
expect(useAppStore.getState().activeTabId).toBe('t1')
|
||||
expect(useAppStore.getState().activeTabIdByWorktree['wt-active']).toBe('t1')
|
||||
})
|
||||
|
||||
it('activates the active worktree unified tab when another worktree reuses the entity id', () => {
|
||||
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',
|
||||
activeTabId: 't2',
|
||||
activeTabIdByWorktree: { 'wt-active': 't2' },
|
||||
tabsByWorktree: {
|
||||
'wt-other': [terminalTab('t1', 'wt-other')],
|
||||
'wt-active': [terminalTab('t1', 'wt-active'), terminalTab('t2', 'wt-active')]
|
||||
},
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-other': [otherTab, otherPreviousTab],
|
||||
'wt-active': [activeTab, previousActiveTab]
|
||||
},
|
||||
groupsByWorktree: {
|
||||
'wt-other': [
|
||||
tabGroup('g-other', 'wt-other', otherPreviousTab.id, [otherTab.id, otherPreviousTab.id])
|
||||
],
|
||||
'wt-active': [
|
||||
tabGroup('g-active', 'wt-active', previousActiveTab.id, [
|
||||
activeTab.id,
|
||||
previousActiveTab.id
|
||||
])
|
||||
]
|
||||
},
|
||||
activeGroupIdByWorktree: { 'wt-other': 'g-other', 'wt-active': 'g-active' }
|
||||
})
|
||||
|
||||
act(() => {
|
||||
useAppStore.getState().setActiveTab('t1')
|
||||
})
|
||||
|
||||
expect(useAppStore.getState().groupsByWorktree['wt-active'][0].activeTabId).toBe(activeTab.id)
|
||||
expect(useAppStore.getState().groupsByWorktree['wt-other'][0].activeTabId).toBe(
|
||||
otherPreviousTab.id
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps unified-only terminal activation as a fallback', () => {
|
||||
const targetTab = unifiedTerminalTab('u-target', 't1', 'wt-active', 'g-active')
|
||||
const previousTab = unifiedTerminalTab('u-previous', 't2', 'wt-active', 'g-active')
|
||||
useAppStore.setState({
|
||||
activeWorktreeId: 'wt-active',
|
||||
activeTabId: null,
|
||||
activeTabIdByWorktree: {},
|
||||
tabsByWorktree: {},
|
||||
unifiedTabsByWorktree: { 'wt-active': [targetTab, previousTab] },
|
||||
groupsByWorktree: {
|
||||
'wt-active': [
|
||||
tabGroup('g-active', 'wt-active', previousTab.id, [targetTab.id, previousTab.id])
|
||||
]
|
||||
},
|
||||
activeGroupIdByWorktree: { 'wt-active': 'g-active' }
|
||||
})
|
||||
|
||||
act(() => {
|
||||
useAppStore.getState().setActiveTab('t1')
|
||||
})
|
||||
|
||||
expect(useAppStore.getState().groupsByWorktree['wt-active'][0].activeTabId).toBe(targetTab.id)
|
||||
expect(useAppStore.getState().activeTabId).toBeNull()
|
||||
})
|
||||
|
||||
it('does not reallocate activeTabIdByWorktree when the tab is already active', () => {
|
||||
// Why: that map is a dependency of both the repair effect and the parked
|
||||
// watcher sync, so a redundant activation must not re-run either.
|
||||
useAppStore.setState({
|
||||
activeWorktreeId: 'wt-active',
|
||||
activeTabType: 'terminal',
|
||||
activeTabId: 't1',
|
||||
activeTabIdByWorktree: {},
|
||||
tabsByWorktree: { 'wt-active': [terminalTab('t1', 'wt-active')] },
|
||||
unifiedTabsByWorktree: {}
|
||||
})
|
||||
act(() => {
|
||||
useAppStore.getState().setActiveTab('t1')
|
||||
})
|
||||
const settled = useAppStore.getState().activeTabIdByWorktree
|
||||
act(() => {
|
||||
useAppStore.getState().setActiveTab('t1')
|
||||
})
|
||||
expect(useAppStore.getState().activeTabIdByWorktree).toBe(settled)
|
||||
})
|
||||
|
||||
it('keeps bell attribution off a background worktree tab', () => {
|
||||
useAppStore.setState({
|
||||
activeWorktreeId: 'wt-active',
|
||||
activeTabType: 'terminal',
|
||||
activeTabId: 'visible-tab',
|
||||
activeTabIdByWorktree: {},
|
||||
tabsByWorktree: {
|
||||
'wt-active': [terminalTab('visible-tab', 'wt-active')],
|
||||
'wt-background': [terminalTab('bg-tab', 'wt-background')]
|
||||
},
|
||||
unifiedTabsByWorktree: {}
|
||||
})
|
||||
act(() => {
|
||||
useAppStore.getState().setActiveTab('bg-tab')
|
||||
})
|
||||
expect(useAppStore.getState().activeTabId).toBe('visible-tab')
|
||||
expect(useAppStore.getState().activeTabIdByWorktree['wt-background']).toBe('bg-tab')
|
||||
})
|
||||
|
||||
it('records activation for a falsy-but-valid worktree id', () => {
|
||||
useAppStore.setState({
|
||||
activeWorktreeId: '',
|
||||
activeTabId: null,
|
||||
activeTabIdByWorktree: {},
|
||||
tabsByWorktree: { '': [terminalTab('t1', '')] },
|
||||
unifiedTabsByWorktree: {}
|
||||
})
|
||||
act(() => {
|
||||
useAppStore.getState().setActiveTab('t1')
|
||||
})
|
||||
expect(useAppStore.getState().activeTabId).toBe('t1')
|
||||
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,
|
||||
activeTabId: null,
|
||||
activeTabIdByWorktree: {},
|
||||
tabsByWorktree: {},
|
||||
unifiedTabsByWorktree: {},
|
||||
unreadTerminalTabs: { 'missing-tab': true }
|
||||
})
|
||||
act(() => {
|
||||
useAppStore.getState().setActiveTab('missing-tab')
|
||||
})
|
||||
expect(useAppStore.getState().activeTabId).toBeNull()
|
||||
expect(useAppStore.getState().activeTabIdByWorktree).toEqual({})
|
||||
expect(useAppStore.getState().unreadTerminalTabs['missing-tab']).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
@ -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
|
||||
])
|
||||
}
|
||||
|
|
@ -0,0 +1,224 @@
|
|||
/**
|
||||
* A direct-SSH snapshot can retain the same tab under old and new worktree IDs
|
||||
* after a path or repo-ID change. This exercises that hydration path and proves
|
||||
* active-tab repair converges; it deliberately does not remove the duplicate or
|
||||
* reproduce React's scheduler-level #185 throw. See PR #11950 for that evidence.
|
||||
*/
|
||||
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, 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 { 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) => {
|
||||
const actual = await importOriginal<typeof AgentStatusModule>()
|
||||
return { ...actual, detectAgentStatusFromTitle: vi.fn().mockReturnValue(null) }
|
||||
})
|
||||
|
||||
const TARGET_ID = 'ssh-target-1'
|
||||
const OLD_PATH = '/srv/proj/wt'
|
||||
const NEW_PATH = '/srv/proj/wt-renamed'
|
||||
const OLD_ID = `repoA::${OLD_PATH}`
|
||||
const NEW_ID = `repoA::${NEW_PATH}`
|
||||
// Why a cap and not a while(true): on unfixed code this cycle never terminates.
|
||||
const MAX_REPAIR_PASSES = 200
|
||||
|
||||
const authority: DirectSshAuthority = {
|
||||
targetId: TARGET_ID,
|
||||
providerEpoch: 'provider-epoch-1' as SshProviderEpoch,
|
||||
connectionGeneration: 1
|
||||
}
|
||||
|
||||
function token(snapshotRevision: number): DirectSshSnapshotApplyToken {
|
||||
return {
|
||||
authority,
|
||||
catalogRevision: 0,
|
||||
repoFingerprint: 'fp',
|
||||
authorityRequirement: 'required',
|
||||
snapshotRevision,
|
||||
outcome: 'complete'
|
||||
}
|
||||
}
|
||||
|
||||
function snapshot(
|
||||
revision: number,
|
||||
worktreePath: string,
|
||||
tabIds: readonly string[],
|
||||
activeTabId: string | null
|
||||
): RemoteWorkspaceSnapshot {
|
||||
return {
|
||||
namespace: 'workspace',
|
||||
revision,
|
||||
updatedAt: revision,
|
||||
schemaVersion: 1,
|
||||
session: {
|
||||
activeWorktreePath: worktreePath,
|
||||
activeTabId,
|
||||
tabsByWorktreePath: {
|
||||
[worktreePath]: tabIds.map((tabId, index) => ({
|
||||
id: tabId,
|
||||
worktreePath,
|
||||
ptyId: `pty-${tabId}`,
|
||||
title: `Terminal ${index + 1}`,
|
||||
customTitle: null,
|
||||
color: null,
|
||||
sortOrder: index,
|
||||
createdAt: index + 1
|
||||
}))
|
||||
},
|
||||
terminalLayoutsByTabId: {},
|
||||
activeWorktreePathsOnShutdown: [],
|
||||
activeTabIdByWorktreePath: { [worktreePath]: activeTabId },
|
||||
remoteSessionIdsByTabId: Object.fromEntries(tabIds.map((id) => [id, `pty-${id}`])),
|
||||
lastVisitedAtByWorktreePath: { [worktreePath]: revision },
|
||||
defaultTerminalTabsAppliedByWorktreePath: { [worktreePath]: true }
|
||||
}
|
||||
} satisfies RemoteWorkspaceSnapshot
|
||||
}
|
||||
|
||||
type TestStore = ReturnType<typeof createTestStore>
|
||||
|
||||
async function applySnapshot(store: TestStore, snap: RemoteWorkspaceSnapshot): Promise<void> {
|
||||
await applyDirectSshRemoteWorkspaceSnapshot({
|
||||
store,
|
||||
snapshot: snap,
|
||||
token: token(snap.revision),
|
||||
arrival: 1,
|
||||
isArrivalCurrent: () => true,
|
||||
isPreparationTokenCurrent: () => true,
|
||||
waitForWorkspaceSessionReady: async () => true,
|
||||
finalizeHydratedTerminals: () => 0
|
||||
})
|
||||
}
|
||||
|
||||
function worktreeIdsOwningTab(store: TestStore, tabId: string): string[] {
|
||||
return Object.entries(store.getState().tabsByWorktree)
|
||||
.filter(([, tabs]) => tabs.some((tab) => tab.id === tabId))
|
||||
.map(([worktreeId]) => worktreeId)
|
||||
}
|
||||
|
||||
function seedCatalog(store: TestStore, worktreePath: string): void {
|
||||
store.setState({
|
||||
worktreesByRepo: {
|
||||
repoA: [
|
||||
makeWorktree({
|
||||
id: `repoA::${worktreePath}`,
|
||||
repoId: 'repoA',
|
||||
path: worktreePath,
|
||||
hostId: `ssh:${TARGET_ID}`
|
||||
} as never)
|
||||
]
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* One turn of the loop in Terminal.tsx's active-terminal repair effect:
|
||||
* recompute the repaired id from live state, then activate it. Reports how many
|
||||
* turns it took to stop and how often `activeTabIdByWorktree` — a declared dep
|
||||
* of that effect, so a fresh identity re-runs it — was reallocated.
|
||||
*/
|
||||
function runRepairCycle(store: TestStore): {
|
||||
converged: boolean
|
||||
passes: number
|
||||
depIdentityChurn: number
|
||||
} {
|
||||
let passes = 0
|
||||
let depIdentityChurn = 0
|
||||
for (; passes < MAX_REPAIR_PASSES; passes += 1) {
|
||||
const live = store.getState()
|
||||
const depsBefore = live.activeTabIdByWorktree
|
||||
const repaired = repairActiveTerminalTab({
|
||||
activeTabType: 'terminal',
|
||||
activeTabId: live.activeTabId,
|
||||
activeTabIdByWorktree: live.activeTabIdByWorktree,
|
||||
renderedActiveWorktreeId: live.activeWorktreeId,
|
||||
setActiveTab: live.setActiveTab,
|
||||
tabs: live.activeWorktreeId ? (live.tabsByWorktree[live.activeWorktreeId] ?? []) : []
|
||||
})
|
||||
if (!repaired) {
|
||||
return { converged: true, passes, depIdentityChurn }
|
||||
}
|
||||
if (store.getState().activeTabIdByWorktree !== depsBefore) {
|
||||
depIdentityChurn += 1
|
||||
}
|
||||
}
|
||||
return { converged: false, passes, depIdentityChurn }
|
||||
}
|
||||
|
||||
describe('direct-SSH snapshot apply, tab id owned by two worktrees', () => {
|
||||
it('converges the active-terminal repair instead of re-running it forever', async () => {
|
||||
const store = createTestStore()
|
||||
|
||||
store.setState({
|
||||
repos: [
|
||||
{
|
||||
id: 'repoA',
|
||||
path: '/srv/proj',
|
||||
displayName: 'Proj',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
connectionId: TARGET_ID
|
||||
} as never
|
||||
],
|
||||
// Load-bearing, do not drop: the IPC attach is the only thing stubbed, and
|
||||
// it leaves behind exactly what a real reconnect leaves behind — one
|
||||
// registered live PTY per tab. Without that the orphan sweep on the next
|
||||
// worktree visit treats the duplicated tab as dead, cleans it up, and the
|
||||
// bug evaporates before the repair effect ever sees it.
|
||||
reconnectPersistedTerminals: (async () => {
|
||||
const live = store.getState()
|
||||
const registered: Record<string, string[]> = { ...live.ptyIdsByTabId }
|
||||
for (const tabs of Object.values(live.tabsByWorktree)) {
|
||||
for (const tab of tabs) {
|
||||
registered[tab.id] = [`pty-${tab.id}`]
|
||||
}
|
||||
}
|
||||
store.setState({ ptyIdsByTabId: registered })
|
||||
}) as never,
|
||||
markRemoteWorkspaceHydrated: (() => {}) as never,
|
||||
setRemoteWorkspaceSyncStatus: (() => {}) as never
|
||||
})
|
||||
|
||||
seedCatalog(store, OLD_PATH)
|
||||
await applySnapshot(store, snapshot(1, OLD_PATH, ['tab-1', 'tab-2'], 'tab-1'))
|
||||
store.getState().setActiveWorktree(OLD_ID)
|
||||
|
||||
// The worktree is renamed on the host; the catalog re-detects it at the new
|
||||
// path, so the worktree id changes while the tab ids do not.
|
||||
seedCatalog(store, NEW_PATH)
|
||||
await applySnapshot(store, snapshot(2, NEW_PATH, ['tab-1', 'tab-2'], 'tab-1'))
|
||||
store.getState().setActiveWorktree(NEW_ID)
|
||||
|
||||
// The remote deselects; importRemoteWorkspaceSession nulls an activeTabId it
|
||||
// cannot find among the imported tabs, which is what arms the repair effect.
|
||||
await applySnapshot(store, snapshot(3, NEW_PATH, ['tab-1', 'tab-2'], null))
|
||||
|
||||
// Why assert the precondition and not its removal: the fix stops the owner
|
||||
// resolver being fooled by the duplicate, it does not remove the duplicate.
|
||||
// Pinned so the test cannot pass vacuously if hydration stops producing one.
|
||||
expect(worktreeIdsOwningTab(store, 'tab-1')).toEqual([OLD_ID, NEW_ID])
|
||||
expect(store.getState().activeTabId).toBeNull()
|
||||
|
||||
const repair = runRepairCycle(store)
|
||||
|
||||
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.
|
||||
const settled = runRepairCycle(store)
|
||||
expect(settled.depIdentityChurn).toBe(0)
|
||||
expect(settled.passes).toBe(0)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,125 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { TerminalTab } from '../../../../shared/types'
|
||||
|
||||
const recordRendererCrashBreadcrumb = vi.fn()
|
||||
vi.mock('../../lib/crash-breadcrumb-recorder', () => ({
|
||||
recordRendererCrashBreadcrumb: (...args: unknown[]) => recordRendererCrashBreadcrumb(...args)
|
||||
}))
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
recordRendererCrashBreadcrumb.mockClear()
|
||||
_resetDuplicateTabOwnerBreadcrumbsForTests()
|
||||
})
|
||||
|
||||
describe('resolveActiveTabOwnerWorktreeId', () => {
|
||||
it('returns the sole owner and stays quiet', () => {
|
||||
const owner = resolveActiveTabOwnerWorktreeId(
|
||||
{ 'wt-a': [tab('t1', 'wt-a')], 'wt-b': [tab('t2', 'wt-b')] },
|
||||
'wt-a',
|
||||
't1'
|
||||
)
|
||||
expect(owner).toBe('wt-a')
|
||||
expect(recordRendererCrashBreadcrumb).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns null when no worktree owns the tab', () => {
|
||||
expect(resolveActiveTabOwnerWorktreeId({ 'wt-a': [tab('t1', 'wt-a')] }, 'wt-a', 'gone')).toBe(
|
||||
null
|
||||
)
|
||||
})
|
||||
|
||||
it('prefers the active worktree over an earlier-scanned duplicate', () => {
|
||||
const owner = resolveActiveTabOwnerWorktreeId(
|
||||
{ 'wt-other': [tab('t1', 'wt-other')], 'wt-active': [tab('t1', 'wt-active')] },
|
||||
'wt-active',
|
||||
't1'
|
||||
)
|
||||
expect(owner).toBe('wt-active')
|
||||
expect(recordRendererCrashBreadcrumb).toHaveBeenCalledWith(
|
||||
'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
{ ownerCount: 2, resolvedToActiveWorktree: true }
|
||||
)
|
||||
})
|
||||
|
||||
it('falls back to first match when the active worktree is not an owner', () => {
|
||||
const owner = resolveActiveTabOwnerWorktreeId(
|
||||
{ 'wt-x': [tab('t1', 'wt-x')], 'wt-y': [tab('t1', 'wt-y')] },
|
||||
'wt-active',
|
||||
't1'
|
||||
)
|
||||
expect(owner).toBe('wt-x')
|
||||
expect(recordRendererCrashBreadcrumb).toHaveBeenCalledWith(
|
||||
'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
{ ownerCount: 2, resolvedToActiveWorktree: false }
|
||||
)
|
||||
})
|
||||
|
||||
// Why: a truthiness guard on the active id would drop this back to first-match.
|
||||
it('prefers a falsy-but-valid active worktree id', () => {
|
||||
const owner = resolveActiveTabOwnerWorktreeId(
|
||||
{ 'wt-other': [tab('t1', 'wt-other')], '': [tab('t1', '')] },
|
||||
'',
|
||||
't1'
|
||||
)
|
||||
expect(owner).toBe('')
|
||||
})
|
||||
|
||||
it('breadcrumbs a given tab id once per verdict so it cannot flood the ring', () => {
|
||||
const maps = { 'wt-a': [tab('t1', 'wt-a')], 'wt-b': [tab('t1', 'wt-b')] }
|
||||
for (let i = 0; i < 5; i += 1) {
|
||||
resolveActiveTabOwnerWorktreeId(maps, 'wt-a', 't1')
|
||||
}
|
||||
expect(recordRendererCrashBreadcrumb).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
// Why: the active worktree changes under a persisting duplicate, and coalescing
|
||||
// keeps only the newest payload — keyed on the tab id alone, whichever verdict
|
||||
// a tab reported first would suppress the other for the rest of the session.
|
||||
it('still reports a non-converging verdict after that tab id reported a converging one', () => {
|
||||
const maps = { 'wt-other': [tab('t1', 'wt-other')], 'wt-active': [tab('t1', 'wt-active')] }
|
||||
resolveActiveTabOwnerWorktreeId(maps, 'wt-active', 't1')
|
||||
resolveActiveTabOwnerWorktreeId(maps, 'wt-third', 't1')
|
||||
resolveActiveTabOwnerWorktreeId(maps, 'wt-third', 't1')
|
||||
|
||||
expect(recordRendererCrashBreadcrumb.mock.calls).toEqual([
|
||||
[
|
||||
'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
{ ownerCount: 2, resolvedToActiveWorktree: true }
|
||||
],
|
||||
[
|
||||
'terminal_tab_id_owned_by_multiple_worktrees',
|
||||
{ ownerCount: 2, resolvedToActiveWorktree: false }
|
||||
]
|
||||
])
|
||||
})
|
||||
|
||||
// Why the count and not just "reports twice": a guard keyed on the active
|
||||
// worktree id passes the two tests above yet emits once per worktree, which is
|
||||
// the flood this guard exists to prevent.
|
||||
it('never exceeds two crumbs for one tab id however the active worktree moves', () => {
|
||||
const maps = { 'wt-a': [tab('t1', 'wt-a')], 'wt-b': [tab('t1', 'wt-b')] }
|
||||
const activeWorktreeIds = ['wt-a', 'wt-b', 'wt-c', '', 'wt-d', 'wt-a']
|
||||
for (let i = 0; i < 600; i += 1) {
|
||||
resolveActiveTabOwnerWorktreeId(maps, activeWorktreeIds[i % activeWorktreeIds.length], 't1')
|
||||
}
|
||||
expect(recordRendererCrashBreadcrumb).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
// Why: the guard set is never pruned and tab ids are minted per created tab.
|
||||
it('stops recording once the per-session sample cap is reached', () => {
|
||||
for (let i = 0; i < 400; i += 1) {
|
||||
const id = `t-${i}`
|
||||
const maps = { 'wt-a': [tab(id, 'wt-a')], 'wt-b': [tab(id, 'wt-b')] }
|
||||
resolveActiveTabOwnerWorktreeId(maps, 'wt-a', id)
|
||||
resolveActiveTabOwnerWorktreeId(maps, 'wt-c', id)
|
||||
}
|
||||
expect(recordRendererCrashBreadcrumb).toHaveBeenCalledTimes(256)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,82 @@
|
|||
import type { TerminalTab } from '../../../../shared/types'
|
||||
import { recordRendererCrashBreadcrumb } from '../../lib/crash-breadcrumb-recorder'
|
||||
|
||||
const reportedDuplicateTabVerdicts = new Set<string>()
|
||||
// Why capped: this set is never pruned and each tab id adds up to two verdict
|
||||
// keys. 256 keys cover 128–256 duplicated ids, enough evidence for a bundle.
|
||||
const MAX_REPORTED_DUPLICATE_TAB_VERDICTS = 256
|
||||
|
||||
/** Test seam: the duplicate breadcrumb is once-per-tab-id-per-verdict per session. */
|
||||
export function _resetDuplicateTabOwnerBreadcrumbsForTests(): void {
|
||||
reportedDuplicateTabVerdicts.clear()
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve which worktree owns a terminal tab, preferring the active worktree.
|
||||
*
|
||||
* Why the preference: a stale map can leave one tab id under two worktrees, and
|
||||
* attributing it to an arbitrary first match leaves `activeTabId` permanently
|
||||
* unconvergeable — which strands Terminal's active-terminal repair effect in a
|
||||
* self-retriggering loop (React #185).
|
||||
*/
|
||||
export function resolveActiveTabOwnerWorktreeId(
|
||||
tabsByWorktree: Record<string, TerminalTab[]>,
|
||||
activeWorktreeId: string | null,
|
||||
tabId: string
|
||||
): string | null {
|
||||
let firstOwnerId: string | null = null
|
||||
let ownerCount = 0
|
||||
// Why tracked in-loop rather than re-read by key: `tabsByWorktree[activeWorktreeId]`
|
||||
// resolves inherited members for ids like `toString`, and `?.some` would then throw.
|
||||
// Why the id and not a boolean: a falsy-but-valid active id ('') would fail a
|
||||
// truthiness guard below and silently fall back to the first match — the very
|
||||
// misattribution this function exists to remove.
|
||||
let activeOwnerId: string | null = null
|
||||
// Why keys and not entries: entries allocates a pair array per worktree on a path
|
||||
// that runs per tab activation. Own keys stay safe to index by.
|
||||
for (const worktreeId of Object.keys(tabsByWorktree)) {
|
||||
const tabs = tabsByWorktree[worktreeId]
|
||||
if (!tabs.some((tab) => tab.id === tabId)) {
|
||||
continue
|
||||
}
|
||||
ownerCount += 1
|
||||
if (firstOwnerId === null) {
|
||||
firstOwnerId = worktreeId
|
||||
}
|
||||
if (worktreeId === activeWorktreeId) {
|
||||
activeOwnerId = worktreeId
|
||||
}
|
||||
}
|
||||
|
||||
// Why breadcrumb: hydration can retain duplicates after a worktree id change,
|
||||
// but current field reports predate this signal.
|
||||
// Reading it: `ownerCount > 1` is the load-bearing datum; the verdict only
|
||||
// hints at the caller. A sustained repair loop shows up as `true`, since that
|
||||
// effect picks from the active worktree's own list — but it does not prove the
|
||||
// caller, and the repair effect can emit `false` too: its closure holds the
|
||||
// worktree from its render while this runs against live state, so a worktree
|
||||
// switch landing in between (an earlier-flushed effect, or IPC before the
|
||||
// passive flush) reattributes the tab. So `false` covers that race as well as
|
||||
// a deliberate background activation such as jump-to-agent — discard neither.
|
||||
// Why the verdict is in the guard key: it flips under a persisting duplicate,
|
||||
// and coalescing keeps only the newest payload, so one would erase the other.
|
||||
// Still at most two crumbs per tab id.
|
||||
const resolvedToActiveWorktree = activeOwnerId !== null
|
||||
const verdictKey = `${tabId}:${resolvedToActiveWorktree}`
|
||||
if (
|
||||
ownerCount > 1 &&
|
||||
!reportedDuplicateTabVerdicts.has(verdictKey) &&
|
||||
reportedDuplicateTabVerdicts.size < MAX_REPORTED_DUPLICATE_TAB_VERDICTS
|
||||
) {
|
||||
reportedDuplicateTabVerdicts.add(verdictKey)
|
||||
recordRendererCrashBreadcrumb('terminal_tab_id_owned_by_multiple_worktrees', {
|
||||
ownerCount,
|
||||
resolvedToActiveWorktree
|
||||
})
|
||||
}
|
||||
|
||||
if (ownerCount > 1 && activeOwnerId !== null) {
|
||||
return activeOwnerId
|
||||
}
|
||||
return firstOwnerId
|
||||
}
|
||||
|
|
@ -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,6 +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 { resolveActiveTabOwnerWorktreeId } from './active-tab-owner-worktree'
|
||||
import { isSameCodexRestartNoticeAccount } from './codex-restart-notice-account-identity'
|
||||
import {
|
||||
getRepoIdFromWorktreeId,
|
||||
|
|
@ -1903,17 +1904,18 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
|||
},
|
||||
|
||||
setActiveTab: (tabId) => {
|
||||
let tabOwnerWorktreeId: string | null = null
|
||||
set((s) => {
|
||||
// Why: focusing a terminal tab clears its bell, but only for the active worktree — clearing a not-yet-visible background tab (worktree activation / jump-to-agent) would swallow the signal.
|
||||
let tabOwnerWorktreeId: string | null = null
|
||||
for (const [wId, tabs] of Object.entries(s.tabsByWorktree)) {
|
||||
if (tabs.some((t) => t.id === tabId)) {
|
||||
tabOwnerWorktreeId = wId
|
||||
break
|
||||
}
|
||||
}
|
||||
tabOwnerWorktreeId = resolveActiveTabOwnerWorktreeId(
|
||||
s.tabsByWorktree,
|
||||
s.activeWorktreeId,
|
||||
tabId
|
||||
)
|
||||
const isActiveWorktreeTab =
|
||||
tabOwnerWorktreeId !== null && tabOwnerWorktreeId === s.activeWorktreeId
|
||||
const nextUnreadTerminalTabs =
|
||||
tabOwnerWorktreeId === s.activeWorktreeId && s.unreadTerminalTabs[tabId]
|
||||
isActiveWorktreeTab && s.unreadTerminalTabs[tabId]
|
||||
? (() => {
|
||||
const copy = { ...s.unreadTerminalTabs }
|
||||
delete copy[tabId]
|
||||
|
|
@ -1921,20 +1923,37 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
|||
})()
|
||||
: s.unreadTerminalTabs
|
||||
// Why: only pin global activeTabId to active-worktree tabs — markTerminalTabUnread treats it as "the visible tab" and would swallow BELs on a background tab (e.g. jump-to-agent).
|
||||
const isActiveWorktreeTab = tabOwnerWorktreeId === s.activeWorktreeId
|
||||
return {
|
||||
activeTabId: isActiveWorktreeTab ? tabId : s.activeTabId,
|
||||
activeTabIdByWorktree: tabOwnerWorktreeId
|
||||
? { ...s.activeTabIdByWorktree, [tabOwnerWorktreeId]: tabId }
|
||||
: s.activeTabIdByWorktree,
|
||||
// Why: a redundant activation must not reallocate this map — Terminal's
|
||||
// active-terminal repair effect depends on it, so a re-activation that
|
||||
// can't converge activeTabId (tab id reused by an earlier-scanned
|
||||
// worktree) would otherwise re-trigger itself into React error #185.
|
||||
activeTabIdByWorktree:
|
||||
tabOwnerWorktreeId !== null && s.activeTabIdByWorktree[tabOwnerWorktreeId] !== tabId
|
||||
? { ...s.activeTabIdByWorktree, [tabOwnerWorktreeId]: tabId }
|
||||
: s.activeTabIdByWorktree,
|
||||
unreadTerminalTabs: nextUnreadTerminalTabs
|
||||
}
|
||||
})
|
||||
const item = Object.values(get().unifiedTabsByWorktree)
|
||||
.flat()
|
||||
.find((entry) => entry.contentType === 'terminal' && entry.entityId === tabId)
|
||||
const state = get()
|
||||
const ownerUnifiedTabs =
|
||||
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(
|
||||
(entry) => entry.contentType === 'terminal' && entry.entityId === tabId
|
||||
) ??
|
||||
Object.values(state.unifiedTabsByWorktree)
|
||||
.flat()
|
||||
.find((entry) => entry.contentType === 'terminal' && entry.entityId === tabId)
|
||||
if (item) {
|
||||
get().activateTab(item.id)
|
||||
state.activateTab(
|
||||
item.id,
|
||||
tabOwnerWorktreeId !== null ? { worktreeId: tabOwnerWorktreeId } : undefined
|
||||
)
|
||||
}
|
||||
},
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue