mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 00:02:10 +00:00
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
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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<string, unknown>
|
||||
}
|
||||
).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<string>((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()
|
||||
|
||||
|
||||
@@ -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<PtyForegroundProcessRead>
|
||||
}
|
||||
|
||||
function copySleepingAgentLaunchConfig(
|
||||
config: SleepingAgentLaunchConfig
|
||||
): SleepingAgentLaunchConfig {
|
||||
@@ -2873,6 +2885,7 @@ export class OrcaRuntimeService {
|
||||
private cloneInFlightByPath = new Map<string, Promise<void>>()
|
||||
private agentDetector: AgentDetector | null = null
|
||||
private ptyForegroundAgentRefreshes = new Map<string, PtyForegroundAgentRefresh>()
|
||||
private ptyForegroundProcessReads = new Map<string, PtyForegroundProcessReadEntry>()
|
||||
private ptyDelayedForegroundSnapshotTitleObservations = new Map<string, number>()
|
||||
private _orchestrationDb: OrchestrationDb | null = null
|
||||
private messageWaitersByHandle = new Map<string, Set<MessageWaiter>>()
|
||||
@@ -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<PtyForegroundProcessRead> | 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<string | null>
|
||||
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<boolean> => {
|
||||
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<boolean> {
|
||||
private async loadPtyForegroundAgentFromController(
|
||||
ptyId: string,
|
||||
afterTitleObservation = 0
|
||||
): Promise<boolean> {
|
||||
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
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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'
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user