From 76dd836ee62f7d53898e804d142df0b83b3773d6 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 1 Aug 2026 04:26:16 -0700 Subject: [PATCH] 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 --- ...ive-terminal-repair-loop.react185.test.tsx | 112 ++++++++++++++++++ src/renderer/src/store/slices/terminals.ts | 11 +- 2 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/components/terminal/active-terminal-repair-loop.react185.test.tsx 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 new file mode 100644 index 00000000000..4da1227e015 --- /dev/null +++ b/src/renderer/src/components/terminal/active-terminal-repair-loop.react185.test.tsx @@ -0,0 +1,112 @@ +/** @vitest-environment happy-dom */ +import { act, useEffect, useMemo } from 'react' +import { createRoot } from 'react-dom/client' +import { afterEach, describe, expect, it } from 'vitest' +import { useAppStore } from '@/store' +import type { TerminalTab } from '../../../../shared/types' +import { resolveRepairedActiveTerminalTabId } from './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 +} + +/** Mirrors the active-terminal repair effect in Terminal.tsx (same deps, same store call). */ +function RepairEffectHarness({ onPass }: { onPass: () => boolean }): 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 ? (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 + }, [ + activeTabId, + activeTabType, + setActiveTab, + tabs, + activeTabIdByWorktree, + renderedActiveWorktreeId + ]) + return null +} + +let cleanup: (() => void) | null = null + +afterEach(() => { + cleanup?.() + cleanup = null +}) + +function measureRepairPasses(): number { + let passes = 0 + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + cleanup = () => { + act(() => root.unmount()) + container.remove() + } + act(() => { + root.render( (passes += 1) <= MAX_PASSES} />) + }) + 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) + }) + + 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. + 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) + }) +}) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 11fbb55c158..36d24c7f505 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -1924,9 +1924,14 @@ export const createTerminalSlice: StateCreator 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 && s.activeTabIdByWorktree[tabOwnerWorktreeId] !== tabId + ? { ...s.activeTabIdByWorktree, [tabOwnerWorktreeId]: tabId } + : s.activeTabIdByWorktree, unreadTerminalTabs: nextUnreadTerminalTabs } })