Skip to content

Commit 2f824ff

Browse files
Aniruddh25aaronburtleCopilotCopilot
authored
fix(mcp-stdio): propagate isError from CallToolResult to wire response (#3625)
## Summary Fixes a bug in the MCP stdio transport where `isError` was silently dropped from every `tools/call` response. ### Root Cause `HandleCallTool` in `McpStdioServer` used `CoerceToMcpContentBlocks` (reflection-based) to extract only the `Content` list from the `CallToolResult`, then constructed the JSON-RPC result with `new { content }`. The `IsError` property was never read, so all tool responses — successes and errors alike — looked identical on the wire. The HTTP transport was not affected; the MCP SDK serializes the full `CallToolResult` returned by `WithCallToolHandler`, correctly emitting `"isError": true` when set. ### Fix After `CoerceToMcpContentBlocks`, read `callResult.IsError` and include `isError` in the result object only when `== true`. The existing `WhenWritingNull` serializer option on `_jsonOptions` ensures a null `isError` (success path) is omitted from the wire — matching the MCP spec. ```csharp bool? isError = callResult.IsError; if (isError == true) { WriteResult(id, new { content, isError }); } else { WriteResult(id, new { content }); } ``` ### Tests Added (`McpStdioServerContentBlockTests`) - **`HandleCallTool_ErrorResult_EmitsIsErrorTrueOnWire`** — creates a real `CallToolResult { IsError = true }`, replicates the exact `HandleCallTool` conditional logic, and asserts `"isError": true` is present in the JSON-RPC wire output. - **`HandleCallTool_SuccessResult_OmitsIsErrorFromWire`** — verifies that a success result produces no `isError` field at all on the wire. --------- Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 4d92d12 commit 2f824ff

2 files changed

Lines changed: 134 additions & 5 deletions

File tree

src/Azure.DataApiBuilder.Mcp/Core/McpStdioServer.cs

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -517,18 +517,42 @@ private async Task HandleCallToolAsync(JsonElement? id, JsonElement root, Cancel
517517
tool, toolName!, argsDoc, _serviceProvider, ct);
518518
}
519519

520-
// Normalize to MCP content blocks (array). We try to pass through if a 'Content' property exists,
521-
// otherwise we wrap into a single text block.
522-
object[] content = CoerceToMcpContentBlocks(callResult);
523-
524-
WriteResult(id, new { content });
520+
await HandleCallToolAsync(id ?? default, callResult);
525521
}
526522
finally
527523
{
528524
argsDoc?.Dispose();
529525
}
530526
}
531527

528+
/// <summary>
529+
/// Writes the JSON-RPC result for a completed tool call, propagating <see cref="CallToolResult.IsError"/>
530+
/// to the wire so MCP clients can distinguish tool errors from successes.
531+
/// Extracted as a separate overload so it can be exercised directly in unit tests.
532+
/// </summary>
533+
/// <param name="id">The request identifier used to correlate the response.</param>
534+
/// <param name="callResult">The result returned by the tool execution.</param>
535+
private Task HandleCallToolAsync(JsonElement id, CallToolResult callResult)
536+
{
537+
// Normalize to MCP content blocks (array). We try to pass through if a 'Content' property exists,
538+
// otherwise we wrap into a single text block.
539+
object[] content = CoerceToMcpContentBlocks(callResult);
540+
541+
// Propagate isError so MCP clients can distinguish tool errors from successes.
542+
// _jsonOptions has WhenWritingNull, so a null isError is omitted from the wire.
543+
bool? isError = callResult.IsError;
544+
if (isError == true)
545+
{
546+
WriteResult(id, new { content, isError });
547+
}
548+
else
549+
{
550+
WriteResult(id, new { content });
551+
}
552+
553+
return Task.CompletedTask;
554+
}
555+
532556
/// <summary>
533557
/// Coerces the call result into an array of MCP content blocks.
534558
/// Tools can either return a custom object with a public "Content" property

src/Service.Tests/UnitTests/McpStdioServerContentBlockTests.cs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#nullable enable
55

66
using System;
7+
using System.Collections.Generic;
78
using System.IO;
89
using System.Reflection;
910
using System.Text;
@@ -98,6 +99,110 @@ public void WriteResult_WithTextContentBlock_OmitsNullAnnotationsAndMetaFromWire
9899
"_meta should be omitted from wire output when null.");
99100
}
100101

102+
/// <summary>
103+
/// Verifies that when a tool returns a real <see cref="CallToolResult"/> with IsError=true,
104+
/// the stdio wire output contains "isError": true in the JSON-RPC result object.
105+
/// Regression test for the bug where CoerceToMcpContentBlocks discarded IsError.
106+
/// </summary>
107+
[TestMethod]
108+
public void HandleCallTool_ErrorResult_EmitsIsErrorTrueOnWire()
109+
{
110+
(McpStdioServer server, MemoryStream memoryStream, McpStdoutWriter stdoutWriter) = CreateServerWithCapturedOutput();
111+
112+
// Use a real CallToolResult (the actual type returned by every tool's error path)
113+
// to match exactly what HandleCallToolAsync receives from McpTelemetryHelper.
114+
CallToolResult callToolResult = new()
115+
{
116+
IsError = true,
117+
Content = new List<ContentBlock> { new TextContentBlock { Text = "{\"status\":\"error\"}" } }
118+
};
119+
120+
JsonElement id = JsonDocument.Parse("1").RootElement;
121+
MethodInfo? handleCallToolAsync = typeof(McpStdioServer).GetMethod(
122+
"HandleCallToolAsync",
123+
BindingFlags.Instance | BindingFlags.NonPublic,
124+
binder: null,
125+
types: new[] { typeof(JsonElement), typeof(CallToolResult) },
126+
modifiers: null);
127+
128+
Assert.IsNotNull(handleCallToolAsync, "Expected to find McpStdioServer.HandleCallToolAsync(JsonElement, CallToolResult).");
129+
130+
object? handleCallTask = handleCallToolAsync.Invoke(server, new object[] { id, callToolResult });
131+
Assert.IsNotNull(handleCallTask, "HandleCallToolAsync should return a Task.");
132+
((System.Threading.Tasks.Task)handleCallTask).GetAwaiter().GetResult();
133+
134+
string wireOutput = ReadCapturedOutput(stdoutWriter, memoryStream);
135+
136+
using JsonDocument doc = JsonDocument.Parse(wireOutput);
137+
JsonElement result = doc.RootElement.GetProperty("result");
138+
139+
Assert.IsTrue(result.TryGetProperty("isError", out JsonElement isErrorEl),
140+
"isError must be present on the wire for error tool results.");
141+
Assert.AreEqual(JsonValueKind.True, isErrorEl.ValueKind,
142+
"isError must be true for error tool results.");
143+
}
144+
145+
/// <summary>
146+
/// Verifies that when a tool returns a success result (IsError=null), the stdio wire
147+
/// output does NOT contain an "isError" field (omitted, not present as null or false).
148+
/// </summary>
149+
[TestMethod]
150+
public void HandleCallTool_SuccessResult_OmitsIsErrorFromWire()
151+
{
152+
(McpStdioServer server, MemoryStream memoryStream, McpStdoutWriter stdoutWriter) = CreateServerWithCapturedOutput();
153+
154+
object[] contentBlocks = InvokeCoerceToMcpContentBlocks(new
155+
{
156+
Content = new ContentBlock[] { new TextContentBlock { Text = "{\"status\":\"success\"}" } }
157+
});
158+
159+
JsonElement id = JsonDocument.Parse("2").RootElement;
160+
161+
// Simulate what HandleCallTool does when IsError is null (success)
162+
InvokeWriteResult(server, id, new { content = contentBlocks });
163+
164+
string wireOutput = ReadCapturedOutput(stdoutWriter, memoryStream);
165+
166+
using JsonDocument doc = JsonDocument.Parse(wireOutput);
167+
JsonElement result = doc.RootElement.GetProperty("result");
168+
169+
Assert.IsFalse(result.TryGetProperty("isError", out _),
170+
"isError must be absent from the wire for successful tool results.");
171+
}
172+
173+
private static (McpStdioServer server, MemoryStream memoryStream, McpStdoutWriter stdoutWriter) CreateServerWithCapturedOutput()
174+
{
175+
MemoryStream memoryStream = new();
176+
StreamWriter streamWriter = new(
177+
memoryStream,
178+
new UTF8Encoding(encoderShouldEmitUTF8Identifier: false),
179+
bufferSize: -1,
180+
leaveOpen: true)
181+
{
182+
AutoFlush = true
183+
};
184+
McpStdoutWriter stdoutWriter = new(streamWriter);
185+
186+
ServiceCollection services = new();
187+
services.AddSingleton(stdoutWriter);
188+
services.AddSingleton<McpToolRegistry>();
189+
IServiceProvider serviceProvider = services.BuildServiceProvider();
190+
191+
McpStdioServer server = new(
192+
serviceProvider.GetRequiredService<McpToolRegistry>(),
193+
serviceProvider);
194+
195+
return (server, memoryStream, stdoutWriter);
196+
}
197+
198+
private static string ReadCapturedOutput(McpStdoutWriter stdoutWriter, MemoryStream memoryStream)
199+
{
200+
stdoutWriter.Dispose();
201+
memoryStream.Position = 0;
202+
using StreamReader reader = new(memoryStream, new UTF8Encoding(encoderShouldEmitUTF8Identifier: false));
203+
return reader.ReadToEnd().TrimEnd();
204+
}
205+
101206
private static object[] InvokeCoerceToMcpContentBlocks(object callResult)
102207
{
103208
MethodInfo? coerceMethod = typeof(McpStdioServer).GetMethod(

0 commit comments

Comments
 (0)