From c62c39edd644444d0169fe2047bba6f83f7adc85 Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Sun, 30 Aug 2026 16:22:09 +0530 Subject: [PATCH 1/4] fix(oauth): complete browser callbacks before tool sync Persist the refreshed OAuth grant and connection before returning the popup callback, then keep remote catalog synchronization alive through the host lifecycle. Preserve synchronous completion for programmatic callers and cover slow MCP discovery with unit and browser E2E tests. --- ...mcp-oauth-callback-background-sync.test.ts | 112 ++++++++++++++++++ packages/core/api/src/handlers/oauth.ts | 17 +-- packages/core/sdk/src/executor.ts | 43 ++++++- packages/core/sdk/src/oauth-client.ts | 8 ++ packages/core/sdk/src/oauth-flow.test.ts | 89 +++++++++++++- packages/core/sdk/src/oauth-service.ts | 14 +++ packages/core/sdk/src/test-config.ts | 2 + packages/plugins/mcp/src/testing/server.ts | 12 ++ 8 files changed, 286 insertions(+), 11 deletions(-) create mode 100644 e2e/selfhost/mcp-oauth-callback-background-sync.test.ts diff --git a/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts b/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts new file mode 100644 index 0000000000..eef038e3c9 --- /dev/null +++ b/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts @@ -0,0 +1,112 @@ +// An OAuth callback commits the fresh grant before it synchronizes a remote +// MCP catalog. A slow tools/list response must not keep the popup request open; +// the host keeps catalog work alive and the tools converge afterward. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect, Schedule } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { deriveMcpNamespace } from "@executor-js/plugin-mcp"; +import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; +import { makeGreetingMcpServer, serveMcpServerWithOAuth } from "@executor-js/plugin-mcp/testing"; +import { IntegrationSlug } from "@executor-js/sdk/shared"; +import { OAuthTestServer } from "@executor-js/sdk/testing"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; +import { visit } from "../src/surfaces/browser"; + +const api = composePluginApi([mcpHttpPlugin()] as const); +const CATALOG_REQUEST_DELAY_MS = 2_000; + +const submitProviderLogin = async (loginUrl: string): Promise => { + const response = await fetch(loginUrl, { + method: "POST", + redirect: "manual", + headers: { authorization: `Basic ${Buffer.from("alice:password").toString("base64")}` }, + }); + const location = response.headers.get("location"); + if (response.status !== 302 || !location) { + throw new Error(`provider login did not redirect (${response.status})`); + } + return new URL(location, loginUrl).toString(); +}; + +scenario( + "MCP OAuth · callback closes before a slow remote catalog finishes syncing", + { timeout: 240_000 }, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: makeApiClient } = yield* Api; + const server = yield* serveMcpServerWithOAuth( + () => makeGreetingMcpServer({ name: "slow-callback-mcp" }), + { path: "/mcp", authenticatedRequestDelayMs: CATALOG_REQUEST_DELAY_MS }, + ); + const identity = yield* target.newIdentity(); + const client = yield* makeApiClient(api, identity); + const displayName = `Slow callback MCP ${randomBytes(3).toString("hex")}`; + const slug = IntegrationSlug.make(deriveMcpNamespace({ name: displayName })); + const clientsBefore = new Set((yield* client.oauth.listClients()).map((item) => item.slug)); + + yield* Effect.gen(function* () { + yield* browser.session(identity, async ({ page, step }) => { + await step("Add an OAuth-protected MCP integration", async () => { + const addUrl = new URL("/integrations/add/mcp", target.baseUrl); + addUrl.searchParams.set("url", server.endpoint); + await visit(page, addUrl.toString()); + await page.getByText("How does this server authenticate?").waitFor({ timeout: 30_000 }); + await page.getByPlaceholder("e.g. Linear").fill(displayName); + await page.getByRole("button", { name: "Add integration" }).click(); + await page.waitForURL(/\/integrations\/(?!add\b)[^/?]+$/, { timeout: 30_000 }); + }); + + await step("Authorize while the MCP catalog is deliberately slow", async () => { + await page.getByRole("button", { name: "Add connection" }).first().click(); + await page.getByRole("heading", { name: /Add connection/ }).waitFor(); + + const popupPromise = page.waitForEvent("popup", { timeout: 30_000 }); + await page.getByRole("button", { name: "Connect", exact: true }).click(); + const popup = await popupPromise; + await popup.waitForURL(/\/login\?/, { timeout: 30_000 }); + const callbackUrl = await submitProviderLogin(popup.url()); + + // Each authenticated MCP transport request is held for two seconds. + // The callback has 1.5 seconds to render, so this can pass only if + // catalog discovery is no longer part of the callback response. + await popup.goto(callbackUrl, { waitUntil: "domcontentloaded", timeout: 1_500 }); + await page.getByText("Connection added", { exact: true }).waitFor({ timeout: 30_000 }); + }); + }); + + const tools = yield* client.tools.list({ query: { integration: slug } }).pipe( + Effect.filterOrFail( + (items) => items.some((tool) => String(tool.name) === "simple_echo"), + () => "slow_mcp_catalog_pending" as const, + ), + Effect.retry(Schedule.both(Schedule.spaced("1 second"), Schedule.recurs(20))), + ); + expect( + tools.map((tool) => String(tool.name)), + "the host-kept background sync eventually publishes the remote tool", + ).toContain("simple_echo"); + }).pipe( + Effect.ensuring( + Effect.gen(function* () { + const clientsAfter = yield* client.oauth.listClients(); + for (const oauthClient of clientsAfter) { + if (!clientsBefore.has(oauthClient.slug)) { + yield* client.oauth.removeClient({ + params: { slug: oauthClient.slug }, + payload: { owner: oauthClient.owner }, + }); + } + } + yield* client.mcp.removeServer({ params: { slug } }); + }).pipe(Effect.ignore), + ), + ); + }), + ).pipe(Effect.provide(OAuthTestServer.layer())), +); diff --git a/packages/core/api/src/handlers/oauth.ts b/packages/core/api/src/handlers/oauth.ts index 058ff4e8ab..eb4b1f939b 100644 --- a/packages/core/api/src/handlers/oauth.ts +++ b/packages/core/api/src/handlers/oauth.ts @@ -212,13 +212,16 @@ export const OAuthHandlers = HttpApiBuilder.group(ExecutorApi, "oauth", (handler const html = yield* runOAuthCallback({ complete: ({ state, code, callbackDomain }) => executor.oauth - .complete({ - // `runOAuthCallback`'s `state` is a raw string from the URL; - // the SDK speaks the branded `OAuthState` (nominal brand). - state: OAuthState.make(state), - code: code ?? "", - callbackDomain, - }) + .complete( + { + // `runOAuthCallback`'s `state` is a raw string from the URL; + // the SDK speaks the branded `OAuthState` (nominal brand). + state: OAuthState.make(state), + code: code ?? "", + callbackDomain, + }, + { toolSync: "background" }, + ) .pipe( Effect.tapError((cause: unknown) => Effect.logError("OAuth callback completion failed", cause), diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 6916233cb5..d70ae691cb 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -4453,6 +4453,12 @@ export const createExecutor = Effect.succeed([] as readonly Tool[]), ), ); + if (input.toolSync === "background") { + const fiber = yield* Effect.forkDetach( + syncTools.pipe( + Effect.catch((error) => + Effect.logWarning("executor OAuth tool sync failed", { + integration: String(ref.integration), + connection: String(ref.name), + error: describeSyncFailure(error), + }), + ), + Effect.withSpan("executor.oauth.tools.sync", { + attributes: { + "executor.integration": String(ref.integration), + "executor.connection": String(ref.name), + }, + }), + ), + ); + config.waitUntil?.( + new Promise((resolve) => fiber.addObserver(() => resolve(undefined))), + ); + } else { + yield* syncTools; + } }), ); diff --git a/packages/core/sdk/src/oauth-client.ts b/packages/core/sdk/src/oauth-client.ts index 7330ad753b..8778205014 100644 --- a/packages/core/sdk/src/oauth-client.ts +++ b/packages/core/sdk/src/oauth-client.ts @@ -376,6 +376,13 @@ export interface OAuthCompleteInput { readonly callbackDomain?: string | null; } +/** Host-lifecycle behavior for OAuth completion. The HTTP popup uses + * background tool synchronization so it can close after the durable grant; + * programmatic callers keep the default explicit catalog guarantee. */ +export interface OAuthCompleteOptions { + readonly toolSync?: "explicit" | "background"; +} + /** Probe a base/issuer URL for OAuth 2.1 authorization-server metadata so the * onboarding UI can pre-fill a client's endpoints. */ export interface OAuthProbeInput { @@ -535,6 +542,7 @@ export interface OAuthService { ) => Effect.Effect; readonly complete: ( input: OAuthCompleteInput, + options?: OAuthCompleteOptions, ) => Effect.Effect< Connection, OAuthCompleteError | OAuthSessionNotFoundError | OrgWriteDeniedError | StorageFailure diff --git a/packages/core/sdk/src/oauth-flow.test.ts b/packages/core/sdk/src/oauth-flow.test.ts index b9860574bb..d818d24644 100644 --- a/packages/core/sdk/src/oauth-flow.test.ts +++ b/packages/core/sdk/src/oauth-flow.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "@effect/vitest"; -import { Deferred, Effect, Fiber, Predicate } from "effect"; +import { Deferred, Effect, Fiber, Option, Predicate } from "effect"; import { withQueryContext } from "@executor-js/fumadb/query"; import { @@ -264,6 +264,93 @@ describe("oauth.start / oauth.complete", () => { ), ); + it.effect("complete returns after the durable grant while remote tool discovery continues", () => + Effect.scoped( + Effect.gen(function* () { + const discoveryStarted = yield* Deferred.make(); + const releaseDiscovery = yield* Deferred.make(); + const keptAlive: Promise[] = []; + const slowOAuthPlugin = definePlugin(() => ({ + id: "acme" as const, + storage: () => ({}), + resolveTools: () => + Effect.gen(function* () { + yield* Deferred.succeed(discoveryStarted, undefined); + yield* Deferred.await(releaseDiscovery); + return { + tools: [{ name: ToolName.make("whoami"), description: "whoami" }], + }; + }), + describeAuthMethods: () => [ + { + id: "oauth", + label: "OAuth2", + kind: "oauth" as const, + template: String(TEMPLATE), + oauth: { scopes: ["read"] }, + }, + ], + invokeTool: ({ credential }) => Effect.succeed({ token: credential.value }), + extension: (ctx) => ({ + seed: () => + ctx.core.integrations.register({ + slug: INTEG, + description: "Slow Acme", + config: {}, + }), + }), + }))(); + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ + plugins: [memoryCredentialsPlugin(), slowOAuthPlugin] as const, + waitUntil: (promise) => keptAlive.push(promise), + }); + yield* executor.acme.seed(); + + yield* executor.oauth.createClient({ + owner: "org", + slug: CLIENT, + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "authorization_code", + clientId: "test-client", + clientSecret: "test-secret", + }); + const started = yield* executor.oauth.start({ + owner: "org", + client: CLIENT, + clientOwner: "org", + name: ConnectionName.make("main-account"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + const callback = yield* server.completeAuthorizationCodeFlow({ + authorizationUrl: started.authorizationUrl, + }); + + const completed = yield* executor.oauth + .complete({ state: started.state, code: callback.code }, { toolSync: "background" }) + .pipe(Effect.timeoutOption("1 second")); + expect( + Option.isSome(completed), + "the callback returns while listTools remains deliberately blocked", + ).toBe(true); + expect(keptAlive).toHaveLength(1); + yield* Deferred.await(discoveryStarted); + + const connections = yield* executor.connections.list({ integration: INTEG }); + expect(connections.map((connection) => String(connection.name))).toEqual(["mainAccount"]); + + yield* Deferred.succeed(releaseDiscovery, undefined); + yield* Effect.promise(() => Promise.all(keptAlive)); + const tools = yield* executor.tools.list({ integration: INTEG }); + expect(tools.map((tool) => String(tool.name))).toEqual(["whoami"]); + }), + ), + ); + it.effect("persists HTTP Basic client auth for code exchange and refresh", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index dd443144bd..99ab41c26c 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -60,6 +60,7 @@ import { type OAuthClientOrigin, type OAuthClientSummary, type OAuthCompleteInput, + type OAuthCompleteOptions, type OAuthGrant, type OAuthProbeInput, type OAuthProbeResult, @@ -149,6 +150,12 @@ export interface MintOAuthConnectionInput { * code was redeemed at a region other than the client's configured token * host (Datadog multi-site). Null means refresh uses the client's token URL. */ readonly oauthTokenUrl?: string | null; + /** Whether connection tool discovery must finish before the mint returns. + * Interactive authorization-code callbacks persist the fresh grant first, + * then synchronize the remote catalog in host-kept background work so a + * slow MCP server cannot strand the browser popup. Non-interactive grants + * keep the explicit behavior because their caller has no callback window. */ + readonly toolSync?: "explicit" | "background"; } /** Project an enterprise-managed mint failure onto the connect boundary, @@ -2078,6 +2085,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { // ----------------------------------------------------------------------- const complete = ( input: OAuthCompleteInput, + options?: OAuthCompleteOptions, ): Effect.Effect< Connection, OAuthCompleteError | OAuthSessionNotFoundError | OrgWriteDeniedError | StorageFailure @@ -2215,6 +2223,10 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { // Persist the regional token endpoint ONLY when it differs from the // client's configured one, so refresh redeems against the same region. tokenUrl === client.tokenUrl ? null : tokenUrl, + // The grant and connection row are the callback's durable contract. + // Remote catalog discovery can be arbitrarily slow and must not keep + // the popup waiting after that contract has committed. + options?.toolSync ?? "explicit", ).pipe( Effect.mapError((cause) => Predicate.isTagged(cause, "OrgWriteDeniedError") @@ -2290,6 +2302,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { /** Regional token endpoint override to persist when the code was redeemed * off the client's configured host; null to use the client's token URL. */ oauthTokenUrl: string | null, + toolSync: "explicit" | "background" = "explicit", ): Effect.Effect => Effect.gen(function* () { // The token exchange may outlive the role that admitted `start`. Re-read @@ -2357,6 +2370,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { oauthScope, missingOAuthScopes: missingScopes, oauthTokenUrl, + toolSync, }); }); diff --git a/packages/core/sdk/src/test-config.ts b/packages/core/sdk/src/test-config.ts index 17b8fbc321..df32bcda0f 100644 --- a/packages/core/sdk/src/test-config.ts +++ b/packages/core/sdk/src/test-config.ts @@ -138,6 +138,7 @@ export type TestConfigOptions["orgWrites"]; + readonly waitUntil?: ExecutorConfig["waitUntil"]; }; export const makeTestConfig = ( @@ -181,6 +182,7 @@ export const makeTestConfig = Effect.Effect; readonly authorizationServerUrls?: readonly string[]; @@ -173,6 +176,14 @@ export const serveMcpServer = (factory: () => McpServer, options: McpTestServerO writeUnauthorized(response, origin); return; } + if (options.authenticatedRequestDelayMs !== undefined) { + yield* Effect.promise( + () => + new Promise((resolve) => + setTimeout(resolve, options.authenticatedRequestDelayMs), + ), + ); + } } if (sessionId && request.method === "POST" && nextSessionRequestStatus !== undefined) { @@ -344,6 +355,7 @@ export const serveMcpServerWithOAuth = ( const oauth = yield* OAuthTestServer; return yield* serveMcpServer(factory, { path: options.path, + authenticatedRequestDelayMs: options.authenticatedRequestDelayMs, auth: { validateAuthorization: oauth.acceptsAuthorizationHeader, authorizationServerUrls: [oauth.issuerUrl], From 42b6b11ec954d3c3e020f32596a860f94f6a18ae Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:46:41 -0700 Subject: [PATCH 2/4] Verify OAuth callbacks with blocked and failing catalogs --- ...mcp-oauth-callback-background-sync.test.ts | 180 +++++++++++------- packages/core/sdk/src/oauth-flow.test.ts | 5 + packages/plugins/mcp/src/testing/server.ts | 17 +- 3 files changed, 119 insertions(+), 83 deletions(-) diff --git a/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts b/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts index eef038e3c9..ce0a1e5383 100644 --- a/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts +++ b/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts @@ -3,12 +3,15 @@ // the host keeps catalog work alive and the tools converge afterward. import { randomBytes } from "node:crypto"; +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { ListToolsRequestSchema } from "@modelcontextprotocol/sdk/types.js"; + import { expect } from "@effect/vitest"; import { Effect, Schedule } from "effect"; import { composePluginApi } from "@executor-js/api/server"; import { deriveMcpNamespace } from "@executor-js/plugin-mcp"; import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; -import { makeGreetingMcpServer, serveMcpServerWithOAuth } from "@executor-js/plugin-mcp/testing"; +import { serveMcpServerWithOAuth } from "@executor-js/plugin-mcp/testing"; import { IntegrationSlug } from "@executor-js/sdk/shared"; import { OAuthTestServer } from "@executor-js/sdk/testing"; @@ -17,7 +20,6 @@ import { Api, Browser, Target } from "../src/services"; import { visit } from "../src/surfaces/browser"; const api = composePluginApi([mcpHttpPlugin()] as const); -const CATALOG_REQUEST_DELAY_MS = 2_000; const submitProviderLogin = async (loginUrl: string): Promise => { const response = await fetch(loginUrl, { @@ -32,81 +34,115 @@ const submitProviderLogin = async (loginUrl: string): Promise => { return new URL(location, loginUrl).toString(); }; -scenario( - "MCP OAuth · callback closes before a slow remote catalog finishes syncing", - { timeout: 240_000 }, - Effect.scoped( - Effect.gen(function* () { - const target = yield* Target; - const browser = yield* Browser; - const { client: makeApiClient } = yield* Api; - const server = yield* serveMcpServerWithOAuth( - () => makeGreetingMcpServer({ name: "slow-callback-mcp" }), - { path: "/mcp", authenticatedRequestDelayMs: CATALOG_REQUEST_DELAY_MS }, - ); - const identity = yield* target.newIdentity(); - const client = yield* makeApiClient(api, identity); - const displayName = `Slow callback MCP ${randomBytes(3).toString("hex")}`; - const slug = IntegrationSlug.make(deriveMcpNamespace({ name: displayName })); - const clientsBefore = new Set((yield* client.oauth.listClients()).map((item) => item.slug)); +for (const failsFirst of [false, true]) { + scenario( + `MCP OAuth · callback closes before ${failsFirst ? "failing" : "blocked"} catalog discovery and preserves the grant`, + { timeout: 240_000 }, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: makeApiClient } = yield* Api; + const gate = Promise.withResolvers(); + const listing = Promise.withResolvers(); + let failListing = failsFirst; + const server = yield* serveMcpServerWithOAuth( + () => { + const mcp = new McpServer( + { name: "callback-mcp", version: "1.0.0" }, + { capabilities: { tools: {} } }, + ); + mcp.server.setRequestHandler(ListToolsRequestSchema, async () => { + listing.resolve(); + if (failListing) throw new Error("Temporary catalog outage"); + return { tools: [{ name: "simple_echo", inputSchema: { type: "object" as const } }] }; + }); + return mcp; + }, + { path: "/mcp", beforeAuthenticatedRequest: () => gate.promise }, + ); + const identity = yield* target.newIdentity(); + const client = yield* makeApiClient(api, identity); + const displayName = `Slow callback MCP ${randomBytes(3).toString("hex")}`; + const slug = IntegrationSlug.make(deriveMcpNamespace({ name: displayName })); + const clientsBefore = new Set((yield* client.oauth.listClients()).map((item) => item.slug)); - yield* Effect.gen(function* () { - yield* browser.session(identity, async ({ page, step }) => { - await step("Add an OAuth-protected MCP integration", async () => { - const addUrl = new URL("/integrations/add/mcp", target.baseUrl); - addUrl.searchParams.set("url", server.endpoint); - await visit(page, addUrl.toString()); - await page.getByText("How does this server authenticate?").waitFor({ timeout: 30_000 }); - await page.getByPlaceholder("e.g. Linear").fill(displayName); - await page.getByRole("button", { name: "Add integration" }).click(); - await page.waitForURL(/\/integrations\/(?!add\b)[^/?]+$/, { timeout: 30_000 }); - }); + yield* Effect.gen(function* () { + yield* browser.session(identity, async ({ page, step }) => { + await step("Add an OAuth-protected MCP integration", async () => { + const addUrl = new URL("/integrations/add/mcp", target.baseUrl); + addUrl.searchParams.set("url", server.endpoint); + await visit(page, addUrl.toString()); + await page + .getByText("How does this server authenticate?") + .waitFor({ timeout: 30_000 }); + await page.getByPlaceholder("e.g. Linear").fill(displayName); + await page.getByRole("button", { name: "Add integration" }).click(); + await page.waitForURL(/\/integrations\/(?!add\b)[^/?]+$/, { timeout: 30_000 }); + }); - await step("Authorize while the MCP catalog is deliberately slow", async () => { - await page.getByRole("button", { name: "Add connection" }).first().click(); - await page.getByRole("heading", { name: /Add connection/ }).waitFor(); + await step("Authorize while the MCP catalog is deliberately slow", async () => { + await page.getByRole("button", { name: "Add connection" }).first().click(); + await page.getByRole("heading", { name: /Add connection/ }).waitFor(); - const popupPromise = page.waitForEvent("popup", { timeout: 30_000 }); - await page.getByRole("button", { name: "Connect", exact: true }).click(); - const popup = await popupPromise; - await popup.waitForURL(/\/login\?/, { timeout: 30_000 }); - const callbackUrl = await submitProviderLogin(popup.url()); + const popupPromise = page.waitForEvent("popup", { timeout: 30_000 }); + await page.getByRole("button", { name: "Connect", exact: true }).click(); + const popup = await popupPromise; + await popup.waitForURL(/\/login\?/, { timeout: 30_000 }); + const callbackUrl = await submitProviderLogin(popup.url()); - // Each authenticated MCP transport request is held for two seconds. - // The callback has 1.5 seconds to render, so this can pass only if - // catalog discovery is no longer part of the callback response. - await popup.goto(callbackUrl, { waitUntil: "domcontentloaded", timeout: 1_500 }); - await page.getByText("Connection added", { exact: true }).waitFor({ timeout: 30_000 }); + // No authenticated MCP request can complete before we release + // the gate. Callback success therefore proves ordering without + // racing a timer against a cold browser or a loaded host. + await popup.goto(callbackUrl, { waitUntil: "domcontentloaded", timeout: 30_000 }); + await page + .getByText("Connection added", { exact: true }) + .waitFor({ timeout: 30_000 }); + }); }); - }); - const tools = yield* client.tools.list({ query: { integration: slug } }).pipe( - Effect.filterOrFail( - (items) => items.some((tool) => String(tool.name) === "simple_echo"), - () => "slow_mcp_catalog_pending" as const, + const committed = yield* client.connections.list({ query: { integration: slug } }); + expect(committed.length, "the callback persisted the connection before discovery").toBe( + 1, + ); + gate.resolve(); + yield* Effect.promise(() => listing.promise); + const afterListing = yield* client.connections.list({ query: { integration: slug } }); + expect( + afterListing.length, + "a failed remote listing cannot remove the durable grant", + ).toBe(1); + failListing = false; + + const tools = yield* client.tools.list({ query: { integration: slug } }).pipe( + Effect.filterOrFail( + (items) => items.some((tool) => String(tool.name) === "simple_echo"), + () => "slow_mcp_catalog_pending" as const, + ), + Effect.retry(Schedule.both(Schedule.spaced("1 second"), Schedule.recurs(20))), + ); + expect( + tools.map((tool) => String(tool.name)), + "the host-kept background sync eventually publishes the remote tool", + ).toContain("simple_echo"); + }).pipe( + Effect.ensuring( + Effect.gen(function* () { + gate.resolve(); + const clientsAfter = yield* client.oauth.listClients(); + for (const oauthClient of clientsAfter) { + if (!clientsBefore.has(oauthClient.slug)) { + yield* client.oauth.removeClient({ + params: { slug: oauthClient.slug }, + payload: { owner: oauthClient.owner }, + }); + } + } + yield* client.mcp.removeServer({ params: { slug } }); + }).pipe(Effect.ignore), ), - Effect.retry(Schedule.both(Schedule.spaced("1 second"), Schedule.recurs(20))), ); - expect( - tools.map((tool) => String(tool.name)), - "the host-kept background sync eventually publishes the remote tool", - ).toContain("simple_echo"); - }).pipe( - Effect.ensuring( - Effect.gen(function* () { - const clientsAfter = yield* client.oauth.listClients(); - for (const oauthClient of clientsAfter) { - if (!clientsBefore.has(oauthClient.slug)) { - yield* client.oauth.removeClient({ - params: { slug: oauthClient.slug }, - payload: { owner: oauthClient.owner }, - }); - } - } - yield* client.mcp.removeServer({ params: { slug } }); - }).pipe(Effect.ignore), - ), - ); - }), - ).pipe(Effect.provide(OAuthTestServer.layer())), -); + }), + ).pipe(Effect.provide(OAuthTestServer.layer())), + ); +} diff --git a/packages/core/sdk/src/oauth-flow.test.ts b/packages/core/sdk/src/oauth-flow.test.ts index d818d24644..572b1de305 100644 --- a/packages/core/sdk/src/oauth-flow.test.ts +++ b/packages/core/sdk/src/oauth-flow.test.ts @@ -305,6 +305,11 @@ describe("oauth.start / oauth.complete", () => { plugins: [memoryCredentialsPlugin(), slowOAuthPlugin] as const, waitUntil: (promise) => keptAlive.push(promise), }); + yield* Effect.addFinalizer(() => + Deferred.succeed(releaseDiscovery, undefined).pipe( + Effect.andThen(Effect.promise(() => Promise.all(keptAlive))), + ), + ); yield* executor.acme.seed(); yield* executor.oauth.createClient({ diff --git a/packages/plugins/mcp/src/testing/server.ts b/packages/plugins/mcp/src/testing/server.ts index 0628b9b5ab..b5ccdc1c9f 100644 --- a/packages/plugins/mcp/src/testing/server.ts +++ b/packages/plugins/mcp/src/testing/server.ts @@ -36,9 +36,9 @@ export type McpTestRequest = { export type McpTestServerOptions = { readonly path?: string; - /** Delay each authenticated MCP request after auth succeeds. Exercises host - * request lifecycles against a real transport whose catalog is slow. */ - readonly authenticatedRequestDelayMs?: number; + /** Hold authenticated requests at the transport boundary until the test + * releases them, so callback ordering does not depend on elapsed time. */ + readonly beforeAuthenticatedRequest?: () => Promise; readonly auth?: { readonly validateAuthorization: (authorization: string | undefined) => Effect.Effect; readonly authorizationServerUrls?: readonly string[]; @@ -176,13 +176,8 @@ export const serveMcpServer = (factory: () => McpServer, options: McpTestServerO writeUnauthorized(response, origin); return; } - if (options.authenticatedRequestDelayMs !== undefined) { - yield* Effect.promise( - () => - new Promise((resolve) => - setTimeout(resolve, options.authenticatedRequestDelayMs), - ), - ); + if (options.beforeAuthenticatedRequest !== undefined) { + yield* Effect.promise(options.beforeAuthenticatedRequest); } } @@ -355,7 +350,7 @@ export const serveMcpServerWithOAuth = ( const oauth = yield* OAuthTestServer; return yield* serveMcpServer(factory, { path: options.path, - authenticatedRequestDelayMs: options.authenticatedRequestDelayMs, + beforeAuthenticatedRequest: options.beforeAuthenticatedRequest, auth: { validateAuthorization: oauth.acceptsAuthorizationHeader, authorizationServerUrls: [oauth.issuerUrl], From 5c87502250df9da57559d3f6ca0607e8ac388385 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:49:36 -0700 Subject: [PATCH 3/4] Release catalog gate before the separate health probe --- .../mcp-oauth-callback-background-sync.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts b/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts index ce0a1e5383..15f7cfd655 100644 --- a/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts +++ b/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts @@ -95,17 +95,22 @@ for (const failsFirst of [false, true]) { // the gate. Callback success therefore proves ordering without // racing a timer against a cold browser or a loaded host. await popup.goto(callbackUrl, { waitUntil: "domcontentloaded", timeout: 30_000 }); + const committed = await Effect.runPromise( + client.connections.list({ query: { integration: slug } }), + ); + expect( + committed.length, + "the callback persisted the connection before discovery", + ).toBe(1); + // Release before the opener's separate health probe, which also + // uses this transport, after proving the callback has returned. + gate.resolve(); await page .getByText("Connection added", { exact: true }) .waitFor({ timeout: 30_000 }); }); }); - const committed = yield* client.connections.list({ query: { integration: slug } }); - expect(committed.length, "the callback persisted the connection before discovery").toBe( - 1, - ); - gate.resolve(); yield* Effect.promise(() => listing.promise); const afterListing = yield* client.connections.list({ query: { integration: slug } }); expect( From 99e3b814150002865b0ea4e0001b23837b6ccca7 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:54:16 -0700 Subject: [PATCH 4/4] Verify OAuth discovery recovery through explicit refresh --- .../mcp-oauth-callback-background-sync.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts b/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts index 15f7cfd655..bcc45dacfe 100644 --- a/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts +++ b/e2e/selfhost/mcp-oauth-callback-background-sync.test.ts @@ -117,7 +117,19 @@ for (const failsFirst of [false, true]) { afterListing.length, "a failed remote listing cannot remove the durable grant", ).toBe(1); + const connection = afterListing[0]; + if (connection === undefined) return yield* Effect.die("Missing committed connection"); + expect( + connection.lastHealth?.status, + "discovery reports the actual upstream health", + ).toBe(failsFirst ? "degraded" : "healthy"); failListing = false; + if (failsFirst) { + // Failed discovery intentionally backs off until the catalog TTL. + // An explicit refresh is the supported immediate recovery action. + const params = { owner: connection.owner, integration: slug, name: connection.name }; + yield* client.connections.refresh({ params }); + } const tools = yield* client.tools.list({ query: { integration: slug } }).pipe( Effect.filterOrFail( @@ -130,6 +142,11 @@ for (const failsFirst of [false, true]) { tools.map((tool) => String(tool.name)), "the host-kept background sync eventually publishes the remote tool", ).toContain("simple_echo"); + const healthy = yield* client.connections.checkHealth({ + params: { owner: connection.owner, integration: slug, name: connection.name }, + query: {}, + }); + expect(healthy.status, "the recovered server accepts the existing grant").toBe("healthy"); }).pipe( Effect.ensuring( Effect.gen(function* () {