From 0eb334c7ec80b174b639f01dcacceee309e4020c Mon Sep 17 00:00:00 2001 From: pragnyanramtha Date: Sat, 16 May 2026 15:50:41 +0000 Subject: [PATCH 1/2] fix(client): propagate refreshed token save errors --- .changeset/propagate-token-save-errors.md | 5 +++ packages/client/src/client/auth.ts | 11 +++-- packages/client/test/client/auth.test.ts | 52 +++++++++++++++++++++++ 3 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 .changeset/propagate-token-save-errors.md diff --git a/.changeset/propagate-token-save-errors.md b/.changeset/propagate-token-save-errors.md new file mode 100644 index 0000000000..91637dac06 --- /dev/null +++ b/.changeset/propagate-token-save-errors.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Propagate token persistence errors after a successful OAuth refresh instead of silently falling back to a new authorization flow. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 5f55fb7a08..20468e01ba 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -763,9 +763,10 @@ async function authInternal( // Handle token refresh or new authorization if (tokens?.refresh_token) { + let newTokens: OAuthTokens | undefined; try { // Attempt to refresh the token - const newTokens = await refreshAuthorization(authorizationServerUrl, { + newTokens = await refreshAuthorization(authorizationServerUrl, { metadata, clientInformation, refreshToken: tokens.refresh_token, @@ -773,9 +774,6 @@ async function authInternal( addClientAuthentication: provider.addClientAuthentication, fetchFn }); - - await provider.saveTokens(newTokens); - return 'AUTHORIZED'; } catch (error) { // If this is a ServerError, or an unknown type, log it out and try to continue. Otherwise, escalate so we can fix things and retry. if (!(error instanceof OAuthError) || error.code === OAuthErrorCode.ServerError) { @@ -785,6 +783,11 @@ async function authInternal( throw error; } } + + if (newTokens) { + await provider.saveTokens(newTokens); + return 'AUTHORIZED'; + } } const state = provider.state ? await provider.state() : undefined; diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 04d7f4a3fb..f5508f20ac 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -2158,6 +2158,58 @@ describe('OAuth Authorization', () => { vi.clearAllMocks(); }); + it('propagates saveTokens errors after a successful refresh', async () => { + const saveError = new Error('failed to persist refreshed tokens'); + const provider: OAuthClientProvider = { + get redirectUrl() { + return 'http://localhost:3000/callback'; + }, + get clientMetadata() { + return { + redirect_uris: ['http://localhost:3000/callback'], + client_name: 'Test Client' + }; + }, + discoveryState: vi.fn().mockResolvedValue({ + authorizationServerUrl: 'https://auth.example.com', + authorizationServerMetadata: { + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'] + } + }), + clientInformation: vi.fn().mockResolvedValue({ + client_id: 'test-client-id', + client_secret: 'test-client-secret' + }), + tokens: vi.fn().mockResolvedValue({ + access_token: 'expired-token', + refresh_token: 'refresh-token', + token_type: 'bearer' + }), + saveTokens: vi.fn().mockRejectedValue(saveError), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn() + }; + + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ + access_token: 'new-token', + token_type: 'bearer', + expires_in: 3600, + refresh_token: 'new-refresh-token' + }) + }); + + await expect(auth(provider, { serverUrl: 'https://resource.example.com' })).rejects.toThrow(saveError); + expect(provider.redirectToAuthorization).not.toHaveBeenCalled(); + }); + it('performs client_credentials with private_key_jwt when provider has addClientAuthentication', async () => { // Arrange: metadata discovery for PRM and AS mockFetch.mockImplementation(url => { From 5e25f0e6e86a8da796e477f7c7f58b1027362293 Mon Sep 17 00:00:00 2001 From: pragnyanramtha Date: Tue, 19 May 2026 02:24:21 +0000 Subject: [PATCH 2/2] test: stabilize Cloudflare Workers integration startup --- .../test/server/cloudflareWorkers.test.ts | 137 ++++++++++++------ 1 file changed, 93 insertions(+), 44 deletions(-) diff --git a/test/integration/test/server/cloudflareWorkers.test.ts b/test/integration/test/server/cloudflareWorkers.test.ts index 9c2d73a40e..a5261f8240 100644 --- a/test/integration/test/server/cloudflareWorkers.test.ts +++ b/test/integration/test/server/cloudflareWorkers.test.ts @@ -8,25 +8,45 @@ import type { ChildProcess } from 'node:child_process'; import { execSync, spawn } from 'node:child_process'; import * as fs from 'node:fs'; +import { createServer } from 'node:net'; import * as os from 'node:os'; import path from 'node:path'; import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; -const PORT = 8787; - interface TestEnv { tempDir: string; process: ChildProcess; + port: number; + stdout: string; + stderr: string; cleanup: () => Promise; } +async function getAvailablePort(): Promise { + return new Promise((resolve, reject) => { + const server = createServer(); + server.once('error', reject); + server.listen(0, '127.0.0.1', () => { + const address = server.address(); + server.close(() => { + if (address && typeof address === 'object') { + resolve(address.port); + } else { + reject(new Error('Unable to allocate a port for wrangler')); + } + }); + }); + }); +} + describe('Cloudflare Workers compatibility (no nodejs_compat)', () => { let env: TestEnv | null = null; beforeAll(async () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cf-worker-test-')); + const port = await getAvailablePort(); // Pack server package const serverPkgPath = path.resolve(__dirname, '../../../../packages/server'); @@ -85,52 +105,16 @@ export default { execSync('npm install', { cwd: tempDir, stdio: 'pipe', timeout: 60_000 }); // Start wrangler dev server - const proc = spawn('npx', ['wrangler', 'dev', '--local', '--port', String(PORT)], { + const proc = spawn('npx', ['wrangler', 'dev', '--local', '--port', String(port)], { cwd: tempDir, shell: true, stdio: 'pipe' }); - // Wait for server to be ready - await new Promise((resolve, reject) => { - const timeout = setTimeout(() => reject(new Error('Wrangler startup timeout')), 60_000); - let stderrData = ''; - - proc.stdout?.on('data', data => { - const output = data.toString(); - if (/Ready on|Listening on/.test(output)) { - clearTimeout(timeout); - // Extra delay for wrangler to fully initialize - setTimeout(resolve, 1000); - } - }); - - proc.stderr?.on('data', data => { - stderrData += data.toString(); - // Check for fatal errors like missing node: modules - if (/No such module "node:/.test(stderrData)) { - clearTimeout(timeout); - reject(new Error(`Wrangler fatal error: ${stderrData}`)); - } - }); - - proc.on('error', err => { - clearTimeout(timeout); - reject(err); - }); - - proc.on('close', code => { - if (code !== 0 && code !== null) { - clearTimeout(timeout); - reject(new Error(`Wrangler exited with code ${code}. stderr: ${stderrData}`)); - } - }); - }); - const cleanup = async () => { proc.kill('SIGTERM'); await new Promise(resolve => { - proc.on('close', () => resolve()); + proc.once('close', () => resolve()); setTimeout(resolve, 5000); }); try { @@ -140,7 +124,69 @@ export default { } }; - env = { tempDir, process: proc, cleanup }; + env = { tempDir, process: proc, port, stdout: '', stderr: '', cleanup }; + + // Wait for server to be ready + try { + await new Promise((resolve, reject) => { + let settled = false; + const timeout = setTimeout(() => { + if (settled) { + return; + } + settled = true; + reject(new Error(`Wrangler startup timeout. stdout: ${env?.stdout}\nstderr: ${env?.stderr}`)); + }, 60_000); + const settle = (callback: () => void) => { + if (settled) { + return; + } + settled = true; + clearTimeout(timeout); + callback(); + }; + + proc.stdout?.on('data', data => { + const output = data.toString(); + if (env) { + env.stdout += output; + } + if (/Ready on|Listening on/.test(output)) { + settle(() => { + // Extra delay for wrangler to fully initialize + setTimeout(resolve, 1000); + }); + } + }); + + proc.stderr?.on('data', data => { + const output = data.toString(); + if (env) { + env.stderr += output; + } + // Check for fatal errors like missing node: modules + if (/No such module "node:/.test(env?.stderr ?? '')) { + settle(() => reject(new Error(`Wrangler fatal error: ${env?.stderr}`))); + } + }); + + proc.on('error', err => { + settle(() => reject(err)); + }); + + proc.on('close', code => { + if (code !== 0 && code !== null) { + settle(() => + reject(new Error(`Wrangler exited with code ${code}. stdout: ${env?.stdout}\nstderr: ${env?.stderr}`)) + ); + } + }); + }); + } catch (error) { + await cleanup(); + env = null; + throw error; + } }, 120_000); afterAll(async () => { @@ -153,20 +199,23 @@ export default { // Retry connection — wrangler may report "Ready" before it can handle requests let client!: Client; let lastError: unknown; - for (let attempt = 0; attempt < 5; attempt++) { + for (let attempt = 0; attempt < 10; attempt++) { try { client = new Client({ name: 'test-client', version: '1.0.0' }); - const transport = new StreamableHTTPClientTransport(new URL(`http://127.0.0.1:${PORT}/`)); + const transport = new StreamableHTTPClientTransport(new URL(`http://127.0.0.1:${env!.port}/`)); await client.connect(transport); lastError = undefined; break; } catch (error) { lastError = error; + await client?.close().catch(() => {}); await new Promise(resolve => setTimeout(resolve, 1000)); } } if (lastError) { - throw lastError; + throw new Error(`Unable to connect to wrangler MCP worker. stdout: ${env?.stdout}\nstderr: ${env?.stderr}`, { + cause: lastError + }); } const result = await client.callTool({ name: 'greet', arguments: { name: 'World' } });