Adding invalidateCredentials() to OAuthClientProvider

[geelen]

From working on mcp-remote, I'm seeing a lot of cases where the local state and the server state drift apart. It's especially common when iterating locally (see geelen/mcp-remote#36), but seems to be happening with production servers too.

The issue was that while the SDK defines a series of specific OAuthError types, they're only used on the server side of things. At the crucial point, where POST /token is being called and a invalid_client or invalid_grant are being received, the client simply logs that the request failed and continues: https://github.com/modelcontextprotocol/typescript-sdk/blob/main/src/client/auth.ts#L166

This PR addresses this by looking for those particular error codes and invoking a new method on the OAuthClientProvider (if present): invalidateCredentials. This takes an argument of 'all' | 'client' | 'tokens' | 'verifier', but currently only all and tokens are used.

How Has This Been Tested?

Some tests have been added (generated by Amp, I'm not entirely happy with them but ran out of time and wanted to submit for feedback).

mcp-remote has been released in preview under https://pkg.pr.new/mcp-remote@96 with these changes and has confirmed that the errors in geelen/mcp-remote#36 are fixed.

Breaking Changes

Any error other than invalid_client or invalid_grant are now re-thrown, rather than silently swallowed. But those errors were likely unrecoverable anyway so this would arguably just change the kind of crash message.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

Marked as draft as main has moved on so merging isn't possible yet. Would love reviews on the approach though. I will clean up and merge next week.

[pcarleton]

I like this idea! a few notes on the approach:

• the nested try/catch + passing the error in as an arg is a bit tough to follow, might be time to refactor that method into some sub-methods. put an alternative structure below, lmkwyt
• if you can add an example in the examples repo, that'd be great to show what you expect folks to do in the invalidate method.

alternative to avoid recursion + nested try/catch:

  type AuthSuccess = {
    success: true;
    result: AuthResult;
  };

  type AuthError = {
    success: false;
    error: Error;
  };

  type AuthResponse = AuthSuccess | AuthError;

  export async function auth(
    provider: OAuthClientProvider,
    options: { serverUrl: string | URL, authorizationCode?: string },
  ): Promise<AuthResult> {
    const response = await authInternal(provider, options);

    if (response.success) {
      return response.result;
    }

    // Handle specific error types
    if (response.error instanceof InvalidClientError ||
         response.error instanceof UnauthorizedClientError) {
      await provider.invalidateCredentials?.('all');
      const retryResponse = await authInternal(provider, options);
      if (retryResponse.success) {
        return retryResponse.result;
      }
    } else if (response.error instanceof InvalidGrantError) {
      await provider.invalidateCredentials?.('tokens');
      const retryResponse = await authInternal(provider, options);
      if (retryResponse.success) {
        return retryResponse.result;
      }
    }

    // Couldn't recover
    throw response.error;
  }

  async function authInternal(
    provider: OAuthClientProvider,
    options: { serverUrl: string | URL, authorizationCode?: string },
  ): Promise<AuthResponse> {
    try {
      // Normal auth flow implementation
      // ...existing implementation...

      return { success: true, result: resultValue };
    } catch (error) {
      return {
        success: false,
        error: error instanceof Error ? error : new Error(String(error)),
      };
    }
  }

[geelen]

Updated. Available for testing on https://pkg.pr.new/geelen/typescript-sdk/@modelcontextprotocol/sdk@cdf3508

I'm going to cut an mcp-remote release using this now, since it seems to make a big difference to have any invalid data automatically cleaned

[pcarleton]

this looks good to me, one open question about the double errorCode declaration, and I'm wondering if there's a more idiomatic way to do that. I'll try to chase that down and if we don't come up with anything, merge as is.

[pcarleton]

this and the this.constructor below seem odd, and I'm wondering if there's another way to do this.

[geelen]

I've refactored this slightly, makes it more obvious where errorCode is actually coming from.

[geelen]

Rebased and tweaked based on feedback above, should be good to go

[pcarleton]

I think if you override getError() here it will be work. We only call it in the BearerAuth test, and we never get errorCode statically outside of this file