global mcp retry

[demianbrecht]

No description provided.

[demianbrecht]

🤖 [Security] No findings. Reviewed the RetryTransport extension (transient transport-fault retry) and the MCP read_timeout change.

Checked and clear:

• TLS verification is preserved — ssl_verify is still threaded through _build_transport into httpx.AsyncHTTPTransport(verify=...); the retry wiring does not bypass or weaken verify.
• No secret exposure — the retry warning logs emit only server name, attempt count, exception type name, and delay; no tokens or request/response bodies.
• DoS via attacker-controlled Retry-After is bounded — _retry_after_seconds clamps to _RETRY_BACKOFF_MAX_SECONDS (60s) and worst-case wall clock is bounded by max_retries × per-attempt timeout + capped backoff.
• No new injection / authz / crypto / SSRF / path-traversal surface.

Non-security note (out of scope, flagging for the quality reviewer): retrying on ReadTimeout replays the JSON-RPC request, which can re-execute a side-effecting MCP tool if the server already processed it. The code documents the replayable-body invariant; idempotency of tool calls is the open question, not a security flaw.

Critical: 0
High: 0
Medium: 0
Low: 0

[demianbrecht]

🤖 [Quality] When max_retries > 0 the SDK read_timeout_seconds is set to None, deliberately delegating all timeout authority to the httpx client timeout (httpx.Timeout(config.timeout) in oauth.py). That works for the default timeout=30.0, but McpServerConfig.timeout is float | None and httpx.Timeout(None) disables the client timeout entirely. So a config with timeout=None and retries enabled removes the last remaining bound — a hung server hangs forever with nothing to cancel it (the old code at least kept the SDK read_timeout mirror). Consider documenting that timeout=None is unsupported with retries, or falling back to a sane ceiling in that combination.

[demianbrecht]

🤖 [Quality] Retrying ReadTimeout/RemoteProtocolError re-sends the same JSON-RPC request, and the handle_async_request comment reasons only about body replayability (buffered = re-sendable). But a read timeout or mid-flight protocol error can occur after the server has already begun — or finished — executing the call; the response just never made it back. MCP tools/call requests are not guaranteed idempotent (a tool may mutate remote state), so a retry here can double-execute. 429 retries are safe because the server rejected the request before processing; transport-fault retries are not in the same category. Worth a sentence in the docstring acknowledging that retried tool calls assume idempotent-or-tolerant-of-replay semantics, so future readers do not treat this as universally safe.

[demianbrecht]

🤖 [Quality] Solid, well-scoped change extending MCP retry coverage from HTTP 429 to transient transport faults (timeouts, connection errors, protocol errors), with the SDK read-timeout ceiling lifted so it cannot cancel a live retry sequence. The reasoning is captured carefully in docstrings and comments, and the tests are genuinely meaningful — _FaultTransport exercises the retry loop, body-replay invariant, max-retries exhaustion, non-transient passthrough, and the max_retries=0 path, asserting real behavior rather than framework wiring. Version bump and config-comment updates are consistent.

Two non-blocking observations posted inline:

• mcp.py:165 — with max_retries>0 and timeout=None, both the SDK ceiling and the httpx client timeout are disabled, leaving a hung server unbounded. Edge case (default timeout is 30.0), but worth documenting or guarding.
• oauth.py:465 — retrying transport faults re-sends potentially non-idempotent tools/call requests; the replayability comment covers body buffering but not semantic idempotency. Suggest a docstring note so the safety assumption is explicit.

Minor: the class docstring/comment cite ConnectTimeout specifically but tests only cover ReadTimeout and ConnectError; since both are caught via the httpx.TimeoutException base, coverage is adequate.

CI: lint/test (3.12–3.14)/SAST pass; e2e (3.12/3.13) pending at review time. No blockers.