Strip stale requestState across MRTR retry rounds (#1458 review)

When an MRTR round emits an InputRequiredResult whose RequestState is null,
the next round's request must not carry a stale requestState forwarded from
a prior round's params clone. Both the client retry loop
(McpClientImpl.ResolveInputRequestsAsync) and the server backcompat resolver
(McpServerImpl.InvokeWithInputRequiredResultHandlingAsync) deep-cloned the
prior request params, overwrote requestState only when the new result
supplied one, and left any prior value untouched otherwise.

Fixed by adding the symmetric else branch that explicitly removes the
requestState key whenever the latest InputRequiredResult clears it. This
mirrors the existing paramsObj.Remove("inputResponses") in the
state-only-retry sibling branch and matches the wire-format intent that
requestState is round-scoped, not session-scoped.

Regression tests:

* MrtrIntegrationTests.IncompleteResultRetry_OmittingRequestState_StripsStaleStateFromRetryParams
  (client) — uses the fake-stream harness to send InputRequiredResult with
  requestState on round 1, then without it on round 2, and asserts the
  third retry params do not contain a "requestState" key.

* MrtrServerBackcompatTests.InputRequiredException_TransitioningRequestStateToNull_DoesNotLeakStaleState
  (server) — uses a non-MRTR client so the server falls into the backcompat
  resolver path; a tool throws InputRequiredException with requestState
  then with null, and the third handler invocation asserts
  context.Params.RequestState is null.

Both tests were verified to fail with their respective fixes reverted.

Also removed a duplicate XML inheritdoc tag above McpClientImpl.AddKnownTools per review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: halter73

src/ModelContextProtocol.Core/Client/McpClientImpl.cs

@@ -760,7 +760,6 @@ internal void ResumeSession(ResumeClientSessionOptions resumeOptions)
         LogClientSessionResumed(_endpointName);
     }
 
-    /// <inheritdoc/>
     /// <inheritdoc/>
     public override void AddKnownTools(IEnumerable<Tool> tools)
     {
@@ -877,6 +876,12 @@ request.Params is System.Text.Json.Nodes.JsonObject paramsObjForHeaders &&
                     {
                         paramsObj["requestState"] = requestState;
                     }
+                    else
+                    {
+                        // Strip any stale requestState carried over from the previous round's clone so
+                        // the server doesn't see a continuation token the current round is not using.
+                        paramsObj.Remove("requestState");
+                    }
 
                     request = new JsonRpcRequest { Method = request.Method, Params = paramsObj, Context = request.Context };
                 }

src/ModelContextProtocol.Core/Server/McpServerImpl.cs

@@ -1262,6 +1262,12 @@ internal bool IsStatefulSession() =>
                 {
                     paramsObj["requestState"] = requestState;
                 }
+                else
+                {
+                    // Strip any stale requestState carried over from the previous round's clone so
+                    // the next tool invocation doesn't see a continuation token the current round is not using.
+                    paramsObj.Remove("requestState");
+                }
 
                 request = new JsonRpcRequest
                 {

tests/ModelContextProtocol.Tests/Client/MrtrIntegrationTests.cs

@@ -609,6 +609,168 @@ public async Task IncompleteResultOnNonMrtrSession_LogsWarning()
         serverToClient.Writer.Complete();
     }
 
+    [Fact]
+    public async Task IncompleteResultRetry_OmittingRequestState_StripsStaleStateFromRetryParams()
+    {
+        // Regression test for #1458 review feedback: when the server returns InputRequiredResult
+        // with requestState on round 1 and then InputRequiredResult WITHOUT requestState on round 2,
+        // the client's third retry must NOT carry the stale round-1 requestState forward via the
+        // params deep clone. Without the fix, the third retry's params contain {"requestState": "round1-state"}
+        // even though the round-2 InputRequiredResult cleared it.
+        StartServer(); // base-class disposal hook
+        var clientToServer = new Pipe();
+        var serverToClient = new Pipe();
+
+        var clientOptions = new McpClientOptions();
+        clientOptions.Handlers.ElicitationHandler = (_, _) =>
+            new ValueTask<ElicitResult>(new ElicitResult
+            {
+                Action = "accept",
+                Content = new Dictionary<string, JsonElement>
+                {
+                    ["confirmed"] = JsonDocument.Parse("\"yes\"").RootElement.Clone()
+                }
+            });
+
+        var clientTask = McpClient.CreateAsync(
+            new StreamClientTransport(
+                clientToServer.Writer.AsStream(),
+                serverToClient.Reader.AsStream(),
+                LoggerFactory),
+            clientOptions,
+            loggerFactory: LoggerFactory,
+            cancellationToken: TestContext.Current.CancellationToken);
+
+        var serverReader = new StreamReader(clientToServer.Reader.AsStream());
+        var serverWriter = serverToClient.Writer.AsStream();
+
+        // Initialize handshake — negotiate DRAFT-2026-v1 so the client treats InputRequiredResult as MRTR.
+        var initLine = await serverReader.ReadLineAsync(TestContext.Current.CancellationToken);
+        Assert.NotNull(initLine);
+        var initRequest = JsonSerializer.Deserialize<JsonRpcRequest>(initLine, McpJsonUtilities.DefaultOptions);
+        Assert.NotNull(initRequest);
+        Assert.Equal("initialize", initRequest.Method);
+
+        var initResponse = new JsonRpcResponse
+        {
+            Id = initRequest.Id,
+            Result = JsonSerializer.SerializeToNode(new InitializeResult
+            {
+                ProtocolVersion = "DRAFT-2026-v1",
+                Capabilities = new ServerCapabilities { Tools = new() },
+                ServerInfo = new Implementation { Name = "MrtrServer", Version = "1.0" }
+            }, McpJsonUtilities.DefaultOptions),
+        };
+        await WriteJsonRpcAsync(serverWriter, initResponse);
+
+        var initializedLine = await serverReader.ReadLineAsync(TestContext.Current.CancellationToken);
+        Assert.NotNull(initializedLine);
+
+        await using var client = await clientTask;
+        Assert.Equal("DRAFT-2026-v1", client.NegotiatedProtocolVersion);
+
+        var cancellationToken = TestContext.Current.CancellationToken;
+
+        // Capture the retry payloads sent by the client so we can inspect them after the call completes.
+        JsonObject? retry1Params = null;
+        JsonObject? retry2Params = null;
+
+        var serverLoop = Task.Run(async () =>
+        {
+            // --- Round 1: receive original tools/call, respond with InputRequiredResult + requestState="round1-state".
+            var call1Line = await serverReader.ReadLineAsync(cancellationToken);
+            Assert.NotNull(call1Line);
+            var call1 = JsonSerializer.Deserialize<JsonRpcRequest>(call1Line, McpJsonUtilities.DefaultOptions);
+            Assert.NotNull(call1);
+            Assert.Equal("tools/call", call1.Method);
+
+            var round1Result = new JsonObject
+            {
+                ["resultType"] = "input_required",
+                ["inputRequests"] = new JsonObject
+                {
+                    ["q1"] = JsonSerializer.SerializeToNode(
+                        InputRequest.ForElicitation(new ElicitRequestParams { Message = "round1", RequestedSchema = new() }),
+                        McpJsonUtilities.DefaultOptions),
+                },
+                ["requestState"] = "round1-state",
+            };
+            await WriteJsonRpcAsync(serverWriter, new JsonRpcResponse { Id = call1.Id, Result = round1Result });
+
+            // --- Round 2: receive first retry (should include requestState="round1-state" + inputResponses).
+            var call2Line = await serverReader.ReadLineAsync(cancellationToken);
+            Assert.NotNull(call2Line);
+            var call2 = JsonSerializer.Deserialize<JsonRpcRequest>(call2Line, McpJsonUtilities.DefaultOptions);
+            Assert.NotNull(call2);
+            retry1Params = call2.Params as JsonObject;
+
+            // Respond with another InputRequiredResult — this time WITHOUT requestState — to force the
+            // client to clear any stale state on the next retry params clone.
+            var round2Result = new JsonObject
+            {
+                ["resultType"] = "input_required",
+                ["inputRequests"] = new JsonObject
+                {
+                    ["q2"] = JsonSerializer.SerializeToNode(
+                        InputRequest.ForElicitation(new ElicitRequestParams { Message = "round2", RequestedSchema = new() }),
+                        McpJsonUtilities.DefaultOptions),
+                },
+                // Intentionally NO "requestState" key.
+            };
+            await WriteJsonRpcAsync(serverWriter, new JsonRpcResponse { Id = call2.Id, Result = round2Result });
+
+            // --- Round 3: receive second retry — assertion target. Must NOT contain "requestState".
+            var call3Line = await serverReader.ReadLineAsync(cancellationToken);
+            Assert.NotNull(call3Line);
+            var call3 = JsonSerializer.Deserialize<JsonRpcRequest>(call3Line, McpJsonUtilities.DefaultOptions);
+            Assert.NotNull(call3);
+            retry2Params = call3.Params as JsonObject;
+
+            // Final success response so the client's call completes cleanly.
+            await WriteJsonRpcAsync(serverWriter, new JsonRpcResponse
+            {
+                Id = call3.Id,
+                Result = JsonSerializer.SerializeToNode(new CallToolResult
+                {
+                    Content = [new TextContentBlock { Text = "done" }]
+                }, McpJsonUtilities.DefaultOptions),
+            });
+        }, cancellationToken);
+
+        var response = await client.SendRequestAsync(
+            new JsonRpcRequest
+            {
+                Method = "tools/call",
+                Params = JsonSerializer.SerializeToNode(new CallToolRequestParams { Name = "any-tool" }, McpJsonUtilities.DefaultOptions),
+            },
+            cancellationToken);
+
+        await serverLoop;
+
+        // Sanity check the final result reached us.
+        Assert.NotNull(response.Result);
+        var result = JsonSerializer.Deserialize<CallToolResult>(response.Result, McpJsonUtilities.DefaultOptions);
+        Assert.NotNull(result);
+        Assert.Equal("done", Assert.IsType<TextContentBlock>(Assert.Single(result.Content)).Text);
+
+        // The first retry must carry requestState="round1-state".
+        Assert.NotNull(retry1Params);
+        Assert.NotNull(retry1Params!["inputResponses"]);
+        Assert.Equal("round1-state", retry1Params["requestState"]?.GetValue<string>());
+
+        // The second retry must NOT carry a stale requestState. Pre-fix, the deep clone of the
+        // round-1 request kept "round1-state" in paramsObj because the client only OVERWROTE it
+        // when InputRequiredResult.RequestState was non-null. With the fix, it explicitly removes
+        // the key whenever the server's new InputRequiredResult clears it.
+        Assert.NotNull(retry2Params);
+        Assert.NotNull(retry2Params!["inputResponses"]);
+        Assert.False(retry2Params.ContainsKey("requestState"),
+            "Retry params must not carry a stale requestState from the previous round.");
+
+        clientToServer.Writer.Complete();
+        serverToClient.Writer.Complete();
+    }
+
     private static async Task WriteJsonRpcAsync(Stream writer, JsonRpcMessage message)
     {
         var bytes = JsonSerializer.SerializeToUtf8Bytes<JsonRpcMessage>(message, McpJsonUtilities.DefaultOptions);

tests/ModelContextProtocol.Tests/Server/MrtrServerBackcompatTests.cs

@@ -0,0 +1,113 @@
+using System.Text.Json;
+using Microsoft.Extensions.DependencyInjection;
+using ModelContextProtocol.Client;
+using ModelContextProtocol.Protocol;
+using ModelContextProtocol.Server;
+using ModelContextProtocol.Tests.Utils;
+
+namespace ModelContextProtocol.Tests.Server;
+
+/// <summary>
+/// Tests for the legacy MRTR backcompat resolver in <c>McpServerImpl.InvokeWithInputRequiredResultHandlingAsync</c>.
+/// This path runs only when the client did NOT negotiate MRTR (DRAFT-2026-v1) and the session is stateful —
+/// the server dispatches each input request to the client via standard JSON-RPC and re-invokes the handler
+/// with the merged responses. To exercise it the server must NOT pin a protocol version; the client picks
+/// a non-draft version during initialize negotiation.
+/// </summary>
+public class MrtrServerBackcompatTests : ClientServerTestBase
+{
+    private readonly List<string?> _observedRequestStates = [];
+    private int _attempt;
+
+    public MrtrServerBackcompatTests(ITestOutputHelper testOutputHelper)
+        : base(testOutputHelper, startServer: false)
+    {
+    }
+
+    protected override void ConfigureServices(ServiceCollection services, IMcpServerBuilder mcpServerBuilder)
+    {
+        mcpServerBuilder.WithTools([
+            McpServerTool.Create(
+                (RequestContext<CallToolRequestParams> context) =>
+                {
+                    var attempt = Interlocked.Increment(ref _attempt);
+                    _observedRequestStates.Add(context.Params?.RequestState);
+
+                    return attempt switch
+                    {
+                        // Round 1: caller has no state; emit one and request elicitation.
+                        1 => throw new InputRequiredException(
+                            inputRequests: new Dictionary<string, InputRequest>
+                            {
+                                ["confirm"] = InputRequest.ForElicitation(new ElicitRequestParams
+                                {
+                                    Message = "round1",
+                                    RequestedSchema = new()
+                                })
+                            },
+                            requestState: "round1"),
+                        // Round 2: deliberately clear the state by passing requestState: null while still
+                        // asking for another elicitation. This exercises the params clone path that
+                        // previously preserved the stale "round1" carry-over from round 1's deep clone.
+                        2 => throw new InputRequiredException(
+                            inputRequests: new Dictionary<string, InputRequest>
+                            {
+                                ["confirm"] = InputRequest.ForElicitation(new ElicitRequestParams
+                                {
+                                    Message = "round2",
+                                    RequestedSchema = new()
+                                })
+                            },
+                            requestState: null),
+                        // Round 3 (final): report what the handler observed so the test can assert it.
+                        _ => $"final-state:{context.Params?.RequestState ?? "<null>"}",
+                    };
+                },
+                new McpServerToolCreateOptions
+                {
+                    Name = "requeststate-transition",
+                    Description = "Tool that transitions requestState from set to null across MRTR rounds."
+                }),
+        ]);
+    }
+
+    [Fact]
+    public async Task InputRequiredException_TransitioningRequestStateToNull_DoesNotLeakStaleState()
+    {
+        StartServer();
+
+        // Non-MRTR client → server falls into the legacy backcompat resolver path on InputRequiredException.
+        var clientOptions = new McpClientOptions
+        {
+            ProtocolVersion = "2025-06-18",
+            Capabilities = new ClientCapabilities { Elicitation = new() },
+        };
+        clientOptions.Handlers.ElicitationHandler = (_, _) =>
+            new ValueTask<ElicitResult>(new ElicitResult
+            {
+                Action = "accept",
+                Content = new Dictionary<string, JsonElement>
+                {
+                    ["answer"] = JsonDocument.Parse("\"ok\"").RootElement,
+                },
+            });
+
+        await using var client = await CreateMcpClientForServer(clientOptions);
+
+        var result = await client.CallToolAsync(
+            "requeststate-transition",
+            cancellationToken: TestContext.Current.CancellationToken);
+
+        // Three attempts: round 1 (no state) → round 2 (state="round1") → round 3 (state=null after fix).
+        // Without the fix, the third observed state would erroneously remain "round1" because the deep-clone
+        // of the prior request params carried it forward when InputRequiredException.RequestState was null.
+        Assert.Equal(3, _observedRequestStates.Count);
+        Assert.Null(_observedRequestStates[0]);
+        Assert.Equal("round1", _observedRequestStates[1]);
+        Assert.Null(_observedRequestStates[2]);
+
+        var content = Assert.Single(result.Content);
+        var text = Assert.IsType<TextContentBlock>(content).Text;
+        Assert.Equal("final-state:<null>", text);
+    }
+}