Skip to content

Commit bafbb0a

Browse files
CopilotstephentoubMackinnonBuck
authored
Fix CreateSamplingHandler to use ChatRole.Tool for tool result messages (#1128)
Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: stephentoub <[email protected]> Co-authored-by: Stephen Toub <[email protected]> Co-authored-by: MackinnonBuck <[email protected]> Co-authored-by: Mackinnon Buck <[email protected]>
1 parent db0beeb commit bafbb0a

File tree

2 files changed

+103
-2
lines changed

2 files changed

+103
-2
lines changed

src/ModelContextProtocol.Core/AIContentExtensions.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,11 @@ public static class AIContentExtensions
128128
{
129129
if (sm.Content?.Select(b => b.ToAIContent()).OfType<AIContent>().ToList() is { Count: > 0 } aiContents)
130130
{
131-
messages.Add(new ChatMessage(sm.Role is Role.Assistant ? ChatRole.Assistant : ChatRole.User, aiContents));
131+
ChatRole role =
132+
aiContents.All(static c => c is FunctionResultContent) ? ChatRole.Tool :
133+
sm.Role is Role.Assistant ? ChatRole.Assistant :
134+
ChatRole.User;
135+
messages.Add(new ChatMessage(role, aiContents));
132136
}
133137
}
134138

tests/ModelContextProtocol.Tests/Client/McpClientTests.cs

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,103 @@ public async Task CreateSamplingHandler_ShouldHandleResourceMessages()
260260
Assert.Equal("endTurn", result.StopReason);
261261
}
262262

263+
[Fact]
264+
public async Task CreateSamplingHandler_ShouldUseToolRoleForToolResultMessages()
265+
{
266+
// Arrange
267+
var mockChatClient = new Mock<IChatClient>();
268+
var requestParams = new CreateMessageRequestParams
269+
{
270+
Messages =
271+
[
272+
new SamplingMessage
273+
{
274+
Role = Role.User,
275+
Content = [new TextContentBlock { Text = "What is the weather in Paris?" }]
276+
},
277+
new SamplingMessage
278+
{
279+
Role = Role.Assistant,
280+
Content = [new ToolUseContentBlock
281+
{
282+
Id = "call_weather_123",
283+
Name = "get_weather",
284+
Input = JsonElement.Parse("""{"location":"Paris"}""")
285+
}]
286+
},
287+
new SamplingMessage
288+
{
289+
Role = Role.User,
290+
Content = [new ToolResultContentBlock
291+
{
292+
ToolUseId = "call_weather_123",
293+
Content = [new TextContentBlock { Text = "Weather: 18°C, sunny" }]
294+
}]
295+
},
296+
new SamplingMessage
297+
{
298+
Role = Role.User,
299+
Content =
300+
[
301+
new ToolResultContentBlock
302+
{
303+
ToolUseId = "call_mixed_123",
304+
Content = [new TextContentBlock { Text = "Tool result" }]
305+
},
306+
new TextContentBlock { Text = "Additional text content" }
307+
]
308+
}
309+
],
310+
MaxTokens = 100
311+
};
312+
313+
IEnumerable<ChatMessage>? capturedMessages = null;
314+
var cancellationToken = CancellationToken.None;
315+
var expectedResponse = new[] {
316+
new ChatResponseUpdate
317+
{
318+
ModelId = "test-model",
319+
FinishReason = ChatFinishReason.Stop,
320+
Role = ChatRole.Assistant,
321+
Contents = [new TextContent("The weather in Paris is 18°C and sunny.")]
322+
}
323+
}.ToAsyncEnumerable();
324+
325+
mockChatClient
326+
.Setup(client => client.GetStreamingResponseAsync(It.IsAny<IEnumerable<ChatMessage>>(), It.IsAny<ChatOptions>(), cancellationToken))
327+
.Callback<IEnumerable<ChatMessage>, ChatOptions?, CancellationToken>((messages, _, _) => capturedMessages = messages.ToList())
328+
.Returns(expectedResponse);
329+
330+
var handler = mockChatClient.Object.CreateSamplingHandler();
331+
332+
// Act
333+
var result = await handler(requestParams, Mock.Of<IProgress<ProgressNotificationValue>>(), cancellationToken);
334+
335+
// Assert
336+
Assert.NotNull(result);
337+
Assert.NotNull(capturedMessages);
338+
var messagesList = capturedMessages.ToList();
339+
Assert.Equal(4, messagesList.Count);
340+
341+
// First message should be User role (text message)
342+
Assert.Equal(ChatRole.User, messagesList[0].Role);
343+
Assert.IsType<TextContent>(messagesList[0].Contents.Single());
344+
345+
// Second message should be Assistant role (tool use)
346+
Assert.Equal(ChatRole.Assistant, messagesList[1].Role);
347+
Assert.IsType<FunctionCallContent>(messagesList[1].Contents.Single());
348+
349+
// Third message should be Tool role (tool result only) - this is the bug fix
350+
Assert.Equal(ChatRole.Tool, messagesList[2].Role);
351+
Assert.IsType<FunctionResultContent>(messagesList[2].Contents.Single());
352+
353+
// Fourth message should be User role (mixed content: tool result + text)
354+
Assert.Equal(ChatRole.User, messagesList[3].Role);
355+
Assert.Equal(2, messagesList[3].Contents.Count);
356+
Assert.Contains(messagesList[3].Contents, c => c is FunctionResultContent);
357+
Assert.Contains(messagesList[3].Contents, c => c is TextContent);
358+
}
359+
263360
[Fact]
264361
public async Task ListToolsAsync_AllToolsReturned()
265362
{
@@ -690,4 +787,4 @@ public async Task SetLoggingLevelAsync_WithRequestParams_NullThrows()
690787
await Assert.ThrowsAsync<ArgumentNullException>("requestParams",
691788
() => client.SetLoggingLevelAsync((SetLevelRequestParams)null!, TestContext.Current.CancellationToken));
692789
}
693-
}
790+
}

0 commit comments

Comments
 (0)