fix(mcp): require ready connection after remote MCP OAuth callback

docs/contributing/architecture/mcp-client-servers.md

@@ -42,9 +42,16 @@ the `/mcp` endpoint (where Kody is the server) and complements remote connectors
    the browser session cookie, forwards the full callback URL to that user's hub
    DO, and the SDK exchanges the code (matching the `state` parameter to the
    pending authorization) and establishes the connection.
-5. The route redirects to `/account/mcp-servers?auth=success|error` for user
-   feedback. Tokens live only in the DO storage; they never reach D1 or the
-   client.
+5. The hub only treats the callback as successful when the connection reaches
+   `ready`. If the Agents SDK reports `authSuccess` but the connection stays in
+   `authenticating` (including the stuck case with no stored auth URL after the
+   SDK clears it), the route redirects with `auth=error` and a concrete reason.
+   Reconnect also recovers that stuck state by invalidating unusable tokens and
+   requesting a fresh authorization URL.
+6. The route redirects to `/account/mcp-servers/:serverId?auth=success|error`
+   when the callback resolves to a server (including failures), or
+   `/account/mcp-servers?auth=error` when it does not, for user feedback. Tokens
+   live only in the DO storage; they never reach D1 or the client.
 
 Because the callback is resolved through the session cookie, the OAuth state is
 always looked up in the hub belonging to the signed-in user — cross-user

packages/worker/client/routes/account-mcp-servers.tsx

@@ -631,6 +631,14 @@ export function AccountMcpServersRoute(handle: Handle) {
 									</AccountManagementMessage>
 								) : null}
 
+								{server.state === 'authenticating' && !server.authUrl ? (
+									<AccountManagementMessage tone="error">
+										Authorization is incomplete and no authorization link is
+										available. Click Reconnect to restart OAuth, or remove and
+										add the server again.
+									</AccountManagementMessage>
+								) : null}
+
 								{server.authUrl && server.state === 'authenticating' ? (
 									<div
 										mix={css({

packages/worker/src/app/handlers/account-mcp-servers.node.test.ts

@@ -275,6 +275,29 @@ test('MCP servers OAuth callback redirects with the auth outcome', async () => {
 	} as never)
 	expect(failureResponse.status).toBe(303)
 	const failureLocation = new URL(failureResponse.headers.get('Location') ?? '')
+	expect(failureLocation.pathname).toBe('/account/mcp-servers')
 	expect(failureLocation.searchParams.get('auth')).toBe('error')
 	expect(failureLocation.searchParams.get('reason')).toBe('Invalid state.')
+
+	mockModule.handleOAuthCallback.mockResolvedValueOnce({
+		serverId: 'server-1',
+		authSuccess: false,
+		authError: 'Authorization completed, but no authorization link.',
+		serverName: 'linear',
+	})
+	const incompleteResponse = await handler.handler({
+		request: new Request(
+			'https://example.com/account/mcp-servers/oauth/callback?code=abc&state=xyz.server-1',
+		),
+		params: {},
+	} as never)
+	expect(incompleteResponse.status).toBe(303)
+	const incompleteLocation = new URL(
+		incompleteResponse.headers.get('Location') ?? '',
+	)
+	expect(incompleteLocation.pathname).toBe('/account/mcp-servers/server-1')
+	expect(incompleteLocation.searchParams.get('auth')).toBe('error')
+	expect(incompleteLocation.searchParams.get('reason')).toContain(
+		'no authorization link',
+	)
 })

packages/worker/src/app/handlers/account-mcp-servers.ts

@@ -143,13 +143,12 @@ export function createAccountMcpServersOauthCallbackHandler(env: Env) {
 				authError = getErrorMessage(error)
 			}
 
-			const target =
-				authSuccess && serverId
-					? new URL(
-							`/account/mcp-servers/${encodeURIComponent(serverId)}`,
-							request.url,
-						)
-					: new URL('/account/mcp-servers', request.url)
+			const target = serverId
+				? new URL(
+						`/account/mcp-servers/${encodeURIComponent(serverId)}`,
+						request.url,
+					)
+				: new URL('/account/mcp-servers', request.url)
 			if (authSuccess) {
 				target.searchParams.set('auth', 'success')
 				if (serverName) {

packages/worker/src/mcp-client/hub.ts

@@ -4,6 +4,10 @@ import { MCPClientManager } from 'agents/mcp/client'
 import { DurableObjectOAuthClientProvider } from 'agents/mcp/do-oauth-client-provider'
 import { type CallToolResult } from '@modelcontextprotocol/sdk/types.js'
 import { buildSentryOptions } from '#worker/sentry-options.ts'
+import {
+	isStuckMcpAuthenticatingWithoutAuthUrl,
+	resolveMcpOAuthCallbackOutcome,
+} from './oauth-callback-outcome.ts'
 import {
 	type McpClientHubSnapshot,
 	type McpServerConnectResult,
@@ -182,7 +186,11 @@ class McpClientHubBase extends DurableObject<Env> {
 				})
 			}
 		}
-		return this.buildConnectResult(input.serverId)
+		const connected = this.buildConnectResult(input.serverId)
+		if (isStuckMcpAuthenticatingWithoutAuthUrl(connected)) {
+			return await this.recoverStuckAuthenticating(input.serverId)
+		}
+		return connected
 	}
 
 	/** Re-discover tools for a connected server. */
@@ -208,6 +216,11 @@ class McpClientHubBase extends DurableObject<Env> {
 	 * Complete an OAuth authorization redirect. The worker forwards the
 	 * callback URL (including `code` and `state`) after authenticating the
 	 * browser session that owns this hub.
+	 *
+	 * Success is reported only when the MCP connection reaches `ready`. The
+	 * Agents SDK `authSuccess` flag alone is not enough: it can clear the
+	 * stored auth URL and leave the connection in `authenticating` with no
+	 * error after a provider (e.g. Clerk) authorization redirect.
 	 */
 	async handleOAuthCallback(input: {
 		url: string
@@ -233,13 +246,54 @@ class McpClientHubBase extends DurableObject<Env> {
 			await this.manager.waitForConnections({
 				timeout: connectionSettleTimeoutMs,
 			})
+			let connection = this.buildConnectResult(serverId)
+			if (isStuckMcpAuthenticatingWithoutAuthUrl(connection)) {
+				connection = await this.recoverStuckAuthenticating(serverId)
+			}
+			return resolveMcpOAuthCallbackOutcome({
+				sdkAuthSuccess: true,
+				sdkAuthError: result.authError ?? null,
+				serverId,
+				serverName,
+				connection,
+			})
 		}
-		return {
+		return resolveMcpOAuthCallbackOutcome({
+			sdkAuthSuccess: result.authSuccess,
+			sdkAuthError: result.authError ?? null,
 			serverId,
-			authSuccess: result.authSuccess,
-			authError: result.authError ?? null,
 			serverName,
+			connection: serverId ? this.buildConnectResult(serverId) : null,
+		})
+	}
+
+	/**
+	 * Clear unusable tokens and reconnect so the Agents SDK can mint a fresh
+	 * auth URL. Needed when SQL `auth_url` was cleared on callback success but
+	 * the live connection never reached `ready`.
+	 */
+	private async recoverStuckAuthenticating(
+		serverId: string,
+	): Promise<McpServerConnectResult> {
+		const connection = this.manager.mcpConnections[serverId]
+		const authProvider = connection?.options.transport.authProvider
+		if (
+			authProvider &&
+			typeof authProvider.invalidateCredentials === 'function'
+		) {
+			try {
+				await authProvider.invalidateCredentials('tokens')
+			} catch {
+				// Best-effort: reconnect below still regenerates OAuth when possible.
+			}
+		}
+		const result = await this.manager.connectToServer(serverId)
+		if (result.state === 'connected') {
+			await this.manager.discoverIfConnected(serverId, {
+				timeoutMs: discoverTimeoutMs,
+			})
 		}
+		return this.buildConnectResult(serverId)
 	}
 
 	async getSnapshot(): Promise<McpClientHubSnapshot> {

packages/worker/src/mcp-client/oauth-callback-outcome.node.test.ts

@@ -0,0 +1,115 @@
+import { expect, test } from 'vitest'
+import {
+	describeIncompleteMcpOAuthConnection,
+	isStuckMcpAuthenticatingWithoutAuthUrl,
+	resolveMcpOAuthCallbackOutcome,
+} from './oauth-callback-outcome.ts'
+
+test('OAuth callback success requires the MCP connection to be ready', () => {
+	expect(
+		resolveMcpOAuthCallbackOutcome({
+			sdkAuthSuccess: true,
+			sdkAuthError: null,
+			serverId: 'server-1',
+			serverName: 'recipe-keeper',
+			connection: {
+				state: 'ready',
+				authUrl: null,
+				error: null,
+			},
+		}),
+	).toEqual({
+		serverId: 'server-1',
+		authSuccess: true,
+		authError: null,
+		serverName: 'recipe-keeper',
+	})
+})
+
+test('OAuth callback does not report success when left authenticating without authUrl', () => {
+	const outcome = resolveMcpOAuthCallbackOutcome({
+		sdkAuthSuccess: true,
+		sdkAuthError: null,
+		serverId: 'server-1',
+		serverName: 'recipe-keeper',
+		connection: {
+			state: 'authenticating',
+			authUrl: null,
+			error: null,
+		},
+	})
+
+	expect(outcome.authSuccess).toBe(false)
+	expect(outcome.serverId).toBe('server-1')
+	expect(outcome.serverName).toBe('recipe-keeper')
+	expect(outcome.authError).toContain('no authorization link')
+	expect(
+		isStuckMcpAuthenticatingWithoutAuthUrl({
+			state: 'authenticating',
+			authUrl: null,
+		}),
+	).toBe(true)
+})
+
+test('OAuth callback surfaces connection errors after SDK authSuccess', () => {
+	expect(
+		resolveMcpOAuthCallbackOutcome({
+			sdkAuthSuccess: true,
+			sdkAuthError: null,
+			serverId: 'server-1',
+			serverName: 'recipe-keeper',
+			connection: {
+				state: 'failed',
+				authUrl: null,
+				error: 'Token exchange failed.',
+			},
+		}),
+	).toEqual({
+		serverId: 'server-1',
+		authSuccess: false,
+		authError: 'Token exchange failed.',
+		serverName: 'recipe-keeper',
+	})
+})
+
+test('OAuth callback keeps SDK failures as failures', () => {
+	expect(
+		resolveMcpOAuthCallbackOutcome({
+			sdkAuthSuccess: false,
+			sdkAuthError: 'Invalid state',
+			serverId: 'server-1',
+			serverName: 'recipe-keeper',
+			connection: {
+				state: 'authenticating',
+				authUrl: 'https://auth.example/authorize',
+				error: null,
+			},
+		}),
+	).toEqual({
+		serverId: 'server-1',
+		authSuccess: false,
+		authError: 'Invalid state',
+		serverName: 'recipe-keeper',
+	})
+})
+
+test('incomplete OAuth connection messages distinguish missing auth URLs', () => {
+	expect(
+		describeIncompleteMcpOAuthConnection({
+			state: 'authenticating',
+			authUrl: 'https://auth.example/authorize',
+		}),
+	).toContain('still requires authorization')
+	expect(
+		describeIncompleteMcpOAuthConnection({
+			state: 'authenticating',
+			authUrl: null,
+		}),
+	).toContain('no authorization link')
+	expect(
+		describeIncompleteMcpOAuthConnection({
+			state: 'connecting',
+			authUrl: null,
+		}),
+	).toContain('"connecting"')
+})

packages/worker/src/mcp-client/oauth-callback-outcome.ts

@@ -0,0 +1,91 @@
+import {
+	type McpServerConnectionState,
+	type McpServerOAuthCallbackOutcome,
+} from './types.ts'
+
+/**
+ * Decide the user-facing OAuth callback outcome from the Agents SDK result
+ * plus the post-establish connection snapshot.
+ *
+ * The SDK's `authSuccess` only means the authorization code was accepted (or
+ * treated as already accepted). Kody must not report success until the MCP
+ * connection is actually `ready`. Otherwise users can land on
+ * "Authorization required" with no auth URL and no error — the Clerk / Convex
+ * MCP failure mode reported by Bernardo.
+ */
+export function resolveMcpOAuthCallbackOutcome(input: {
+	sdkAuthSuccess: boolean
+	sdkAuthError: string | null
+	serverId: string | null
+	serverName: string | null
+	connection: {
+		state: McpServerConnectionState
+		authUrl: string | null
+		error: string | null
+	} | null
+}): McpServerOAuthCallbackOutcome {
+	const { serverId, serverName } = input
+
+	if (!input.sdkAuthSuccess || !serverId) {
+		return {
+			serverId,
+			authSuccess: false,
+			authError: input.sdkAuthError ?? 'Authorization failed.',
+			serverName,
+		}
+	}
+
+	if (!input.connection) {
+		return {
+			serverId,
+			authSuccess: false,
+			authError:
+				'Authorization completed, but Kody lost the MCP server connection. Try reconnecting from /account/mcp-servers.',
+			serverName,
+		}
+	}
+
+	if (input.connection.state === 'ready') {
+		return {
+			serverId,
+			authSuccess: true,
+			authError: null,
+			serverName,
+		}
+	}
+
+	return {
+		serverId,
+		authSuccess: false,
+		authError:
+			input.connection.error ??
+			describeIncompleteMcpOAuthConnection(input.connection),
+		serverName,
+	}
+}
+
+export function describeIncompleteMcpOAuthConnection(connection: {
+	state: McpServerConnectionState
+	authUrl: string | null
+}): string {
+	if (connection.state === 'authenticating' && connection.authUrl) {
+		return 'Authorization completed at the identity provider, but the MCP server still requires authorization. Open the authorization link again from /account/mcp-servers.'
+	}
+	if (connection.state === 'authenticating') {
+		return 'Authorization completed at the identity provider, but Kody could not finish connecting and has no authorization link to offer. Reconnect the server from /account/mcp-servers, or remove and add it again.'
+	}
+	return `Authorization completed at the identity provider, but the MCP server is still "${connection.state}". Reconnect it from /account/mcp-servers.`
+}
+
+/**
+ * Agents SDK quirk: `connectToServer` can leave `connectionState` as
+ * `authenticating` while returning FAILED when `authUrl` is missing, and
+ * `oauthCallbackSuccess` clears the stored auth URL. Detect that stuck state
+ * so callers can recover before surfacing success/error to the user.
+ */
+export function isStuckMcpAuthenticatingWithoutAuthUrl(connection: {
+	state: McpServerConnectionState
+	authUrl: string | null
+}): boolean {
+	return connection.state === 'authenticating' && !connection.authUrl
+}