Skip to content

Commit 3dd9938

Browse files
committed
Address PR feedback on auto-discovery APIs
1 parent 071b700 commit 3dd9938

File tree

11 files changed

+111
-35
lines changed

11 files changed

+111
-35
lines changed

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ using Microsoft.Extensions.DependencyInjection;
278278
await using var serviceProvider = services.BuildServiceProvider();
279279

280280
var innerChatClient = serviceProvider.GetRequiredService<IChatClient>();
281-
using var chatClient = innerChatClient.UseManagedCodeMcpGatewayAutoDiscovery(
281+
using var chatClient = innerChatClient.UseMcpGatewayAutoDiscovery(
282282
serviceProvider,
283283
options =>
284284
{
@@ -321,7 +321,7 @@ await using var serviceProvider = services.BuildServiceProvider();
321321

322322
var innerChatClient = serviceProvider.GetRequiredService<IChatClient>();
323323
var loggerFactory = serviceProvider.GetRequiredService<ILoggerFactory>();
324-
using var chatClient = innerChatClient.UseManagedCodeMcpGatewayAutoDiscovery(
324+
using var chatClient = innerChatClient.UseMcpGatewayAutoDiscovery(
325325
serviceProvider,
326326
options =>
327327
{

docs/ADR/ADR-0003-reusable-chat-client-and-agent-tool-modules.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ At the same time, the repository still wants to keep the core package generic, l
2626
- `McpGatewayToolSet.AddTools(...)` composes those tools into an existing `IList<AITool>` without duplicating names
2727
- `ChatOptions.AddMcpGatewayTools(...)` attaches the same tools to chat-client requests
2828
- `McpGatewayToolSet.CreateDiscoveredTools(...)` projects the latest search matches as direct proxy tools
29-
- `McpGatewayAutoDiscoveryChatClient` and `UseManagedCodeMcpGatewayAutoDiscovery(...)` provide the recommended staged host wrapper for both plain `IChatClient` and Agent Framework hosts
29+
- `McpGatewayAutoDiscoveryChatClient` and `UseMcpGatewayAutoDiscovery(...)` provide the recommended staged host wrapper for both plain `IChatClient` and Agent Framework hosts
3030

3131
The recommended host flow is:
3232

@@ -127,7 +127,7 @@ Mitigations:
127127
Rollout:
128128

129129
1. Keep `McpGatewayToolSet` as the reusable module entry point.
130-
2. Add `McpGatewayAutoDiscoveryChatClient` and `UseManagedCodeMcpGatewayAutoDiscovery(...)` as the recommended host wrapper.
130+
2. Add `McpGatewayAutoDiscoveryChatClient` and `UseMcpGatewayAutoDiscovery(...)` as the recommended host wrapper.
131131
3. Document both chat-client and agent composition examples in `README.md`.
132132
4. Keep architecture docs aligned with the generic `AITool`-module approach.
133133

docs/Architecture/Overview.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ Out of scope:
2626

2727
`McpGateway` stays a thin facade over `McpGatewayRuntime`, which reads immutable catalog snapshots, coordinates vector or tokenizer-backed search, optionally rewrites queries through a keyed `IChatClient`, and invokes local or MCP tools. Optional startup warmup is available through a service-provider extension or hosted background service without changing the lazy default.
2828

29-
The package also keeps chat-client and agent integration generic: `McpGatewayToolSet` is the source of reusable `AITool` meta-tools and discovered proxy tools, `ChatOptions.AddMcpGatewayTools(...)` remains the low-level bridge, and `McpGatewayAutoDiscoveryChatClient` plus `UseManagedCodeMcpGatewayAutoDiscovery(...)` provide the recommended staged host wrapper that starts with two meta-tools and replaces the discovered proxy set on each new search result without introducing a hard Agent Framework dependency into the core package.
29+
The package also keeps chat-client and agent integration generic: `McpGatewayToolSet` is the source of reusable `AITool` meta-tools and discovered proxy tools, `ChatOptions.AddMcpGatewayTools(...)` remains the low-level bridge, and `McpGatewayAutoDiscoveryChatClient` plus `UseMcpGatewayAutoDiscovery(...)` provide the recommended staged host wrapper that starts with two meta-tools and replaces the discovered proxy set on each new search result without introducing a hard Agent Framework dependency into the core package.
3030

3131
## System And Module Map
3232

@@ -63,7 +63,7 @@ flowchart LR
6363
ToolSet --> ToolList["IList<AITool> composition"]
6464
ToolSet --> DiscoveredTools["CreateDiscoveredTools(...)"]
6565
ChatOptions["ChatOptions.AddMcpGatewayTools(...)"] --> ToolSet
66-
AutoDiscovery["McpGatewayAutoDiscoveryChatClient / UseManagedCodeMcpGatewayAutoDiscovery(...)"] --> ToolSet
66+
AutoDiscovery["McpGatewayAutoDiscoveryChatClient / UseMcpGatewayAutoDiscovery(...)"] --> ToolSet
6767
AutoDiscovery --> ChatClient
6868
Warmup["McpGatewayServiceProviderExtensions / McpGatewayIndexWarmupService"] --> IMcpGateway
6969
McpGateway --> Runtime["McpGatewayRuntime"]

src/ManagedCode.MCPGateway/McpGatewayAutoDiscoveryChatClient.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,19 @@ when functionNamesByCallId.TryGetValue(functionResult.CallId, out var functionNa
230230
}
231231

232232
var serialized = McpGatewayJsonSerializer.TrySerializeToElement(result);
233-
if (serialized is not JsonElement jsonElement)
233+
if (serialized is not JsonElement { ValueKind: JsonValueKind.Object } jsonElement)
234234
{
235235
return null;
236236
}
237237

238-
return jsonElement.Deserialize<McpGatewaySearchResult>(McpGatewayJsonSerializer.Options);
238+
try
239+
{
240+
return jsonElement.Deserialize<McpGatewaySearchResult>(McpGatewayJsonSerializer.Options);
241+
}
242+
catch (JsonException)
243+
{
244+
return null;
245+
}
239246
}
240247
}
241248

src/ManagedCode.MCPGateway/McpGatewayToolSet.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public IList<AITool> AddTools(
5050
{
5151
ArgumentNullException.ThrowIfNull(tools);
5252

53-
var targetTools = tools.IsReadOnly ? new List<AITool>(tools) : tools;
53+
var targetTools = new List<AITool>(tools);
5454
var toolNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
5555

5656
foreach (var tool in targetTools)
@@ -249,6 +249,16 @@ private static string SanitizeToolName(string value)
249249
builder.Append(char.IsLetterOrDigit(character) || character == '_' ? character : '_');
250250
}
251251

252+
if (builder.Length == 0)
253+
{
254+
return "gateway_tool";
255+
}
256+
257+
if (!char.IsLetter(builder[0]) && builder[0] != '_')
258+
{
259+
builder.Insert(0, "t_");
260+
}
261+
252262
return builder.ToString();
253263
}
254264
}

src/ManagedCode.MCPGateway/Registration/McpGatewayChatClientExtensions.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace ManagedCode.MCPGateway;
66

77
public static class McpGatewayChatClientExtensions
88
{
9-
public static IChatClient UseManagedCodeMcpGatewayAutoDiscovery(
9+
public static IChatClient UseMcpGatewayAutoDiscovery(
1010
this IChatClient chatClient,
1111
McpGatewayToolSet toolSet,
1212
ILoggerFactory? loggerFactory = null,
@@ -24,7 +24,7 @@ public static IChatClient UseManagedCodeMcpGatewayAutoDiscovery(
2424
options);
2525
}
2626

27-
public static IChatClient UseManagedCodeMcpGatewayAutoDiscovery(
27+
public static IChatClient UseMcpGatewayAutoDiscovery(
2828
this IChatClient chatClient,
2929
IServiceProvider serviceProvider,
3030
Action<McpGatewayAutoDiscoveryOptions>? configure = null)
@@ -35,7 +35,7 @@ public static IChatClient UseManagedCodeMcpGatewayAutoDiscovery(
3535
var options = new McpGatewayAutoDiscoveryOptions();
3636
configure?.Invoke(options);
3737

38-
return chatClient.UseManagedCodeMcpGatewayAutoDiscovery(
38+
return chatClient.UseMcpGatewayAutoDiscovery(
3939
serviceProvider.GetRequiredService<McpGatewayToolSet>(),
4040
serviceProvider.GetService<ILoggerFactory>(),
4141
serviceProvider,

src/ManagedCode.MCPGateway/Registration/McpGatewayChatOptionsExtensions.cs

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,4 @@ public static ChatOptions AddMcpGatewayTools(
3232
searchToolName,
3333
invokeToolName);
3434
}
35-
36-
[Obsolete("Use AddMcpGatewayTools(...) instead.")]
37-
public static ChatOptions AddManagedCodeMcpGatewayTools(
38-
this ChatOptions options,
39-
McpGatewayToolSet toolSet,
40-
string searchToolName = McpGatewayToolSet.DefaultSearchToolName,
41-
string invokeToolName = McpGatewayToolSet.DefaultInvokeToolName)
42-
=> options.AddMcpGatewayTools(toolSet, searchToolName, invokeToolName);
43-
44-
[Obsolete("Use AddMcpGatewayTools(...) instead.")]
45-
public static ChatOptions AddManagedCodeMcpGatewayTools(
46-
this ChatOptions options,
47-
IServiceProvider serviceProvider,
48-
string searchToolName = McpGatewayToolSet.DefaultSearchToolName,
49-
string invokeToolName = McpGatewayToolSet.DefaultInvokeToolName)
50-
=> options.AddMcpGatewayTools(serviceProvider, searchToolName, invokeToolName);
5135
}

tests/ManagedCode.MCPGateway.Tests/Agents/McpGatewayAgentFrameworkIntegrationTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public async Task ChatClientAgent_UsesAutoDiscoveryWithoutEmbeddings()
2222
{
2323
Scenarios = GatewayIntegrationTestSupport.CreateAutoDiscoveryScenarios(useSemanticQueries: false)
2424
});
25-
using var autoDiscoveryClient = modelClient.UseManagedCodeMcpGatewayAutoDiscovery(
25+
using var autoDiscoveryClient = modelClient.UseMcpGatewayAutoDiscovery(
2626
serviceProvider,
2727
options => options.MaxDiscoveredTools = 2);
2828

@@ -67,7 +67,7 @@ public async Task ChatClientAgent_UsesAutoDiscoveryWithEmbeddings()
6767
{
6868
Scenarios = GatewayIntegrationTestSupport.CreateAutoDiscoveryScenarios(useSemanticQueries: true)
6969
});
70-
using var autoDiscoveryClient = modelClient.UseManagedCodeMcpGatewayAutoDiscovery(
70+
using var autoDiscoveryClient = modelClient.UseMcpGatewayAutoDiscovery(
7171
serviceProvider,
7272
options => options.MaxDiscoveredTools = 2);
7373

tests/ManagedCode.MCPGateway.Tests/ChatClient/McpGatewayChatClientIntegrationTests.cs

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
using Microsoft.Extensions.AI;
44
using Microsoft.Extensions.DependencyInjection;
5+
using System.Text.Json;
56

67
namespace ManagedCode.MCPGateway.Tests;
78

@@ -25,6 +26,59 @@ await Assert.That(options.Tools.Select(static tool => tool.Name)).IsEquivalentTo
2526
]);
2627
}
2728

29+
[TUnit.Core.Test]
30+
public async Task ToolSet_AddTools_ReturnsNewListWithoutMutatingInput()
31+
{
32+
await using var serviceProvider = GatewayTestServiceProviderFactory.Create(static _ => { });
33+
var toolSet = serviceProvider.GetRequiredService<McpGatewayToolSet>();
34+
var existingTools = new List<AITool>
35+
{
36+
TestFunctionFactory.CreateFunction(
37+
static () => "existing",
38+
"existing_tool",
39+
"Existing tool.")
40+
};
41+
42+
var composedTools = toolSet.AddTools(existingTools);
43+
44+
await Assert.That(existingTools.Count).IsEqualTo(1);
45+
await Assert.That(composedTools.Count).IsEqualTo(3);
46+
await Assert.That(ReferenceEquals(existingTools, composedTools)).IsFalse();
47+
}
48+
49+
[TUnit.Core.Test]
50+
public async Task AutoDiscoveryChatClient_IgnoresPrimitiveSearchResultPayloads()
51+
{
52+
await using var serviceProvider = GatewayTestServiceProviderFactory.Create(static _ => { });
53+
var modelClient = new TestChatClient(new TestChatClientOptions
54+
{
55+
Scenarios =
56+
[
57+
new TestChatClientScenario(
58+
"return text",
59+
static _ => true,
60+
static _ => TestChatClientScenario.Text("ok"))
61+
]
62+
});
63+
64+
using var chatClient = modelClient.UseMcpGatewayAutoDiscovery(serviceProvider);
65+
66+
var response = await chatClient.GetResponseAsync(
67+
[
68+
new ChatMessage(ChatRole.User, "Find the right tools."),
69+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("search-1", McpGatewayToolSet.DefaultSearchToolName, new Dictionary<string, object?>())]),
70+
new ChatMessage(ChatRole.Assistant, [new FunctionResultContent("search-1", JsonSerializer.SerializeToElement("not-an-object"))])
71+
]);
72+
73+
await Assert.That(response.Text).IsEqualTo("ok");
74+
await Assert.That(modelClient.Invocations.Count).IsEqualTo(1);
75+
await Assert.That(modelClient.Invocations[0].ToolNames).IsEquivalentTo(
76+
[
77+
McpGatewayToolSet.DefaultSearchToolName,
78+
McpGatewayToolSet.DefaultInvokeToolName
79+
]);
80+
}
81+
2882
[TUnit.Core.Test]
2983
public async Task AutoDiscoveryChatClient_ReplacesDiscoveredToolsWithoutEmbeddings()
3084
{
@@ -38,7 +92,7 @@ public async Task AutoDiscoveryChatClient_ReplacesDiscoveredToolsWithoutEmbeddin
3892
Scenarios = GatewayIntegrationTestSupport.CreateAutoDiscoveryScenarios(useSemanticQueries: false)
3993
});
4094

41-
using var chatClient = modelClient.UseManagedCodeMcpGatewayAutoDiscovery(
95+
using var chatClient = modelClient.UseMcpGatewayAutoDiscovery(
4296
serviceProvider,
4397
options => options.MaxDiscoveredTools = 2);
4498

@@ -72,7 +126,7 @@ public async Task AutoDiscoveryChatClient_ReplacesDiscoveredToolsWithEmbeddings(
72126
Scenarios = GatewayIntegrationTestSupport.CreateAutoDiscoveryScenarios(useSemanticQueries: true)
73127
});
74128

75-
using var chatClient = modelClient.UseManagedCodeMcpGatewayAutoDiscovery(
129+
using var chatClient = modelClient.UseMcpGatewayAutoDiscovery(
76130
serviceProvider,
77131
options => options.MaxDiscoveredTools = 2);
78132

tests/ManagedCode.MCPGateway.Tests/MetaTools/McpGatewayMetaToolTests.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,30 @@ public async Task CreateMetaTools_InvokeToolSupportsContextSummary()
6969
await Assert.That(GetJsonProperty(invokeResult, "output").GetString()).IsEqualTo("open github|user is on repository settings page");
7070
}
7171

72+
[TUnit.Core.Test]
73+
public async Task CreateDiscoveredTools_PrefixesNamesThatStartWithDigits()
74+
{
75+
await using var serviceProvider = GatewayTestServiceProviderFactory.Create(static _ => { });
76+
var toolSet = serviceProvider.GetRequiredService<McpGatewayToolSet>();
77+
78+
var discoveredTools = toolSet.CreateDiscoveredTools(
79+
[
80+
new McpGatewaySearchMatch(
81+
ToolId: "local:123-tool",
82+
SourceId: "9-source",
83+
SourceKind: McpGatewaySourceKind.Local,
84+
ToolName: "123 tool",
85+
DisplayName: null,
86+
Description: "Example tool.",
87+
RequiredArguments: [],
88+
InputSchemaJson: null,
89+
Score: 0.9d)
90+
]);
91+
92+
await Assert.That(discoveredTools.Count).IsEqualTo(1);
93+
await Assert.That(discoveredTools[0].Name).IsEqualTo("t_123_tool");
94+
}
95+
7296
private static AIFunction GetFunction(IReadOnlyList<AITool> tools, string toolName)
7397
=> (tools.Single(tool => tool.Name == toolName) as AIFunction)
7498
?? throw new InvalidOperationException($"Tool '{toolName}' is not an AIFunction.");

0 commit comments

Comments
 (0)