diff --git a/src/main/codex/codex-structured-background-terminals.test.ts b/src/main/codex/codex-structured-background-terminals.test.ts index 3d0d2291a03..e24e80c1cb2 100644 --- a/src/main/codex/codex-structured-background-terminals.test.ts +++ b/src/main/codex/codex-structured-background-terminals.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import { CodexAppServerRequestError } from './codex-app-server-request-error' +import { CodexAppServerUnsupportedError } from './codex-app-server-session' import { createCodexBackgroundTerminals, refreshCodexBackgroundTerminals, @@ -74,13 +75,14 @@ describe('codex background terminals', () => { describe('capability probe', () => { it('latches off and stays quiet when the host refuses the operation', async () => { + // Method-not-found reaches callers as CodexAppServerUnsupportedError: + // the dispatcher converts -32601 before anyone sees it, so a + // RequestError carrying -32601 is a value production cannot produce. const request = vi .fn() .mockRejectedValue( - new CodexAppServerRequestError( - 'thread/backgroundTerminals/list', - -32601, - 'method not found' + new CodexAppServerUnsupportedError( + 'codex app-server does not support thread/backgroundTerminals/list: method not found' ) ) const terminals = createCodexBackgroundTerminals() @@ -157,10 +159,8 @@ describe('codex background terminals', () => { const request = vi .fn() .mockRejectedValue( - new CodexAppServerRequestError( - 'thread/backgroundTerminals/clean', - -32601, - 'method not found' + new CodexAppServerUnsupportedError( + 'codex app-server does not support thread/backgroundTerminals/clean: method not found' ) ) const terminals = createCodexBackgroundTerminals() @@ -174,3 +174,52 @@ describe('codex background terminals', () => { }) }) }) + +// Errors the dispatcher really produces for a host that HAS the methods. None +// of these may latch the capability off: one bad turn would otherwise hide the +// row and the Stop button for the rest of the session. +describe('refusals that must not latch the capability off', () => { + const transient = (code: number, message: string): CodexAppServerRequestError => + new CodexAppServerRequestError( + 'thread/backgroundTerminals/list', + code, + `codex app-server thread/backgroundTerminals/list failed: ${message}` + ) + + it.each([ + ['a thread lost to a resume race', -32600, 'invalid_request: thread not found'], + ['an internal server error', -32603, 'internal_error'], + ['the experimental-API gate', -32600, 'invalid_request: experimental API not enabled'] + ])('keeps asking after %s', async (_case, code, message) => { + const request = vi.fn().mockRejectedValue(transient(code as number, message as string)) + const terminals = createCodexBackgroundTerminals() + const session = makeSession(request) + + await refreshCodexBackgroundTerminals(terminals, session) + await refreshCodexBackgroundTerminals(terminals, session) + + // Not latched, so the next turn tries again... + expect(terminals.supported).not.toBe(false) + expect(request).toHaveBeenCalledTimes(2) + // ...and until one succeeds the client is shown nothing, which is the safe + // direction to fail in. + expect(terminals.state).toBeNull() + }) + + it('does not hide a live row because one stop attempt failed', async () => { + const terminals = createCodexBackgroundTerminals() + terminals.supported = true + terminals.state = { state: 'monitoring', tasks: [], supportsTaskStop: true } + const request = vi + .fn() + .mockRejectedValue( + new CodexAppServerRequestError('thread/backgroundTerminals/clean', -32603, 'internal_error') + ) + + const result = await stopCodexBackgroundTerminals(terminals, makeSession(request)) + + expect(result).toEqual({ cancelled: false }) + expect(terminals.supported).toBe(true) + expect(terminals.state).not.toBeNull() + }) +}) diff --git a/src/main/codex/codex-structured-background-terminals.ts b/src/main/codex/codex-structured-background-terminals.ts index ac3a9b26e13..95b93433bbd 100644 --- a/src/main/codex/codex-structured-background-terminals.ts +++ b/src/main/codex/codex-structured-background-terminals.ts @@ -2,7 +2,7 @@ import type { AgentSessionBackgroundTask, AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire' -import { isCodexAppServerRequestError } from './codex-app-server-connection' +import { isCodexAppServerUnsupportedError } from './codex-app-server-session' import type { CodexSession } from './codex-structured-session-state' /** @@ -10,10 +10,18 @@ import type { CodexSession } from './codex-structured-session-state' * outlive the turn. Interrupting a turn does not reap them; the app-server * exposes that as its own operation, and this is the only path to it. * - * The operations are experimental upstream, so a host can answer any of them - * with a refusal. `supported` starts null (never asked) and latches to false on - * the first refusal, which keeps the client's Stop control hidden rather than - * offering one that silently does nothing. + * The operations are experimental upstream, so a host may not have them at all. + * `supported` starts null (never asked) and latches to false only on + * CodexAppServerUnsupportedError — the class the dispatcher raises for + * method-not-found, and the only one capability caches are allowed to treat as + * unsupported (see CodexAppServerUnsupportedError's own contract). + * + * Every other refusal is transient by definition: a `thread not found` after a + * resume race, an internal error, the experimental-API gate on a host that may + * admit the call next launch. Those must not latch, or one bad turn would hide + * the control for the rest of the session. They leave `supported` at null, so + * the client still shows nothing — the safe direction — and the next turn asks + * again. */ export type CodexBackgroundTerminals = { supported: boolean | null @@ -99,7 +107,12 @@ export async function refreshCodexBackgroundTerminals( { timeoutMs } ) } catch (error) { - if (isCodexAppServerRequestError(error)) { + // Why only this class: method-not-found is the one signal that means "never + // going to work". Latching on the general request error would mark a healthy + // host unsupported after a single transient failure, and — worse — would + // never fire on a host that genuinely lacks the method, since that arrives + // as the unsupported class instead. + if (isCodexAppServerUnsupportedError(error)) { const had = terminals.state !== null terminals.supported = false terminals.state = null @@ -145,7 +158,7 @@ export async function stopCodexBackgroundTerminals( { timeoutMs } )) } catch (error) { - if (isCodexAppServerRequestError(error)) { + if (isCodexAppServerUnsupportedError(error)) { terminals.supported = false terminals.state = null } diff --git a/src/main/codex/codex-structured-session-background-tasks.test.ts b/src/main/codex/codex-structured-session-background-tasks.test.ts index b2e7fa02d53..60c821b95e6 100644 --- a/src/main/codex/codex-structured-session-background-tasks.test.ts +++ b/src/main/codex/codex-structured-session-background-tasks.test.ts @@ -1,13 +1,13 @@ import { describe, expect, it } from 'vitest' import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types' import type { AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire' -import { - CodexAppServerRequestError, - type CodexAppServerConnection, - type CodexAppServerConnectionHandlers, - type CodexAppServerLaunch, - type openCodexAppServerConnection +import type { + CodexAppServerConnection, + CodexAppServerConnectionHandlers, + CodexAppServerLaunch, + openCodexAppServerConnection } from './codex-app-server-connection' +import { CodexAppServerUnsupportedError } from './codex-app-server-session' import { CodexStructuredSessionAdapter } from './codex-structured-session-adapter' const THREAD_ID = 'thread-abc' @@ -171,9 +171,14 @@ describe('CodexStructuredSessionAdapter background terminals', () => { }) describe('a host without the operations', () => { + // The dispatcher converts -32601 into CodexAppServerUnsupportedError before + // any caller sees it, so a RequestError carrying -32601 is a value + // production can never build. Refuse the way a real host refuses. const refuse = (method: string): Route => { return () => { - throw new CodexAppServerRequestError(method, -32601, 'method not found') + throw new CodexAppServerUnsupportedError( + `codex app-server does not support ${method}: method not found` + ) } }