Repository navigation
Reconnect native MCP without replaying uncertain calls - #5556
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Confirmed the existing operation-deadline finding. Admission recovery occurs before the operation timeout starts, and reconnect after a dispatched call also runs outside it. A stalled handshake can delay the original uncertain-call error indefinitely or allow dispatch after the supplied timeout. Carry one deadline through admission/recovery and preserve the original uncertain-call error, while retaining cancellation-safe process cleanup. Referencing the existing thread rather than duplicating an inline finding.
Static review only; no local code or tests executed. Hosted native platform checks passed; Python 3.10 failed three optional MCP Tasks cancellation traceback tests outside this diff (baseline causation unverified).
(cherry picked from commit 88740c4)
…133-mcp-reconnect-python # Conflicts: # packages/codex/tests/test_deferred_mcp_tasks.py
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved, superseding my request changes. The Rust reconnect patch exactly matches the approved JavaScript implementation. One active-time budget covers admission, replacement initialization, retries and dispatch; an uncertain tool failure keeps its original error without replay. Shutdown fencing blocks replacement dispatch and joins owned process cleanup. Python integration fixtures are unchanged. No new actionable findings.
Hosted CI remains pending with no observed current-head failures. Static coordinator and independent review only; no candidate code or local tests executed.
Summary
The native MCP client reconnects before sending a tool request on a known-closed connection. If a tool loses its response after dispatch, the client preserves the original error and marks the connection for repair. A later request can use the replacement without repeating the interrupted tool.
Recovery and dispatch share the request's timeout budget. The client preserves the existing transient HTTP retry policy for tool discovery. Shutdown prevents a replacement from accepting work and waits for SDK-owned stdio servers to exit. Active HTTP requests can finish on their original connection.
Test plan
Checks
AGENTS.md, including.agents/skills/code-change-verification/scripts/run.shwhen requiredAGENTS.md