feat(mcp): server launch plumbing and tool naming

Spec: the complete environment goes to the SDK with InheritEnvironmentVariables=false instead of nulls; the cache key also covers cwd and prefix.
This commit is contained in:
EugeneTes
2026-09-26 20:17:37 +02:00
parent 5b7d593427
commit b30c71d30a
8 changed files with 319 additions and 5 deletions

View File

@@ -111,10 +111,11 @@ two meet only in tool naming (see "Names").
the value is explicit when it reaches the policy.
- The environment a server process gets is `IProcessSpawnPolicy`'s result for a request with `Purpose = McpServer`,
`Origin` = the server name, the expanded command, args and cwd, and the expanded `env` entries as the explicit
environment. It's passed to `StdioClientTransport.EnvironmentVariables` in full, with a null for every inherited
name the policy removed, so the SDK's merge over the host environment can't reintroduce it.
- The catalogue cache key is the server name plus a digest of the **unexpanded** command, args and env, so every chat
shares one catalogue even though their expanded args differ. Secrets from `${env:…}` never enter the key.
environment. It's passed to `StdioClientTransportOptions.EnvironmentVariables` in full with
`InheritEnvironmentVariables = false` (SDK 2.x), so the SDK doesn't merge the host environment back in.
- The catalogue cache key is the server name plus a digest of the **unexpanded** command, args, env, cwd and tool
prefix (the catalogue holds exposed names, which depend on the prefix), so every chat shares one catalogue even
though their expanded args differ. Secrets from `${env:…}` never enter the key.
- `McpOptions.ApplyEnvironment(getEnv)` works like `TelegramOptions.ApplyEnvironment` and fills in the preset from
`PLAYWRIGHT_ENABLED`, `PLAYWRIGHT_HEADLESS` (default true), `PLAYWRIGHT_BROWSER`, `PLAYWRIGHT_EXTRA_ARGS`
(shell-style split honouring quotes, so `--device "iPhone 15"` is two argv entries) and `PLAYWRIGHT_COMMAND`.

View File

@@ -227,7 +227,7 @@ Process-wide spawners without a session (the scheduler, MCP catalogue discovery)
| `BashTool` | `BashTool`, `Command` = the model's command, `Cwd` = `ctx.Cwd` | none | Through `ctx.PrepareProcessAsync`. Then it runs the shell as today with the returned command and cwd, and a cleared-then-copied environment. |
| `ExtensionApi.ExecAsync` | `ExtensionExec`, `Origin` = extension name | `ExecOptions.Environment` | Session-bound. `ExecOptions.Environment` keeps its meaning (set, or null to remove), now on top of the filtered base. |
| Scheduler `ScriptRunner` | `ScheduledScript`, `Scope` = target, `Origin` = job id, `TimeZoneId` = the target's zone | none | Through the DI `IProcessSpawnPolicy`: jobs fire for chats with no live session. Replaces `SchedulerOptions.ScrubbedEnvironment`. |
| MCP `McpServerConnection` | `McpServer`, `Origin` = server name, `Arguments` = expanded args | the server's `env` entries | Through `ctx.PrepareProcessAsync` from the first tool call. The result's env goes to `StdioClientTransport.EnvironmentVariables`, with a null for every inherited name it doesn't contain. |
| MCP `McpServerConnection` | `McpServer`, `Origin` = server name, `Arguments` = expanded args | the server's `env` entries | Through `ctx.PrepareProcessAsync` from the first tool call. The result's env goes to `StdioClientTransportOptions.EnvironmentVariables`, with `InheritEnvironmentVariables = false`. |
| MCP `McpCatalogCache` discovery | as above, `Cwd` = the temp dir | the server's `env` entries | Through `api.Processes.Prepare` (activation, no session). |
A spawner whose `Prepare` throws reports it the way it reports a failed start: `bash` returns an error result

View File

@@ -0,0 +1,64 @@
using System.Security.Cryptography;
using System.Text;
using System.Text.Json;
using System.Text.RegularExpressions;
using ModelContextProtocol.Client;
using Volcanic.AgenticCore.Abstractions.Processes;
namespace Volcanic.AgenticCore.Extensions.Mcp;
/// <summary>Turns a server config into a spawn request and the SDK's transport options.</summary>
internal static partial class McpLaunch
{
public const string WorkspaceToken = "${workspace}";
public static readonly TimeSpan ShutdownTimeout = TimeSpan.FromSeconds(5);
public static string ExpandWorkspace(string value, string workspace) =>
value.Replace(WorkspaceToken, workspace, StringComparison.Ordinal);
/// <summary><c>${workspace}</c>, then <c>${env:NAME}</c> from the host (unset → empty). Only env values get the second.</summary>
public static string ExpandEnvValue(string value, string workspace, Func<string, string?> getEnv) =>
EnvReference().Replace(ExpandWorkspace(value, workspace), m => getEnv(m.Groups[1].Value) ?? "");
public static ProcessSpawnRequest BuildRequest(string name, McpServerConfig config, string workspace, Func<string, string?> getEnv) => new()
{
Purpose = ProcessPurpose.McpServer,
Command = ExpandWorkspace(config.Command, workspace),
Arguments = [.. config.Args.Select(a => ExpandWorkspace(a, workspace))],
Cwd = config.Cwd is { Length: > 0 } cwd ? ExpandWorkspace(cwd, workspace) : workspace,
Environment = config.Env.Count == 0
? null
: config.Env.ToDictionary(kv => kv.Key, kv => kv.Value is null ? null : ExpandEnvValue(kv.Value, workspace, getEnv), StringComparer.Ordinal),
Origin = name,
};
/// <summary>The spawn's environment is complete (process policy), so the SDK must not merge the host's back in.</summary>
public static StdioClientTransportOptions TransportOptions(string name, ProcessSpawnContext spawn, Action<string> onStderrLine) => new()
{
Name = name,
Command = spawn.Command,
Arguments = [.. spawn.Arguments],
WorkingDirectory = spawn.Cwd,
InheritEnvironmentVariables = false,
EnvironmentVariables = new Dictionary<string, string?>(spawn.Environment.Select(kv => KeyValuePair.Create(kv.Key, (string?)kv.Value))),
ShutdownTimeout = ShutdownTimeout,
StandardErrorLines = onStderrLine,
};
/// <summary>Server name plus a digest of the unexpanded command, args, env, cwd and prefix: every chat shares it, and no host secret enters it.</summary>
public static string CacheKey(string name, McpServerConfig config)
{
var canonical = JsonSerializer.Serialize(new
{
config.Command,
config.Args,
Env = config.Env.OrderBy(kv => kv.Key, StringComparer.Ordinal).Select(kv => new[] { kv.Key, kv.Value }).ToArray(),
config.Cwd,
config.ToolPrefix,
});
return $"{name}:{Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(canonical)))}";
}
[GeneratedRegex(@"\$\{env:([A-Za-z_][A-Za-z0-9_]*)\}")]
private static partial Regex EnvReference();
}

View File

@@ -0,0 +1,21 @@
namespace Volcanic.AgenticCore.Extensions.Mcp;
/// <summary>The last lines a server wrote to stderr: a server that dies explains why only there.</summary>
internal sealed class StderrTail(int capacity = 20)
{
private readonly Queue<string> _lines = new();
private readonly Lock _lock = new();
public void Add(string line)
{
lock (_lock)
{
_lines.Enqueue(line);
while (_lines.Count > capacity) _lines.Dequeue();
}
}
public void Clear() { lock (_lock) _lines.Clear(); }
public IReadOnlyList<string> Lines { get { lock (_lock) return [.. _lines]; } }
}

View File

@@ -0,0 +1,40 @@
using System.Text.RegularExpressions;
namespace Volcanic.AgenticCore.Extensions.Mcp;
/// <summary>
/// Exposed tool names: <c>prefix + mcpName</c>, characters outside <c>[A-Za-z0-9_-]</c> replaced by <c>_</c>, at most 48
/// characters so the Claude Code provider's <c>ToolCatalog</c> keeps them readable, duplicates suffixed <c>_2</c>, <c>_3</c>.
/// </summary>
internal static partial class ToolNaming
{
public const int MaxLength = 48;
public static string Sanitize(string? prefix, string mcpName)
{
var name = InvalidCharacters().Replace((prefix ?? "") + mcpName, "_");
if (name.Length == 0) name = "tool";
return name.Length > MaxLength ? name[..MaxLength] : name;
}
public static IReadOnlyList<string> Expose(IReadOnlyList<string> mcpNames, string? prefix)
{
var used = new HashSet<string>(StringComparer.Ordinal);
var result = new List<string>(mcpNames.Count);
foreach (var mcpName in mcpNames)
{
var baseName = Sanitize(prefix, mcpName);
var name = baseName;
for (var n = 2; !used.Add(name); n++)
{
var suffix = $"_{n}";
name = (baseName.Length + suffix.Length > MaxLength ? baseName[..(MaxLength - suffix.Length)] : baseName) + suffix;
}
result.Add(name);
}
return result;
}
[GeneratedRegex("[^A-Za-z0-9_-]")]
private static partial Regex InvalidCharacters();
}

View File

@@ -114,4 +114,15 @@ public class ToolCatalogTests
var notObject = new ToolDescriptor("x", "d", Fixtures.Record("[]"));
Assert.Throws<ClaudeCodeException>(() => new ToolCatalog([notObject]));
}
[Fact]
public void Mcp_style_tool_names_alias_to_themselves()
{
var maxLength = new string('b', 48);
var catalog = new ToolCatalog([Tool("browser_navigate"), Tool("browser_take_screenshot"), Tool("weird_name_x_2"), Tool(maxLength)]);
Assert.Equal([maxLength, "browser_navigate", "browser_take_screenshot", "weird_name_x_2"], catalog.Aliases); // ordinal order
Assert.Contains("mcp__volcanic__browser_navigate", catalog.TransportNames);
Assert.DoesNotContain(catalog.Aliases, a => a.StartsWith("tool_", StringComparison.Ordinal));
}
}

View File

@@ -0,0 +1,134 @@
using Volcanic.AgenticCore.Abstractions.Processes;
using Volcanic.AgenticCore.Extensions.Mcp;
namespace Volcanic.AgenticCore.Extensions.Tests.Mcp;
public class McpLaunchTests
{
private const string Ws = "/work/chat-1";
private static Func<string, string?> Env(params (string Name, string Value)[] vars)
{
var map = vars.ToDictionary(v => v.Name, v => v.Value);
return name => map.TryGetValue(name, out var value) ? value : null;
}
[Fact]
public void BuildRequest_expands_the_workspace_in_command_args_cwd_and_env()
{
var config = new McpServerConfig
{
Command = "${workspace}/bin/server",
Args = ["--out", "${workspace}/out", "--literal", "${env:HOME}"],
Cwd = "${workspace}/sub",
Env = { ["DATA"] = "${workspace}/data" },
};
var request = McpLaunch.BuildRequest("srv", config, Ws, Env(("HOME", "/home/me")));
Assert.Equal(ProcessPurpose.McpServer, request.Purpose);
Assert.Equal("srv", request.Origin);
Assert.Equal("/work/chat-1/bin/server", request.Command);
Assert.Equal(["--out", "/work/chat-1/out", "--literal", "${env:HOME}"], request.Arguments);
Assert.Equal("/work/chat-1/sub", request.Cwd);
Assert.Equal("/work/chat-1/data", request.Environment!["DATA"]);
Assert.Null(request.TimeZoneId);
Assert.Null(request.Scope);
}
[Fact]
public void BuildRequest_defaults_the_cwd_to_the_workspace_and_sends_no_environment_without_env_entries()
{
var request = McpLaunch.BuildRequest("srv", new McpServerConfig { Command = "npx" }, Ws, Env());
Assert.Equal(Ws, request.Cwd);
Assert.Null(request.Environment);
Assert.Empty(request.Arguments);
}
[Fact]
public void Env_references_expand_from_the_host_and_unset_ones_become_empty()
{
var config = new McpServerConfig
{
Command = "x",
Env = { ["GITHUB_TOKEN"] = "${env:GITHUB_TOKEN}", ["MISSING"] = "a${env:NOPE}b", ["MIXED"] = "${workspace}:${env:A}" },
};
var env = McpLaunch.BuildRequest("srv", config, Ws, Env(("GITHUB_TOKEN", "ghp_secret_value_1"), ("A", "1"))).Environment!;
Assert.Equal("ghp_secret_value_1", env["GITHUB_TOKEN"]);
Assert.Equal("ab", env["MISSING"]);
Assert.Equal("/work/chat-1:1", env["MIXED"]);
}
[Fact]
public void Null_env_values_are_kept_to_remove_variables()
{
var config = new McpServerConfig { Command = "x", Env = { ["DROP_ME"] = null } };
var env = McpLaunch.BuildRequest("srv", config, Ws, Env()).Environment!;
Assert.True(env.ContainsKey("DROP_ME"));
Assert.Null(env["DROP_ME"]);
}
[Fact]
public void Transport_options_replace_the_environment_instead_of_merging()
{
var spawn = new ProcessSpawnContext(ProcessPurpose.McpServer, "cmd", ["a", "b"], "/cwd",
new Dictionary<string, string> { ["PATH"] = "/bin" }, "scope", "sid", "srv");
var lines = new List<string>();
var options = McpLaunch.TransportOptions("srv", spawn, lines.Add);
Assert.False(options.InheritEnvironmentVariables);
Assert.Equal("srv", options.Name);
Assert.Equal("cmd", options.Command);
Assert.Equal(["a", "b"], options.Arguments!);
Assert.Equal("/cwd", options.WorkingDirectory);
var only = Assert.Single(options.EnvironmentVariables!);
Assert.Equal(("PATH", "/bin"), (only.Key, only.Value));
Assert.Equal(McpLaunch.ShutdownTimeout, options.ShutdownTimeout);
Assert.NotNull(options.StandardErrorLines);
options.StandardErrorLines("oops");
Assert.Equal(["oops"], lines);
}
[Fact]
public void Cache_key_uses_the_unexpanded_configuration_only()
{
McpServerConfig Make() => new() { Command = "npx", Args = ["${workspace}/x"], Env = { ["GITHUB_TOKEN"] = "${env:GITHUB_TOKEN}" } };
var key = McpLaunch.CacheKey("srv", Make());
Assert.StartsWith("srv:", key, StringComparison.Ordinal);
Assert.Equal(key, McpLaunch.CacheKey("srv", Make()));
Assert.DoesNotContain("GITHUB", key, StringComparison.Ordinal);
Assert.NotEqual(key, McpLaunch.CacheKey("other", Make()));
}
public static TheoryData<string> Mutations => new() { "command", "args", "env", "cwd", "prefix" };
[Theory]
[MemberData(nameof(Mutations))]
public void Cache_key_changes_with_command_args_env_cwd_or_prefix(string what)
{
McpServerConfig Make() => new() { Command = "npx", Args = ["a"], Env = { ["K"] = "v" } };
var changed = Make();
switch (what)
{
case "command": changed.Command = "node"; break;
case "args": changed.Args.Add("b"); break;
case "env": changed.Env["K"] = "w"; break;
case "cwd": changed.Cwd = "/elsewhere"; break;
case "prefix": changed.ToolPrefix = "p_"; break;
}
Assert.NotEqual(McpLaunch.CacheKey("s", Make()), McpLaunch.CacheKey("s", changed));
}
[Fact]
public void Stderr_tail_keeps_the_last_20_lines()
{
var tail = new StderrTail();
for (var i = 1; i <= 25; i++) tail.Add($"line {i}");
Assert.Equal(20, tail.Lines.Count);
Assert.Equal("line 6", tail.Lines[0]);
Assert.Equal("line 25", tail.Lines[^1]);
tail.Clear();
Assert.Empty(tail.Lines);
}
}

View File

@@ -0,0 +1,43 @@
using Volcanic.AgenticCore.Extensions.Mcp;
namespace Volcanic.AgenticCore.Extensions.Tests.Mcp;
public class ToolNamingTests
{
[Fact]
public void Valid_names_are_unchanged() => Assert.Equal("browser_navigate", ToolNaming.Sanitize(null, "browser_navigate"));
[Fact]
public void Invalid_characters_become_underscores()
{
Assert.Equal("weird_name_x", ToolNaming.Sanitize(null, "weird.name/x"));
Assert.Equal("a_b_c-d", ToolNaming.Sanitize(null, "a b:c-d"));
}
[Fact]
public void Prefix_is_prepended() => Assert.Equal("gh_create_issue", ToolNaming.Sanitize("gh_", "create_issue"));
[Fact]
public void Names_are_truncated_to_48_characters()
{
Assert.Equal(new string('a', 48), ToolNaming.Sanitize(null, new string('a', 60)));
var prefixed = ToolNaming.Sanitize("p_", new string('b', 60));
Assert.Equal(48, prefixed.Length);
Assert.StartsWith("p_bbb", prefixed, StringComparison.Ordinal);
}
[Fact]
public void Duplicates_get_numeric_suffixes_within_the_limit()
{
Assert.Equal(["weird_name_x", "weird_name_x_2", "weird_name_x_3"],
ToolNaming.Expose(["weird.name/x", "weird_name_x", "weird:name:x"], null));
var longNames = ToolNaming.Expose([new string('a', 50), new string('a', 49)], null);
Assert.Equal(new string('a', 48), longNames[0]);
Assert.Equal(new string('a', 46) + "_2", longNames[1]);
Assert.All(longNames, n => Assert.True(n.Length <= ToolNaming.MaxLength));
}
[Fact]
public void Empty_name_falls_back_to_tool() => Assert.Equal("tool", ToolNaming.Sanitize(null, ""));
}