mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
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>
This commit is contained in:
@@ -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(<RepairEffectHarness onPass={() => (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)
|
||||
})
|
||||
})
|
||||
@@ -1924,9 +1924,14 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
||||
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
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user