From 59aa8c6d2ba2e898aa58a38783dfad2c3cb255a7 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 26 Jul 2026 16:33:24 -0700 Subject: [PATCH] fix(codex): sweep stale panes on real PTY binds and stop filtering out launcher panes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live Windows 11 validation of #10770 found fix 3 never produced a card, for two independent reasons. Both are fixed here. **The startup sweep raced PTY bind.** markRestoredStaleCodexSessionsForRestart was called once, fire-and-forget, during hydration — when nothing is attached yet. Every inspectProcess answered terminal_gone, the pty-id list came back empty, the function early-returned and nothing ever retried. Invoking the same function by hand once the app had settled produced the correct notice immediately, so the logic was right and only the scheduling was wrong. Drive it from the bind chokepoint instead. Every real bind — fresh spawn, provider rebind, daemon reattach — funnels through the pane->PTY layout write in connectPanePty, which is the first moment a restored shell can be inspected at all. Each bound PTY is queued, coalesced over a short window so updateTabPtyId lands before the tab walk, and swept once. A pane whose process read is unusable, or a Codex tab still showing its shell mid-reattach, is retried on a three-step ladder and then dropped; an authoritative "not stale" stops immediately, and a pane that has already been notified is never queued again. **Launcher-started Codex panes were filtered out before the registry was even consulted.** The foreground test accepted only `codex`/`codex-*`, but Windows reports the DEEPEST process in the tree, so an Orca "Codex" pane running a subagent reads as pwsh -> node -> codex.exe -> claude.exe => "claude". Any live child — git, node, a subagent — did this, on the switch-time path as well as the sweep. Fall back to the tab's recorded launchAgent, which is metadata rather than a repaintable label, and pair it with a non-shell foreground plus a live child: a restart notice makes the pane drop every keystroke, so a pane the user exited back to its prompt must never be marked. --- src/renderer/src/App.tsx | 7 - .../terminal-pane/pty-connection.test.ts | 8 + .../terminal-pane/pty-connection.ts | 5 + .../codex-pane-restart-eligibility.test.ts | 95 ++++++++ .../src/lib/codex-pane-restart-eligibility.ts | 51 +++++ .../src/lib/codex-session-restart.test.ts | 44 ++++ src/renderer/src/lib/codex-session-restart.ts | 88 +++---- .../src/lib/codex-stale-pane-sweep.test.ts | 215 ++++++++++++++++++ .../src/lib/codex-stale-pane-sweep.ts | 105 +++++++++ 9 files changed, 573 insertions(+), 45 deletions(-) create mode 100644 src/renderer/src/lib/codex-pane-restart-eligibility.test.ts create mode 100644 src/renderer/src/lib/codex-pane-restart-eligibility.ts create mode 100644 src/renderer/src/lib/codex-stale-pane-sweep.test.ts create mode 100644 src/renderer/src/lib/codex-stale-pane-sweep.ts diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index e5c6ee83080..842a7a9e213 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -122,7 +122,6 @@ import { shouldPersistWorkspaceSession } from './lib/workspace-session' import { createSessionWriteSubscriber } from './lib/session-write-subscriber' -import { markRestoredStaleCodexSessionsForRestart } from './lib/codex-session-restart' import { buildActiveViewUnloadPatch } from './lib/active-view-persist' import { buildWorkspaceSessionHostSnapshots, @@ -1090,12 +1089,6 @@ function App(): React.JSX.Element { } } })() - // Why: restored Codex panes keep the account that was selected when - // their shell first opened, so re-raise the restart prompt the app - // restart discarded. Deferred — it is a hint, never startup-critical. - void markRestoredStaleCodexSessionsForRestart().catch((err: unknown) => { - console.warn('Codex stale-pane restart sweep failed:', err) - }) } } catch (error) { // Why (issue #1158): leave in-memory state untouched and keep hydrationSucceeded false (default-hydrating here once erased saved tabs); still flip the ready flags so the UI mounts. diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 422c25307ca..2984789cc43 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -96,6 +96,7 @@ async function renderHeadlessTerminalState( } const toastInfo = vi.fn() +const notifyCodexPaneBoundForStaleSweep = vi.fn() const LEAF_1 = '11111111-1111-4111-8111-111111111111' as const const LEAF_2 = '22222222-2222-4222-8222-222222222222' as const const UUID_RE = '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}' @@ -323,6 +324,10 @@ vi.mock('sonner', () => ({ } })) +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ + notifyCodexPaneBoundForStaleSweep +})) + // Why: the working→idle test invokes the real useNotificationDispatch hook outside React, so useCallback must pass through (safe suite-wide: no test here renders React). vi.mock('react', async (importOriginal) => { const actual = await importOriginal() @@ -4322,6 +4327,9 @@ describe('connectPanePty', () => { expect(pane.container.dataset.ptyId).toBe('pty-daemon-reattach') expect(deps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(1, 'pty-daemon-reattach') expect(deps.updateTabPtyId).toHaveBeenCalledWith('tab-1', 'pty-daemon-reattach') + // Why: the restored shell keeps the CODEX_HOME it was spawned with, and this + // bind is the first moment the daemon PTY can be inspected for it. + expect(notifyCodexPaneBoundForStaleSweep).toHaveBeenCalledWith('pty-daemon-reattach') }) it('drops xterm onData while pane is replaying restored bytes', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index df1e39e6ebb..3dde6bb6f6b 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -129,6 +129,9 @@ import { registerPtyTitleSource } from './pty-buffer-serializer' import { inspectRuntimeTerminalProcess } from '@/runtime/runtime-terminal-inspection' +// Why: a restored pane's stale-account prompt can only be raised once a PTY is +// actually attached — nothing is inspectable while the session hydrates. +import { notifyCodexPaneBoundForStaleSweep } from '@/lib/codex-stale-pane-sweep' import { getRemoteRuntimePtyEnvironmentId } from '@/runtime/runtime-terminal-stream' import { discardTerminalOutput, @@ -2815,6 +2818,7 @@ export function connectPanePty( registerSideEffectFactConsumerForPty(ptyId) syncHiddenRendererPtyDelivery() deps.syncPanePtyLayoutBinding(pane.id, ptyId) + notifyCodexPaneBoundForStaleSweep(ptyId) const tabPtyIds = useAppStore.getState().ptyIdsByTabId?.[deps.tabId] ?? [] if (options.updateTabPtyId !== 'if-missing' || !tabPtyIds.includes(ptyId)) { if (options.replacePtyId) { @@ -7326,6 +7330,7 @@ export function connectPanePty( registerSideEffectFactConsumerForPty(ptyId) syncHiddenRendererPtyDelivery() deps.syncPanePtyLayoutBinding(pane.id, ptyId) + notifyCodexPaneBoundForStaleSweep(ptyId) deps.updateTabPtyId(deps.tabId, ptyId) agentCompletionCoordinator.startProcessTracking() sampleVisiblePaneForegroundAgent() diff --git a/src/renderer/src/lib/codex-pane-restart-eligibility.test.ts b/src/renderer/src/lib/codex-pane-restart-eligibility.test.ts new file mode 100644 index 00000000000..91e70cc8ddb --- /dev/null +++ b/src/renderer/src/lib/codex-pane-restart-eligibility.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest' +import { isCodexRestartEligiblePane } from './codex-pane-restart-eligibility' + +describe('isCodexRestartEligiblePane', () => { + it('accepts a pane whose foreground is Codex itself', () => { + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess: 'codex', hasChildProcesses: false }, + launchAgent: undefined + }) + ).toBe(true) + }) + + it('accepts the shipped Codex binary name and the Windows .exe suffix', () => { + for (const foregroundProcess of ['codex-aarch64-ap', 'codex.exe']) { + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess, hasChildProcesses: false }, + launchAgent: undefined + }) + ).toBe(true) + } + }) + + it('accepts a launcher-started Codex pane whose deepest process is a subagent', () => { + // Windows reports pwsh -> node -> codex.exe -> claude.exe as "claude". + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess: 'claude.exe', hasChildProcesses: true }, + launchAgent: 'codex' + }) + ).toBe(true) + }) + + it('accepts a launcher-started Codex pane running an ordinary child such as git', () => { + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess: 'git', hasChildProcesses: true }, + launchAgent: 'codex' + }) + ).toBe(true) + }) + + it('rejects a Codex-launched pane the user exited back to a shell prompt', () => { + // A restart notice drops every keystroke, so a bare prompt must stay unmarked. + for (const foregroundProcess of ['pwsh.exe', 'powershell.exe', 'bash', 'zsh']) { + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess, hasChildProcesses: false }, + launchAgent: 'codex' + }) + ).toBe(false) + } + }) + + it('rejects a Codex-launched pane whose shell is reported with no live child', () => { + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess: 'pwsh.exe', hasChildProcesses: true }, + launchAgent: 'codex' + }) + ).toBe(false) + }) + + it('rejects a non-Codex pane with a live child process', () => { + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess: 'claude.exe', hasChildProcesses: true }, + launchAgent: 'claude' + }) + ).toBe(false) + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess: 'node', hasChildProcesses: true }, + launchAgent: undefined + }) + ).toBe(false) + }) + + it('rejects a pane whose process could not be read', () => { + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess: null, hasChildProcesses: false }, + launchAgent: 'codex' + }) + ).toBe(false) + // Why: a stale remote handle reports the last-known name; it is not evidence. + expect( + isCodexRestartEligiblePane({ + inspection: { foregroundProcess: 'codex', hasChildProcesses: true, unavailable: true }, + launchAgent: 'codex' + }) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/lib/codex-pane-restart-eligibility.ts b/src/renderer/src/lib/codex-pane-restart-eligibility.ts new file mode 100644 index 00000000000..4411f2bc5f5 --- /dev/null +++ b/src/renderer/src/lib/codex-pane-restart-eligibility.ts @@ -0,0 +1,51 @@ +import { isShellProcess } from '../../../shared/shell-process-detection' +import type { TuiAgent } from '../../../shared/types' +import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-terminal-inspection' + +function normalizeProcessName(processName: string | null): string | null { + if (!processName) { + return null + } + return processName.toLowerCase().replace(/\.exe$/, '') +} + +export function isCodexForegroundProcess(processName: string | null): boolean { + const normalized = normalizeProcessName(processName) + if (!normalized) { + return false + } + // Why: node-pty exposes the OS foreground process name, which can be the + // shipped Codex binary name (for example "codex-aarch64-ap" on macOS) + // instead of the shell command the user typed. Match on a Codex prefix so + // account-switch restart prompts still appear for real Codex sessions. + return normalized === 'codex' || normalized.startsWith('codex-') +} + +/** + * Decides whether a pane may be shown a Codex account-restart prompt. + * + * Why this is not just a foreground-name match: Windows reports the DEEPEST + * process in the PTY tree, so an Orca "Codex" pane running a subagent reads as + * `pwsh -> node -> codex.exe -> claude.exe` => "claude" and was filtered out + * before the stale-account registry was ever consulted. `launchAgent` is + * recorded metadata (Orca started Codex in this tab), not a repaintable label, + * so it survives that. It is deliberately paired with a non-shell foreground: + * a restart notice makes the pane drop every keystroke, so a pane the user has + * exited back to its shell prompt must never be marked. + */ +export function isCodexRestartEligiblePane(args: { + inspection: RuntimeTerminalProcessInspection + launchAgent: TuiAgent | undefined +}): boolean { + const { foregroundProcess, hasChildProcesses, unavailable } = args.inspection + if (unavailable === true) { + return false + } + if (isCodexForegroundProcess(foregroundProcess)) { + return true + } + if (args.launchAgent !== 'codex' || foregroundProcess === null) { + return false + } + return hasChildProcesses && !isShellProcess(foregroundProcess) +} diff --git a/src/renderer/src/lib/codex-session-restart.test.ts b/src/renderer/src/lib/codex-session-restart.test.ts index 2182a79a2bb..2c246866a61 100644 --- a/src/renderer/src/lib/codex-session-restart.test.ts +++ b/src/renderer/src/lib/codex-session-restart.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useAppStore } from '@/store' import { shouldUseShellReadyStartupDelivery } from '../../../shared/codex-startup-delivery' +import type { TuiAgent } from '../../../shared/types' import { CODEX_ACCOUNT_RESTART_STARTUP, markLiveCodexSessionsForRestart, @@ -16,6 +17,14 @@ const ACCOUNT_A = 'account-a@example.com' const ACCOUNT_B = 'account-b@example.com' const ACCOUNT_C = 'account-c@example.com' +function setLaunchAgentOnFirstTab(launchAgent: TuiAgent): void { + const [tab, ...rest] = useAppStore.getState().tabsByWorktree.wt1 ?? [] + if (!tab) { + throw new Error('expected a seeded tab') + } + useAppStore.setState({ tabsByWorktree: { wt1: [{ ...tab, launchAgent }, ...rest] } }) +} + describe('CODEX_ACCOUNT_RESTART_STARTUP', () => { it('waits for shell readiness before relaunching Codex after an account switch', () => { expect(CODEX_ACCOUNT_RESTART_STARTUP).toEqual({ @@ -181,6 +190,41 @@ describe('markLiveCodexSessionsForRestart', () => { expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) }) + it('marks a launcher-started Codex pane whose deepest process is a subagent', async () => { + // Windows reports pwsh -> node -> codex.exe -> claude.exe as "claude". + setLaunchAgentOnFirstTab('codex') + vi.mocked(window.api.pty.inspectProcess).mockResolvedValue({ + foregroundProcess: 'claude.exe', + hasChildProcesses: true + }) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + + expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + }) + + it('leaves a Codex-launched pane alone once the user exits back to the shell', async () => { + setLaunchAgentOnFirstTab('codex') + vi.mocked(window.api.pty.inspectProcess).mockResolvedValue({ + foregroundProcess: 'pwsh.exe', + hasChildProcesses: false + }) + + await markLiveCodexSessionsForRestart({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + + // Why: a restart notice drops every keystroke in that pane. + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + }) + it('still marks a confirmed Codex pane when another pane is unreachable', async () => { useAppStore.setState({ ptyIdsByTabId: { 'tab-1': ['pty-1', 'pty-stale'] } }) vi.mocked(window.api.pty.inspectProcess).mockImplementation(async (ptyId) => { diff --git a/src/renderer/src/lib/codex-session-restart.ts b/src/renderer/src/lib/codex-session-restart.ts index ba380c2250f..ddde138f48b 100644 --- a/src/renderer/src/lib/codex-session-restart.ts +++ b/src/renderer/src/lib/codex-session-restart.ts @@ -2,6 +2,7 @@ import type { AppState } from '@/store' import { useAppStore } from '@/store' import { inspectRuntimeTerminalProcess } from '@/runtime/runtime-terminal-inspection' import { translate } from '@/i18n/i18n' +import { isCodexRestartEligiblePane } from './codex-pane-restart-eligibility' // 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. @@ -10,52 +11,54 @@ export const CODEX_ACCOUNT_RESTART_STARTUP = { startupCommandDelivery: 'shell-ready' } as const -function normalizeProcessName(processName: string | null): string | null { - if (!processName) { - return null - } - return processName.toLowerCase().replace(/\.exe$/, '') +export type CodexPaneScanResult = { + ptyId: string + /** The pane may be shown a restart prompt (see isCodexRestartEligiblePane). */ + eligible: boolean + /** Inspection failed or the handle was stale, so a later read may answer differently. */ + inconclusive: boolean + /** Orca launched Codex in this tab, so a shell foreground can still be reattach settle. */ + launchedCodex: boolean + /** A restart notice was raised for this pane by this scan. */ + notified: boolean } -function isCodexForegroundProcess(processName: string | null): boolean { - const normalized = normalizeProcessName(processName) - if (!normalized) { - return false - } - // Why: node-pty exposes the OS foreground process name, which can be the - // shipped Codex binary name (for example "codex-aarch64-ap" on macOS) - // instead of the shell command the user typed. Match on a Codex prefix so - // account-switch restart prompts still appear for real Codex sessions. - return normalized === 'codex' || normalized.startsWith('codex-') -} - -async function getLiveCodexSessionPtyIds(state: AppState): Promise { +async function scanCodexPanes( + state: AppState, + ptyIdFilter: ReadonlySet | null +): Promise { const tabs = Object.values(state.tabsByWorktree).flat() - const checks = await Promise.all( + const scans = await Promise.all( tabs.map(async (tab) => { - const ptyIds = state.ptyIdsByTabId[tab.id] ?? [] - if (ptyIds.length === 0) { - return [] as string[] - } - + 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 foreground PTY process is the stable - // source of truth for whether this live tab is actually running Codex. - const foregroundProcesses = await Promise.all( - ptyIds.map((ptyId) => - inspectRuntimeTerminalProcess(state.settings, ptyId).then( - (inspection) => inspection.foregroundProcess, + // 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 + } + }) ) - return ptyIds.filter((_, index) => isCodexForegroundProcess(foregroundProcesses[index])) }) ) - return checks.flat() + return scans.flat() } export async function markLiveCodexSessionsForRestart(args: { @@ -63,7 +66,8 @@ export async function markLiveCodexSessionsForRestart(args: { nextAccountLabel: string }): Promise { const state = useAppStore.getState() - const liveCodexSessionPtyIds = await getLiveCodexSessionPtyIds(state) + const scans = await scanCodexPanes(state, null) + const liveCodexSessionPtyIds = scans.filter((scan) => scan.eligible).map((scan) => scan.ptyId) if (liveCodexSessionPtyIds.length === 0) { return } @@ -84,18 +88,24 @@ export async function markLiveCodexSessionsForRestart(args: { * the PTY daemon and survive a full app restart with the old account still * baked into their environment. Without this, quitting Orca before restarting a * stale pane silently strands it on the previous account forever. + * + * 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. */ -export async function markRestoredStaleCodexSessionsForRestart(): Promise { +export async function markRestoredStaleCodexSessionsForRestart(args?: { + ptyIds?: readonly string[] +}): Promise { const state = useAppStore.getState() - const liveCodexSessionPtyIds = await getLiveCodexSessionPtyIds(state) + const scans = await scanCodexPanes(state, args?.ptyIds ? new Set(args.ptyIds) : null) + const liveCodexSessionPtyIds = scans.filter((scan) => scan.eligible).map((scan) => scan.ptyId) if (liveCodexSessionPtyIds.length === 0) { - return + return scans } const stalePanes = await window.api.codexAccounts.listStalePanes({ ptyIds: liveCodexSessionPtyIds }) if (stalePanes.length === 0) { - return + return scans } const resolveAccountLabel = await createCodexAccountLabelResolver() @@ -106,6 +116,8 @@ export async function markRestoredStaleCodexSessionsForRestart(): Promise nextAccountLabel: resolveAccountLabel(pane.activeAccountId) })) ) + const notifiedPtyIds = new Set(stalePanes.map((pane) => pane.ptyId)) + return scans.map((scan) => (notifiedPtyIds.has(scan.ptyId) ? { ...scan, notified: true } : scan)) } async function createCodexAccountLabelResolver(): Promise<(accountId: string | null) => string> { diff --git a/src/renderer/src/lib/codex-stale-pane-sweep.test.ts b/src/renderer/src/lib/codex-stale-pane-sweep.test.ts new file mode 100644 index 00000000000..2ee889b51ae --- /dev/null +++ b/src/renderer/src/lib/codex-stale-pane-sweep.test.ts @@ -0,0 +1,215 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { + notifyCodexPaneBoundForStaleSweep, + resetCodexStalePaneSweepForTests +} from './codex-stale-pane-sweep' + +const ACCOUNT_A = 'account-a@example.com' +const ACCOUNT_B = 'account-b@example.com' + +const STALE_PANE = { + ptyId: 'pty-1', + launchAccountId: 'account-a', + activeAccountId: 'account-b' +} + +describe('notifyCodexPaneBoundForStaleSweep', () => { + const originalWindow = (globalThis as { window?: typeof window }).window + + beforeEach(() => { + vi.useFakeTimers() + resetCodexStalePaneSweepForTests() + useAppStore.setState({ + tabsByWorktree: { + wt1: [ + { + id: 'tab-1', + ptyId: 'pty-1', + worktreeId: 'wt1', + title: 'orca-1', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + launchAgent: 'codex' + } + ] + }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] }, + pendingCodexPaneRestartIds: {}, + codexRestartNoticeByPtyId: {} + }) + ;(globalThis as { window: typeof window }).window = { + ...originalWindow, + api: { + ...originalWindow?.api, + pty: { + ...originalWindow?.api?.pty, + inspectProcess: vi + .fn() + .mockResolvedValue({ foregroundProcess: 'codex', hasChildProcesses: false }) + }, + codexAccounts: { + ...originalWindow?.api?.codexAccounts, + list: vi.fn().mockResolvedValue({ + accounts: [ + { id: 'account-a', email: ACCOUNT_A }, + { id: 'account-b', email: ACCOUNT_B } + ], + activeAccountId: 'account-b' + }), + listStalePanes: vi.fn().mockResolvedValue([]) + } + } + } as unknown as typeof window + }) + + afterEach(() => { + resetCodexStalePaneSweepForTests() + vi.useRealTimers() + if (originalWindow) { + ;(globalThis as { window: typeof window }).window = originalWindow + } else { + delete (globalThis as { window?: typeof window }).window + } + }) + + it('raises the prompt once the pane PTY actually binds', async () => { + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + + notifyCodexPaneBoundForStaleSweep('pty-1') + // Nothing inspected yet: the bind has not settled. + expect(window.api.pty.inspectProcess).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(300) + + expect(window.api.codexAccounts.listStalePanes).toHaveBeenCalledWith({ ptyIds: ['pty-1'] }) + expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + }) + + it('coalesces a burst of startup binds into one sweep', async () => { + useAppStore.setState({ ptyIdsByTabId: { 'tab-1': ['pty-1', 'pty-2'] } }) + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + + notifyCodexPaneBoundForStaleSweep('pty-1') + notifyCodexPaneBoundForStaleSweep('pty-2') + await vi.advanceTimersByTimeAsync(300) + + expect(window.api.codexAccounts.listStalePanes).toHaveBeenCalledExactlyOnceWith({ + ptyIds: ['pty-1', 'pty-2'] + }) + }) + + it('retries a PTY whose process read is unusable until the reattach settles', async () => { + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + vi.mocked(window.api.pty.inspectProcess).mockRejectedValueOnce(new Error('terminal_gone')) + + notifyCodexPaneBoundForStaleSweep('pty-1') + await vi.advanceTimersByTimeAsync(300) + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + + await vi.advanceTimersByTimeAsync(1500) + + expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + }) + + it('gives up after a bounded ladder instead of polling forever', async () => { + vi.mocked(window.api.pty.inspectProcess).mockRejectedValue(new Error('terminal_gone')) + + notifyCodexPaneBoundForStaleSweep('pty-1') + await vi.advanceTimersByTimeAsync(60_000) + + expect(window.api.pty.inspectProcess).toHaveBeenCalledTimes(3) + }) + + it('stops retrying a pane the registry reports as not stale', async () => { + notifyCodexPaneBoundForStaleSweep('pty-1') + await vi.advanceTimersByTimeAsync(60_000) + + expect(window.api.codexAccounts.listStalePanes).toHaveBeenCalledTimes(1) + }) + + it('does not re-prompt a pane that already got its notice', async () => { + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + + notifyCodexPaneBoundForStaleSweep('pty-1') + await vi.advanceTimersByTimeAsync(300) + notifyCodexPaneBoundForStaleSweep('pty-1') + await vi.advanceTimersByTimeAsync(60_000) + + expect(window.api.codexAccounts.listStalePanes).toHaveBeenCalledTimes(1) + }) + + it('never marks a plain shell pane, so its input is never blocked', async () => { + useAppStore.setState({ + tabsByWorktree: { + wt1: [ + { + id: 'tab-1', + ptyId: 'pty-1', + worktreeId: 'wt1', + title: 'orca-1', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + }) + vi.mocked(window.api.pty.inspectProcess).mockResolvedValue({ + foregroundProcess: 'zsh', + hasChildProcesses: false + }) + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + + notifyCodexPaneBoundForStaleSweep('pty-1') + await vi.advanceTimersByTimeAsync(60_000) + + expect(window.api.codexAccounts.listStalePanes).not.toHaveBeenCalled() + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + }) + + it('retries a Codex tab still showing its shell, then prompts when Codex is up', async () => { + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + vi.mocked(window.api.pty.inspectProcess).mockResolvedValueOnce({ + foregroundProcess: 'pwsh.exe', + hasChildProcesses: false + }) + + notifyCodexPaneBoundForStaleSweep('pty-1') + await vi.advanceTimersByTimeAsync(300) + expect(useAppStore.getState().codexRestartNoticeByPtyId).toEqual({}) + + await vi.advanceTimersByTimeAsync(1500) + + expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ + previousAccountLabel: ACCOUNT_A, + nextAccountLabel: ACCOUNT_B + }) + }) + + it('retries a PTY the store has not yet listed against its tab', async () => { + useAppStore.setState({ ptyIdsByTabId: {} }) + vi.mocked(window.api.codexAccounts.listStalePanes).mockResolvedValue([STALE_PANE]) + + notifyCodexPaneBoundForStaleSweep('pty-1') + await vi.advanceTimersByTimeAsync(300) + expect(window.api.pty.inspectProcess).not.toHaveBeenCalled() + + useAppStore.setState({ ptyIdsByTabId: { 'tab-1': ['pty-1'] } }) + await vi.advanceTimersByTimeAsync(1500) + + 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 new file mode 100644 index 00000000000..cdaa8fd6db0 --- /dev/null +++ b/src/renderer/src/lib/codex-stale-pane-sweep.ts @@ -0,0 +1,105 @@ +import { + markRestoredStaleCodexSessionsForRestart, + type CodexPaneScanResult +} from './codex-session-restart' + +// Why: the first delay coalesces the startup burst of binds and lets +// updateTabPtyId (written just after the layout binding) land, since the scan +// walks tabs. The later two cover a reattached daemon shell that answers +// `inspectProcess` with terminal_gone for a beat. Bounded on purpose — this is +// a hint, and a pane that never resolves must not become a polling loop. +const SWEEP_ATTEMPT_DELAYS_MS = [300, 1500, 4000] as const + +const queuedPtyIds = new Set() +const attemptsByPtyId = new Map() +const notifiedPtyIds = new Set() +let flushTimer: ReturnType | null = null + +/** + * Queues a stale-account check for a PTY that has just bound to a pane. + * + * Why a bind signal rather than a startup call: nothing is attached while the + * session hydrates, so a one-shot sweep inspects zero PTYs and never retries. + * Every real bind rewrites the pane→PTY layout binding, which is the earliest + * point the daemon shell can be inspected at all. + */ +export function notifyCodexPaneBoundForStaleSweep(ptyId: string): void { + if (notifiedPtyIds.has(ptyId)) { + return + } + queuedPtyIds.add(ptyId) + arm(SWEEP_ATTEMPT_DELAYS_MS[0]) +} + +export function resetCodexStalePaneSweepForTests(): void { + if (flushTimer !== null) { + clearTimeout(flushTimer) + flushTimer = null + } + queuedPtyIds.clear() + attemptsByPtyId.clear() + notifiedPtyIds.clear() +} + +function arm(delayMs: number): void { + if (flushTimer !== null) { + return + } + flushTimer = setTimeout(() => { + flushTimer = null + void flush() + }, delayMs) +} + +function shouldRetry(scan: CodexPaneScanResult): boolean { + // Why: an eligible-but-unlisted pane got an authoritative "not stale" from the + // registry, so only an unusable read or a Codex tab still showing its shell + // (mid-reattach) can change answer on a later attempt. + return !scan.eligible && (scan.inconclusive || scan.launchedCodex) +} + +async function flush(): Promise { + const ptyIds = [...queuedPtyIds] + queuedPtyIds.clear() + if (ptyIds.length === 0) { + return + } + + let scans: CodexPaneScanResult[] + try { + scans = await markRestoredStaleCodexSessionsForRestart({ ptyIds }) + } catch (err) { + console.warn('Codex stale-pane restart sweep failed:', err) + return + } + + const scanByPtyId = new Map(scans.map((scan) => [scan.ptyId, scan])) + let nextDelayMs: number | null = null + for (const ptyId of ptyIds) { + const scan = scanByPtyId.get(ptyId) + if (scan?.notified === true) { + notifiedPtyIds.add(ptyId) + attemptsByPtyId.delete(ptyId) + continue + } + // Why: a PTY the scan never saw is not yet listed against its tab, which is + // the same "ask again shortly" case as an unusable process read. + if (scan !== undefined && !shouldRetry(scan)) { + attemptsByPtyId.delete(ptyId) + continue + } + const attempt = (attemptsByPtyId.get(ptyId) ?? 0) + 1 + const delayMs = SWEEP_ATTEMPT_DELAYS_MS[attempt] + if (delayMs === undefined) { + attemptsByPtyId.delete(ptyId) + continue + } + attemptsByPtyId.set(ptyId, attempt) + queuedPtyIds.add(ptyId) + nextDelayMs = nextDelayMs === null ? delayMs : Math.min(nextDelayMs, delayMs) + } + + if (nextDelayMs !== null) { + arm(nextDelayMs) + } +}