From 27b0a2da63f5a61038f172c7428a75b991a3ebbd Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 25 Dec 2025 15:59:43 +0700 Subject: [PATCH] Fix Codex MCP duplicate user/assistant message emissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: Codex MCP sends BOTH direct events (agent_message, agent_reasoning_delta) AND item events (item.started/updated/completed) for the same message content. - User messages were emitted once by us in stream() and again 3 times from Codex MCP's item.started/updated/completed events (4x total) - Agent messages were emitted from both the agent_message direct event AND the item.completed event (2x total) Fix: - Skip user_message items in threadItemToTimeline (we emit in stream()) - Only emit agent_message/reasoning on item.completed (skip started/updated) - Skip direct agent_message/agent_reasoning events (use item.completed path) Added test to verify exactly 1 user_message and 1 assistant_message per turn. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../agent/providers/codex-mcp-agent.test.ts | 68 +++++++++++++++++++ .../server/agent/providers/codex-mcp-agent.ts | 31 +++++---- plan.md | 37 ++++++++++ 3 files changed, 122 insertions(+), 14 deletions(-) diff --git a/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts b/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts index d8467094b..127da8a32 100644 --- a/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts @@ -401,6 +401,74 @@ function getConversationIdFromMetadata(metadata: unknown): string | undefined { } describe("CodexMcpAgentClient (MCP integration)", () => { + test( + "emits exactly one user_message and one assistant_message per turn", + async () => { + const cwd = tmpCwd(); + const restoreSessionDir = useTempCodexSessionDir(); + const { CodexMcpAgentClient } = await loadCodexMcpAgentClient(); + const client = new CodexMcpAgentClient(); + const config = { + provider: "codex", + cwd, + modeId: "full-access", + } satisfies AgentSessionConfig; + + let session: AgentSession | null = null; + const userMessages: AgentTimelineItem[] = []; + const assistantMessages: AgentTimelineItem[] = []; + const allEvents: AgentStreamEvent[] = []; + + try { + session = await client.createSession(config); + + // Simple prompt that should result in exactly one user message and one assistant message + const prompt = "Say hello"; + + for await (const event of session.stream(prompt)) { + allEvents.push(event); + if (event.type === "timeline" && providerFromEvent(event) === "codex") { + if (event.item.type === "user_message") { + userMessages.push(event.item); + } + if (event.item.type === "assistant_message") { + assistantMessages.push(event.item); + } + } + if (event.type === "turn_completed" || event.type === "turn_failed") { + break; + } + } + + // CRITICAL: There should be exactly ONE user_message event + expect(userMessages.length).toBe(1); + expect(userMessages[0].type).toBe("user_message"); + expect(userMessages[0].text).toBe(prompt); + + // CRITICAL: There should be exactly ONE assistant_message event (not duplicated) + expect(assistantMessages.length).toBe(1); + expect(assistantMessages[0].type).toBe("assistant_message"); + expect(typeof assistantMessages[0].text).toBe("string"); + + // The assistant message should NOT be duplicated/concatenated + const text = assistantMessages[0].text; + if (text.length > 20) { + // Check that the message doesn't repeat itself + const firstHalf = text.slice(0, Math.floor(text.length / 2)); + const secondHalf = text.slice(Math.floor(text.length / 2)); + // If duplicated, the message would be something like "Hello!Hello!" + // which means firstHalf === secondHalf + expect(firstHalf).not.toBe(secondHalf); + } + } finally { + await session?.close(); + rmSync(cwd, { recursive: true, force: true }); + restoreSessionDir(); + } + }, + 180_000 + ); + test( "responds with text", async () => { diff --git a/packages/server/src/server/agent/providers/codex-mcp-agent.ts b/packages/server/src/server/agent/providers/codex-mcp-agent.ts index 5e1a128b4..2520e8d00 100644 --- a/packages/server/src/server/agent/providers/codex-mcp-agent.ts +++ b/packages/server/src/server/agent/providers/codex-mcp-agent.ts @@ -3191,20 +3191,13 @@ class CodexMcpAgentSession implements AgentSession { case "error": this.handleThreadEvent(parsedEvent); return; + // NOTE: agent_message and agent_reasoning events are handled via item.completed + // events in handleThreadEvent. We skip them here to avoid duplicate emissions. + // The item.completed path provides the complete text after all deltas are received. case "agent_message": - this.emitEvent({ - type: "timeline", - provider: CODEX_PROVIDER, - item: { type: "assistant_message", text: parsedEvent.text }, - }); - return; case "agent_reasoning": case "agent_reasoning_delta": - this.emitEvent({ - type: "timeline", - provider: CODEX_PROVIDER, - item: { type: "reasoning", text: parsedEvent.text }, - }); + // Skip - handled via item.completed in handleThreadEvent return; case "task_started": this.emitEvent({ type: "turn_started", provider: CODEX_PROVIDER }); @@ -3618,15 +3611,25 @@ class CodexMcpAgentSession implements AgentSession { item: ThreadItem, eventType?: "item.started" | "item.updated" | "item.completed" ): AgentTimelineItem | null { + // IMPORTANT: user_message is emitted directly in stream() at turn start. + // Skip user_message items from Codex MCP events to avoid duplicates. + if (isThreadItemType(item, "user_message")) { + return null; + } + // For agent_message and reasoning, only emit on item.completed to avoid duplicates. + // Codex MCP sends item.started, item.updated, and item.completed for these. if (isThreadItemType(item, "agent_message")) { + if (eventType && eventType !== "item.completed") { + return null; + } return { type: "assistant_message", text: item.text }; } if (isThreadItemType(item, "reasoning")) { + if (eventType && eventType !== "item.completed") { + return null; + } return { type: "reasoning", text: item.text }; } - if (isThreadItemType(item, "user_message")) { - return { type: "user_message", text: item.text }; - } if (isThreadItemType(item, "command_execution")) { const command = normalizeCommand(item.command); let resolvedExitCode = item.exitCode; diff --git a/plan.md b/plan.md index 84796607c..44a091e32 100644 --- a/plan.md +++ b/plan.md @@ -862,3 +862,40 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov - Typecheck passes - Tests pass - **Done (2025-12-25 15:46)**: WHAT: Removed `"codex-mcp"` registration from `packages/server/src/server/bootstrap.ts:134`, removed from `AgentProvider` type in `packages/server/src/server/agent/agent-sdk-types.ts:3`, removed provider definition from `packages/server/src/server/agent/provider-manifest.ts:71-78`, removed conditional from `packages/server/src/server/agent/model-catalog.ts:36`, removed conditional from `packages/server/src/server/agent/providers/claude-agent.ts:183`, updated `CODEX_PROVIDER` constant to `"codex"` in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:63`, and replaced all `provider: "codex-mcp"` occurrences with `provider: CODEX_PROVIDER` in same file, updated all test assertions in `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts` to expect `"codex"` instead of `"codex-mcp"`. RESULT: Only ONE Codex provider exists with ID `"codex"`, no duplicate "Codex MCP" option in UI. EVIDENCE: `rg '"codex-mcp"' packages/` (no matches), `npm run typecheck --workspace=@paseo/server` (exit 0). + +- [x] **BUG**: Codex provider shows quadrupled user messages and duplicated agent messages. + + **Symptom**: When sending "hello" to Codex provider: + - User message "hello" appears 4 times + - Agent response appears 2 times (concatenated: "Hi! What can I help with...Hi! What can I help with...") + + **CRITICAL: Write a FAILING TEST FIRST. No ad-hoc fixes.** + + This bug proves our test coverage is inadequate. The fix must: + + 1. **First**: Write a test in `codex-mcp-agent.test.ts` that: + - Sends a simple message to the Codex provider + - Collects ALL emitted events (user_message, agent_message, etc.) + - Asserts: exactly ONE user_message event + - Asserts: exactly ONE agent_message event (or text_delta events that concatenate to ONE message) + - This test MUST FAIL with current code + + 2. **Second**: Investigate the root cause: + - Why are events being emitted multiple times? + - Is it in event handlers? In the MCP client? In message processing? + - Add debug logging to trace where duplication occurs + + 3. **Third**: Fix the root cause (not symptoms) + + 4. **Fourth**: Verify the test passes + + **DO NOT**: + - Add deduplication logic (that hides the bug) + - Skip the test + - Fix without understanding why + + **Acceptance criteria**: + - New test exists that would have caught this bug + - Test passes after fix + - No duplicate messages in UI + - **Done (2025-12-25 15:58)**: WHAT: (1) Added test "emits exactly one user_message and one assistant_message per turn" in `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:404-483` that verifies exactly 1 user_message and 1 assistant_message per turn. (2) Fixed `threadItemToTimeline` in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3621-3639` to skip `user_message` items (since we emit directly in stream()) and only emit `agent_message`/`reasoning` on `item.completed` (not on item.started/item.updated). (3) Fixed `handleMcpEvent` in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3194-3201` to skip `agent_message`, `agent_reasoning`, and `agent_reasoning_delta` events since they're now handled via `item.completed` path to avoid duplicates. ROOT CAUSE: Codex MCP sends BOTH direct events (agent_message, agent_reasoning_delta) AND item events (item.started, item.updated, item.completed) for the same message. User messages were emitted once by us in stream() AND again 3 times from Codex MCP's item.started/updated/completed events. Agent messages were emitted from both the agent_message direct event AND the item.completed event. RESULT: All 14 Codex MCP tests pass; typecheck passes. EVIDENCE: `npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts` (14 passed, 0 failed).