Skip to content

Commit 7384f73

Browse files
felixweinbergerfelixweinberger
committed
fix: reset _authRetryInFlight in send() catch; add migration-SKILL entry
Addresses both review comments on the breaking-change commit: 1. _authRetryInFlight was never reset when onUnauthorized() throws in send() — if the refresh failed transiently, subsequent 401s would skip the retry path permanently. The connect paths already had try/finally; send() paths now reset in the outer catch block. (Resetting in a try/finally around just onUnauthorized would break the circuit breaker — the finally would fire before the recursive not-awaited send() resolves.) Added regression test: onUnauthorized throws once, succeeds on second send() call. 2. migration-SKILL.md now has the AuthProvider mapping table per CLAUDE.md's both-files requirement.
1 parent 9e2716a commit 7384f73

4 files changed

Lines changed: 109 additions & 47 deletions

File tree

‎docs/migration-SKILL.md‎

Lines changed: 85 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,15 @@ Replace all `@modelcontextprotocol/sdk/...` imports using this table.
4747

4848
### Server imports
4949

50-
| v1 import path | v2 package |
51-
| ---------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
52-
| `@modelcontextprotocol/sdk/server/mcp.js` | `@modelcontextprotocol/server` |
53-
| `@modelcontextprotocol/sdk/server/index.js` | `@modelcontextprotocol/server` |
54-
| `@modelcontextprotocol/sdk/server/stdio.js` | `@modelcontextprotocol/server` |
50+
| v1 import path | v2 package |
51+
| ---------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
52+
| `@modelcontextprotocol/sdk/server/mcp.js` | `@modelcontextprotocol/server` |
53+
| `@modelcontextprotocol/sdk/server/index.js` | `@modelcontextprotocol/server` |
54+
| `@modelcontextprotocol/sdk/server/stdio.js` | `@modelcontextprotocol/server` |
5555
| `@modelcontextprotocol/sdk/server/streamableHttp.js` | `@modelcontextprotocol/node` (class renamed to `NodeStreamableHTTPServerTransport`) OR `@modelcontextprotocol/server` (web-standard `WebStandardStreamableHTTPServerTransport` for Cloudflare Workers, Deno, etc.) |
56-
| `@modelcontextprotocol/sdk/server/sse.js` | REMOVED (migrate to Streamable HTTP) |
57-
| `@modelcontextprotocol/sdk/server/auth/*` | REMOVED (use external auth library) |
58-
| `@modelcontextprotocol/sdk/server/middleware.js` | `@modelcontextprotocol/express` (signature changed, see section 8) |
56+
| `@modelcontextprotocol/sdk/server/sse.js` | REMOVED (migrate to Streamable HTTP) |
57+
| `@modelcontextprotocol/sdk/server/auth/*` | REMOVED (use external auth library) |
58+
| `@modelcontextprotocol/sdk/server/middleware.js` | `@modelcontextprotocol/express` (signature changed, see section 8) |
5959

6060
### Types / shared imports
6161

@@ -203,7 +203,41 @@ import { OAuthError, OAuthErrorCode } from '@modelcontextprotocol/core';
203203
if (error instanceof OAuthError && error.code === OAuthErrorCode.InvalidClient) { ... }
204204
```
205205

206-
**Unchanged APIs** (only import paths changed): `Client` constructor and most methods, `McpServer` constructor, `server.connect()`, `server.close()`, all client transports (`StreamableHTTPClientTransport`, `SSEClientTransport`, `StdioClientTransport`), `StdioServerTransport`, all Zod schemas, all callback return types. Note: `callTool()` and `request()` signatures changed (schema parameter removed, see section 11).
206+
### Client `OAuthClientProvider` now extends `AuthProvider`
207+
208+
Transport `authProvider` options now accept the minimal `AuthProvider` interface. `OAuthClientProvider` extends it, so built-in providers work unchanged — custom implementations must add `token()`.
209+
210+
| v1 pattern | v2 equivalent |
211+
| ----------------------------------------------------- | --------------------------------------------------------------------------- |
212+
| `authProvider?: OAuthClientProvider` (option type) | `authProvider?: AuthProvider` (accepts `OAuthClientProvider` via extension) |
213+
| Transport reads `authProvider.tokens()?.access_token` | Transport calls `authProvider.token()` |
214+
| Transport inlines `auth()` on 401 | Transport calls `authProvider.onUnauthorized()` then retries once |
215+
| `_hasCompletedAuthFlow` circuit breaker | `_authRetryInFlight` circuit breaker |
216+
| N/A | `handleOAuthUnauthorized(provider, ctx)` — standard `onUnauthorized` impl |
217+
| N/A | `isOAuthClientProvider(provider)` — type guard |
218+
| N/A | `UnauthorizedContext` — `{ response, serverUrl, fetchFn }` |
219+
220+
**For custom `OAuthClientProvider` implementations**, add:
221+
222+
```typescript
223+
async token(): Promise<string | undefined> {
224+
return (await this.tokens())?.access_token;
225+
}
226+
227+
async onUnauthorized(ctx: UnauthorizedContext): Promise<void> {
228+
await handleOAuthUnauthorized(this, ctx);
229+
}
230+
```
231+
232+
**For simple bearer tokens** (previously required stubbing 8 `OAuthClientProvider` members):
233+
234+
```typescript
235+
// v2: one-liner
236+
const authProvider: AuthProvider = { token: async () => process.env.API_KEY };
237+
```
238+
239+
**Unchanged APIs** (only import paths changed): `Client` constructor and most methods, `McpServer` constructor, `server.connect()`, `server.close()`, all client transports (`StreamableHTTPClientTransport`, `SSEClientTransport`, `StdioClientTransport`), `StdioServerTransport`, all
240+
Zod schemas, all callback return types. Note: `callTool()` and `request()` signatures changed (schema parameter removed, see section 11).
207241

208242
## 6. McpServer API Changes
209243

@@ -279,12 +313,12 @@ Note: the third argument (`metadata`) is required — pass `{}` if no metadata.
279313

280314
### Schema Migration Quick Reference
281315

282-
| v1 (raw shape) | v2 (Zod schema) |
283-
|----------------|-----------------|
284-
| `{ name: z.string() }` | `z.object({ name: z.string() })` |
316+
| v1 (raw shape) | v2 (Zod schema) |
317+
| ---------------------------------- | -------------------------------------------- |
318+
| `{ name: z.string() }` | `z.object({ name: z.string() })` |
285319
| `{ count: z.number().optional() }` | `z.object({ count: z.number().optional() })` |
286-
| `{}` (empty) | `z.object({})` |
287-
| `undefined` (no schema) | `undefined` or omit the field |
320+
| `{}` (empty) | `z.object({})` |
321+
| `undefined` (no schema) | `undefined` or omit the field |
288322

289323
## 7. Headers API
290324

@@ -370,31 +404,31 @@ Request/notification params remain fully typed. Remove unused schema imports aft
370404

371405
`RequestHandlerExtra` → structured context types with nested groups. Rename `extra` → `ctx` in all handler callbacks.
372406

373-
| v1 | v2 |
374-
|----|-----|
375-
| `RequestHandlerExtra` | `ServerContext` (server) / `ClientContext` (client) / `BaseContext` (base) |
376-
| `extra` (param name) | `ctx` |
377-
| `extra.signal` | `ctx.mcpReq.signal` |
378-
| `extra.requestId` | `ctx.mcpReq.id` |
379-
| `extra._meta` | `ctx.mcpReq._meta` |
380-
| `extra.sendRequest(...)` | `ctx.mcpReq.send(...)` |
381-
| `extra.sendNotification(...)` | `ctx.mcpReq.notify(...)` |
382-
| `extra.authInfo` | `ctx.http?.authInfo` |
383-
| `extra.sessionId` | `ctx.sessionId` |
384-
| `extra.requestInfo` | `ctx.http?.req` (only `ServerContext`) |
385-
| `extra.closeSSEStream` | `ctx.http?.closeSSE` (only `ServerContext`) |
386-
| `extra.closeStandaloneSSEStream` | `ctx.http?.closeStandaloneSSE` (only `ServerContext`) |
387-
| `extra.taskStore` | `ctx.task?.store` |
388-
| `extra.taskId` | `ctx.task?.id` |
389-
| `extra.taskRequestedTtl` | `ctx.task?.requestedTtl` |
407+
| v1 | v2 |
408+
| -------------------------------- | -------------------------------------------------------------------------- |
409+
| `RequestHandlerExtra` | `ServerContext` (server) / `ClientContext` (client) / `BaseContext` (base) |
410+
| `extra` (param name) | `ctx` |
411+
| `extra.signal` | `ctx.mcpReq.signal` |
412+
| `extra.requestId` | `ctx.mcpReq.id` |
413+
| `extra._meta` | `ctx.mcpReq._meta` |
414+
| `extra.sendRequest(...)` | `ctx.mcpReq.send(...)` |
415+
| `extra.sendNotification(...)` | `ctx.mcpReq.notify(...)` |
416+
| `extra.authInfo` | `ctx.http?.authInfo` |
417+
| `extra.sessionId` | `ctx.sessionId` |
418+
| `extra.requestInfo` | `ctx.http?.req` (only `ServerContext`) |
419+
| `extra.closeSSEStream` | `ctx.http?.closeSSE` (only `ServerContext`) |
420+
| `extra.closeStandaloneSSEStream` | `ctx.http?.closeStandaloneSSE` (only `ServerContext`) |
421+
| `extra.taskStore` | `ctx.task?.store` |
422+
| `extra.taskId` | `ctx.task?.id` |
423+
| `extra.taskRequestedTtl` | `ctx.task?.requestedTtl` |
390424

391425
`ServerContext` convenience methods (new in v2, no v1 equivalent):
392426

393-
| Method | Description | Replaces |
394-
|--------|-------------|----------|
395-
| `ctx.mcpReq.log(level, data, logger?)` | Send log notification (respects client's level filter) | `server.sendLoggingMessage(...)` from within handler |
396-
| `ctx.mcpReq.elicitInput(params, options?)` | Elicit user input (form or URL) | `server.elicitInput(...)` from within handler |
397-
| `ctx.mcpReq.requestSampling(params, options?)` | Request LLM sampling from client | `server.createMessage(...)` from within handler |
427+
| Method | Description | Replaces |
428+
| ---------------------------------------------- | ------------------------------------------------------ | ---------------------------------------------------- |
429+
| `ctx.mcpReq.log(level, data, logger?)` | Send log notification (respects client's level filter) | `server.sendLoggingMessage(...)` from within handler |
430+
| `ctx.mcpReq.elicitInput(params, options?)` | Elicit user input (form or URL) | `server.elicitInput(...)` from within handler |
431+
| `ctx.mcpReq.requestSampling(params, options?)` | Request LLM sampling from client | `server.createMessage(...)` from within handler |
398432

399433
## 11. Schema parameter removed from `request()`, `send()`, and `callTool()`
400434

@@ -413,14 +447,14 @@ const elicit = await ctx.mcpReq.send({ method: 'elicitation/create', params: { .
413447
const tool = await client.callTool({ name: 'my-tool', arguments: {} });
414448
```
415449

416-
| v1 call | v2 call |
417-
|---------|---------|
418-
| `client.request(req, ResultSchema)` | `client.request(req)` |
419-
| `client.request(req, ResultSchema, options)` | `client.request(req, options)` |
420-
| `ctx.mcpReq.send(req, ResultSchema)` | `ctx.mcpReq.send(req)` |
421-
| `ctx.mcpReq.send(req, ResultSchema, options)` | `ctx.mcpReq.send(req, options)` |
422-
| `client.callTool(params, CompatibilityCallToolResultSchema)` | `client.callTool(params)` |
423-
| `client.callTool(params, schema, options)` | `client.callTool(params, options)` |
450+
| v1 call | v2 call |
451+
| ------------------------------------------------------------ | ---------------------------------- |
452+
| `client.request(req, ResultSchema)` | `client.request(req)` |
453+
| `client.request(req, ResultSchema, options)` | `client.request(req, options)` |
454+
| `ctx.mcpReq.send(req, ResultSchema)` | `ctx.mcpReq.send(req)` |
455+
| `ctx.mcpReq.send(req, ResultSchema, options)` | `ctx.mcpReq.send(req, options)` |
456+
| `client.callTool(params, CompatibilityCallToolResultSchema)` | `client.callTool(params)` |
457+
| `client.callTool(params, schema, options)` | `client.callTool(params, options)` |
424458

425459
Remove unused schema imports: `CallToolResultSchema`, `CompatibilityCallToolResultSchema`, `ElicitResultSchema`, `CreateMessageResultSchema`, etc., when they were only used in `request()`/`send()`/`callTool()` calls.
426460

@@ -431,16 +465,20 @@ Remove unused schema imports: `CallToolResultSchema`, `CompatibilityCallToolResu
431465
## 13. Runtime-Specific JSON Schema Validators (Enhancement)
432466

433467
The SDK now auto-selects the appropriate JSON Schema validator based on runtime:
468+
434469
- Node.js → `AjvJsonSchemaValidator` (no change from v1)
435470
- Cloudflare Workers (workerd) → `CfWorkerJsonSchemaValidator` (previously required manual config)
436471

437472
**No action required** for most users. Cloudflare Workers users can remove explicit `jsonSchemaValidator` configuration:
438473

439474
```typescript
440475
// v1 (Cloudflare Workers): Required explicit validator
441-
new McpServer({ name: 'server', version: '1.0.0' }, {
442-
jsonSchemaValidator: new CfWorkerJsonSchemaValidator()
443-
});
476+
new McpServer(
477+
{ name: 'server', version: '1.0.0' },
478+
{
479+
jsonSchemaValidator: new CfWorkerJsonSchemaValidator()
480+
}
481+
);
444482

445483
// v2 (Cloudflare Workers): Auto-selected, explicit config optional
446484
new McpServer({ name: 'server', version: '1.0.0' }, {});

‎packages/client/src/client/sse.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ export class SSEClientTransport implements Transport {
285285
// Release connection - POST responses don't have content we need
286286
await response.text?.().catch(() => {});
287287
} catch (error) {
288+
this._authRetryInFlight = false;
288289
this.onerror?.(error as Error);
289290
throw error;
290291
}

‎packages/client/src/client/streamableHttp.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,7 @@ export class StreamableHTTPClientTransport implements Transport {
600600
await response.text?.().catch(() => {});
601601
}
602602
} catch (error) {
603+
this._authRetryInFlight = false;
603604
this.onerror?.(error as Error);
604605
throw error;
605606
}

‎packages/client/test/client/tokenProvider.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,28 @@ describe('StreamableHTTPClientTransport with AuthProvider', () => {
9797
expect(authProvider.onUnauthorized).toHaveBeenCalledTimes(1);
9898
});
9999

100+
it('should reset retry guard when onUnauthorized throws, allowing retry on next send', async () => {
101+
const authProvider: AuthProvider = {
102+
token: vi.fn(async () => 'token'),
103+
onUnauthorized: vi.fn().mockRejectedValueOnce(new Error('transient network error')).mockResolvedValueOnce(undefined)
104+
};
105+
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { authProvider });
106+
vi.spyOn(globalThis, 'fetch');
107+
108+
(globalThis.fetch as Mock)
109+
.mockResolvedValueOnce({ ok: false, status: 401, headers: new Headers(), text: async () => 'unauthorized' })
110+
.mockResolvedValueOnce({ ok: false, status: 401, headers: new Headers(), text: async () => 'unauthorized' })
111+
.mockResolvedValueOnce({ ok: true, status: 202, headers: new Headers() });
112+
113+
// First send: onUnauthorized throws transient error
114+
await expect(transport.send(message)).rejects.toThrow('transient network error');
115+
expect(authProvider.onUnauthorized).toHaveBeenCalledTimes(1);
116+
117+
// Second send: flag should be reset, so onUnauthorized gets a second chance
118+
await transport.send(message);
119+
expect(authProvider.onUnauthorized).toHaveBeenCalledTimes(2);
120+
});
121+
100122
it('should work with no authProvider at all', async () => {
101123
transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'));
102124
vi.spyOn(globalThis, 'fetch');

0 commit comments

Comments
 (0)