From 3a71d3f3e4c507ee41882e33f6862140a8d94702 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:50:44 -0700 Subject: [PATCH] fix(codex): execute accepted account-switch restarts for unmounted panes Accepting the Codex account-switch restart prompt queued every awaiting pane but only mounted TerminalPane instances executed the queue, so background-tab and parked panes stayed input-blocked on the old account with no prompt. - Add a detached store-level driver (codex-detached-pane-restart) that kill-and-respawns any queued pane no mounted transport claims, rebinding tab/layout state so a later mount reattaches to the replacement PTY. - Re-offer the prompt when a detached execution fails, and clear the notice when the pane is gone, so input is never silently blocked. - Sweep restored PTY ids at startup so stale panes in never-mounted tabs are re-offered after an app restart. - Carry launchAgent codex on the restart respawn so it waits for managed-auth readiness and records the pane's launch account. --- src/renderer/src/App.tsx | 7 + .../components/terminal-pane/TerminalPane.tsx | 3 + .../codex-detached-pane-restart.test.ts | 299 ++++++++++++++++++ .../codex-detached-pane-restart.ts | 297 +++++++++++++++++ .../src/lib/codex-session-restart.test.ts | 7 +- src/renderer/src/lib/codex-session-restart.ts | 7 +- .../src/lib/codex-stale-pane-sweep.test.ts | 25 +- .../src/lib/codex-stale-pane-sweep.ts | 19 ++ .../codex-restart-notice-lifecycle.test.ts | 59 ++++ src/renderer/src/store/slices/terminals.ts | 39 +++ 10 files changed, 759 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts create mode 100644 src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 179108b194b..63fcf8d6ec5 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -123,6 +123,8 @@ import { shouldPersistWorkspaceSession } from './lib/workspace-session' import { createSessionWriteSubscriber } from './lib/session-write-subscriber' +import { sweepRestoredCodexPanesForStaleAccounts } from './lib/codex-stale-pane-sweep' +import { installCodexDetachedPaneRestartExecutor } from '@/components/terminal-pane/codex-detached-pane-restart' import { buildActiveViewUnloadPatch } from './lib/active-view-persist' import { buildWorkspaceSessionHostSnapshots, @@ -1090,6 +1092,9 @@ function App(): React.JSX.Element { await timeRendererStartupStep('recover-legacy-worker-terminals-post-reconnect', () => window.api.app.recoverLegacyWorkerTerminalsForRendererStartup() ) + // Why here: reconnect just published restored PTY ids; sweeping them now + // re-offers stale Codex panes whose tabs never mount this session. + sweepRestoredCodexPanesForStaleAccounts(useAppStore.getState()) syncZoomCSSVar() // Why (issue #1158): unlock the session writer only after hydration and all dependent steps succeeded, so a mid-startup throw can't serialize partially-mutated state to disk. actions.setHydrationSucceeded(true) @@ -1210,6 +1215,8 @@ function App(): React.JSX.Element { } }, []) + useEffect(() => installCodexDetachedPaneRestartExecutor(), []) + useEffect(() => { let previousKey = getRuntimeMobileSessionSyncKey(useAppStore.getState()) return useAppStore.subscribe((state, previousState) => { diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 2cd4d3c0d8b..bfb2377bfb9 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -1709,6 +1709,9 @@ function TerminalPane( // transport, so a queued restart has no ptyId to match on the mount pass. The // reconnected PTY rewrites this map when it binds — `ptyIdsByTabId` does not, // because a restored id is already listed there before the pane ever mounts. + // Panes with no mounted TerminalPane at all are executed by the detached + // driver instead (codex-detached-pane-restart), which leaves anything a live + // transport owns to this effect. const panePtyLayoutBindings = savedLayout.ptyIdsByLeafId useEffect(() => { const manager = managerRef.current diff --git a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts new file mode 100644 index 00000000000..bb2dca129b5 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts @@ -0,0 +1,299 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { registerRuntimeTerminalTab } from '@/runtime/sync-runtime-graph' +import { awaitsCodexRestartAnswer, blocksCodexPaneInput } from '../codex-restart-notice-state' +import { ptyDataHandlers } from './pty-dispatcher' +import { + installCodexDetachedPaneRestartExecutor, + resetCodexDetachedPaneRestartExecutorForTests, + sweepUnclaimedCodexPaneRestarts +} from './codex-detached-pane-restart' + +const ACCOUNT_A = 'a@example.com' +const ACCOUNT_B = 'b@example.com' +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const OLD_PTY = 'wt1@@old' +const NEW_PTY = 'wt1@@new' + +function seedQueuedRestart( + opts: { leafId?: string | null; layoutRoot?: 'leaf' | 'none' } = {} +): void { + const leafId = opts.leafId === undefined ? LEAF_ID : opts.leafId + useAppStore.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + worktreesByRepo: { + repo1: [{ id: 'wt1', path: '/Users/dev/code/orca' }] + } as never, + tabsByWorktree: { + wt1: [ + { + id: 'tab-1', + ptyId: OLD_PTY, + worktreeId: 'wt1', + title: 'codex', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + launchAgent: 'codex' as const + } + ] + } as never, + ptyIdsByTabId: { 'tab-1': [OLD_PTY] }, + terminalLayoutsByTabId: leafId + ? { + 'tab-1': { + root: opts.layoutRoot === 'none' ? null : { type: 'leaf' as const, leafId }, + activeLeafId: leafId, + expandedLeafId: null, + ptyIdsByLeafId: { [leafId]: OLD_PTY } + } + } + : {} + }) + useAppStore + .getState() + .markCodexRestartNotices([ + { ptyId: OLD_PTY, previousAccountLabel: ACCOUNT_A, nextAccountLabel: ACCOUNT_B } + ]) + useAppStore.getState().queueCodexPaneRestarts([OLD_PTY]) +} + +describe('codex detached pane restart executor', () => { + const originalWindow = (globalThis as { window?: typeof window }).window + + beforeEach(() => { + resetCodexDetachedPaneRestartExecutorForTests() + useAppStore.setState(useAppStore.getInitialState(), true) + ;(globalThis as { window: typeof window }).window = { + ...originalWindow, + api: { + ...originalWindow?.api, + pty: { + ...originalWindow?.api?.pty, + getSize: vi.fn().mockResolvedValue({ cols: 120, rows: 30 }), + spawn: vi.fn().mockResolvedValue({ id: NEW_PTY }), + kill: vi.fn().mockResolvedValue(undefined) + } + } + } as unknown as typeof window + }) + + afterEach(() => { + resetCodexDetachedPaneRestartExecutorForTests() + ptyDataHandlers.delete(OLD_PTY) + vi.useRealTimers() + if (originalWindow) { + ;(globalThis as { window: typeof window }).window = originalWindow + } else { + delete (globalThis as { window?: typeof window }).window + } + }) + + it('kill-and-respawns an accepted restart no mounted transport claims', async () => { + seedQueuedRestart() + + await sweepUnclaimedCodexPaneRestarts() + + expect(window.api.pty.spawn).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + cols: 120, + rows: 30, + cwd: '/Users/dev/code/orca', + command: 'codex', + startupCommandDelivery: 'shell-ready', + launchAgent: 'codex', + worktreeId: 'wt1', + tabId: 'tab-1', + leafId: LEAF_ID, + initiallyHidden: true + }) + ) + expect(vi.mocked(window.api.pty.spawn).mock.calls[0]?.[0]?.env).toEqual( + expect.objectContaining({ + ORCA_PANE_KEY: `tab-1:${LEAF_ID}`, + ORCA_TAB_ID: 'tab-1', + ORCA_WORKTREE_ID: 'wt1', + ORCA_WORKSPACE_ID: 'wt1' + }) + ) + expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY) + + const state = useAppStore.getState() + expect(state.ptyIdsByTabId['tab-1']).toEqual([NEW_PTY]) + expect(state.tabsByWorktree.wt1?.[0]?.ptyId).toBe(NEW_PTY) + expect(state.terminalLayoutsByTabId['tab-1']?.ptyIdsByLeafId).toEqual({ [LEAF_ID]: NEW_PTY }) + expect(state.pendingCodexPaneRestartIds).toEqual({}) + // The whole point: the restart completed, so nothing may block input anymore. + expect(blocksCodexPaneInput(state.codexRestartNoticeByPtyId[OLD_PTY])).toBe(false) + expect(blocksCodexPaneInput(state.codexRestartNoticeByPtyId[NEW_PTY])).toBe(false) + }) + + it('executes via the store subscription once the claim grace elapses', async () => { + vi.useFakeTimers() + const uninstall = installCodexDetachedPaneRestartExecutor() + try { + seedQueuedRestart() + expect(window.api.pty.spawn).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(400) + + expect(window.api.pty.spawn).toHaveBeenCalledTimes(1) + expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY) + } finally { + uninstall() + } + }) + + it('leaves a PTY owned by a mounted transport to the pane effect', async () => { + seedQueuedRestart() + ptyDataHandlers.set(OLD_PTY, () => {}) + + await sweepUnclaimedCodexPaneRestarts() + + expect(window.api.pty.spawn).not.toHaveBeenCalled() + expect(window.api.pty.kill).not.toHaveBeenCalled() + expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({ [OLD_PTY]: true }) + }) + + it('leaves foreign-machine PTYs queued for their mounted pane path', async () => { + useAppStore.setState({ pendingCodexPaneRestartIds: { 'remote:term-1': true } }) + + await sweepUnclaimedCodexPaneRestarts() + + expect(window.api.pty.spawn).not.toHaveBeenCalled() + expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({ 'remote:term-1': true }) + }) + + it('clears the notice when the queued pane no longer exists', async () => { + useAppStore + .getState() + .markCodexRestartNotices([ + { ptyId: 'wt1@@gone', previousAccountLabel: ACCOUNT_A, nextAccountLabel: ACCOUNT_B } + ]) + useAppStore.getState().queueCodexPaneRestarts(['wt1@@gone']) + + await sweepUnclaimedCodexPaneRestarts() + + const state = useAppStore.getState() + expect(window.api.pty.spawn).not.toHaveBeenCalled() + expect(state.pendingCodexPaneRestartIds).toEqual({}) + expect(blocksCodexPaneInput(state.codexRestartNoticeByPtyId['wt1@@gone'])).toBe(false) + }) + + it('rebuilds a rootless single-pane layout so the mount replays this leaf', async () => { + // Regression guard: replayTerminalLayout mints a fresh leaf when the root + // doesn't name this one, silently orphaning the respawned PTY on reveal. + seedQueuedRestart({ layoutRoot: 'none' }) + + await sweepUnclaimedCodexPaneRestarts() + + expect(useAppStore.getState().terminalLayoutsByTabId['tab-1']).toEqual( + expect.objectContaining({ + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + ptyIdsByLeafId: { [LEAF_ID]: NEW_PTY } + }) + ) + expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY) + }) + + it('rebinds only the codex leaf of a split and keeps the sibling', async () => { + const SIBLING_LEAF = '22222222-2222-4222-8222-222222222222' + seedQueuedRestart() + useAppStore.setState({ + ptyIdsByTabId: { 'tab-1': [OLD_PTY, 'wt1@@sibling'] }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_ID }, + second: { type: 'leaf', leafId: SIBLING_LEAF } + }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: OLD_PTY, [SIBLING_LEAF]: 'wt1@@sibling' } + } + } + }) + + await sweepUnclaimedCodexPaneRestarts() + + const layout = useAppStore.getState().terminalLayoutsByTabId['tab-1'] + expect(layout?.root?.type).toBe('split') + expect(layout?.ptyIdsByLeafId).toEqual({ + [LEAF_ID]: NEW_PTY, + [SIBLING_LEAF]: 'wt1@@sibling' + }) + // Split-pane safety: only the codex pane's PTY dies. + expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY) + expect(useAppStore.getState().ptyIdsByTabId['tab-1']).toEqual([NEW_PTY, 'wt1@@sibling']) + }) + + it('leaves a tab with a mounted TerminalPane to its own restart effect', async () => { + seedQueuedRestart() + const unregister = registerRuntimeTerminalTab({ + tabId: 'tab-1', + worktreeId: 'wt1', + getManager: () => null, + getContainer: () => null, + getPtyIdForPane: () => null + }) + try { + await sweepUnclaimedCodexPaneRestarts() + + expect(window.api.pty.spawn).not.toHaveBeenCalled() + expect(window.api.pty.kill).not.toHaveBeenCalled() + expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({ [OLD_PTY]: true }) + } finally { + unregister() + } + }) + + it('leaves a sleep-retained pending id alone so wake can migrate it', async () => { + // Why: hibernation unbinds a pane's PTY but keeps its pending restart, and + // wake moves that entry onto the respawned PTY. No notice means nothing is + // blocked, so consuming here would silently lose the accepted restart. + useAppStore.setState({ pendingCodexPaneRestartIds: { 'wt1@@sleeping': true } }) + + await sweepUnclaimedCodexPaneRestarts() + + expect(window.api.pty.spawn).not.toHaveBeenCalled() + expect(window.api.pty.kill).not.toHaveBeenCalled() + expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({ 'wt1@@sleeping': true }) + }) + + it('re-offers the prompt instead of leaving a silent block when the respawn fails', async () => { + seedQueuedRestart() + vi.mocked(window.api.pty.spawn).mockRejectedValue(new Error('managed auth unavailable')) + + await sweepUnclaimedCodexPaneRestarts() + + const state = useAppStore.getState() + expect(window.api.pty.kill).not.toHaveBeenCalled() + expect(state.ptyIdsByTabId['tab-1']).toEqual([OLD_PTY]) + expect(state.pendingCodexPaneRestartIds).toEqual({}) + // The question is back on screen; input stays blocked but never silently. + expect(awaitsCodexRestartAnswer(state.codexRestartNoticeByPtyId[OLD_PTY])).toBe(true) + }) + + it('kills now and defers the Codex respawn to mount when the layout leaf is unknown', async () => { + seedQueuedRestart({ leafId: null }) + + await sweepUnclaimedCodexPaneRestarts() + + const state = useAppStore.getState() + expect(window.api.pty.spawn).not.toHaveBeenCalled() + expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY) + expect(state.ptyIdsByTabId['tab-1']).toEqual([]) + expect(state.pendingStartupByTabId['tab-1']).toEqual( + expect.objectContaining({ + command: 'codex', + startupCommandDelivery: 'shell-ready', + launchAgent: 'codex' + }) + ) + expect(blocksCodexPaneInput(state.codexRestartNoticeByPtyId[OLD_PTY])).toBe(false) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts new file mode 100644 index 00000000000..0a024710f8e --- /dev/null +++ b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts @@ -0,0 +1,297 @@ +/** + * Detached executor for accepted Codex account-switch restarts. + * + * Why: queued restarts used to execute only inside mounted TerminalPane + * instances, so accepting the prompt stranded every unmounted pane — prompt + * gone, keyboard blocked, Codex still running under the old account until the + * tab was next revealed. This driver watches pendingCodexPaneRestartIds and + * kill-and-respawns any pane no mounted transport claims, rebinding the store + * so a later mount reattaches to the replacement PTY like a restored session. + */ +import { isTerminalLeafId, makePaneKey } from '../../../../shared/stable-pane-id' +import { parseWorkspaceKey } from '../../../../shared/workspace-scope' +import type { TerminalPaneLayoutNode, TerminalTab } from '../../../../shared/types' +import type { AppState } from '@/store' +import { useAppStore } from '@/store' +import { getWorktreeMapFromState } from '@/store/selectors' +import { singlePaneLayoutSnapshot } from '@/store/slices/terminal-helpers' +import { hasRegisteredRuntimeTerminalTab } from '@/runtime/sync-runtime-graph' +import { CODEX_ACCOUNT_RESTART_STARTUP } from '@/lib/codex-session-restart' +import { isForeignMachineCodexPtyId } from '@/lib/codex-pane-selection-lane' +import { getLocalProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' +import { + getCachedWindowsTerminalCapabilities, + hasCachedWindowsTerminalCapabilities +} from '@/lib/windows-terminal-capabilities' +import { ptyDataHandlers, unregisterPtyDataHandlers } from './pty-dispatcher' +import { discardPreHandlerPtyState } from './pty-pre-handler-buffer' +import { disposeParkedTerminalWatchersForPtyIds } from './terminal-parked-watcher-registry' + +// Why this long: a mounted pane claims its pending restart from a React effect +// in the same commit that observed the queue write, so anything still pending +// after a few frames has no pane coming for it. +const CLAIM_GRACE_MS = 400 + +let sweepTimer: ReturnType | null = null +const inFlightPtyIds = new Set() + +/** Installed once at app startup; returns the uninstaller (tests). */ +export function installCodexDetachedPaneRestartExecutor(): () => void { + const unsubscribe = useAppStore.subscribe((state, previousState) => { + if (state.pendingCodexPaneRestartIds === previousState.pendingCodexPaneRestartIds) { + return + } + scheduleClaimSweep() + }) + return () => { + unsubscribe() + if (sweepTimer !== null) { + clearTimeout(sweepTimer) + sweepTimer = null + } + } +} + +export function resetCodexDetachedPaneRestartExecutorForTests(): void { + if (sweepTimer !== null) { + clearTimeout(sweepTimer) + sweepTimer = null + } + inFlightPtyIds.clear() +} + +function scheduleClaimSweep(): void { + if (sweepTimer !== null) { + return + } + sweepTimer = setTimeout(() => { + sweepTimer = null + void sweepUnclaimedCodexPaneRestarts() + }, CLAIM_GRACE_MS) +} + +export async function sweepUnclaimedCodexPaneRestarts(): Promise { + for (const ptyId of Object.keys(useAppStore.getState().pendingCodexPaneRestartIds)) { + // Why: remote-runtime spawns need that machine's transport assembly, which + // only the mounted pane path carries today; leave those queued for mount. + if (isForeignMachineCodexPtyId(ptyId)) { + continue + } + // Why: a live primary handler means a mounted pane owns this PTY, and its + // restart effect re-runs on both the queue write and the transport bind — + // it is guaranteed to claim, and only it can reconnect the xterm in place. + if (ptyDataHandlers.has(ptyId) || inFlightPtyIds.has(ptyId)) { + continue + } + const state = useAppStore.getState() + const located = locateCodexPane(state, ptyId) + if (!located) { + // Why not consume: a sleep-retained pending id is unbound on purpose and + // wake migrates it onto the respawned PTY — taking it here would lose + // that restart. Only a notice still muting input forces a resolution. + if (state.codexRestartNoticeByPtyId[ptyId]) { + if (state.consumePendingCodexPaneRestart(ptyId)) { + state.clearCodexRestartNotice(ptyId) + } + } + continue + } + // Why the registry check too: a revealed tab reads its layout into a ref at + // mount, before its transports bind (and register a primary handler). A + // takeover in that window would kill the PTY the pane is attaching to. + if (hasRegisteredRuntimeTerminalTab(located.tab.id)) { + continue + } + if (!useAppStore.getState().consumePendingCodexPaneRestart(ptyId)) { + continue + } + inFlightPtyIds.add(ptyId) + try { + await executeDetachedCodexPaneRestart(located, ptyId) + } catch (err) { + console.warn('[codex-restart] detached pane restart failed:', err) + // Why: the answered-but-unexecuted state is the one this module exists to + // prevent — put the question back rather than leave a silent input block. + useAppStore.getState().reopenCodexRestartPrompt(ptyId) + } finally { + inFlightPtyIds.delete(ptyId) + } + } +} + +type LocatedCodexPane = { + worktreeId: string + tab: TerminalTab + leafId: string | null +} + +function locateCodexPane(state: AppState, ptyId: string): LocatedCodexPane | null { + for (const [worktreeId, tabs] of Object.entries(state.tabsByWorktree)) { + for (const tab of tabs) { + if (tab.ptyId !== ptyId && !(state.ptyIdsByTabId[tab.id] ?? []).includes(ptyId)) { + continue + } + const leafId = + Object.entries(state.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId ?? {}).find( + ([, boundPtyId]) => boundPtyId === ptyId + )?.[0] ?? null + // Why the format check: pre-UUID layouts carry numeric leaf ids, which the + // pane-key env and main's binding flush both reject. + return { + worktreeId, + tab, + leafId: leafId !== null && isTerminalLeafId(leafId) ? leafId : null + } + } + } + return null +} + +function getWorkspacePath(state: AppState, worktreeId: string): string | null { + const parsed = parseWorkspaceKey(worktreeId) + if (parsed?.type === 'folder') { + return ( + (state.folderWorkspaces ?? []).find((workspace) => workspace.id === parsed.folderWorkspaceId) + ?.folderPath ?? null + ) + } + return getWorktreeMapFromState(state).get(worktreeId)?.path ?? null +} + +function buildPaneIdentityEnv( + state: AppState, + worktreeId: string, + tabId: string, + leafId: string +): Record { + const parsed = parseWorkspaceKey(worktreeId) + const folderWorkspace = + parsed?.type === 'folder' + ? state.folderWorkspaces.find((workspace) => workspace.id === parsed.folderWorkspaceId) + : null + return { + ORCA_WORKSPACE_ID: worktreeId, + ...(folderWorkspace + ? { + ORCA_PROJECT_GROUP_ID: folderWorkspace.projectGroupId, + ORCA_WORKSPACE_ROOT: folderWorkspace.folderPath + } + : {}), + ORCA_PANE_KEY: makePaneKey(tabId, leafId), + ORCA_TAB_ID: tabId, + ORCA_WORKTREE_ID: worktreeId + } +} + +async function executeDetachedCodexPaneRestart( + located: LocatedCodexPane, + ptyId: string +): Promise { + const state = useAppStore.getState() + if (!located.leafId) { + // Why: without a usable layout leaf the replacement cannot be bound in + // place, so kill now and let the tab's next mount run the Codex startup. + await killReplacedCodexPanePty(ptyId) + const store = useAppStore.getState() + store.clearTabPtyId(located.tab.id, ptyId) + store.queueTabStartupCommand(located.tab.id, { ...CODEX_ACCOUNT_RESTART_STARTUP }) + store.clearCodexRestartNotice(ptyId) + return + } + const { worktreeId, tab, leafId } = located + + const size = await window.api.pty.getSize(ptyId).catch(() => null) + const workspacePath = getWorkspacePath(state, worktreeId) + const cwd = tab.startupCwd ?? workspacePath ?? undefined + const capabilities = hasCachedWindowsTerminalCapabilities() + ? getCachedWindowsTerminalCapabilities() + : null + // Why: same runtime context the mounted spawn ships (pty-connection.ts), so a + // WSL-defaulted project respawns into the same distro it launched from. + const projectRuntime = getLocalProjectExecutionRuntimeContext(state, worktreeId, undefined, { + wslAvailable: capabilities?.wslAvailable, + availableWslDistros: capabilities?.wslDistros ?? null + }) + + const spawned = await window.api.pty.spawn({ + cols: size?.cols ?? 80, + rows: size?.rows ?? 24, + ...(cwd ? { cwd } : {}), + cwdFallback: 'worktree', + env: buildPaneIdentityEnv(state, worktreeId, tab.id, leafId), + command: CODEX_ACCOUNT_RESTART_STARTUP.command, + startupCommandDelivery: CODEX_ACCOUNT_RESTART_STARTUP.startupCommandDelivery, + launchAgent: CODEX_ACCOUNT_RESTART_STARTUP.launchAgent, + worktreeId, + tabId: tab.id, + leafId, + ...(tab.shellOverride ? { shellOverride: tab.shellOverride } : {}), + ...(projectRuntime ? { projectRuntime } : {}), + initiallyHidden: true + }) + + const store = useAppStore.getState() + store.updateTabPtyId(tab.id, spawned.id, ptyId) + if (!useAppStore.getState().ptyIdsByTabId[tab.id]?.includes(spawned.id)) { + // Why: the tab was retired while the spawn was in flight; without a binding + // the fresh PTY would idle in the daemon forever, so reap it and stand down. + store.clearCodexRestartNotice(ptyId) + await window.api.pty.kill(spawned.id).catch(() => {}) + return + } + rebindCodexPaneLayoutLeaf(tab.id, leafId, spawned.id) + // Why both ids: updateTabPtyId migrates the replaced pane's notice onto the + // new PTY; the restart it recorded is now done, so the block must lift. + store.clearCodexRestartNotice(spawned.id) + store.clearCodexRestartNotice(ptyId) + + await killReplacedCodexPanePty(ptyId) +} + +function layoutRootContainsLeaf( + node: TerminalPaneLayoutNode | null | undefined, + leafId: string +): boolean { + if (!node) { + return false + } + if (node.type === 'leaf') { + return node.leafId === leafId + } + return layoutRootContainsLeaf(node.first, leafId) || layoutRootContainsLeaf(node.second, leafId) +} + +function rebindCodexPaneLayoutLeaf(tabId: string, leafId: string, newPtyId: string): void { + const store = useAppStore.getState() + const layout = store.terminalLayoutsByTabId[tabId] + const boundLeafIds = Object.keys(layout?.ptyIdsByLeafId ?? {}) + // Why: mount replays panes from the root — a root that doesn't name this leaf + // mints a fresh one and silently orphans the replacement PTY. Rewriting is + // only safe when this is the tab's sole bound pane; a split keeps its root. + if (!layoutRootContainsLeaf(layout?.root, leafId) && boundLeafIds.every((id) => id === leafId)) { + store.setTabLayout( + tabId, + singlePaneLayoutSnapshot(leafId, newPtyId, layout?.titlesByLeafId?.[leafId] ?? null) + ) + return + } + store.replaceTerminalLayoutPanePtyId(tabId, leafId, newPtyId) +} + +async function killReplacedCodexPanePty(ptyId: string): Promise { + // Why the disposal: a parked tab's exit sidecar treats any exit as the pane + // dying — it would collapse the just-rebound leaf or close the whole tab. + disposeParkedTerminalWatchersForPtyIds([ptyId]) + useAppStore.getState().suppressPtyExit(ptyId) + for (const snapshot of unregisterPtyDataHandlers([ptyId])) { + snapshot.commit() + } + try { + await window.api.pty.kill(ptyId) + } catch (err) { + // Why not rethrow: the stale shell can only be reaped, not re-adopted, so a + // failed kill is log-worthy only — the restart itself already happened. + console.warn('[codex-restart] failed to kill replaced Codex pane PTY:', err) + } + discardPreHandlerPtyState(ptyId) +} diff --git a/src/renderer/src/lib/codex-session-restart.test.ts b/src/renderer/src/lib/codex-session-restart.test.ts index 5a478df23f7..0f659aa2521 100644 --- a/src/renderer/src/lib/codex-session-restart.test.ts +++ b/src/renderer/src/lib/codex-session-restart.test.ts @@ -27,9 +27,14 @@ function setLaunchAgentOnFirstTab(launchAgent: TuiAgent): void { describe('CODEX_ACCOUNT_RESTART_STARTUP', () => { it('waits for shell readiness before relaunching Codex after an account switch', () => { + // Why launchAgent is load-bearing: pty:spawn runs the managed-auth + // readiness gate and Codex launch prep only for launchAgent 'codex', so + // dropping it would let a restart respawn race the account handoff and + // record a launch account the pane does not actually read. expect(CODEX_ACCOUNT_RESTART_STARTUP).toEqual({ command: 'codex', - startupCommandDelivery: 'shell-ready' + startupCommandDelivery: 'shell-ready', + launchAgent: 'codex' }) expect(shouldUseShellReadyStartupDelivery(CODEX_ACCOUNT_RESTART_STARTUP)).toBe(true) }) diff --git a/src/renderer/src/lib/codex-session-restart.ts b/src/renderer/src/lib/codex-session-restart.ts index 0a8ab5ca01a..4b3ed2e43b2 100644 --- a/src/renderer/src/lib/codex-session-restart.ts +++ b/src/renderer/src/lib/codex-session-restart.ts @@ -22,9 +22,14 @@ import type { TuiAgent } from '../../../shared/types' // Why: prompt integrations such as Starship can outlast the daemon's 300ms // Codex fast-path timeout; account restarts must wait until the shell accepts input. +// Why launchAgent: pty:spawn runs the managed-auth readiness gate and Codex +// launch prep (project trust pre-mark) only for launchAgent 'codex', so without +// it a restart respawn could race the account handoff and record a launch +// account the pane does not actually read. export const CODEX_ACCOUNT_RESTART_STARTUP = { command: 'codex', - startupCommandDelivery: 'shell-ready' + startupCommandDelivery: 'shell-ready', + launchAgent: 'codex' } as const export type CodexPaneScanResult = { diff --git a/src/renderer/src/lib/codex-stale-pane-sweep.test.ts b/src/renderer/src/lib/codex-stale-pane-sweep.test.ts index 390021b6bd1..9f793872f88 100644 --- a/src/renderer/src/lib/codex-stale-pane-sweep.test.ts +++ b/src/renderer/src/lib/codex-stale-pane-sweep.test.ts @@ -2,7 +2,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useAppStore } from '@/store' import { notifyCodexPaneBoundForStaleSweep, - resetCodexStalePaneSweepForTests + resetCodexStalePaneSweepForTests, + sweepRestoredCodexPanesForStaleAccounts } from './codex-stale-pane-sweep' const ACCOUNT_A = 'account-a@example.com' @@ -443,4 +444,26 @@ describe('notifyCodexPaneBoundForStaleSweep', () => { nextAccountId: 'account-b' }) }) + + it('re-raises the prompt for a restored pane whose tab never mounts', async () => { + // Regression: the bind-driven sweep needed a pane mount to fire, so a stale + // pane restored into a background tab stayed on the old account silently. + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + + sweepRestoredCodexPanesForStaleAccounts({ + ptyIdsByTabId: { 'tab-1': ['pty-1', 'remote:env-1@@term-1'] } + }) + await vi.advanceTimersByTimeAsync(300) + + expect(window.api.codexAccounts.listStalePanes).toHaveBeenCalledExactlyOnceWith({ + ptyIds: ['pty-1'] + }) + expect(inspectCallCountFor('remote:env-1@@term-1')).toBe(0) + expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + previousAccountId: 'account-a', + nextAccountId: 'account-b' + }) + }) }) diff --git a/src/renderer/src/lib/codex-stale-pane-sweep.ts b/src/renderer/src/lib/codex-stale-pane-sweep.ts index c75cea5fa95..9740e5c900f 100644 --- a/src/renderer/src/lib/codex-stale-pane-sweep.ts +++ b/src/renderer/src/lib/codex-stale-pane-sweep.ts @@ -47,6 +47,25 @@ export function notifyCodexPaneBoundForStaleSweep(ptyId: string): void { armForEarliestDue() } +/** + * Queues every restored PTY for the stale-account check without waiting for a + * pane to mount. + * + * Why: the bind-driven sweep above only fires when a pane's transport binds, so + * a stale Codex pane restored into a never-revealed tab kept running under the + * old account with no prompt — and an accepted-then-quit restart was never + * re-offered. Foreign-machine ids are dropped by the notify guard. + */ +export function sweepRestoredCodexPanesForStaleAccounts(state: { + ptyIdsByTabId: Record +}): void { + for (const ptyIds of Object.values(state.ptyIdsByTabId)) { + for (const ptyId of ptyIds) { + notifyCodexPaneBoundForStaleSweep(ptyId) + } + } +} + export function resetCodexStalePaneSweepForTests(): void { if (flushTimer !== null) { clearTimeout(flushTimer) diff --git a/src/renderer/src/store/slices/codex-restart-notice-lifecycle.test.ts b/src/renderer/src/store/slices/codex-restart-notice-lifecycle.test.ts index a13bbdeff7f..5c72a20b2aa 100644 --- a/src/renderer/src/store/slices/codex-restart-notice-lifecycle.test.ts +++ b/src/renderer/src/store/slices/codex-restart-notice-lifecycle.test.ts @@ -222,4 +222,63 @@ describe('codex restart notices with colliding account labels', () => { nextAccountId: 'account-c' }) }) + + it('puts a failed accepted restart back to the unanswered prompt', () => { + switchAccount('pty-1', A, B) + useAppStore.getState().queueCodexPaneRestarts(['pty-1']) + + useAppStore.getState().reopenCodexRestartPrompt('pty-1') + + // Why: the executor could not run the restart, so the pane must show the + // question again rather than stay muted behind an answered prompt. + expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ + previousAccountLabel: A, + nextAccountLabel: B + }) + expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({}) + }) + + it('leaves unanswered and dismissed notices alone on a reopen request', () => { + switchAccount('pty-1', A, B) + const unanswered = useAppStore.getState().codexRestartNoticeByPtyId + + useAppStore.getState().reopenCodexRestartPrompt('pty-1') + expect(useAppStore.getState().codexRestartNoticeByPtyId).toBe(unanswered) + + useAppStore.getState().dismissCodexRestartNotices(['pty-1']) + const dismissed = useAppStore.getState().codexRestartNoticeByPtyId + useAppStore.getState().reopenCodexRestartPrompt('pty-1') + expect(useAppStore.getState().codexRestartNoticeByPtyId).toBe(dismissed) + }) +}) + +describe('replaceTerminalLayoutPanePtyId', () => { + const LEAF = '11111111-1111-4111-8111-111111111111' + + it('rebinds one leaf and leaves split siblings alone', () => { + const OTHER_LEAF = '22222222-2222-4222-8222-222222222222' + useAppStore.setState({ + terminalLayoutsByTabId: { + 'tab-1': { + root: null, + activeLeafId: LEAF, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF]: 'pty-old', [OTHER_LEAF]: 'pty-sibling' } + } + } + }) + + useAppStore.getState().replaceTerminalLayoutPanePtyId('tab-1', LEAF, 'pty-new') + + expect(useAppStore.getState().terminalLayoutsByTabId['tab-1']?.ptyIdsByLeafId).toEqual({ + [LEAF]: 'pty-new', + [OTHER_LEAF]: 'pty-sibling' + }) + }) + + it('does nothing for a tab with no layout', () => { + const before = useAppStore.getState().terminalLayoutsByTabId + useAppStore.getState().replaceTerminalLayoutPanePtyId('tab-none', LEAF, 'pty-new') + expect(useAppStore.getState().terminalLayoutsByTabId).toBe(before) + }) }) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 1b670957ddb..03077f9d007 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -748,6 +748,11 @@ export type TerminalSlice = { ) => string[] clearCodexRestartNotice: (ptyId: string) => void dismissCodexRestartNotices: (ptyIds: string[]) => void + /** Puts an accepted-but-unexecuted restart back to the unanswered prompt, so a + * failed execution never leaves a pane input-blocked with nothing visible. */ + reopenCodexRestartPrompt: (ptyId: string) => void + /** Rebinds one layout leaf to a replacement PTY without a mounted pane. */ + replaceTerminalLayoutPanePtyId: (tabId: string, leafId: string, ptyId: string) => void setTabPaneExpanded: (tabId: string, expanded: boolean) => void setTabCanExpandPane: (tabId: string, canExpand: boolean) => void setTabLayout: (tabId: string, layout: TerminalLayoutSnapshot | null) => void @@ -3586,6 +3591,40 @@ export const createTerminalSlice: StateCreator }) }, + reopenCodexRestartPrompt: (ptyId) => { + set((s) => { + const notice = s.codexRestartNoticeByPtyId[ptyId] + if (!notice?.restartRequested) { + return {} + } + const { restartRequested: _restartRequested, ...kept } = notice + const nextPendingCodexPaneRestartIds = { ...s.pendingCodexPaneRestartIds } + delete nextPendingCodexPaneRestartIds[ptyId] + return { + codexRestartNoticeByPtyId: { ...s.codexRestartNoticeByPtyId, [ptyId]: kept }, + pendingCodexPaneRestartIds: nextPendingCodexPaneRestartIds + } + }) + }, + + replaceTerminalLayoutPanePtyId: (tabId, leafId, ptyId) => { + set((s) => { + const layout = s.terminalLayoutsByTabId[tabId] + if (!layout || layout.ptyIdsByLeafId?.[leafId] === ptyId) { + return {} + } + return { + terminalLayoutsByTabId: { + ...s.terminalLayoutsByTabId, + [tabId]: { + ...layout, + ptyIdsByLeafId: { ...layout.ptyIdsByLeafId, [leafId]: ptyId } + } + } + } + }) + }, + setTabPaneExpanded: (tabId, expanded) => { set((s) => ({ expandedPaneByTabId: { ...s.expandedPaneByTabId, [tabId]: expanded }