Fix MCP tool call events for Codex MCP
This commit is contained in:
@@ -0,0 +1,23 @@
|
|||||||
|
# Codex MCP test hang investigation
|
||||||
|
|
||||||
|
## Test that appeared stuck
|
||||||
|
- `CodexMcpAgentClient (MCP integration) > maps thread/item events for file changes, MCP tools, web search, and todo lists`
|
||||||
|
- Wait loop: `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:577` (`for await (const event of session.stream(prompt))` until `turn_completed`/`turn_failed`)
|
||||||
|
|
||||||
|
## What was actually happening
|
||||||
|
- The test was slow (40s+) because the Codex MCP server performed multiple tool call cycles before completing the turn.
|
||||||
|
- The run ultimately failed (not a true deadlock) because the MCP tool call timeline item never appeared, so `rawItemTypes.has("mcp_tool_call")` stayed false.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
- Codex MCP server emits `mcp_tool_call_begin` and `mcp_tool_call_end` events with tool invocation + result payloads.
|
||||||
|
- `codex-mcp-agent.ts` handled raw_response_item tool calls but ignored `mcp_tool_call_*` events, so MCP tool calls were never mapped to timeline items when those events were the only reliable signal.
|
||||||
|
|
||||||
|
## Evidence
|
||||||
|
- Observed event shapes from Codex MCP:
|
||||||
|
- `mcp_tool_call_begin` with `call_id` and `invocation` (server/tool/arguments)
|
||||||
|
- `mcp_tool_call_end` with `result.Ok.structuredContent`
|
||||||
|
- When mapping those events to tool_call timeline items, the test passed.
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
- Add schemas + handlers for `mcp_tool_call_begin`/`mcp_tool_call_end` in `packages/server/src/server/agent/providers/codex-mcp-agent.ts`.
|
||||||
|
- Emit `tool_call` timeline items for running/completed MCP tool calls and surface structured tool output.
|
||||||
@@ -735,6 +735,46 @@ const PatchApplyEndEventSchema = z
|
|||||||
stderr: data.stderr,
|
stderr: data.stderr,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
const McpToolCallInvocationSchema = z
|
||||||
|
.object({
|
||||||
|
server: z.string(),
|
||||||
|
tool: z.string(),
|
||||||
|
arguments: z.unknown().optional(),
|
||||||
|
})
|
||||||
|
.passthrough();
|
||||||
|
|
||||||
|
const McpToolCallBeginEventSchema = z
|
||||||
|
.object({
|
||||||
|
type: z.literal("mcp_tool_call_begin"),
|
||||||
|
call_id: CallIdSchema,
|
||||||
|
invocation: McpToolCallInvocationSchema,
|
||||||
|
})
|
||||||
|
.passthrough()
|
||||||
|
.transform((data) => ({
|
||||||
|
type: data.type,
|
||||||
|
callId: data.call_id,
|
||||||
|
server: data.invocation.server,
|
||||||
|
tool: data.invocation.tool,
|
||||||
|
input: data.invocation.arguments,
|
||||||
|
}));
|
||||||
|
|
||||||
|
const McpToolCallEndEventSchema = z
|
||||||
|
.object({
|
||||||
|
type: z.literal("mcp_tool_call_end"),
|
||||||
|
call_id: CallIdSchema,
|
||||||
|
invocation: McpToolCallInvocationSchema,
|
||||||
|
result: z.unknown().optional(),
|
||||||
|
})
|
||||||
|
.passthrough()
|
||||||
|
.transform((data) => ({
|
||||||
|
type: data.type,
|
||||||
|
callId: data.call_id,
|
||||||
|
server: data.invocation.server,
|
||||||
|
tool: data.invocation.tool,
|
||||||
|
input: data.invocation.arguments,
|
||||||
|
result: data.result,
|
||||||
|
}));
|
||||||
|
|
||||||
const AgentMessageEventSchema = z
|
const AgentMessageEventSchema = z
|
||||||
.object({
|
.object({
|
||||||
type: z.literal("agent_message"),
|
type: z.literal("agent_message"),
|
||||||
@@ -1667,6 +1707,8 @@ const CodexEventSchema = z.union([
|
|||||||
ExecCommandEndEventSchema,
|
ExecCommandEndEventSchema,
|
||||||
PatchApplyBeginEventSchema,
|
PatchApplyBeginEventSchema,
|
||||||
PatchApplyEndEventSchema,
|
PatchApplyEndEventSchema,
|
||||||
|
McpToolCallBeginEventSchema,
|
||||||
|
McpToolCallEndEventSchema,
|
||||||
ThreadStartedEventSchema,
|
ThreadStartedEventSchema,
|
||||||
TurnStartedEventSchema,
|
TurnStartedEventSchema,
|
||||||
TurnCompletedEventSchema,
|
TurnCompletedEventSchema,
|
||||||
@@ -1876,6 +1918,10 @@ function normalizeStructuredPayload(value: unknown): unknown {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isKeyedObject(value: unknown): value is { [key: string]: unknown } {
|
||||||
|
return typeof value === "object" && value !== null;
|
||||||
|
}
|
||||||
|
|
||||||
function normalizeToolName(toolName: string): string {
|
function normalizeToolName(toolName: string): string {
|
||||||
if (!toolName.startsWith("mcp__")) {
|
if (!toolName.startsWith("mcp__")) {
|
||||||
return toolName;
|
return toolName;
|
||||||
@@ -1889,6 +1935,44 @@ function normalizeToolName(toolName: string): string {
|
|||||||
return `${serverName}.${toolParts.join("__")}`;
|
return `${serverName}.${toolParts.join("__")}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function extractMcpToolResultPayload(result: unknown): {
|
||||||
|
output: unknown;
|
||||||
|
success: boolean;
|
||||||
|
} {
|
||||||
|
let success = true;
|
||||||
|
let payload = result;
|
||||||
|
|
||||||
|
if (isKeyedObject(result)) {
|
||||||
|
if ("Ok" in result) {
|
||||||
|
payload = result.Ok;
|
||||||
|
success = true;
|
||||||
|
} else if ("ok" in result) {
|
||||||
|
payload = result.ok;
|
||||||
|
success = true;
|
||||||
|
} else if ("Err" in result) {
|
||||||
|
payload = result.Err;
|
||||||
|
success = false;
|
||||||
|
} else if ("err" in result) {
|
||||||
|
payload = result.err;
|
||||||
|
success = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (isKeyedObject(payload)) {
|
||||||
|
if ("structuredContent" in payload && payload.structuredContent !== undefined) {
|
||||||
|
return { output: payload.structuredContent, success };
|
||||||
|
}
|
||||||
|
if ("structured_content" in payload && payload.structured_content !== undefined) {
|
||||||
|
return { output: payload.structured_content, success };
|
||||||
|
}
|
||||||
|
if ("content" in payload && payload.content !== undefined) {
|
||||||
|
return { output: payload.content, success };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return { output: payload, success };
|
||||||
|
}
|
||||||
|
|
||||||
function extractWebSearchQuery(
|
function extractWebSearchQuery(
|
||||||
input: unknown,
|
input: unknown,
|
||||||
output: unknown
|
output: unknown
|
||||||
@@ -3190,6 +3274,46 @@ class CodexMcpAgentSession implements AgentSession {
|
|||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
case "mcp_tool_call_begin": {
|
||||||
|
const input = normalizeStructuredPayload(parsedEvent.input);
|
||||||
|
this.emitEvent({
|
||||||
|
type: "timeline",
|
||||||
|
provider: CODEX_PROVIDER,
|
||||||
|
item: createToolCallTimelineItem({
|
||||||
|
server: parsedEvent.server,
|
||||||
|
tool: parsedEvent.tool,
|
||||||
|
status: "running",
|
||||||
|
callId: parsedEvent.callId,
|
||||||
|
displayName: `${parsedEvent.server}.${parsedEvent.tool}`,
|
||||||
|
kind: "tool",
|
||||||
|
input,
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
case "mcp_tool_call_end": {
|
||||||
|
const input = normalizeStructuredPayload(parsedEvent.input);
|
||||||
|
const { output, success } = extractMcpToolResultPayload(parsedEvent.result);
|
||||||
|
const normalizedOutput = normalizeStructuredPayload(output);
|
||||||
|
if (!success) {
|
||||||
|
this.turnState && (this.turnState.sawError = true);
|
||||||
|
}
|
||||||
|
this.emitEvent({
|
||||||
|
type: "timeline",
|
||||||
|
provider: CODEX_PROVIDER,
|
||||||
|
item: createToolCallTimelineItem({
|
||||||
|
server: parsedEvent.server,
|
||||||
|
tool: parsedEvent.tool,
|
||||||
|
status: success ? "completed" : "failed",
|
||||||
|
callId: parsedEvent.callId,
|
||||||
|
displayName: `${parsedEvent.server}.${parsedEvent.tool}`,
|
||||||
|
kind: "tool",
|
||||||
|
input,
|
||||||
|
output: normalizedOutput,
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -624,6 +624,28 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
|
|||||||
|
|
||||||
- [⏳] **Fix**: Codex MCP thread/item mapping failure in `codex-mcp-agent.test.ts` (file change, MCP tool, web search, todo list assertions).
|
- [⏳] **Fix**: Codex MCP thread/item mapping failure in `codex-mcp-agent.test.ts` (file change, MCP tool, web search, todo list assertions).
|
||||||
|
|
||||||
|
**BREAK IT DOWN - test each event type individually:**
|
||||||
|
|
||||||
|
1. Run a minimal test for JUST `file_change`:
|
||||||
|
```bash
|
||||||
|
npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts -t "file_change"
|
||||||
|
```
|
||||||
|
Does Codex emit file_change events? Log raw events to see.
|
||||||
|
|
||||||
|
2. Run a minimal test for JUST `mcp_tool_call`:
|
||||||
|
- Does Codex emit `function_call` or `mcp_tool_call`?
|
||||||
|
- What's the EXACT shape of the event?
|
||||||
|
- Log: `console.log("RAW EVENT:", JSON.stringify(event))`
|
||||||
|
|
||||||
|
3. Run a minimal test for JUST `web_search`:
|
||||||
|
- What event type does Codex actually send?
|
||||||
|
|
||||||
|
4. For each: capture the EXACT raw event JSON, then fix the parser.
|
||||||
|
|
||||||
|
**Don't try to fix everything at once. Fix ONE event type, verify it works, then move to the next.**
|
||||||
|
|
||||||
|
**IMMEDIATE ACTION:** Add `console.log("RAW MCP EVENT:", JSON.stringify(msg))` in `handleMcpEvent` and run the test. Post the output.
|
||||||
|
|
||||||
- [x] **Fix**: Typecheck error in `agent-projections.ts:198` - {} not assignable to JsonValue.
|
- [x] **Fix**: Typecheck error in `agent-projections.ts:198` - {} not assignable to JsonValue.
|
||||||
|
|
||||||
- Error: TS2322 at `src/server/agent/agent-projections.ts:198:3`
|
- Error: TS2322 at `src/server/agent/agent-projections.ts:198:3`
|
||||||
@@ -648,7 +670,7 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
|
|||||||
- Run `npm run typecheck --workspace=@paseo/server` → zero errors
|
- Run `npm run typecheck --workspace=@paseo/server` → zero errors
|
||||||
- **Done (2025-12-25 12:38)**: WHAT: tightened thread item schema transforms with literal `as const` types and added thread item type guards/read_file narrowing in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:872`, `packages/server/src/server/agent/providers/codex-mcp-agent.ts:1453`, `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3247`, `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3366`. RESULT: TS2339 discriminated-union errors resolved and server typecheck completes with no errors. EVIDENCE: `npm run typecheck --workspace=@paseo/server`.
|
- **Done (2025-12-25 12:38)**: WHAT: tightened thread item schema transforms with literal `as const` types and added thread item type guards/read_file narrowing in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:872`, `packages/server/src/server/agent/providers/codex-mcp-agent.ts:1453`, `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3247`, `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3366`. RESULT: TS2339 discriminated-union errors resolved and server typecheck completes with no errors. EVIDENCE: `npm run typecheck --workspace=@paseo/server`.
|
||||||
|
|
||||||
- [ ] **Fix**: Test hang in `codex-mcp-agent.test.ts` - stuck at 3/13 tests.
|
- [x] **Fix**: Test hang in `codex-mcp-agent.test.ts` - stuck at 3/13 tests.
|
||||||
|
|
||||||
**DO NOT just report "it stalled" - INVESTIGATE:**
|
**DO NOT just report "it stalled" - INVESTIGATE:**
|
||||||
|
|
||||||
@@ -685,6 +707,7 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
|
|||||||
- "It stalled" is NOT an acceptable answer
|
- "It stalled" is NOT an acceptable answer
|
||||||
- "Codex MCP server issue" is NOT an acceptable answer
|
- "Codex MCP server issue" is NOT an acceptable answer
|
||||||
- This is OUR bug. Find it. Fix it.
|
- This is OUR bug. Find it. Fix it.
|
||||||
|
- **Done (2025-12-25 13:03)**: WHAT: added MCP tool call begin/end event schemas + handlers and MCP result parsing in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:746`, `packages/server/src/server/agent/providers/codex-mcp-agent.ts:1938`, `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3277` to emit `tool_call` timeline items for MCP tools; documented investigation in `REPORT-codex-mcp-test-hang.md:1`. RESULT: the previously “stuck at 3/13” test now completes and passes with MCP tool calls recorded. EVIDENCE: `npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts -t "maps thread/item events for file changes, MCP tools, web search, and todo lists"` (1 passed, 12 skipped).
|
||||||
|
|
||||||
- [ ] **Verify**: Rerun typecheck and full test suite after fixes.
|
- [ ] **Verify**: Rerun typecheck and full test suite after fixes.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user