Skip to content

fix(codex): surface remote auth and app-server errors #483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Shujakuinkuraudo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(codex): surface remote auth and app-server errors #483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 85 additions & 10 deletions cli/src/codex/codexAppServerClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,44 @@
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
import { logger } from '@/ui/logger';
import { killProcessByChildProcess } from '@/utils/process';

const MAX_STDERR_TAIL_CHARS = 4000;

export type CodexErrorStage = 'app-server-spawn' | 'app-server-request' | 'process-exit' | 'protocol' | 'response-error';

export class CodexAppServerError extends Error {
readonly stage: CodexErrorStage;
readonly stderrTail?: string;
readonly exitCode?: number | null;
readonly signal?: NodeJS.Signals | null;
readonly errorCode?: number;
readonly retryable?: boolean;

constructor(message: string, options: {
stage: CodexErrorStage;
stderrTail?: string;
exitCode?: number | null;
signal?: NodeJS.Signals | null;
errorCode?: number;
retryable?: boolean;
cause?: unknown;
}) {
super(message, options.cause !== undefined ? { cause: options.cause } : undefined);
this.name = 'CodexAppServerError';
this.stage = options.stage;
this.stderrTail = options.stderrTail;
this.exitCode = options.exitCode;
this.signal = options.signal;
this.errorCode = options.errorCode;
this.retryable = options.retryable;
}
}

function appendTail(current: string, chunk: string): string {
if (!chunk) return current;
const combined = current + chunk;
return combined.length > MAX_STDERR_TAIL_CHARS ? combined.slice(-MAX_STDERR_TAIL_CHARS) : combined;
}
import type {
InitializeParams,
InitializeResponse,
Expand Down Expand Up @@ -65,6 +103,7 @@ export class CodexAppServerClient {
private readonly requestHandlers = new Map<string, RequestHandler>();
private notificationHandler: ((method: string, params: unknown) => void) | null = null;
private protocolError: Error | null = null;
private stderrTail = '';

static readonly DEFAULT_TIMEOUT_MS = 14 * 24 * 60 * 60 * 1000;

Expand All @@ -88,16 +127,23 @@ export class CodexAppServerClient {

this.process.stderr.setEncoding('utf8');
this.process.stderr.on('data', (chunk) => {
const text = chunk.toString().trim();
if (text.length > 0) {
logger.debug(`[CodexAppServer][stderr] ${text}`);
const text = chunk.toString();
this.stderrTail = appendTail(this.stderrTail, text);
const trimmed = text.trim();
if (trimmed.length > 0) {
logger.debug(`[CodexAppServer][stderr] ${trimmed}`);
}
});

this.process.on('exit', (code, signal) => {
const message = `Codex app-server exited (code=${code ?? 'null'}, signal=${signal ?? 'null'})`;
logger.debug(message);
this.rejectAllPending(new Error(message));
this.rejectAllPending(new CodexAppServerError(message, {
stage: 'process-exit',
stderrTail: this.getStderrTail(),
exitCode: code,
signal
}));
this.connected = false;
this.resetParserState();
this.process = null;
Expand All @@ -106,9 +152,13 @@ export class CodexAppServerClient {
this.process.on('error', (error) => {
logger.debug('[CodexAppServer] Process error', error);
const message = error instanceof Error ? error.message : String(error);
this.rejectAllPending(new Error(
this.rejectAllPending(new CodexAppServerError(
`Failed to spawn codex app-server: ${message}. Is it installed and on PATH?`,
{ cause: error }
{
stage: 'app-server-spawn',
stderrTail: this.getStderrTail(),
cause: error
}
));
this.connected = false;
this.resetParserState();
Expand Down Expand Up @@ -180,7 +230,10 @@ export class CodexAppServerClient {
} catch (error) {
logger.debug('[CodexAppServer] Error while stopping process', error);
} finally {
this.rejectAllPending(new Error('Codex app-server disconnected'));
this.rejectAllPending(new CodexAppServerError('Codex app-server disconnected', {
stage: 'process-exit',
stderrTail: this.getStderrTail()
}));
this.connected = false;
this.resetParserState();
}
Expand Down Expand Up @@ -240,7 +293,10 @@ export class CodexAppServerClient {
if (this.pending.has(id)) {
this.pending.delete(id);
cleanup();
reject(new Error(`Codex app-server request '${method}' timed out after ${timeoutMs}ms`));
reject(new CodexAppServerError(`Codex app-server request '${method}' timed out after ${timeoutMs}ms`, {
stage: 'app-server-request',
stderrTail: this.getStderrTail()
}));
}
}, timeoutMs);
timeout.unref();
Expand Down Expand Up @@ -297,7 +353,10 @@ export class CodexAppServerClient {
return;
}
} catch (error) {
const protocolError = new Error('Failed to parse JSON from codex app-server');
const protocolError = new CodexAppServerError('Failed to parse JSON from codex app-server', {
stage: 'protocol',
stderrTail: this.getStderrTail()
});
this.protocolError = protocolError;
logger.debug('[CodexAppServer] Failed to parse JSON line', { line, error });
this.rejectAllPending(protocolError);
Expand Down Expand Up @@ -382,7 +441,12 @@ export class CodexAppServerClient {
this.pending.delete(response.id);

if (response.error) {
pending.reject(new Error(response.error.message));
pending.reject(new CodexAppServerError(response.error.message, {
stage: 'response-error',
stderrTail: this.getStderrTail(),
errorCode: typeof response.error.code === 'number' ? response.error.code : undefined,
retryable: this.isRetryableError(response.error)
}));
return;
}

Expand All @@ -397,6 +461,17 @@ export class CodexAppServerClient {
private resetParserState(): void {
this.buffer = '';
this.protocolError = null;
this.stderrTail = '';
}

getStderrTail(): string | undefined {
const trimmed = this.stderrTail.trim();
return trimmed.length > 0 ? trimmed : undefined;
}

private isRetryableError(error: { code?: number; message: string; data?: unknown }): boolean {
const haystack = [error.message, typeof error.data === 'string' ? error.data : ''].join('\n').toLowerCase();
return haystack.includes('high demand') || haystack.includes('rate limit') || haystack.includes('temporar');
}

private rejectAllPending(error: Error): void {
Expand Down
3 changes: 2 additions & 1 deletion cli/src/codex/codexLocal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export async function codexLocal(opts: {
installHint: 'Codex CLI',
includeCause: true,
logExit: true,
shell: process.platform === 'win32'
shell: process.platform === 'win32',
stdio: ['inherit', 'inherit', 'pipe']
Copy link
Copy Markdown

@github-actions github-actions bot Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] Piping stderr here hides it from local interactive users. In this path spawnWithAbort only buffers child.stderr for the final rejection text and never writes it back to process.stderr, so auth failures and other diagnostics disappear until the process exits.

Suggested fix:

await spawnWithTerminalGuard({
    ...,
    stdio: ['inherit', 'inherit', 'inherit']
});

});
}
44 changes: 39 additions & 5 deletions cli/src/codex/codexRemoteLauncher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { randomUUID } from 'node:crypto';

import { CodexAppServerClient } from './codexAppServerClient';
import { CodexAppServerClient, CodexAppServerError } from './codexAppServerClient';
import { CodexPermissionHandler } from './utils/permissionHandler';
import { ReasoningProcessor } from './utils/reasoningProcessor';
import { DiffProcessor } from './utils/diffProcessor';
Expand All @@ -25,6 +25,25 @@ import {
type HappyServer = Awaited<ReturnType<typeof buildHapiMcpBridge>>['server'];
type QueuedMessage = { message: string; mode: EnhancedMode; isolate: boolean; hash: string };

function formatCodexErrorForUser(error: unknown): string {
if (!(error instanceof Error)) {
return String(error);
}

const lines: string[] = [error.message];
if (error instanceof CodexAppServerError) {
const details: string[] = [];
details.push(`stage=${error.stage}`);
if (typeof error.errorCode === 'number') details.push(`errorCode=${error.errorCode}`);
if (typeof error.exitCode === 'number') details.push(`exitCode=${error.exitCode}`);
if (error.signal) details.push(`signal=${error.signal}`);
if (error.retryable) details.push('retryable=true');
if (details.length > 0) lines.push(`(${details.join(', ')})`);
if (error.stderrTail) lines.push(`stderr:\n${error.stderrTail}`);
}
return lines.join('\n');
}

class CodexRemoteLauncher extends RemoteLauncherBase {
private readonly session: CodexSession;
private readonly appServerClient: CodexAppServerClient;
Expand Down Expand Up @@ -268,11 +287,17 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
}

if (isTerminalEvent) {
const hasFailureDetails = msgType === 'task_failed' && Boolean(
asString(msg.error) ||
asString(msg.stderr) ||
typeof msg.exit_code === 'number'
);
if (shouldIgnoreTerminalEvent({
eventTurnId,
currentTurnId: this.currentTurnId,
turnInFlight,
allowAnonymousTerminalEvent
allowAnonymousTerminalEvent,
acceptAnonymousFailureWithDetails: hasFailureDetails
})) {
logger.debug(
`[Codex] Ignoring terminal event ${msgType} without matching turn context; ` +
Expand Down Expand Up @@ -314,7 +339,15 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
messageBuffer.addMessage('Turn aborted', 'status');
} else if (msgType === 'task_failed') {
const error = asString(msg.error);
messageBuffer.addMessage(error ? `Task failed: ${error}` : 'Task failed', 'status');
const stderr = asString(msg.stderr);
const exitCode = typeof msg.exit_code === 'number' ? msg.exit_code : null;
const retryable = msg.retryable === true;
const detail = [error, stderr ? `stderr: ${stderr}` : null, exitCode !== null ? `exitCode=${exitCode}` : null, retryable ? 'retryable=true' : null]
.filter((value): value is string => typeof value === 'string' && value.length > 0)
.join(' | ');
const taskFailedMessage = detail ? `Task failed: ${detail}` : 'Task failed';
messageBuffer.addMessage(taskFailedMessage, 'status');
session.sendSessionEvent({ type: 'message', message: taskFailedMessage });
}

if (msgType === 'task_started') {
Expand Down Expand Up @@ -711,8 +744,9 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
messageBuffer.addMessage('Aborted by user', 'status');
session.sendSessionEvent({ type: 'message', message: 'Aborted by user' });
} else {
messageBuffer.addMessage('Process exited unexpectedly', 'status');
session.sendSessionEvent({ type: 'message', message: 'Process exited unexpectedly' });
const detail = formatCodexErrorForUser(error);
messageBuffer.addMessage(`Codex error: ${detail.split('\n')[0]}`, 'status');
session.sendSessionEvent({ type: 'message', message: `Codex error:\n${detail}` });
this.currentTurnId = null;
this.currentThreadId = null;
hasThread = false;
Expand Down
30 changes: 21 additions & 9 deletions cli/src/codex/utils/appServerEventConverter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,15 @@ export class AppServerEventConverter {
event.turn_id = turnId;
}
if (msgType === 'task_failed') {
const error = asString(msg.error ?? msg.message ?? asRecord(msg.error)?.message);
if (error) {
event.error = error;
}
const errorRecord = asRecord(msg.error);
const error = asString(msg.error ?? msg.message ?? errorRecord?.message);
const stderr = asString(msg.stderr ?? errorRecord?.stderr);
const exitCode = asNumber(msg.exit_code ?? msg.exitCode ?? errorRecord?.exit_code ?? errorRecord?.exitCode);
const retryable = asBoolean(msg.retryable ?? errorRecord?.retryable);
if (error) event.error = error;
if (stderr) event.stderr = stderr;
if (exitCode !== null) event.exit_code = exitCode;
if (retryable !== null) event.retryable = retryable;
}
return [event];
}
Expand Down Expand Up @@ -278,15 +283,19 @@ export class AppServerEventConverter {
const statusRaw = asString(paramsRecord.status ?? turn.status);
const status = statusRaw?.toLowerCase();
const turnId = asString(turn.turnId ?? turn.turn_id ?? turn.id);
const errorMessage = asString(paramsRecord.error ?? paramsRecord.message ?? paramsRecord.reason);
const errorRecord = asRecord(paramsRecord.error);
const errorMessage = asString(paramsRecord.error ?? paramsRecord.message ?? paramsRecord.reason ?? errorRecord?.message);
const stderr = asString(paramsRecord.stderr ?? errorRecord?.stderr);
const exitCode = asNumber(paramsRecord.exit_code ?? paramsRecord.exitCode ?? errorRecord?.exit_code ?? errorRecord?.exitCode);
const retryable = asBoolean(paramsRecord.retryable ?? errorRecord?.retryable);

if (status === 'interrupted' || status === 'cancelled' || status === 'canceled') {
events.push({ type: 'turn_aborted', ...(turnId ? { turn_id: turnId } : {}) });
return events;
}

if (status === 'failed' || status === 'error') {
events.push({ type: 'task_failed', ...(turnId ? { turn_id: turnId } : {}), ...(errorMessage ? { error: errorMessage } : {}) });
events.push({ type: 'task_failed', ...(turnId ? { turn_id: turnId } : {}), ...(errorMessage ? { error: errorMessage } : {}), ...(stderr ? { stderr } : {}), ...(exitCode !== null ? { exit_code: exitCode } : {}), ...(retryable !== null ? { retryable } : {}) });
return events;
}

Expand All @@ -311,9 +320,12 @@ export class AppServerEventConverter {
if (method === 'error') {
const willRetry = asBoolean(paramsRecord.will_retry ?? paramsRecord.willRetry) ?? false;
if (willRetry) return events;
const message = asString(paramsRecord.message) ?? asString(asRecord(paramsRecord.error)?.message);
if (message) {
events.push({ type: 'task_failed', error: message });
const errorRecord = asRecord(paramsRecord.error);
const message = asString(paramsRecord.message) ?? asString(errorRecord?.message);
const stderr = asString(paramsRecord.stderr ?? errorRecord?.stderr);
const exitCode = asNumber(paramsRecord.exit_code ?? paramsRecord.exitCode ?? errorRecord?.exit_code ?? errorRecord?.exitCode);
if (message || stderr) {
events.push({ type: 'task_failed', ...(message ? { error: message } : {}), ...(stderr ? { stderr } : {}), ...(exitCode !== null ? { exit_code: exitCode } : {}) });
}
return events;
}
Expand Down
22 changes: 22 additions & 0 deletions cli/src/codex/utils/terminalEventGuard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,28 @@ describe('shouldIgnoreTerminalEvent', () => {
expect(ignored).toBe(true);
});

it('accepts anonymous failed terminal event with details even when current turn id exists', () => {
const ignored = shouldIgnoreTerminalEvent({
eventTurnId: null,
currentTurnId: 'turn-1',
turnInFlight: true,
acceptAnonymousFailureWithDetails: true
});

expect(ignored).toBe(false);
});

it('accepts anonymous failed terminal event with details while turn is still in flight', () => {
const ignored = shouldIgnoreTerminalEvent({
eventTurnId: null,
currentTurnId: null,
turnInFlight: true,
acceptAnonymousFailureWithDetails: true
});

expect(ignored).toBe(false);
});

it('ignores stale terminal events from another turn', () => {
const ignored = shouldIgnoreTerminalEvent({
eventTurnId: 'turn-old',
Expand Down
8 changes: 8 additions & 0 deletions cli/src/codex/utils/terminalEventGuard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,28 @@ export type TerminalEventGuardInput = {
currentTurnId: string | null;
turnInFlight: boolean;
allowAnonymousTerminalEvent?: boolean;
acceptAnonymousFailureWithDetails?: boolean;
};

export function shouldIgnoreTerminalEvent(input: TerminalEventGuardInput): boolean {
const allowAnonymousTerminalEvent = input.allowAnonymousTerminalEvent === true;
const acceptAnonymousFailureWithDetails = input.acceptAnonymousFailureWithDetails === true;

if (input.eventTurnId) {
return Boolean(input.currentTurnId && input.eventTurnId !== input.currentTurnId);
}

if (input.currentTurnId) {
if (acceptAnonymousFailureWithDetails) {
return false;
}
return true;
}

if (input.turnInFlight && !allowAnonymousTerminalEvent) {
if (acceptAnonymousFailureWithDetails) {
return false;
}
return true;
}

Expand Down
Loading
Loading