fix(mcp): logged, single-flight, proactive OAuth refresh for MCP servers by Aaronontheweb · Pull Request #1697 · netclaw-dev/netclaw

Aaronontheweb · 2026-07-18T21:51:08Z

Defects (from #1696)

Refresh failures were invisible — the live path delegated to the MCP SDK's ClientOAuthProvider, which refreshes silently and falls back to a headless-fatal interactive flow; Netclaw's own logged refresh path (McpOAuthService.GetValidTokenAsync) had zero callers.
No single-flight on refresh — concurrent callers against an expired or rejected token could redeem a rotating refresh token twice, which providers treat as theft and revoke the grant. Concurrent reconnects could also replace each other's newly-created client.
Refresh was reactive-only — nothing refreshed ahead of the persisted expiry, so idle/restarted daemons only discovered a dead token via a 401.
Token sets without a refresh token dead-ended silently at expiry with no advance warning.

Fix

McpOAuthService is now the sole runtime token authority. A Netclaw-owned HTTP authorization handler attaches access tokens, routes both proactive and on-401 refresh through the same per-server single-flight, and retries the rejected request once. The MCP SDK no longer receives the refresh token or independently redeems it; the former ITokenCache bridge was removed.
On a 401, refresh is tied to the exact rejected access token. Concurrent requests waiting behind the winner reuse its replacement token instead of redeeming the rotated refresh token again.
McpClientManager.TryReconnectAsync now uses a version-aware per-server gate. Overlapping reconnect callers reuse the first successful replacement client; a failed leader allows the next waiter to retry. Terminal teardown uses the same gate and observes cancellation.
Proactive refresh rides McpReconnectionService's existing 30s tick: connected OAuth servers refresh once within a 10-minute window of ExpiresAt (constant, TimeProvider-driven).
invalid_grant, invalid_client, and unauthorized_client are terminal: tokens are cleared, the connection is torn down, and diagnostics report a distinct AuthFailed status with reauthorization guidance. Client-registration errors also clear the cached DCR client ID so reauthorization registers a fresh client. Transient failures retain tokens and remain retryable.
Missing-refresh-token and unknown-expiry conditions are surfaced without log spam. Static Authorization headers remain operator-owned and bypass managed OAuth refresh.
netclaw-operations diagnostics and netclaw doctor explain the distinct statuses.

Tests

Concurrent proactive refresh produces exactly one token-endpoint request.
Concurrent resource-server 401s produce exactly one refresh request, and every request retries with the replacement token.
Retried POST requests preserve their body and headers.
Concurrent reconnects create/reuse one successful replacement; a failed leader permits a waiter to retry; teardown waiting observes cancellation.
A real HTTP MCP client is connected, terminally rejected during proactive refresh, torn down, and moved to the distinct AuthFailed status.
Terminal OAuth error parsing, transient retention, proactive timing, missing refresh-token warnings, doctor output, and encrypted persistence remain covered.

Validation

Full dotnet test --no-restore passed.
dotnet slopwatch analyze passed with zero findings.
pwsh ./scripts/Add-FileHeaders.ps1 -Verify and git diff --check passed.
Behavioral eval execution was attempted but the required NETCLAW_EVAL_PROVIDER_TYPE, NETCLAW_EVAL_PROVIDER_ENDPOINT, and NETCLAW_EVAL_MODEL_ID are not configured in this workspace.

Fixes #1696

…h rejections Parse the RFC 6749 error body at the token endpoint (4xx only) and surface invalid_grant, invalid_client, and unauthorized_client as a typed OAuthTokenRefreshRejectedException. McpOAuthService handles all three in one shared terminal path (clear + persist + alert with the code as reason); client errors additionally drop the cached DCR client_id so re-auth performs a fresh registration instead of reusing the dead one. 5xx, network errors, and unrecognized/unparsable bodies stay transient.

Aaronontheweb · 2026-07-18T22:05:26Z

Addressed review: invalid_client / unauthorized_client are now terminal alongside invalid_grant. The token endpoint's 4xx error body is parsed (RFC 6749 §5.2) into a typed OAuthTokenRefreshRejectedException; McpOAuthService handles all three codes in one shared terminal path (clear tokens + persist + alert with the actual code as reason), and client errors additionally drop the cached DCR client_id — EnsureClientRegisteredAsync short-circuits on it, so without the clear a re-auth would have reused the dead registration. 5xx / network / unrecognized or unparsable bodies remain transient. New tests cover terminal classification per code, the 5xx-with-terminal-shaped-body case, and a full invalid_client → re-auth → fresh DCR recovery.

Aaronontheweb · 2026-07-19T00:22:13Z

Refresh was reactive-only — nothing refreshed ahead of the persisted expiry, so idle/restarted daemons only discovered a dead token via a 401.

Yeah, this is the real killer. I noticed that my Notion MCP connections were needing to be reset every couple of weeks or so, and this is why.

Aaronontheweb

Rejected. This is massively over-engineered for the problem we were trying to solve.

Aaronontheweb · 2026-07-19T00:26:04Z

+                    // 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)


Aaronontheweb · 2026-07-19T00:26:54Z

+    [InlineData("invalid_grant", HttpStatusCode.BadRequest)]
+    [InlineData("invalid_client", HttpStatusCode.Unauthorized)]
+    [InlineData("unauthorized_client", HttpStatusCode.BadRequest)]
+    public async Task RefreshToken_TerminalOAuthError_ThrowsRejectedWithErrorCode(


Forcibly tests the types of error codes we get back on token exchange to ensure that there's a visible error that appears in the logs this time around

Aaronontheweb · 2026-07-21T02:51:09Z

+        var results = await Task.WhenAll(reconnects);
+
+        Assert.All(results, Assert.True);
+        Assert.Equal(1, Volatile.Read(ref reconnectCount));


Aaronontheweb · 2026-07-21T02:52:49Z

+            ct);
    }

+    private McpReconnectGate GetReconnectGate(McpServerName name)


populates the McpReconnectGate for this MCP server on-demand

Aaronontheweb · 2026-07-21T02:53:12Z

+    /// state so `netclaw doctor` and the daemon status API can show it ahead
+    /// of time (see McpServersDoctorCheck's "Connected" case).
+    /// </summary>
+    private static string? BuildOAuthAdvisory(McpServerName serverName, McpOAuthTokenSet? tokenSet)


LGTM - give user a heads up that they'll need to log back in in N days.

Aaronontheweb · 2026-07-21T02:53:32Z

            client = null;
-            _statuses[name] = new McpServerStatus(name, McpConnectionState.Connected, tools.Count, null);
+
+            // Advance warning for OAuth-managed servers whose token can never


Aaronontheweb · 2026-07-21T02:54:24Z

+                // refresh path before constructing the HTTP transport. Runtime
+                // requests use McpOAuthAuthorizationHandler, so the SDK never
+                // receives or independently redeems the refresh token.
+                await _oauthService.GetValidTokenAsync(name, entry, ct);


Refresh happens internally inside here otherwise

Aaronontheweb · 2026-07-21T02:58:23Z

+/// refresh. The MCP SDK's OAuth provider cannot participate here because it
+/// redeems cached refresh tokens outside Netclaw's per-server single-flight.
+/// </summary>
+internal sealed class McpOAuthAuthorizationHandler : DelegatingHandler


So this looks like we basically ripped out what was previously supported by the ModelContextProtocol SDK and started doing it ourselves to work around, what is fundamentally a race condition? That seems:

A bad idea

Are you sure we aren't solving a problem that the ModelContextProtocol SDK already handles perfectly fine on its own?

Going to lock this PR down until we figure that out definitively. This stinks.

Aaronontheweb · 2026-07-21T03:03:48Z

Closing after revisiting the OAuth ownership boundary. This PR made Netclaw responsible for bearer injection, request replay, reactive 401 handling, and token refresh in order to provide single-flight behavior. The MCP SDK maintainers have confirmed that concurrency defect in modelcontextprotocol/csharp-sdk#1595 and are implementing the fix inside ClientOAuthProvider in modelcontextprotocol/csharp-sdk#1708, which is the correct ownership layer. No published SDK version contains that fix yet. Netclaw #1696 has been converted into an upstream dependency tracker and now records the remaining Netclaw-owned lifecycle and diagnostic work. We will upgrade to a published SDK fix and validate it through the real SDK transport rather than maintain a parallel OAuth stack. The branch is being retained for auditability.

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

29b6e4d

Aaronontheweb added the observability label Jul 18, 2026

Merge branch 'dev' into fix/mcp-oauth-refresh

dbad913

Aaronontheweb added mcp Model context protocol server / client issues. security Security-related changes labels Jul 19, 2026

Aaronontheweb added 2 commits July 19, 2026 01:51

fix(mcp): make OAuth refresh ownership single-flight

eeb1dad

Merge branch 'dev' into fix/mcp-oauth-refresh

8d131d5

Aaronontheweb commented Jul 21, 2026

View reviewed changes

Aaronontheweb mentioned this pull request Jul 21, 2026

MCP OAuth: track upstream single-flight refresh and harden Netclaw diagnostics #1696

Open

7 tasks

Aaronontheweb closed this Jul 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

fix(mcp): logged, single-flight, proactive OAuth refresh for MCP servers#1697
Aaronontheweb wants to merge 5 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/mcp-oauth-refresh

Aaronontheweb commented Jul 18, 2026 •

edited

Loading

Uh oh!

Aaronontheweb commented Jul 18, 2026

Uh oh!

Aaronontheweb commented Jul 19, 2026

Uh oh!

Aaronontheweb left a comment

Uh oh!

Aaronontheweb Jul 19, 2026

Uh oh!

Aaronontheweb Jul 19, 2026

Uh oh!

Aaronontheweb Jul 21, 2026

Uh oh!

Aaronontheweb Jul 21, 2026

Uh oh!

Aaronontheweb Jul 21, 2026

Uh oh!

Aaronontheweb Jul 21, 2026

Uh oh!

Aaronontheweb Jul 21, 2026

Uh oh!

Aaronontheweb Jul 21, 2026

Uh oh!

Aaronontheweb commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Aaronontheweb commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Defects (from #1696)

Fix

Tests

Validation

Uh oh!

Aaronontheweb commented Jul 18, 2026

Uh oh!

Aaronontheweb commented Jul 19, 2026

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aaronontheweb commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Aaronontheweb commented Jul 18, 2026 •

edited

Loading