Retry replay-safe MCP tool calls - #2154
Conversation
ApprovabilityVerdict: Needs human review This PR changes retry behavior for MCP tool calls from never-retry to conditionally-retry based on annotations. Changing retry/replay semantics has potential for non-obvious runtime impacts and warrants human review. You can customize Macroscope's approvability policy. Learn more. |
ee4ee4e to
493e7d1
Compare
493e7d1 to
235ee42
Compare
mikasenghaas
left a comment
There was a problem hiding this comment.
i dont love that the tool server has to opt-in. would be curious how mcp retries are handled in codex, etc. do they just blindly retry or is there any standard mechanism in mcp to interact with stateful tool servers?
| read_only = getattr(fn, "tool_read_only", False) | ||
| idempotent = getattr(fn, "tool_idempotent", False) |
There was a problem hiding this comment.
isnt read only also idempotent? so could we just use mark idempotent = replay is safe?
235ee42 to
5ce6281
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Overview
Teach the shared null/bash MCP client to retry a tool call after response loss only when the evaluator-configured server explicitly declares that repeating it is safe.
Details
read_onlyandidempotentoptions to@vf.tooland publish the corresponding standard MCP tool annotations.readOnlyHintandidempotentHintvalues into the delivery-aware retry policy introduced by Fix MCP session lifecycle and replay safety #2134.Scope
This pull request is stacked on #2138, which builds on #2134. It does not claim generic exactly-once execution: non-idempotent actions still need server-owned operation IDs, revisions, and result caching before opting into retry.
Note
Medium Risk
Changes retry behavior for MCP tools after transport loss; mis-declared idempotent/read-only tools could be executed twice, though the default remains no replay without explicit hints.
Overview
MCP tool retries after a lost response are now gated on explicit server hints, instead of treating all
call_toolinvocations as non-replayable.The shared minimal harness reads each tool’s MCP
readOnlyHint/idempotentHintat enumeration time, stores a per-toolreplay_safeflag in dispatch metadata, and passes it intomcp_sessionforcall_toolso the existing delivery-aware retry logic can replay only when repetition is declared safe. Unannotated tools keep the prior conservative behavior (no retry after the operation may have started).Evaluator-authored toolsets can opt in via
@vf.tool(read_only=…)/idempotent=…, whichToolsetmaps to standard MCPToolAnnotationswhen registering tools with FastMCP.Reviewed by Cursor Bugbot for commit 5ce6281. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Retry replay-safe MCP tool calls based on
readOnlyHintandidempotentHintannotations@tooldecorator indecorators.pywithread_onlyandidempotentkeyword parameters that attach metadata to decorated functions.Toolset._registerreads this metadata and advertises matchingToolAnnotations(readOnlyHint/idempotentHint) when registering tools with MCP.connect_mcpinspects each tool's annotations to compute areplay_safeflag, stored in the dispatch map alongside the server and tool name.call_mcppasses thereplay_safeflag tomcp_session, enabling retries on lost responses for read-only or idempotent tools; previously all calls usedreplay_safe=False.Macroscope summarized 5ce6281.