diff --git a/src/main/ipc/pty-runtime-kill-and-exit.test.ts b/src/main/ipc/pty-runtime-kill-and-exit.test.ts index a8c5d816e2d..2c655264c73 100644 --- a/src/main/ipc/pty-runtime-kill-and-exit.test.ts +++ b/src/main/ipc/pty-runtime-kill-and-exit.test.ts @@ -166,7 +166,8 @@ describe('registerPtyHandlers', () => { const runtime = { setPtyController: vi.fn(), markPtyStopRequested: vi.fn(), - markPtyHistoryPreservingStopRequested: vi.fn(), + markPtyHistoryPreservingStopRequested: vi.fn(() => 1), + preservePtyHistoryThroughLateExit: vi.fn(), clearPtyHistoryPreservingStopRequested: vi.fn() } setLocalPtyProvider({ @@ -198,7 +199,8 @@ describe('registerPtyHandlers', () => { handlers.get('pty:kill')!(null, { id: 'local-pty', keepHistory: true }) ).rejects.toThrow('daemon unavailable') expect(runtime.markPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty') - expect(runtime.clearPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty') + expect(runtime.preservePtyHistoryThroughLateExit).toHaveBeenCalledWith('local-pty', 1) + expect(runtime.clearPtyHistoryPreservingStopRequested).not.toHaveBeenCalled() }) it('rejects runtime terminal IDs before unowned local provider routing', async () => { const shutdown = vi.spyOn(getLocalPtyProvider(), 'shutdown') @@ -215,7 +217,7 @@ describe('registerPtyHandlers', () => { const runtime = { setPtyController: vi.fn(), onPtyExit: vi.fn(), - markPtyHistoryPreservingStopRequested: vi.fn(), + markPtyHistoryPreservingStopRequested: vi.fn(() => 1), clearPtyHistoryPreservingStopRequested: vi.fn() } setLocalPtyProvider({ @@ -251,7 +253,7 @@ describe('registerPtyHandlers', () => { }) expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', -1, undefined) expect(runtime.markPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty') - expect(runtime.clearPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty') + expect(runtime.clearPtyHistoryPreservingStopRequested).toHaveBeenCalledWith('local-pty', 1) expect(runtime.markPtyHistoryPreservingStopRequested.mock.invocationCallOrder[0]).toBeLessThan( runtime.onPtyExit.mock.invocationCallOrder[0]! ) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 7701b3d9a6f..64b85e6c1d0 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -4033,7 +4033,8 @@ export function registerPtyHandlers( async function shutdownProviderAndDetectExit( provider: IPtyProvider, id: string, - opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number } + opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number }, + onExitObserved?: () => void ): Promise { let providerExitObserved = false const expectedIncarnationId = ptyIncarnationById.get(id) @@ -4043,6 +4044,7 @@ export function registerPtyHandlers( (!expectedIncarnationId || payload.incarnationId === expectedIncarnationId) ) { providerExitObserved = true + onExitObserved?.() } }) try { @@ -7722,9 +7724,9 @@ export function registerPtyHandlers( throw new Error('Invalid PTY provider id') } runtime?.markPtyStopRequested?.(args.id) - if (args.keepHistory) { - runtime?.markPtyHistoryPreservingStopRequested(args.id) - } + let historyPreservingStopId = args.keepHistory + ? runtime?.markPtyHistoryPreservingStopRequested(args.id) + : undefined try { const ownedConnectionId = ptyOwnership.get(args.id) const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null @@ -7749,13 +7751,24 @@ export function registerPtyHandlers( const shutdownProvider = provider ?? getProviderForPty(args.id) let providerExitObserved = false try { - providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, { - immediate: true, - keepHistory: args.keepHistory ?? false - }) + providerExitObserved = await shutdownProviderAndDetectExit( + shutdownProvider, + args.id, + { + immediate: true, + keepHistory: args.keepHistory ?? false + }, + () => { + providerExitObserved = true + } + ) } catch (err) { if (!isPtyAlreadyGoneError(err)) { // Why: a failed shutdown can leave the process alive (SSH relay grace window / local daemon); keep ownership/lease state so the user can retry. + if (historyPreservingStopId !== undefined && !providerExitObserved) { + runtime?.preservePtyHistoryThroughLateExit(args.id, historyPreservingStopId) + historyPreservingStopId = undefined + } throw err } /* session already dead — cleanup below handles the rest */ @@ -7769,8 +7782,8 @@ export function registerPtyHandlers( sendPtyExitToRenderer({ id: args.id, code: -1 }) } } finally { - if (args.keepHistory) { - runtime?.clearPtyHistoryPreservingStopRequested(args.id) + if (historyPreservingStopId !== undefined) { + runtime?.clearPtyHistoryPreservingStopRequested(args.id, historyPreservingStopId) } } }) diff --git a/src/main/runtime/orca-runtime-terminal-retirement.test.ts b/src/main/runtime/orca-runtime-terminal-retirement.test.ts index f1d80bfb6d1..9547075d6e7 100644 --- a/src/main/runtime/orca-runtime-terminal-retirement.test.ts +++ b/src/main/runtime/orca-runtime-terminal-retirement.test.ts @@ -488,9 +488,9 @@ describe('OrcaRuntimeService terminal surface retirement', () => { incarnationId: 'incarnation-left' }) - runtime.markPtyHistoryPreservingStopRequested('pty-left') + const stopRequestId = runtime.markPtyHistoryPreservingStopRequested('pty-left') runtime.onPtyExit('pty-left', 0, 'incarnation-left') - runtime.clearPtyHistoryPreservingStopRequested('pty-left') + runtime.clearPtyHistoryPreservingStopRequested('pty-left', stopRequestId) expect((await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs).toEqual([ expect.objectContaining({ id: 'tab::left' }), @@ -499,6 +499,71 @@ describe('OrcaRuntimeService terminal surface retirement', () => { expect(setWorkspaceSession).not.toHaveBeenCalled() }) + it('preserves a surface until every overlapping history-preserving stop finishes', async () => { + const session = makePersistedSplitSession() + const setWorkspaceSession = vi.fn() + const runtime = new OrcaRuntimeService( + runtimeStore({ + getWorkspaceSession: () => session, + setWorkspaceSession, + flushOrThrow: vi.fn() + }) + ) + runtime.attachWindow(1) + syncSplit(runtime) + runtime.registerPty('pty-left', WORKTREE_ID, null, { + tabId: 'tab', + leafId: 'left', + incarnationId: 'incarnation-left' + }) + + const firstStopRequestId = runtime.markPtyHistoryPreservingStopRequested('pty-left') + const secondStopRequestId = runtime.markPtyHistoryPreservingStopRequested('pty-left') + runtime.clearPtyHistoryPreservingStopRequested('pty-left', firstStopRequestId) + runtime.onPtyExit('pty-left', 0, 'incarnation-left') + runtime.clearPtyHistoryPreservingStopRequested('pty-left', secondStopRequestId) + + expect((await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs).toEqual([ + expect.objectContaining({ id: 'tab::left' }), + expect.objectContaining({ id: 'tab::right' }) + ]) + expect(setWorkspaceSession).not.toHaveBeenCalled() + }) + + it('consumes a failed stop lease when its late exit arrives', async () => { + const session = makePersistedSplitSession() + const runtime = new OrcaRuntimeService( + runtimeStore({ + getWorkspaceSession: () => session, + setWorkspaceSession: vi.fn(), + flushOrThrow: vi.fn() + }) + ) + runtime.attachWindow(1) + syncSplit(runtime) + runtime.registerPty('pty-left', WORKTREE_ID, null, { + tabId: 'tab', + leafId: 'left', + incarnationId: 'incarnation-left' + }) + + const stopRequestId = runtime.markPtyHistoryPreservingStopRequested('pty-left') + runtime.preservePtyHistoryThroughLateExit('pty-left', stopRequestId) + runtime.onPtyExit('pty-left', 0, 'incarnation-left') + + expect((await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs).toEqual([ + expect.objectContaining({ id: 'tab::left' }), + expect.objectContaining({ id: 'tab::right' }) + ]) + + runtime.onPtyExit('pty-left', 0, 'incarnation-left') + expect( + (await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs.find( + (tab) => tab.id === 'tab::left' + ) + ).toBeUndefined() + }) + it('ignores a delayed exit from an older incarnation of a reused PTY id', async () => { const setWorkspaceSession = vi.fn() const runtime = new OrcaRuntimeService( diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 8b2271a9db0..77a6fa795b2 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -3091,9 +3091,12 @@ export class OrcaRuntimeService { // Why: provider exit can beat surface registration; that exact dead incarnation must never publish. private earlyExitedPtyIncarnations = new Map() private pendingPtyRegistrationIncarnations = new Map() - // Why: exact-stop is the current sleep transaction boundary; its exit must - // leave the renderer's intentional sleeping surface available for wake. - private intentionalHandlelessPtyStops = new Map() + // Why: every overlapping hibernation owns its lease until its stop settles. + private intentionalHandlelessPtyStops = new Map< + string, + Map + >() + private nextIntentionalHandlelessPtyStopId = 1 // Why: coalesces title/status-driven session.tabs emits so spinner churn // doesn't fan out (and per-client JSON.stringify) a snapshot several times a // second. Emit reads the latest snapshot, so only the freshest version ships. @@ -15217,10 +15220,27 @@ export class OrcaRuntimeService { exitIncarnationId ?? pendingIncarnation ?? pty?.incarnationId ?? null ) } - const intentionalStopIncarnation = this.intentionalHandlelessPtyStops.get(ptyId) + const intentionalStops = this.intentionalHandlelessPtyStops.get(ptyId) const preservesIntentionalHandlelessSurface = - this.intentionalHandlelessPtyStops.has(ptyId) && - (intentionalStopIncarnation === null || intentionalStopIncarnation === incarnationId) + intentionalStops !== undefined && + [...intentionalStops.values()].some( + (intentionalStop) => + intentionalStop.incarnationId === null || intentionalStop.incarnationId === incarnationId + ) + if (intentionalStops) { + for (const [requestId, intentionalStop] of intentionalStops) { + if ( + intentionalStop.awaitsLateExit && + (intentionalStop.incarnationId === null || + intentionalStop.incarnationId === incarnationId) + ) { + intentionalStops.delete(requestId) + } + } + if (intentionalStops.size === 0) { + this.intentionalHandlelessPtyStops.delete(ptyId) + } + } advertisedUrlWatcher.unbindPty(ptyId) // Clean up new mobile state for this PTY this.mobileSubscribers.delete(ptyId) @@ -18211,12 +18231,32 @@ export class OrcaRuntimeService { this.stopRequestedPtyIds.add(ptyId) } - markPtyHistoryPreservingStopRequested(ptyId: string): void { - this.intentionalHandlelessPtyStops.set(ptyId, this.ptysById.get(ptyId)?.incarnationId ?? null) + markPtyHistoryPreservingStopRequested(ptyId: string): number { + const incarnationId = this.ptysById.get(ptyId)?.incarnationId ?? null + const requestId = this.nextIntentionalHandlelessPtyStopId + this.nextIntentionalHandlelessPtyStopId += 1 + const requests = this.intentionalHandlelessPtyStops.get(ptyId) ?? new Map() + requests.set(requestId, { incarnationId, awaitsLateExit: false }) + this.intentionalHandlelessPtyStops.set(ptyId, requests) + return requestId } - clearPtyHistoryPreservingStopRequested(ptyId: string): void { - this.intentionalHandlelessPtyStops.delete(ptyId) + preservePtyHistoryThroughLateExit(ptyId: string, requestId: number): void { + const request = this.intentionalHandlelessPtyStops.get(ptyId)?.get(requestId) + if (request) { + request.awaitsLateExit = true + } + } + + clearPtyHistoryPreservingStopRequested(ptyId: string, requestId: number): void { + const requests = this.intentionalHandlelessPtyStops.get(ptyId) + if (!requests) { + return + } + requests.delete(requestId) + if (requests.size === 0) { + this.intentionalHandlelessPtyStops.delete(ptyId) + } } isPtyStopRequested(ptyId: string): boolean { @@ -30798,15 +30838,17 @@ export class OrcaRuntimeService { const stoppedPtyIds: string[] = [] for (const ptyId of [...expected].sort()) { - if (opts.keepHistory) { - this.markPtyHistoryPreservingStopRequested(ptyId) - } + const historyPreservingStopId = opts.keepHistory + ? this.markPtyHistoryPreservingStopRequested(ptyId) + : null try { if (!(await this.ptyController.stopAndWait(ptyId, { keepHistory: opts.keepHistory }))) { throw Object.assign(new Error('terminal_exact_stop_failed'), { ptyId }) } } finally { - this.clearPtyHistoryPreservingStopRequested(ptyId) + if (historyPreservingStopId !== null) { + this.clearPtyHistoryPreservingStopRequested(ptyId, historyPreservingStopId) + } } stoppedPtyIds.push(ptyId) } diff --git a/src/renderer/src/store/slices/store-sleep-pane-hibernation.test.ts b/src/renderer/src/store/slices/store-sleep-pane-hibernation.test.ts index 11bba797aac..fccbafe3962 100644 --- a/src/renderer/src/store/slices/store-sleep-pane-hibernation.test.ts +++ b/src/renderer/src/store/slices/store-sleep-pane-hibernation.test.ts @@ -332,6 +332,88 @@ describe('shutdownWorktreeTerminals (sleep) — agent status hygiene', () => { expect(state.sleepingAgentSessionsByPaneKey[targetPaneKey]).toBeDefined() }) + it('does not clear a replacement pane that binds while hibernation is stopping', async () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + const targetLeaf = '11111111-1111-4111-8111-111111111111' + const targetPaneKey = `tab-1:${targetLeaf}` + let finishKill: (() => void) | undefined + mockApi.pty.kill.mockImplementationOnce( + () => + new Promise((resolve) => { + finishKill = resolve + }) + ) + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + }, + tabsByWorktree: { + [wt]: [makeTab({ id: 'tab-1', worktreeId: wt, title: 'Codex', ptyId: 'pty-agent' })] + }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: targetLeaf }, + activeLeafId: targetLeaf, + expandedLeafId: null, + ptyIdsByLeafId: { [targetLeaf]: 'pty-agent' } + } + }, + ptyIdsByTabId: { 'tab-1': ['pty-agent'] } + }) + store + .getState() + .setAgentStatus( + targetPaneKey, + { state: 'done', prompt: 'old run', agentType: 'codex' }, + 'Codex', + { updatedAt: 2000, stateStartedAt: 1000 }, + { tabId: 'tab-1', worktreeId: wt }, + { providerSession: { key: 'session_id', id: 'old-session' } } + ) + + const shutdown = store.getState().shutdownCompletedAgentPaneForHibernation(wt, { + paneKey: targetPaneKey, + tabId: 'tab-1', + leafId: targetLeaf, + ptyId: 'pty-agent' + }) + store.setState((state) => ({ + tabsByWorktree: { + ...state.tabsByWorktree, + [wt]: state.tabsByWorktree[wt]!.map((tab) => + tab.id === 'tab-1' ? { ...tab, ptyId: 'pty-replacement' } : tab + ) + }, + terminalLayoutsByTabId: { + ...state.terminalLayoutsByTabId, + 'tab-1': { + ...state.terminalLayoutsByTabId['tab-1']!, + ptyIdsByLeafId: { [targetLeaf]: 'pty-replacement' } + } + }, + ptyIdsByTabId: { ...state.ptyIdsByTabId, 'tab-1': ['pty-replacement'] } + })) + store + .getState() + .setAgentStatus( + targetPaneKey, + { state: 'working', prompt: 'replacement run', agentType: 'codex' }, + 'Codex', + { updatedAt: 3000, stateStartedAt: 3000 }, + { tabId: 'tab-1', worktreeId: wt }, + { providerSession: { key: 'session_id', id: 'replacement-session' } } + ) + finishKill?.() + await shutdown + + expect(store.getState().ptyIdsByTabId['tab-1']).toEqual(['pty-replacement']) + expect(store.getState().agentStatusByPaneKey[targetPaneKey]).toMatchObject({ + state: 'working', + providerSession: { key: 'session_id', id: 'replacement-session' } + }) + }) + it('keeps manual sleep worktree-wide', async () => { const store = createTestStore() const wt = 'repo1::/path/wt1' diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 4e570420a11..42bf3f191ce 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -2771,6 +2771,21 @@ export const createTerminalSlice: StateCreator } } + const stateAfterShutdown = get() + const tabAfterShutdown = (stateAfterShutdown.tabsByWorktree[worktreeId] ?? []).find( + (candidate) => candidate.id === opts.tabId + ) + const panePtyAfterShutdown = + stateAfterShutdown.terminalLayoutsByTabId[opts.tabId]?.ptyIdsByLeafId?.[opts.leafId] + if ( + !tabAfterShutdown || + tabAfterShutdown.createdAt !== tab.createdAt || + (panePtyAfterShutdown !== undefined && panePtyAfterShutdown !== opts.ptyId) + ) { + // Why: the stopped process is gone, but a replacement pane owns all later store cleanup. + return + } + set((s) => { const existingPtyIds = s.ptyIdsByTabId[opts.tabId] ?? [] const shutdownPtyIdSet = new Set(rendererShutdownPtyIds)