forked from admin/VolcanicAgenticCore
feat(mcp): process-wide tool catalogue discovery
McpProcess starts a server through the SDK stdio client under the startup timeout and keeps a stderr tail for its errors. McpCatalogCache runs one shared discovery per server config in a temp dir, with the environment from the process spawn policy. It keeps successes for the process lifetime and failures for 60 s. Deviation from the plan: the discovery process is disposed and its temp dir deleted in the background, after the catalogue is returned. SDK 2.2 waits the whole ShutdownTimeout (5 s) with stdin still open before it kills the process, so an awaited dispose held every first activation for 5 s. A new test (Discovery_returns_without_waiting_for_the_server_shutdown) covers this. Spec section 3 gets the new GetAsync signature, the detached discovery and the background cleanup. Section 5 now describes the SDK's shutdown order correctly.
This commit is contained in:
EugeneTes
@@ -0,0 +1,180 @@
|
||||
using System.Diagnostics;
|
||||
using System.Text.Json;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Time.Testing;
|
||||
using Volcanic.AgenticCore.Abstractions.Processes;
|
||||
using Volcanic.AgenticCore.Extensions.Mcp;
|
||||
using Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer;
|
||||
using Volcanic.AgenticCore.Extensions.Tests.Mcp.Support;
|
||||
|
||||
namespace Volcanic.AgenticCore.Extensions.Tests.Mcp;
|
||||
|
||||
public class McpCatalogCacheTests : IDisposable
|
||||
{
|
||||
private readonly string _dir = Path.Combine(Path.GetTempPath(), "volcanic-mcp-catalog", Guid.NewGuid().ToString("N"));
|
||||
private readonly Dictionary<string, string?> _env = McpTestEnvironment.Snapshot();
|
||||
private readonly FakeTimeProvider _time = new();
|
||||
private readonly RecordingLogger _logger = new();
|
||||
private string LogPath => Path.Combine(_dir, "fake.log");
|
||||
|
||||
public McpCatalogCacheTests() => Directory.CreateDirectory(_dir);
|
||||
public void Dispose() { try { Directory.Delete(_dir, true); } catch (IOException) { } }
|
||||
|
||||
private Task<McpCatalog> Discover(McpCatalogCache cache, McpServerConfig config, IProcessSpawnPolicy? policy = null) =>
|
||||
cache.GetAsync("fake", config, policy ?? McpTestEnvironment.Policy(_env), McpTestEnvironment.Lookup(_env), _logger, CancellationToken.None);
|
||||
|
||||
[Fact]
|
||||
public async Task Discovery_lists_tools_with_names_descriptions_and_schemas_verbatim()
|
||||
{
|
||||
var catalog = await Discover(new McpCatalogCache(_time), FakeMcp.Server());
|
||||
|
||||
Assert.Equal("fake", catalog.ServerName);
|
||||
Assert.Equal("fake-mcp", catalog.ServerInfoName);
|
||||
Assert.Equal("1.2.3", catalog.ServerVersion);
|
||||
var echo = catalog.Tools.Single(t => t.McpName == "echo");
|
||||
Assert.Equal("echo", echo.ExposedName);
|
||||
Assert.Equal("Echoes text back", echo.Description);
|
||||
Assert.True(JsonElement.DeepEquals(JsonSerializer.Deserialize<JsonElement>(FakeServerProgram.EchoSchema), echo.InputSchema));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Discovery_sanitises_truncates_and_suffixes_names()
|
||||
{
|
||||
var catalog = await Discover(new McpCatalogCache(_time), FakeMcp.Server());
|
||||
|
||||
Assert.Equal("weird_name_x", catalog.Tools.Single(t => t.McpName == "weird.name/x").ExposedName);
|
||||
Assert.Equal("weird_name_x_2", catalog.Tools.Single(t => t.McpName == "weird_name_x").ExposedName);
|
||||
Assert.Equal(FakeServerProgram.LongToolName[..48], catalog.Tools.Single(t => t.McpName == FakeServerProgram.LongToolName).ExposedName);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Discovery_applies_the_prefix()
|
||||
{
|
||||
var catalog = await Discover(new McpCatalogCache(_time), FakeMcp.Server(configure: c => c.ToolPrefix = "fk_"));
|
||||
Assert.Equal("fk_echo", catalog.Tools.Single(t => t.McpName == "echo").ExposedName);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Tool_without_input_schema_arrives_as_an_object_schema()
|
||||
{
|
||||
var catalog = await Discover(new McpCatalogCache(_time), FakeMcp.Server());
|
||||
var schema = catalog.Tools.Single(t => t.McpName == "noschema").InputSchema;
|
||||
Assert.Equal(JsonValueKind.Object, schema.ValueKind);
|
||||
Assert.Equal("object", schema.GetProperty("type").GetString());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Concurrent_requests_share_one_discovery()
|
||||
{
|
||||
var cache = new McpCatalogCache(_time);
|
||||
var config = FakeMcp.Server("count-starts", LogPath);
|
||||
var catalogs = await Task.WhenAll(Enumerable.Range(0, 10).Select(_ => Task.Run(() => Discover(cache, config))));
|
||||
|
||||
Assert.Single(FakeMcp.Starts(LogPath));
|
||||
Assert.All(catalogs, c => Assert.Same(catalogs[0], c));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Success_is_cached_for_the_process_lifetime()
|
||||
{
|
||||
var cache = new McpCatalogCache(_time);
|
||||
await Discover(cache, FakeMcp.Server("count-starts", LogPath));
|
||||
_time.Advance(TimeSpan.FromDays(1));
|
||||
await Discover(cache, FakeMcp.Server("count-starts", LogPath)); // an equal config, a new instance
|
||||
|
||||
Assert.Single(FakeMcp.Starts(LogPath));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Crash_on_start_is_cached_for_the_cooldown_with_the_stderr_traceback()
|
||||
{
|
||||
var cache = new McpCatalogCache(_time);
|
||||
var config = FakeMcp.Server("crash-on-start,count-starts", LogPath);
|
||||
|
||||
var first = await Assert.ThrowsAsync<McpServerStartException>(() => Discover(cache, config));
|
||||
Assert.Contains("Traceback (most recent call last):", first.Message, StringComparison.Ordinal);
|
||||
Assert.Contains("RuntimeError: fake crash on start", first.Message, StringComparison.Ordinal);
|
||||
|
||||
_time.Advance(McpCatalogCache.FailureCooldown - TimeSpan.FromSeconds(1));
|
||||
var second = await Assert.ThrowsAsync<McpServerStartException>(() => Discover(cache, config));
|
||||
Assert.Equal(first.Message, second.Message);
|
||||
Assert.Single(FakeMcp.Starts(LogPath));
|
||||
Assert.True(_logger.Has(LogLevel.Warning, "fake"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Failure_is_retried_after_the_cooldown()
|
||||
{
|
||||
var cache = new McpCatalogCache(_time);
|
||||
var config = FakeMcp.Server("crash-on-start,count-starts", LogPath);
|
||||
await Assert.ThrowsAsync<McpServerStartException>(() => Discover(cache, config));
|
||||
|
||||
_time.Advance(McpCatalogCache.FailureCooldown);
|
||||
await Assert.ThrowsAsync<McpServerStartException>(() => Discover(cache, config));
|
||||
Assert.Equal(2, FakeMcp.Starts(LogPath).Count);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Hang_on_start_gives_up_after_the_startup_timeout()
|
||||
{
|
||||
var config = FakeMcp.Server("hang-on-start,count-starts", LogPath, c => c.StartupTimeoutSeconds = 1);
|
||||
var watch = Stopwatch.StartNew();
|
||||
|
||||
var ex = await Assert.ThrowsAsync<McpServerStartException>(() => Discover(new McpCatalogCache(_time), config));
|
||||
|
||||
Assert.Contains("did not start within 1s", ex.Message, StringComparison.Ordinal);
|
||||
Assert.True(watch.Elapsed < TimeSpan.FromSeconds(15), $"took {watch.Elapsed}");
|
||||
var pid = Assert.Single(FakeMcp.Starts(LogPath)).GetProperty("pid").GetInt32();
|
||||
await FakeMcp.WaitForAsync(() => FakeMcp.HasExited(pid), "the hung server to be stopped");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Discovery_runs_in_a_temp_directory_that_is_deleted_and_its_process_stops()
|
||||
{
|
||||
await Discover(new McpCatalogCache(_time), FakeMcp.Server("count-starts", LogPath));
|
||||
|
||||
var start = Assert.Single(FakeMcp.Starts(LogPath));
|
||||
var cwd = start.GetProperty("cwd").GetString()!;
|
||||
Assert.Contains("volcanic-mcp-discovery", cwd, StringComparison.Ordinal);
|
||||
var pid = start.GetProperty("pid").GetInt32();
|
||||
await FakeMcp.WaitForAsync(() => FakeMcp.HasExited(pid), "the discovery process to exit");
|
||||
await FakeMcp.WaitForAsync(() => !Directory.Exists(cwd), "the discovery directory to be deleted");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Discovery_returns_without_waiting_for_the_server_shutdown()
|
||||
{
|
||||
// SDK 2.2 waits ShutdownTimeout with stdin still open before it kills the process, so an awaited dispose
|
||||
// would hold every first activation for that long.
|
||||
var watch = Stopwatch.StartNew();
|
||||
await Discover(new McpCatalogCache(_time), FakeMcp.Server("count-starts", LogPath));
|
||||
|
||||
Assert.True(watch.Elapsed < McpLaunch.ShutdownTimeout, $"took {watch.Elapsed}");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Discovery_prepares_the_environment_through_the_policy()
|
||||
{
|
||||
ProcessSpawnContext? seen = null;
|
||||
var policy = McpTestEnvironment.Policy(_env, o => o.Hooks.Add(c =>
|
||||
{
|
||||
seen = c;
|
||||
return c with { Environment = new Dictionary<string, string>(c.Environment) { ["FAKE_HOOK"] = "from-hook" } };
|
||||
}));
|
||||
await Discover(new McpCatalogCache(_time), FakeMcp.Server("env,count-starts", LogPath), policy);
|
||||
|
||||
Assert.NotNull(seen);
|
||||
Assert.Equal(ProcessPurpose.McpServer, seen!.Purpose);
|
||||
Assert.Equal("fake", seen.Origin);
|
||||
Assert.Equal("from-hook", Assert.Single(FakeMcp.Starts(LogPath)).GetProperty("env").GetProperty("FAKE_HOOK").GetString());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Policy_failure_fails_discovery_with_its_message()
|
||||
{
|
||||
var policy = McpTestEnvironment.Policy(_env, o => o.Hooks.Add(_ => throw new InvalidOperationException("boom")));
|
||||
var ex = await Assert.ThrowsAsync<McpServerStartException>(() => Discover(new McpCatalogCache(_time), FakeMcp.Server(), policy));
|
||||
Assert.Contains("process environment hook failed:", ex.Message, StringComparison.Ordinal);
|
||||
Assert.Contains("boom", ex.Message, StringComparison.Ordinal);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,29 @@
|
||||
using System.Collections;
|
||||
using Volcanic.AgenticCore.Features.Processes;
|
||||
|
||||
namespace Volcanic.AgenticCore.Extensions.Tests.Mcp.Support;
|
||||
|
||||
/// <summary>Injected host environments: a copy of the real one (so <c>dotnet</c> starts) without the variables the tests control.</summary>
|
||||
internal static class McpTestEnvironment
|
||||
{
|
||||
private static readonly string[] Controlled = ["FAKE_MCP_MODE", "FAKE_MCP_LOG", "FAKE_MCP_DIE_MESSAGE", "OPENAI_API_KEY", "GITHUB_TOKEN", "VOLCANIC_AGENT_DIR", "TZ"];
|
||||
|
||||
public static Dictionary<string, string?> Snapshot(params (string Name, string Value)[] extra)
|
||||
{
|
||||
var env = new Dictionary<string, string?>(OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal);
|
||||
foreach (DictionaryEntry e in Environment.GetEnvironmentVariables()) env[(string)e.Key] = (string?)e.Value;
|
||||
foreach (var name in Controlled) env.Remove(name);
|
||||
foreach (var (name, value) in extra) env[name] = value;
|
||||
return env;
|
||||
}
|
||||
|
||||
public static ProcessSpawnPolicy Policy(IReadOnlyDictionary<string, string?> env, Action<ProcessEnvironmentOptions>? configure = null)
|
||||
{
|
||||
var options = new ProcessEnvironmentOptions { HardenHostProcess = false };
|
||||
configure?.Invoke(options);
|
||||
return new ProcessSpawnPolicy(options, null, () => env);
|
||||
}
|
||||
|
||||
public static Func<string, string?> Lookup(IReadOnlyDictionary<string, string?> env) =>
|
||||
name => env.TryGetValue(name, out var value) ? value : null;
|
||||
}
|
||||
@@ -10,6 +10,7 @@
|
||||
<PackageReference Include="xunit.runner.visualstudio" Version="3.1.1" />
|
||||
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="10.0.0" />
|
||||
<PackageReference Include="Microsoft.Extensions.Logging" Version="10.0.0" />
|
||||
<PackageReference Include="Microsoft.Extensions.TimeProvider.Testing" Version="10.0.0" />
|
||||
</ItemGroup>
|
||||
<ItemGroup>
|
||||
<None Include="ClaudeCode/Fixtures/*" CopyToOutputDirectory="PreserveNewest" />
|
||||
|
||||
Reference in New Issue
Block a user