diff --git a/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs b/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs index cab29ce03..31907ac9c 100644 --- a/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs +++ b/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs @@ -3,12 +3,14 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Netclaw.Actors.Tools; using Netclaw.Configuration; using Netclaw.Configuration.Secrets; using Netclaw.Daemon.Mcp; using Netclaw.Tools; +using Xunit; namespace Netclaw.Daemon.Tests.Mcp; @@ -31,9 +33,29 @@ private McpSmokeHarness(McpClientManager manager, McpOAuthFlowBroker flowBroker) public McpClientManager Manager { get; } + /// + /// Asserts that the named MCP server reached the + /// state after completed. `StartAsync` awaits the whole + /// connect attempt — either tools are published to the registry or a failure status with + /// the underlying error is published — so there is nothing to poll: this is the + /// deterministic completion signal. Asserting on it turns an intermittent Windows CI + /// connect failure (previously a bare `Assert.NotNull` null on the tool lookup) into a + /// failure that carries the manager's actual error message. + /// + public void AssertConnected(string serverName) + { + var status = Manager.GetServerStatuses().GetValueOrDefault(new McpServerName(serverName)); + Assert.NotNull(status); + Assert.True( + status.State is McpConnectionState.Connected, + $"MCP server '{serverName}' failed to connect: state={status.State}, " + + $"error={status.ErrorMessage ?? "(none)"}"); + } + public static McpSmokeHarness Create( Dictionary serverEntries, - ToolRegistry registry) + ToolRegistry registry, + ITestOutputHelper? output = null) { var paths = new NetclawPaths(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString())); paths.EnsureDirectoriesExist(); @@ -54,11 +76,47 @@ public static McpSmokeHarness Create( NullNotificationSink.Instance, TimeProvider.System, new McpClientRuntime(), - NullLogger.Instance, + // Real logger wired to test output: when a connect fails the manager + // logs the full exception via ReportConnectionFailure, and NullLogger + // was discarding it — leaving only the generic status ErrorMessage. + output is null + ? NullLogger.Instance + : new TestOutputLogger(output), new SessionConfig()); return new McpSmokeHarness(manager, flowBroker); } + /// + /// Minimal that forwards to xunit test output so + /// the manager's own diagnostics (including the full connect-failure + /// exception) show up in the CI log when a smoke test fails. + /// + private sealed class TestOutputLogger : ILogger + { + private readonly ITestOutputHelper _output; + + public TestOutputLogger(ITestOutputHelper output) => _output = output; + + public IDisposable? BeginScope(TState state) + where TState : notnull + => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + var message = formatter(state, exception); + _output.WriteLine($"[{logLevel}] {message}"); + if (exception is not null) + _output.WriteLine(exception.ToString()); + } + } + public async ValueTask DisposeAsync() { await Manager.StopAsync(CancellationToken.None); diff --git a/src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs b/src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs index 153f0b11a..94dd159f8 100644 --- a/src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs +++ b/src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs @@ -31,6 +31,10 @@ namespace Netclaw.Daemon.Tests.Mcp; /// public sealed class SmokeMcpServerHttpHeaderTests { + private readonly ITestOutputHelper _output; + + public SmokeMcpServerHttpHeaderTests(ITestOutputHelper output) => _output = output; + [Fact] public async Task ConfiguredHeader_IsAttachedToOutboundMcpRequest() { @@ -54,9 +58,17 @@ public async Task ConfiguredHeader_IsAttachedToOutboundMcpRequest() var registry = new ToolRegistry(); await using var harness = McpSmokeHarness.Create( - new Dictionary { ["smoke-http"] = entry }, registry); + new Dictionary { ["smoke-http"] = entry }, registry, + _output); await harness.Manager.StartAsync(ct); + // Deterministic completion signal: StartAsync awaits the whole connect + // attempt, so by the time it returns the manager has either published + // tools or recorded the failure with its real error message. Asserting + // Connected here turns an intermittent Windows CI connect failure into + // a failure that names the underlying error instead of a bare null on + // the tool lookup below. + harness.AssertConnected("smoke-http"); var lastAuthHeader = registry.GetAllRegistrations() .Select(r => r.Tool) @@ -90,9 +102,17 @@ public async Task Netclaw_user_agent_and_component_headers_are_attached_to_mcp_r var registry = new ToolRegistry(); await using var harness = McpSmokeHarness.Create( - new Dictionary { ["smoke-http"] = entry }, registry); + new Dictionary { ["smoke-http"] = entry }, registry, + _output); await harness.Manager.StartAsync(ct); + // Deterministic completion signal: StartAsync awaits the whole connect + // attempt, so by the time it returns the manager has either published + // tools or recorded the failure with its real error message. Asserting + // Connected here turns an intermittent Windows CI connect failure into + // a failure that names the underlying error instead of a bare null on + // the tool lookup below. + harness.AssertConnected("smoke-http"); var lastUserAgent = registry.GetAllRegistrations() .Select(r => r.Tool) @@ -138,9 +158,17 @@ public async Task NoConfiguredHeader_ResultsInNoAuthorizationHeaderOnTheWire() var registry = new ToolRegistry(); await using var harness = McpSmokeHarness.Create( - new Dictionary { ["smoke-http"] = entry }, registry); + new Dictionary { ["smoke-http"] = entry }, registry, + _output); await harness.Manager.StartAsync(ct); + // Deterministic completion signal: StartAsync awaits the whole connect + // attempt, so by the time it returns the manager has either published + // tools or recorded the failure with its real error message. Asserting + // Connected here turns an intermittent Windows CI connect failure into + // a failure that names the underlying error instead of a bare null on + // the tool lookup below. + harness.AssertConnected("smoke-http"); var lastAuthHeader = registry.GetAllRegistrations() .Select(r => r.Tool) @@ -185,9 +213,17 @@ public async Task ConfiguredHeader_WhenOAuthProbeReturnsMetadata_StillReachesSer var registry = new ToolRegistry(); await using var harness = McpSmokeHarness.Create( - new Dictionary { ["smoke-http"] = entry }, registry); + new Dictionary { ["smoke-http"] = entry }, registry, + _output); await harness.Manager.StartAsync(ct); + // Deterministic completion signal: StartAsync awaits the whole connect + // attempt, so by the time it returns the manager has either published + // tools or recorded the failure with its real error message. Asserting + // Connected here turns an intermittent Windows CI connect failure into + // a failure that names the underlying error instead of a bare null on + // the tool lookup below. + harness.AssertConnected("smoke-http"); var lastAuthHeader = registry.GetAllRegistrations() .Select(r => r.Tool)