From 9d00829a59072f666f51ac5e564efdb211c4df41 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 3 Sep 2026 17:20:20 -0400 Subject: [PATCH] fix(terminal): let a slept pane wait for its wake instead of latching cold A pane whose connect ran while its workspace was slept used to mark itself connected and stop; nothing re-armed it, so a wake that produced a live PTY before the user clicked (CLI create, background agent resume, split panes) left panes stranded. The connect now waits on the sleep marker and resumes when the marker clears, and a torn-down pane drops its listener. Tabs created with a live PTY clear the marker too, the sleep flow marks each workspace only when its own teardown starts, and purge forgets the marker without waking anything. --- .../sidebar/sleep-worktree-flow.test.ts | 21 +++++++++ .../components/sidebar/sleep-worktree-flow.ts | 15 +++--- ...-connection-deliberate-sleep-guard.test.ts | 47 +++++++++++++++++++ .../pty-connection/run-deferred-connect.ts | 40 ++++++++++++---- src/renderer/src/lib/worktree-sleep-intent.ts | 32 +++++++++++-- .../worktree-sleep-intent-lifecycle.test.ts | 45 +++++++++++++++--- .../worktrees/teardown/remove-worktree.ts | 4 +- .../teardown/worktree-purge-state.ts | 4 +- .../store/terminals/terminal-pty-bindings.ts | 4 +- .../store/terminals/terminal-tab-creation.ts | 5 ++ tests/e2e/helpers/slept-workspace-probe.ts | 2 +- .../e2e/slept-workspace-remount-wake.spec.ts | 2 + 12 files changed, 187 insertions(+), 34 deletions(-) diff --git a/src/renderer/src/components/sidebar/sleep-worktree-flow.test.ts b/src/renderer/src/components/sidebar/sleep-worktree-flow.test.ts index 16415f54886..4ba100116c1 100644 --- a/src/renderer/src/components/sidebar/sleep-worktree-flow.test.ts +++ b/src/renderer/src/components/sidebar/sleep-worktree-flow.test.ts @@ -190,6 +190,27 @@ describe('runSleepWorktree', () => { expect(mocks.clearWorktreeSleepIntent).not.toHaveBeenCalled() }) + it('marks each worktree only when its own teardown starts', async () => { + let releaseFirst: () => void = () => {} + mocks.state.shutdownWorktreeBrowsers.mockImplementationOnce( + () => + new Promise((resolve) => { + releaseFirst = resolve + }) + ) + + const run = runSleepWorktrees(['wt-1', 'wt-2']) + await Promise.resolve() + + // Why: wt-2 is still awake while wt-1 tears down; marking it early would + // hold its panes cold and swallow its activity. + expect(mocks.markWorktreeSleepIntent).toHaveBeenCalledWith('wt-1') + expect(mocks.markWorktreeSleepIntent).not.toHaveBeenCalledWith('wt-2') + releaseFirst() + await run + expect(mocks.markWorktreeSleepIntent).toHaveBeenCalledWith('wt-2') + }) + it('surfaces a toast and skips terminals when browsers throws', async () => { mocks.state.activeWorktreeId = 'wt-1' mocks.state.shutdownWorktreeBrowsers.mockRejectedValueOnce(new Error('boom')) diff --git a/src/renderer/src/components/sidebar/sleep-worktree-flow.ts b/src/renderer/src/components/sidebar/sleep-worktree-flow.ts index 52d405e9dbe..16ecdc133ea 100644 --- a/src/renderer/src/components/sidebar/sleep-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/sleep-worktree-flow.ts @@ -141,16 +141,15 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise shutdownWorktreeBrowsers, shutdownWorktreeTerminals } = useAppStore.getState() - // Why: mark before any teardown so exits do not stamp activity and the panes - // left mounted stay cold until an explicit wake (#10205). Kept off the store - // so it cannot disturb the sidebar's scroll restoration. - for (const worktreeId of worktreeIds) { - markWorktreeSleepIntent(worktreeId) - } const sleptActiveWorktreeId = activeWorktreeId && worktreeIds.includes(activeWorktreeId) ? activeWorktreeId : null if (sleptActiveWorktreeId) { const restoreSidebarPosition = preserveSidebarWorktreePosition(sleptActiveWorktreeId) + // Why: clearing the active workspace can unmount TerminalPanes before + // shutdownWorktreeTerminals writes PTY suppressions; mark first so those + // exits do not stamp activity. Kept off the store so it cannot disturb the + // sidebar's scroll restoration. + markWorktreeSleepIntent(sleptActiveWorktreeId) setActiveWorktree(null) restoreSidebarPosition() } @@ -158,6 +157,10 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise const failedWorktreeIds = new Set() try { for (const worktreeId of worktreeIds) { + // Why: the marker outlives teardown so the panes left mounted stay cold + // until an explicit wake (#10205); mark per workspace so an earlier + // slow teardown never leaves a later, still-awake one marked. + markWorktreeSleepIntent(worktreeId) try { // Why: sleep mirrors removeWorktree's shutdown sequence — browsers first // so destroyPersistentWebview unregisters the Chromium guests before any diff --git a/src/renderer/src/components/terminal-pane/pty-connection-deliberate-sleep-guard.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-deliberate-sleep-guard.test.ts index 38869747f3d..caee431a60c 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-deliberate-sleep-guard.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-deliberate-sleep-guard.test.ts @@ -185,6 +185,53 @@ describe('deliberate sleep keeps mounted panes cold', () => { expect(transport.connect).not.toHaveBeenCalled() }) + it('connects a waiting pane once the workspace is woken', async () => { + const { connectPanePty } = await import('./pty-connection') + const { clearWorktreeSleepIntent, markWorktreeSleepIntent } = + await import('@/lib/worktree-sleep-intent') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + activeWorktreeId: 'wt-other', + tabsByWorktree: { 'wt-1': [{ id: 'tab-woken', ptyId: 'wt-1@@dead' }] } + } + markWorktreeSleepIntent('wt-1') + const deps = createDeps({ + tabId: 'tab-woken', + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: 'wt-1@@dead' }, + isVisibleRef: { current: false } + }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks() + expect(transport.connect).not.toHaveBeenCalled() + + clearWorktreeSleepIntent('wt-1') + await flushAsyncTicks() + + expect(transport.connect).toHaveBeenCalledTimes(1) + }) + + it('drops the wake listener when a waiting pane is disposed', async () => { + const { connectPanePty } = await import('./pty-connection') + const { clearWorktreeSleepIntent, markWorktreeSleepIntent } = + await import('@/lib/worktree-sleep-intent') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + markWorktreeSleepIntent('wt-1') + const deps = createDeps({ tabId: 'tab-disposed', isVisibleRef: { current: false } }) + + const binding = connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks() + binding.dispose() + clearWorktreeSleepIntent('wt-1') + await flushAsyncTicks() + + expect(transport.connect).not.toHaveBeenCalled() + }) + it('still connects a slept pane that carries a queued startup', async () => { const { connectPanePty } = await import('./pty-connection') const { markWorktreeSleepIntent } = await import('@/lib/worktree-sleep-intent') diff --git a/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts b/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts index 5139b7eb312..0539f340459 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts @@ -2,7 +2,7 @@ import { createTerminalZeroDimensionsMessage } from '../../../../../shared/termi import { isWorktreeRemovalFenceError } from '../../../../../shared/worktree/removal-fence-error' import { safeFit } from '@/lib/pane-manager/pane-tree-ops' import { createCodexBackfillErrorDetector } from '../codex-backfill-error-detector' -import { hasWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' +import { hasWorktreeSleepIntent, onWorktreeSleepIntentCleared } from '@/lib/worktree-sleep-intent' import { isRemoteRuntimePtyId } from './paired-parked-terminal-restore' import { recordPtyConnectDiagnostic } from './pty-connect-limits' @@ -28,11 +28,40 @@ export function installRunDeferredConnect(session: ConnectPanePtySession): void const cwdPromise = session.deps.cwdPromise let cwdPromiseSettled = cwdPromise === undefined let cwdPromiseWaitStarted = false + let wakeWaitStarted = false session.runDeferredConnect = (): void => { if (session.connectStarted) { return } + // Why: a deliberately slept workspace keeps its panes mounted, so connecting + // would reattach the retained session id and respawn the shell (#10205). + // Wait for the wake instead; a queued startup is an explicit launch. + if (hasWorktreeSleepIntent(session.deps.worktreeId) && !session.paneStartup) { + session.cancelScheduledConnectFrame() + if (session.connectFallbackTimer !== null) { + clearTimeout(session.connectFallbackTimer) + session.connectFallbackTimer = null + } + if (!wakeWaitStarted) { + wakeWaitStarted = true + recordPtyConnectDiagnostic( + `pane=${session.pane.id} tab=${session.deps.tabId} -> WAIT FOR WAKE (deliberate sleep)` + ) + const unsubscribe = onWorktreeSleepIntentCleared(session.deps.worktreeId, () => { + const index = session.waitTeardowns.indexOf(unsubscribe) + if (index !== -1) { + session.waitTeardowns.splice(index, 1) + } + if (!session.disposed) { + session.runDeferredConnect() + } + }) + // Why: disposal unsubscribes so a torn-down pane never connects on a later wake. + session.waitTeardowns.push(unsubscribe) + } + return + } if (!cwdPromiseSettled) { session.cancelScheduledConnectFrame() if (session.connectFallbackTimer !== null) { @@ -83,15 +112,6 @@ export function installRunDeferredConnect(session: ConnectPanePtySession): void if (session.disposed) { return } - // Why: a deliberately slept workspace keeps its panes mounted, so any later - // remount would reattach its retained session id and respawn the shell - // (#10205). A queued startup is an explicit launch and still connects. - if (hasWorktreeSleepIntent(session.deps.worktreeId) && !session.paneStartup) { - recordPtyConnectDiagnostic( - `pane=${session.pane.id} tab=${session.deps.tabId} -> SKIP CONNECT (deliberate sleep)` - ) - return - } safeFit(session.pane) session.cols = session.pane.terminal.cols session.rows = session.pane.terminal.rows diff --git a/src/renderer/src/lib/worktree-sleep-intent.ts b/src/renderer/src/lib/worktree-sleep-intent.ts index 66100275fb0..d7fdfec4b45 100644 --- a/src/renderer/src/lib/worktree-sleep-intent.ts +++ b/src/renderer/src/lib/worktree-sleep-intent.ts @@ -1,18 +1,42 @@ // Why: a slept workspace keeps its panes mounted with only dead PTYs behind them. -// Until the user explicitly opens it (or something binds a PTY to it), any -// remount of those panes must stay cold instead of reattaching or respawning. +// Any pane connect that runs while the marker is set waits here, and the clear +// that marks the workspace awake resumes every waiting connect. const sleepingWorktreeIds = new Set() +const wakeListenersByWorktreeId = new Map void>>() export function markWorktreeSleepIntent(worktreeId: string): void { sleepingWorktreeIds.add(worktreeId) } export function clearWorktreeSleepIntent(worktreeId: string | null): void { - if (worktreeId) { - sleepingWorktreeIds.delete(worktreeId) + if (!worktreeId || !sleepingWorktreeIds.delete(worktreeId)) { + return } + const listeners = wakeListenersByWorktreeId.get(worktreeId) + wakeListenersByWorktreeId.delete(worktreeId) + for (const listener of listeners ?? []) { + listener() + } +} + +// Why: a purged worktree must not wake its panes; they are being unmounted. +export function forgetWorktreeSleepIntent(worktreeId: string): void { + sleepingWorktreeIds.delete(worktreeId) + wakeListenersByWorktreeId.delete(worktreeId) } export function hasWorktreeSleepIntent(worktreeId: string | null): boolean { return worktreeId !== null && sleepingWorktreeIds.has(worktreeId) } + +export function onWorktreeSleepIntentCleared(worktreeId: string, listener: () => void): () => void { + const listeners = wakeListenersByWorktreeId.get(worktreeId) ?? new Set<() => void>() + listeners.add(listener) + wakeListenersByWorktreeId.set(worktreeId, listeners) + return () => { + listeners.delete(listener) + if (listeners.size === 0 && wakeListenersByWorktreeId.get(worktreeId) === listeners) { + wakeListenersByWorktreeId.delete(worktreeId) + } + } +} diff --git a/src/renderer/src/store/slices/worktree-sleep-intent-lifecycle.test.ts b/src/renderer/src/store/slices/worktree-sleep-intent-lifecycle.test.ts index 09883138446..08ddf9b6d9c 100644 --- a/src/renderer/src/store/slices/worktree-sleep-intent-lifecycle.test.ts +++ b/src/renderer/src/store/slices/worktree-sleep-intent-lifecycle.test.ts @@ -1,13 +1,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { - clearWorktreeSleepIntent, - hasWorktreeSleepIntent, - markWorktreeSleepIntent -} from '@/lib/worktree-sleep-intent' +import * as intent from '@/lib/worktree-sleep-intent' import { buildWorktreePurgeState } from './worktrees/teardown/worktree-purge-state' import { createTestStore, makeWorktree, seedStore } from './store-test-helpers' import { createStoreCascadesMockApi } from './store-cascades-test-harness' +const { clearWorktreeSleepIntent, hasWorktreeSleepIntent, markWorktreeSleepIntent } = intent const WORKTREE_ID = 'repo1::/path/wt1' const FOLDER_KEY = 'folder:folder-1' @@ -92,13 +89,47 @@ describe('worktree sleep intent lifecycle', () => { expect(hasWorktreeSleepIntent(WORKTREE_ID)).toBe(false) }) - it('is pruned when the worktree is purged', () => { + it('is released when a tab is created with a live PTY', () => { const store = createTestStore() seedWorktree(store) markWorktreeSleepIntent(WORKTREE_ID) - store.setState(buildWorktreePurgeState(store.getState(), [WORKTREE_ID])) + store.getState().createTab(WORKTREE_ID, undefined, undefined, { + activate: false, + initialPtyId: 'pty-cli-created' + }) expect(hasWorktreeSleepIntent(WORKTREE_ID)).toBe(false) }) + + it('notifies wake listeners once and only on a real clear', () => { + const { onWorktreeSleepIntentCleared } = intent + const woke = vi.fn() + markWorktreeSleepIntent(WORKTREE_ID) + const unsubscribe = onWorktreeSleepIntentCleared(WORKTREE_ID, woke) + + clearWorktreeSleepIntent('repo1::/path/other') + expect(woke).not.toHaveBeenCalled() + clearWorktreeSleepIntent(WORKTREE_ID) + clearWorktreeSleepIntent(WORKTREE_ID) + expect(woke).toHaveBeenCalledTimes(1) + + markWorktreeSleepIntent(WORKTREE_ID) + clearWorktreeSleepIntent(WORKTREE_ID) + expect(woke).toHaveBeenCalledTimes(1) + unsubscribe() + }) + + it('is forgotten without waking panes when the worktree is purged', () => { + const store = createTestStore() + seedWorktree(store) + markWorktreeSleepIntent(WORKTREE_ID) + const woke = vi.fn() + intent.onWorktreeSleepIntentCleared(WORKTREE_ID, woke) + + store.setState(buildWorktreePurgeState(store.getState(), [WORKTREE_ID])) + + expect(hasWorktreeSleepIntent(WORKTREE_ID)).toBe(false) + expect(woke).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts index 32d7afa8199..a6a9c87d838 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts @@ -6,7 +6,7 @@ import { parseExecutionHostId } from '../../../../../../shared/execution-host' import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' import { getActiveRuntimeTarget } from '../../../../runtime/runtime-rpc-client' import { forgetHugeRepoWarningDismissalsForWorktrees } from '@/lib/source-control-huge-repo-warning-dismissals' -import { clearWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' +import { forgetWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' import { showPreservedBranchToast } from '@/components/sidebar/preserved-branch-toast' import { resolveWorktreeOperationRouteResult, @@ -223,7 +223,7 @@ export function createRemoveWorktree( // Why: invalidate stale probes once deletion is authoritative, so an old toast can't mutate a same-path replacement. forgetHugeRepoWarningDismissalsForWorktrees([worktreeId]) - clearWorktreeSleepIntent(worktreeId) + forgetWorktreeSleepIntent(worktreeId) // Why: forget-local is legal while the host is unreachable, so record the removal here too — otherwise an // in-flight metadata read that snapshotted this row re-appends it, and disconnected polls never drop it. if (hostId && parseExecutionHostId(hostId)?.kind === 'ssh') { diff --git a/src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts b/src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts index a6ec6b7aa91..e2f6e5945fe 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts @@ -8,7 +8,7 @@ import { createWorktreePurgeOmitters } from './worktree-purge-omitters' import { removeDeleteStatesForWorktreeIds } from './worktree-delete-state' import { removeWorktreeVisitEntriesForTargets } from '@/lib/worktree-visit-recency' import { forgetAmbiguousOwnerWarnings } from '../listing/worktree-owner-settings' -import { clearWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' +import { forgetWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' export function buildWorktreePurgeState( s: AppState, @@ -21,7 +21,7 @@ export function buildWorktreePurgeState( pruneHostedReviewLinkMutationGenerations(worktreeIdSet) // Why: ids are repo::path, so a worktree recreated at the same path must not inherit a stale sleep. for (const id of worktreeIdSet) { - clearWorktreeSleepIntent(id) + forgetWorktreeSleepIntent(id) } // Why: every authoritative and explicit purge converges here, so a deleted path can't inherit stale UI state. forgetHugeRepoWarningDismissalsForWorktrees(worktreeIdSet) diff --git a/src/renderer/src/store/terminals/terminal-pty-bindings.ts b/src/renderer/src/store/terminals/terminal-pty-bindings.ts index 520edc00251..499ea63e8de 100644 --- a/src/renderer/src/store/terminals/terminal-pty-bindings.ts +++ b/src/renderer/src/store/terminals/terminal-pty-bindings.ts @@ -121,8 +121,6 @@ export function createTerminalPtyBindingActions( } break } - // Why: a bound PTY means the workspace is awake by any route (CLI, automation, client wake), not only activation. - clearWorktreeSleepIntent(worktreeId) // Why: the first active PTY changes sorting, except activation-spawn side effects. const isFirstPty = existingPtyIds.length === 0 const isActiveWorktree = worktreeId != null && s.activeWorktreeId === worktreeId @@ -278,6 +276,8 @@ export function createTerminalPtyBindingActions( ...(shouldBumpSortEpoch ? { sortEpoch: s.sortEpoch + 1 } : {}) } }) + // Why: a bound PTY means the workspace is awake by any route (CLI, automation, client wake), not only activation. + clearWorktreeSleepIntent(worktreeId) // Why: activation spawns come from clicking a worktree, not work in it — skip the lastActivityAt stamp and sortEpoch bump; other spawn reasons still bump. if (worktreeId && !wasActivationSpawn && !isRemoteRuntimeMirror) { get().bumpWorktreeActivity(worktreeId) diff --git a/src/renderer/src/store/terminals/terminal-tab-creation.ts b/src/renderer/src/store/terminals/terminal-tab-creation.ts index 11f9d1d2a59..83310850475 100644 --- a/src/renderer/src/store/terminals/terminal-tab-creation.ts +++ b/src/renderer/src/store/terminals/terminal-tab-creation.ts @@ -1,3 +1,4 @@ +import { clearWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { isValidHostTerminalTabId } from '../../../../shared/terminal-tab-id' import { emptyLayoutSnapshot, singlePaneLayoutSnapshot } from '../slices/terminal-helpers' @@ -271,6 +272,10 @@ export function createTerminalTabCreationActions( } } }) + if (options?.initialPtyId) { + // Why: a tab born with a live PTY (CLI/runtime create) wakes the workspace like any other bind. + clearWorktreeSleepIntent(worktreeId) + } const shouldRecordInteraction = options?.recordInteraction ?? (!options?.pendingActivationSpawn && !options?.initialPtyId) if (shouldRecordInteraction) { diff --git a/tests/e2e/helpers/slept-workspace-probe.ts b/tests/e2e/helpers/slept-workspace-probe.ts index 92b1eeeabe7..b2b5635b526 100644 --- a/tests/e2e/helpers/slept-workspace-probe.ts +++ b/tests/e2e/helpers/slept-workspace-probe.ts @@ -77,7 +77,7 @@ export async function readConnectDiagnostics(page: Page, worktreeId: string): Pr const tabByPaneId = new Map() const owned: string[] = [] for (const line of diag) { - const connect = /^pane=(\d+) tab=(\S+)/.exec(line) + const connect = /^pane=(\d+) tab=(\S+) /.exec(line) if (connect) { tabByPaneId.set(connect[1], connect[2]) if (tabIds.has(connect[2])) { diff --git a/tests/e2e/slept-workspace-remount-wake.spec.ts b/tests/e2e/slept-workspace-remount-wake.spec.ts index f2f84a85e05..c4588e76a79 100644 --- a/tests/e2e/slept-workspace-remount-wake.spec.ts +++ b/tests/e2e/slept-workspace-remount-wake.spec.ts @@ -35,6 +35,8 @@ async function assertStaysCold(page: Page, worktreeId: string): Promise { expect(peakLivePty, 'slept workspace grew a live PTY').toBe(0) expect(peakTabs, 'slept workspace grew a tab').toBe(1) expect(hostLive, 'host created a session for the slept workspace').toBe(0) + // Why: proves the gate held rather than the pane having quietly unmounted. + expect(diag.at(-1), 'remounted pane did not wait for the wake').toContain('WAIT FOR WAKE') } test('remounting a slept hidden pane does not respawn its PTY', async ({ orcaPage }) => {