diff --git a/src/main/codex-accounts/runtime-selection.ts b/src/main/codex-accounts/runtime-selection.ts index a8479dba1..5ac9f91b0 100644 --- a/src/main/codex-accounts/runtime-selection.ts +++ b/src/main/codex-accounts/runtime-selection.ts @@ -3,28 +3,23 @@ import type { CodexManagedAccountRuntimeSelection, GlobalSettings } from '../../shared/types' +// Why: the renderer's switch-time lane guard has to key panes the same way a +// launch does, so the lane vocabulary lives in shared rather than in main. +import { + getWslSelectionKey, + normalizeCodexAccountSelectionTarget, + type CodexAccountSelectionTarget +} from '../../shared/codex-selection-lane' -export type CodexAccountSelectionTarget = { - runtime?: 'host' | 'wsl' - wslDistro?: string | null -} - -export type NormalizedCodexAccountSelectionTarget = { - runtime: 'host' | 'wsl' - wslDistro: string | null -} - -export function normalizeCodexAccountSelectionTarget( - target?: CodexAccountSelectionTarget | null -): NormalizedCodexAccountSelectionTarget { - if (target?.runtime === 'wsl') { - return { - runtime: 'wsl', - wslDistro: normalizeWslDistro(target.wslDistro) - } - } - return { runtime: 'host', wslDistro: null } -} +export { + getCodexSelectionLaneKey, + getWslSelectionKey, + normalizeCodexAccountSelectionTarget +} from '../../shared/codex-selection-lane' +export type { + CodexAccountSelectionTarget, + NormalizedCodexAccountSelectionTarget +} from '../../shared/codex-selection-lane' export function normalizeCodexRuntimeSelection( settings: Pick< @@ -135,18 +130,3 @@ export function getCodexSelectionTargetForAccount( } return { runtime: 'host' } } - -/** Stable identifier for the selection lane a launch resolves its account from. */ -export function getCodexSelectionLaneKey(target?: CodexAccountSelectionTarget | null): string { - const normalized = normalizeCodexAccountSelectionTarget(target) - return normalized.runtime === 'host' ? 'host' : `wsl:${getWslSelectionKey(normalized.wslDistro)}` -} - -export function getWslSelectionKey(wslDistro: string | null | undefined): string { - return normalizeWslDistro(wslDistro) ?? '__default__' -} - -function normalizeWslDistro(wslDistro: string | null | undefined): string | null { - const trimmed = wslDistro?.trim() - return trimmed ? trimmed : null -} diff --git a/src/main/codex/codex-pane-account-registry.ts b/src/main/codex/codex-pane-account-registry.ts index b5ad92673..f1d839894 100644 --- a/src/main/codex/codex-pane-account-registry.ts +++ b/src/main/codex/codex-pane-account-registry.ts @@ -139,6 +139,26 @@ export function getCodexPaneAccount(ptyId: string): CodexPaneAccountRecord | nul return readRegistry().panes[ptyId] ?? null } +/** + * Reports the lane each given PTY launched from, omitting panes with no record. + * + * Why the renderer needs this: an account switch has to know which panes the + * change could have stranded, and this key was written from the shell, cwd and + * distro the spawn actually resolved. Re-deriving it from current settings + * answers for a launch that never happened once the user edits those settings. + */ +export function listRecordedCodexPaneLanes(ptyIds: readonly string[]): Record { + const registry = readRegistry() + const lanesByPtyId: Record = {} + for (const ptyId of ptyIds) { + const record = registry.panes[ptyId] + if (record) { + lanesByPtyId[ptyId] = record.selectionKey + } + } + return lanesByPtyId +} + export const _internals = { resetCache: (): void => { cachedRegistry = null diff --git a/src/main/ipc/codex-accounts.ts b/src/main/ipc/codex-accounts.ts index e5cad61d3..22cee5340 100644 --- a/src/main/ipc/codex-accounts.ts +++ b/src/main/ipc/codex-accounts.ts @@ -1,6 +1,7 @@ import { ipcMain } from 'electron' import type { CodexAccountAddTarget, CodexAccountService } from '../codex-accounts/service' import type { CodexAccountSelectionTarget } from '../codex-accounts/runtime-selection' +import { listRecordedCodexPaneLanes } from '../codex/codex-pane-account-registry' import { forgetStaleCodexPanes, listStaleCodexPanes } from '../codex/codex-stale-pane-accounts' import type { GlobalSettings } from '../../shared/types' @@ -18,6 +19,14 @@ export function registerCodexAccountHandlers( settings }) }) + ipcMain.handle('codexAccounts:listRecordedPaneLanes', (_event, args: { ptyIds?: unknown }) => { + if (!Array.isArray(args?.ptyIds)) { + return {} + } + return listRecordedCodexPaneLanes( + args.ptyIds.filter((ptyId): ptyId is string => typeof ptyId === 'string') + ) + }) ipcMain.handle('codexAccounts:forgetStalePanes', (_event, args: { ptyIds?: unknown }) => { if (!Array.isArray(args?.ptyIds)) { return diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index a9218f0c4..d60947df6 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2342,6 +2342,8 @@ export type PreloadApi = { }) => Promise< { ptyId: string; launchAccountId: string | null; activeAccountId: string | null }[] > + /** The selection lane each PTY launched from, keyed by pty id; unrecorded panes are absent. */ + listRecordedPaneLanes: (args: { ptyIds: string[] }) => Promise> /** Drops launch records so a dismissed prompt stays dismissed across restarts. */ forgetStalePanes: (args: { ptyIds: string[] }) => Promise } diff --git a/src/preload/index.ts b/src/preload/index.ts index 3c28865b1..bb0a17220 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1999,6 +1999,8 @@ const api = { }): Promise< { ptyId: string; launchAccountId: string | null; activeAccountId: string | null }[] > => ipcRenderer.invoke('codexAccounts:listStalePanes', args), + listRecordedPaneLanes: (args: { ptyIds: string[] }): Promise> => + ipcRenderer.invoke('codexAccounts:listRecordedPaneLanes', args), forgetStalePanes: (args: { ptyIds: string[] }): Promise => ipcRenderer.invoke('codexAccounts:forgetStalePanes', args) }, diff --git a/src/renderer/src/components/settings/AccountsPane.tsx b/src/renderer/src/components/settings/AccountsPane.tsx index 6f96cd2e1..4188db5c8 100644 --- a/src/renderer/src/components/settings/AccountsPane.tsx +++ b/src/renderer/src/components/settings/AccountsPane.tsx @@ -745,9 +745,29 @@ export function AccountsPane({ action === `reauth:${nextActiveAccountId}`) || (action.startsWith('remove:') && previousActiveAccountId !== nextActiveAccountId) if (shouldPromptRestart) { + // Why: `add` creates the managed home against the machine's own distro, + // so the slot it wrote is the created account's — not this row's, which + // may still say "WSL default". Found by diffing the roster rather than + // by the row's active id, which resolves to null once two distro slots + // are filled and would send the notice to the wrong lane. + const newAccounts = + action === 'adding' + ? next.accounts.filter( + (account) => !codexAccounts.accounts.some((prior) => prior.id === account.id) + ) + : [] + // Why exactly one: an unloaded prior roster makes every account look new, + // and picking one of those would aim the notice at an unrelated lane. + // Falling back to the row is the pre-existing behaviour, not a new risk. + const addedAccount = newAccounts.length === 1 ? newAccounts[0] : undefined void markLiveCodexSessionsForRestart({ previousAccountLabel: getCodexAccountLabel(codexAccounts, previousActiveAccountId), - nextAccountLabel: getCodexAccountLabel(next, nextActiveAccountId) + nextAccountLabel: getCodexAccountLabel(next, nextActiveAccountId), + // Why: the mutation wrote this row's slot only, so panes on any other + // lane still launch under the account they already had. + target: addedAccount ? getProviderAccountRuntime(addedAccount) : actionRuntime, + // Why: clearing a distro-less WSL row nulls every distro slot at once. + clearsEveryWslDistro: action === 'select:system' }) } } catch (error) { diff --git a/src/renderer/src/components/status-bar/StatusBar.tsx b/src/renderer/src/components/status-bar/StatusBar.tsx index 89cb89eb5..2fb006b34 100644 --- a/src/renderer/src/components/status-bar/StatusBar.tsx +++ b/src/renderer/src/components/status-bar/StatusBar.tsx @@ -1467,7 +1467,12 @@ export function CodexSwitcherMenu({ if (previousActiveAccountId !== nextActiveAccountId) { await markLiveCodexSessionsForRestart({ previousAccountLabel: getCodexAccountLabel(accountState, previousActiveAccountId), - nextAccountLabel: getCodexAccountLabel(next, nextActiveAccountId) + nextAccountLabel: getCodexAccountLabel(next, nextActiveAccountId), + // Why: the mutation wrote this row's slot only, so panes on any other + // lane still launch under the account they already had. + target, + // Why: clearing a distro-less WSL row nulls every distro slot at once. + clearsEveryWslDistro: accountId === null }) // Why: collapse to the summary row (not close) so the follow-up "restart open tabs" prompt appears in the same flow. if (mountedRef.current) { diff --git a/src/renderer/src/lib/codex-pane-selection-lane.test.ts b/src/renderer/src/lib/codex-pane-selection-lane.test.ts new file mode 100644 index 000000000..a4b1dcefc --- /dev/null +++ b/src/renderer/src/lib/codex-pane-selection-lane.test.ts @@ -0,0 +1,491 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AppState } from '@/store' +import { getCodexSelectionLaneKey } from '../../../shared/codex-selection-lane' +import { + getCodexAccountSwitchLaneMatcher, + isForeignMachineCodexPtyId, + isLocalCodexSelectionLaneKey, + resolveCodexPaneSelectionLane, + resolveCodexPaneSelectionLaneKey +} from './codex-pane-selection-lane' + +type LaneState = Pick< + AppState, + | 'activeRepoId' + | 'activeWorktreeId' + | 'folderWorkspaces' + | 'projects' + | 'repos' + | 'settings' + | 'worktreesByRepo' +> + +function laneState(args?: { + activeRuntimeEnvironmentId?: string | null + worktreePath?: string + folderPath?: string + terminalWindowsShell?: string + terminalWindowsWslDistro?: string | null + projectWslDistro?: string + localWindowsRuntimeDefault?: { kind: 'wsl'; distro: string | null } +}): LaneState { + return { + folderWorkspaces: args?.folderPath ? [{ id: 'fw1', folderPath: args.folderPath }] : [], + settings: { + activeRuntimeEnvironmentId: args?.activeRuntimeEnvironmentId ?? null, + ...(args?.terminalWindowsShell ? { terminalWindowsShell: args.terminalWindowsShell } : {}), + ...(args?.localWindowsRuntimeDefault + ? { localWindowsRuntimeDefault: args.localWindowsRuntimeDefault } + : {}), + terminalWindowsWslDistro: args?.terminalWindowsWslDistro ?? null + }, + repos: [{ id: 'repo1', path: 'C:\\code\\app' }], + projects: args?.projectWslDistro + ? [ + { + id: 'proj1', + sourceRepoIds: ['repo1'], + localWindowsRuntimePreference: { kind: 'wsl', distro: args.projectWslDistro } + } + ] + : [], + worktreesByRepo: { + repo1: [{ id: 'wt1', repoId: 'repo1', path: args?.worktreePath ?? '/Users/dev/code/orca' }] + } + } as unknown as LaneState +} + +/** The Windows-only shell resolution is gated on the renderer platform. */ +function withWindowsRenderer(run: () => void): void { + const originalNavigator = globalThis.navigator + Object.defineProperty(globalThis, 'navigator', { + value: { userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' }, + configurable: true + }) + try { + run() + } finally { + Object.defineProperty(globalThis, 'navigator', { + value: originalNavigator, + configurable: true + }) + } +} + +const HOST_TAB = { worktreeId: 'wt1', shellOverride: undefined } + +describe('resolveCodexPaneSelectionLaneKey', () => { + it('keys an ordinary local pane to the host lane', () => { + expect( + resolveCodexPaneSelectionLaneKey({ state: laneState(), tab: HOST_TAB, ptyId: 'pty-1' }) + ).toBe('host') + }) + + it('keys a pane in a WSL UNC worktree to that distro lane', () => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ worktreePath: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\orca' }), + tab: HOST_TAB, + ptyId: 'pty-1' + }) + ).toBe('wsl:Ubuntu') + }) + + it('keys a wsl.exe pane outside a UNC worktree to the default WSL lane', () => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState(), + tab: { worktreeId: 'wt1', shellOverride: 'wsl.exe' }, + ptyId: 'pty-1' + }) + ).toBe('wsl:__default__') + }) + + // Why this matters: pty.ts keys such a pane `wsl:` from the resolved + // runtime, so keying it `wsl:__default__` would make its own distro's switch + // miss it — the pane keeps the old account with no notice. + it('keys a wsl.exe pane on a Windows-path worktree to the configured distro', () => { + withWindowsRenderer(() => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ + terminalWindowsShell: 'wsl.exe', + terminalWindowsWslDistro: 'Ubuntu', + worktreePath: 'C:\\code\\app' + }), + tab: { worktreeId: 'wt1', shellOverride: 'wsl.exe' }, + ptyId: 'pty-1' + }) + ).toBe('wsl:Ubuntu') + }) + }) + + it('still keys an ordinary host pane to host when a WSL distro is configured', () => { + withWindowsRenderer(() => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ terminalWindowsWslDistro: 'Ubuntu', worktreePath: 'C:\\code\\app' }), + tab: HOST_TAB, + ptyId: 'pty-1' + }) + ).toBe('host') + }) + }) + + // Why: main resolves the shell through resolveLocalWindowsTerminalRuntimeOptions, + // so an unset override still lands on WSL when that is the Windows default. + // Reading only the tab would key this pane `host` and mute it on a host switch. + it('keys an override-less pane by the default Windows shell', () => { + withWindowsRenderer(() => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ + terminalWindowsShell: 'wsl.exe', + terminalWindowsWslDistro: 'Ubuntu', + worktreePath: 'C:\\code\\app' + }), + tab: { worktreeId: 'wt1', shellOverride: undefined }, + ptyId: 'pty-1' + }) + ).toBe('wsl:Ubuntu') + }) + }) + + // Why: the startup cwd is deliberately unconstrained (#7685), and main keys + // the lane off it, so a pane split across filesystems must follow the cwd. + it('follows the pane startup cwd out of the workspace filesystem', () => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ worktreePath: 'C:\\code\\app' }), + tab: { + worktreeId: 'wt1', + shellOverride: undefined, + startupCwd: '\\\\wsl.localhost\\Ubuntu\\home\\dev' + }, + ptyId: 'pty-1' + }) + ).toBe('wsl:Ubuntu') + }) + + it("keys a pane by its own project's WSL distro", () => { + withWindowsRenderer(() => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ projectWslDistro: 'Debian', worktreePath: 'C:\\code\\app' }), + tab: HOST_TAB, + ptyId: 'pty-1' + }) + ).toBe('wsl:Debian') + }) + }) + + // Why: a floating terminal's cwd never reaches the tab, so it is keyed by its + // shell. Pinning that it does not throw or resolve against some other + // workspace's root, which is what the lane would otherwise inherit. + it('keys a floating terminal by its shell, not another workspace root', () => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ worktreePath: '\\\\wsl.localhost\\Ubuntu\\home\\dev' }), + tab: { worktreeId: 'global-floating-terminal', shellOverride: undefined }, + ptyId: 'pty-1' + }) + ).toBe('host') + }) + + // Why: resolveLocalWindowsTerminalRuntimeOptions THROWS on repair-required, + // and this runs outside scanCodexPanes' per-pane failure guard — so without + // the early return the Promise.all rejects and EVERY pane in the batch loses + // its notice, not just this one. + it('answers a repair-required runtime instead of throwing the batch away', () => { + withWindowsRenderer(() => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ + localWindowsRuntimeDefault: { kind: 'wsl', distro: null }, + worktreePath: 'C:\\code\\app' + }), + tab: HOST_TAB, + ptyId: 'pty-1' + }) + ).toBe('wsl:__default__') + }) + }) + + it('reads the distro from a folder workspace path too', () => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ folderPath: '\\\\wsl$\\Debian\\srv\\app' }), + tab: { worktreeId: 'folder:fw1', shellOverride: undefined }, + ptyId: 'pty-1' + }) + ).toBe('wsl:Debian') + }) + + it('keys an owned remote runtime pane to its own environment, not the active one', () => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ activeRuntimeEnvironmentId: 'env-active' }), + tab: HOST_TAB, + ptyId: 'remote:env-owner@@term-1' + }) + ).toBe('env:env-owner') + }) + + it('routes an owner-less remote pane to the active environment, as inspection does', () => { + expect( + resolveCodexPaneSelectionLaneKey({ + state: laneState({ activeRuntimeEnvironmentId: 'env-1' }), + tab: HOST_TAB, + ptyId: 'remote:term-1' + }) + ).toBe('env:env-1') + }) + + it('keeps an owner-less remote pane off the host lane when no environment is active', () => { + const laneKey = resolveCodexPaneSelectionLaneKey({ + state: laneState(), + tab: HOST_TAB, + ptyId: 'remote:term-1' + }) + // Why assert disjointness rather than the literal key: colliding with `host` + // is the whole failure mode — a local switch would mute a working remote pane. + expect(laneKey).not.toBe(getCodexSelectionLaneKey({ runtime: 'host' })) + expect(isLocalCodexSelectionLaneKey(laneKey)).toBe(false) + }) + + it('keys an SSH-connection pane to a lane no account selection can name', () => { + const laneKey = resolveCodexPaneSelectionLaneKey({ + state: laneState(), + tab: HOST_TAB, + ptyId: 'ssh:my-box@@pty-7' + }) + expect(laneKey).toBe('ssh-connection') + // Why: managed Codex accounts are only ever 'host' or 'wsl:', so no + // switch can produce this key — the pane is unreachable by any selection. + expect(laneKey).not.toBe(getCodexSelectionLaneKey({ runtime: 'host' })) + expect(isLocalCodexSelectionLaneKey(laneKey)).toBe(false) + }) +}) + +describe('resolveCodexPaneSelectionLane', () => { + it('prefers the lane main recorded at spawn over the current derivation', () => { + // Why this is the whole point: the derivation reads CURRENT state, so the + // user editing a runtime preference after the pane opened must not re-key it. + const lane = resolveCodexPaneSelectionLane({ + state: laneState(), + tab: HOST_TAB, + ptyId: 'pty-1', + recordedLaneKey: 'wsl:Ubuntu' + }) + expect(lane).toEqual({ + laneKey: 'wsl:Ubuntu', + source: 'recorded', + derivedLaneKey: 'host' + }) + }) + + it('reports the disagreement so a re-derivation bug stays diagnosable', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + resolveCodexPaneSelectionLane({ + state: laneState(), + tab: HOST_TAB, + ptyId: 'pty-1', + recordedLaneKey: 'wsl:Ubuntu' + }) + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('[codex-lane]'), + expect.objectContaining({ ptyId: 'pty-1', recorded: 'wsl:Ubuntu', derived: 'host' }) + ) + } finally { + warn.mockRestore() + } + }) + + it('stays quiet when the record and the derivation agree', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const lane = resolveCodexPaneSelectionLane({ + state: laneState(), + tab: HOST_TAB, + ptyId: 'pty-1', + recordedLaneKey: 'host' + }) + expect(lane.source).toBe('recorded') + expect(warn).not.toHaveBeenCalled() + } finally { + warn.mockRestore() + } + }) + + // THE regression that matters: over-filtering silently kills the feature. + it('derives a local host pane that main never recorded', () => { + for (const recordedLaneKey of [undefined, null, '', ' ']) { + expect( + resolveCodexPaneSelectionLane({ + state: laneState(), + tab: HOST_TAB, + ptyId: 'pty-1', + recordedLaneKey + }) + ).toEqual({ laneKey: 'host', source: 'derived', derivedLaneKey: 'host' }) + } + }) + + it('falls back to the derivation when the record names no selectable lane', () => { + // Why: the registry accepts any string it finds on disk, and a lane key that + // matches no switch would silently drop the pane's notice instead of failing. + expect( + resolveCodexPaneSelectionLane({ + state: laneState(), + tab: HOST_TAB, + ptyId: 'pty-1', + recordedLaneKey: 'env:env-1' + }) + ).toEqual({ laneKey: 'host', source: 'derived', derivedLaneKey: 'host' }) + }) + + it.each([ + ['remote:env-1@@term-1', 'env:env-1'], + ['ssh:my-box@@pty-7', 'ssh-connection'] + ])('keeps a record from re-keying the foreign pane %s', (ptyId, expectedLaneKey) => { + // Why: a foreign pane's lane is settled by its id, so a record here can only + // be a recycled id — and honouring it would mute a working remote terminal. + expect( + resolveCodexPaneSelectionLane({ + state: laneState({ activeRuntimeEnvironmentId: 'env-1' }), + tab: HOST_TAB, + ptyId, + recordedLaneKey: 'host' + }) + ).toEqual({ laneKey: expectedLaneKey, source: 'derived', derivedLaneKey: expectedLaneKey }) + }) + + it('still answers with the record when the derivation throws', () => { + const exploding = new Proxy(laneState(), { + get(target, property) { + if (property === 'worktreesByRepo') { + throw new Error('state read blew up') + } + return Reflect.get(target, property) + } + }) as LaneState + // Why: this call sits outside the scan's per-pane failure guard, so a throw + // would lose the notice for every pane in the batch, not just this one. + expect( + resolveCodexPaneSelectionLane({ + state: exploding, + tab: HOST_TAB, + ptyId: 'pty-1', + recordedLaneKey: 'host' + }) + ).toEqual({ laneKey: 'host', source: 'recorded', derivedLaneKey: null }) + }) +}) + +describe('getCodexAccountSwitchLaneMatcher', () => { + it('scopes a local switch to the runtime slot it wrote', () => { + const hostSwitch = getCodexAccountSwitchLaneMatcher({ + settings: null, + target: { runtime: 'host' } + }) + expect(hostSwitch('host')).toBe(true) + expect(hostSwitch('wsl:Ubuntu')).toBe(false) + + const ubuntuSwitch = getCodexAccountSwitchLaneMatcher({ + settings: null, + target: { runtime: 'wsl', wslDistro: 'Ubuntu' } + }) + expect(ubuntuSwitch('wsl:Ubuntu')).toBe(true) + expect(ubuntuSwitch('wsl:Debian')).toBe(false) + expect(ubuntuSwitch('host')).toBe(false) + }) + + // Why a family: clearing a distro-less WSL selection nulls EVERY wsl slot, so + // matching only `wsl:__default__` would leave those panes stranded, unnoticed. + it('claims every WSL distro when the change cleared them all', () => { + const wslDefaultSwitch = getCodexAccountSwitchLaneMatcher({ + settings: null, + target: { runtime: 'wsl', wslDistro: null }, + clearsEveryWslDistro: true + }) + expect(wslDefaultSwitch('wsl:__default__')).toBe(true) + expect(wslDefaultSwitch('wsl:Ubuntu')).toBe(true) + expect(wslDefaultSwitch('wsl:Debian')).toBe(true) + // Still cannot reach another machine, which is the point of the guard. + expect(wslDefaultSwitch('host')).toBe(false) + expect(wslDefaultSwitch('env:env-1')).toBe(false) + expect(wslDefaultSwitch('ssh-connection')).toBe(false) + expect(wslDefaultSwitch('remote-runtime')).toBe(false) + }) + + // Why the negative half matters: pointing a distro-less WSL row at a real + // account writes only the `__default__` slot, so claiming the family there + // would card — and mute — every sibling distro's healthy Codex pane. + it('keeps a distro-less WSL selection off sibling distro panes', () => { + const wslDefaultSelect = getCodexAccountSwitchLaneMatcher({ + settings: null, + target: { runtime: 'wsl', wslDistro: null } + }) + expect(wslDefaultSelect('wsl:__default__')).toBe(true) + expect(wslDefaultSelect('wsl:Ubuntu')).toBe(false) + expect(wslDefaultSelect('host')).toBe(false) + }) + + // Why: StatusBar passes clearsEveryWslDistro for the "System default" row of + // EVERY WSL group, including a concrete-distro one, where the clear lands in + // that slot alone. Without the null-distro condition it would mute them all. + it('keeps a cleared concrete-distro row off the other distros', () => { + const ubuntuClear = getCodexAccountSwitchLaneMatcher({ + settings: null, + target: { runtime: 'wsl', wslDistro: 'Ubuntu' }, + clearsEveryWslDistro: true + }) + expect(ubuntuClear('wsl:Ubuntu')).toBe(true) + expect(ubuntuClear('wsl:Debian')).toBe(false) + expect(ubuntuClear('wsl:__default__')).toBe(false) + }) + + it('scopes a switch made against a runtime environment to that machine', () => { + const environmentSwitch = getCodexAccountSwitchLaneMatcher({ + settings: { activeRuntimeEnvironmentId: 'env-1' }, + target: { runtime: 'host' } + }) + expect(environmentSwitch('env:env-1')).toBe(true) + expect(environmentSwitch('host')).toBe(false) + expect(environmentSwitch('env:env-2')).toBe(false) + }) + + it('never lets a local host switch claim a remote or SSH pane', () => { + const hostSwitch = getCodexAccountSwitchLaneMatcher({ + settings: null, + target: { runtime: 'host' } + }) + const state = laneState() + for (const ptyId of ['remote:env-owner@@term-1', 'remote:term-1', 'ssh:my-box@@pty-7']) { + expect(hostSwitch(resolveCodexPaneSelectionLaneKey({ state, tab: HOST_TAB, ptyId }))).toBe( + false + ) + } + }) +}) + +describe('isForeignMachineCodexPtyId', () => { + it('separates panes whose shell runs on another machine from local ones', () => { + expect(isForeignMachineCodexPtyId('remote:env-1@@term-1')).toBe(true) + expect(isForeignMachineCodexPtyId('remote:term-1')).toBe(true) + expect(isForeignMachineCodexPtyId('ssh:my-box@@pty-7')).toBe(true) + expect(isForeignMachineCodexPtyId('pty-1')).toBe(false) + }) + + it('agrees with the lane keys, so the sweep and the scan skip the same panes', () => { + const state = laneState({ activeRuntimeEnvironmentId: 'env-1' }) + for (const ptyId of ['remote:env-1@@term-1', 'remote:term-1', 'ssh:my-box@@pty-7', 'pty-1']) { + expect( + isLocalCodexSelectionLaneKey( + resolveCodexPaneSelectionLaneKey({ state, tab: HOST_TAB, ptyId }) + ) + ).toBe(!isForeignMachineCodexPtyId(ptyId)) + } + }) +}) diff --git a/src/renderer/src/lib/codex-pane-selection-lane.ts b/src/renderer/src/lib/codex-pane-selection-lane.ts new file mode 100644 index 000000000..e6c8a6877 --- /dev/null +++ b/src/renderer/src/lib/codex-pane-selection-lane.ts @@ -0,0 +1,304 @@ +import type { AppState } from '@/store' +import { getActiveRuntimeTarget } from '@/runtime/runtime-client-target' +import { parseRemoteRuntimePtyId } from '@/runtime/runtime-terminal-stream' +import { + getCodexSelectionLaneKey, + normalizeCodexAccountSelectionTarget, + type CodexAccountSelectionTarget +} from '../../../shared/codex-selection-lane' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' +import { + isWslShellName, + resolveLocalWindowsTerminalRuntimeOptions, + type LocalWindowsTerminalRuntimeOptions +} from '../../../shared/local-windows-terminal-runtime' +import { parseAppSshPtyId } from '../../../shared/ssh-pty-id' +import { resolveTerminalStartupCwd } from '../../../shared/terminal-startup-cwd' +import type { GlobalSettings, TerminalTab } from '../../../shared/types' +import { parseWorkspaceKey } from '../../../shared/workspace-scope' +import { parseWslUncPath } from '../../../shared/wsl-paths' +import { getLocalProjectExecutionRuntimeContext } from './local-preflight-context' +import { getRendererAppPlatform } from './renderer-app-platform' +import { + getCachedWindowsTerminalCapabilities, + hasCachedWindowsTerminalCapabilities +} from './windows-terminal-capabilities' + +type RuntimeEnvironmentSettings = Pick + +/** Everything the pane lane needs: the workspace path plus the project runtime inputs. */ +type CodexPaneLaneState = Pick< + AppState, + | 'activeRepoId' + | 'activeWorktreeId' + | 'folderWorkspaces' + | 'projects' + | 'repos' + | 'settings' + | 'worktreesByRepo' +> + +/** + * Lane keys for panes whose Codex credentials come from another machine. + * + * Why they need keys at all: a managed Codex account is scoped to one machine + * AND one runtime (`host` or `wsl:`). A relay environment keeps its own + * account roster, and an SSH connection has no Orca-managed selection whatsoever + * — the remote Codex reads that machine's own credentials. Neither can be + * stranded by a local selection change, so they must not share the local keys. + */ +const RUNTIME_ENVIRONMENT_LANE_PREFIX = 'env:' +const SSH_CONNECTION_LANE_KEY = 'ssh-connection' +const UNATTRIBUTED_REMOTE_LANE_KEY = 'remote-runtime' +const HOST_LANE_KEY = 'host' +const WSL_LANE_PREFIX = 'wsl:' + +/** True for the lanes an on-disk pane-account record can name. */ +export function isLocalCodexSelectionLaneKey(laneKey: string): boolean { + return laneKey === HOST_LANE_KEY || laneKey.startsWith(WSL_LANE_PREFIX) +} + +/** + * True when the pane's shell runs on a machine other than this one. + * + * Why it takes only the id: a `remote:`/`ssh:` prefix is assigned at spawn and + * is decisive on its own, so callers with no store access (the bind-driven + * sweep) can skip these panes before spending a 15s RPC on them. + */ +export function isForeignMachineCodexPtyId(ptyId: string): boolean { + return parseRemoteRuntimePtyId(ptyId) !== null || parseAppSshPtyId(ptyId) !== null +} + +/** Matches the panes a Codex account mutation could have re-pointed. */ +export function getCodexAccountSwitchLaneMatcher(args: { + settings: RuntimeEnvironmentSettings | null | undefined + target?: CodexAccountSelectionTarget | null + /** + * True only when the mutation cleared every WSL distro slot at once, which + * setSelectedCodexAccountIdForTarget does for a null account on a distro-less + * WSL target. Any other write lands in a single slot, so defaulting this to + * false keeps the matcher from muting a sibling distro's healthy panes. + */ + clearsEveryWslDistro?: boolean +}): (laneKey: string) => boolean { + const runtimeTarget = getActiveRuntimeTarget(args.settings) + // Why: with an environment active the mutation is RPC'd to that machine's + // roster and local GlobalSettings are never touched, so the local host/WSL + // panes are exactly the ones the switch cannot have affected. + if (runtimeTarget.kind === 'environment') { + const environmentLaneKey = `${RUNTIME_ENVIRONMENT_LANE_PREFIX}${runtimeTarget.environmentId}` + return (laneKey) => laneKey === environmentLaneKey + } + const normalized = normalizeCodexAccountSelectionTarget(args.target) + // Why a family rather than the `wsl:__default__` key: clearing a distro-less + // WSL selection nulls every distro slot, so every WSL pane really is stranded. + // Keying that to `__default__` alone would leave them all without a notice. + if (args.clearsEveryWslDistro && normalized.runtime === 'wsl' && normalized.wslDistro === null) { + return (laneKey) => laneKey.startsWith(WSL_LANE_PREFIX) + } + const switchLaneKey = getCodexSelectionLaneKey(normalized) + return (laneKey) => laneKey === switchLaneKey +} + +export type CodexPaneSelectionLane = { + /** The lane the caller must filter on. */ + laneKey: string + /** Which answer won: main's spawn-time record, or the renderer's re-derivation. */ + source: 'recorded' | 'derived' + /** What the derivation said, or null when it threw. Diagnostics only. */ + derivedLaneKey: string | null +} + +/** + * The lane a pane launched from, preferring the one main recorded at spawn. + * + * Why recorded wins: main writes `selectionKey` from the shell, cwd and distro + * the spawn actually resolved, so it cannot drift. The derivation below reads + * CURRENT state, so flipping the global WSL distro or a project's runtime + * preference after a pane opened makes it answer for a launch that never + * happened — a missed notice one way, a muted working terminal the other. + * + * The derivation stays because it is the only answer for the panes main never + * records: every pre-feature pane, and every LocalPtyProvider or remote spawn. + */ +export function resolveCodexPaneSelectionLane(args: { + state: CodexPaneLaneState + tab: Pick + ptyId: string + /** The pane's `selectionKey` from the on-disk registry, when it has one. */ + recordedLaneKey?: string | null +}): CodexPaneSelectionLane { + const recorded = args.recordedLaneKey?.trim() + // Why the local-key check: the registry accepts any string it finds on disk, + // and a lane key that matches no switch silently drops that pane's notice. + // Why foreign ids still derive: their lane is settled by the id itself and no + // record can exist for one, so a hit here would mean a recycled id. + const trustsRecord = + Boolean(recorded) && + isLocalCodexSelectionLaneKey(recorded as string) && + !isForeignMachineCodexPtyId(args.ptyId) + if (!trustsRecord) { + const laneKey = resolveCodexPaneSelectionLaneKey(args) + return { laneKey, source: 'derived', derivedLaneKey: laneKey } + } + const derivedLaneKey = deriveLaneKeyForDiagnostics(args) + if (derivedLaneKey !== null && derivedLaneKey !== recorded) { + // Why loud: every divergence found in review was this exact disagreement, + // and the recorded key now hides it instead of producing a visible bug. + console.warn('[codex-lane] recorded launch lane disagrees with the derived one:', { + ptyId: args.ptyId, + recorded, + derived: derivedLaneKey + }) + } + return { laneKey: recorded as string, source: 'recorded', derivedLaneKey } +} + +/** Never let the diagnostic derivation break a pane whose lane is already known. */ +function deriveLaneKeyForDiagnostics(args: { + state: CodexPaneLaneState + tab: Pick + ptyId: string +}): string | null { + try { + return resolveCodexPaneSelectionLaneKey(args) + } catch { + return null + } +} + +/** The lane a live pane resolves its Codex account from, re-derived from state. */ +export function resolveCodexPaneSelectionLaneKey(args: { + state: CodexPaneLaneState + tab: Pick + ptyId: string +}): string { + const remoteParts = parseRemoteRuntimePtyId(args.ptyId) + if (remoteParts !== null) { + const runtimeTarget = getActiveRuntimeTarget(args.state.settings) + // Why: mirror inspectRuntimeTerminalProcess — an owner-less remote id is + // routed to whichever environment is active, so that is its lane too. + const environmentId = + remoteParts.environmentId?.trim() || + (runtimeTarget.kind === 'environment' ? runtimeTarget.environmentId : null) + return environmentId + ? `${RUNTIME_ENVIRONMENT_LANE_PREFIX}${environmentId}` + : UNATTRIBUTED_REMOTE_LANE_KEY + } + if (parseAppSshPtyId(args.ptyId) !== null) { + return SSH_CONNECTION_LANE_KEY + } + return getCodexSelectionLaneKey(resolveLocalPaneSelectionTarget(args)) +} + +/** + * Mirrors the main-process getCodexSelectionTargetForPty, from renderer state. + * + * Why the pane cwd and not the workspace root: a terminal's startup cwd is + * deliberately NOT constrained to the worktree (see resolveTerminalStartupCwd, + * #7685), so a pane split after `cd \\wsl.localhost\...` runs on a different + * filesystem than its workspace. Main keys the lane off that cwd, so reading the + * root instead would call a live WSL pane `host` and mute it on a host switch. + */ +function resolveLocalPaneSelectionTarget(args: { + state: CodexPaneLaneState + tab: Pick +}): CodexAccountSelectionTarget { + const paneCwd = resolvePaneCwd(args) + const wslPath = paneCwd ? parseWslUncPath(paneCwd) : null + if (wslPath) { + return { runtime: 'wsl', wslDistro: wslPath.distro } + } + const terminalRuntime = resolveLocalPaneTerminalRuntime(args) + if (isWslShellName(terminalRuntime.shellOverride)) { + return { runtime: 'wsl', wslDistro: terminalRuntime.terminalWindowsWslDistro } + } + return { runtime: 'host' } +} + +/** The absolute directory the pane's shell was spawned in, as main resolved it. */ +function resolvePaneCwd(args: { + state: CodexPaneLaneState + tab: Pick +}): string | null { + // Why floating terminals get no cwd: theirs never reaches the tab. It is + // resolved over IPC from settings.floatingTerminalCwd and handed to the + // transport as a prop, so the store cannot see the path main keyed off. Such a + // pane falls through to its shell below, which is right unless the configured + // floating cwd is a WSL UNC path under a host shell — a known gap, not a guess + // worth making, since guessing wrong here mutes a working terminal. + if (args.tab.worktreeId === FLOATING_TERMINAL_WORKTREE_ID) { + return null + } + const workspacePath = getWorkspacePath(args.state, args.tab.worktreeId) + if (!workspacePath) { + return null + } + // Why this exact call: it is the same one main spawns through, so a relative + // or inherited startup folder resolves to the identical absolute path. + return resolveTerminalStartupCwd(workspacePath, args.tab.startupCwd) ?? workspacePath +} + +/** + * The shell and distro the launch resolved, not merely the ones the tab asked for. + * + * Why getLocalProjectExecutionRuntimeContext specifically: for a local pane the + * RENDERER computes the project runtime and ships it with the spawn + * (pty-connection.ts), so this is not an approximation of main — it is the same + * call on the same state. Re-deriving it by hand drops the global WSL default, + * which turns `inherit-global` into WSL and would key a live WSL pane `host`. + */ +function resolveLocalPaneTerminalRuntime(args: { + state: CodexPaneLaneState + tab: Pick +}): LocalWindowsTerminalRuntimeOptions { + // Why the platform gate: pty.ts only consults the Windows terminal runtime on + // win32, so elsewhere the tab's own override is the whole answer. + if (getRendererAppPlatform() !== 'win32') { + return { shellOverride: args.tab.shellOverride, terminalWindowsWslDistro: null } + } + const capabilities = hasCachedWindowsTerminalCapabilities() + ? getCachedWindowsTerminalCapabilities() + : null + const projectRuntime = getLocalProjectExecutionRuntimeContext( + args.state, + args.tab.worktreeId, + undefined, + { + wslAvailable: capabilities?.wslAvailable, + availableWslDistros: capabilities?.wslDistros ?? null + } + ) + if (projectRuntime?.status === 'repair-required') { + // Why not delegate: resolveLocalWindowsTerminalRuntimeOptions throws here, + // and this call sits outside the scan's per-pane failure guard, so a throw + // would lose the notice for every pane in the batch, not just this one. + return { + shellOverride: 'wsl.exe', + terminalWindowsWslDistro: projectRuntime.repair.preferredRuntime.distro + } + } + return resolveLocalWindowsTerminalRuntimeOptions({ + requestedShellOverride: args.tab.shellOverride, + settings: args.state.settings ?? undefined, + projectRuntime + }) +} + +function getWorkspacePath( + state: Pick, + worktreeId: string +): string | null { + const parsed = parseWorkspaceKey(worktreeId) + if (parsed?.type === 'folder') { + return ( + (state.folderWorkspaces ?? []).find((workspace) => workspace.id === parsed.folderWorkspaceId) + ?.folderPath ?? null + ) + } + return ( + Object.values(state.worktreesByRepo ?? {}) + .flat() + .find((entry) => entry.id === worktreeId)?.path ?? null + ) +} diff --git a/src/renderer/src/lib/codex-session-restart.test.ts b/src/renderer/src/lib/codex-session-restart.test.ts index 7e03475bd..3c9c2c816 100644 --- a/src/renderer/src/lib/codex-session-restart.test.ts +++ b/src/renderer/src/lib/codex-session-restart.test.ts @@ -416,6 +416,316 @@ describe('markLiveCodexSessionsForRestart', () => { }) }) +/** + * A restart notice blocks every keystroke in the pane it names, so raising one + * on a pane the switch could not have touched takes a working terminal deaf. + * A managed Codex account is scoped to one machine AND one runtime: a remote + * spawn carries a connectionId, so no CODEX_HOME is ever injected into it, and + * WSL selections live in their own per-distro slot. + */ +describe('markLiveCodexSessionsForRestart lane scoping', () => { + const originalWindow = (globalThis as { window?: typeof window }).window + const runtimeEnvironmentCall = vi.fn() + const runtimeEnvironmentTransportCall = vi.fn() + + function seedPanes( + panes: { ptyId: string; worktreeId?: string; shellOverride?: string }[], + worktreePaths: Record = {} + ): void { + useAppStore.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + worktreesByRepo: { + repo1: [ + { id: 'wt1', path: worktreePaths.wt1 ?? '/Users/dev/code/orca' }, + ...(worktreePaths.wt2 ? [{ id: 'wt2', path: worktreePaths.wt2 }] : []) + ] + } as never, + tabsByWorktree: { + wt1: panes.map((pane, index) => ({ + id: `tab-${index}`, + ptyId: pane.ptyId, + worktreeId: pane.worktreeId ?? 'wt1', + title: `orca-${index}`, + customTitle: null, + color: null, + sortOrder: index, + createdAt: 1, + launchAgent: 'codex' as const, + ...(pane.shellOverride ? { shellOverride: pane.shellOverride } : {}) + })) + }, + ptyIdsByTabId: Object.fromEntries(panes.map((pane, index) => [`tab-${index}`, [pane.ptyId]])), + pendingCodexPaneRestartIds: {}, + codexRestartNoticeByPtyId: {} + }) + } + + beforeEach(() => { + clearRuntimeCompatibilityCacheForTests() + runtimeEnvironmentCall.mockReset() + runtimeEnvironmentTransportCall.mockReset() + runtimeEnvironmentTransportCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => { + return createCompatibleRuntimeStatusResponseIfNeeded(args) ?? runtimeEnvironmentCall(args) + }) + // Why: every pane in this block reads as a live Codex session, so any pane + // left uncarded was excluded by its lane and nothing else. + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: { process: { foregroundProcess: 'codex', hasChildProcesses: true } }, + _meta: { runtimeId: 'remote-runtime' } + }) + ;(globalThis as { window: typeof window }).window = { + ...originalWindow, + api: { + ...originalWindow?.api, + pty: { + ...originalWindow?.api?.pty, + inspectProcess: vi + .fn() + .mockResolvedValue({ foregroundProcess: 'codex', hasChildProcesses: true }) + }, + codexAccounts: { + ...originalWindow?.api?.codexAccounts, + list: vi.fn().mockResolvedValue({ accounts: [], activeAccountId: null }), + listStalePanes: vi.fn().mockResolvedValue([]), + listRecordedPaneLanes: vi.fn().mockResolvedValue({}) + }, + runtimeEnvironments: { + ...originalWindow?.api?.runtimeEnvironments, + call: runtimeEnvironmentTransportCall + } + } + } as unknown as typeof window + }) + + afterEach(() => { + useAppStore.setState({ settings: null as never, worktreesByRepo: {} as never }) + if (originalWindow) { + ;(globalThis as { window: typeof window }).window = originalWindow + } else { + delete (globalThis as { window?: typeof window }).window + } + }) + + it('leaves a live remote Codex pane alone on a host switch, and never inspects it', async () => { + seedPanes([{ ptyId: 'remote:env-1@@term-1' }]) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + + it('leaves a live SSH-connection Codex pane alone on a host switch', async () => { + seedPanes([{ ptyId: 'ssh:my-box@@pty-7' }]) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + expect(window.api.pty.inspectProcess).not.toHaveBeenCalled() + }) + + it('still marks the local host pane while sparing the remote one beside it', async () => { + seedPanes([{ ptyId: 'pty-1' }, { ptyId: 'remote:env-1@@term-1' }]) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({ + 'pty-1': { previousAccountLabel: ACCOUNT_A, nextAccountLabel: ACCOUNT_B } + }) + expect(window.api.pty.inspectProcess).toHaveBeenCalledWith('pty-1') + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + + it('still marks a local host pane when the switch names no target at all', async () => { + seedPanes([{ ptyId: 'pty-1' }]) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({ + 'pty-1': { previousAccountLabel: ACCOUNT_A, nextAccountLabel: ACCOUNT_B } + }) + }) + + // Why these two: a Windows validation saw a WSL pane escape a host switch, but + // only because its foreground read as `wsl.exe` and failed the Codex test. Pin + // the lane instead — a WSL pane whose foreground IS codex must escape too. + it('leaves a WSL Codex pane alone on a host switch even when its foreground is codex', async () => { + seedPanes([{ ptyId: 'pty-wsl' }], { wt1: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\orca' }) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + expect(window.api.pty.inspectProcess).not.toHaveBeenCalled() + }) + + it('marks that same WSL pane when its own distro is the lane that changed', async () => { + seedPanes([{ ptyId: 'pty-wsl' }], { wt1: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\orca' }) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'wsl', wslDistro: 'Ubuntu' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({ + 'pty-wsl': { previousAccountLabel: ACCOUNT_A, nextAccountLabel: ACCOUNT_B } + }) + }) + + it('keeps one distro switch off another distro pane', async () => { + seedPanes([{ ptyId: 'pty-wsl' }], { wt1: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\orca' }) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'wsl', wslDistro: 'Debian' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + }) + + it('leaves the local host pane alone when the switch was made on a runtime environment', async () => { + seedPanes([{ ptyId: 'pty-1' }, { ptyId: 'remote:env-1@@term-1' }]) + useAppStore.setState({ settings: { activeRuntimeEnvironmentId: 'env-1' } as never }) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + // Why: that mutation was RPC'd to env-1's own roster, so env-1's panes are + // the stale ones and the local shell is untouched — the mirror of the bug. + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({ + 'remote:env-1@@term-1': { + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + } + }) + expect(window.api.pty.inspectProcess).not.toHaveBeenCalled() + }) + + /** + * Main writes the lane from the shell, cwd and distro the spawn resolved, so + * it is exact where re-deriving from current state can only approximate. Four + * review rounds each found another divergence in that derivation; these pin + * the record beating it in both directions. + */ + describe('recorded launch lanes', () => { + it('spares a pane the record puts in another lane, and never inspects it', async () => { + // Derivation says `host`; the pane really launched under WSL. Carding it + // would take a working terminal deaf — this is the bug class in one test. + seedPanes([{ ptyId: 'pty-1' }]) + vi.mocked(window.api.codexAccounts.listRecordedPaneLanes).mockResolvedValue({ + 'pty-1': 'wsl:Ubuntu' + }) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + expect(window.api.pty.inspectProcess).not.toHaveBeenCalled() + }) + + it('cards a pane the record puts in the switched lane against the derivation', async () => { + // The mirror: the user changed a runtime preference after this WSL-looking + // pane spawned on the host, and re-derivation would now miss its notice. + seedPanes([{ ptyId: 'pty-1' }], { wt1: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\orca' }) + vi.mocked(window.api.codexAccounts.listRecordedPaneLanes).mockResolvedValue({ + 'pty-1': 'host' + }) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + }) + + // THE regression check: over-filtering silently kills the feature and reads + // as a pass. A genuine local host pane must be carded on every fallback path. + it.each([ + ['no record exists for the pane', () => ({ 'pty-other': 'wsl:Ubuntu' })], + ['the lookup rejects', null], + ['the preload predates the lookup', undefined] + ])('still cards a local host pane when %s', async (_label, recorded) => { + seedPanes([{ ptyId: 'pty-1' }]) + if (recorded === null) { + vi.mocked(window.api.codexAccounts.listRecordedPaneLanes).mockRejectedValue( + new Error('no handler') + ) + } else if (recorded === undefined) { + ;( + window.api.codexAccounts as unknown as { listRecordedPaneLanes?: unknown } + ).listRecordedPaneLanes = undefined + } else { + vi.mocked(window.api.codexAccounts.listRecordedPaneLanes).mockResolvedValue(recorded()) + } + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + }) + + it('asks only about panes main could have recorded', async () => { + seedPanes([ + { ptyId: 'pty-1' }, + { ptyId: 'remote:env-1@@term-1' }, + { ptyId: 'ssh:my-box@@pty-7' } + ]) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B, + target: { runtime: 'host' } + }) + + // Why: main only records daemon host spawns, so a foreign id is a certain + // miss — and one batched call, not one per pane. + expect(window.api.codexAccounts.listRecordedPaneLanes).toHaveBeenCalledTimes(1) + expect(window.api.codexAccounts.listRecordedPaneLanes).toHaveBeenCalledWith({ + ptyIds: ['pty-1'] + }) + }) + }) +}) + describe('markRestoredStaleCodexSessionsForRestart', () => { const originalWindow = (globalThis as { window?: typeof window }).window diff --git a/src/renderer/src/lib/codex-session-restart.ts b/src/renderer/src/lib/codex-session-restart.ts index ddde138f4..cf5a6f5ac 100644 --- a/src/renderer/src/lib/codex-session-restart.ts +++ b/src/renderer/src/lib/codex-session-restart.ts @@ -3,6 +3,13 @@ import { useAppStore } from '@/store' import { inspectRuntimeTerminalProcess } from '@/runtime/runtime-terminal-inspection' import { translate } from '@/i18n/i18n' import { isCodexRestartEligiblePane } from './codex-pane-restart-eligibility' +import { + getCodexAccountSwitchLaneMatcher, + isForeignMachineCodexPtyId, + isLocalCodexSelectionLaneKey, + resolveCodexPaneSelectionLane +} from './codex-pane-selection-lane' +import type { CodexAccountSelectionTarget } from '../../../shared/codex-selection-lane' // 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. @@ -21,52 +28,131 @@ export type CodexPaneScanResult = { launchedCodex: boolean /** A restart notice was raised for this pane by this scan. */ notified: boolean + /** The lane this pane was filtered on. */ + laneKey: string + /** Whether that lane came from main's spawn record or the renderer's derivation. */ + laneSource: 'recorded' | 'derived' } +/** + * Asks main which lane each pane actually launched from. + * + * Why failure is silent: the answer only upgrades the derivation's accuracy, so + * an older preload, a web client, or a missing registry must degrade to the + * derivation rather than lose every pane's restart notice. + */ +async function readRecordedCodexPaneLanes( + ptyIds: readonly string[] +): Promise> { + // Why filtered: main only records daemon host spawns, so asking about a + // remote or SSH pane is a guaranteed miss. + const localPtyIds = ptyIds.filter((ptyId) => !isForeignMachineCodexPtyId(ptyId)) + if (localPtyIds.length === 0) { + return {} + } + const listRecordedPaneLanes = window.api.codexAccounts.listRecordedPaneLanes + // Why the shape check: a preload older than this handler has no such method, + // and reaching that case must read as "no records", not as a scan failure. + if (typeof listRecordedPaneLanes !== 'function') { + return {} + } + return await listRecordedPaneLanes({ ptyIds: localPtyIds }).catch(() => ({})) +} + +/** + * Reports which panes are running Codex, skipping any outside the caller's lane. + * + * Why the lane filter runs BEFORE inspection rather than after: an out-of-lane + * answer cannot change the outcome, and the inspection is not free — a pane on a + * relay environment costs a 15s-timeout RPC per look. Skipping first is also + * what keeps a restart notice (which drops every keystroke in the pane) off a + * remote Codex session that no local account change can possibly strand. + */ async function scanCodexPanes( state: AppState, - ptyIdFilter: ReadonlySet | null + args: { + ptyIdFilter: ReadonlySet | null + isLaneInScope: (laneKey: string) => boolean + } ): Promise { - const tabs = Object.values(state.tabsByWorktree).flat() - const scans = await Promise.all( - tabs.map(async (tab) => { - const ptyIds = (state.ptyIdsByTabId[tab.id] ?? []).filter( - (ptyId) => ptyIdFilter === null || ptyIdFilter.has(ptyId) - ) - // Why: Codex sessions are not reliably discoverable from tab labels. - // Tabs keep fallback names until a CLI emits an OSC title, and Codex - // does not always do that. The live process tree plus the tab's recorded - // launchAgent are the stable evidence that this pane is running Codex. - return Promise.all( - ptyIds.map(async (ptyId) => { - const inspection = await inspectRuntimeTerminalProcess(state.settings, ptyId).then( - (result) => result, - // Why: one stale remote pane must not hide restart notices for other confirmed Codex panes. - () => null - ) - return { - ptyId, - eligible: - inspection !== null && - isCodexRestartEligiblePane({ inspection, launchAgent: tab.launchAgent }), - inconclusive: inspection === null || inspection.unavailable === true, - launchedCodex: tab.launchAgent === 'codex', - notified: false - } - }) + const panes = Object.values(state.tabsByWorktree) + .flat() + .flatMap((tab) => + (state.ptyIdsByTabId[tab.id] ?? []) + .filter((ptyId) => args.ptyIdFilter === null || args.ptyIdFilter.has(ptyId)) + .map((ptyId) => ({ tab, ptyId })) + ) + const recordedLanes = await readRecordedCodexPaneLanes(panes.map((pane) => pane.ptyId)) + + // Why: Codex sessions are not reliably discoverable from tab labels. Tabs keep + // fallback names until a CLI emits an OSC title, and Codex does not always do + // that. The live process tree plus the tab's recorded launchAgent are the + // stable evidence that this pane is running Codex. + return Promise.all( + panes.map(async ({ tab, ptyId }) => { + const lane = resolveCodexPaneSelectionLane({ + state, + tab, + ptyId, + recordedLaneKey: recordedLanes[ptyId] + }) + if (!args.isLaneInScope(lane.laneKey)) { + // Why not inconclusive: a pane's lane is fixed at spawn, so this is a + // final answer and the sweep must not spend a retry rung re-asking. + return { + ptyId, + eligible: false, + inconclusive: false, + launchedCodex: false, + notified: false, + laneKey: lane.laneKey, + laneSource: lane.source + } + } + const inspection = await inspectRuntimeTerminalProcess(state.settings, ptyId).then( + (result) => result, + // Why: one stale remote pane must not hide restart notices for other confirmed Codex panes. + () => null ) + return { + ptyId, + eligible: + inspection !== null && + isCodexRestartEligiblePane({ inspection, launchAgent: tab.launchAgent }), + inconclusive: inspection === null || inspection.unavailable === true, + launchedCodex: tab.launchAgent === 'codex', + notified: false, + laneKey: lane.laneKey, + laneSource: lane.source + } }) ) - - return scans.flat() } +/** + * Prompts the panes a just-applied account change stranded. + * + * `target` names the selection slot the change wrote. Panes outside that lane — + * a WSL pane on a host switch, or anything running on a relay/SSH machine — + * never had this account injected, so a notice there is pure damage: it mutes a + * terminal that is working correctly. + */ export async function markLiveCodexSessionsForRestart(args: { previousAccountLabel: string nextAccountLabel: string + target?: CodexAccountSelectionTarget | null + /** Set when the change cleared the selection rather than pointing it somewhere. */ + clearsEveryWslDistro?: boolean }): Promise { const state = useAppStore.getState() - const scans = await scanCodexPanes(state, null) + const scans = await scanCodexPanes(state, { + ptyIdFilter: null, + isLaneInScope: getCodexAccountSwitchLaneMatcher({ + settings: state.settings, + target: args.target, + clearsEveryWslDistro: args.clearsEveryWslDistro + }) + }) const liveCodexSessionPtyIds = scans.filter((scan) => scan.eligible).map((scan) => scan.ptyId) if (liveCodexSessionPtyIds.length === 0) { return @@ -91,12 +177,20 @@ export async function markLiveCodexSessionsForRestart(args: { * * Returns one result per inspected pane so the bind-driven sweep can tell an * answered pane from one whose PTY has not reported a usable process yet. + * + * Scoped to the local host/WSL lanes because the pane-account registry only + * records daemon host spawns: a relay or SSH pane can never be listed stale, so + * inspecting one is a guaranteed-fruitless RPC. listStalePanes then does the + * host-vs-WSL check itself, against each pane's own recorded lane. */ export async function markRestoredStaleCodexSessionsForRestart(args?: { ptyIds?: readonly string[] }): Promise { const state = useAppStore.getState() - const scans = await scanCodexPanes(state, args?.ptyIds ? new Set(args.ptyIds) : null) + const scans = await scanCodexPanes(state, { + ptyIdFilter: args?.ptyIds ? new Set(args.ptyIds) : null, + isLaneInScope: isLocalCodexSelectionLaneKey + }) const liveCodexSessionPtyIds = scans.filter((scan) => scan.eligible).map((scan) => scan.ptyId) if (liveCodexSessionPtyIds.length === 0) { return scans 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 5448315e3..251881a57 100644 --- a/src/renderer/src/lib/codex-stale-pane-sweep.test.ts +++ b/src/renderer/src/lib/codex-stale-pane-sweep.test.ts @@ -353,4 +353,50 @@ describe('notifyCodexPaneBoundForStaleSweep', () => { nextAccountLabel: ACCOUNT_B }) }) + + // Why: recordCodexPaneAccountForSpawn bails on anything that is not a daemon + // HOST spawn, so no remote/SSH pane is ever in the registry and listStalePanes + // can never report one. Every rung one takes is a 15s-timeout RPC spent to + // learn nothing, five times over. + it.each(['remote:env-1@@term-1', 'remote:term-1', 'ssh:my-box@@pty-7'])( + 'never queues %s, so no rung spends an RPC on it', + async (ptyId) => { + useAppStore.setState({ ptyIdsByTabId: { 'tab-1': [ptyId] } }) + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([ + { ptyId, launchAccountId: 'account-a', activeAccountId: 'account-b' } + ]) + + notifyCodexPaneBoundForStaleSweep(ptyId) + // Why assert the timer before advancing it: the scan would skip this pane + // anyway, so only an unarmed queue proves it was rejected at the door + // rather than costing a flush + scan on every rung. + expect(vi.getTimerCount()).toBe(0) + + // Well past the whole ladder, so this pins "never queued", not "not yet". + await vi.advanceTimersByTimeAsync(120_000) + + expect(vi.getTimerCount()).toBe(0) + expect(window.api.codexAccounts.listStalePanes).not.toHaveBeenCalled() + expect(window.api.pty.inspectProcess).not.toHaveBeenCalled() + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + } + ) + + it('still sweeps the local panes bound alongside a remote one', async () => { + useAppStore.setState({ ptyIdsByTabId: { 'tab-1': ['pty-1', 'remote:env-1@@term-1'] } }) + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + + notifyCodexPaneBoundForStaleSweep('remote:env-1@@term-1') + notifyCodexPaneBoundForStaleSweep('pty-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 + }) + }) }) diff --git a/src/renderer/src/lib/codex-stale-pane-sweep.ts b/src/renderer/src/lib/codex-stale-pane-sweep.ts index 178334643..c75cea5fa 100644 --- a/src/renderer/src/lib/codex-stale-pane-sweep.ts +++ b/src/renderer/src/lib/codex-stale-pane-sweep.ts @@ -2,6 +2,7 @@ import { markRestoredStaleCodexSessionsForRestart, type CodexPaneScanResult } from './codex-session-restart' +import { isForeignMachineCodexPtyId } from './codex-pane-selection-lane' // Why: the first delay coalesces the startup burst of binds and lets // updateTabPtyId (written just after the layout binding) land, since the scan @@ -36,6 +37,12 @@ export function notifyCodexPaneBoundForStaleSweep(ptyId: string): void { if (notifiedPtyIds.has(ptyId)) { return } + // Why: the pane-account registry only records daemon HOST spawns, so a relay + // or SSH pane can never come back stale — every rung it takes is a remote RPC + // (15s timeout) spent to learn nothing. Drop it before it reaches the queue. + if (isForeignMachineCodexPtyId(ptyId)) { + return + } queue(ptyId, SWEEP_ATTEMPT_DELAYS_MS[0]) armForEarliestDue() } diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index adf36b191..b01292b9b 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -2904,6 +2904,9 @@ function createAccountsApi(): never { // Why: launch accounts are recorded on the host that owns the PTY, which the // web client never is — report no stale panes rather than reject the sweep. listStalePanes: () => Promise.resolve([]), + // Why empty rather than absent: the same host owns both records, so a web + // client has no recorded lane to offer and every pane falls to derivation. + listRecordedPaneLanes: () => Promise.resolve({}), forgetStalePanes: () => Promise.resolve() } as never } diff --git a/src/shared/codex-selection-lane.ts b/src/shared/codex-selection-lane.ts new file mode 100644 index 000000000..c7b80f671 --- /dev/null +++ b/src/shared/codex-selection-lane.ts @@ -0,0 +1,36 @@ +export type CodexAccountSelectionTarget = { + runtime?: 'host' | 'wsl' + wslDistro?: string | null +} + +export type NormalizedCodexAccountSelectionTarget = { + runtime: 'host' | 'wsl' + wslDistro: string | null +} + +export function normalizeCodexAccountSelectionTarget( + target?: CodexAccountSelectionTarget | null +): NormalizedCodexAccountSelectionTarget { + if (target?.runtime === 'wsl') { + return { + runtime: 'wsl', + wslDistro: normalizeWslDistro(target.wslDistro) + } + } + return { runtime: 'host', wslDistro: null } +} + +/** Stable identifier for the selection lane a launch resolves its account from. */ +export function getCodexSelectionLaneKey(target?: CodexAccountSelectionTarget | null): string { + const normalized = normalizeCodexAccountSelectionTarget(target) + return normalized.runtime === 'host' ? 'host' : `wsl:${getWslSelectionKey(normalized.wslDistro)}` +} + +export function getWslSelectionKey(wslDistro: string | null | undefined): string { + return normalizeWslDistro(wslDistro) ?? '__default__' +} + +function normalizeWslDistro(wslDistro: string | null | undefined): string | null { + const trimmed = wslDistro?.trim() + return trimmed ? trimmed : null +}