From ca786b3808771414ea18f8096bafdf0c5e229932 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 3 Aug 2026 23:14:37 -0700 Subject: [PATCH] fix(activity): keep the portal readiness flip budget across slot churn createActivityPortalReadinessLatch() lived inside the readiness layout effect, whose deps are [target, paneKey, forceUnavailable]. The swap effect writes both setActivePortalSlotId and setDisplayedPaneKey, so every oscillation step rebuilt the latch with flips=0 and ACTIVITY_PORTAL_READINESS_MAX_FLIPS never accumulated; the spin then saturated React's root-wide nestedUpdateCount (>50) -> error #185. Hoist the latch onto a ref keyed by tab id, which the swap loop cannot toggle. --- .../activity/ActivityPrototypePage.tsx | 13 ++- ...iness-subscription-churn.react185.test.tsx | 93 ++++++++++++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/components/activity/ActivityPrototypePage.tsx b/src/renderer/src/components/activity/ActivityPrototypePage.tsx index 4ee1e7c8be6..b933c3f53be 100644 --- a/src/renderer/src/components/activity/ActivityPrototypePage.tsx +++ b/src/renderer/src/components/activity/ActivityPrototypePage.tsx @@ -57,6 +57,7 @@ import { } from './activity-portal-thread-reconciliation' import { createActivityPortalReadinessLatch, + type ActivityPortalReadinessLatch, type ActivityPortalReadinessStatus } from './activity-portal-readiness-oscillation' import type { Repo, TerminalTab, Worktree } from '../../../../shared/types' @@ -300,6 +301,12 @@ export function useActivityTerminalPortalStatus( paneKey: null, status: 'loading' }) + // Why: the swap effect churns target and paneKey, so a latch owned by this effect would restart + // its flip budget on every oscillation step; only the tab id is stable across that churn. + const readinessLatchRef = useRef<{ tabId: string | null; latch: ActivityPortalReadinessLatch }>({ + tabId: null, + latch: createActivityPortalReadinessLatch() + }) useLayoutEffect(() => { let disposed = false @@ -347,7 +354,11 @@ export function useActivityTerminalPortalStatus( return disposeFrame } - const readinessLatch = createActivityPortalReadinessLatch() + const tabId = parsePaneKey(paneKey)?.tabId ?? null + if (readinessLatchRef.current.tabId !== tabId) { + readinessLatchRef.current = { tabId, latch: createActivityPortalReadinessLatch() } + } + const readinessLatch = readinessLatchRef.current.latch const updateReadiness = (status: ActivityTerminalPortalReadiness['status']): void => { scheduleReadiness(readinessLatch.next(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 index ff3a085dc79..2605e9d290e 100644 --- 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 @@ -8,11 +8,13 @@ import { useActivityTerminalPortalStatus } from './ActivityPrototypePage' ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true const TAB_ID = 'tab-readiness-churn' +const OTHER_TAB_ID = 'tab-readiness-churn-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 PANE_A = `${TAB_ID}:${LEAF_A}` const PANE_B = `${TAB_ID}:${LEAF_B}` +const OTHER_PANE_A = `${OTHER_TAB_ID}:${LEAF_A}` let root: Root @@ -23,9 +25,13 @@ afterEach(() => { document.body.replaceChildren() }) -function buildNeverReadyRoot(target: HTMLElement): void { +function buildNeverReadyRoot( + target: HTMLElement, + tabId: string = TAB_ID, + { append = false }: { append?: boolean } = {} +): void { const tabRoot = document.createElement('div') - tabRoot.dataset.terminalTabId = TAB_ID + tabRoot.dataset.terminalTabId = tabId for (const leafId of [LEAF_A, LEAF_C]) { const pane = document.createElement('div') pane.dataset.leafId = leafId @@ -34,6 +40,10 @@ function buildNeverReadyRoot(target: HTMLElement): void { Object.defineProperty(pane, 'getClientRects', { value: () => [{}], configurable: true }) tabRoot.appendChild(pane) } + if (append) { + target.appendChild(tabRoot) + return + } target.replaceChildren(tabRoot) } @@ -77,4 +87,83 @@ describe('Activity portal readiness subscription churn', () => { expect(status).toBe('unavailable') expect(renders).toBeLessThanOrEqual(32) }) + + // Why: the swap effect rewrites the slot element and the pane key together, so a latch scoped to + // the subscription is rebuilt with an empty flip budget on every oscillation step. + it('latches readiness when the portal target churns alongside the pane key', async () => { + const targets = [document.createElement('div'), document.createElement('div')] + for (const target of targets) { + buildNeverReadyRoot(target) + document.body.append(target) + } + + let selectFlip: (flip: number) => void = () => {} + let status = 'loading' + + function ActivityTerminalSlot(): null { + const [flip, setFlip] = useState(0) + selectFlip = setFlip + // Alternating pane keys report 'loading' (unisolated sibling) then 'unavailable' (missing leaf). + status = useActivityTerminalPortalStatus(targets[flip % 2], flip % 2 === 0 ? PANE_A : PANE_B) + return null + } + + root = createRoot(document.createElement('div')) + act(() => { + root.render() + }) + + for (let flip = 1; flip <= 40; flip += 1) { + await act(async () => { + selectFlip(flip) + }) + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve())) + }) + } + + // Flip 40 is the 'loading' pairing, so only a latch that survived the churn can report this. + expect(status).toBe('unavailable') + }) + + it('starts a fresh flip budget when the subscription moves to another tab', async () => { + const target = document.createElement('div') + buildNeverReadyRoot(target, TAB_ID) + buildNeverReadyRoot(target, OTHER_TAB_ID, { append: true }) + document.body.append(target) + + let selectPane: (paneKey: string) => void = () => {} + let status = 'loading' + + function ActivityTerminalSlot(): null { + const [paneKey, setPaneKey] = useState(PANE_A) + selectPane = setPaneKey + status = useActivityTerminalPortalStatus(target, paneKey) + return null + } + + root = createRoot(document.createElement('div')) + act(() => { + root.render() + }) + + for (let flip = 0; flip < 40; flip += 1) { + await act(async () => { + selectPane(flip % 2 === 0 ? PANE_B : PANE_A) + }) + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve())) + }) + } + expect(status).toBe('unavailable') + + await act(async () => { + selectPane(OTHER_PANE_A) + }) + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve())) + }) + // A latch shared across tabs would report the previous tab's latched 'unavailable' here. + expect(status).toBe('loading') + }) })