Skip to content

Commit f0bcb9a

Browse files
committed
Merge PR #1818: surface MCP OAuth reauthorization during catalog discovery
2 parents d596181 + 1f566b8 commit f0bcb9a

9 files changed

Lines changed: 210 additions & 8 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@executor-js/sdk": patch
3+
"@executor-js/plugin-mcp": patch
4+
---
5+
6+
**Rejected MCP OAuth grants now request reconnect without registering a disposable client**
7+
8+
Remote MCP catalog discovery used the MCP SDK's interactive OAuth fallback when an upstream rejected Executor's stored bearer with `401`. A background refresh cannot finish that browser authorization, but the SDK first fetched OAuth metadata and dynamically registered another client. Executor then preserved the old catalog under a generic degraded health verdict, so clients saw zero or stale tools without a reliable reconnect signal.
9+
10+
Executor now stops at the authenticated HTTP boundary for OAuth-backed MCP transports. A rejected stored bearer becomes a structured reauthorization result before OAuth discovery or Dynamic Client Registration runs. Catalog refresh still preserves the last authoritative tools, but records the connection as expired with a reconnect-required detail so the UI and API can direct the user through authorization again.
11+
12+
API-key and unauthenticated MCP transports keep their existing `401` behavior, and ordinary incomplete discovery results remain degraded.

packages/core/sdk/src/executor.test.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,20 @@ const demoPlugin = definePlugin(() => ({
125125
const diagnosticsPlugin = definePlugin(() => ({
126126
id: "diagnostics" as const,
127127
storage: () => ({}),
128-
resolveTools: () =>
128+
resolveTools: ({ connection }) =>
129129
Effect.succeed({
130130
tools: [],
131131
incomplete: true,
132132
incompleteReason: "Schema introspection was rejected",
133+
...(String(connection.integration) === "diagnostics_expired"
134+
? {
135+
health: {
136+
status: "expired" as const,
137+
checkedAt: Date.now(),
138+
detail: "Reconnect the upstream OAuth grant",
139+
},
140+
}
141+
: {}),
133142
}),
134143
extension: (ctx) => ({
135144
seed: () =>
@@ -138,6 +147,12 @@ const diagnosticsPlugin = definePlugin(() => ({
138147
description: "Diagnostics",
139148
config: {},
140149
}),
150+
seedExpired: () =>
151+
ctx.core.integrations.register({
152+
slug: IntegrationSlug.make("diagnostics_expired"),
153+
description: "Expired diagnostics",
154+
config: {},
155+
}),
141156
}),
142157
}))();
143158

@@ -470,6 +485,44 @@ describe("createExecutor", () => {
470485
}),
471486
);
472487

488+
it.effect("preserves actionable health from an incomplete tool catalog", () =>
489+
Effect.gen(function* () {
490+
const executor = yield* makeTestExecutor({
491+
plugins: [memoryCredentialsPlugin(), diagnosticsPlugin] as const,
492+
coreTools: {},
493+
});
494+
yield* executor.diagnostics.seedExpired();
495+
496+
yield* executor.execute(
497+
ToolAddress.make("executor.coreTools.connections.create"),
498+
{
499+
owner: "org",
500+
name: "main",
501+
integration: "diagnostics_expired",
502+
template: "none",
503+
},
504+
{ onElicitation: "accept-all" },
505+
);
506+
507+
const refreshed = yield* executor.execute(
508+
ToolAddress.make("executor.coreTools.connections.refresh"),
509+
{
510+
owner: "org",
511+
name: "main",
512+
integration: "diagnostics_expired",
513+
},
514+
{ onElicitation: "accept-all" },
515+
);
516+
expect(refreshed).toMatchObject({
517+
tools: [],
518+
lastHealth: {
519+
status: "expired",
520+
detail: "Reconnect the upstream OAuth grant",
521+
},
522+
});
523+
}),
524+
);
525+
473526
it.effect("hands pasted credential entry to the web UI", () =>
474527
Effect.gen(function* () {
475528
const executor = yield* makeTestExecutor({

packages/core/sdk/src/executor.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3229,8 +3229,10 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
32293229
// clears it, and reconnect re-syncs tools anyway). Read fresh, since
32303230
// the recorder wrote AFTER this sync's row was loaded. Keep stamping
32313231
// the sync time so the stale-catalog check does not re-attempt this
3232-
// connection on every read.
3233-
const stampSyncedWithHealth = (reason: string) =>
3232+
// connection on every read. A plugin-supplied actionable `health`
3233+
// (e.g. the MCP plugin's reauthorization-required verdict) replaces
3234+
// the generic tool-sync verdict, never a recorded dead grant's.
3235+
const stampSyncedWithHealth = (reason: string, health?: HealthCheckResult) =>
32343236
findConnectionRow(ref).pipe(
32353237
Effect.flatMap((fresh) =>
32363238
core.updateMany("connection", {
@@ -3241,7 +3243,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
32413243
? { tools_synced_at: Date.now() }
32423244
: {
32433245
tools_synced_at: Date.now(),
3244-
last_health: toolSyncHealth(reason),
3246+
last_health: health ?? toolSyncHealth(reason),
32453247
updated_at: new Date(),
32463248
},
32473249
}),
@@ -3310,7 +3312,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
33103312
// server isn't re-dialed on every read; the freshness TTL re-attempts
33113313
// later.
33123314
const reason = syncHealthReason(result);
3313-
yield* stampSyncedWithHealth(reason);
3315+
yield* stampSyncedWithHealth(reason, result.health);
33143316
yield* Effect.logWarning("executor tool sync preserved catalog", {
33153317
reason,
33163318
integration: String(ref.integration),

packages/core/sdk/src/plugin.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,10 @@ export interface ResolveToolsResult {
367367
/** Human-readable reason for an incomplete listing. Persisted by core when it
368368
* preserves the prior catalog so operators can see why data is stale. */
369369
readonly incompleteReason?: string;
370+
/** An actionable connection-health outcome discovered while enumerating the
371+
* catalog. Core persists it while preserving the prior non-authoritative
372+
* catalog. Omit for ordinary transient discovery failures. */
373+
readonly health?: HealthCheckResult;
370374
}
371375

372376
export interface ProjectToolSchemaInput<TStore = unknown> {

packages/plugins/mcp/src/sdk/connection.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ export type RemoteConnectorInput = Omit<
4949
readonly headers?: Record<string, string>;
5050
readonly queryParams?: Record<string, string>;
5151
readonly authProvider?: OAuthClientProvider;
52+
/** This provider only replays a resolved bearer. A 401 cannot be recovered
53+
* inside the MCP SDK and must return to core as reconnect-required before
54+
* the SDK attempts discovery or Dynamic Client Registration. */
55+
readonly staticOAuthBearer?: boolean;
5256
readonly httpClientLayer?: Layer.Layer<HttpClient.HttpClient>;
5357
};
5458

@@ -153,6 +157,18 @@ const nestedMcpHttpTransportError = (cause: unknown): Option.Option<McpHttpTrans
153157
return Option.none();
154158
};
155159

160+
const hasNestedOAuthReauthorization = (cause: unknown): boolean => {
161+
let current: unknown = cause;
162+
for (let depth = 0; depth < 8; depth += 1) {
163+
if (Predicate.isTagged(current, "McpOAuthReauthorizationRequired")) return true;
164+
const decodedCause = decodeExternalTransportCause(current);
165+
if (Option.isNone(decodedCause)) return false;
166+
current = decodedCause.value.cause ?? decodedCause.value.data?.cause;
167+
if (current === undefined) return false;
168+
}
169+
return false;
170+
};
171+
156172
const externalTransportCodes = (cause: unknown): ReadonlySet<string> => {
157173
const codes = new Set<string>();
158174
let current: unknown = cause;
@@ -230,6 +246,7 @@ const awaitAbort = (signal: AbortSignal): Effect.Effect<void> =>
230246

231247
const fetchFromHttpClientLayer = (
232248
httpClientLayer: Layer.Layer<HttpClient.HttpClient>,
249+
staticOAuthBearer: boolean,
233250
): FetchLike => {
234251
const execute: FetchLike = async (url, init) => {
235252
const headers = headersFrom(init?.headers);
@@ -262,6 +279,10 @@ const fetchFromHttpClientLayer = (
262279
headers: responseHeaders,
263280
});
264281
}).pipe(Effect.mapError(normalizeHttpClientFailure), Effect.provide(httpClientLayer));
282+
// Executor resolves and refreshes OAuth credentials before constructing
283+
// this transport. If that stored bearer is rejected, the MCP SDK cannot
284+
// complete its interactive fallback in a catalog refresh and would perform
285+
// avoidable DCR first. Stop at the authenticated HTTP boundary instead.
265286
// A 403 carrying an RFC 6750 insufficient_scope challenge is intercepted
266287
// HERE, below the SDK: with an authProvider the SDK would consume the
267288
// challenge and re-run auth ("upscoping"), which our static-token
@@ -271,6 +292,12 @@ const fetchFromHttpClientLayer = (
271292
// consumes promise rejections) so it reaches the invoke/connect catch
272293
// sites verbatim.
273294
const promise = Effect.runPromise(effect).then((response) => {
295+
if (staticOAuthBearer && response.status === 401) {
296+
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: Fetch-compatible adapter can only signal through a rejected promise
297+
throw new McpOAuthReauthorizationRequired({
298+
message: "MCP OAuth re-authorization required",
299+
});
300+
}
274301
if (response.status === 403) {
275302
const challenge = response.headers.get("www-authenticate");
276303
if (
@@ -335,7 +362,7 @@ const connectionFailure = (
335362
message: string,
336363
cause: unknown,
337364
): McpConnectionError | McpOAuthReauthorizationRequired => {
338-
if (Predicate.isTagged(cause, "McpOAuthReauthorizationRequired")) {
365+
if (hasNestedOAuthReauthorization(cause)) {
339366
return new McpOAuthReauthorizationRequired({ message: "MCP OAuth re-authorization required" });
340367
}
341368
if (Predicate.isTagged(cause, "McpInsufficientScopeError")) {
@@ -487,7 +514,9 @@ export const createMcpConnector = (input: ConnectorInput): McpConnector => {
487514
const headers = input.headers ?? {};
488515
const remoteTransport = input.remoteTransport ?? "auto";
489516
const requestInit = Object.keys(headers).length > 0 ? { headers } : undefined;
490-
const fetch = input.httpClientLayer ? fetchFromHttpClientLayer(input.httpClientLayer) : undefined;
517+
const fetch = input.httpClientLayer
518+
? fetchFromHttpClientLayer(input.httpClientLayer, input.staticOAuthBearer === true)
519+
: undefined;
491520

492521
const endpoint = buildEndpointUrl(input.endpoint, input.queryParams ?? {});
493522

packages/plugins/mcp/src/sdk/discover.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,15 @@ export const discoverTools = (
114114
const httpStatus = Predicate.isTagged(failure, "McpConnectionError")
115115
? failure.httpStatus
116116
: undefined;
117+
const reauthorizationRequired = Predicate.isTagged(
118+
failure,
119+
"McpOAuthReauthorizationRequired",
120+
);
117121
return new McpToolDiscoveryError({
118122
stage: "connect",
119123
message: `Failed connecting to MCP server: ${failure.message}`,
120124
...(httpStatus !== undefined ? { httpStatus } : {}),
125+
...(reauthorizationRequired ? { reauthorizationRequired: true } : {}),
121126
});
122127
}),
123128
),

packages/plugins/mcp/src/sdk/errors.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ export class McpToolDiscoveryError extends Schema.TaggedErrorClass<McpToolDiscov
4242
message: Schema.String,
4343
/** HTTP status from the underlying connect failure, when known. */
4444
httpStatus: Schema.optional(Schema.Number),
45+
/** The MCP OAuth provider reached the interactive authorization boundary.
46+
* Catalog callers use this structural signal to request reconnect without
47+
* parsing or exposing an upstream error message. */
48+
reauthorizationRequired: Schema.optional(Schema.Boolean),
4549
},
4650
{ httpApiStatus: 400 },
4751
) {}

packages/plugins/mcp/src/sdk/plugin.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,45 @@ const serveCallToolServer = (callTool: CallToolResponder) =>
103103
}),
104104
);
105105

106+
const rejectedOAuthDiscoveryLayer = (endpoint: string) => {
107+
const issuer = new URL(endpoint).origin;
108+
const requests: string[] = [];
109+
const layer = Layer.succeed(HttpClient.HttpClient)(
110+
HttpClient.make((request: HttpClientRequest.HttpClientRequest) => {
111+
requests.push(request.url);
112+
const url = new URL(request.url);
113+
const response =
114+
request.url === endpoint
115+
? new Response("", { status: 401 })
116+
: url.pathname === "/.well-known/oauth-protected-resource/mcp"
117+
? Response.json({ resource: endpoint, authorization_servers: [issuer] })
118+
: url.pathname === "/.well-known/oauth-authorization-server"
119+
? Response.json({
120+
issuer,
121+
authorization_endpoint: `${issuer}/authorize`,
122+
token_endpoint: `${issuer}/token`,
123+
registration_endpoint: `${issuer}/register`,
124+
response_types_supported: ["code"],
125+
code_challenge_methods_supported: ["S256"],
126+
})
127+
: url.pathname === "/register"
128+
? Response.json(
129+
{
130+
client_id: "replacement-client",
131+
redirect_uris: ["http://localhost/oauth/callback"],
132+
grant_types: ["authorization_code", "refresh_token"],
133+
response_types: ["code"],
134+
token_endpoint_auth_method: "none",
135+
},
136+
{ status: 201 },
137+
)
138+
: new Response("unexpected request", { status: 500 });
139+
return Effect.succeed(HttpClientResponse.fromWeb(request, response));
140+
}),
141+
);
142+
return { layer, requests };
143+
};
144+
106145
// `tools/call` responders. Both embed a "do-not-leak" sentinel the assertions
107146
// confirm never reaches the caller-facing failure.
108147
const httpStatusCallTool =
@@ -335,6 +374,44 @@ describe("joinToolPath", () => {
335374
// ---------------------------------------------------------------------------
336375

337376
describe("mcpPlugin", () => {
377+
it.effect("surfaces OAuth reauthorization from resolveTools as expired health", () =>
378+
Effect.gen(function* () {
379+
const endpoint = "https://mcp.example.test/mcp";
380+
const plugin = mcpPlugin();
381+
const ledger = rejectedOAuthDiscoveryLayer(endpoint);
382+
const result = yield* plugin.resolveTools!({
383+
config: {
384+
transport: "remote",
385+
endpoint,
386+
remoteTransport: "streamable-http",
387+
authenticationTemplate: [{ slug: "oauth2", kind: "oauth2" }],
388+
},
389+
connection: {
390+
owner: "org",
391+
integration: IntegrationSlug.make("oauth_mcp"),
392+
name: ConnectionName.make("main"),
393+
},
394+
template: AuthTemplateSlug.make("oauth2"),
395+
getValues: () => Effect.succeed({ token: "rejected-token" }),
396+
getValue: () => Effect.succeed("rejected-token"),
397+
httpClientLayer: ledger.layer,
398+
ctx: null as never,
399+
integration: null as never,
400+
storage: {},
401+
});
402+
403+
expect(result).toMatchObject({
404+
tools: [],
405+
incomplete: true,
406+
health: {
407+
status: "expired",
408+
detail: expect.stringContaining("reauthorization"),
409+
},
410+
});
411+
expect(ledger.requests.filter((url) => new URL(url).pathname === "/register")).toEqual([]);
412+
}),
413+
);
414+
338415
it.effect("creates executor with mcp plugin", () =>
339416
Effect.gen(function* () {
340417
const executor = yield* createExecutor(

packages/plugins/mcp/src/sdk/plugin.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,7 @@ const buildConnectorInput = (
643643
queryParams: Object.keys(queryParams).length > 0 ? queryParams : undefined,
644644
headers: Object.keys(headers).length > 0 ? headers : undefined,
645645
authProvider,
646+
...(authProvider === undefined ? {} : { staticOAuthBearer: true }),
646647
httpClientLayer,
647648
});
648649
};
@@ -1327,10 +1328,20 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => {
13271328
}),
13281329
);
13291330
if (Result.isFailure(discovered)) {
1331+
const reauthorizationRequired = discovered.failure.reauthorizationRequired === true;
13301332
return {
13311333
tools: [] as readonly ToolDef[],
13321334
incomplete: true,
13331335
incompleteReason: discovered.failure.message,
1336+
...(reauthorizationRequired
1337+
? {
1338+
health: {
1339+
status: "expired" as const,
1340+
checkedAt: Date.now(),
1341+
detail: "MCP OAuth reauthorization required",
1342+
},
1343+
}
1344+
: {}),
13341345
};
13351346
}
13361347
return { tools: discovered.success.tools.map(toToolDef) };
@@ -1339,7 +1350,12 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => {
13391350
attributes: { "mcp.connection.name": String(connection.name) },
13401351
}),
13411352
) as Effect.Effect<
1342-
{ readonly tools: readonly ToolDef[]; readonly incomplete?: boolean },
1353+
{
1354+
readonly tools: readonly ToolDef[];
1355+
readonly incomplete?: boolean;
1356+
readonly incompleteReason?: string;
1357+
readonly health?: HealthCheckResult;
1358+
},
13431359
StorageFailure
13441360
>,
13451361

0 commit comments

Comments
 (0)