🔑 fix: Clear Stale Client Registration on `invalid_client` During OAuth Token Refresh#12643

danny-avila · 2026-04-13T13:10:54Z

Summary

I fixed a bug where MCP servers using Dynamic Client Registration (RFC 7591) would leave users permanently stuck when a stored client_id became invalid. The token refresh handler only recognized unauthorized_client as a recoverable error, so invalid_client rejections caused every retry to silently return null and reuse the same stale registration in an infinite loop.

Detect invalid_client in the getTokens() refresh error handler, call deleteClientRegistration() to clear the stale DCR entry, and throw ReauthenticationRequiredError with a new 'invalid_client' reason to trigger a fresh OAuth flow.
Add deleteTokens to the GetTokensParams interface so the cleanup has access to the token deletion method inside the storage layer.
Pass deleteTokens from MCPConnectionFactory.getOAuthTokens() into the MCPTokenStorage.getTokens() call.
Extend the ReauthenticationRequiredError constructor's reason union type to accept 'invalid_client', with a distinct human-readable detail message for this case.

The existing clearStaleClientIfRejected logic in MCPConnectionFactory (added in #11925) does not catch this case because getTokens() returns null on refresh failure rather than throwing, so the error never propagates to the factory's handler.

Change Type

Bug fix (non-breaking change which fixes an issue)

Testing

Tested against MCP servers with Dynamic Client Registration:

Connected to GitHub MCP successfully, confirming a valid DCR client registration was stored.
Corrupted the stored client_id to a bogus UUID directly in the database to simulate a stale registration.
Deleted the stored access token to force a token refresh using the corrupted client_id.
Reinitialized GitHub MCP. LibreChat attempted to refresh with the corrupted client_id, received invalid_client, cleared the stale registration, and automatically completed a fresh DCR registration.
Verified the stored client_id was replaced with a new valid UUID and the connection was working.

Recommend adding unit tests for MCPTokenStorage.getTokens() covering:

invalid_client during refresh with deleteTokens present: verify deleteClientRegistration is called and ReauthenticationRequiredError is thrown.
invalid_client during refresh with deleteTokens absent: verify return null behavior.
deleteClientRegistration failure: verify the .catch path does not block the rethrow.
The 'invalid_client' reason variant produces the correct error message.

Checklist

My code adheres to this project's style guidelines
I have performed a self-review of my own code
I have written tests demonstrating that my changes are effective or that my feature works
Local unit tests pass with my changes

…efresh When a token refresh fails with `invalid_client`, the stored DCR client registration is no longer valid on the authorization server. The existing error handler only checked for `unauthorized_client` and returned null, leaving the stale client_id cached in the database permanently. Every subsequent token refresh attempt would fail with the same error. Now when `invalid_client` is detected during refresh: 1. The stale client registration is deleted from the database 2. A `ReauthenticationRequiredError` is thrown to trigger a fresh OAuth flow with new dynamic client registration Also passes `deleteTokens` from MCPConnectionFactory to getTokens() so the cleanup has access to the token deletion method.

- Delete stale refresh token alongside client registration on invalid_client (Finding 1) - Add tests for all new code paths: cleanup, warning, case-insensitivity, cleanup failure (Finding 2) - Detect all vendor-specific client rejection patterns (client_id mismatch, client not found, unknown client) with case-insensitive matching (Finding 3) - Use else-if for mutually exclusive error branches (Finding 4) - Log warning when deleteTokens is not available on client rejection (Finding 6) - Fix log message to say "attempting to clear" before async cleanup (Finding 7) - Extract isClientRejectionMessage to shared utility, refactor MCPConnectionFactory.isClientRejection to use it

danny-avila · 2026-04-13T13:11:51Z

@codex review

Copilot

Pull request overview

This PR improves MCP OAuth resiliency by detecting “invalid client” refresh failures and clearing stale stored client registration/refresh token data so subsequent OAuth flows can re-register cleanly.

Changes:

Added a shared isClientRejectionMessage() helper to centralize OAuth client-rejection error-message detection.
Updated MCPTokenStorage.getTokens() to optionally clear stale client registration + refresh token on invalid_client-style refresh failures when deleteTokens is available, and to throw ReauthenticationRequiredError('invalid_client').
Updated MCPConnectionFactory to use the shared helper and to pass deleteTokens into token retrieval; added/expanded token storage tests for the new cleanup behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
packages/api/src/mcp/utils.ts	Introduces `isClientRejectionMessage()` helper for standardized client-rejection detection.
packages/api/src/mcp/oauth/tokens.ts	Adds invalid-client reauth reason and implements stale registration/refresh-token cleanup using `deleteTokens`.
packages/api/src/mcp/MCPConnectionFactory.ts	Uses `isClientRejectionMessage()` and wires `deleteTokens` through to token retrieval.
packages/api/src/mcp/tests/MCPOAuthTokenStorage.test.ts	Adds coverage for invalid-client cleanup and related edge cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T13:15:10Z

+          } else if (
+            lowerMessage.includes('invalid_client') ||
+            lowerMessage.includes('client_id mismatch') ||
+            lowerMessage.includes('client not found') ||
+            lowerMessage.includes('unknown client')
+          ) {


The client-rejection detection logic here duplicates the pattern list that was just centralized in isClientRejectionMessage() (packages/api/src/mcp/utils.ts). To avoid drift (e.g., future additions applied in one place but not the other), consider reusing the shared helper here as well (e.g., compute lowerMessage once and delegate the check).

Copilot · 2026-04-13T13:15:10Z

+              await Promise.allSettled([
+                MCPTokenStorage.deleteClientRegistration({ userId, serverName, deleteTokens }),
+                deleteTokens({
+                  userId,
+                  type: 'mcp_oauth_refresh',
+                  identifier: `${staleIdentifier}:refresh`,
+                }),
+              ]).then((results) => {
+                for (const r of results) {
+                  if (r.status === 'rejected') {
+                    logger.warn(`${logPrefix} Failed to clear stale token data`, r.reason);
+                  }
+                }
+              });


Minor readability issue: await Promise.allSettled(...).then(...) mixes await with a .then chain. Consider awaiting Promise.allSettled into a local results variable and iterating it directly; it’s equivalent but easier to follow and debug.

Suggested change

await Promise.allSettled([

MCPTokenStorage.deleteClientRegistration({ userId, serverName, deleteTokens }),

deleteTokens({

userId,

type: 'mcp_oauth_refresh',

identifier: `${staleIdentifier}:refresh`,

}),

]).then((results) => {

for (const r of results) {

if (r.status === 'rejected') {

logger.warn(`${logPrefix} Failed to clear stale token data`, r.reason);

}

}

});

const results = await Promise.allSettled([

MCPTokenStorage.deleteClientRegistration({ userId, serverName, deleteTokens }),

deleteTokens({

userId,

type: 'mcp_oauth_refresh',

identifier: `${staleIdentifier}:refresh`,

}),

]);

for (const r of results) {

if (r.status === 'rejected') {

logger.warn(`${logPrefix} Failed to clear stale token data`, r.reason);

}

}

chatgpt-codex-connector · 2026-04-13T13:16:59Z

Codex Review: Didn't find any major issues. Swish!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- Extract isInvalidClientMessage (4 stale-client patterns) from isClientRejectionMessage to eliminate pattern duplication between utils.ts and tokens.ts (Finding 1) - Remove redundant staleIdentifier variable, reuse identifier already in scope (Finding 2) - Separate await from .then() on Promise.allSettled for readability (Finding 3) - Add dedicated unit tests for isInvalidClientMessage and isClientRejectionMessage (Finding 4) - Assert error message content in primary invalid_client test (Finding 5) - Add JSDoc on deleteTokens in GetTokensParams (Finding 6)

danny-avila · 2026-04-13T13:46:02Z

@codex review

chatgpt-codex-connector · 2026-04-13T13:51:43Z

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…th Token Refresh (danny-avila#12643) * fix: clear stale client registration on invalid_client during token refresh When a token refresh fails with `invalid_client`, the stored DCR client registration is no longer valid on the authorization server. The existing error handler only checked for `unauthorized_client` and returned null, leaving the stale client_id cached in the database permanently. Every subsequent token refresh attempt would fail with the same error. Now when `invalid_client` is detected during refresh: 1. The stale client registration is deleted from the database 2. A `ReauthenticationRequiredError` is thrown to trigger a fresh OAuth flow with new dynamic client registration Also passes `deleteTokens` from MCPConnectionFactory to getTokens() so the cleanup has access to the token deletion method. * fix: address review findings for stale client cleanup on token refresh - Delete stale refresh token alongside client registration on invalid_client (Finding 1) - Add tests for all new code paths: cleanup, warning, case-insensitivity, cleanup failure (Finding 2) - Detect all vendor-specific client rejection patterns (client_id mismatch, client not found, unknown client) with case-insensitive matching (Finding 3) - Use else-if for mutually exclusive error branches (Finding 4) - Log warning when deleteTokens is not available on client rejection (Finding 6) - Fix log message to say "attempting to clear" before async cleanup (Finding 7) - Extract isClientRejectionMessage to shared utility, refactor MCPConnectionFactory.isClientRejection to use it * fix: address followup review findings - Extract isInvalidClientMessage (4 stale-client patterns) from isClientRejectionMessage to eliminate pattern duplication between utils.ts and tokens.ts (Finding 1) - Remove redundant staleIdentifier variable, reuse identifier already in scope (Finding 2) - Separate await from .then() on Promise.allSettled for readability (Finding 3) - Add dedicated unit tests for isInvalidClientMessage and isClientRejectionMessage (Finding 4) - Assert error message content in primary invalid_client test (Finding 5) - Add JSDoc on deleteTokens in GetTokensParams (Finding 6) --------- Co-authored-by: Mani Japra <mani@muonspace.com>

mani-muon and others added 2 commits April 10, 2026 14:37

Copilot AI review requested due to automatic review settings April 13, 2026 13:10

danny-avila mentioned this pull request Apr 13, 2026

fix: clear stale client registration on invalid_client during token refresh #12610

Closed

Copilot started reviewing on behalf of danny-avila April 13, 2026 13:11 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

danny-avila changed the title ~~fix/clear stale client on invalid client review~~ 🔑 fix: Clear Stale Client Registration on invalid_client During OAuth Token Refresh Apr 13, 2026

danny-avila merged commit c4bb411 into dev Apr 13, 2026
10 of 11 checks passed

danny-avila deleted the fix/clear-stale-client-on-invalid-client-review branch April 13, 2026 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🔑 fix: Clear Stale Client Registration on `invalid_client` During OAuth Token Refresh#12643