fix(mcp): logged, single-flight, proactive OAuth refresh for MCP servers

feeds/skills/.system/files/netclaw-operations/SKILL.md

@@ -3,7 +3,7 @@ name: netclaw-operations
 description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance."
 metadata:
   author: netclaw
-  version: "2.35.0"
+  version: "2.36.0"
 ---
 
 # Netclaw Operations

feeds/skills/.system/files/netclaw-operations/references/diagnostics.md

@@ -74,6 +74,7 @@ debugging a daemon-wide problem → read `daemon.log`.
 |---------|-------|
 | No LLM responses | `netclaw doctor`; verify provider credentials |
 | Missing tools | `netclaw mcp list`; check MCP connection state |
+| MCP server `auth failed` in `netclaw doctor` / `netclaw mcp status` | Fixed by `netclaw mcp auth <name>` either way, but the message tells you which case: "refresh rejected (<code>)" means Netclaw's proactive or on-401 refresh got a terminal rejection from the provider — the stored tokens were already cleared and the connection torn down. The code names the cause: `invalid_grant` = the grant itself was revoked/rotated; `invalid_client` / `unauthorized_client` = the provider no longer recognizes the client registration (Netclaw also dropped the cached client_id, so re-auth performs a fresh registration automatically). Neither is a config error. A generic "authentication rejected by server" means static credentials (token/headers) were rejected. `awaiting auth` is different from all of these — that server has never completed OAuth at all. A server reported **connected** with a "No refresh token" note is healthy now but cannot silently refresh; re-run `netclaw mcp auth <name>` before its current token expires. |
 | Memory recall degraded | `netclaw status` memory section |
 | Daemon won't start | crash logs at `<NETCLAW_HOME>/logs/crash-*.log` (`NETCLAW_HOME` defaults to `~/.netclaw`) |
 | Docker daemon cannot create `/home/netclaw/.netclaw/*` | Official image entrypoint repairs writable bind mounts to UID/GID `1654:1654`; if bypassed or read-only, run `sudo chown -R 1654:1654 <host-data-dir>` or use a Docker named volume |

src/Netclaw.Cli.Tests/Doctor/McpServersDoctorCheckTests.cs

@@ -231,6 +231,40 @@ public async Task DaemonReportedAwaitingAuth_ReturnsWarning()
         Assert.Contains("awaiting auth", result.Message);
     }
 
+    [Fact]
+    public async Task DaemonReportedConnectedWithNoRefreshTokenAdvisory_ReturnsPassButSurfacesAdvisory()
+    {
+        WriteConfig(new
+        {
+            configVersion = 1,
+            McpServers = new
+            {
+                notion = new
+                {
+                    Transport = "http",
+                    Url = "https://mcp.example.com",
+                    Enabled = true,
+                }
+            }
+        });
+
+        var check = new McpServersDoctorCheck(_paths, CreateDaemonApi(_ => FakeHttpMessageHandler.JsonResponse(new
+        {
+            notion = new
+            {
+                state = "Connected",
+                toolCount = 5,
+                error = "No refresh token — re-authorization will be required at expiry. Run: netclaw mcp auth notion"
+            }
+        })));
+
+        var result = await check.RunAsync(TestContext.Current.CancellationToken);
+
+        Assert.Equal(DoctorSeverity.Pass, result.Severity);
+        Assert.Contains("connected (5 tools)", result.Message);
+        Assert.Contains("No refresh token", result.Message);
+    }
+
     [Fact]
     public async Task OfflineOAuthProbe_DoesNotClaimAuthFailure()
     {

src/Netclaw.Cli/Doctor/McpServersDoctorCheck.cs

@@ -263,7 +263,13 @@ private static DoctorCheckResult EvaluateDaemonStatuses(
             switch (state)
             {
                 case "Connected":
-                    statusMessages.Add($"{name}: connected ({toolCount} tools)");
+                    // `error` doubles as an advisory slot for otherwise-healthy
+                    // servers — e.g. an OAuth token with no refresh token, which
+                    // will hard-fail at expiry with no chance of silent
+                    // recovery. See McpClientManager.BuildOAuthAdvisory.
+                    statusMessages.Add(string.IsNullOrEmpty(error)
+                        ? $"{name}: connected ({toolCount} tools)"
+                        : $"{name}: connected ({toolCount} tools) — {error}");
                     break;
                 case "AwaitingAuth":
                     hasAwaitingAuth = true;

src/Netclaw.Configuration.Tests/Providers/OAuth/OAuthPkceServiceTests.cs

@@ -288,20 +288,77 @@ public async Task RefreshToken_Success_ReturnsNewTokenPreservingRefresh()
         Assert.Equal("old-refresh", result.RefreshToken!.Value);
     }
 
+    [Theory]
+    [InlineData("invalid_grant", HttpStatusCode.BadRequest)]
+    [InlineData("invalid_client", HttpStatusCode.Unauthorized)]
+    [InlineData("unauthorized_client", HttpStatusCode.BadRequest)]
+    public async Task RefreshToken_TerminalOAuthError_ThrowsRejectedWithErrorCode(
+        string errorCode, HttpStatusCode status)
+    {
+        var handler = new FakeHttpMessageHandler(_ =>
+            JsonResponse(new { error = errorCode }, status));
+
+        var service = new OAuthPkceService(new HttpClient(handler));
+
+        var ex = await Assert.ThrowsAsync<OAuthTokenRefreshRejectedException>(() =>
+            service.RefreshTokenAsync(
+                "https://auth.example.com/token",
+                "test-client",
+                new SensitiveString("expired-refresh"), ct: TestContext.Current.CancellationToken));
+
+        Assert.Equal(errorCode, ex.ErrorCode);
+        Assert.Equal(status, ex.StatusCode);
+    }
+
     [Fact]
-    public async Task RefreshToken_InvalidGrant_ReturnsNull()
+    public async Task RefreshToken_UnrecognizedOAuthError_ThrowsHttpRequestException()
     {
+        // "invalid_request" is a 4xx OAuth error but NOT a terminal refresh
+        // code — it must stay transient (retryable), not clear credentials.
         var handler = new FakeHttpMessageHandler(_ =>
-            JsonResponse(new { error = "invalid_grant" }, HttpStatusCode.BadRequest));
+            JsonResponse(new { error = "invalid_request" }, HttpStatusCode.BadRequest));
 
         var service = new OAuthPkceService(new HttpClient(handler));
 
-        var result = await service.RefreshTokenAsync(
-            "https://auth.example.com/token",
-            "test-client",
-            new SensitiveString("expired-refresh"), ct: TestContext.Current.CancellationToken);
+        await Assert.ThrowsAsync<HttpRequestException>(() =>
+            service.RefreshTokenAsync(
+                "https://auth.example.com/token",
+                "test-client",
+                new SensitiveString("expired-refresh"), ct: TestContext.Current.CancellationToken));
+    }
+
+    [Fact]
+    public async Task RefreshToken_UnparsableErrorBody_ThrowsHttpRequestException()
+    {
+        var handler = new FakeHttpMessageHandler(_ => new HttpResponseMessage(HttpStatusCode.BadRequest)
+        {
+            Content = new StringContent("<html>gateway error</html>"),
+        });
+
+        var service = new OAuthPkceService(new HttpClient(handler));
+
+        await Assert.ThrowsAsync<HttpRequestException>(() =>
+            service.RefreshTokenAsync(
+                "https://auth.example.com/token",
+                "test-client",
+                new SensitiveString("expired-refresh"), ct: TestContext.Current.CancellationToken));
+    }
+
+    [Fact]
+    public async Task RefreshToken_ServerErrorWithTerminalShapedBody_ThrowsHttpRequestException()
+    {
+        // A 5xx is a server-side fault worth retrying even when its body
+        // happens to carry a terminal-looking error code.
+        var handler = new FakeHttpMessageHandler(_ =>
+            JsonResponse(new { error = "invalid_grant" }, HttpStatusCode.ServiceUnavailable));
+
+        var service = new OAuthPkceService(new HttpClient(handler));
 
-        Assert.Null(result);
+        await Assert.ThrowsAsync<HttpRequestException>(() =>
+            service.RefreshTokenAsync(
+                "https://auth.example.com/token",
+                "test-client",
+                new SensitiveString("expired-refresh"), ct: TestContext.Current.CancellationToken));
     }
 
     [Fact]

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

@@ -74,4 +74,28 @@ public void BuildConnectionFailureStatus_ForNetworkFailure_ReturnsUnreachable()
         Assert.Equal(McpConnectionState.Unreachable, status.State);
         Assert.Contains("Connection refused", status.ErrorMessage);
     }
+
+    [Theory]
+    [InlineData("invalid_grant")]
+    [InlineData("invalid_client")]
+    [InlineData("unauthorized_client")]
+    public void CreateRefreshRejectedStatus_ReturnsAuthFailedNamingTheErrorCode(string errorCode)
+    {
+        var status = McpClientManager.CreateRefreshRejectedStatus(new McpServerName("notion"), errorCode);
+
+        // AuthFailed (not Unreachable) so McpReconnectionService's backoff loop
+        // — which only retries Unreachable servers — never auto-retries dead
+        // credentials into a loop.
+        Assert.Equal(McpConnectionState.AuthFailed, status.State);
+        Assert.Contains(errorCode, status.ErrorMessage);
+        Assert.Contains("netclaw mcp auth notion", status.ErrorMessage);
+
+        // Distinct wording from the generic AuthFailed message so operators can
+        // tell "credentials rejected by provider" apart from "never authenticated".
+        var genericAuthFailed = McpClientManager.CreateAuthFailedStatus(
+            new McpServerName("notion"),
+            new HttpRequestException(httpRequestError: HttpRequestError.Unknown, "Unauthorized", null, System.Net.HttpStatusCode.Unauthorized),
+            oauthManaged: true);
+        Assert.NotEqual(genericAuthFailed.ErrorMessage, status.ErrorMessage);
+    }
 }

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

@@ -406,6 +406,8 @@ public IReadOnlyDictionary<McpServerName, McpServerStatus> GetServerStatuses() =
 
         public Task<bool> TryReconnectAsync(McpServerName serverName, CancellationToken ct = default) =>
             Task.FromResult(false);
+
+        public Task RefreshOAuthTokensAsync(CancellationToken ct = default) => Task.CompletedTask;
     }
 
     private sealed class TrackingReconnectable : IMcpReconnectable
@@ -429,5 +431,7 @@ public Task<bool> TryReconnectAsync(McpServerName serverName, CancellationToken
             _tcs.TrySetResult();
             return Task.FromResult(true);
         }
+
+        public Task RefreshOAuthTokensAsync(CancellationToken ct = default) => Task.CompletedTask;
     }
 }