Skip to content

Commit 5a9a6f9

Browse files
committed
Track and handle codex process exit codes to pass better error message to a user
1 parent 924ee93 commit 5a9a6f9

4 files changed

Lines changed: 39 additions & 15 deletions

File tree

src/CodexAcpServer.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,27 @@ export class CodexAcpServer implements acp.Agent {
1717
private readonly codexAcpClient: CodexAcpClient;
1818
private readonly connection: acp.AgentSideConnection;
1919
private readonly defaultAuthRequest: CodexAuthRequest | null;
20+
private readonly getExitCode: () => number | null;
2021

2122
private readonly sessions: Map<string, SessionState>;
2223

2324
constructor(
2425
connection: acp.AgentSideConnection,
2526
codexAcpClient: CodexAcpClient,
2627
defaultAuthRequest?: CodexAuthRequest,
28+
getExitCode?: () => number | null,
2729
) {
2830
this.sessions = new Map();
2931
this.connection = connection;
30-
this.defaultAuthRequest = defaultAuthRequest ?? null;
3132
this.codexAcpClient = codexAcpClient;
33+
this.defaultAuthRequest = defaultAuthRequest ?? null;
34+
this.getExitCode = getExitCode ?? (() => null);
3235
}
3336

3437
async initialize(
3538
_params: acp.InitializeRequest,
3639
): Promise<acp.InitializeResponse> {
37-
await this.codexAcpClient.initialize(_params);
40+
await this.runWithProcessCheck(() => this.codexAcpClient.initialize(_params));
3841
return {
3942
protocolVersion: acp.PROTOCOL_VERSION,
4043
agentCapabilities: {
@@ -47,15 +50,15 @@ export class CodexAcpServer implements acp.Agent {
4750
async newSession(
4851
_params: acp.NewSessionRequest,
4952
): Promise<acp.NewSessionResponse> {
50-
if (await this.codexAcpClient.authRequired()) {
53+
if (await this.runWithProcessCheck(() => this.codexAcpClient.authRequired())) {
5154
if (this.defaultAuthRequest) {
5255
await this.authenticate(this.defaultAuthRequest)
5356
} else {
5457
throw RequestError.authRequired();
5558
}
5659
}
5760

58-
const sessionMetadata = await this.codexAcpClient.newSession(_params);
61+
const sessionMetadata = await this.runWithProcessCheck(() => this.codexAcpClient.newSession(_params));
5962
const {sessionId, currentModelId, models} = sessionMetadata;
6063
this.sessions.set(sessionId, {
6164
sessionMetadata: sessionMetadata,
@@ -76,7 +79,7 @@ export class CodexAcpServer implements acp.Agent {
7679
async authenticate(
7780
_params: acp.AuthenticateRequest,
7881
): Promise<acp.AuthenticateResponse> {
79-
const isAuthenticated = await this.codexAcpClient.authenticate(_params);
82+
const isAuthenticated = await this.runWithProcessCheck(() => this.codexAcpClient.authenticate(_params));
8083
if (!isAuthenticated) {
8184
throw RequestError.invalidParams();
8285
}
@@ -118,10 +121,10 @@ export class CodexAcpServer implements acp.Agent {
118121
}
119122

120123

121-
await this.codexAcpClient.setModel({
124+
await this.runWithProcessCheck(() => this.codexAcpClient.setModel({
122125
model: model.model,
123126
reasoningEffort,
124-
});
127+
}));
125128
sessionState.sessionMetadata.currentModelId = ModelId.fromComponents(model, reasoningEffort).toString();
126129

127130
return {};
@@ -155,7 +158,7 @@ export class CodexAcpServer implements acp.Agent {
155158

156159
try {
157160
const messageHandler = new CodexEventHandler(this.connection, sessionState);
158-
await this.codexAcpClient.sendPrompt(params, (event) => messageHandler.handleNotification(event));
161+
await this.runWithProcessCheck(() => this.codexAcpClient.sendPrompt(params, (event) => messageHandler.handleNotification(event)));
159162
} catch (err) {
160163
if (sessionState.pendingPrompt.signal.aborted) {
161164
return {stopReason: "cancelled"};
@@ -171,6 +174,22 @@ export class CodexAcpServer implements acp.Agent {
171174
};
172175
}
173176

177+
private async runWithProcessCheck<T>(operation: () => Promise<T>): Promise<T> {
178+
try {
179+
return await operation();
180+
} catch (err) {
181+
const exitCode = this.getExitCode();
182+
const requestErrorCode = 1001 // Just some magic number
183+
if (exitCode == 3221225781) {
184+
throw new RequestError(requestErrorCode, `VC++ redistributable should be installed`);
185+
}
186+
if (exitCode !== null) {
187+
throw new RequestError(requestErrorCode, `Codex process has exited with code ${exitCode}`);
188+
}
189+
throw err;
190+
}
191+
}
192+
174193
async cancel(params: acp.CancelNotification): Promise<void> {
175194
//TODO not supported yet
176195
this.sessions.get(params.sessionId)?.pendingPrompt?.abort();

src/CodexJsonRpcConnection.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import fs from "node:fs";
77
import path from "node:path";
88
import {createJSONRPCReader, createJSONRPCWriter} from "./StdUtils";
99

10-
export function startCodexConnection(codexPath: string, logPath?: string): MessageConnection {
10+
export interface CodexConnection {
11+
readonly connection: MessageConnection
12+
readonly process: ChildProcessWithoutNullStreams;
13+
}
14+
15+
export function startCodexConnection(codexPath: string, logPath?: string): CodexConnection {
1116
const codex: ChildProcessWithoutNullStreams = spawn(`"${codexPath}" app-server`, {
1217
shell: process.platform === 'win32'
1318
});
@@ -28,7 +33,7 @@ export function startCodexConnection(codexPath: string, logPath?: string): Messa
2833
connection.dispose();
2934
});
3035

31-
return connection;
36+
return {connection: connection, process: codex};
3237
}
3338

3439
function attachLogs(proc: ChildProcessWithoutNullStreams, logPath: string) {

src/__tests__/acp-test-utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,10 @@ export function createTestFixture(): TestFixture {
4141
acpEventHandlers.forEach(handler => handler(event));
4242
});
4343
const codexConnection = startCodexConnection(pathToCodex);
44-
const codexAppServerClient = new CodexAppServerClient(codexConnection);
44+
const codexAppServerClient = new CodexAppServerClient(codexConnection.connection);
4545

4646
const codexAcpClient = new CodexAcpClient(codexAppServerClient);
47-
const codexAcpAgent = new CodexAcpServer(acpConnection, codexAcpClient);
47+
const codexAcpAgent = new CodexAcpServer(acpConnection, codexAcpClient, undefined, () => codexConnection.process.exitCode);
4848

4949
const transportEvents: CodexConnectionEvent[] = []
5050
const codexEventHandlers: ((event: CodexConnectionEvent) => void)[] = [];

src/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ if (process.argv.includes("--version")) {
1717
const codexPath = process.env["CODEX_PATH"] ?? "codex";
1818
const logPath = process.env["APP_SERVER_LOGS"];
1919

20-
const appServerConnection = startCodexConnection(codexPath, logPath);
20+
const codexConnection = startCodexConnection(codexPath, logPath);
2121
const acpJsonStream = createJsonStream(process.stdin, process.stdout);
2222

2323
function createAgent(connection: acp.AgentSideConnection): CodexAcpServer {
@@ -27,9 +27,9 @@ function createAgent(connection: acp.AgentSideConnection): CodexAcpServer {
2727
const authRequestString = process.env["DEFAULT_AUTH_REQUEST"];
2828
const parsedRequest = authRequestString ? JSON.parse(authRequestString) : undefined;
2929
const defaultAuthRequest = parsedRequest && isCodexAuthRequest(parsedRequest) ? parsedRequest : undefined;
30-
const appServerClient = new CodexAppServerClient(appServerConnection);
30+
const appServerClient = new CodexAppServerClient(codexConnection.connection);
3131
const codexClient = new CodexAcpClient(appServerClient, config, modelProvider)
32-
return new CodexAcpServer(connection, codexClient, defaultAuthRequest);
32+
return new CodexAcpServer(connection, codexClient, defaultAuthRequest, () => codexConnection.process.exitCode);
3333
}
3434

3535
new acp.AgentSideConnection(createAgent, acpJsonStream);

0 commit comments

Comments
 (0)