Skip to content

mcp: don't mutate caller params on multi-round-trip retries - #1145

Merged
guglielmo-san merged 5 commits into
modelcontextprotocol:mainfrom
yhxlele:fix-mrtr-caller-params-mutation
Aug 31, 2026
Merged

mcp: don't mutate caller params on multi-round-trip retries#1145
guglielmo-san merged 5 commits into
modelcontextprotocol:mainfrom
yhxlele:fix-mrtr-caller-params-mutation

Conversation

@yhxlele

@yhxlele yhxlele commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #1144.

clientMultiRoundTripMiddleware calls setMultiRoundTripRetryParams(req, ...), which writes InputResponses and RequestState through the caller's params pointer and never clears them. A *CallToolParams (or GetPromptParams / ReadResourceParams) struct reused across calls then carries the first call's answers into the next call: a server gating on len(req.Params.InputResponses) == 0 — the idiom in this repo's own tests and examples — treats the fresh call as already answered, so the client's ElicitationHandler never fires and an elicitation-gated (e.g. destructive) tool runs without confirmation.

This change makes the retry loop carry the fulfilled responses and echoed state on a shallow copy of the params (multiRoundTripRetryRequest) instead of mutating the original request. Caller-owned params are now left untouched by CallTool, GetPrompt, and ReadResource. The server-side shim (serverMultiRoundTripMiddleware) is unchanged: the params it mutates are SDK-owned, decoded from the wire.

Tests: TestMultiRoundTrip_AutoRetryDoesNotMutateCallerParams (CallTool: params unchanged after the call, and reusing the same struct fulfills the input requests again instead of silently replaying the first call's answers — it failed with elicitations after reuse = 1, want 2 before the fix), plus GetPrompt/ReadResource analogues asserting params are left clean. All three fail on main and pass with this change; go test ./... and go vet ./... are clean.

Note on observable behavior: callers can no longer read the fulfilled responses off their own params struct after a call returns. That side effect was undocumented, and relying on it is exactly what makes the replay hazard above possible.

Comment thread mcp/mrtr.go Outdated
Comment thread mcp/mrtr.go Outdated
@yhxlele
yhxlele requested a review from guglielmo-san August 12, 2026 03:43
@yhxlele

yhxlele commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

@guglielmo-san Both review comments have been addressed (the clone is merged into setMultiRoundTripRetryParams, and the default case now returns an explicit error) and the branch has main merged in. Whenever you have a moment for another look, mainly want to make sure it isn't waiting on anything from my side.

@guglielmo-san

Copy link
Copy Markdown
Contributor

@yhxlele thank you for the contribution!

@guglielmo-san
guglielmo-san merged commit 69c17de into modelcontextprotocol:main Aug 31, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mcp: client multi-round-trip middleware mutates caller params, replaying inputResponses across calls

2 participants