From 67799f40e9b7eca255a1aeebae87619f854f99b8 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:27:36 -0700 Subject: [PATCH] fix(activity): make terminal portal convergence structural --- .../activity/ActivityPrototypePage.tsx | 41 +-- .../activity-portal-churn-budget.test.ts | 73 ----- .../activity/activity-portal-churn-budget.ts | 42 --- ...y-portal-readiness-decay.react185.test.tsx | 111 -------- ...ty-portal-readiness-loop.react185.test.tsx | 266 +----------------- ...ivity-portal-readiness-oscillation.test.ts | 93 ------ .../activity-portal-readiness-oscillation.ts | 40 --- ...iness-subscription-churn.react185.test.tsx | 150 ++-------- ...ivity-portal-thread-reconciliation.test.ts | 3 +- .../activity-portal-thread-reconciliation.ts | 12 +- 10 files changed, 42 insertions(+), 789 deletions(-) delete mode 100644 src/renderer/src/components/activity/activity-portal-churn-budget.test.ts delete mode 100644 src/renderer/src/components/activity/activity-portal-churn-budget.ts delete mode 100644 src/renderer/src/components/activity/activity-portal-readiness-decay.react185.test.tsx delete mode 100644 src/renderer/src/components/activity/activity-portal-readiness-oscillation.test.ts delete mode 100644 src/renderer/src/components/activity/activity-portal-readiness-oscillation.ts diff --git a/src/renderer/src/components/activity/ActivityPrototypePage.tsx b/src/renderer/src/components/activity/ActivityPrototypePage.tsx index 8e732ee044c..b6615729e53 100644 --- a/src/renderer/src/components/activity/ActivityPrototypePage.tsx +++ b/src/renderer/src/components/activity/ActivityPrototypePage.tsx @@ -55,12 +55,6 @@ import { reconcileActivityPortalThreads, resolveActivityPortalSwap } from './activity-portal-thread-reconciliation' -import { - ACTIVITY_PORTAL_READINESS_BURST_WINDOW_MS, - createActivityPortalReadinessLatch, - type ActivityPortalReadinessLatch, - type ActivityPortalReadinessStatus -} from './activity-portal-readiness-oscillation' import type { Repo, TerminalTab, Worktree } from '../../../../shared/types' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { @@ -142,7 +136,7 @@ type ActivityThreadGroup = { type ActivityTerminalPortalReadiness = { target: HTMLElement | null paneKey: string | null - status: ActivityPortalReadinessStatus + status: 'loading' | 'ready' | 'unavailable' } type ActivityTerminalPortalDomStatus = { @@ -291,13 +285,10 @@ export function useActivityTerminalPortalStatus( paneKey: null, status: 'loading' }) - // Why: portal churn replaces every subscription identity, so the burst budget must outlive it. - const readinessLatchRef = useRef(null) useLayoutEffect(() => { let disposed = false let readinessFrame: number | null = null - let readinessReleaseTimer: number | null = null let pendingStatus: ActivityTerminalPortalReadiness['status'] | null = null // Why: coalesce observer bursts and cancel frames from stale portal subscriptions. @@ -330,10 +321,6 @@ export function useActivityTerminalPortalStatus( cancelAnimationFrame(readinessFrame) readinessFrame = null } - if (readinessReleaseTimer !== null) { - window.clearTimeout(readinessReleaseTimer) - readinessReleaseTimer = null - } } if (!target || !paneKey) { @@ -345,35 +332,17 @@ export function useActivityTerminalPortalStatus( return disposeFrame } - const readinessLatch = (readinessLatchRef.current ??= createActivityPortalReadinessLatch()) - - const updateReadiness = (status: ActivityTerminalPortalReadiness['status']): void => { - const nextStatus = readinessLatch.next(status) - scheduleReadiness(nextStatus) - if (readinessReleaseTimer !== null) { - window.clearTimeout(readinessReleaseTimer) - readinessReleaseTimer = null - } - if (nextStatus !== status) { - // Why: a quiet loading pane has no mutation to release the burst latch on its own. - readinessReleaseTimer = window.setTimeout( - checkReadiness, - ACTIVITY_PORTAL_READINESS_BURST_WINDOW_MS - ) - } - } - const checkReadiness = (): void => { const status = getSelectedActivityTerminalPortalStatus(target, paneKey) if (status.unavailable) { - updateReadiness('unavailable') + scheduleReadiness('unavailable') return } if (status.ready) { - updateReadiness('ready') + scheduleReadiness('ready') return } - updateReadiness('loading') + scheduleReadiness('loading') } checkReadiness() @@ -1602,6 +1571,7 @@ export default function ActivityPrototypePage(): React.JSX.Element { // Why: swap-staged makes the displayed thread selected, so this branch cannot repeat by itself. useLayoutEffect(() => { const swap = resolveActivityPortalSwap({ + displayedPaneKey, selectedThread, selectedHasLiveTab: Boolean(selectedHasLiveTab), visibleThread, @@ -1624,6 +1594,7 @@ export default function ActivityPrototypePage(): React.JSX.Element { setDisplayedPaneKey(swap.paneKey) } }, [ + displayedPaneKey, inactivePortalSlotId, selectedHasLiveTab, selectedThread, diff --git a/src/renderer/src/components/activity/activity-portal-churn-budget.test.ts b/src/renderer/src/components/activity/activity-portal-churn-budget.test.ts deleted file mode 100644 index 65b5d264899..00000000000 --- a/src/renderer/src/components/activity/activity-portal-churn-budget.test.ts +++ /dev/null @@ -1,73 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { - createActivityPortalChurnBudget, - type ActivityPortalChurnBudget -} from './activity-portal-churn-budget' - -function budgetAt(clock: { ms: number }): ActivityPortalChurnBudget { - return createActivityPortalChurnBudget({ limit: 3, windowMs: 1_000, now: () => clock.ms }) -} - -describe('createActivityPortalChurnBudget', () => { - it('spends once the window holds a full burst', () => { - const clock = { ms: 0 } - const budget = budgetAt(clock) - expect(budget.record()).toBe(false) - expect(budget.record()).toBe(false) - expect(budget.record()).toBe(true) - clock.ms += 900 - expect(budget.isSpent()).toBe(true) - }) - - it('stays spent while the churn keeps firing', () => { - const clock = { ms: 0 } - const budget = budgetAt(clock) - for (let i = 0; i < 500; i += 1) { - clock.ms += 10 - budget.record() - } - expect(budget.isSpent()).toBe(true) - }) - - it('releases a window after the churn stops', () => { - const clock = { ms: 0 } - const budget = budgetAt(clock) - for (let i = 0; i < 3; i += 1) { - budget.record() - } - clock.ms += 1_000 - expect(budget.isSpent()).toBe(false) - expect(budget.record()).toBe(false) - }) - - it('never spends on events paced slower than the window allows', () => { - // A user hopping between threads must keep the seamless path, however long they keep hopping. - const clock = { ms: 0 } - const budget = budgetAt(clock) - for (let i = 0; i < 100; i += 1) { - clock.ms += 500 - expect(budget.record()).toBe(false) - } - }) - - it('treats a backwards clock as an expired window', () => { - // Date.now() jumps backwards on NTP/sleep-wake. - const clock = { ms: 10_000 } - const budget = budgetAt(clock) - for (let i = 0; i < 3; i += 1) { - budget.record() - } - clock.ms = 5_000 - expect(budget.isSpent()).toBe(false) - }) - - it('clears on demand', () => { - const clock = { ms: 0 } - const budget = budgetAt(clock) - for (let i = 0; i < 3; i += 1) { - budget.record() - } - budget.clear() - expect(budget.isSpent()).toBe(false) - }) -}) diff --git a/src/renderer/src/components/activity/activity-portal-churn-budget.ts b/src/renderer/src/components/activity/activity-portal-churn-budget.ts deleted file mode 100644 index ced7dc31f4a..00000000000 --- a/src/renderer/src/components/activity/activity-portal-churn-budget.ts +++ /dev/null @@ -1,42 +0,0 @@ -/** Sliding-window burst budget for Activity portal readiness churn. */ -export type ActivityPortalChurnBudget = { - /** Records one churn event; returns whether the budget is spent afterwards. */ - record: () => boolean - /** True while the window still holds a full burst. */ - isSpent: () => boolean - clear: () => void -} - -export function createActivityPortalChurnBudget(args: { - limit: number - windowMs: number - now?: () => number -}): ActivityPortalChurnBudget { - const { limit, windowMs, now = () => Date.now() } = args - // Why: only the newest limit events can affect the verdict. - let eventsAt: number[] = [] - - const prune = (at: number): void => { - // Why: clock rollback must not preserve a stale spent budget. - eventsAt = eventsAt.filter((eventAt) => eventAt > at - windowMs && eventAt <= at) - } - - return { - record() { - const at = now() - prune(at) - eventsAt.push(at) - if (eventsAt.length > limit) { - eventsAt.shift() - } - return eventsAt.length >= limit - }, - isSpent() { - prune(now()) - return eventsAt.length >= limit - }, - clear() { - eventsAt = [] - } - } -} diff --git a/src/renderer/src/components/activity/activity-portal-readiness-decay.react185.test.tsx b/src/renderer/src/components/activity/activity-portal-readiness-decay.react185.test.tsx deleted file mode 100644 index f9a6581d608..00000000000 --- a/src/renderer/src/components/activity/activity-portal-readiness-decay.react185.test.tsx +++ /dev/null @@ -1,111 +0,0 @@ -/** @vitest-environment happy-dom */ -import { act } from 'react' -import { createRoot, type Root } from 'react-dom/client' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { useActivityTerminalPortalStatus } from './ActivityPrototypePage' -import { - ACTIVITY_PORTAL_READINESS_BURST_WINDOW_MS, - ACTIVITY_PORTAL_READINESS_MAX_FLIPS -} from './activity-portal-readiness-oscillation' - -;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true - -const TAB_ID = 'tab-readiness-decay' -const LEAF_A = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa1' -const LEAF_B = 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbb1' -const PANE_KEY = `${TAB_ID}:${LEAF_A}` - -let root: Root - -beforeEach(() => vi.useFakeTimers()) - -afterEach(() => { - act(() => root?.unmount()) - vi.useRealTimers() - vi.unstubAllGlobals() - document.body.replaceChildren() -}) - -function installControllers(): { flushFrame: () => Promise; notify: () => void } { - let frame: FrameRequestCallback | null = null - const observers = new Map() - vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { - frame = callback - return 1 - }) - vi.stubGlobal('cancelAnimationFrame', () => { - frame = null - }) - class ControlledMutationObserver implements MutationObserver { - constructor(callback: MutationCallback) { - observers.set(this, callback) - } - observe(): void {} - disconnect(): void { - observers.delete(this) - } - takeRecords(): MutationRecord[] { - return [] - } - } - vi.stubGlobal('MutationObserver', ControlledMutationObserver) - return { - async flushFrame() { - const callback = frame - frame = null - await act(async () => callback?.(performance.now())) - }, - notify() { - for (const [observer, callback] of observers) { - callback([], observer) - } - } - } -} - -function renderPortalDom(target: HTMLElement, loading: boolean): void { - const tabRoot = document.createElement('div') - tabRoot.dataset.terminalTabId = TAB_ID - for (const leafId of loading ? [LEAF_A, LEAF_B] : [LEAF_B]) { - const pane = document.createElement('div') - pane.dataset.leafId = leafId - pane.dataset.ptyId = `pty-${leafId}` - pane.appendChild(Object.assign(document.createElement('div'), { className: 'xterm-screen' })) - Object.defineProperty(pane, 'getClientRects', { value: () => [{}] }) - tabRoot.appendChild(pane) - } - target.replaceChildren(tabRoot) -} - -describe('Activity portal readiness latch decay', () => { - it('rechecks a quiet loading pane when the burst window expires', async () => { - const controls = installControllers() - const target = document.createElement('div') - document.body.append(target) - renderPortalDom(target, false) - let status = 'loading' - - function PortalStatus(): null { - status = useActivityTerminalPortalStatus(target, PANE_KEY) - return null - } - - root = createRoot(document.createElement('div')) - act(() => root.render()) - await controls.flushFrame() - - for (let flip = 0; flip < ACTIVITY_PORTAL_READINESS_MAX_FLIPS * 2; flip += 1) { - renderPortalDom(target, flip % 2 === 0) - controls.notify() - await controls.flushFrame() - } - renderPortalDom(target, true) - controls.notify() - await controls.flushFrame() - expect(status).toBe('unavailable') - - act(() => vi.advanceTimersByTime(ACTIVITY_PORTAL_READINESS_BURST_WINDOW_MS)) - await controls.flushFrame() - expect(status).toBe('loading') - }) -}) 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 index 2389e753bb3..fcc54c00288 100644 --- 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 @@ -1,7 +1,7 @@ /** @vitest-environment happy-dom */ import { act, useLayoutEffect, useState } from 'react' import { createRoot, type Root } from 'react-dom/client' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it } from 'vitest' ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true @@ -15,15 +15,6 @@ import { resolveActivityPortalSwap, type ActivityPortalThreadRef } from './activity-portal-thread-reconciliation' -import { - ACTIVITY_PORTAL_READINESS_MAX_FLIPS, - type ActivityPortalReadinessStatus -} from './activity-portal-readiness-oscillation' - -// Why: re-applying ready DOM never consumes the readiness flip budget (a 'ready' -// status resets the latch), so this retry budget is independent of -// ACTIVITY_PORTAL_READINESS_MAX_FLIPS and must not be derived from it. -const PORTAL_READY_REAPPLY_ATTEMPTS = 32 const WORKTREE_ID = 'wt-1' const TAB_ID = 'tab-react185' @@ -46,109 +37,13 @@ const PANE_C = thread(OTHER_TAB_ID, LEAF_C) let root: Root -// Freeze Date (not timers/rAF) so the latch's flip window cannot expire between two drains on a -// loaded CI machine; the readiness frames are still driven by the controllers below. -beforeEach(() => { - vi.useFakeTimers({ toFake: ['Date'] }) -}) - afterEach(() => { act(() => { root?.unmount() }) - vi.useRealTimers() 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 - } -} - -function installMutationObserverController(): { notify: () => void } { - const callbacks = new Map() - class ControlledMutationObserver implements MutationObserver { - constructor(callback: MutationCallback) { - callbacks.set(this, callback) - } - - observe(): void {} - - disconnect(): void { - callbacks.delete(this) - } - - takeRecords(): MutationRecord[] { - return [] - } - } - vi.stubGlobal('MutationObserver', ControlledMutationObserver) - return { - notify() { - for (const [observer, callback] of callbacks) { - callback([], observer) - } - } - } -} - -async function flushPortalFramesUntil( - frames: ReturnType, - settled: () => boolean -): Promise { - for (let frame = 0; frame < 4 && !settled(); frame += 1) { - await frames.flush() - } -} - -// Drain MutationObserver microtasks and the readiness rAF they schedule. Reports whether the -// drain settled so a caller never reads a transition whose readiness callbacks are still queued. -async function flushPortalReadiness( - frames: ReturnType -): Promise { - for (let attempt = 0; attempt < 8; attempt += 1) { - await act(async () => { - await Promise.resolve() - }) - if (frames.pending() === 0) { - await act(async () => { - await Promise.resolve() - }) - if (frames.pending() === 0) { - return true - } - } - await frames.flush() - } - return frames.pending() === 0 -} - // 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] @@ -256,6 +151,7 @@ async function runActivityPortalPage(args: { useLayoutEffect(() => { const swap = resolveActivityPortalSwap({ + displayedPaneKey: displayed, selectedThread, selectedHasLiveTab: true, visibleThread, @@ -277,7 +173,7 @@ async function runActivityPortalPage(args: { setDisplayed(swap.paneKey) } // Mirror ActivityPrototypePage's swap dependencies. - }, [inactiveSlotId, stagedStatus, stagedThread, visibleStatus, visibleThread]) + }, [displayed, inactiveSlotId, stagedStatus, stagedThread, visibleStatus, visibleThread]) return null } @@ -313,160 +209,4 @@ describe('Activity portal pane switching', () => { 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 frames = installAnimationFrameController() - const mutations = installMutationObserverController() - 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') - - const statuses: ActivityPortalReadinessStatus[] = [] - - function ActivityTerminalSlot(): null { - const status = useActivityTerminalPortalStatus(target, PANE_A.paneKey) - statuses.push(status) - return null - } - - root = createRoot(document.createElement('div')) - await act(async () => { - root.render() - }) - expect(await flushPortalReadiness(frames)).toBe(true) - await flushPortalFramesUntil(frames, () => statuses.at(-1) === 'unavailable') - expect(statuses.at(-1)).toBe('unavailable') - - // Feed each DOM state separately and keep going until sibling DOM reports latched - // unavailable. A fixed 9-flip budget flakes when CI load drops MutationObserver - // deliveries below ACTIVITY_PORTAL_READINESS_MAX_FLIPS transitions. - let sawSiblingLoading = false - let sawLatchedSibling = false - for ( - let flip = 0; - flip < ACTIVITY_PORTAL_READINESS_MAX_FLIPS * 4 && !sawLatchedSibling; - flip += 1 - ) { - const mode = flip % 2 === 0 ? 'sibling' : 'hidden' - const statusesBefore = statuses.length - await act(async () => { - buildRoot(mode) - mutations.notify() - }) - expect(await flushPortalReadiness(frames)).toBe(true) - if (mode !== 'sibling') { - continue - } - // Transition-local evidence: an unlatched subscription answers sibling DOM with 'loading', - // so the latch is only proven once a sibling transition that previously emitted 'loading' - // stops doing so and leaves 'unavailable' standing. - if (statuses.slice(statusesBefore).includes('loading')) { - sawSiblingLoading = true - } else if (sawSiblingLoading && statuses.at(-1) === 'unavailable') { - sawLatchedSibling = true - } - } - expect(sawLatchedSibling).toBe(true) - expect(statuses.at(-1)).toBe('unavailable') - - // Why: under CI load MutationObserver may miss one replaceChildren; re-apply ready DOM - // and keep draining until attach is observed. - let sawReady = false - for (let attempt = 0; attempt < PORTAL_READY_REAPPLY_ATTEMPTS && !sawReady; attempt += 1) { - await act(async () => { - buildRoot('ready') - mutations.notify() - }) - expect(await flushPortalReadiness(frames)).toBe(true) - await flushPortalFramesUntil(frames, () => statuses.at(-1) === 'ready') - sawReady = statuses.at(-1) === 'ready' - } - 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 deleted file mode 100644 index ffb068c88ee..00000000000 --- a/src/renderer/src/components/activity/activity-portal-readiness-oscillation.test.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { - ACTIVITY_PORTAL_READINESS_BURST_WINDOW_MS, - 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') - }) - - it('holds the latch for as long as the DOM keeps flipping', () => { - // The field loop ran 239 renders in 847ms, so the window never empties while it spins. - const clock = { ms: 0 } - const latch = createActivityPortalReadinessLatch(() => clock.ms) - const seen: string[] = [] - for (let i = 0; i < 240; i += 1) { - clock.ms += 4 - seen.push(latch.next(i % 2 === 0 ? 'loading' : 'unavailable')) - } - expect( - seen.slice(ACTIVITY_PORTAL_READINESS_MAX_FLIPS + 2).every((s) => s === 'unavailable') - ).toBe(true) - }) - - it('releases a latched pane once the churn stops, so a slow attach still reports loading', () => { - // A latched pane must not answer for the next pane the Activity slot shows (SSH panes take seconds). - const clock = { ms: 0 } - const latch = createActivityPortalReadinessLatch(() => clock.ms) - for (let i = 0; i < ACTIVITY_PORTAL_READINESS_MAX_FLIPS * 2; i += 1) { - clock.ms += 5 - latch.next(i % 2 === 0 ? 'loading' : 'unavailable') - } - expect(latch.next('loading')).toBe('unavailable') - clock.ms += ACTIVITY_PORTAL_READINESS_BURST_WINDOW_MS - expect(latch.next('loading')).toBe('loading') - }) - - it('ignores flips paced by a human clicking between threads', () => { - // The latch outlives the subscription, so thread-hopping must never spend the burst budget. - const clock = { ms: 0 } - const latch = createActivityPortalReadinessLatch(() => clock.ms) - for (let i = 0; i < ACTIVITY_PORTAL_READINESS_MAX_FLIPS * 4; i += 1) { - clock.ms += 500 - expect(latch.next(i % 2 === 0 ? 'loading' : 'unavailable')).toBe( - i % 2 === 0 ? 'loading' : 'unavailable' - ) - } - }) -}) diff --git a/src/renderer/src/components/activity/activity-portal-readiness-oscillation.ts b/src/renderer/src/components/activity/activity-portal-readiness-oscillation.ts deleted file mode 100644 index e632bf7d8bd..00000000000 --- a/src/renderer/src/components/activity/activity-portal-readiness-oscillation.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { createActivityPortalChurnBudget } from './activity-portal-churn-budget' - -export type ActivityPortalReadinessStatus = 'loading' | 'ready' | 'unavailable' - -// Why: stop a subscription from repainting forever after frame coalescing breaks its sync cascade. -export const ACTIVITY_PORTAL_READINESS_MAX_FLIPS = 8 -/** Separates render-cadence churn from deliberate thread selection. */ -export const ACTIVITY_PORTAL_READINESS_BURST_WINDOW_MS = 500 - -export type ActivityPortalReadinessLatch = { - next: (status: ActivityPortalReadinessStatus) => ActivityPortalReadinessStatus -} - -/** Bounds non-ready flips across every identity hosted by one portal slot. */ -export function createActivityPortalReadinessLatch( - now: () => number = () => Date.now() -): ActivityPortalReadinessLatch { - let lastStatus: ActivityPortalReadinessStatus | null = null - const flipBudget = createActivityPortalChurnBudget({ - limit: ACTIVITY_PORTAL_READINESS_MAX_FLIPS, - windowMs: ACTIVITY_PORTAL_READINESS_BURST_WINDOW_MS, - now - }) - - return { - next(status) { - // Why: a slow terminal may become ready after exhausting the flip budget. - if (status === 'ready') { - lastStatus = status - flipBudget.clear() - return status - } - // Why: underlying flips must keep the latch engaged even while its output is stable. - const flipped = lastStatus !== null && lastStatus !== status - lastStatus = status - const spent = flipped ? flipBudget.record() : flipBudget.isSpent() - return spent ? 'unavailable' : 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 d8b9c3caff4..172b07e189b 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 @@ -2,40 +2,31 @@ import { act, useState } from 'react' import { flushSync } from 'react-dom' import { createRoot, type Root } from 'react-dom/client' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { useActivityTerminalPortalStatus } from './ActivityPrototypePage' -import { ACTIVITY_PORTAL_READINESS_MAX_FLIPS } from './activity-portal-readiness-oscillation' ;(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_B = `${OTHER_TAB_ID}:${LEAF_B}` let root: Root -// Freeze Date (not timers/rAF, which still drive the readiness frames) so the latch's flip window is -// decided by the churn under test rather than by how slowly the machine runs it. -beforeEach(() => { - vi.useFakeTimers({ toFake: ['Date'] }) -}) - afterEach(() => { act(() => { root?.unmount() }) - vi.useRealTimers() document.body.replaceChildren() + vi.restoreAllMocks() }) -function buildNeverReadyRoot(target: HTMLElement, tabId: string = TAB_ID): void { +function buildNeverReadyRoot(target: HTMLElement): void { const tabRoot = document.createElement('div') - tabRoot.dataset.terminalTabId = tabId + tabRoot.dataset.terminalTabId = TAB_ID for (const leafId of [LEAF_A, LEAF_C]) { const pane = document.createElement('div') pane.dataset.leafId = leafId @@ -47,8 +38,7 @@ function buildNeverReadyRoot(target: HTMLElement, tabId: string = TAB_ID): void target.replaceChildren(tabRoot) } -/** Lets the readiness rAF this churn step scheduled land in React state. */ -async function drainReadinessFrame(): Promise { +async function settleReadinessFrame(): Promise { await act(async () => { await new Promise((resolve) => requestAnimationFrame(() => resolve())) }) @@ -131,141 +121,41 @@ describe('Activity portal readiness subscription churn', () => { }) expect(renders).toBeLessThanOrEqual(31) - await act(async () => { - await new Promise((resolve) => requestAnimationFrame(() => resolve())) - }) + await settleReadinessFrame() expect(status).toBe('unavailable') expect(renders).toBeLessThanOrEqual(32) }) - // Asserts the loop-terminating property, not just a final value: the status that drives the page's - // swap effect (stagedPortalUnavailable) must stop alternating part-way through the churn and stay - // put. While it alternates, every step re-targets both readiness subscriptions and re-stages. - function expectReadinessStopsAlternating(statuses: string[]): void { - // Sanity: the fixture really does alternate early, so the run below is the latch, not a rig. - const head = statuses.slice(0, ACTIVITY_PORTAL_READINESS_MAX_FLIPS) - expect(new Set(head)).toEqual(new Set(['loading', 'unavailable'])) - // Termination: the churn keeps flipping the DOM but the status the swap effect reads stops moving. - const tail = statuses.slice(ACTIVITY_PORTAL_READINESS_MAX_FLIPS) - expect(tail.length).toBeGreaterThan(20) - expect(new Set(tail)).toEqual(new Set(['unavailable'])) - } - - // Why: staging only happens across tabs (reconcileActivityPortalThreads returns no stagedThread for - // same-tab panes), so the production swap moves slot element, pane key AND tab id in lockstep -- - // every identity a subscription-scoped latch could key on. - it('stops alternating readiness when the slot, pane key and tab id all churn together', async () => { - const targets = [document.createElement('div'), document.createElement('div')] - buildNeverReadyRoot(targets[0], TAB_ID) - buildNeverReadyRoot(targets[1], OTHER_TAB_ID) - for (const target of targets) { - document.body.append(target) - } - - let selectFlip: (flip: number) => void = () => {} - let status = 'loading' - const settledStatuses: string[] = [] - - function ActivityTerminalSlot(): null { - const [flip, setFlip] = useState(0) - selectFlip = setFlip - // Alternating panes report 'loading' (unisolated sibling) then 'unavailable' (missing leaf). - status = useActivityTerminalPortalStatus( - targets[flip % 2], - flip % 2 === 0 ? PANE_A : OTHER_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 drainReadinessFrame() - settledStatuses.push(status) - } - - expectReadinessStopsAlternating(settledStatuses) - }) - - it('stops alternating readiness when only the pane key churns within one tab', async () => { + it('reports a later loading pane truthfully after rapid readiness changes', async () => { + vi.spyOn(Date, 'now').mockReturnValue(1_000) const target = document.createElement('div') - buildNeverReadyRoot(target, TAB_ID) + buildNeverReadyRoot(target) document.body.append(target) let selectPane: (paneKey: string) => void = () => {} let status = 'loading' - const settledStatuses: string[] = [] function ActivityTerminalSlot(): null { - const [paneKey, setPaneKey] = useState(PANE_A) + const [paneKey, setPaneKey] = useState(PANE_B) selectPane = setPaneKey status = useActivityTerminalPortalStatus(target, paneKey) return null } root = createRoot(document.createElement('div')) - act(() => { - root.render() - }) + act(() => root.render()) + await settleReadinessFrame() + expect(status).toBe('unavailable') - for (let flip = 0; flip < 40; flip += 1) { - await act(async () => { - selectPane(flip % 2 === 0 ? PANE_B : PANE_A) - }) - await drainReadinessFrame() - settledStatuses.push(status) + for (let index = 0; index < 10; index += 1) { + const paneKey = index % 2 === 0 ? PANE_A : PANE_B + act(() => selectPane(paneKey)) + await settleReadinessFrame() + expect(status).toBe(paneKey === PANE_A ? 'loading' : 'unavailable') } - expectReadinessStopsAlternating(settledStatuses) - }) - - // Why: the flip budget outlives the subscription, so a burst window wide enough to cover ordinary - // clicking would let one user's thread-hopping answer 'unavailable' for the next healthy pane. - it('does not answer for a later attaching pane after human-paced selections', async () => { - const churnTarget = document.createElement('div') - buildNeverReadyRoot(churnTarget, TAB_ID) - const attachingTarget = document.createElement('div') - buildNeverReadyRoot(attachingTarget, OTHER_TAB_ID) - for (const target of [churnTarget, attachingTarget]) { - document.body.append(target) - } - - type SelectedPane = { target: HTMLElement; paneKey: string } - let selectPane: (pane: SelectedPane) => void = () => {} - let status = 'loading' - - function ActivityTerminalSlot(): null { - const [pane, setPane] = useState({ target: churnTarget, paneKey: PANE_A }) - selectPane = setPane - status = useActivityTerminalPortalStatus(pane.target, pane.paneKey) - return null - } - - root = createRoot(document.createElement('div')) - act(() => { - root.render() - }) - - // Two clicks a second between a live thread ('loading') and a retained one ('unavailable'). - for (let click = 1; click <= 10; click += 1) { - vi.setSystemTime(click * 500) - await act(async () => { - selectPane({ target: churnTarget, paneKey: click % 2 === 0 ? PANE_A : PANE_B }) - }) - await drainReadinessFrame() - } - - vi.setSystemTime(5_500) - await act(async () => { - selectPane({ target: attachingTarget, paneKey: `${OTHER_TAB_ID}:${LEAF_A}` }) - }) - await drainReadinessFrame() + act(() => selectPane(PANE_A)) + await settleReadinessFrame() expect(status).toBe('loading') }) }) diff --git a/src/renderer/src/components/activity/activity-portal-thread-reconciliation.test.ts b/src/renderer/src/components/activity/activity-portal-thread-reconciliation.test.ts index 8da62b43e56..690b8f5a6a1 100644 --- a/src/renderer/src/components/activity/activity-portal-thread-reconciliation.test.ts +++ b/src/renderer/src/components/activity/activity-portal-thread-reconciliation.test.ts @@ -38,6 +38,7 @@ function replaySwapCascade(args: { }) // Most permissive readiness the page can hand in, so no swap is withheld. const swap = resolveActivityPortalSwap({ + displayedPaneKey, selectedThread: args.selectedThread, selectedHasLiveTab: true, visibleThread, @@ -74,7 +75,7 @@ describe('resolveActivityPortalSwap cascade', () => { `${label}: ${kinds.join(',')}` ).toBeLessThanOrEqual(1) // And the cascade settles: no write at all once displayedPaneKey caught up. - expect(kinds.at(-1), `${label}: ${kinds.join(',')}`).toBe('settle-visible') + expect(kinds.at(-1), `${label}: ${kinds.join(',')}`).toBe('none') } } }) diff --git a/src/renderer/src/components/activity/activity-portal-thread-reconciliation.ts b/src/renderer/src/components/activity/activity-portal-thread-reconciliation.ts index a65aefaeb02..a2dc25ddb0c 100644 --- a/src/renderer/src/components/activity/activity-portal-thread-reconciliation.ts +++ b/src/renderer/src/components/activity/activity-portal-thread-reconciliation.ts @@ -52,6 +52,7 @@ export type ActivityPortalSwap = /** Decides how displayedPaneKey advances for one commit. */ export function resolveActivityPortalSwap(args: { + displayedPaneKey: string | null selectedThread: TThread | null selectedHasLiveTab: boolean visibleThread: TThread | null @@ -61,6 +62,7 @@ export function resolveActivityPortalSwap