Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion src/CodexAcpServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export class CodexAcpServer implements acp.Agent {
private readonly connection: acp.AgentSideConnection;
private readonly defaultAuthRequest: CodexAuthRequest | null;
private readonly getExitCode: () => number | null;
private readonly getRecentStderr: () => string;
private readonly availableCommands: CodexCommands;
private clientInfo: acp.Implementation | null;
private terminalOutputMode: TerminalOutputMode;
Expand All @@ -130,6 +131,7 @@ export class CodexAcpServer implements acp.Agent {
codexAcpClient: CodexAcpClient,
defaultAuthRequest?: CodexAuthRequest,
getExitCode?: () => number | null,
getRecentStderr?: () => string,
) {
this.sessions = new Map();
this.pendingMcpStartupSessions = new Map();
Expand All @@ -142,6 +144,7 @@ export class CodexAcpServer implements acp.Agent {
this.codexAcpClient = codexAcpClient;
this.defaultAuthRequest = defaultAuthRequest ?? null;
this.getExitCode = getExitCode ?? (() => null);
this.getRecentStderr = getRecentStderr ?? (() => "");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lastError instead of recent maybe?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's not actually last, it's all stderr limited to some size.
I'm not sure we can actually get single error - it still can be spitted into a few data events.

this.clientInfo = null;
this.terminalOutputMode = "terminal_output_delta";
this.availableCommands = new CodexCommands(
Expand Down Expand Up @@ -1458,7 +1461,9 @@ export class CodexAcpServer implements acp.Agent {
throw new RequestError(requestErrorCode, `VC++ redistributable should be installed`);
}
if (exitCode !== null) {
throw new RequestError(requestErrorCode, `Codex process has exited with code ${exitCode}`);
const stderr = this.getRecentStderr().trim();
const detail = stderr ? `:\n${stderr}` : "";
throw new RequestError(requestErrorCode, `Codex process has exited with code ${exitCode}${detail}`);
}
throw err;
}
Expand Down
37 changes: 37 additions & 0 deletions src/__tests__/CodexACPAgent/process-exit-error.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { describe, it, expect } from 'vitest';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { once } from 'node:events';
import * as acp from '@agentclientprotocol/sdk';
import { startCodexConnection } from '../../CodexJsonRpcConnection';
import { CodexAppServerClient } from '../../CodexAppServerClient';
import { CodexAcpClient } from '../../CodexAcpClient';
import { CodexAcpServer } from '../../CodexAcpServer';
import { createMockConnections } from './test-utils';

describe('CodexACPAgent - process exit error', () => {
it.skipIf(process.platform === 'win32')('includes the crashed process stderr', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe add win case also? :)

@AlexandrSuhinin AlexandrSuhinin Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We don't have CI on win, also the change is not OS specific. I suggest to keep it as is, otherwise I need to blindly add win case support without actual checking or spend unreasonable time to check it.

const fakeCodex = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'codex-bad-')), 'codex');
fs.writeFileSync(fakeCodex, "#!/bin/sh\necho 'codex: failed to launch' >&2\nexit 1\n");
fs.chmodSync(fakeCodex, 0o755);

const connection = startCodexConnection(fakeCodex);
let stderr = '';
connection.process.stderr.on('data', (chunk: Buffer) => { stderr += chunk.toString(); });

const codexClient = new CodexAcpClient(new CodexAppServerClient(connection.connection));
const agent = new CodexAcpServer(
createMockConnections().mockAcpConnection,
codexClient,
undefined,
() => connection.process.exitCode,
() => stderr,
);

await once(connection.process, 'close'); // process exited and stderr flushed

await expect(agent.initialize({ protocolVersion: acp.PROTOCOL_VERSION }))
.rejects.toThrow("Codex process has exited with code 1:\ncodex: failed to launch");
});
});
9 changes: 8 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ function startAcpServer() {
});

const codexConnection = startCodexConnection(codexPath);

const maxStderrTailChars = 2 * 1024;
let stderr = "";
codexConnection.process.stderr.addListener("data", (data: Buffer) => {
stderr = (stderr + data.toString()).slice(-maxStderrTailChars);
});

process.stdin.on("close", (chunk: Buffer) => {
codexConnection.process.stdin.end();
// Kill the codex process if it doesn't exit naturally
Expand All @@ -73,7 +80,7 @@ function startAcpServer() {
function createAgent(connection: acp.AgentSideConnection): CodexAcpServer {
const appServerClient = new CodexAppServerClient(codexConnection.connection);
const codexClient = new CodexAcpClient(appServerClient, config, modelProvider);
return new CodexAcpServer(connection, codexClient, defaultAuthRequest, () => codexConnection.process.exitCode);
return new CodexAcpServer(connection, codexClient, defaultAuthRequest, () => codexConnection.process.exitCode, () => stderr);
}

new acp.AgentSideConnection(createAgent, acpJsonStream);
Expand Down
Loading