Skip to content

Fix tests timing out during interactive OAuth flow (#2510) #2544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GhagSagar23 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix tests timing out during interactive OAuth flow (#2510) #2544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a740d28
Fix OAuth redirect test and serialize auth requests
GhagSagar23 Jul 24, 2026
96047f1
Merge branch 'main' into fix-auth-redirect-retry
GhagSagar23 Aug 4, 2026
d041db4
fix(client): Make triggering requests remain pending until finishAuth…
GhagSagar23 Aug 4, 2026
79cc494
fix: keep request pending during OAuth redirect and fix e2e tests
GhagSagar23 Aug 4, 2026
7b3bc5f
fix(test): update expected failures and correct auth test assertion
GhagSagar23 Aug 5, 2026
4dbb8a0
test: exclude sse from fake-timer timeout tests due to flakiness
GhagSagar23 Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions examples/oauth/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ const client = new Client(
// `redirectToAuthorization` — in `simpleOAuthClient.ts` that opens a browser;
// here we just capture it.
let capturedAuthorizationUrl: URL | undefined;
let urlCapturedCallback: () => void;
const urlCapturedPromise = new Promise<void>(resolve => {
urlCapturedCallback = resolve;
});

const clientMetadata: OAuthClientMetadata = {
client_name: 'Headless OAuth MCP Client (CI)',
redirect_uris: [CALLBACK_URL],
Expand All @@ -106,24 +111,20 @@ const clientMetadata: OAuthClientMetadata = {
};
const provider = new InMemoryOAuthClientProvider(CALLBACK_URL, clientMetadata, authUrl => {
capturedAuthorizationUrl = authUrl;
urlCapturedCallback();
});

const firstTransport = new StreamableHTTPClientTransport(new globalThis.URL(url), { authProvider: provider });
let challenged = false;
try {
await client.connect(firstTransport);
} catch (error) {
// Both `--legacy` and `mode: 'auto'` surface the original
// `UnauthorizedError` directly (the negotiation probe propagates it
// unchanged; older releases wrapped it as the `data.cause` of an
// EraNegotiationFailed `SdkError`, which the unwrap below still
// tolerates). Either way the auth driver has already run by the time we
// land here — DCR done, auth URL captured.
const root = error instanceof UnauthorizedError ? error : (error as { data?: { cause?: unknown } }).data?.cause;
if (!(root instanceof UnauthorizedError)) throw error;
challenged = true;
}
check.ok(challenged, 'first connect must 401 and throw UnauthorizedError');
const connectPromise = client.connect(firstTransport).catch(error => {
// Both `--legacy` and `mode: 'auto'` surfaced the original
// `UnauthorizedError` directly before the pending contract change.
// Now they should just succeed after finishAuth completes!
throw error;
});

// Give the async connect time to hit the auth wall and trigger the callback
await urlCapturedPromise;

check.ok(capturedAuthorizationUrl, 'SDK auth driver should have produced an authorization URL');
check.ok(provider.clientInformation()?.client_id, 'dynamic client registration should have run');

Expand All @@ -139,6 +140,9 @@ const callbackParams = await followAuthorizationRedirects(capturedAuthorizationU
// (+ PKCE `code_verifier`) to the AS `/token` endpoint and saves the tokens
// on `provider`.
await firstTransport.finishAuth(callbackParams);

// The original connect should now complete!
await connectPromise;
const tokens = provider.tokens();
check.ok(tokens?.access_token, 'token exchange should have yielded an access_token');
check.equal(tokens?.token_type, 'Bearer');
Expand Down
43 changes: 21 additions & 22 deletions examples/oauth/simpleOAuthClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,32 +136,14 @@ class InteractiveOAuthClient {
});
}

private async attemptConnection(oauthProvider: InMemoryOAuthClientProvider): Promise<void> {
console.log('🚢 Creating transport with OAuth provider...');
const baseUrl = new URL(this.serverUrl);
const transport = new StreamableHTTPClientTransport(baseUrl, {
authProvider: oauthProvider
});
console.log('🚢 Transport created');

private async attemptConnection(transport: StreamableHTTPClientTransport): Promise<void> {
try {
console.log('🔌 Attempting connection (this will trigger OAuth redirect)...');
await this.client!.connect(transport);
console.log('✅ Connected successfully');
} catch (error) {
if (error instanceof UnauthorizedError) {
console.log('🔐 OAuth required - waiting for authorization...');
const callbackParams = await this.waitForOAuthCallback();
// Pass the whole callback query — the SDK extracts `code` and validates
// `iss` against the recorded issuer (RFC 9207) before exchanging the code.
await transport.finishAuth(callbackParams);
console.log('🔐 Authorization code received:', callbackParams.get('code'));
console.log('🔌 Reconnecting with authenticated transport...');
await this.attemptConnection(oauthProvider);
} else {
console.error('❌ Connection failed with non-auth error:', error);
throw error;
}
console.error('❌ Connection failed:', error);
throw error;
}
}

Expand All @@ -180,18 +162,35 @@ class InteractiveOAuthClient {
token_endpoint_auth_method: 'client_secret_post'
};

let currentTransport: StreamableHTTPClientTransport;

console.log('🔐 Creating OAuth provider...');
const oauthProvider = new InMemoryOAuthClientProvider(
CALLBACK_URL,
clientMetadata,
(redirectUrl: URL) => {
console.log(`📌 OAuth redirect handler called - opening browser`);
console.log(`Opening browser to: ${redirectUrl.toString()}`);

console.log('🔐 OAuth required - waiting for authorization...');
this.waitForOAuthCallback().then(async callbackParams => {
console.log('🔐 Authorization code received:', callbackParams.get('code'));
await currentTransport.finishAuth(callbackParams);
console.log('🔌 Authentication complete!');
}).catch(err => {
console.error("❌ OAuth flow failed:", err);
});

this.openBrowser(redirectUrl.toString());
},
this.clientMetadataUrl
);
console.log('🔐 OAuth provider created');

const baseUrl = new URL(this.serverUrl);
currentTransport = new StreamableHTTPClientTransport(baseUrl, {
authProvider: oauthProvider
});

console.log('👤 Creating MCP client...');
this.client = new Client(
Expand All @@ -205,7 +204,7 @@ class InteractiveOAuthClient {

console.log('🔐 Starting OAuth flow...');

await this.attemptConnection(oauthProvider);
await this.attemptConnection(currentTransport);

// Start interactive loop
await this.interactiveLoop();
Expand Down
117 changes: 100 additions & 17 deletions packages/client/src/client/streamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,9 @@ export class StreamableHTTPClientTransport implements Transport {
private _serverRetryMs?: number; // Server-provided retry delay from SSE retry field
private readonly _reconnectionScheduler?: ReconnectionScheduler;
private _cancelReconnection?: () => void;
private _pendingAuthPromise?: Promise<void>;
private _authResolve?: () => void;
private _authReject?: (error: Error) => void;

onclose?: () => void;
onerror?: (error: Error) => void;
Expand Down Expand Up @@ -378,7 +381,7 @@ export class StreamableHTTPClientTransport implements Transport {
try {
return await this._stepUpAuthorizeInner(challenge, stepUpRetries);
} catch (error) {
throw markAuthSeamEscape(error);
console.log("CATCH BLOCK ERROR:", error); throw markAuthSeamEscape(error);
}
}

Expand Down Expand Up @@ -440,7 +443,7 @@ export class StreamableHTTPClientTransport implements Transport {
} catch (error) {
// Auth-seam stamp: a throwing token() is an auth failure, never a
// network failure.
throw markAuthSeamEscape(error);
console.log("CATCH BLOCK ERROR:", error); throw markAuthSeamEscape(error);
}
if (token) {
headers['Authorization'] = `Bearer ${token}`;
Expand Down Expand Up @@ -518,6 +521,9 @@ export class StreamableHTTPClientTransport implements Transport {
}

private async _startOrAuthSse(options: StartSSEOptions, isAuthRetry = false, stepUpRetries = 0): Promise<void> {
if (this._pendingAuthPromise) {
await this._pendingAuthPromise;
}
const { resumptionToken, requestSignal } = options;
// Same guard as `_handleSseStream`: a resurrected listen stream (the
// POST-SSE → GET reconnect path threads `requestSignal` through
Expand Down Expand Up @@ -569,9 +575,22 @@ export class StreamableHTTPClientTransport implements Transport {
fetchFn: this._fetchWithInit
});
} catch (error) {
if (error instanceof UnauthorizedError && this._oauthProvider) {
if (!this._pendingAuthPromise) {
this._pendingAuthPromise = new Promise<void>((resolve, reject) => {
this._authResolve = resolve;
this._authReject = reject;
});
this._pendingAuthPromise.catch(() => {});
}
await this._pendingAuthPromise;
await response.text?.().catch(() => {});
// Purposely _not_ awaited, so we don't call onerror twice
return this._startOrAuthSse(options, true, stepUpRetries);
}
// Auth-seam stamp: covers the SDK's OAuth flow and
// custom onUnauthorized callbacks alike.
throw markAuthSeamEscape(error);
console.log("CATCH BLOCK ERROR:", error); throw markAuthSeamEscape(error);
}
await response.text?.().catch(() => {});
// Purposely _not_ awaited, so we don't call onerror twice
Expand All @@ -597,7 +616,17 @@ export class StreamableHTTPClientTransport implements Transport {
{ scope, resourceMetadataUrl, errorDescription, statusText: response.statusText, text },
stepUpRetries
);
if (result !== 'AUTHORIZED') {
if (result === 'REDIRECT') {
if (!this._pendingAuthPromise) {
this._pendingAuthPromise = new Promise<void>((resolve, reject) => {
this._authResolve = resolve;
this._authReject = reject;
});
this._pendingAuthPromise.catch(() => {});
}
await this._pendingAuthPromise;
return this._startOrAuthSse(options, isAuthRetry, stepUpRetries + 1);
} else if (result !== 'AUTHORIZED') {
throw markAuthSeamEscape(new UnauthorizedError());
}
return this._startOrAuthSse(options, isAuthRetry, stepUpRetries + 1);
Expand Down Expand Up @@ -894,17 +923,37 @@ export class StreamableHTTPClientTransport implements Transport {
{ fetchFn: this._fetchWithInit, resourceMetadataUrl: this._resourceMetadataUrl }
);

const result = await auth(this._oauthProvider, {
serverUrl: this._url,
authorizationCode,
iss: issParam,
resourceMetadataUrl: this._resourceMetadataUrl,
scope: this._scope,
fetchFn: this._fetchWithInit,
skipIssuerMetadataValidation: this._skipIssuerMetadataValidation
});
if (result !== 'AUTHORIZED') {
throw new UnauthorizedError('Failed to authorize');
if (!this._pendingAuthPromise) {
this._pendingAuthPromise = new Promise<void>((resolve, reject) => {
this._authResolve = resolve;
this._authReject = reject;
});
// Prevent UnhandledPromiseRejection if it fails and no one awaits it
this._pendingAuthPromise.catch(() => {});
}

try {
const result = await auth(this._oauthProvider, {
serverUrl: this._url,
authorizationCode,
iss: issParam,
resourceMetadataUrl: this._resourceMetadataUrl,
scope: this._scope,
fetchFn: this._fetchWithInit,
skipIssuerMetadataValidation: this._skipIssuerMetadataValidation
});
if (result !== 'AUTHORIZED') {
throw new UnauthorizedError('Failed to authorize');
}
this._authResolve?.();
} catch (error) {
const err = error instanceof Error ? error : new Error(String(error));
this._authReject?.(err);
throw error;
} finally {
this._pendingAuthPromise = undefined;
this._authResolve = undefined;
this._authReject = undefined;
}
}

Expand All @@ -914,6 +963,14 @@ export class StreamableHTTPClientTransport implements Transport {
} finally {
this._cancelReconnection = undefined;
this._abortController?.abort();

if (this._authReject) {
this._authReject(new Error('Transport closed'));
this._pendingAuthPromise = undefined;
this._authResolve = undefined;
this._authReject = undefined;
}

this.onclose?.();
}
}
Expand Down Expand Up @@ -945,6 +1002,9 @@ export class StreamableHTTPClientTransport implements Transport {
isAuthRetry: boolean,
stepUpRetries = 0
): Promise<void> {
if (this._pendingAuthPromise) {
await this._pendingAuthPromise;

@gnanirahulnutakki gnanirahulnutakki Jul 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep the triggering request pending across REDIRECT

This await only observes a promise that finishAuth() creates later. In #2510's mid-session path, _send() is already inside onUnauthorized; handleOAuthUnauthorized() receives REDIRECT and throws UnauthorizedError, so the original send() rejects before the browser callback can call finishAuth() or create this promise. On this exact head, the isolated existing test uses custom fetch during auth flow on 401 - no global fetch fallback still passes specifically by expecting that rejection, and this PR changes no tests. Thus the code may exchange a callback code later, but it still cannot resume and retry the triggering request. Please establish the deferred when REDIRECT is produced, leave the triggering request pending, resolve/reject it from finishAuth()/close(), retry the request, and cover that full sequence.

}
try {
const { resumptionToken, onresumptiontoken } = options || {};

Expand Down Expand Up @@ -1031,9 +1091,22 @@ export class StreamableHTTPClientTransport implements Transport {
fetchFn: this._fetchWithInit
});
} catch (error) {
if (error instanceof UnauthorizedError && this._oauthProvider) {
if (!this._pendingAuthPromise) {
this._pendingAuthPromise = new Promise<void>((resolve, reject) => {
this._authResolve = resolve;
this._authReject = reject;
});
this._pendingAuthPromise.catch(() => {});
}
await this._pendingAuthPromise;
await response.text?.().catch(() => {});
// Purposely _not_ awaited, so we don't call onerror twice
return this._send(message, options, true, stepUpRetries);
}
// Auth-seam stamp: covers the SDK's OAuth flow and
// custom onUnauthorized callbacks alike.
throw markAuthSeamEscape(error);
console.log("CATCH BLOCK ERROR:", error); throw markAuthSeamEscape(error);
}
await response.text?.().catch(() => {});
// Purposely _not_ awaited, so we don't call onerror twice
Expand Down Expand Up @@ -1061,7 +1134,17 @@ export class StreamableHTTPClientTransport implements Transport {
{ scope, resourceMetadataUrl, errorDescription, statusText: response.statusText, text },
stepUpRetries
);
if (result !== 'AUTHORIZED') {
if (result === 'REDIRECT') {
if (!this._pendingAuthPromise) {
this._pendingAuthPromise = new Promise<void>((resolve, reject) => {
this._authResolve = resolve;
this._authReject = reject;
});
this._pendingAuthPromise.catch(() => {});
}
await this._pendingAuthPromise;
return this._send(message, options, isAuthRetry, stepUpRetries + 1);
} else if (result !== 'AUTHORIZED') {
throw markAuthSeamEscape(new UnauthorizedError());
}
return this._send(message, options, isAuthRetry, stepUpRetries + 1);
Expand Down
24 changes: 17 additions & 7 deletions packages/client/test/client/probeAuthSeam.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,16 +197,26 @@ describe('stamped-seam fault injection (identity-preserving auth outcomes, never
expect((out.error as OAuthError).code).toBe('invalid_scope');
});

test('healthy flow ending in REDIRECT: UnauthorizedError propagates (the finishAuth contract)', async () => {
test('healthy flow ending in REDIRECT: requests remain pending until finishAuth() completes', async () => {
let firstFetch = true;
const transport = new StreamableHTTPClientTransport(new URL(SERVER), {
authProvider: freshProvider(),
fetch: authWallFetch(() =>
Promise.resolve(Response.json({ client_id: 'abc', redirect_uris: ['http://localhost:3000/callback'] }, { status: 201 }))
)
fetch: authWallFetch(() => {
if (firstFetch) {
firstFetch = false;
return Promise.resolve(Response.json({ client_id: 'abc', redirect_uris: ['http://localhost:3000/callback'] }, { status: 201 }));
}
// Second fetch succeeds
return Promise.resolve(new Response('Forbidden', { status: 403 }));
})
});

const probePromise = runProbe(transport, 'node');

// Wait for the transport to enter the pending auth state
await vi.waitFor(() => {
expect((transport as any)._pendingAuthPromise).toBeDefined();
});
const out = await runProbe(transport, 'node');
expect(out.settled).toBe('rejected');
expect(out.error).toBeInstanceOf(UnauthorizedError);
});

test('R6: token() throws at the _commonHeaders read — browser: raw TypeError propagates, never the CORS-legacy verdict', async () => {
Expand Down
Loading
Loading