fix(agent-codex): fix token double-counting and spawn error handling
- Remove additive cached_tokens/reasoning_tokens from cost calculation (they are subsets of input_tokens/output_tokens per OpenAI API convention) - Wrap entire connect() body in try/catch so connecting flag resets if spawn() throws synchronously (EMFILE, ENOMEM) - Add test for spawn-throws-synchronously recovery - Update cost assertions to reflect correct non-additive token counting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
c0105476da
commit
2dfbf8a778
|
|
@ -983,6 +983,27 @@ describe("CodexAppServerClient", () => {
|
|||
expect(client.isConnected).toBe(true);
|
||||
await closeClient(client, proc);
|
||||
});
|
||||
|
||||
it("resets connecting flag if spawn throws synchronously", async () => {
|
||||
// Make spawn throw (e.g. EMFILE, ENOMEM)
|
||||
mockSpawn.mockImplementationOnce(() => {
|
||||
throw new Error("spawn EMFILE");
|
||||
});
|
||||
|
||||
const client = new CodexAppServerClient({ requestTimeout: 500 });
|
||||
await expect(client.connect()).rejects.toThrow("spawn EMFILE");
|
||||
|
||||
// connecting flag should be reset — a retry should not throw "already connecting"
|
||||
const proc = createFakeProcess();
|
||||
const connectPromise = client.connect();
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
const initReq = findRequest(proc, "initialize");
|
||||
proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} }));
|
||||
await connectPromise;
|
||||
|
||||
expect(client.isConnected).toBe(true);
|
||||
await closeClient(client, proc);
|
||||
});
|
||||
});
|
||||
|
||||
describe("readline cleanup on process exit/error", () => {
|
||||
|
|
|
|||
|
|
@ -174,37 +174,34 @@ export class CodexAppServerClient extends EventEmitter {
|
|||
|
||||
this.connecting = true;
|
||||
|
||||
this.process = spawn(this.binaryPath, ["app-server"], {
|
||||
cwd: this.cwd,
|
||||
env: this.env ? { ...process.env, ...this.env } : undefined,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
|
||||
if (!this.process.stdout || !this.process.stdin) {
|
||||
this.connecting = false;
|
||||
throw new Error("Failed to open stdio pipes for codex app-server");
|
||||
}
|
||||
|
||||
// Drain stderr to prevent the child process from blocking when
|
||||
// the pipe buffer fills up.
|
||||
this.process.stderr?.resume();
|
||||
|
||||
// Set up line-based reading from stdout
|
||||
this.readline = createInterface({ input: this.process.stdout });
|
||||
this.readline.on("line", (line) => this.handleLine(line));
|
||||
|
||||
// Handle process exit
|
||||
this.process.once("exit", (code, signal) => {
|
||||
this.handleProcessExit(code, signal);
|
||||
});
|
||||
|
||||
this.process.once("error", (err) => {
|
||||
this.handleProcessError(err);
|
||||
});
|
||||
|
||||
// Perform initialization handshake. On failure, clean up the spawned
|
||||
// process so the caller doesn't need to call close() explicitly.
|
||||
try {
|
||||
this.process = spawn(this.binaryPath, ["app-server"], {
|
||||
cwd: this.cwd,
|
||||
env: this.env ? { ...process.env, ...this.env } : undefined,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
|
||||
if (!this.process.stdout || !this.process.stdin) {
|
||||
throw new Error("Failed to open stdio pipes for codex app-server");
|
||||
}
|
||||
|
||||
// Drain stderr to prevent the child process from blocking when
|
||||
// the pipe buffer fills up.
|
||||
this.process.stderr?.resume();
|
||||
|
||||
// Set up line-based reading from stdout
|
||||
this.readline = createInterface({ input: this.process.stdout });
|
||||
this.readline.on("line", (line) => this.handleLine(line));
|
||||
|
||||
// Handle process exit
|
||||
this.process.once("exit", (code, signal) => {
|
||||
this.handleProcessExit(code, signal);
|
||||
});
|
||||
|
||||
this.process.once("error", (err) => {
|
||||
this.handleProcessError(err);
|
||||
});
|
||||
|
||||
await this.initialize();
|
||||
} catch (err) {
|
||||
this.connecting = false;
|
||||
|
|
|
|||
|
|
@ -638,10 +638,11 @@ describe("getSessionInfo", () => {
|
|||
expect(result!.summary).toBe("Codex session (o3-mini)");
|
||||
expect(result!.summaryIsFallback).toBe(true);
|
||||
expect(result!.cost).toBeDefined();
|
||||
// input: 1000 + 200 (cached) + 2000 + 0 = 3200
|
||||
// output: 500 + 100 (reasoning) + 300 + 0 = 900
|
||||
expect(result!.cost!.inputTokens).toBe(3200);
|
||||
expect(result!.cost!.outputTokens).toBe(900);
|
||||
// cached_tokens/reasoning_tokens are subsets, not additive
|
||||
// input: 1000 + 2000 = 3000
|
||||
// output: 500 + 300 = 800
|
||||
expect(result!.cost!.inputTokens).toBe(3000);
|
||||
expect(result!.cost!.outputTokens).toBe(800);
|
||||
expect(result!.cost!.estimatedCostUsd).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -443,9 +443,9 @@ function extractCodexCost(lines: CodexJsonlLine[]): CostEstimate | undefined {
|
|||
for (const line of lines) {
|
||||
if (line.type === "event_msg" && line.msg?.type === "token_count") {
|
||||
inputTokens += line.msg.input_tokens ?? 0;
|
||||
inputTokens += line.msg.cached_tokens ?? 0;
|
||||
// cached_tokens is a subset of input_tokens (not additive)
|
||||
outputTokens += line.msg.output_tokens ?? 0;
|
||||
outputTokens += line.msg.reasoning_tokens ?? 0;
|
||||
// reasoning_tokens is a subset of output_tokens (not additive)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue