fix: align MCP call responses and streaming events with OpenAI [2/2]

[maralbahari]

Summary

This PR follows the MCP request, discovery, and execution changes from the first MCP shipment. It aligns the gateway's public MCP tool-call responses with the OpenAI Responses contract, using recorded OpenAI behavior as the ground truth.

This PR:

• Replaces the gateway-specific mcp_tool_call output item with OpenAI's mcp_call item.
• Exposes the original MCP identity through:
  • server_label
  • name
  • string-encoded arguments
• Adds the OpenAI-compatible approval_request_id, output, and error fields.
• Represents MCP execution failures as structured mcp_tool_execution_error values.
• Emits the OpenAI MCP call streaming lifecycle:
  • response.mcp_call.in_progress
  • response.mcp_call_arguments.delta
  • response.mcp_call_arguments.done
  • response.mcp_call.completed
  • response.mcp_call.failed
• Ensures internal function calls used for upstream inference do not leak into the public response.
• Updates response accumulation to recognize and preserve mcp_call output items.
• Adds a recorder that captures matching MCP scenarios against OpenAI and the gateway.
• Adds paired OpenAI and gateway cassettes for:
  • tool discovery without invocation
  • successful streaming tool calls
  • successful blocking tool calls
  • missing required arguments
  • invalid argument types
• Compares gateway output items, error shapes, and streaming event transitions directly against the recorded OpenAI behavior.

The separate OpenAI mcp_list_tools discovery lifecycle is not emitted yet and remains follow-up work.

This PR is stacked on the first MCP shipment and should initially target fix-mcp-tool-call-1. After the first PR merges, it can be rebased and retargeted to main.

Test Plan

• cargo check --workspace
• cargo test --workspace
• cargo test -p agentic-server-core --test mcp_tool_test
• cargo clippy --all-targets -- -D warnings
• cargo fmt -- --check

[franciscojavierarceo]

One additional documentation issue could not be attached inline because the file is outside this PR's diff:

[P2] docs/design/codex-integration.md lines 140 and 151 still document the removed mcp_tool_call wire shape. Please update both tables to describe mcp_call, server_label, name, output, structured errors, and the new streaming lifecycle.

The PR head otherwise passed the full agentic-server-core test suite, targeted Clippy with warnings denied, formatting checks, and the current GitHub checks.

[ashwing]

Two accumulator-side drops that the cassette tests miss — both are only reachable through the streaming from_sse_lines path, not the from_json reconstruction the tests use. I wrote repro tests for both against this branch; they fail. Inline.

The mcp_call output-item + error shapes match the recorded OpenAI behavior 1:1, and there's no await-holding-lock on the call path.

Two non-blocking follow-ups:

• The failed path (status=Failed -> mcp_call.failed) has no synthetic-emit unit test asserting sequence_number stays contiguous — only the completed branch does. Given the earlier error-path sequence gaps in #132/#136, worth one.
• The mcp_list_tools deferral means the emitted stream is a strict subset of OpenAI's (no list-tools output item before mcp_call). Known/accepted — just worth a line in the mcp-gateway doc so clients don't assume it's present.

[maralbahari]

@franciscojavierarceo @ashwing addressed the issues. ready to check again thanks

[franciscojavierarceo]

Holding approval on the reproduced done-only MCP ordering bug. The current head otherwise passes the full agentic-server-core suite (269 unit tests plus integration/cassette tests), Clippy with warnings denied, formatting, shell syntax, and all GitHub checks.

[franciscojavierarceo]

[P1] Preserve output order when output_item.added is omitted

This fallback pushes a done-only MCP item immediately, while earlier reasoning/message items remain in in_flight until response.done. That reverses valid output order when, for example, reasoning is added at index 0 and an MCP output_item.done arrives at index 1 without a preceding added event. The branch explicitly supports done-only MCP events in test_streaming_preserves_all_documented_mcp_call_statuses, so the two supported paths need to compose.

I reproduced this on 83e1fd0: the accumulator returned [McpCall, Reasoning] instead of [Reasoning, McpCall]. Please retain fallback items by output_index and finalize all slots in index order, with a combined regression test.

[franciscojavierarceo]

Fixed in 44d3bbc: streaming items now retain their output_index and are finalized in upstream order. Added a regression covering reasoning at index 0 followed by a done-only MCP call at index 1. The PR source fork disables maintainer edits, so please cherry-pick 44d3bbc onto this branch.

[franciscojavierarceo]

[P2] Keep ID-less parallel MCP calls distinct

When both provider identifiers are absent, FunctionToolCall deserializes them as empty strings and every call hashes the same "\0" source identity. Parallel ID-less calls therefore receive the same public MCP item ID. Please allocate the public ID once per call and carry it through the event plan/result, or otherwise give the absent-identity case unique stored state. Add a regression with two calls whose id and call_id are both omitted.

[franciscojavierarceo]

Fixed in 44d3bbc: deserialized function calls with a missing or empty provider item ID now receive a unique request-local ID, while started/completed MCP items still reuse the same public ID. Added a parallel ID-less lifecycle regression. Please cherry-pick 44d3bbc; GitHub rejected a direct push because maintainer edits are disabled.

[franciscojavierarceo]

[P2] Keep the tracked reusable fixture on a safe endpoint

The recorder script now correctly requires an explicit operator-trusted endpoint, but the committed reusable fixture still points to a public ephemeral TryCloudflare tunnel with require_approval: "never". This moves the stale/untrusted endpoint problem rather than removing it. Please keep this tracked file on a loopback or non-routable example value and generate the operator-supplied recording config in a temporary file; historical cassette request bodies can retain the recorded URL.

[franciscojavierarceo]

Fixed in 44d3bbc: the tracked reusable fixture now uses loopback, and recorder-generated tool configuration lives in a temporary file removed on exit. The negative recorder guard preserves the tracked fixture. Please cherry-pick 44d3bbc; the source fork does not allow maintainer edits.

[maralbahari]

@franciscojavierarceo thanksss. I just pushed the cherry pick