From 5d2ad3597a96a34decd828d71d9136c60da0f6ac Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:27:12 -0700 Subject: [PATCH] fix(native-chat): add direct Codex model selection (#12657) * fix(native-chat): select Codex models directly * fix(native-chat): confirm agent exits before switching views * fix(runtime): handle unavailable foreground probes --- .../use-mobile-native-chat-controller.ts | 3 +- ...mobile-native-chat-session-options.test.ts | 10 +- src/main/runtime/orca-runtime.test.ts | 93 +++++++++++++ src/main/runtime/orca-runtime.ts | 129 +++++++++++++++++- .../native-chat/NativeChatComposer.test.tsx | 10 +- .../native-chat-pty-session-options.test.ts | 2 +- ...ent-session-option-catalog-claude-codex.ts | 7 +- src/shared/agent-title-status.ts | 14 ++ ...ative-chat-session-option-commands.test.ts | 4 +- ...ative-chat-session-option-snapshot.test.ts | 9 +- src/shared/terminal-output-side-effects.ts | 5 + 11 files changed, 261 insertions(+), 25 deletions(-) diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index 1207a34c5e9..49d75409c05 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -295,8 +295,7 @@ export function useMobileNativeChatController(args: { onSendError }) - // A Codex-style model change happens in the agent's own TUI picker — bring - // the terminal view forward so the dispatched `/model` selector is visible. + // Bring the terminal view forward when an agent-owned picker command is used. const handleAgentPicker = useCallback(() => { if (activeSessionTabId && isTabChatView(activeSessionTabId)) { toggleTabChatView(activeSessionTabId) diff --git a/mobile/src/session/use-mobile-native-chat-session-options.test.ts b/mobile/src/session/use-mobile-native-chat-session-options.test.ts index 7cf5b2a683b..246c3021948 100644 --- a/mobile/src/session/use-mobile-native-chat-session-options.test.ts +++ b/mobile/src/session/use-mobile-native-chat-session-options.test.ts @@ -98,14 +98,14 @@ describe('useMobileNativeChatSessionOptions', () => { expect(api!.snapshot[0]).toMatchObject({ valueSource: 'unknown' }) }) - it('routes Codex model changes through the agent picker action', async () => { + it('applies Codex model changes through the native command', async () => { mount({ agent: 'codex' }) - expect(api!.snapshot[0]).toMatchObject({ action: { type: 'agent-picker' } }) + expect(api!.snapshot[0]?.action).toBeUndefined() await act(async () => { - await api!.invokeAction('model') + await api!.setOption('model', 'gpt-5.5') }) - expect(dispatchCommand).toHaveBeenCalledWith('/model') - expect(onAgentPicker).toHaveBeenCalledTimes(1) + expect(dispatchCommand).toHaveBeenCalledWith('/model gpt-5.5') + expect(onAgentPicker).not.toHaveBeenCalled() }) it('seeds the current model from a hook-reported provider model', () => { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 58cf61c533a..0454246661c 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -8835,6 +8835,99 @@ describe('OrcaRuntimeService', () => { expect(runtime.getPtyOutputSequence('pty-1')).toBe(0) }) + it('confirms title-based agent exits against the foreground process', async () => { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + runtime.onPtyData('pty-1', '\x1b]0;⠋ bichir\x07', 100) + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + batches.length = 0 + + const getForegroundProcess = vi.fn().mockResolvedValueOnce('codex') + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess + }) + runtime.onPtyData('pty-1', '\x1b]0;bichir\x07', 101) + + await vi.waitFor(() => expect(getForegroundProcess).toHaveBeenCalledOnce()) + await vi.waitFor(() => + expect(batches.flatMap((batch) => batch.facts)).toEqual([ + { kind: 'title', normalizedTitle: 'bichir', rawTitle: 'bichir' } + ]) + ) + await vi.waitFor(() => + expect( + ( + runtime as unknown as { + ptyForegroundProcessReads: Map + } + ).ptyForegroundProcessReads.size + ).toBe(0) + ) + await Promise.resolve() + + getForegroundProcess.mockResolvedValueOnce('zsh') + runtime.onPtyData('pty-1', '\x1b]0;other cwd\x07', 102) + + await vi.waitFor(() => + expect(batches.flatMap((batch) => batch.facts)).toContainEqual({ kind: 'agent-exited' }) + ) + expect(getForegroundProcess).toHaveBeenCalledTimes(2) + }) + + it('does not confirm an agent exit from a foreground read predating its title', async () => { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + let resolveStaleRead!: (process: string) => void + const staleRead = new Promise((resolve) => { + resolveStaleRead = resolve + }) + const getForegroundProcess = vi + .fn() + .mockReturnValueOnce(staleRead) + .mockResolvedValueOnce('zsh') + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess + }) + + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + runtime.onPtyData('pty-1', '\x1b]0;bichir\x07', 100) + expect(getForegroundProcess).toHaveBeenCalledOnce() + + resolveStaleRead('codex') + + await vi.waitFor(() => expect(getForegroundProcess).toHaveBeenCalledTimes(2)) + await vi.waitFor(() => + expect(batches.flatMap((batch) => batch.facts)).toContainEqual({ kind: 'agent-exited' }) + ) + }) + + it('treats synchronous foreground read failures as unavailable', async () => { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + const getForegroundProcess = vi.fn(() => { + throw new TypeError('getForegroundProcess is unavailable') + }) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess + }) + + runtime.onPtyData('pty-1', '\x1b]0;bichir\x07', 100) + + await vi.waitFor(() => + expect(batches.flatMap((batch) => batch.facts)).toContainEqual({ kind: 'agent-exited' }) + ) + expect(getForegroundProcess).toHaveBeenCalledOnce() + }) + it('aligns a restored session and pre-response bytes to the provider sequence', async () => { const { runtime } = createSideEffectRuntime() diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 7c1c092411e..5e23498f370 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1365,6 +1365,18 @@ type PtyForegroundAgentRefresh = { requestedAfterTitleObservation: number } +type PtyForegroundProcessRead = { + controller: RuntimePtyController + process: string | null + available: boolean +} + +type PtyForegroundProcessReadEntry = { + controller: RuntimePtyController + startedAfterTitleObservation: number + promise: Promise +} + function copySleepingAgentLaunchConfig( config: SleepingAgentLaunchConfig ): SleepingAgentLaunchConfig { @@ -2873,6 +2885,7 @@ export class OrcaRuntimeService { private cloneInFlightByPath = new Map>() private agentDetector: AgentDetector | null = null private ptyForegroundAgentRefreshes = new Map() + private ptyForegroundProcessReads = new Map() private ptyDelayedForegroundSnapshotTitleObservations = new Map() private _orchestrationDb: OrchestrationDb | null = null private messageWaitersByHandle = new Map>() @@ -10154,7 +10167,7 @@ export class OrcaRuntimeService { }) }, onAgentExited: () => { - this.recordTerminalSideEffectFact(ptyId, { kind: 'agent-exited' }) + this.confirmPtyAgentExit(ptyId) }, onCommandFinished: (exitCode: number | null) => { this.retirePtyAgentLaunchAuthority(ptyId) @@ -16645,6 +16658,99 @@ export class OrcaRuntimeService { ) } + private readPtyForegroundProcessFromController( + ptyId: string, + afterTitleObservation = 0 + ): Promise | null { + const controller = this.ptyController + if (!controller) { + return null + } + const pending = this.ptyForegroundProcessReads.get(ptyId) + if ( + pending?.controller === controller && + pending.startedAfterTitleObservation >= afterTitleObservation + ) { + return pending.promise + } + if (pending?.controller === controller) { + return pending.promise.then( + () => + this.readPtyForegroundProcessFromController(ptyId, afterTitleObservation) ?? { + controller, + process: null, + available: false + } + ) + } + const unavailable: PtyForegroundProcessRead = { + controller, + process: null, + available: false + } + let processRead: Promise + try { + processRead = Promise.resolve(controller.getForegroundProcess(ptyId)) + } catch { + const entry: PtyForegroundProcessReadEntry = { + controller, + startedAfterTitleObservation: afterTitleObservation, + promise: Promise.resolve(unavailable) + } + entry.promise = entry.promise.finally(() => { + if (this.ptyForegroundProcessReads.get(ptyId) === entry) { + this.ptyForegroundProcessReads.delete(ptyId) + } + }) + this.ptyForegroundProcessReads.set(ptyId, entry) + return entry.promise + } + let entry: PtyForegroundProcessReadEntry + const promise = processRead + .then((process) => ({ controller, process, available: true })) + .catch(() => unavailable) + .finally(() => { + if (this.ptyForegroundProcessReads.get(ptyId) === entry) { + this.ptyForegroundProcessReads.delete(ptyId) + } + }) + entry = { + controller, + startedAfterTitleObservation: afterTitleObservation, + promise + } + this.ptyForegroundProcessReads.set(ptyId, entry) + return entry.promise + } + + private confirmPtyAgentExit(ptyId: string): void { + const pty = this.ptysById.get(ptyId) + const titleObservedAt = pty?.lastOscTitleAt ?? null + const foregroundRead = this.readPtyForegroundProcessFromController(ptyId, titleObservedAt ?? 0) + if (!pty?.connected || !foregroundRead) { + this.recordTerminalSideEffectFact(ptyId, { kind: 'agent-exited' }) + return + } + void foregroundRead.then((result) => { + const current = this.ptysById.get(ptyId) + if (current !== pty || !current.connected) { + return + } + if (current.lastOscTitleAt !== titleObservedAt && current.lastAgentStatus !== null) { + return + } + if ( + result.controller === this.ptyController && + result.available && + recognizeAgentProcess(result.process) !== null + ) { + this.ptyTitleTrackersByPtyId.get(ptyId)?.tracker.restoreLastAgentExit() + return + } + this.recordTerminalSideEffectFact(ptyId, { kind: 'agent-exited' }) + }) + } + /** * Schedules an asynchronous query to check which agent process is currently * running in the foreground of a PTY. @@ -16707,7 +16813,10 @@ export class OrcaRuntimeService { const refresh = (async (): Promise => { while (true) { entry.startedAfterTitleObservation = entry.requestedAfterTitleObservation - const foregroundAgentChanged = await this.loadPtyForegroundAgentFromController(ptyId) + const foregroundAgentChanged = await this.loadPtyForegroundAgentFromController( + ptyId, + entry.startedAfterTitleObservation + ) if ( foregroundAgentChanged || entry.requestedAfterTitleObservation <= entry.startedAfterTitleObservation @@ -16729,7 +16838,10 @@ export class OrcaRuntimeService { * Queries the PTY controller for the active foreground process, identifies if it * is a recognized agent, and updates the PTY's foreground agent state if changed. */ - private async loadPtyForegroundAgentFromController(ptyId: string): Promise { + private async loadPtyForegroundAgentFromController( + ptyId: string, + afterTitleObservation = 0 + ): Promise { if (!this.ptyController) { return false } @@ -16743,12 +16855,15 @@ export class OrcaRuntimeService { if (pty.launchAgent) { return false } - let foregroundProcess: string | null - try { - foregroundProcess = await this.ptyController.getForegroundProcess(ptyId) - } catch { + const foregroundRead = this.readPtyForegroundProcessFromController(ptyId, afterTitleObservation) + if (!foregroundRead) { return false } + const result = await foregroundRead + if (result.controller !== this.ptyController || !result.available) { + return false + } + const foregroundProcess = result.process const foregroundAgent = foregroundProcess ? (recognizeAgentProcess(foregroundProcess)?.agent ?? null) : null diff --git a/src/renderer/src/components/native-chat/NativeChatComposer.test.tsx b/src/renderer/src/components/native-chat/NativeChatComposer.test.tsx index 4eac2b5739c..665f95449e9 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposer.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposer.test.tsx @@ -550,7 +550,7 @@ describe('NativeChatComposer', () => { expect(onSwitchToTerminal).toHaveBeenCalledOnce() }) - it('waits for the Codex picker command before switching to the terminal', async () => { + it('applies a Codex model change without switching to the terminal', async () => { mocks.sendHandle.settleAfterMs = 0 const onSwitchToTerminal = vi.fn() render( @@ -563,17 +563,17 @@ describe('NativeChatComposer', () => { /> ) - // Why: Codex model is agent-picker mid-session — setOption rejects; UI uses invokeAction. + // Codex model changes are value-bearing commands; only effort still uses the TUI picker. await act(async () => { - await mocks.fieldProps?.sessionOptionsSurface?.invokeAction('model') + await mocks.fieldProps?.sessionOptionsSurface?.setOption('model', 'gpt-5.5') }) expect(mocks.sendNativeChatMessageVerified).toHaveBeenCalledWith( {}, 'pty-1', - '/model', + '/model gpt-5.5', expect.any(AbortSignal) ) - expect(onSwitchToTerminal).toHaveBeenCalledOnce() + expect(onSwitchToTerminal).not.toHaveBeenCalled() }) }) diff --git a/src/renderer/src/components/native-chat/native-chat-pty-session-options.test.ts b/src/renderer/src/components/native-chat/native-chat-pty-session-options.test.ts index c4de028b244..4a90af7130e 100644 --- a/src/renderer/src/components/native-chat/native-chat-pty-session-options.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-pty-session-options.test.ts @@ -537,7 +537,7 @@ describe('native chat PTY session options', () => { ) }) - it('hands Codex model changes to the TUI picker and drops stale truth', async () => { + it('hands Codex effort changes to the TUI picker and drops stale truth', async () => { seedNativeChatAppliedSessionOptions('pty-1', 'codex', { model: 'gpt-5.5', effort: 'high' diff --git a/src/shared/agent-session-option-catalog-claude-codex.ts b/src/shared/agent-session-option-catalog-claude-codex.ts index 71327f1d532..ab13dda4dc5 100644 --- a/src/shared/agent-session-option-catalog-claude-codex.ts +++ b/src/shared/agent-session-option-catalog-claude-codex.ts @@ -203,6 +203,11 @@ export const CODEX_SESSION_OPTION_CATALOG: AgentSessionOptionCatalog = { modelApply: { launchArgs: (value) => ['-m', String(value)], agentArgsOverride: (tokens) => hasFlag(tokens, ['-m', '--model']), - midSession: { kind: 'agent-picker', command: '/model' } + // Codex accepts a model argument in its live /model command. + midSession: { + kind: 'command', + build: (value) => `/model ${String(value)}`, + pickerCommand: '/model' + } } } diff --git a/src/shared/agent-title-status.ts b/src/shared/agent-title-status.ts index 7c589e9935e..68c9dd4633a 100644 --- a/src/shared/agent-title-status.ts +++ b/src/shared/agent-title-status.ts @@ -56,16 +56,21 @@ export function createAgentStatusTracker( ): { handleTitle: (title: string) => void seedTitle: (title: string) => void + restoreLastExit: () => void reset: () => void } { // Why: trackers restored mid-session need a last-known status without firing // callbacks, or a hidden working agent can miss its later idle transition. let lastStatus: AgentStatus | null = initialTitle !== undefined ? detectAgentStatusFromTitle(initialTitle) : null + let restorableExitStatus: AgentStatus | null = null return { handleTitle(title: string): void { const newStatus = detectAgentStatusFromTitle(title) + if (newStatus !== null) { + restorableExitStatus = null + } if (lastStatus === 'working' && newStatus !== null && newStatus !== 'working') { onBecameIdle(title) } @@ -75,6 +80,7 @@ export function createAgentStatusTracker( // Why: reverting to a plain shell prompt after idle/permission means the // agent exited; while working it can just be a transient internal title. if (lastStatus !== null && lastStatus !== 'working' && newStatus === null) { + restorableExitStatus = lastStatus lastStatus = null onAgentExited?.() } @@ -84,9 +90,17 @@ export function createAgentStatusTracker( }, seedTitle(title: string): void { lastStatus = detectAgentStatusFromTitle(title) + restorableExitStatus = null + }, + restoreLastExit(): void { + if (lastStatus === null && restorableExitStatus !== null) { + lastStatus = restorableExitStatus + } + restorableExitStatus = null }, reset(): void { lastStatus = null + restorableExitStatus = null } } } diff --git a/src/shared/native-chat-session-option-commands.test.ts b/src/shared/native-chat-session-option-commands.test.ts index 71869339272..94f97cf23d1 100644 --- a/src/shared/native-chat-session-option-commands.test.ts +++ b/src/shared/native-chat-session-option-commands.test.ts @@ -67,7 +67,7 @@ describe('buildNativeChatSessionOptionCommand', () => { ).toBe('/fast') }) - it('has no absolute command for agent-picker applies (Codex model)', () => { + it('builds an absolute command for live Codex model changes', () => { expect( buildNativeChatSessionOptionCommand({ optionId: 'model', @@ -78,7 +78,7 @@ describe('buildNativeChatSessionOptionCommand', () => { models: CODEX_SESSION_OPTION_CATALOG.models, record: createNativeChatSessionOptionRecord('codex') }) - ).toBeNull() + ).toBe('/model gpt-5.5') }) }) diff --git a/src/shared/native-chat-session-option-snapshot.test.ts b/src/shared/native-chat-session-option-snapshot.test.ts index b7fba78ff0e..304c51f0073 100644 --- a/src/shared/native-chat-session-option-snapshot.test.ts +++ b/src/shared/native-chat-session-option-snapshot.test.ts @@ -172,7 +172,7 @@ describe('buildNativeChatSessionOptionSnapshot', () => { }) }) - it('exposes Codex model changes as an agent-picker action', () => { + it('exposes Codex model changes as native selectable values', () => { const snapshot = buildNativeChatSessionOptionSnapshot({ catalog: CODEX_SESSION_OPTION_CATALOG, models: CODEX_SESSION_OPTION_CATALOG.models, @@ -180,7 +180,12 @@ describe('buildNativeChatSessionOptionSnapshot', () => { mode: 'live', modelLabel: 'Model' }) - expect(snapshot[0]).toMatchObject({ settable: true, action: { type: 'agent-picker' } }) + expect(snapshot[0]).toMatchObject({ settable: true }) + expect(snapshot[0]?.action).toBeUndefined() + expect(snapshot[0]?.kind).toMatchObject({ + type: 'select', + choices: expect.arrayContaining([{ value: 'gpt-5.5', label: 'GPT-5.5' }]) + }) }) it('marks flip-only toggles without a baseline as toggle actions', () => { diff --git a/src/shared/terminal-output-side-effects.ts b/src/shared/terminal-output-side-effects.ts index a022e2469f2..227df5bd4e3 100644 --- a/src/shared/terminal-output-side-effects.ts +++ b/src/shared/terminal-output-side-effects.ts @@ -90,6 +90,8 @@ export type TerminalTitleTracker = { * No-ops once any title has been observed or seeded (live state wins); fires no callbacks. */ seedInitialTitle: (rawTitle: string) => void + /** Restore the status consumed by the latest exit candidate when process evidence disproves it. */ + restoreLastAgentExit: () => void /** Last title surfaced through onTitle, after normalization. */ getLastNormalizedTitle: () => string | null /** @@ -262,6 +264,9 @@ export function createTerminalTitleTracker( lastEmittedTitle = normalizeTerminalTitle(rawTitle) agentTracker?.seedTitle(rawTitle) }, + restoreLastAgentExit(): void { + agentTracker?.restoreLastExit() + }, getLastNormalizedTitle: () => lastEmittedTitle, setTransientFactScanningSuppressed(suppressed: boolean): void { if (suppressed === transientFactScanningSuppressed) {