mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
Fix Codex MCP duplicate user/assistant message emissions
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -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;
|
||||
|
||||
37
plan.md
37
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).
|
||||
|
||||
Reference in New Issue
Block a user