From 4543bb68263a89ab520cea62ca69d7ac78330dd3 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 29 Jul 2026 02:36:54 -0700 Subject: [PATCH] fix(activity): bound Activity portal readiness oscillation (React #185) (#11326) * fix(activity): bound Activity portal readiness oscillation (React #185) React error #185 is "Maximum update depth exceeded" -- an unbounded render loop, not setState-after-unmount as the crash reports superficially suggest. Verified in react-dom@19.2.7's production bundle: getRootForUpdatedFiber throws Error(formatProdErrorMessage(185)) when 50 < nestedUpdateCount. That frame is the top of all 14 crash stacks across shipped 1.4.156 / 1.4.158 / 1.4.159. nestedUpdateCount and rootWithNestedUpdates are module-level globals keyed on the ROOT, not on a component. One runaway loop saturates the counter and the throw lands on whichever fiber calls setState next, so SortableTab (terminal.workbench), the Radix Presence/MenuPortal (page.settings) and the sidebar.worktrees button are innocent bystanders with misleading component stacks -- four boundaries, one bug. The driver is Activity's portal readiness pass. When 'ready' is unreachable while 'unavailable' stays reachable, updateReadiness flips loading<->unavailable from inside useLayoutEffect -- React's sync lane, the lane that increments nestedUpdateCount -- so it saturates and throws rather than merely being slow. A prior investigation captured this live on Windows via CDP at 239 renders in 847ms. Bound the oscillation: after ACTIVITY_PORTAL_READINESS_MAX_FLIPS consecutive loading<->unavailable transitions the latch parks on 'unavailable' until a real 'ready' arrives. The latch rewrites only the hook's output, never the DOM probe, so a terminal that churns during a slow attach and then genuinely comes up still releases it. Because the counter is global per root, this also protects every innocent bystander in the app -- most of the user-visible value. Extract reconcileActivityPortalThreads and resolveActivityPortalSwap so the portal loop is testable against the real collaborators. Note the reconciliation keeps its worktree+tab comparison deliberately: Terminal mounts one TerminalPane per (worktree, tab) and routes it by worktree+tab, so staging a same-tab pane would leave the staged slot empty and its readiness stuck on 'loading' forever. Same-tab switches swap in place via isolatedPaneKey instead. Co-authored-by: Orca * fix(activity): coalesce portal readiness updates * test(activity): tighten readiness regression coverage --------- Co-authored-by: Orca --- .../activity/ActivityPrototypePage.tsx | 160 ++++---- ...ty-portal-readiness-loop.react185.test.tsx | 366 ++++++++++++++++++ ...ivity-portal-readiness-oscillation.test.ts | 53 +++ .../activity-portal-readiness-oscillation.ts | 39 ++ ...iness-subscription-churn.react185.test.tsx | 80 ++++ .../activity-portal-thread-reconciliation.ts | 82 ++++ 6 files changed, 697 insertions(+), 83 deletions(-) create mode 100644 src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx create mode 100644 src/renderer/src/components/activity/activity-portal-readiness-oscillation.test.ts create mode 100644 src/renderer/src/components/activity/activity-portal-readiness-oscillation.ts create mode 100644 src/renderer/src/components/activity/activity-portal-readiness-subscription-churn.react185.test.tsx create mode 100644 src/renderer/src/components/activity/activity-portal-thread-reconciliation.ts diff --git a/src/renderer/src/components/activity/ActivityPrototypePage.tsx b/src/renderer/src/components/activity/ActivityPrototypePage.tsx index aa7f06c4747..4ee1e7c8be6 100644 --- a/src/renderer/src/components/activity/ActivityPrototypePage.tsx +++ b/src/renderer/src/components/activity/ActivityPrototypePage.tsx @@ -51,6 +51,14 @@ import { setActivityTerminalPortals, type ActivityTerminalPortalTarget } from './activity-terminal-portal' +import { + reconcileActivityPortalThreads, + resolveActivityPortalSwap +} from './activity-portal-thread-reconciliation' +import { + createActivityPortalReadinessLatch, + type ActivityPortalReadinessStatus +} from './activity-portal-readiness-oscillation' import type { Repo, TerminalTab, Worktree } from '../../../../shared/types' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { @@ -131,11 +139,10 @@ type ActivityThreadGroup = { type ActivityTerminalPortalReadiness = { target: HTMLElement | null paneKey: string | null - status: 'loading' | 'ready' | 'unavailable' + status: ActivityPortalReadinessStatus } type ActivityTerminalPortalDomStatus = { - hasSelectedRoot: boolean ready: boolean unavailable: boolean } @@ -251,7 +258,7 @@ function getSelectedActivityTerminalPortalStatus( ): ActivityTerminalPortalDomStatus { const parsed = parsePaneKey(paneKey) if (!parsed) { - return { hasSelectedRoot: false, ready: false, unavailable: true } + return { ready: false, unavailable: true } } let selectedRoot: HTMLElement | null = null for (const candidate of target.querySelectorAll('[data-terminal-tab-id]')) { @@ -261,12 +268,12 @@ function getSelectedActivityTerminalPortalStatus( } } if (!selectedRoot) { - return { hasSelectedRoot: false, ready: false, unavailable: false } + return { ready: false, unavailable: false } } const { foundAnyPane, pane: selectedPane } = findActivityTerminalPane(selectedRoot, parsed.leafId) if (!selectedPane) { - return { hasSelectedRoot: true, ready: false, unavailable: foundAnyPane } + return { ready: false, unavailable: foundAnyPane } } const unavailable = hasInlineDisplayNoneBetween(selectedPane, selectedRoot) @@ -278,13 +285,12 @@ function getSelectedActivityTerminalPortalStatus( selectedPane.querySelector('[data-pty-id]') !== null const hasXtermScreen = selectedPane.querySelector('.xterm-screen') !== null return { - hasSelectedRoot: true, ready: isVisibleRoot && !hasUnisolatedSibling && hasPtyBinding && hasXtermScreen, unavailable } } -function useActivityTerminalPortalStatus( +export function useActivityTerminalPortalStatus( target: HTMLElement | null, paneKey: string | null, forceUnavailable = false @@ -296,75 +302,70 @@ function useActivityTerminalPortalStatus( }) useLayoutEffect(() => { + let disposed = false + let readinessFrame: number | null = null + let pendingStatus: ActivityTerminalPortalReadiness['status'] | null = null + + // Why: subscription churn can otherwise chain layout-effect updates past React's root-wide limit. + const scheduleReadiness = (status: ActivityTerminalPortalReadiness['status']): void => { + if (disposed) { + return + } + pendingStatus = status + if (readinessFrame !== null) { + return + } + readinessFrame = requestAnimationFrame(() => { + readinessFrame = null + const nextStatus = pendingStatus + pendingStatus = null + if (disposed || nextStatus === null) { + return + } + setReadiness((prev) => + prev.target === target && prev.paneKey === paneKey && prev.status === nextStatus + ? prev + : { target, paneKey, status: nextStatus } + ) + }) + } + + const disposeFrame = (): void => { + disposed = true + if (readinessFrame !== null) { + cancelAnimationFrame(readinessFrame) + readinessFrame = null + } + } + if (!target || !paneKey) { - setReadiness((prev) => - prev.target === null && prev.paneKey === null && prev.status === 'loading' - ? prev - : { target: null, paneKey: null, status: 'loading' } - ) - return + scheduleReadiness('loading') + return disposeFrame } if (forceUnavailable) { - setReadiness((prev) => - prev.target === target && prev.paneKey === paneKey && prev.status === 'unavailable' - ? prev - : { target, paneKey, status: 'unavailable' } - ) - return + scheduleReadiness('unavailable') + return disposeFrame } - let disposed = false - let readyFrame: number | null = null - let sawUnreadySelectedRoot = false + const readinessLatch = createActivityPortalReadinessLatch() const updateReadiness = (status: ActivityTerminalPortalReadiness['status']): void => { - setReadiness((prev) => - prev.target === target && prev.paneKey === paneKey && prev.status === status - ? prev - : { target, paneKey, status } - ) - } - - const cancelReadyFrame = (): void => { - if (readyFrame !== null) { - cancelAnimationFrame(readyFrame) - readyFrame = null - } + scheduleReadiness(readinessLatch.next(status)) } const checkReadiness = (): void => { const status = getSelectedActivityTerminalPortalStatus(target, paneKey) if (status.unavailable) { - cancelReadyFrame() updateReadiness('unavailable') return } if (status.ready) { - if (!sawUnreadySelectedRoot) { - cancelReadyFrame() - updateReadiness('ready') - return - } - if (readyFrame !== null) { - return - } - // Why: PTY id can appear before xterm paints replayed output; wait one frame so Activity's cover hides the blank frame. - readyFrame = requestAnimationFrame(() => { - readyFrame = null - if (!disposed && getSelectedActivityTerminalPortalStatus(target, paneKey).ready) { - updateReadiness('ready') - } - }) + updateReadiness('ready') return } - if (status.hasSelectedRoot) { - sawUnreadySelectedRoot = true - } - cancelReadyFrame() updateReadiness('loading') } - updateReadiness('loading') checkReadiness() const observer = new MutationObserver(checkReadiness) @@ -376,8 +377,7 @@ function useActivityTerminalPortalStatus( }) return () => { - disposed = true - cancelReadyFrame() + disposeFrame() observer.disconnect() } }, [target, paneKey, forceUnavailable]) @@ -1512,27 +1512,12 @@ export default function ActivityPrototypePage(): React.JSX.Element { (tab) => tab.id === displayedTabId ) : false - const displayedIsSelectedTerminal = - selectedThread && - displayedThread && - displayedThread.worktree.id === selectedThread.worktree.id && - displayedThread.tab.id === selectedThread.tab.id - const visibleThread = - selectedThread && selectedHasLiveTab - ? displayedThread && displayedHasLiveTab && displayedThread.paneKey !== selectedThread.paneKey - ? displayedIsSelectedTerminal - ? selectedThread - : displayedThread - : selectedThread - : null - const stagedThread = - selectedThread && - selectedHasLiveTab && - visibleThread && - visibleThread.paneKey !== selectedThread.paneKey && - !displayedIsSelectedTerminal - ? selectedThread - : null + const { visibleThread, stagedThread } = reconcileActivityPortalThreads({ + selectedThread, + displayedThread, + selectedHasLiveTab: Boolean(selectedHasLiveTab), + displayedHasLiveTab: Boolean(displayedHasLiveTab) + }) const inactivePortalSlotId = otherActivityTerminalSlot(activePortalSlotId) const portalTargetBySlot = { primary: primaryPortalTargetEl, @@ -1605,18 +1590,27 @@ export default function ActivityPrototypePage(): React.JSX.Element { ]) useLayoutEffect(() => { - if (!selectedThread || !selectedHasLiveTab) { + const swap = resolveActivityPortalSwap({ + selectedThread, + selectedHasLiveTab: Boolean(selectedHasLiveTab), + visibleThread, + stagedThread, + visiblePortalReady, + stagedPortalReady, + stagedPortalUnavailable + }) + if (swap?.kind === 'clear') { setDisplayedPaneKey(null) return } - if (stagedThread && (stagedPortalReady || stagedPortalUnavailable)) { + if (swap?.kind === 'swap-staged') { // Why: a stale selected pane must swap to the unavailable state, not leave the previous pane visible under the new row. setActivePortalSlotId(inactivePortalSlotId) - setDisplayedPaneKey(stagedThread.paneKey) + setDisplayedPaneKey(swap.paneKey) return } - if (!stagedThread && visibleThread?.paneKey === selectedThread.paneKey && visiblePortalReady) { - setDisplayedPaneKey(selectedThread.paneKey) + if (swap?.kind === 'settle-visible') { + setDisplayedPaneKey(swap.paneKey) } }, [ inactivePortalSlotId, diff --git a/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx b/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx new file mode 100644 index 00000000000..11e094472a6 --- /dev/null +++ b/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx @@ -0,0 +1,366 @@ +/** @vitest-environment happy-dom */ +import { act, useLayoutEffect, useState } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +import { useActivityTerminalPortalStatus } from './ActivityPrototypePage' +import { + findActivityTerminalPortal, + type ActivityTerminalPortalTarget +} from './activity-terminal-portal' +import { + reconcileActivityPortalThreads, + resolveActivityPortalSwap, + type ActivityPortalThreadRef +} from './activity-portal-thread-reconciliation' +import type { ActivityPortalReadinessStatus } from './activity-portal-readiness-oscillation' + +const WORKTREE_ID = 'wt-1' +const TAB_ID = 'tab-react185' +const OTHER_TAB_ID = 'tab-react185-other' +const LEAF_A = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa1' +const LEAF_B = 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbb1' +const LEAF_C = 'cccccccc-cccc-4ccc-8ccc-ccccccccccc1' + +const thread = (tabId: string, leafId: string): ActivityPortalThreadRef => ({ + paneKey: `${tabId}:${leafId}`, + worktree: { id: WORKTREE_ID }, + tab: { id: tabId } +}) + +// Same-tab panes share one TerminalPane and swap via isolatedPaneKey. +const PANE_A = thread(TAB_ID, LEAF_A) +const PANE_B = thread(TAB_ID, LEAF_B) +// Cross-tab panes use separate TerminalPanes, so staging applies. +const PANE_C = thread(OTHER_TAB_ID, LEAF_C) + +let root: Root + +afterEach(() => { + act(() => { + root?.unmount() + }) + document.body.replaceChildren() + vi.unstubAllGlobals() +}) + +function installAnimationFrameController(): { + flush: () => Promise + pending: () => number +} { + let nextFrameId = 1 + const callbacks = new Map() + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback): number => { + const frameId = nextFrameId + nextFrameId += 1 + callbacks.set(frameId, callback) + return frameId + }) + vi.stubGlobal('cancelAnimationFrame', (frameId: number): void => { + callbacks.delete(frameId) + }) + return { + async flush() { + const queued = Array.from(callbacks.values()) + callbacks.clear() + await act(async () => { + for (const callback of queued) { + callback(performance.now()) + } + await Promise.resolve() + }) + }, + pending: () => callbacks.size + } +} + +// Models the tab-root DOM and sibling hiding emitted by a portaled TerminalPane. +function renderPortaledTerminalPane(target: HTMLElement, tabId: string, leafIds: string[]): void { + const isolatedLeafId = leafIds[0] + const tabRoot = document.createElement('div') + tabRoot.dataset.terminalTabId = tabId + for (const leafId of leafIds) { + const pane = document.createElement('div') + pane.dataset.leafId = leafId + pane.setAttribute('data-pty-id', `pty-${leafId}`) + pane.appendChild(Object.assign(document.createElement('div'), { className: 'xterm-screen' })) + if (leafId !== isolatedLeafId) { + pane.style.display = 'none' + } + Object.defineProperty(pane, 'getClientRects', { + value: () => (leafId === isolatedLeafId ? [{}] : []), + configurable: true + }) + tabRoot.appendChild(pane) + } + target.replaceChildren(tabRoot) +} + +// Exercises reconciliation, routing, readiness, and swapping on React's sync lane. +async function runActivityPortalPage(args: { + selectedThread: ActivityPortalThreadRef + initialDisplayed: ActivityPortalThreadRef + leafIdsByTabId: Record +}): Promise<{ displayedPaneKey: string | null; renders: number }> { + const { selectedThread, initialDisplayed, leafIdsByTabId } = args + const slotEls = { + primary: document.createElement('div'), + secondary: document.createElement('div') + } + document.body.append(slotEls.primary, slotEls.secondary) + const threadsByPaneKey = new Map( + [selectedThread, initialDisplayed].map((entry) => [entry.paneKey, entry]) + ) + let renders = 0 + let displayedPaneKey: string | null = initialDisplayed.paneKey + + function ActivityPortalPage(): null { + renders += 1 + const [displayed, setDisplayed] = useState(initialDisplayed.paneKey) + const [activeSlotId, setActiveSlotId] = useState<'primary' | 'secondary'>('primary') + displayedPaneKey = displayed + const inactiveSlotId = activeSlotId === 'primary' ? 'secondary' : 'primary' + + const { visibleThread, stagedThread } = reconcileActivityPortalThreads({ + selectedThread, + displayedThread: displayed ? (threadsByPaneKey.get(displayed) ?? null) : null, + selectedHasLiveTab: true, + displayedHasLiveTab: true + }) + + const descriptors: ActivityTerminalPortalTarget[] = [] + if (visibleThread) { + descriptors.push({ + slotId: activeSlotId, + requestToken: `${activeSlotId}:${visibleThread.paneKey}`, + target: slotEls[activeSlotId], + worktreeId: WORKTREE_ID, + tabId: visibleThread.tab.id, + paneKey: visibleThread.paneKey, + active: true + }) + } + if (stagedThread) { + descriptors.push({ + slotId: inactiveSlotId, + requestToken: `${inactiveSlotId}:${stagedThread.paneKey}`, + target: slotEls[inactiveSlotId], + worktreeId: WORKTREE_ID, + tabId: stagedThread.tab.id, + paneKey: stagedThread.paneKey, + active: false + }) + } + + // Match Terminal's one-pane-per-(worktree, tab) routing. + useLayoutEffect(() => { + slotEls.primary.replaceChildren() + slotEls.secondary.replaceChildren() + for (const tabId of Object.keys(leafIdsByTabId)) { + const routed = findActivityTerminalPortal(descriptors, { worktreeId: WORKTREE_ID, tabId }) + if (!routed) { + continue + } + const isolatedLeafId = routed.paneKey.slice(routed.paneKey.indexOf(':') + 1) + const leafIds = leafIdsByTabId[tabId] + renderPortaledTerminalPane(routed.target, tabId, [ + isolatedLeafId, + ...leafIds.filter((leafId) => leafId !== isolatedLeafId) + ]) + } + }) + + const visibleStatus = useActivityTerminalPortalStatus( + slotEls[activeSlotId], + visibleThread?.paneKey ?? null + ) + const stagedStatus = useActivityTerminalPortalStatus( + slotEls[inactiveSlotId], + stagedThread?.paneKey ?? null + ) + + useLayoutEffect(() => { + const swap = resolveActivityPortalSwap({ + selectedThread, + selectedHasLiveTab: true, + visibleThread, + stagedThread, + visiblePortalReady: visibleStatus === 'ready', + stagedPortalReady: stagedStatus === 'ready', + stagedPortalUnavailable: stagedStatus === 'unavailable' + }) + if (swap?.kind === 'clear') { + setDisplayed(null) + return + } + if (swap?.kind === 'swap-staged') { + setActiveSlotId(inactiveSlotId) + setDisplayed(swap.paneKey) + return + } + if (swap?.kind === 'settle-visible') { + setDisplayed(swap.paneKey) + } + // Mirror ActivityPrototypePage's swap dependencies. + }, [inactiveSlotId, stagedStatus, stagedThread, visibleStatus, visibleThread]) + return null + } + + root = createRoot(document.createElement('div')) + await act(async () => { + root.render() + await new Promise((resolve) => setTimeout(resolve, 40)) + }) + return { displayedPaneKey, renders } +} + +describe('Activity portal pane switching', () => { + it('converges on a newly selected pane of the tab already on screen', async () => { + // Same-tab staging would wait forever for a second TerminalPane that never mounts. + const run = (): Promise<{ displayedPaneKey: string | null; renders: number }> => + runActivityPortalPage({ + selectedThread: PANE_B, + initialDisplayed: PANE_A, + leafIdsByTabId: { [TAB_ID]: [LEAF_A, LEAF_B] } + }) + + const result = await run() + expect(result.displayedPaneKey).toBe(PANE_B.paneKey) + expect(result.renders).toBeLessThan(50) + }) + + it('stages and swaps when the selected pane belongs to a different tab', async () => { + const result = await runActivityPortalPage({ + selectedThread: PANE_C, + initialDisplayed: PANE_A, + leafIdsByTabId: { [TAB_ID]: [LEAF_A, LEAF_B], [OTHER_TAB_ID]: [LEAF_C] } + }) + expect(result.displayedPaneKey).toBe(PANE_C.paneKey) + expect(result.renders).toBeLessThan(50) + }) + + // Drive the latch through production wiring because React's nested-update limit is root-wide. + it('bounds a readiness oscillation driven through the real portal-status hook', async () => { + const frames = installAnimationFrameController() + const target = document.createElement('div') + document.body.append(target) + // Alternate hidden and ambiguous DOM states so ready remains unreachable. + const buildRoot = (hiddenLeafId: string | null): void => { + const tabRoot = document.createElement('div') + tabRoot.dataset.terminalTabId = TAB_ID + for (const leafId of [LEAF_A, LEAF_B]) { + const pane = document.createElement('div') + pane.dataset.leafId = leafId + pane.setAttribute('data-pty-id', `pty-${leafId}`) + pane.appendChild( + Object.assign(document.createElement('div'), { className: 'xterm-screen' }) + ) + if (leafId === hiddenLeafId) { + pane.style.display = 'none' + } + Object.defineProperty(pane, 'getClientRects', { value: () => [{}], configurable: true }) + tabRoot.appendChild(pane) + } + target.replaceChildren(tabRoot) + } + buildRoot(LEAF_A) + + let renders = 0 + const statuses: ActivityPortalReadinessStatus[] = [] + // Stop feeding an unlatched spin so failure is immediate and legible. + const RENDER_CAP = 50 + + function ActivityTerminalSlot(): null { + renders += 1 + const status = useActivityTerminalPortalStatus(target, PANE_A.paneKey) + statuses.push(status) + // Reapply opposite isolation so MutationObserver reports opposite readiness. + useLayoutEffect(() => { + if (renders > RENDER_CAP) { + return + } + if (status === 'unavailable') { + buildRoot(null) + } else if (status === 'loading') { + buildRoot(LEAF_A) + } + }) + return null + } + + root = createRoot(document.createElement('div')) + await act(async () => { + root.render() + }) + for (let frame = 0; frame < 20 && frames.pending() > 0; frame += 1) { + await frames.flush() + } + const settledRenders = renders + await frames.flush() + + expect(renders).toBeLessThanOrEqual(RENDER_CAP) + expect(renders).toBe(settledRenders) + expect(frames.pending()).toBe(0) + expect(statuses.at(-1)).toBe('unavailable') + }) + + it('releases a latched readiness once the terminal attaches', async () => { + const target = document.createElement('div') + document.body.append(target) + const buildRoot = (mode: 'hidden' | 'sibling' | 'ready'): void => { + const tabRoot = document.createElement('div') + tabRoot.dataset.terminalTabId = TAB_ID + for (const leafId of [LEAF_A, LEAF_B]) { + const pane = document.createElement('div') + pane.dataset.leafId = leafId + pane.setAttribute('data-pty-id', `pty-${leafId}`) + pane.appendChild( + Object.assign(document.createElement('div'), { className: 'xterm-screen' }) + ) + if (mode === 'hidden' && leafId === LEAF_A) { + pane.style.display = 'none' + } + if (mode === 'ready' && leafId === LEAF_B) { + pane.style.display = 'none' + } + Object.defineProperty(pane, 'getClientRects', { value: () => [{}], configurable: true }) + tabRoot.appendChild(pane) + } + target.replaceChildren(tabRoot) + } + buildRoot('hidden') + + let churning = true + let churns = 0 + const statuses: ActivityPortalReadinessStatus[] = [] + + function ActivityTerminalSlot(): null { + const status = useActivityTerminalPortalStatus(target, PANE_A.paneKey) + statuses.push(status) + useLayoutEffect(() => { + if (!churning || churns > 30) { + return + } + churns += 1 + buildRoot(status === 'unavailable' ? 'sibling' : 'hidden') + }) + return null + } + + root = createRoot(document.createElement('div')) + await act(async () => { + root.render() + await new Promise((resolve) => setTimeout(resolve, 180)) + }) + expect(statuses.at(-1)).toBe('unavailable') + + churning = false + await act(async () => { + buildRoot('ready') + await new Promise((resolve) => setTimeout(resolve, 40)) + }) + expect(statuses.at(-1)).toBe('ready') + }) +}) diff --git a/src/renderer/src/components/activity/activity-portal-readiness-oscillation.test.ts b/src/renderer/src/components/activity/activity-portal-readiness-oscillation.test.ts new file mode 100644 index 00000000000..0a43130b2e4 --- /dev/null +++ b/src/renderer/src/components/activity/activity-portal-readiness-oscillation.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest' +import { + ACTIVITY_PORTAL_READINESS_MAX_FLIPS, + createActivityPortalReadinessLatch +} from './activity-portal-readiness-oscillation' + +describe('createActivityPortalReadinessLatch', () => { + it('latches to unavailable once loading<->unavailable keeps flipping', () => { + const latch = createActivityPortalReadinessLatch() + const seen: string[] = [] + for (let i = 0; i < 40; i += 1) { + seen.push(latch.next(i % 2 === 0 ? 'loading' : 'unavailable')) + } + // Settle well before React's 50 nested sync updates throw #185. + expect(seen.slice(-10).every((status) => status === 'unavailable')).toBe(true) + expect(seen.indexOf('unavailable')).toBeLessThan(ACTIVITY_PORTAL_READINESS_MAX_FLIPS + 2) + }) + + it('passes through a normal loading -> ready startup', () => { + const latch = createActivityPortalReadinessLatch() + expect(latch.next('loading')).toBe('loading') + expect(latch.next('ready')).toBe('ready') + expect(latch.next('ready')).toBe('ready') + }) + + it('does not latch when ready keeps refunding the budget', () => { + const latch = createActivityPortalReadinessLatch() + for (let i = 0; i < 30; i += 1) { + expect(latch.next('loading')).toBe('loading') + expect(latch.next('ready')).toBe('ready') + } + }) + + it('tolerates a few legitimate flips while xterm attaches', () => { + const latch = createActivityPortalReadinessLatch() + expect(latch.next('loading')).toBe('loading') + expect(latch.next('unavailable')).toBe('unavailable') + expect(latch.next('loading')).toBe('loading') + expect(latch.next('ready')).toBe('ready') + }) + + it('releases once the terminal genuinely comes up after a churny attach', () => { + // A slow SSH host may burn the flip budget before attaching successfully. + const latch = createActivityPortalReadinessLatch() + for (let i = 0; i < ACTIVITY_PORTAL_READINESS_MAX_FLIPS + 4; i += 1) { + latch.next(i % 2 === 0 ? 'loading' : 'unavailable') + } + expect(latch.next('unavailable')).toBe('unavailable') + expect(latch.next('ready')).toBe('ready') + expect(latch.next('loading')).toBe('loading') + expect(latch.next('ready')).toBe('ready') + }) +}) diff --git a/src/renderer/src/components/activity/activity-portal-readiness-oscillation.ts b/src/renderer/src/components/activity/activity-portal-readiness-oscillation.ts new file mode 100644 index 00000000000..714c53f8b48 --- /dev/null +++ b/src/renderer/src/components/activity/activity-portal-readiness-oscillation.ts @@ -0,0 +1,39 @@ +export type ActivityPortalReadinessStatus = 'loading' | 'ready' | 'unavailable' + +// Why: stop a fixed subscription from repainting forever after frame coalescing breaks its sync cascade. +export const ACTIVITY_PORTAL_READINESS_MAX_FLIPS = 8 + +export type ActivityPortalReadinessLatch = { + next: (status: ActivityPortalReadinessStatus) => ActivityPortalReadinessStatus +} + +/** Bounds non-ready status flips for one readiness subscription. */ +export function createActivityPortalReadinessLatch(): ActivityPortalReadinessLatch { + let lastStatus: ActivityPortalReadinessStatus | null = null + let flips = 0 + let latched = false + + return { + next(status) { + // Why: a slow terminal may become ready after exhausting the flip budget. + if (status === 'ready') { + lastStatus = status + flips = 0 + latched = false + return status + } + if (latched) { + return 'unavailable' + } + if (lastStatus !== null && lastStatus !== status) { + flips += 1 + } + lastStatus = status + if (flips >= ACTIVITY_PORTAL_READINESS_MAX_FLIPS) { + latched = true + return 'unavailable' + } + return status + } + } +} diff --git a/src/renderer/src/components/activity/activity-portal-readiness-subscription-churn.react185.test.tsx b/src/renderer/src/components/activity/activity-portal-readiness-subscription-churn.react185.test.tsx new file mode 100644 index 00000000000..ff3a085dc79 --- /dev/null +++ b/src/renderer/src/components/activity/activity-portal-readiness-subscription-churn.react185.test.tsx @@ -0,0 +1,80 @@ +/** @vitest-environment happy-dom */ +import { act, useState } from 'react' +import { flushSync } from 'react-dom' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it } from 'vitest' +import { useActivityTerminalPortalStatus } from './ActivityPrototypePage' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +const TAB_ID = 'tab-readiness-churn' +const LEAF_A = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa1' +const LEAF_B = 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbb1' +const LEAF_C = 'cccccccc-cccc-4ccc-8ccc-ccccccccccc1' +const PANE_A = `${TAB_ID}:${LEAF_A}` +const PANE_B = `${TAB_ID}:${LEAF_B}` + +let root: Root + +afterEach(() => { + act(() => { + root?.unmount() + }) + document.body.replaceChildren() +}) + +function buildNeverReadyRoot(target: HTMLElement): void { + const tabRoot = document.createElement('div') + tabRoot.dataset.terminalTabId = TAB_ID + for (const leafId of [LEAF_A, LEAF_C]) { + const pane = document.createElement('div') + pane.dataset.leafId = leafId + pane.setAttribute('data-pty-id', `pty-${leafId}`) + pane.appendChild(Object.assign(document.createElement('div'), { className: 'xterm-screen' })) + Object.defineProperty(pane, 'getClientRects', { value: () => [{}], configurable: true }) + tabRoot.appendChild(pane) + } + target.replaceChildren(tabRoot) +} + +describe('Activity portal readiness subscription churn', () => { + it('coalesces pane-key churn and commits the latest readiness', async () => { + const target = document.createElement('div') + buildNeverReadyRoot(target) + document.body.append(target) + + let selectPane: (paneKey: string) => void = () => {} + let renders = 0 + let status = 'loading' + + function ActivityTerminalSlot(): null { + renders += 1 + const [paneKey, setPaneKey] = useState(PANE_A) + selectPane = setPaneKey + status = useActivityTerminalPortalStatus(target, paneKey) + return null + } + + root = createRoot(document.createElement('div')) + act(() => { + root.render() + }) + + act(() => { + expect(() => { + for (let index = 0; index < 29; index += 1) { + flushSync(() => { + selectPane(index % 2 === 0 ? PANE_B : PANE_A) + }) + } + }).not.toThrow() + }) + expect(renders).toBeLessThanOrEqual(31) + + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve())) + }) + expect(status).toBe('unavailable') + expect(renders).toBeLessThanOrEqual(32) + }) +}) diff --git a/src/renderer/src/components/activity/activity-portal-thread-reconciliation.ts b/src/renderer/src/components/activity/activity-portal-thread-reconciliation.ts new file mode 100644 index 00000000000..a65aefaeb02 --- /dev/null +++ b/src/renderer/src/components/activity/activity-portal-thread-reconciliation.ts @@ -0,0 +1,82 @@ +export type ActivityPortalThreadRef = { + paneKey: string + worktree: { id: string } + tab: { id: string } +} + +export type ActivityPortalReconciliation = { + displayedIsSelectedTerminal: boolean + visibleThread: TThread | null + stagedThread: TThread | null +} + +/** Picks the visible and staged Activity portal threads. */ +export function reconcileActivityPortalThreads(args: { + selectedThread: TThread | null + displayedThread: TThread | null + selectedHasLiveTab: boolean + displayedHasLiveTab: boolean +}): ActivityPortalReconciliation { + const { selectedThread, displayedThread, selectedHasLiveTab, displayedHasLiveTab } = args + // Why: same-tab panes share one TerminalPane and swap through isolatedPaneKey, not staging. + const displayedIsSelectedTerminal = Boolean( + selectedThread && + displayedThread && + displayedThread.worktree.id === selectedThread.worktree.id && + displayedThread.tab.id === selectedThread.tab.id + ) + const visibleThread = + selectedThread && selectedHasLiveTab + ? displayedThread && displayedHasLiveTab && displayedThread.paneKey !== selectedThread.paneKey + ? displayedIsSelectedTerminal + ? selectedThread + : displayedThread + : selectedThread + : null + const stagedThread = + selectedThread && + selectedHasLiveTab && + visibleThread && + visibleThread.paneKey !== selectedThread.paneKey && + !displayedIsSelectedTerminal + ? selectedThread + : null + return { displayedIsSelectedTerminal, visibleThread, stagedThread } +} + +export type ActivityPortalSwap = + | { kind: 'clear' } + | { kind: 'swap-staged'; paneKey: string } + | { kind: 'settle-visible'; paneKey: string } + | null + +/** Decides how displayedPaneKey advances for one commit. */ +export function resolveActivityPortalSwap(args: { + selectedThread: TThread | null + selectedHasLiveTab: boolean + visibleThread: TThread | null + stagedThread: TThread | null + visiblePortalReady: boolean + stagedPortalReady: boolean + stagedPortalUnavailable: boolean +}): ActivityPortalSwap { + const { + selectedThread, + selectedHasLiveTab, + visibleThread, + stagedThread, + visiblePortalReady, + stagedPortalReady, + stagedPortalUnavailable + } = args + if (!selectedThread || !selectedHasLiveTab) { + return { kind: 'clear' } + } + if (stagedThread && (stagedPortalReady || stagedPortalUnavailable)) { + return { kind: 'swap-staged', paneKey: stagedThread.paneKey } + } + if (!stagedThread && visibleThread?.paneKey === selectedThread.paneKey && visiblePortalReady) { + return { kind: 'settle-visible', paneKey: selectedThread.paneKey } + } + return null +}