diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 79f2cf76531..6991294da5a 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -58,7 +58,9 @@ const { setMigrationUnsupportedPtyMock, clearMigrationUnsupportedPtyMock, clearMigrationUnsupportedPtysForPaneKeyMock, - clearPaneKeyAliasesForPtyMock + clearPaneKeyAliasesForPtyMock, + recordCodexPaneAccountMock, + forgetCodexPaneAccountMock } = vi.hoisted(() => ({ handleMock: vi.fn(), onMock: vi.fn(), @@ -90,7 +92,9 @@ const { setMigrationUnsupportedPtyMock: vi.fn(), clearMigrationUnsupportedPtyMock: vi.fn(), clearMigrationUnsupportedPtysForPaneKeyMock: vi.fn(), - clearPaneKeyAliasesForPtyMock: vi.fn() + clearPaneKeyAliasesForPtyMock: vi.fn(), + recordCodexPaneAccountMock: vi.fn(), + forgetCodexPaneAccountMock: vi.fn() })) vi.mock('electron', () => ({ @@ -193,6 +197,11 @@ vi.mock('../agent-hooks/migration-unsupported-pty-state', () => ({ clearMigrationUnsupportedPty: clearMigrationUnsupportedPtyMock, clearMigrationUnsupportedPtysForPaneKey: clearMigrationUnsupportedPtysForPaneKeyMock })) + +vi.mock('../codex/codex-pane-account-registry', () => ({ + recordCodexPaneAccount: recordCodexPaneAccountMock, + forgetCodexPaneAccount: forgetCodexPaneAccountMock +})) import { LocalPtyProvider } from '../providers/local-pty-provider' import { makePaneKey } from '../../shared/stable-pane-id' import { SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV } from '../../shared/setup-agent-sequencing' @@ -348,6 +357,8 @@ describe('registerPtyHandlers', () => { clearMigrationUnsupportedPtyMock.mockReset() clearMigrationUnsupportedPtysForPaneKeyMock.mockReset() clearPaneKeyAliasesForPtyMock.mockReset() + recordCodexPaneAccountMock.mockReset() + forgetCodexPaneAccountMock.mockReset() mainWindow.webContents.on.mockReset() mainWindow.webContents.send.mockReset() mainWindow.webContents.removeListener.mockReset() @@ -13504,6 +13515,36 @@ describe('registerPtyHandlers', () => { ) }) + it('records the launch Codex account for a fresh spawn but not for a reattach', async () => { + const spawn = vi + .fn() + .mockResolvedValueOnce({ id: 'pty-fresh' }) + .mockResolvedValueOnce({ id: 'pty-reattached', isReattach: true }) + setLocalPtyProvider({ + spawn, + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + shutdown: vi.fn(), + onData: vi.fn(() => vi.fn()), + onExit: vi.fn(() => vi.fn()), + listProcesses: vi.fn(async () => []), + getForegroundProcess: vi.fn(async () => null) + } as never) + const getSettings = vi.fn().mockReturnValue({ activeCodexManagedAccountId: 'account-a' }) + registerPtyHandlers(mainWindow as never, undefined, undefined, getSettings as never) + + await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24 }) + await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24, sessionId: 'pty-reattached' }) + + // Why: a reattached shell keeps the CODEX_HOME baked in at its original + // spawn, so re-recording it under the current selection would erase the only + // evidence that the pane is stale. + expect(recordCodexPaneAccountMock.mock.calls).toEqual([ + ['pty-fresh', { selectionKey: 'host', accountId: 'account-a' }] + ]) + }) + it('seeds cold restore at recovered dimensions with a legacy dimensionless fallback', async () => { const oscLinks = [{ row: 0, startCol: 0, endCol: 8, uri: 'https://example.com/restored' }] const coldRestore = { diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 427ea6df052..8c846c3d5af 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -830,16 +830,19 @@ function getCompatibleSelectedCodexHomePath( // Why: CODEX_HOME is fixed in a shell's environment at spawn and the daemon // keeps that shell alive across app restarts, so the launch account is the only // way to tell later that a pane still runs Codex as the previously selected -// account. A resume-pinned pane is deliberately on its session's own account, -// so it is forgotten rather than recorded as stale. +// account. A reattach inherits that baked environment rather than choosing one, +// so re-recording it under the current selection would erase the very evidence +// that the pane is stale. A resume-pinned pane is deliberately on its session's +// own account, so it is forgotten rather than recorded as stale. function recordCodexPaneAccountForSpawn(args: { ptyId: string | undefined isDaemonHostSpawn: boolean + isReattach: boolean pinnedByResume: boolean target: CodexAccountSelectionTarget settings: GlobalSettings | undefined }): void { - if (!args.ptyId || !args.isDaemonHostSpawn) { + if (!args.ptyId || !args.isDaemonHostSpawn || args.isReattach) { return } if (args.pinnedByResume || !args.settings) { @@ -3275,13 +3278,6 @@ export function registerPtyHandlers( }) ?? null) ) : null - recordCodexPaneAccountForSpawn({ - ptyId: effectiveSessionAppId ?? sessionId, - isDaemonHostSpawn, - pinnedByResume: Boolean(codexResumeHome), - target: codexSelectionTarget, - settings: getSettings?.() - }) const skipCodexHomeEnv = isDaemonHostSpawn && shouldSkipCodexHomeEnvForWindowsShell(daemonShellOverride, cwd) && @@ -3698,6 +3694,14 @@ export function registerPtyHandlers( if (effectiveSessionAppId !== undefined && effectiveSessionAppId !== result.id) { ptySizes.delete(effectiveSessionAppId) } + recordCodexPaneAccountForSpawn({ + ptyId: result.id, + isDaemonHostSpawn, + isReattach: result.isReattach === true, + pinnedByResume: Boolean(codexResumeHome), + target: codexSelectionTarget, + settings: getSettings?.() + }) if (hostSessionBinding) { try { const binding = { @@ -4431,13 +4435,6 @@ export function registerPtyHandlers( }) ?? null) ) : null - recordCodexPaneAccountForSpawn({ - ptyId: effectiveSessionAppId ?? effectiveSessionId, - isDaemonHostSpawn, - pinnedByResume: Boolean(codexResumeHome), - target: codexSelectionTarget, - settings: getSettings?.() - }) const skipCodexHomeEnv = isDaemonHostSpawn && shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd) && @@ -4739,6 +4736,14 @@ export function registerPtyHandlers( daemon: isDaemonHostSpawn, reattach: result.isReattach ?? false }) + recordCodexPaneAccountForSpawn({ + ptyId: result.id, + isDaemonHostSpawn, + isReattach: result.isReattach === true, + pinnedByResume: Boolean(codexResumeHome), + target: codexSelectionTarget, + settings: getSettings?.() + }) ptyOwnership.set(result.id, args.connectionId ?? null) if (result.incarnationId) { ptyIncarnationById.set(result.id, result.incarnationId) diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 9518def2455..cf837b12af2 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -574,10 +574,6 @@ export default function TerminalPane({ const setTabCanExpandPane = useAppStore((store) => store.setTabCanExpandPane) const suppressPtyExit = useAppStore((store) => store.suppressPtyExit) const pendingCodexPaneRestartIds = useAppStore((store) => store.pendingCodexPaneRestartIds) - // Why: a parked or deferred tab mounts with no transport yet, so the queued - // restart below has no ptyId to match on the mount pass. This re-runs it once - // the reconnected PTY is bound, which is the only moment the pane can act. - const attachedPtyIds = useAppStore((store) => store.ptyIdsByTabId[tabId]) const consumePendingCodexPaneRestart = useAppStore( (store) => store.consumePendingCodexPaneRestart ) @@ -1675,6 +1671,11 @@ export default function TerminalPane({ ] ) + // Why leaf bindings are a dep: a parked or deferred tab mounts with no + // 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. + const panePtyLayoutBindings = savedLayout.ptyIdsByLeafId useEffect(() => { const manager = managerRef.current if (!manager) { @@ -1692,9 +1693,9 @@ export default function TerminalPane({ } } }, [ - attachedPtyIds, consumePendingCodexPaneRestart, handleRestartCodexPane, + panePtyLayoutBindings, pendingCodexPaneRestartIds ])