From 35b7cce484db8c65c0ad551147d0253514350918 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 9 Aug 2026 03:18:53 +0000 Subject: [PATCH 1/2] test(mcp): assert smoke harness connectivity deterministically StartAsync awaits the whole connect attempt, so by the time it returns the manager has either published tools to the registry or recorded a failure status with the underlying error message. Asserting Connected via GetServerStatuses() turns an intermittent Windows CI connect failure (previously a bare Assert.NotNull null on the tool lookup in SmokeMcpServerHttpHeaderTests) into a failure that names the real error, instead of silently swallowing it. --- .../Mcp/McpSmokeHarness.cs | 20 +++++++++++++ .../Mcp/SmokeMcpServerHttpHeaderTests.cs | 28 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs b/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs index cab29ce03..e68ee35c2 100644 --- a/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs +++ b/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs @@ -9,6 +9,7 @@ using Netclaw.Configuration.Secrets; using Netclaw.Daemon.Mcp; using Netclaw.Tools; +using Xunit; namespace Netclaw.Daemon.Tests.Mcp; @@ -31,6 +32,25 @@ 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) diff --git a/src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs b/src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs index 153f0b11a..8b4328d98 100644 --- a/src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs +++ b/src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs @@ -57,6 +57,13 @@ public async Task ConfiguredHeader_IsAttachedToOutboundMcpRequest() new Dictionary { ["smoke-http"] = entry }, registry); 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) @@ -93,6 +100,13 @@ public async Task Netclaw_user_agent_and_component_headers_are_attached_to_mcp_r new Dictionary { ["smoke-http"] = entry }, registry); 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) @@ -141,6 +155,13 @@ public async Task NoConfiguredHeader_ResultsInNoAuthorizationHeaderOnTheWire() new Dictionary { ["smoke-http"] = entry }, registry); 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) @@ -188,6 +209,13 @@ public async Task ConfiguredHeader_WhenOAuthProbeReturnsMetadata_StillReachesSer new Dictionary { ["smoke-http"] = entry }, registry); 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) From 36d53c8820ca1025134c49f2fb5731f1f8b0348e Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 9 Aug 2026 11:43:04 +0000 Subject: [PATCH 2/2] test(mcp): surface connect failure exceptions in smoke tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the McpClientManager logger to xunit test output in the HTTP smoke harness. ReportConnectionFailure logs the full exception, but NullLogger discarded it — the tests only ever saw the generic 'Failed to reach MCP server' status message. On the intermittent Windows connect flake, this now prints the underlying exception to the CI log so the root cause can be fixed precisely instead of guessed. --- .../Mcp/McpSmokeHarness.cs | 42 ++++++++++++++++++- .../Mcp/SmokeMcpServerHttpHeaderTests.cs | 16 +++++-- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs b/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs index e68ee35c2..31907ac9c 100644 --- a/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs +++ b/src/Netclaw.Daemon.Tests/Mcp/McpSmokeHarness.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Netclaw.Actors.Tools; using Netclaw.Configuration; @@ -53,7 +54,8 @@ public void AssertConnected(string serverName) 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(); @@ -74,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 8b4328d98..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,7 +58,8 @@ 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 @@ -97,7 +102,8 @@ 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 @@ -152,7 +158,8 @@ 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 @@ -206,7 +213,8 @@ 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