From fff04a8d88fb535916008a42d2ef682a3568eb1e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:03:05 -0700 Subject: [PATCH] fix(terminal): re-run deferred tab admission when a plan installs mid-activation (#20176) planColdActivationTabDeferral can install an empty allowed set, deferring every tab so the pane filter renders none. The drain that undoes that, useActivationDeferredTabAdmission, depends only on backgroundMountRevision and renderedActiveWorktreeId while reading the deferred set from a mutable ref, and the install bumps neither: the only revision producers are the drain itself and the background-mount event path, which the activation plan never reaches. So this pass strands the workspace with zero panes: the worktree is already rendered-active while the startup gate is closed, which resets lastActivationWorktreeIdRef, then the gate opens on the same worktree and installs a plan. Nothing re-runs the drain until the user switches workspaces and back. The hook's own comment anticipates this launch shape and relies on re-reading on growth, but that re-read only happens on a dep change. applyTerminalColdActivation now returns activationDeferralPlanRevision, backed by a ref in the parking foundation and incremented only when the plan actually installs, which the admission effect takes as a dep. A ref rather than state because the pass runs during render, where a setState would be a render-phase update. The 4-tab admission cap is deliberately untouched: it reproduces the warm set an eager activation used to mount, so steady-state pane and WebGL-context population is unchanged. Ablated: with the change reverted the new suite's drain case fails on the deferred set surviving the timers; the precondition and the away-and-back control pass either way. Co-authored-by: Merge Sim (cherry picked from commit 353a1c8038ae70547a78c07eafa481f7f898be2b) --- .../components/terminal-cold-activation.ts | 13 +- .../terminal-workspace-surface-ids.test.tsx | 1 + ...cold-activation-deferral-stranding.test.ts | 230 ++++++++++++++++++ .../use-activation-deferred-tab-admission.ts | 7 +- .../use-terminal-parking-foundation.ts | 6 +- 5 files changed, 253 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/components/terminal/cold-activation-deferral-stranding.test.ts diff --git a/src/renderer/src/components/terminal-cold-activation.ts b/src/renderer/src/components/terminal-cold-activation.ts index d57cb82d766..8b6e96467ee 100644 --- a/src/renderer/src/components/terminal-cold-activation.ts +++ b/src/renderer/src/components/terminal-cold-activation.ts @@ -16,6 +16,7 @@ import type { TerminalParkingFoundation } from './use-terminal-parking-foundatio export function applyTerminalColdActivation(controller: TerminalParkingFoundation) { const { + activationDeferralPlanRevisionRef, activationDeferredMountTabIdsByWorktreeRef, activeGroupIdByWorktree, activeTabId, @@ -96,7 +97,7 @@ export function applyTerminalColdActivation(controller: TerminalParkingFoundatio if (lastActivationWorktreeIdRef.current !== renderedActiveWorktreeId) { lastActivationWorktreeIdRef.current = renderedActiveWorktreeId const tabById = new Map(worktreeTabs.map((tab) => [tab.id, tab])) - planColdActivationTabDeferral({ + const installedDeferralPlan = planColdActivationTabDeferral({ restrictions: backgroundMountTabIdsByWorktreeRef.current, deferredMountTabIdsByWorktree: activationDeferredMountTabIdsByWorktreeRef.current, worktreeId: renderedActiveWorktreeId, @@ -118,6 +119,11 @@ export function applyTerminalColdActivation(controller: TerminalParkingFoundatio }, immediateTabIds }) + // Why: the install mutates only refs, so without a returned revision the + // admission drain's effect deps never change and the plan strands. + if (installedDeferralPlan) { + activationDeferralPlanRevisionRef.current += 1 + } } else if (!coldActivationDeferralEnabled || !activationHostSupportsDeferral) { backgroundMountTabIdsByWorktreeRef.current.delete(renderedActiveWorktreeId) activationDeferredMountTabIdsByWorktreeRef.current.delete(renderedActiveWorktreeId) @@ -165,7 +171,10 @@ export function applyTerminalColdActivation(controller: TerminalParkingFoundatio groupsByWorktree, activeGroupIdByWorktree ) - return { anyMountedWorktreeHasLayout } + return { + anyMountedWorktreeHasLayout, + activationDeferralPlanRevision: activationDeferralPlanRevisionRef.current + } } export type TerminalColdActivationController = TerminalParkingFoundation & diff --git a/src/renderer/src/components/terminal-workspace-surface-ids.test.tsx b/src/renderer/src/components/terminal-workspace-surface-ids.test.tsx index 59984d7fe06..f93b06a3b05 100644 --- a/src/renderer/src/components/terminal-workspace-surface-ids.test.tsx +++ b/src/renderer/src/components/terminal-workspace-surface-ids.test.tsx @@ -78,6 +78,7 @@ describe('workspace surface ids', () => { const ids = Array.from({ length: 423 }, (_, index) => `repo::/worktree-${index}`) const { surfaces, mapCalls } = countingSurfaces(ids) const controller = { + activationDeferralPlanRevisionRef: { current: 0 }, activationDeferredMountTabIdsByWorktreeRef: { current: new Map() }, activeGroupIdByWorktree: {}, activeTabId: null, diff --git a/src/renderer/src/components/terminal/cold-activation-deferral-stranding.test.ts b/src/renderer/src/components/terminal/cold-activation-deferral-stranding.test.ts new file mode 100644 index 00000000000..89737f42991 --- /dev/null +++ b/src/renderer/src/components/terminal/cold-activation-deferral-stranding.test.ts @@ -0,0 +1,230 @@ +// @vitest-environment happy-dom + +/** + * Defect under test: a startup-gate-open pass can install an activation + * deferral plan (restrictions.set(worktree, EMPTY set) — every tab deferred, + * zero panes rendered) by mutating only refs. The admission drain's effect + * used to depend only on [backgroundMountRevision, renderedActiveWorktreeId], + * and the only producers of backgroundMountRevision are the drain itself and + * the background-mount EVENT path — never the activation plan. Neither dep + * changes on the gate-open pass, so the plan stranded every tab unmounted + * until the user switched workspaces and back. The fix returns + * activationDeferralPlanRevision from applyTerminalColdActivation as a third + * dep, bumped only when a plan actually installs. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, renderHook } from '@testing-library/react' +import { useRef, useState } from 'react' +import { useAppStore } from '@/store' +import { applyTerminalColdActivation } from '../terminal-cold-activation' +import { useActivationDeferredTabAdmission } from './use-activation-deferred-tab-admission' +import { shouldMountBackgroundWorktreeTab } from './background-terminal-worktree-mount' +import { + clearTerminalProviderSnapshotCapabilities, + synchronizeTerminalProviderSnapshotCapabilities, + terminalProviderHasAuthoritativeSnapshot +} from './terminal-provider-snapshot-capability' +import { + canWatcherCoverParkedTerminalTab, + captureParkedTerminalPaneCandidates +} from '../terminal-pane/terminal-parked-tab-watchers' +import { capturedPanesByTabId } from '../terminal-pane/terminal-parked-watcher-registry' +import type { TerminalTab } from '../../../../shared/terminal-tab-types' +import type { TerminalParkingFoundation } from '../use-terminal-parking-foundation' + +const WORKTREE_ID = 'repo::/worktree' +const OTHER_WORKTREE_ID = 'repo::/other-worktree' +const TAB_1 = 'tab-1' +const TAB_2 = 'tab-2' +const PTY_1 = `${WORKTREE_ID}@@session-1` +const PTY_2 = `${WORKTREE_ID}@@session-2` +const LEAF_1 = '11111111-1111-4111-8111-111111111111' +const LEAF_2 = '22222222-2222-4222-8222-222222222222' +const SURFACE_IDS = [WORKTREE_ID, OTHER_WORKTREE_ID] + +const initialState = useAppStore.getInitialState() +const originalRequestIdle = globalThis.requestIdleCallback +const originalCancelIdle = globalThis.cancelIdleCallback + +function terminalTab(id: string, ptyId: string): TerminalTab { + return { + id, + ptyId, + worktreeId: WORKTREE_ID, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +/** Seeds the real store and primes real authoritative-snapshot capabilities. */ +async function seedDeferrableWorktree(): Promise { + useAppStore.setState({ + tabsByWorktree: { [WORKTREE_ID]: [terminalTab(TAB_1, PTY_1), terminalTab(TAB_2, PTY_2)] } + }) + captureParkedTerminalPaneCandidates(TAB_1, WORKTREE_ID, [ + { ptyId: PTY_1, paneId: 1, leafId: LEAF_1, drivesTabTitle: true } + ]) + captureParkedTerminalPaneCandidates(TAB_2, WORKTREE_ID, [ + { ptyId: PTY_2, paneId: 2, leafId: LEAF_2, drivesTabTitle: true } + ]) + await synchronizeTerminalProviderSnapshotCapabilities([PTY_1, PTY_2], async (ids) => + ids.map((id) => ({ id, authoritative: true })) + ) +} + +type HarnessProps = { worktreeId: string | null; gateOpen: boolean } + +/** Mirrors use-terminal-controller.ts:30-32: cold activation during render, then admission. */ +function useStrandingHarness(props: HarnessProps) { + const backgroundMountTabIdsByWorktreeRef = useRef(new Map>()) + const activationDeferredMountTabIdsByWorktreeRef = useRef(new Map>()) + const lastActivationWorktreeIdRef = useRef(null) + const mountedWorktreeIdsRef = useRef(new Set()) + const activationDeferralPlanRevisionRef = useRef(0) + const [backgroundMountRevision, setBackgroundMountRevision] = useState(0) + const foundation = { + activationDeferralPlanRevisionRef, + activationDeferredMountTabIdsByWorktreeRef, + activeGroupIdByWorktree: {}, + activeTabId: null, + activeTabIdByWorktree: {}, + activeWorktreeDeferralHostId: 'local', + activityTerminalPortals: [], + backgroundMountRevision, + backgroundMountTabIdsByWorktreeRef, + groupsByWorktree: {}, + hydrationSucceeded: props.gateOpen, + lastActivationWorktreeIdRef, + layoutByWorktree: {}, + mountedWorktreeIdsRef, + pairedRuntimeParkingEnvironmentIds: new Set(), + pendingStartupByTabId: {}, + renderedActiveWorktreeId: props.worktreeId, + setBackgroundMountRevision, + startupWorktreeRefreshCompleted: props.gateOpen, + tabsByWorktree: useAppStore.getState().tabsByWorktree, + terminalParkingEnabled: true, + terminalTitleSnapshotAuthorityEnabled: true, + workspaceSessionReady: props.gateOpen, + workspaceSurfaceIds: SURFACE_IDS, + workspaceSurfaceIdSet: new Set(SURFACE_IDS) + } as unknown as TerminalParkingFoundation + const coldActivation = Object.assign(foundation, applyTerminalColdActivation(foundation)) + useActivationDeferredTabAdmission(coldActivation) + return { activationDeferredMountTabIdsByWorktreeRef, backgroundMountTabIdsByWorktreeRef } +} + +/** Fires the timer-fallback admission chain: one tab admitted per pass. */ +function drainIdleAdmissions(passes: number): void { + for (let index = 0; index < passes; index += 1) { + act(() => { + vi.advanceTimersByTime(1) + }) + } +} + +describe('cold-activation deferral stranding', () => { + beforeEach(() => { + useAppStore.setState(initialState, true) + // Deterministic drain: force scheduleActivationDeferredAdmission onto timers. + // @ts-expect-error -- exercising the no-requestIdleCallback environment + globalThis.requestIdleCallback = undefined + // @ts-expect-error -- exercising the no-requestIdleCallback environment + globalThis.cancelIdleCallback = undefined + }) + + afterEach(() => { + cleanup() + vi.useRealTimers() + globalThis.requestIdleCallback = originalRequestIdle + globalThis.cancelIdleCallback = originalCancelIdle + clearTerminalProviderSnapshotCapabilities() + capturedPanesByTabId.clear() + useAppStore.setState(initialState, true) + }) + + it('precondition: both seeded tabs are deferrable under the real coverage predicate', async () => { + await seedDeferrableWorktree() + const [first, second] = useAppStore.getState().tabsByWorktree[WORKTREE_ID]! + expect( + canWatcherCoverParkedTerminalTab( + WORKTREE_ID, + first!, + terminalProviderHasAuthoritativeSnapshot + ) + ).toBe(true) + expect( + canWatcherCoverParkedTerminalTab( + WORKTREE_ID, + second!, + terminalProviderHasAuthoritativeSnapshot + ) + ).toBe(true) + }) + + it('drains a plan installed by the gate-open pass without the active worktree changing', async () => { + await seedDeferrableWorktree() + vi.useFakeTimers() + // Pass 1: worktree already rendered-active while the startup gate is + // closed — the else branch resets lastActivationWorktreeIdRef to null. + const { result, rerender } = renderHook((props: HarnessProps) => useStrandingHarness(props), { + initialProps: { worktreeId: WORKTREE_ID, gateOpen: false } + }) + expect(result.current.activationDeferredMountTabIdsByWorktreeRef.current.size).toBe(0) + + // Pass 2: gate opens with the SAME rendered-active worktree; the plan + // installs an empty allowed set — every tab deferred, zero panes. + rerender({ worktreeId: WORKTREE_ID, gateOpen: true }) + const restrictions = result.current.backgroundMountTabIdsByWorktreeRef.current + const deferred = result.current.activationDeferredMountTabIdsByWorktreeRef.current + expect(deferred.get(WORKTREE_ID)?.size).toBe(2) + expect(shouldMountBackgroundWorktreeTab(restrictions.get(WORKTREE_ID) ?? null, TAB_1)).toBe( + false + ) + expect(shouldMountBackgroundWorktreeTab(restrictions.get(WORKTREE_ID) ?? null, TAB_2)).toBe( + false + ) + + // The fix: the install bumps activationDeferralPlanRevision, so the + // admission effect re-runs and drains — no worktree switch required. + drainIdleAdmissions(3) + expect(deferred.has(WORKTREE_ID)).toBe(false) + expect(shouldMountBackgroundWorktreeTab(restrictions.get(WORKTREE_ID) ?? null, TAB_1)).toBe( + true + ) + expect(shouldMountBackgroundWorktreeTab(restrictions.get(WORKTREE_ID) ?? null, TAB_2)).toBe( + true + ) + }) + + it('control: the same stranded state drains when the active worktree bounces away and back', async () => { + await seedDeferrableWorktree() + vi.useFakeTimers() + const { result, rerender } = renderHook((props: HarnessProps) => useStrandingHarness(props), { + initialProps: { worktreeId: WORKTREE_ID, gateOpen: false } + }) + rerender({ worktreeId: WORKTREE_ID, gateOpen: true }) + expect( + result.current.activationDeferredMountTabIdsByWorktreeRef.current.get(WORKTREE_ID)?.size + ).toBe(2) + + // Bounce: renderedActiveWorktreeId changes, so the admission effect's + // pre-fix deps already covered this path — the drain must always work here. + rerender({ worktreeId: OTHER_WORKTREE_ID, gateOpen: true }) + rerender({ worktreeId: WORKTREE_ID, gateOpen: true }) + drainIdleAdmissions(3) + const restrictions = result.current.backgroundMountTabIdsByWorktreeRef.current + expect(result.current.activationDeferredMountTabIdsByWorktreeRef.current.has(WORKTREE_ID)).toBe( + false + ) + expect(shouldMountBackgroundWorktreeTab(restrictions.get(WORKTREE_ID) ?? null, TAB_1)).toBe( + true + ) + expect(shouldMountBackgroundWorktreeTab(restrictions.get(WORKTREE_ID) ?? null, TAB_2)).toBe( + true + ) + }) +}) diff --git a/src/renderer/src/components/terminal/use-activation-deferred-tab-admission.ts b/src/renderer/src/components/terminal/use-activation-deferred-tab-admission.ts index 8a46a4e7594..71390ebbe2e 100644 --- a/src/renderer/src/components/terminal/use-activation-deferred-tab-admission.ts +++ b/src/renderer/src/components/terminal/use-activation-deferred-tab-admission.ts @@ -23,6 +23,7 @@ export function useActivationDeferredTabAdmission( controller: TerminalColdActivationController ): void { const { + activationDeferralPlanRevision, activationDeferredMountTabIdsByWorktreeRef, backgroundMountRevision, backgroundMountTabIdsByWorktreeRef, @@ -78,6 +79,10 @@ export function useActivationDeferredTabAdmission( }) setBackgroundMountRevision((revision) => revision + 1) }) + // Why activationDeferralPlanRevision is a dep: a startup-gate-open pass can + // install a plan for the already-active worktree by mutating only refs — + // neither other dep changes, and without this revision the tabs stay + // unmounted until the user switches workspaces and back. // oxlint-disable-next-line react-hooks/exhaustive-deps -- controller refs and setters preserve their original stable identities. - }, [backgroundMountRevision, renderedActiveWorktreeId]) + }, [activationDeferralPlanRevision, backgroundMountRevision, renderedActiveWorktreeId]) } diff --git a/src/renderer/src/components/use-terminal-parking-foundation.ts b/src/renderer/src/components/use-terminal-parking-foundation.ts index 634813d70be..777b51ee26d 100644 --- a/src/renderer/src/components/use-terminal-parking-foundation.ts +++ b/src/renderer/src/components/use-terminal-parking-foundation.ts @@ -68,6 +68,9 @@ export function useTerminalParkingFoundation(controller: TerminalEditorCloseCont const backgroundMountTabIdsByWorktreeRef = useRef(new Map>()) const activationDeferredMountTabIdsByWorktreeRef = useRef(new Map>()) const lastActivationWorktreeIdRef = useRef(null) + // Why a ref, not state: the cold-activation pass runs during render, where a + // setState would be a render-phase update; the pass returns the count instead. + const activationDeferralPlanRevisionRef = useRef(0) useEffect(() => { const timers = measurableBackgroundWorktreeTimersRef.current @@ -163,7 +166,8 @@ export function useTerminalParkingFoundation(controller: TerminalEditorCloseCont forceParkedCaptureDoneRef, backgroundMountTabIdsByWorktreeRef, activationDeferredMountTabIdsByWorktreeRef, - lastActivationWorktreeIdRef + lastActivationWorktreeIdRef, + activationDeferralPlanRevisionRef } }