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 9d24a6ae471..c38f0d4e36d 100644 --- a/src/renderer/src/components/sidebar/sleep-worktree-flow.test.ts +++ b/src/renderer/src/components/sidebar/sleep-worktree-flow.test.ts @@ -44,7 +44,8 @@ vi.mock('@/store', () => ({ vi.mock('sonner', () => ({ toast: { error: mocks.toastError } })) vi.mock('@/lib/worktree-sleep-intent', () => ({ clearWorktreeSleepIntent: mocks.clearWorktreeSleepIntent, - markWorktreeSleepIntent: mocks.markWorktreeSleepIntent + markWorktreeSleepIntent: mocks.markWorktreeSleepIntent, + withWorktreeSleepTeardown: (_worktreeId: string, teardown: () => Promise) => teardown() })) import { runSleepWorktree, runSleepWorktrees } from './sleep-worktree-flow' @@ -221,14 +222,6 @@ describe('runSleepWorktree', () => { expect(mocks.clearWorktreeSleepIntent).toHaveBeenLastCalledWith('wt-2') }) - it('re-asserts the marker after teardown so a late PTY bind cannot un-sleep it', async () => { - await runSleepWorktree('wt-1') - - const marks = mocks.markWorktreeSleepIntent.mock.invocationCallOrder - const terminalShutdown = mocks.state.shutdownWorktreeTerminals.mock.invocationCallOrder[0] - expect(marks.some((order) => order > terminalShutdown)).toBe(true) - }) - it('marks each worktree only when its own teardown starts', async () => { let releaseFirst: () => void = () => {} mocks.state.shutdownWorktreeBrowsers.mockImplementationOnce( diff --git a/src/renderer/src/components/sidebar/sleep-worktree-flow.ts b/src/renderer/src/components/sidebar/sleep-worktree-flow.ts index fd46039ab1c..1e414a81800 100644 --- a/src/renderer/src/components/sidebar/sleep-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/sleep-worktree-flow.ts @@ -1,6 +1,10 @@ import { toast } from 'sonner' import { useAppStore } from '@/store' -import { clearWorktreeSleepIntent, markWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' +import { + clearWorktreeSleepIntent, + markWorktreeSleepIntent, + withWorktreeSleepTeardown +} from '@/lib/worktree-sleep-intent' import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor' import { translate } from '@/i18n/i18n' @@ -167,7 +171,7 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise // other teardown runs, terminals second so the PTY kill uses the same // ordering on both paths. Without the browser thunk here, sleep leaks // browserPagesByWorkspace entries and live webviews for the slept worktree. - await shutdownWorktreeBrowsers(worktreeId) + await withWorktreeSleepTeardown(worktreeId, () => shutdownWorktreeBrowsers(worktreeId)) } catch (err) { console.error('[sleep-worktree] browser shutdown failed', { worktreeId, error: err }) failedWorktreeIds.add(worktreeId) @@ -182,17 +186,15 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise // history dir (local) or relay session id (SSH); it also captures // serializer buffers into buffersByLeafId for SSH wake to reseed // scrollback. See DESIGN_DOC_TERMINAL_HISTORY_FIX_V2.md §3.3.c. - await shutdownWorktreeTerminals(worktreeId, { keepIdentifiers: true }) - if (typeof window !== 'undefined' && window.api?.ephemeralVm?.suspendWorkspace) { - await window.api.ephemeralVm.suspendWorkspace({ workspaceId: worktreeId }) - } - // Why: a spawn that resolved during teardown binds a PTY and clears the marker; - // the workspace is asleep now, so re-assert it. A workspace the user activated - // meanwhile is awake by their choice and must not be left marked. + await withWorktreeSleepTeardown(worktreeId, async () => { + await shutdownWorktreeTerminals(worktreeId, { keepIdentifiers: true }) + if (typeof window !== 'undefined' && window.api?.ephemeralVm?.suspendWorkspace) { + await window.api.ephemeralVm.suspendWorkspace({ workspaceId: worktreeId }) + } + }) + // Why: a workspace the user activated during the batch is awake by their choice. if (useAppStore.getState().activeWorktreeId === worktreeId) { clearWorktreeSleepIntent(worktreeId) - } else { - markWorktreeSleepIntent(worktreeId) } } catch (err) { console.error('[sleep-worktree] terminal or host suspension failed', { 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 c00535ed4b7..b49a80d9fe3 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 @@ -313,10 +313,16 @@ describe('deliberate sleep keeps mounted panes cold', () => { const binding = connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) await flushAsyncTicks() binding.dispose() + // Why: the connect body already refuses a disposed session, so prove the + // listener itself is gone: a wake after dispose reaches no subscriber. + const wakeCalls: number[] = [] + const { onWorktreeSleepIntentCleared } = await import('@/lib/worktree-sleep-intent') + onWorktreeSleepIntentCleared('wt-1', () => wakeCalls.push(1)) clearWorktreeSleepIntent('wt-1') await flushAsyncTicks() expect(transport.connect).not.toHaveBeenCalled() + expect(wakeCalls).toHaveLength(1) }) it('still connects a slept pane that carries a queued startup', async () => { diff --git a/src/renderer/src/lib/worktree-sleep-intent.ts b/src/renderer/src/lib/worktree-sleep-intent.ts index 5e55c216f1d..6512abec692 100644 --- a/src/renderer/src/lib/worktree-sleep-intent.ts +++ b/src/renderer/src/lib/worktree-sleep-intent.ts @@ -2,14 +2,35 @@ // 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 tearingDownWorktreeIds = new Set() const wakeListenersByWorktreeId = new Map void>>() export function markWorktreeSleepIntent(worktreeId: string): void { sleepingWorktreeIds.add(worktreeId) } +/** + * Why: a spawn that resolves while the sleep teardown is still awaiting its host + * would bind a PTY and clear the marker, waking every waiting pane mid-sleep. + * Binds during the teardown window are not wakes. + */ +export async function withWorktreeSleepTeardown( + worktreeId: string, + teardown: () => Promise +): Promise { + tearingDownWorktreeIds.add(worktreeId) + try { + return await teardown() + } finally { + tearingDownWorktreeIds.delete(worktreeId) + } +} + export function clearWorktreeSleepIntent(worktreeId: string | null): void { - if (!worktreeId || !sleepingWorktreeIds.delete(worktreeId)) { + if (!worktreeId || tearingDownWorktreeIds.has(worktreeId)) { + return + } + if (!sleepingWorktreeIds.delete(worktreeId)) { return } const listeners = wakeListenersByWorktreeId.get(worktreeId) @@ -27,6 +48,7 @@ export function clearWorktreeSleepIntent(worktreeId: string | null): void { // Why: a purged worktree must not wake its panes; they are being unmounted. export function forgetWorktreeSleepIntent(worktreeId: string): void { sleepingWorktreeIds.delete(worktreeId) + tearingDownWorktreeIds.delete(worktreeId) 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 bbeeb95d34e..8b3c73e2ff1 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 @@ -120,6 +120,24 @@ describe('worktree sleep intent lifecycle', () => { unsubscribe() }) + it('ignores a PTY bind that lands while the sleep teardown is in flight', async () => { + const store = createTestStore() + seedWorktree(store) + const tab = store.getState().createTab(WORKTREE_ID, undefined, undefined, { activate: false }) + markWorktreeSleepIntent(WORKTREE_ID) + const woke = vi.fn() + intent.onWorktreeSleepIntentCleared(WORKTREE_ID, woke) + + await intent.withWorktreeSleepTeardown(WORKTREE_ID, async () => { + store.getState().updateTabPtyId(tab.id, 'pty-late-spawn') + }) + + expect(hasWorktreeSleepIntent(WORKTREE_ID)).toBe(true) + expect(woke).not.toHaveBeenCalled() + store.getState().updateTabPtyId(tab.id, 'pty-after-teardown') + expect(hasWorktreeSleepIntent(WORKTREE_ID)).toBe(false) + }) + it('keeps notifying siblings when one wake listener throws', () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) const woke = vi.fn() diff --git a/tests/e2e/slept-workspace-remount-wake.spec.ts b/tests/e2e/slept-workspace-remount-wake.spec.ts index c4588e76a79..f820a7c7f69 100644 --- a/tests/e2e/slept-workspace-remount-wake.spec.ts +++ b/tests/e2e/slept-workspace-remount-wake.spec.ts @@ -72,7 +72,8 @@ test('remounting a slept hidden pane does not respawn its PTY', async ({ orcaPag expect(remounted, 'remountTerminalTabForRecovery did not find the slept tab').toBe(true) await assertStaysCold(orcaPage, slept) - // Non-vacuity: a deliberate click must still wake it. + // Non-vacuity: a deliberate click must still wake it, and exactly once — the + // waiting pane and its remounted successor must not both reattach. await activateWorkspaceByClick(orcaPage, slept) await expect .poll(async () => (await readWorkspaceSample(orcaPage, slept)).livePtyCount, { @@ -80,4 +81,7 @@ test('remounting a slept hidden pane does not respawn its PTY', async ({ orcaPag message: 'the slept workspace never wakes even on deliberate activation' }) .toBeGreaterThan(0) + await orcaPage.waitForTimeout(3_000) + expect((await readWorkspaceSample(orcaPage, slept)).livePtyCount).toBe(1) + expect(await readHostLiveTerminalCount(orcaPage, slept)).toBe(1) })