From 0f6b5616dc17184faf2c366f867559958cd61827 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:46:19 -0700 Subject: [PATCH] fix(terminal): preflight manual sleep capture --- .../sleep-worktree-activation-race.test.ts | 2 + .../sidebar/sleep-worktree-flow.test.ts | 51 ++++++++++++-- .../components/sidebar/sleep-worktree-flow.ts | 26 ++++++- ...anual-sleep-stale-session-recovery.test.ts | 39 +++++++++++ src/renderer/src/store/slices/terminals.ts | 69 +++++++++++++------ 5 files changed, 159 insertions(+), 28 deletions(-) diff --git a/src/renderer/src/components/sidebar/sleep-worktree-activation-race.test.ts b/src/renderer/src/components/sidebar/sleep-worktree-activation-race.test.ts index fbed90765b0..2b2f7b739ef 100644 --- a/src/renderer/src/components/sidebar/sleep-worktree-activation-race.test.ts +++ b/src/renderer/src/components/sidebar/sleep-worktree-activation-race.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => { const state = { activeWorktreeId: null as string | null, + preflightManualWorktreeSleep: vi.fn(), setActiveWorktree: vi.fn((worktreeId: string | null) => { state.activeWorktreeId = worktreeId }), @@ -57,6 +58,7 @@ describe('sleep flow vs slept-workspace activation', () => { } }) mocks.state.activeWorktreeId = 'wt-parent' + mocks.state.preflightManualWorktreeSleep.mockReset() mocks.state.setActiveWorktree.mockClear() mocks.state.shutdownWorktreeBrowsers.mockClear().mockResolvedValue(undefined) mocks.state.shutdownWorktreeTerminals.mockClear().mockImplementation(async (worktreeId) => { 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 016952b71d9..f774dfd9bd7 100644 --- a/src/renderer/src/components/sidebar/sleep-worktree-flow.test.ts +++ b/src/renderer/src/components/sidebar/sleep-worktree-flow.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => { const state = { activeWorktreeId: null as string | null, + preflightManualWorktreeSleep: vi.fn(), setActiveWorktree: vi.fn(), shutdownWorktreeBrowsers: vi.fn().mockResolvedValue(undefined), shutdownWorktreeTerminals: vi.fn().mockResolvedValue(undefined), @@ -49,6 +50,7 @@ describe('runSleepWorktree', () => { }, requestAnimationFrame: vi.fn() }) + mocks.state.preflightManualWorktreeSleep.mockReset() mocks.state.setActiveWorktree.mockClear() mocks.state.shutdownWorktreeBrowsers.mockClear().mockResolvedValue(undefined) mocks.state.shutdownWorktreeTerminals.mockClear().mockResolvedValue(undefined) @@ -102,11 +104,15 @@ describe('runSleepWorktree', () => { expect(mocks.markWorktreeSleepIntent).toHaveBeenCalledWith('wt-1') expect(mocks.clearWorktreeSleepIntent).toHaveBeenCalledWith('wt-1') + const preflightCall = mocks.state.preflightManualWorktreeSleep.mock.invocationCallOrder[0] const markCall = mocks.markWorktreeSleepIntent.mock.invocationCallOrder[0] const activeClear = mocks.state.setActiveWorktree.mock.invocationCallOrder[0] + const browserShutdown = mocks.state.shutdownWorktreeBrowsers.mock.invocationCallOrder[0] const terminalShutdown = mocks.state.shutdownWorktreeTerminals.mock.invocationCallOrder[0] const clearCall = mocks.clearWorktreeSleepIntent.mock.invocationCallOrder[0] + expect(preflightCall).toBeLessThan(markCall) expect(markCall).toBeLessThan(activeClear) + expect(activeClear).toBeLessThan(browserShutdown) expect(terminalShutdown).toBeLessThan(clearCall) }) @@ -212,15 +218,19 @@ describe('runSleepWorktree', () => { ) }) - it('restores the active workspace when resume metadata cannot be captured', async () => { + it('keeps the active workspace untouched when resume metadata cannot be captured', async () => { mocks.state.activeWorktreeId = 'wt-1' - mocks.state.shutdownWorktreeTerminals.mockRejectedValueOnce( - new Error('agent_sleep_capture_missing') - ) + mocks.state.preflightManualWorktreeSleep.mockImplementationOnce(() => { + throw new Error('agent_sleep_capture_missing') + }) await runSleepWorktree('wt-1') - expect(mocks.state.setActiveWorktree.mock.calls).toEqual([[null], ['wt-1']]) + expect(mocks.markWorktreeSleepIntent).not.toHaveBeenCalled() + expect(mocks.clearWorktreeSleepIntent).not.toHaveBeenCalled() + expect(mocks.state.setActiveWorktree).not.toHaveBeenCalled() + expect(mocks.state.shutdownWorktreeBrowsers).not.toHaveBeenCalled() + expect(mocks.state.shutdownWorktreeTerminals).not.toHaveBeenCalled() expect(mocks.suspendWorkspace).not.toHaveBeenCalled() expect(mocks.toastError).toHaveBeenCalledWith( 'Failed to sleep workspace', @@ -276,6 +286,37 @@ describe('runSleepWorktree', () => { ) }) + it('leaves a failed active worktree untouched while sleeping a later safe worktree', async () => { + mocks.state.activeWorktreeId = 'wt-1' + mocks.state.preflightManualWorktreeSleep.mockImplementation((worktreeId: string) => { + if (worktreeId === 'wt-1') { + throw new Error('agent_sleep_capture_missing') + } + }) + + await runSleepWorktrees(['wt-1', 'wt-2']) + + expect(mocks.state.setActiveWorktree).not.toHaveBeenCalled() + expect(mocks.markWorktreeSleepIntent).not.toHaveBeenCalled() + expect(mocks.clearWorktreeSleepIntent).not.toHaveBeenCalled() + expect(mocks.state.shutdownWorktreeBrowsers).not.toHaveBeenCalledWith('wt-1') + expect(mocks.state.shutdownWorktreeTerminals).not.toHaveBeenCalledWith('wt-1', { + keepIdentifiers: true + }) + expect(mocks.state.shutdownWorktreeBrowsers).toHaveBeenCalledWith('wt-2') + expect(mocks.state.shutdownWorktreeTerminals).toHaveBeenCalledWith('wt-2', { + keepIdentifiers: true + }) + expect(mocks.suspendWorkspace).toHaveBeenCalledTimes(1) + expect(mocks.suspendWorkspace).toHaveBeenCalledWith({ workspaceId: 'wt-2' }) + expect(mocks.toastError).toHaveBeenCalledWith( + 'Failed to sleep some workspaces', + expect.objectContaining({ + description: 'A session could not be saved for resume, so the workspace was kept open.' + }) + ) + }) + it('sleeps multiple worktrees and clears active only once when included', async () => { mocks.state.activeWorktreeId = 'wt-2' diff --git a/src/renderer/src/components/sidebar/sleep-worktree-flow.ts b/src/renderer/src/components/sidebar/sleep-worktree-flow.ts index 044d6622a3b..4f04a1a29b9 100644 --- a/src/renderer/src/components/sidebar/sleep-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/sleep-worktree-flow.ts @@ -143,12 +143,31 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise } const { activeWorktreeId, + preflightManualWorktreeSleep, setActiveWorktree, shutdownWorktreeBrowsers, shutdownWorktreeTerminals } = useAppStore.getState() + const errors: string[] = [] + const failedWorktreeIds = new Set() + for (const worktreeId of worktreeIds) { + try { + preflightManualWorktreeSleep(worktreeId) + } catch (err) { + console.error('[sleep-worktree] session capture preflight failed', { + worktreeId, + error: err + }) + failedWorktreeIds.add(worktreeId) + errors.push(describeSleepFailure(err)) + } + } let activeSleepIntentWorktreeId: string | null = null - if (activeWorktreeId && worktreeIds.includes(activeWorktreeId)) { + if ( + activeWorktreeId && + worktreeIds.includes(activeWorktreeId) && + !failedWorktreeIds.has(activeWorktreeId) + ) { const restoreSidebarPosition = preserveSidebarWorktreePosition(activeWorktreeId) // Why: clearing the active workspace can unmount TerminalPanes before // shutdownWorktreeTerminals writes PTY suppressions. Use a non-rendering @@ -159,10 +178,11 @@ export async function runSleepWorktrees(worktreeIds: readonly string[]): Promise setActiveWorktree(null) restoreSidebarPosition() } - const errors: string[] = [] - const failedWorktreeIds = new Set() try { for (const worktreeId of worktreeIds) { + if (failedWorktreeIds.has(worktreeId)) { + continue + } try { // Why: sleep mirrors removeWorktree's shutdown sequence — browsers first // so destroyPersistentWebview unregisters the Chromium guests before any diff --git a/src/renderer/src/store/slices/manual-sleep-stale-session-recovery.test.ts b/src/renderer/src/store/slices/manual-sleep-stale-session-recovery.test.ts index cefb5a6b13a..b2224e787fc 100644 --- a/src/renderer/src/store/slices/manual-sleep-stale-session-recovery.test.ts +++ b/src/renderer/src/store/slices/manual-sleep-stale-session-recovery.test.ts @@ -170,6 +170,45 @@ describe('manual sleep stale session recovery', () => { expect(mockApi.pty.kill).toHaveBeenCalledWith('pty-shell', { keepHistory: true }) }) + it('rejects preflight when a renderer PTY has an unrecordable live Claude pane', () => { + vi.useFakeTimers() + vi.setSystemTime(NOW) + const store = createTestStore() + seedTabs(store) + store.setState({ + ptyIdsByTabId: { 'tab-1': ['pty-claude'] }, + agentStatusByPaneKey: { + 'tab-1:leaf-1': makeAgentEntry({ providerSession: undefined }) + } + } as Partial) + + expect(() => store.getState().preflightManualWorktreeSleep('wt-1')).toThrow( + 'agent_sleep_capture_missing' + ) + + expect(mockApi.pty.kill).not.toHaveBeenCalled() + expect(store.getState().sleepingAgentSessionsByPaneKey).toEqual({}) + }) + + it('allows preflight without renderer PTYs even when a stale row cannot be recorded', () => { + vi.useFakeTimers() + vi.setSystemTime(NOW) + const store = createTestStore() + seedTabs(store) + store.setState({ + ptyIdsByTabId: { 'tab-1': [] }, + agentStatusByPaneKey: { + 'tab-1:leaf-1': makeAgentEntry({ + updatedAt: STALE_AT, + providerSession: undefined + }) + } + } as Partial) + + expect(() => store.getState().preflightManualWorktreeSleep('wt-1')).not.toThrow() + expect(store.getState().sleepingAgentSessionsByPaneKey).toEqual({}) + }) + // Why: killing a still-running resumable agent without a recovery record is the // destructive gap. Sleep must keep the PTY and surface the miss (STA-2844). it('aborts sleep instead of killing a live Claude pane it cannot record', async () => { diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 7542e4c1c75..a1bfb24e0ad 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -449,6 +449,25 @@ function sortedUniquePtyIds(ptyIds: readonly string[] | undefined): string[] { return [...new Set((ptyIds ?? []).filter((ptyId) => ptyId.length > 0))].sort() } +function collectRecordableManualWorktreeSleepSessions( + state: AppState, + worktreeId: string, + rendererShutdownPtyIds: readonly string[], + sleepingPaneKeys?: readonly string[] +): ReturnType { + const records = collectSleepingAgentSessionRecordsForWorktree(state, worktreeId, { + paneKeys: sleepingPaneKeys, + captureMode: 'manual-worktree-sleep' + }) + if ( + rendererShutdownPtyIds.length > 0 && + listUnrecordableManualSleepAgentPanes(state, worktreeId, records, sleepingPaneKeys).length > 0 + ) { + throw new Error(AGENT_SLEEP_CAPTURE_MISSING) + } + return records +} + function equalStringSets(a: readonly string[], b: readonly string[]): boolean { if (a.length !== b.length) { return false @@ -689,6 +708,7 @@ export type TerminalSlice = { invalidateStaleDirectSshTargetPtyBindings: (authority: DirectSshAuthority) => number retryDirectSshTargetPanes: (authority: DirectSshAuthority, now?: number) => number settleDirectSshPaneRetry: (result: DirectSshPaneRetryResult, now?: number) => void + preflightManualWorktreeSleep: (worktreeId: string, sleepingPaneKeys?: readonly string[]) => void shutdownWorktreeTerminals: ( worktreeId: string, opts?: { @@ -2930,6 +2950,20 @@ export const createTerminalSlice: StateCreator }) }, + preflightManualWorktreeSleep: (worktreeId, sleepingPaneKeys) => { + const state = get() + const tabs = state.tabsByWorktree[worktreeId] ?? [] + const rendererShutdownPtyIds = sortedUniquePtyIds( + tabs.flatMap((tab) => state.ptyIdsByTabId[tab.id] ?? []) + ) + collectRecordableManualWorktreeSleepSessions( + state, + worktreeId, + rendererShutdownPtyIds, + sleepingPaneKeys + ) + }, + shutdownWorktreeTerminals: async (worktreeId, opts) => { const keepIdentifiers = opts?.keepIdentifiers ?? false const shutdownReason: AgentStatusWorktreeShutdownReason = @@ -2941,26 +2975,21 @@ export const createTerminalSlice: StateCreator const runtimeEnvironmentId = resolveTerminalStopRuntimeEnvironmentId(get(), worktreeId) // Why: only renderer-bound ids emit pane exit callbacks, so they are the complete guard set (expectedRuntimePtyIds are raw RPC handles). const exitGuardPtyIds = rendererShutdownPtyIds - const sleepingAgentSessionRecords = keepIdentifiers - ? collectSleepingAgentSessionRecordsForWorktree(get(), worktreeId, { - paneKeys: opts?.sleepingPaneKeys, - ...(shutdownReason === 'manual-sleep' ? { captureMode: 'manual-worktree-sleep' } : {}), - ...(shutdownReason === 'auto-hibernate-completed-agent' - ? { captureMode: 'completed-agent-hibernation' } - : {}) - }) - : {} - if (keepIdentifiers && shutdownReason === 'manual-sleep' && rendererShutdownPtyIds.length > 0) { - const unrecordablePaneKeys = listUnrecordableManualSleepAgentPanes( - get(), - worktreeId, - sleepingAgentSessionRecords, - opts?.sleepingPaneKeys - ) - if (unrecordablePaneKeys.length > 0) { - throw new Error(AGENT_SLEEP_CAPTURE_MISSING) - } - } + const sleepingAgentSessionRecords = !keepIdentifiers + ? {} + : shutdownReason === 'manual-sleep' + ? collectRecordableManualWorktreeSleepSessions( + get(), + worktreeId, + rendererShutdownPtyIds, + opts?.sleepingPaneKeys + ) + : collectSleepingAgentSessionRecordsForWorktree(get(), worktreeId, { + paneKeys: opts?.sleepingPaneKeys, + ...(shutdownReason === 'auto-hibernate-completed-agent' + ? { captureMode: 'completed-agent-hibernation' } + : {}) + }) const retainedCompletionEvidence = shutdownReason === 'auto-hibernate-completed-agent' ? collectHibernatedCompletionEvidenceForWorktree(get(), worktreeId, opts?.sleepingPaneKeys)