Reconnect MCP clients only for transport and session failures (#2056)

openspec/changes/mcp-tool-outcome-receipts/tasks.md

@@ -10,8 +10,8 @@
 
 ## 3. Reconnect classification (#2056)
 
-- [ ] 3.1 Make `IsTransportOrSessionFailure` return true for `HttpRequestException` only when `StatusCode` is null or 404; verify lifecycle tests: HTTP 500 and 429 produce no reconnect and an unchanged generation; HTTP 404 and a status-less failure each produce one reconnect.
-- [ ] 3.2 Widen the first catch clause in `McpClientManager.LoadAsync` to `McpException` or `HttpRequestException` when not a transport failure; verify a `McpPromptSkillTests` test that an HTTP 500 on `GetPromptAsync` returns a failed load result that names the prompt, with no reconnect.
+- [x] 3.1 Make `IsTransportOrSessionFailure` return true for `HttpRequestException` only when `StatusCode` is null or 404; verify lifecycle tests: HTTP 500 and 429 produce no reconnect and an unchanged generation; HTTP 404 and a status-less failure each produce one reconnect.
+- [x] 3.2 Widen the first catch clause in `McpClientManager.LoadAsync` to `McpException` or `HttpRequestException` when not a transport failure; verify a `McpPromptSkillTests` test that an HTTP 500 on `GetPromptAsync` returns a failed load result that names the prompt, with no reconnect.
 
 ## 4. Auth guard for servers that cannot use OAuth (#2057)
 

src/Netclaw.Daemon.Tests/Mcp/McpClientManagerLifecycleTests.cs

@@ -372,6 +372,68 @@ public async Task TransportFailure_ReconnectsForLaterCallsAndDoesNotReplay()
         Assert.Equal(2, harness.Manager.GetSnapshot(ServerName)?.Generation);
     }
 
+    [Theory]
+    [InlineData(HttpStatusCode.InternalServerError)]
+    [InlineData(HttpStatusCode.TooManyRequests)]
+    public async Task ApplicationHttpStatus_ReachesTheCallerWithoutAReconnect(HttpStatusCode status)
+    {
+        var runtime = new ControlledMcpClientRuntime();
+        var plan = runtime.Enqueue(new ClientPlan("run")
+        {
+            Invoke = (_, _) => Task.FromException<object?>(
+                new HttpRequestException("boom", null, status)),
+        });
+        // A replacement is queued but must stay unused. Without it a reconnect would fail
+        // inside the runtime before it counts the client, and CreateCount would still
+        // read 1. With it, one reconnect drives CreateCount to 2 and this test fails.
+        var replacement = runtime.Enqueue(new ClientPlan("run"));
+        await using var harness = CreateHarness(runtime);
+        await harness.Manager.StartAsync(TestContext.Current.CancellationToken);
+
+        var error = await Assert.ThrowsAsync<HttpRequestException>(
+            () => InvokeAsync(harness.Manager, TestContext.Current.CancellationToken));
+
+        // A server that answers with a status is reachable. A new session cannot change the
+        // answer, and each reconnect costs about five more requests against a server that
+        // may already enforce a request budget.
+        Assert.Equal(status, error.StatusCode);
+        // The Warning line is the only operator-visible record of a thrown tool call, so it
+        // must name the tool and the status the server sent.
+        var warning = Assert.Single(
+            harness.Logger.Entries,
+            entry => entry.Contains($"{ServerName.Value}/run", StringComparison.Ordinal));
+        Assert.Contains($"(HTTP {(int)status})", warning, StringComparison.Ordinal);
+        Assert.Equal(1, runtime.CreateCount);
+        Assert.Equal(0, plan.DisposeCount);
+        Assert.Null(replacement.Client);
+        Assert.Equal(1, harness.Manager.GetSnapshot(ServerName)?.Generation);
+    }
+
+    [Fact]
+    public async Task SessionExpiryStatus_ReconnectsForLaterCalls()
+    {
+        var runtime = new ControlledMcpClientRuntime();
+        var initial = runtime.Enqueue(new ClientPlan("run")
+        {
+            Invoke = (_, _) => Task.FromException<object?>(
+                new HttpRequestException("session expired", null, HttpStatusCode.NotFound)),
+        });
+        var replacement = runtime.Enqueue(new ClientPlan("run"));
+        await using var harness = CreateHarness(runtime);
+        await harness.Manager.StartAsync(TestContext.Current.CancellationToken);
+
+        var error = await Assert.ThrowsAsync<HttpRequestException>(
+            () => InvokeAsync(harness.Manager, TestContext.Current.CancellationToken));
+
+        // Streamable HTTP reports an expired session as 404, so a new session repairs it.
+        Assert.Equal(HttpStatusCode.NotFound, error.StatusCode);
+        // The token makes a missed reconnect fail the run instead of hanging it.
+        await initial.Disposed.Task.WaitAsync(TestContext.Current.CancellationToken);
+        Assert.Equal(2, runtime.CreateCount);
+        Assert.Equal(0, replacement.InvocationCount);
+        Assert.Equal(2, harness.Manager.GetSnapshot(ServerName)?.Generation);
+    }
+
     [Fact]
     public async Task CandidateInitializationAndDisposalFailures_AreLoudAndPriorToolsRemainPublished()
     {
@@ -738,6 +800,8 @@ public ValueTask<GetPromptResult> GetPromptAsync(
             Interlocked.Increment(ref plan.PromptInvocationCountStorage);
             plan.LastPromptName = promptName;
             plan.LastPromptArguments = new Dictionary<string, string>(arguments, StringComparer.Ordinal);
+            if (plan.GetPromptFailure is not null)
+                return ValueTask.FromException<GetPromptResult>(plan.GetPromptFailure);
             return plan.GetPromptResult is null
                 ? ValueTask.FromException<GetPromptResult>(
                     new InvalidOperationException("The controlled prompt result is not configured."))
@@ -805,6 +869,8 @@ internal sealed class ClientPlan(params string[] toolNames)
 
         public Exception? PromptListFailure { get; set; }
 
+        public Exception? GetPromptFailure { get; set; }
+
         public string? LastPromptName { get; set; }
 
         public IReadOnlyDictionary<string, string>? LastPromptArguments { get; set; }

src/Netclaw.Daemon.Tests/Mcp/McpPromptSkillTests.cs

@@ -3,6 +3,7 @@
 //      Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com>
 // </copyright>
 // -----------------------------------------------------------------------
+using System.Net;
 using Microsoft.Extensions.Time.Testing;
 using ModelContextProtocol.Protocol;
 using Netclaw.Actors.Skills;
@@ -239,6 +240,46 @@ public async Task LoadRejectsUnsupportedPromptContent()
         Assert.Contains("unsupported content type 'image'", result.Error, StringComparison.Ordinal);
     }
 
+    [Fact]
+    public async Task LoadReturnsFailedResultForAnApplicationHttpStatus()
+    {
+        var runtime = new McpClientManagerLifecycleTests.ControlledMcpClientRuntime();
+        var plan = CreatePromptPlan();
+        plan.GetPromptFailure = new HttpRequestException(
+            "boom",
+            null,
+            HttpStatusCode.InternalServerError);
+        runtime.Enqueue(plan);
+        // A replacement is queued but must stay unused. Without it a reconnect would fail
+        // inside the runtime before it counts the client, and CreateCount would still
+        // read 1. With it, one reconnect drives CreateCount to 2 and this test fails.
+        var replacement = runtime.Enqueue(CreatePromptPlan());
+        await using var harness = new McpClientManagerLifecycleTests.ManagerHarness(
+            runtime,
+            new FakeTimeProvider(InitialTime));
+        await harness.Manager.StartAsync(TestContext.Current.CancellationToken);
+        var source = Assert.IsType<McpPromptSkillSource>(
+            harness.SkillRegistry.GetByName("mcp__test__analyze-property")?.Source);
+
+        var result = await harness.Manager.LoadAsync(
+            source,
+            new Dictionary<string, string> { ["property"] = "petabridge-com" },
+            TestToolExecutionContext.CreateUnbound(TrustAudience.Personal).Invocation,
+            TestContext.Current.CancellationToken);
+
+        // The load owns this failure. An escaped exception would reach the tool dispatcher,
+        // and a reconnect cannot change a status the server chose to send.
+        Assert.False(result.Success);
+        Assert.Contains("analyze-property", result.Error, StringComparison.Ordinal);
+        // The first catch clause owns this text. The transport clause says "connection
+        // closed" instead, so this pins which clause ran.
+        Assert.Contains("failed:", result.Error, StringComparison.Ordinal);
+        Assert.DoesNotContain("connection closed", result.Error, StringComparison.Ordinal);
+        Assert.Equal(1, runtime.CreateCount);
+        Assert.Null(replacement.Client);
+        Assert.Equal(1, harness.Manager.GetSnapshot(ServerName)?.Generation);
+    }
+
     private static McpClientManagerLifecycleTests.ClientPlan CreatePromptPlan()
         => new("query")
         {

src/Netclaw.Daemon/Mcp/McpClientManager.cs

@@ -538,7 +538,7 @@ public async ValueTask<McpPromptSkillLoadResult> LoadAsync(
                 suppliedArguments,
                 cancellationToken);
         }
-        catch (McpException ex) when (!IsTransportOrSessionFailure(ex))
+        catch (Exception ex) when (ex is McpException or HttpRequestException && !IsTransportOrSessionFailure(ex))
         {
             return McpPromptSkillLoadResult.Failed(
                 $"MCP prompt '{source.PromptName}' failed: {ex.Message}");
@@ -1798,8 +1798,14 @@ private static IEnumerable<Exception> EnumerateExceptionTree(Exception root)
 
     internal static bool IsTransportOrSessionFailure(Exception ex)
     {
-        if (ex is HttpRequestException
-            or IOException
+        // A missing status means the request never got an answer, and the Streamable HTTP
+        // spec reports an expired session as 404. A new session repairs both. Every other
+        // status is an application error from a server that answered, so a new session
+        // cannot change the answer and the reconnect is wasted work.
+        if (ex is HttpRequestException http)
+            return http.StatusCode is null or HttpStatusCode.NotFound;
+
+        if (ex is IOException
             or EndOfStreamException
             or TimeoutException
             or ObjectDisposedException)