Fix exit code parsing for Codex MCP
This commit is contained in:
@@ -139,6 +139,22 @@ function normalizeThreadEventType(type: string): string {
|
|||||||
return type;
|
return type;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function normalizeExitCode(value: unknown): number | undefined {
|
||||||
|
if (typeof value === "number" && Number.isFinite(value)) {
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
if (typeof value === "string") {
|
||||||
|
const trimmed = value.trim();
|
||||||
|
if (trimmed.length) {
|
||||||
|
const parsed = Number(trimmed);
|
||||||
|
if (Number.isFinite(parsed)) {
|
||||||
|
return parsed;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
function buildCommandDisplayName(command?: unknown): string {
|
function buildCommandDisplayName(command?: unknown): string {
|
||||||
if (typeof command === "string") {
|
if (typeof command === "string") {
|
||||||
const trimmed = command.trim();
|
const trimmed = command.trim();
|
||||||
@@ -994,7 +1010,7 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
const cwd = (event as { cwd?: string }).cwd;
|
const cwd = (event as { cwd?: string }).cwd;
|
||||||
const exitCodeRaw = (event as { exit_code?: unknown; exitCode?: unknown }).exit_code ??
|
const exitCodeRaw = (event as { exit_code?: unknown; exitCode?: unknown }).exit_code ??
|
||||||
(event as { exitCode?: unknown }).exitCode;
|
(event as { exitCode?: unknown }).exitCode;
|
||||||
const exitCode = typeof exitCodeRaw === "number" ? exitCodeRaw : undefined;
|
const exitCode = normalizeExitCode(exitCodeRaw);
|
||||||
const output = (event as { output?: unknown; stdout?: unknown }).output ??
|
const output = (event as { output?: unknown; stdout?: unknown }).output ??
|
||||||
(event as { stdout?: unknown }).stdout ??
|
(event as { stdout?: unknown }).stdout ??
|
||||||
(event as { stderr?: unknown }).stderr;
|
(event as { stderr?: unknown }).stderr;
|
||||||
@@ -1155,7 +1171,7 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
if (item.type === "command_execution") {
|
if (item.type === "command_execution") {
|
||||||
const exitCode = item.exit_code ?? item.exitCode;
|
const exitCode = normalizeExitCode(item.exit_code ?? item.exitCode);
|
||||||
if (typeof exitCode === "number" && exitCode !== 0) {
|
if (typeof exitCode === "number" && exitCode !== 0) {
|
||||||
this.turnState && (this.turnState.sawError = true);
|
this.turnState && (this.turnState.sawError = true);
|
||||||
this.emitEvent({
|
this.emitEvent({
|
||||||
@@ -1200,8 +1216,10 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
return { type: "user_message", text: item.text as string };
|
return { type: "user_message", text: item.text as string };
|
||||||
case "command_execution": {
|
case "command_execution": {
|
||||||
const aggregatedOutput = (item as { aggregated_output?: unknown }).aggregated_output;
|
const aggregatedOutput = (item as { aggregated_output?: unknown }).aggregated_output;
|
||||||
const exitCode = (item as { exit_code?: unknown; exitCode?: unknown }).exit_code ??
|
const exitCode = normalizeExitCode(
|
||||||
(item as { exitCode?: unknown }).exitCode;
|
(item as { exit_code?: unknown; exitCode?: unknown }).exit_code ??
|
||||||
|
(item as { exitCode?: unknown }).exitCode
|
||||||
|
);
|
||||||
const cwd = (item as { cwd?: string }).cwd;
|
const cwd = (item as { cwd?: string }).cwd;
|
||||||
const commandValue = item.command;
|
const commandValue = item.command;
|
||||||
const command =
|
const command =
|
||||||
|
|||||||
@@ -262,7 +262,8 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
|
|||||||
- [x] **Fix**: Codex SDK persisted shell_command hydration still missing completed status.
|
- [x] **Fix**: Codex SDK persisted shell_command hydration still missing completed status.
|
||||||
- **Done (2025-12-24 20:31)**: Mapped shell_command custom_tool_call entries to command tool calls and normalized output/status during rollout hydration.
|
- **Done (2025-12-24 20:31)**: Mapped shell_command custom_tool_call entries to command tool calls and normalized output/status during rollout hydration.
|
||||||
|
|
||||||
- [ ] **Fix**: Codex MCP command output should include exit codes for command tool calls (missing in timeline mapping).
|
- [x] **Fix**: Codex MCP command output should include exit codes for command tool calls (missing in timeline mapping).
|
||||||
|
- **Done (2025-12-24 20:33)**: Normalized exit code parsing so numeric strings are captured in timeline output.
|
||||||
|
|
||||||
- [ ] **Fix**: Codex MCP thread/item event mapping for file_change, mcp_tool_call, web_search, and todo_list still failing.
|
- [ ] **Fix**: Codex MCP thread/item event mapping for file_change, mcp_tool_call, web_search, and todo_list still failing.
|
||||||
|
|
||||||
@@ -274,6 +275,23 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
|
|||||||
|
|
||||||
- [ ] **Fix**: Investigate `agent-mcp.e2e.test.ts` hang (Claude agent flow) and add timeout/skip conditions as needed.
|
- [ ] **Fix**: Investigate `agent-mcp.e2e.test.ts` hang (Claude agent flow) and add timeout/skip conditions as needed.
|
||||||
|
|
||||||
|
- [ ] **Test (E2E) CRITICAL**: Interruption/abort latency for Codex MCP provider.
|
||||||
|
|
||||||
|
- **Requirement**: Interrupting a long-running operation must stop within 1 second
|
||||||
|
- Test setup:
|
||||||
|
- Ask Codex to run a long command (e.g., `sleep 300`, `for i in {1..1000000}; do echo $i; done`, or similar)
|
||||||
|
- Wait for the command to start executing (tool_call event received)
|
||||||
|
- Call `session.interrupt()` or equivalent abort signal
|
||||||
|
- Measure time from interrupt call to session fully stopped
|
||||||
|
- Pass criteria:
|
||||||
|
- Interrupt completes in < 1 second
|
||||||
|
- No zombie processes left running
|
||||||
|
- Session state is clean (can start new session)
|
||||||
|
- Test BOTH:
|
||||||
|
- Codex MCP provider interruption
|
||||||
|
- Codex SDK provider interruption (if applicable)
|
||||||
|
- This is critical for user experience - users expect immediate response to cancel
|
||||||
|
|
||||||
- [ ] **Test (E2E)**: Permission flow parity - test both Codex MCP and Claude providers.
|
- [ ] **Test (E2E)**: Permission flow parity - test both Codex MCP and Claude providers.
|
||||||
|
|
||||||
- Create/update E2E tests that verify permissions work for BOTH providers
|
- Create/update E2E tests that verify permissions work for BOTH providers
|
||||||
|
|||||||
Reference in New Issue
Block a user