diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 179108b194b..a57e7c90f53 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -123,6 +123,8 @@ import { shouldPersistWorkspaceSession } from './lib/workspace-session' import { createSessionWriteSubscriber } from './lib/session-write-subscriber' +import { sweepRestoredCodexPanesForStaleAccounts } from './lib/codex-stale-pane-sweep' +import { installCodexDetachedPaneRestartExecutor } from '@/components/terminal-pane/codex-detached-pane-restart-scheduler' import { buildActiveViewUnloadPatch } from './lib/active-view-persist' import { buildWorkspaceSessionHostSnapshots, @@ -1090,6 +1092,9 @@ function App(): React.JSX.Element { await timeRendererStartupStep('recover-legacy-worker-terminals-post-reconnect', () => window.api.app.recoverLegacyWorkerTerminalsForRendererStartup() ) + // Why here: reconnect just published restored PTY ids; sweeping them now + // re-offers stale Codex panes whose tabs never mount this session. + sweepRestoredCodexPanesForStaleAccounts(useAppStore.getState()) syncZoomCSSVar() // Why (issue #1158): unlock the session writer only after hydration and all dependent steps succeeded, so a mid-startup throw can't serialize partially-mutated state to disk. actions.setHydrationSucceeded(true) @@ -1210,6 +1215,8 @@ function App(): React.JSX.Element { } }, []) + useEffect(() => installCodexDetachedPaneRestartExecutor(), []) + useEffect(() => { let previousKey = getRuntimeMobileSessionSyncKey(useAppStore.getState()) return useAppStore.subscribe((state, previousState) => { diff --git a/src/renderer/src/components/CodexRestartChip.tsx b/src/renderer/src/components/CodexRestartChip.tsx index 4df9c54a9ed..09835b71302 100644 --- a/src/renderer/src/components/CodexRestartChip.tsx +++ b/src/renderer/src/components/CodexRestartChip.tsx @@ -1,70 +1,11 @@ -import { useEffect, useId, useMemo, useRef } from 'react' -import { useShallow } from 'zustand/react/shallow' +import { useEffect, useId, useRef } from 'react' import { RefreshCw } from 'lucide-react' import { Button } from '@/components/ui/button' import { useAppStore } from '../store' -import { selectCodexRestartInputs } from './codex-restart-chip-inputs' import { translate } from '@/i18n/i18n' import { shouldFocusMobileDriverAction } from './terminal-pane/mobile-driver-overlay-focus' import { buildCodexRestartNoticeKey } from './codex-restart-notice-key' import { awaitsCodexRestartAnswer } from './codex-restart-notice-state' -import type { CodexRestartNotice } from '../store/slices/terminals' - -const EMPTY_TABS: { id: string }[] = [] - -export function collectStalePtyIdsForTabs({ - tabs, - ptyIdsByTabId, - codexRestartNoticeByPtyId -}: { - tabs: { id: string }[] - ptyIdsByTabId: Record - codexRestartNoticeByPtyId: Record -}): string[] { - // Why: an already-requested restart runs when its pane next mounts. Keeping it - // out of the prompt is what stops the panel from sticking on a worktree whose - // stale pane is parked or deferred and cannot answer the request yet. A - // dismissed notice is likewise answered — it only survives as launch-account - // memory. - return tabs.flatMap((tab) => - (ptyIdsByTabId[tab.id] ?? []).filter((ptyId) => - awaitsCodexRestartAnswer(codexRestartNoticeByPtyId[ptyId]) - ) - ) -} - -export function collectStaleWorktreePtyIds({ - tabsByWorktree, - ptyIdsByTabId, - codexRestartNoticeByPtyId, - worktreeId -}: { - tabsByWorktree: Record - ptyIdsByTabId: Record - codexRestartNoticeByPtyId: Record - worktreeId: string -}): string[] { - return collectStalePtyIdsForTabs({ - tabs: tabsByWorktree[worktreeId] ?? EMPTY_TABS, - ptyIdsByTabId, - codexRestartNoticeByPtyId - }) -} - -export function dismissStaleWorktreePtyIds( - staleWorktreePtyIds: string[], - dismissCodexRestartNotices: (ptyIds: string[]) => void, - forgetLaunchAccounts: (ptyIds: string[]) => void -): void { - // Why: restart notices are stored per PTY, but the workspace host presents - // one shared prompt. Dismissing all matching PTY notices keeps every pane in - // that worktree consistent with the dismissal. - dismissCodexRestartNotices(staleWorktreePtyIds) - // Why: notices are renderer-only, so without dropping the on-disk launch - // record the startup sweep re-raises this exact prompt — and re-blocks the - // pane's input — after every app restart the user already answered. - forgetLaunchAccounts(staleWorktreePtyIds) -} function isInsideHiddenTree(element: HTMLElement): boolean { return element.closest('[aria-hidden="true"], [hidden], [inert]') !== null @@ -78,58 +19,39 @@ type RestartNotice = { export default function CodexRestartChip({ isVisible = true, - worktreeId + ptyId, + shouldFocus = false }: { isVisible?: boolean - worktreeId: string + ptyId: string + shouldFocus?: boolean }): React.JSX.Element | null { - const tabs = useAppStore((s) => s.tabsByWorktree[worktreeId] ?? EMPTY_TABS) - // Why: both of these maps churn on unrelated pty lifecycle events (ptyIdsByTabId - // on attach/detach; codexRestartNoticeByPtyId is re-spread even when empty on - // pty teardown), so subscribe to them only while a restart notice actually - // exists. Otherwise this per-worktree chip re-rendered on every pty event to - // compute "no notice → render nothing". See codex-restart-chip-inputs. - const { ptyIdsByTabId, codexRestartNoticeByPtyId } = useAppStore( - useShallow(selectCodexRestartInputs) - ) - const staleWorktreePtyIds = useMemo( - () => - collectStalePtyIdsForTabs({ - tabs, - ptyIdsByTabId, - codexRestartNoticeByPtyId - }), - [codexRestartNoticeByPtyId, ptyIdsByTabId, tabs] - ) - const restartNotice = staleWorktreePtyIds[0] - ? codexRestartNoticeByPtyId[staleWorktreePtyIds[0]] - : undefined - const queueCodexPaneRestarts = useAppStore((s) => s.queueCodexPaneRestarts) - const dismissCodexRestartNotices = useAppStore((s) => s.dismissCodexRestartNotices) - - const noticeKey = restartNotice ? buildCodexRestartNoticeKey(restartNotice) : null - - if (staleWorktreePtyIds.length === 0 || !restartNotice) { + // Why: one O(1) selector per mounted pane stays idle when unrelated PTY maps + // churn and prevents a worktree-wide scan for every split pane. + const restartNotice = useAppStore((state) => state.codexRestartNoticeByPtyId[ptyId]) + if (!restartNotice || !awaitsCodexRestartAnswer(restartNotice)) { return null } const handleRestart = (): void => { - queueCodexPaneRestarts(staleWorktreePtyIds) + useAppStore.getState().queueCodexPaneRestarts([ptyId]) } const handleDismiss = (): void => { - dismissStaleWorktreePtyIds(staleWorktreePtyIds, dismissCodexRestartNotices, (ptyIds) => { - void window.api.codexAccounts.forgetStalePanes({ ptyIds }).catch((err: unknown) => { - console.warn('Failed to forget dismissed Codex pane accounts:', err) - }) + useAppStore.getState().dismissCodexRestartNotices([ptyId]) + // Why: notices are renderer-only, so the persisted launch record must be + // cleared for this pane or the startup sweep re-raises its answered prompt. + void window.api.codexAccounts.forgetStalePanes({ ptyIds: [ptyId] }).catch((err: unknown) => { + console.warn('Failed to forget dismissed Codex pane account:', err) }) } return ( @@ -140,12 +62,14 @@ function LoudRestartOverlay({ isVisible, noticeKey, restartNotice, + shouldFocus, onDismiss, onRestart }: { isVisible: boolean noticeKey: string | null restartNotice: RestartNotice + shouldFocus: boolean onDismiss: () => void onRestart: () => void }): React.JSX.Element { @@ -156,15 +80,10 @@ function LoudRestartOverlay({ // Why: move focus to the card only when the user isn't typing elsewhere; // unconditional autoFocus would steal keys from an active composer. // - // The target is the dialog itself, never Restart. This card is mounted per - // WORKTREE (a sibling of the split layout), so its focus scope is every - // terminal in the worktree — a notice for one pane lands while the user may - // be typing in a different, perfectly healthy pane. With Restart focused, the - // next Space/Enter of their prose queued a restart of every stale pane here - // and destroyed those sessions. Focus must not land on a destructive action - // the user never aimed at. See #10863. + // The target is the dialog itself, never Restart, so the next Space/Enter + // cannot destroy a session the user did not explicitly choose. See #10863. useEffect(() => { - if (!isVisible) { + if (!isVisible || !shouldFocus) { return } const root = rootRef.current @@ -175,7 +94,7 @@ function LoudRestartOverlay({ if (shouldFocusMobileDriverAction(document.activeElement, document.body, paneScope)) { root.focus() } - }, [isVisible, noticeKey]) + }, [isVisible, noticeKey, shouldFocus]) return (
- {(tabsByWorktree[workspace.id] ?? []) .filter((tab) => shouldMountBackgroundWorktreeTab( @@ -2656,7 +2654,6 @@ const WorktreeSplitSurface = React.memo(function WorktreeSplitSurface({ inert={!isVisible} aria-hidden={!isVisible} > - { - it('returns the frozen empty bundle while no restart notice exists', () => { - const state: CodexRestartInputsState = { - ptyIdsByTabId: { 'tab-1': ['pty-1'] }, - codexRestartNoticeByPtyId: {} - } - expect(selectCodexRestartInputs(state)).toBe(EMPTY_CODEX_RESTART_INPUTS) - - // Churning EITHER map while no notice exists must NOT change the selected - // reference, so a useShallow subscription skips the re-render. This covers - // the pty-teardown path that re-spreads codexRestartNoticeByPtyId even empty. - const churnedPty: CodexRestartInputsState = { - ...state, - ptyIdsByTabId: { 'tab-1': ['pty-2'], 'tab-2': ['pty-3'] } - } - const churnedNotice: CodexRestartInputsState = { - ptyIdsByTabId: state.ptyIdsByTabId, - codexRestartNoticeByPtyId: {} // fresh empty object, same as a teardown re-spread - } - expect(selectCodexRestartInputs(churnedPty)).toBe(EMPTY_CODEX_RESTART_INPUTS) - expect(selectCodexRestartInputs(churnedNotice)).toBe(EMPTY_CODEX_RESTART_INPUTS) - expect(shallow(selectCodexRestartInputs(state), selectCodexRestartInputs(churnedNotice))).toBe( - true - ) - }) - - it('re-idles once every surviving notice is answered', () => { - // Why: answered notices linger as launch-account memory for the pty's whole - // life, so existence alone would keep every chip subscribed to pty churn. - const dismissed: CodexRestartInputsState = { - ptyIdsByTabId: { 'tab-1': ['pty-1'] }, - codexRestartNoticeByPtyId: { - 'pty-1': { previousAccountLabel: 'a', nextAccountLabel: 'b', dismissed: true } - } - } - expect(selectCodexRestartInputs(dismissed)).toBe(EMPTY_CODEX_RESTART_INPUTS) - - const alsoUnanswered: CodexRestartInputsState = { - ...dismissed, - codexRestartNoticeByPtyId: { - ...dismissed.codexRestartNoticeByPtyId, - 'pty-2': { previousAccountLabel: 'a', nextAccountLabel: 'b' } - } - } - expect(selectCodexRestartInputs(alsoUnanswered)).not.toBe(EMPTY_CODEX_RESTART_INPUTS) - }) - - it('exposes both live maps the instant a restart notice exists', () => { - const ptyIdsByTabId = { 'tab-1': ['pty-1'] } - const codexRestartNoticeByPtyId = { - 'pty-1': { previousAccountLabel: 'a', nextAccountLabel: 'b' } - } - const state: CodexRestartInputsState = { ptyIdsByTabId, codexRestartNoticeByPtyId } - const selected = selectCodexRestartInputs(state) - // Live references pass straight through so the stale-pty memo + notice lookup derive fully. - expect(selected.ptyIdsByTabId).toBe(ptyIdsByTabId) - expect(selected.codexRestartNoticeByPtyId).toBe(codexRestartNoticeByPtyId) - expect(selected).not.toBe(EMPTY_CODEX_RESTART_INPUTS) - }) - - it('shallow-changes only when a live map reference changes while a notice exists', () => { - const ptyIdsByTabId = { 'tab-1': ['pty-1'] } - const s1: CodexRestartInputsState = { - ptyIdsByTabId, - codexRestartNoticeByPtyId: { - 'pty-1': { previousAccountLabel: 'a', nextAccountLabel: 'b' } - } - } - const r1 = selectCodexRestartInputs(s1) - expect(shallow(r1, selectCodexRestartInputs(s1))).toBe(true) - - const s2: CodexRestartInputsState = { ...s1, ptyIdsByTabId: { 'tab-1': ['pty-9'] } } - expect(shallow(r1, selectCodexRestartInputs(s2))).toBe(false) - }) -}) diff --git a/src/renderer/src/components/codex-restart-chip-inputs.ts b/src/renderer/src/components/codex-restart-chip-inputs.ts deleted file mode 100644 index 52adc9b9e26..00000000000 --- a/src/renderer/src/components/codex-restart-chip-inputs.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { awaitsCodexRestartAnswer } from './codex-restart-notice-state' -import type { AppState } from '@/store/types' - -export type CodexRestartInputsState = Pick - -export type CodexRestartInputs = CodexRestartInputsState - -// Why: shared frozen bundle returned while no Codex restart notice exists — the -// overwhelmingly common case. CodexRestartChip is mounted once per worktree -// Terminal (visible AND hidden-measurable) and once per split group, and reads -// these two maps only to find panes carrying a restart notice, which appear only -// on a Codex account switch. BOTH churn on unrelated pty lifecycle: -// ptyIdsByTabId gets a fresh identity on every pty register/attach/detach, and -// codexRestartNoticeByPtyId is re-spread into a new object even when empty on the -// pty exit/teardown path (terminals.ts clearTabPtyId). Subscribing to either -// re-rendered every mounted chip on that churn. Gate both behind notice-existence -// so idle chips keep the same reference and stop reacting; behavior is identical -// (no notice -> [] stale ids -> null render). Frozen so the singleton can't be -// mutated. -export const EMPTY_CODEX_RESTART_INPUTS: CodexRestartInputs = Object.freeze({ - ptyIdsByTabId: {}, - codexRestartNoticeByPtyId: {} -}) - -/** - * Expose the pty and restart-notice maps to the chip only while at least one - * Codex restart notice is live; otherwise return a stable frozen bundle so a - * `useShallow` subscription skips re-renders on unrelated pty-lifecycle churn. - */ -export function selectCodexRestartInputs(s: CodexRestartInputsState): CodexRestartInputs { - // Why: answered notices linger as launch-account memory for the life of the - // pty, so gating on mere existence would leave every chip permanently - // subscribed to that churn after a single dismissal. - if (!Object.values(s.codexRestartNoticeByPtyId).some(awaitsCodexRestartAnswer)) { - return EMPTY_CODEX_RESTART_INPUTS - } - return { - ptyIdsByTabId: s.ptyIdsByTabId, - codexRestartNoticeByPtyId: s.codexRestartNoticeByPtyId - } -} diff --git a/src/renderer/src/components/codex-restart-chip.test.ts b/src/renderer/src/components/codex-restart-chip.test.ts deleted file mode 100644 index d5bdd44e124..00000000000 --- a/src/renderer/src/components/codex-restart-chip.test.ts +++ /dev/null @@ -1,486 +0,0 @@ -// @vitest-environment happy-dom - -import React, { act } from 'react' -import { createRoot, type Root } from 'react-dom/client' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { useAppStore } from '../store' -import CodexRestartChip, { - collectStalePtyIdsForTabs, - collectStaleWorktreePtyIds, - dismissStaleWorktreePtyIds -} from './CodexRestartChip' - -let container: HTMLDivElement -let root: Root - -beforeEach(() => { - useAppStore.setState(useAppStore.getInitialState(), true) - container = document.createElement('div') - document.body.appendChild(container) - root = createRoot(container) -}) - -afterEach(() => { - act(() => { - root.unmount() - }) - container.remove() - useAppStore.setState(useAppStore.getInitialState(), true) -}) - -describe('CodexRestartChip helpers', () => { - it('collects all stale PTY ids for tabs in a worktree', () => { - expect( - collectStaleWorktreePtyIds({ - tabsByWorktree: { - wt1: [{ id: 'tab-1' }, { id: 'tab-2' }], - wt2: [{ id: 'tab-3' }] - }, - ptyIdsByTabId: { - 'tab-1': ['pty-1', 'pty-2'], - 'tab-2': ['pty-3'], - 'tab-3': ['pty-4'] - }, - codexRestartNoticeByPtyId: { - 'pty-1': { previousAccountLabel: 'a', nextAccountLabel: 'b' }, - 'pty-3': { previousAccountLabel: 'a', nextAccountLabel: 'b' }, - 'pty-4': { previousAccountLabel: 'a', nextAccountLabel: 'b' } - }, - worktreeId: 'wt1' - }) - ).toEqual(['pty-1', 'pty-3']) - }) - - it('returns an empty list when a worktree has no stale PTYs', () => { - expect( - collectStaleWorktreePtyIds({ - tabsByWorktree: { - wt1: [{ id: 'tab-1' }] - }, - ptyIdsByTabId: { - 'tab-1': ['pty-1'] - }, - codexRestartNoticeByPtyId: {}, - worktreeId: 'wt1' - }) - ).toEqual([]) - }) - - it('collects from one worktree tab slice without scanning the whole tab map', () => { - expect( - collectStalePtyIdsForTabs({ - tabs: [{ id: 'tab-1' }], - ptyIdsByTabId: { - 'tab-1': ['pty-1'], - 'tab-2': ['pty-2'] - }, - codexRestartNoticeByPtyId: { - 'pty-1': { previousAccountLabel: 'a', nextAccountLabel: 'b' }, - 'pty-2': { previousAccountLabel: 'a', nextAccountLabel: 'b' } - } - }) - ).toEqual(['pty-1']) - }) - - it('drops PTYs whose restart is already requested', () => { - expect( - collectStalePtyIdsForTabs({ - tabs: [{ id: 'tab-1' }], - ptyIdsByTabId: { - 'tab-1': ['pty-1', 'pty-2'] - }, - codexRestartNoticeByPtyId: { - 'pty-1': { previousAccountLabel: 'a', nextAccountLabel: 'b', restartRequested: true }, - 'pty-2': { previousAccountLabel: 'a', nextAccountLabel: 'b' } - } - }) - ).toEqual(['pty-2']) - }) - - it('dismisses every stale PTY notice in the worktree prompt', () => { - const dismissCodexRestartNotices = vi.fn() - const forgetLaunchAccounts = vi.fn() - - dismissStaleWorktreePtyIds(['pty-1', 'pty-3'], dismissCodexRestartNotices, forgetLaunchAccounts) - - expect(dismissCodexRestartNotices).toHaveBeenCalledWith(['pty-1', 'pty-3']) - expect(forgetLaunchAccounts).toHaveBeenCalledWith(['pty-1', 'pty-3']) - }) - - it('drops dismissed PTYs from the prompt while keeping their launch account', () => { - expect( - collectStalePtyIdsForTabs({ - tabs: [{ id: 'tab-1' }], - ptyIdsByTabId: { - 'tab-1': ['pty-1', 'pty-2'] - }, - codexRestartNoticeByPtyId: { - 'pty-1': { previousAccountLabel: 'a', nextAccountLabel: 'b', dismissed: true }, - 'pty-2': { previousAccountLabel: 'a', nextAccountLabel: 'b' } - } - }) - ).toEqual(['pty-2']) - }) - - it('renders only account-resolution actions without an external-store update loop', async () => { - useAppStore.setState({ - tabsByWorktree: { - 'worktree-1': [ - { - id: 'tab-1', - worktreeId: 'worktree-1', - title: 'Terminal', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: 1, - ptyId: null - } - ] - }, - ptyIdsByTabId: { - 'tab-1': ['pty-1'] - }, - codexRestartNoticeByPtyId: { - 'pty-1': { - previousAccountLabel: 'old@example.com', - nextAccountLabel: 'new@example.com' - } - } - }) - - await act(async () => { - root.render(React.createElement(CodexRestartChip, { worktreeId: 'worktree-1' })) - }) - - expect(container.textContent).toContain('Codex is still signed in as old@example.com') - expect( - Array.from(container.querySelectorAll('button'), (button) => button.textContent?.trim()) - ).toEqual(['Keep old account', 'Restart']) - }) - - it('uses configuration wording for a home-route restart', async () => { - useAppStore.setState({ - tabsByWorktree: { - 'worktree-1': [ - { - id: 'tab-1', - worktreeId: 'worktree-1', - title: 'Terminal', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: 1, - ptyId: null - } - ] - }, - ptyIdsByTabId: { 'tab-1': ['pty-1'] }, - codexRestartNoticeByPtyId: { - 'pty-1': { - previousAccountLabel: 'System default', - nextAccountLabel: 'System default', - previousAccountId: null, - nextAccountId: null, - homeRouteChanged: true - } - } - }) - - await act(async () => { - root.render(React.createElement(CodexRestartChip, { worktreeId: 'worktree-1' })) - }) - - expect(container.textContent).toContain('Codex setup changed') - expect(container.textContent).toContain('This Codex session is using an outdated configuration') - expect(container.textContent).toContain( - 'Restart this session to load your current Codex configuration.' - ) - expect( - Array.from(container.querySelectorAll('button'), (button) => button.textContent?.trim()) - ).toEqual(['Keep current session', 'Restart']) - }) - - it('forgets the launch record when the user keeps the old account', async () => { - const forgetStalePanes = vi.fn(() => Promise.resolve()) - Object.defineProperty(window, 'api', { - configurable: true, - value: { codexAccounts: { forgetStalePanes } } - }) - useAppStore.setState({ - tabsByWorktree: { - 'worktree-1': [ - { - id: 'tab-1', - worktreeId: 'worktree-1', - title: 'Terminal', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: 1, - ptyId: null - } - ] - }, - ptyIdsByTabId: { 'tab-1': ['pty-1'] }, - codexRestartNoticeByPtyId: { - 'pty-1': { - previousAccountLabel: 'old@example.com', - nextAccountLabel: 'new@example.com' - } - } - }) - await act(async () => { - root.render(React.createElement(CodexRestartChip, { worktreeId: 'worktree-1' })) - }) - - const dismissButton = Array.from(container.querySelectorAll('button')).find( - (button) => button.textContent?.trim() === 'Keep old account' - ) - await act(async () => { - dismissButton?.click() - }) - - // Why: without this the startup sweep re-raises the prompt the user just answered. - expect(forgetStalePanes).toHaveBeenCalledWith({ ptyIds: ['pty-1'] }) - expect(container.textContent).toBe('') - // Why: the record survives as the pane's launch-account memory, but marked - // answered so it neither prompts again nor blocks the pane's keyboard. - expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toEqual({ - previousAccountLabel: 'old@example.com', - nextAccountLabel: 'new@example.com', - dismissed: true - }) - }) - - it('stays closed when the user re-selects the account the pane launched under', async () => { - const forgetStalePanes = vi.fn(() => Promise.resolve()) - Object.defineProperty(window, 'api', { - configurable: true, - value: { codexAccounts: { forgetStalePanes } } - }) - useAppStore.setState({ - tabsByWorktree: { - 'worktree-1': [ - { - id: 'tab-1', - worktreeId: 'worktree-1', - title: 'Terminal', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: 1, - ptyId: null - } - ] - }, - ptyIdsByTabId: { 'tab-1': ['pty-1'] } - }) - useAppStore.getState().markCodexRestartNotices([ - { - ptyId: 'pty-1', - previousAccountLabel: 'old@example.com', - nextAccountLabel: 'new@example.com' - } - ]) - await act(async () => { - root.render(React.createElement(CodexRestartChip, { worktreeId: 'worktree-1' })) - }) - const dismissButton = Array.from(container.querySelectorAll('button')).find( - (button) => button.textContent?.trim() === 'Keep old account' - ) - await act(async () => { - dismissButton?.click() - }) - - // Re-select the pane's original account: it never left it, so there is - // nothing to restart and nothing to prompt about. - await act(async () => { - useAppStore.getState().markCodexRestartNotices([ - { - ptyId: 'pty-1', - previousAccountLabel: 'new@example.com', - nextAccountLabel: 'old@example.com' - } - ]) - }) - - expect(container.textContent).toBe('') - expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']).toBeUndefined() - }) - - it('reopens the prompt when a dismissed pane is out of date against a third account', async () => { - const forgetStalePanes = vi.fn(() => Promise.resolve()) - Object.defineProperty(window, 'api', { - configurable: true, - value: { codexAccounts: { forgetStalePanes } } - }) - useAppStore.setState({ - tabsByWorktree: { - 'worktree-1': [ - { - id: 'tab-1', - worktreeId: 'worktree-1', - title: 'Terminal', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: 1, - ptyId: null - } - ] - }, - ptyIdsByTabId: { 'tab-1': ['pty-1'] } - }) - useAppStore.getState().markCodexRestartNotices([ - { - ptyId: 'pty-1', - previousAccountLabel: 'old@example.com', - nextAccountLabel: 'new@example.com' - } - ]) - await act(async () => { - root.render(React.createElement(CodexRestartChip, { worktreeId: 'worktree-1' })) - }) - const dismissButton = Array.from(container.querySelectorAll('button')).find( - (button) => button.textContent?.trim() === 'Keep old account' - ) - await act(async () => { - dismissButton?.click() - }) - expect(container.textContent).toBe('') - - await act(async () => { - useAppStore.getState().markCodexRestartNotices([ - { - ptyId: 'pty-1', - previousAccountLabel: 'new@example.com', - nextAccountLabel: 'third@example.com' - } - ]) - }) - - // Why: the dismissal answered "keep old instead of new", not "never prompt - // again"; the pane really is out of date against third@example.com. - expect(container.textContent).toContain('Codex is still signed in as old@example.com') - expect(container.textContent).toContain('Restart this session to use third@example.com') - }) - - it('closes the prompt after Restart even when no mounted pane can run it yet', async () => { - useAppStore.setState({ - tabsByWorktree: { - 'worktree-1': [ - { - id: 'tab-1', - worktreeId: 'worktree-1', - title: 'Terminal', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: 1, - ptyId: null - } - ] - }, - ptyIdsByTabId: { 'tab-1': ['pty-1'] }, - codexRestartNoticeByPtyId: { - 'pty-1': { - previousAccountLabel: 'old@example.com', - nextAccountLabel: 'new@example.com' - } - } - }) - await act(async () => { - root.render(React.createElement(CodexRestartChip, { worktreeId: 'worktree-1' })) - }) - - const restartButton = Array.from(container.querySelectorAll('button')).find( - (button) => button.textContent?.trim() === 'Restart' - ) - await act(async () => { - restartButton?.click() - }) - - expect(container.textContent).toBe('') - // Why: the pane still has to restart; only the prompt is answered. - expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({ 'pty-1': true }) - expect(useAppStore.getState().codexRestartNoticeByPtyId['pty-1']?.restartRequested).toBe(true) - }) -}) - -describe('CodexRestartChip focus target', () => { - function renderWithFocusedSiblingPane(): HTMLTextAreaElement { - // Mirrors Terminal.tsx, which mounts the chip as a SIBLING of the split - // layout — so the chip's parentElement is the whole worktree surface and - // its focus scope covers every pane in it, not just the stale one. - const splitLayout = document.createElement('div') - const healthyPaneInput = document.createElement('textarea') - healthyPaneInput.className = 'xterm-helper-textarea' - splitLayout.appendChild(healthyPaneInput) - container.appendChild(splitLayout) - healthyPaneInput.focus() - - useAppStore.setState({ - tabsByWorktree: { - 'worktree-1': [ - { - id: 'tab-1', - worktreeId: 'worktree-1', - title: 'Terminal', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: 1, - ptyId: null - } - ] - }, - ptyIdsByTabId: { 'tab-1': ['pty-1'] } - }) - useAppStore.getState().markCodexRestartNotices([ - { - ptyId: 'pty-1', - previousAccountLabel: 'old@example.com', - nextAccountLabel: 'new@example.com' - } - ]) - return healthyPaneInput - } - - it('never parks focus on Restart, so a stray keystroke cannot kill the session', async () => { - renderWithFocusedSiblingPane() - - await act(async () => { - root.render(React.createElement(CodexRestartChip, { worktreeId: 'worktree-1' })) - }) - - // Regression (#10863): focus used to land on Restart. The card is worktree - // scoped, so it fires while the user is typing in a DIFFERENT, healthy pane - // — and the next Space/Enter of their prose restarted every stale pane here. - const restartButton = Array.from(container.querySelectorAll('button')).find( - (button) => button.textContent?.trim() === 'Restart' - ) - expect(restartButton).toBeDefined() - expect(document.activeElement).not.toBe(restartButton) - expect((document.activeElement as HTMLElement | null)?.getAttribute('role')).toBe('dialog') - - // The keystroke that used to destroy the session now does nothing. - await act(async () => { - ;(document.activeElement as HTMLElement | null)?.click() - }) - expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({}) - }) - - it('still moves focus off the terminal so the card is reachable by keyboard', async () => { - const healthyPaneInput = renderWithFocusedSiblingPane() - - await act(async () => { - root.render(React.createElement(CodexRestartChip, { worktreeId: 'worktree-1' })) - }) - - // Why: the fix must not turn into "never take focus" — assistive tech and - // keyboard users still need to land on the dialog and Tab to its actions. - expect(document.activeElement).not.toBe(healthyPaneInput) - expect(document.activeElement).toBe(container.querySelector('[role="dialog"]')) - }) -}) diff --git a/src/renderer/src/components/codex-restart-chip.test.tsx b/src/renderer/src/components/codex-restart-chip.test.tsx new file mode 100644 index 00000000000..d3767b2dc92 --- /dev/null +++ b/src/renderer/src/components/codex-restart-chip.test.tsx @@ -0,0 +1,295 @@ +// @vitest-environment happy-dom + +import React, { act, Profiler } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '../store' +import CodexRestartChip from './CodexRestartChip' + +globalThis.IS_REACT_ACT_ENVIRONMENT = true + +const PTY_ONE = 'worktree-1@@pty-1' +const PTY_TWO = 'worktree-1@@pty-2' + +let container: HTMLDivElement +let forgetStalePanes: ReturnType +let root: Root + +function notice(previousAccountLabel: string, nextAccountLabel: string) { + return { previousAccountLabel, nextAccountLabel } +} + +function button(scope: ParentNode, label: string): HTMLButtonElement { + const match = Array.from(scope.querySelectorAll('button')).find( + (candidate) => candidate.textContent?.trim() === label + ) + if (!match) { + throw new Error(`missing ${label} button`) + } + return match +} + +beforeEach(() => { + useAppStore.setState(useAppStore.getInitialState(), true) + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + forgetStalePanes = vi.fn(() => Promise.resolve()) + Object.defineProperty(window, 'api', { + configurable: true, + value: { codexAccounts: { forgetStalePanes } } + }) +}) + +afterEach(() => { + act(() => { + root.unmount() + }) + container.remove() + useAppStore.setState(useAppStore.getInitialState(), true) +}) + +describe('CodexRestartChip pane ownership', () => { + it('renders the notice owned by its exact PTY', async () => { + useAppStore.setState({ + codexRestartNoticeByPtyId: { + [PTY_ONE]: notice('old-one@example.com', 'new-one@example.com'), + [PTY_TWO]: notice('old-two@example.com', 'new-two@example.com') + } + }) + + await act(async () => { + root.render() + }) + + expect(container.textContent).toContain('Codex is still signed in as old-two@example.com') + expect(container.textContent).toContain('Restart this session to use new-two@example.com') + expect(container.textContent).not.toContain('old-one@example.com') + }) + + it('uses configuration wording for a home-route restart', async () => { + useAppStore.setState({ + codexRestartNoticeByPtyId: { + [PTY_ONE]: { + previousAccountLabel: 'System default', + nextAccountLabel: 'System default', + previousAccountId: null, + nextAccountId: null, + homeRouteChanged: true + } + } + }) + + await act(async () => { + root.render() + }) + + expect(container.textContent).toContain('Codex setup changed') + expect(container.textContent).toContain('This Codex session is using an outdated configuration') + expect(container.textContent).toContain( + 'Restart this session to load your current Codex configuration.' + ) + }) + + it('restarts only the pane whose action was clicked', async () => { + useAppStore.setState({ + codexRestartNoticeByPtyId: { + [PTY_ONE]: notice('old-one@example.com', 'new-one@example.com'), + [PTY_TWO]: notice('old-two@example.com', 'new-two@example.com') + } + }) + + await act(async () => { + root.render( + <> +
+ +
+
+ +
+ + ) + }) + const firstPane = container.querySelector('[data-pane="one"]')! + const secondPane = container.querySelector('[data-pane="two"]')! + + await act(async () => { + button(firstPane, 'Restart').click() + }) + + const state = useAppStore.getState() + expect(state.pendingCodexPaneRestartIds).toEqual({ [PTY_ONE]: true }) + expect(state.codexRestartNoticeByPtyId[PTY_ONE]?.restartRequested).toBe(true) + expect(state.codexRestartNoticeByPtyId[PTY_TWO]).toEqual( + notice('old-two@example.com', 'new-two@example.com') + ) + expect(firstPane.textContent).toBe('') + expect(secondPane.textContent).toContain('old-two@example.com') + }) + + it('dismisses and forgets only the pane whose action was clicked', async () => { + useAppStore.setState({ + codexRestartNoticeByPtyId: { + [PTY_ONE]: notice('old-one@example.com', 'new-one@example.com'), + [PTY_TWO]: notice('old-two@example.com', 'new-two@example.com') + } + }) + + await act(async () => { + root.render( + <> +
+ +
+
+ +
+ + ) + }) + const firstPane = container.querySelector('[data-pane="one"]')! + const secondPane = container.querySelector('[data-pane="two"]')! + + await act(async () => { + button(firstPane, 'Keep old account').click() + }) + + expect(forgetStalePanes).toHaveBeenCalledExactlyOnceWith({ ptyIds: [PTY_ONE] }) + expect(useAppStore.getState().codexRestartNoticeByPtyId[PTY_ONE]?.dismissed).toBe(true) + expect(useAppStore.getState().codexRestartNoticeByPtyId[PTY_TWO]?.dismissed).toBeUndefined() + expect(firstPane.textContent).toBe('') + expect(secondPane.textContent).toContain('old-two@example.com') + }) + + it('follows a pane replacement id without acting on the retired PTY', async () => { + useAppStore.setState({ + codexRestartNoticeByPtyId: { + [PTY_ONE]: notice('retired@example.com', 'target@example.com') + } + }) + await act(async () => { + root.render() + }) + + await act(async () => { + useAppStore.setState({ + codexRestartNoticeByPtyId: { + [PTY_TWO]: notice('replacement@example.com', 'target@example.com') + } + }) + root.render() + }) + await act(async () => { + button(container, 'Restart').click() + }) + + expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({ [PTY_TWO]: true }) + expect(useAppStore.getState().pendingCodexPaneRestartIds[PTY_ONE]).toBeUndefined() + }) + + it('stays hidden after this pane answers while a sibling remains unanswered', async () => { + useAppStore.setState({ + codexRestartNoticeByPtyId: { + [PTY_ONE]: { ...notice('a', 'b'), restartRequested: true }, + [PTY_TWO]: notice('c', 'd') + } + }) + + await act(async () => { + root.render() + }) + + expect(container.textContent).toBe('') + }) + + it('does not re-render for unrelated PTY and notice churn', async () => { + const onRender = vi.fn() + const ownedNotice = notice('a', 'b') + useAppStore.setState({ codexRestartNoticeByPtyId: { [PTY_ONE]: ownedNotice } }) + await act(async () => { + root.render( + + + + ) + }) + const commits = onRender.mock.calls.length + + await act(async () => { + useAppStore.setState({ + ptyIdsByTabId: { unrelated: [PTY_TWO] }, + codexRestartNoticeByPtyId: { + [PTY_ONE]: ownedNotice, + [PTY_TWO]: notice('c', 'd') + } + }) + }) + + expect(onRender).toHaveBeenCalledTimes(commits) + }) + + it('does not mutate restart state during a StrictMode replay', async () => { + useAppStore.setState({ + codexRestartNoticeByPtyId: { [PTY_ONE]: notice('old@example.com', 'new@example.com') } + }) + + await act(async () => { + root.render( + + + + ) + }) + + expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({}) + expect(useAppStore.getState().codexRestartNoticeByPtyId[PTY_ONE]).toEqual( + notice('old@example.com', 'new@example.com') + ) + }) +}) + +describe('CodexRestartChip pane focus', () => { + async function renderPane(shouldFocus: boolean, isVisible = true): Promise { + await act(async () => { + root.render( +
+