-
Notifications
You must be signed in to change notification settings - Fork 27
Enhance MCP server integration and tool registration in OpenClaw #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
20a8ec7
Initial plan
Copilot aac9780
feat: register MCP servers as native tools
Copilot b1be299
Merge pull request #8 from geffzhang/main
geffzhang 55f9602
Use ModelContextProtocol client for MCP servers
geffzhang 2e9f4b7
Merge pull request #7 from geffzhang/copilot/add-mcp-to-openclaw-inte…
geffzhang 37428d8
Extract McpNativeTool and improve registry
geffzhang ec4e9c3
Skip validation for disabled MCP servers
geffzhang 5b9fb8e
Dispose displaced tools on name collision
geffzhang 551bf31
Use raw JSON schema and remove owned-resource
geffzhang 1d2144a
Ignore disposal exceptions when replacing tools
geffzhang 4d4ebd5
Guard LoadAsync with SemaphoreSlim
geffzhang 4ac7b25
Prevent duplicate owned resource registration
geffzhang 942db3d
Validate MCP tool JSON arguments are objects
geffzhang 80b8f79
Refactor MCP transport and fix lifecycle bugs
geffzhang 7fb8192
Improve teardown and secret resolution in MCP registry
geffzhang 535d646
Improve MCP tool registry concurrency and safety
geffzhang a22d4ed
Handle null collections and cancellation
geffzhang 4f4e88e
Validate MCP servers and handle null config values
geffzhang dcaac07
Fix MCP tool handling and registry shutdown
Telli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,335 @@ | ||
| using System.Text; | ||
| using System.Text.Json; | ||
| using Microsoft.Extensions.Logging; | ||
| using ModelContextProtocol; | ||
| using ModelContextProtocol.Client; | ||
| using OpenClaw.Agent.Tools; | ||
| using OpenClaw.Core.Abstractions; | ||
| using OpenClaw.Core.Plugins; | ||
| using OpenClaw.Core.Security; | ||
|
|
||
| namespace OpenClaw.Agent.Plugins; | ||
|
|
||
| /// <summary> | ||
| /// Discovers tools from configured MCP servers and registers them as native OpenClaw tools. | ||
| /// </summary> | ||
| public sealed class McpServerToolRegistry : IDisposable | ||
| { | ||
| private readonly McpPluginsConfig _config; | ||
| private readonly ILogger _logger; | ||
| private readonly SemaphoreSlim _loadSemaphore = new(1, 1); | ||
| private readonly List<DiscoveredMcpTool> _tools = []; | ||
| private readonly List<McpClient> _clients = []; | ||
| private bool _loaded; | ||
| private bool _registered; | ||
| private bool _disposed; | ||
|
|
||
| /// <summary> | ||
| /// Creates a registry for configured MCP servers. | ||
| /// </summary> | ||
| public McpServerToolRegistry(McpPluginsConfig config, ILogger logger) | ||
| { | ||
| _config = config; | ||
| _logger = logger; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Connects to configured MCP servers and registers discovered tools into the native registry. | ||
| /// </summary> | ||
| public async Task RegisterToolsAsync(NativePluginRegistry nativeRegistry, CancellationToken ct) | ||
| { | ||
| ThrowIfDisposed(); | ||
| await _loadSemaphore.WaitAsync(ct); | ||
| try | ||
| { | ||
| ThrowIfDisposed(); | ||
| if (_registered) | ||
| return; | ||
|
|
||
| var tools = await LoadInternalAsync(ct); | ||
| foreach (var tool in tools) | ||
| nativeRegistry.RegisterExternalTool(tool.Tool, tool.PluginId, tool.Detail); | ||
|
|
||
| _registered = true; | ||
| } | ||
| finally | ||
| { | ||
| _loadSemaphore.Release(); | ||
| } | ||
| } | ||
|
|
||
| internal async Task<IReadOnlyList<DiscoveredMcpTool>> LoadAsync(CancellationToken ct) | ||
| { | ||
| ThrowIfDisposed(); | ||
| await _loadSemaphore.WaitAsync(ct); | ||
| try | ||
| { | ||
| ThrowIfDisposed(); | ||
| return _loaded ? _tools : await LoadInternalAsync(ct); | ||
| } | ||
| finally | ||
| { | ||
| _loadSemaphore.Release(); | ||
| } | ||
| } | ||
|
|
||
| private async Task<IReadOnlyList<DiscoveredMcpTool>> LoadInternalAsync(CancellationToken ct) | ||
| { | ||
| if (_loaded) | ||
| return _tools; | ||
|
|
||
| if (!_config.Enabled) | ||
| { | ||
| _loaded = true; | ||
| return _tools; | ||
| } | ||
|
|
||
| var discoveredTools = new List<DiscoveredMcpTool>(); | ||
| var discoveredClients = new List<McpClient>(); | ||
|
|
||
| try | ||
| { | ||
| foreach (var (serverId, serverConfig) in _config.Servers ?? []) | ||
| { | ||
| if (!serverConfig.Enabled) | ||
| continue; | ||
|
|
||
| var transport = CreateTransport(serverId, serverConfig); | ||
| using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(ct); | ||
| timeoutCts.CancelAfter(TimeSpan.FromSeconds(serverConfig.StartupTimeoutSeconds)); | ||
| var client = await McpClient.CreateAsync(transport, cancellationToken: timeoutCts.Token); | ||
| discoveredClients.Add(client); | ||
|
|
||
| var displayName = string.IsNullOrWhiteSpace(serverConfig.Name) ? serverId : serverConfig.Name!; | ||
| var pluginId = $"mcp:{serverId}"; | ||
|
|
||
| var tools = await LoadToolsFromClientAsync(client, serverId, pluginId, displayName, serverConfig, ct); | ||
|
|
||
| foreach (var tool in tools) | ||
| { | ||
| discoveredTools.Add(new DiscoveredMcpTool( | ||
| pluginId, | ||
| new McpNativeTool(client, tool.LocalName, tool.RemoteName, tool.Description, tool.InputSchemaText), | ||
| displayName)); | ||
| } | ||
| } | ||
|
|
||
| _clients.AddRange(discoveredClients); | ||
| _tools.AddRange(discoveredTools); | ||
| _loaded = true; | ||
| return _tools; | ||
| } | ||
| catch | ||
| { | ||
| foreach (var client in discoveredClients) | ||
| { | ||
| try | ||
| { | ||
| DisposeClient(client); | ||
| } | ||
| catch | ||
| { | ||
| } | ||
| } | ||
| throw; | ||
| } | ||
| } | ||
|
|
||
| private async Task<IReadOnlyList<McpToolDescriptor>> LoadToolsFromClientAsync( | ||
| McpClient client, | ||
| string serverId, | ||
| string pluginId, | ||
| string displayName, | ||
| McpServerConfig config, | ||
| CancellationToken ct) | ||
| { | ||
| using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(ct); | ||
| timeoutCts.CancelAfter(TimeSpan.FromSeconds(config.RequestTimeoutSeconds)); | ||
| var response = await client.ListToolsAsync(cancellationToken: timeoutCts.Token); | ||
|
|
||
| var tools = new List<McpToolDescriptor>(); | ||
| foreach (var tool in response) | ||
| { | ||
| var remoteName = tool.Name; | ||
| if (string.IsNullOrWhiteSpace(remoteName)) | ||
| throw new InvalidOperationException($"MCP server '{displayName}' returned a tool entry with an empty name."); | ||
|
|
||
| var localName = ResolveToolName(serverId, config.ToolNamePrefix, remoteName); | ||
| var description = !string.IsNullOrWhiteSpace(tool.Description) | ||
| ? $"{tool.Description} (from MCP server '{displayName}')" | ||
| : $"MCP tool '{remoteName}' from server '{displayName}'."; | ||
| var inputSchema = ResolveInputSchemaText(tool.JsonSchema); | ||
| tools.Add(new McpToolDescriptor(localName, remoteName, description, inputSchema)); | ||
| } | ||
|
|
||
| _logger.LogInformation("MCP server enabled: {ServerId} ({DisplayName}) with {ToolCount} tool(s)", | ||
| serverId, displayName, tools.Count); | ||
| return tools; | ||
| } | ||
|
|
||
| private static string ResolveToolName(string serverId, string? toolNamePrefix, string remoteName) | ||
| { | ||
| var prefix = toolNamePrefix; | ||
| if (prefix is null) | ||
| prefix = $"{SanitizePrefixPart(serverId)}."; | ||
|
|
||
| return string.IsNullOrEmpty(prefix) ? remoteName : prefix + remoteName; | ||
| } | ||
|
|
||
| private static string SanitizePrefixPart(string value) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(value)) | ||
| return "mcp"; | ||
|
|
||
| var sb = new StringBuilder(value.Length); | ||
| foreach (var ch in value) | ||
| { | ||
| if (char.IsLetterOrDigit(ch) || ch is '_' or '-' or '.') | ||
| sb.Append(char.ToLowerInvariant(ch)); | ||
| else | ||
| sb.Append('_'); | ||
| } | ||
|
|
||
| return sb.Length == 0 ? "mcp" : sb.ToString(); | ||
| } | ||
|
|
||
| private static string ResolveInputSchemaText(JsonElement inputSchema) | ||
| { | ||
| if (inputSchema.ValueKind is JsonValueKind.Undefined or JsonValueKind.Null) | ||
| return "{}"; | ||
|
|
||
| return inputSchema.GetRawText(); | ||
| } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| bool acquired = false; | ||
| try | ||
| { | ||
| acquired = _loadSemaphore.Wait(TimeSpan.FromSeconds(5)); | ||
| if (!acquired) | ||
| { | ||
| _logger.LogWarning("McpServerToolRegistry.Dispose() timed out waiting for load semaphore, waiting indefinitely to ensure load completes"); | ||
| _loadSemaphore.Wait(); | ||
| acquired = true; | ||
| } | ||
| } | ||
| catch (ObjectDisposedException) | ||
| { | ||
| _logger.LogWarning("McpServerToolRegistry.Dispose() encountered disposed semaphore, load may have completed concurrently"); | ||
| return; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| if (_disposed) | ||
| return; | ||
|
|
||
| _disposed = true; | ||
| foreach (var client in _clients) | ||
| { | ||
| try | ||
| { | ||
| DisposeClient(client); | ||
| } | ||
| catch | ||
| { | ||
| } | ||
| } | ||
| _clients.Clear(); | ||
| } | ||
| finally | ||
| { | ||
| if (acquired) | ||
| _loadSemaphore.Release(); | ||
| } | ||
| } | ||
|
|
||
| private static IClientTransport CreateTransport(string serverId, McpServerConfig config) | ||
| { | ||
| var transport = config.NormalizeTransport(); | ||
| return transport switch | ||
| { | ||
| "stdio" => new StdioClientTransport(new StdioClientTransportOptions | ||
| { | ||
| Command = config.Command!, | ||
| Arguments = config.Arguments ?? [], | ||
| WorkingDirectory = config.WorkingDirectory, | ||
| EnvironmentVariables = ResolveEnv(config.Environment), | ||
| Name = serverId, | ||
| }), | ||
| "http" => new HttpClientTransport(new HttpClientTransportOptions | ||
| { | ||
| Endpoint = new Uri(config.Url!), | ||
| AdditionalHeaders = ResolveHeaders(config.Headers), | ||
| Name = serverId, | ||
| }), | ||
| _ => throw new InvalidOperationException($"Unsupported MCP transport '{config.Transport}' for server '{serverId}'.") | ||
| }; | ||
| } | ||
|
|
||
| private static Dictionary<string, string?>? ResolveEnv(Dictionary<string, string>? environment) | ||
| { | ||
| if (environment is null || environment.Count == 0) | ||
| return null; | ||
|
|
||
| var resolved = new Dictionary<string, string?>(StringComparer.Ordinal); | ||
| foreach (var (name, rawValue) in environment) | ||
| { | ||
| if (rawValue is null) | ||
| { | ||
| resolved[name] = null; | ||
| continue; | ||
| } | ||
| var value = SecretResolver.Resolve(rawValue); | ||
| if (value is null && rawValue.StartsWith("env:", StringComparison.Ordinal)) | ||
| throw new InvalidOperationException($"Environment variable '{name}' references unset env var '{rawValue[4..]}'"); | ||
| resolved[name] = value ?? rawValue; | ||
| } | ||
|
|
||
| return resolved; | ||
| } | ||
|
|
||
| private static Dictionary<string, string>? ResolveHeaders(Dictionary<string, string>? headers) | ||
| { | ||
| if (headers is null || headers.Count == 0) | ||
| return null; | ||
|
|
||
| var resolved = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); | ||
| foreach (var (name, rawValue) in headers) | ||
| { | ||
| if (rawValue is null) | ||
| { | ||
| resolved[name] = string.Empty; | ||
| continue; | ||
| } | ||
| var value = SecretResolver.Resolve(rawValue); | ||
| if (value is null && rawValue.StartsWith("env:", StringComparison.Ordinal)) | ||
| throw new InvalidOperationException($"Header '{name}' references unset env var '{rawValue[4..]}'"); | ||
| resolved[name] = value ?? rawValue; | ||
| } | ||
|
|
||
| return resolved; | ||
| } | ||
|
|
||
| private void ThrowIfDisposed() | ||
| { | ||
| if (_disposed) | ||
| throw new ObjectDisposedException(nameof(McpServerToolRegistry)); | ||
| } | ||
|
|
||
| private static void DisposeClient(McpClient client) | ||
| { | ||
| if (client is IAsyncDisposable asyncDisposable) | ||
| { | ||
| asyncDisposable.DisposeAsync().AsTask().GetAwaiter().GetResult(); | ||
| return; | ||
| } | ||
|
|
||
| if (client is IDisposable disposable) | ||
| disposable.Dispose(); | ||
| } | ||
|
|
||
| internal sealed record DiscoveredMcpTool(string PluginId, ITool Tool, string Detail); | ||
| private sealed record McpToolDescriptor(string LocalName, string RemoteName, string Description, string InputSchemaText); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResolveEnvassumesrawValueis non-null; if configuration binding yields a null dictionary value,rawValue.StartsWith(...)will throw and prevent MCP discovery. This same null-value risk exists inResolveHeaderstoo.Severity: medium
Other Locations
src/OpenClaw.Agent/Plugins/McpServerToolRegistry.cs:292🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.