From e1305a0cc0b0094f68dc0cc214e11f18bb0380c8 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:31:24 -0700 Subject: [PATCH] Surface 4xx JSON refusals from MCP tools/call as typed failures Stripe's OAuth MCP server validates the account context at the HTTP layer: a call without stripe_context gets a 422 whose JSON body names the missing field. That reached the sandbox as an opaque "Internal tool error [id]" because a non-auth HTTP status was treated as a transport defect. Read a string message out of a 4xx JSON body (structurally; never the raw text) and answer with mcp_tool_error so the caller can fix the arguments. 401/403 keep their auth classification; 5xx and bodyless 4xx stay opaque. Co-Authored-By: Claude Fable 5.1 --- .changeset/mcp-http-refusal.md | 5 ++ packages/plugins/mcp/src/sdk/errors.ts | 11 ++++ packages/plugins/mcp/src/sdk/http-status.ts | 38 ++++++++++++++ packages/plugins/mcp/src/sdk/invoke.ts | 22 +++++++- packages/plugins/mcp/src/sdk/plugin.test.ts | 58 +++++++++++++++++++++ packages/plugins/mcp/src/sdk/plugin.ts | 14 +++++ 6 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 .changeset/mcp-http-refusal.md diff --git a/.changeset/mcp-http-refusal.md b/.changeset/mcp-http-refusal.md new file mode 100644 index 0000000000..48a6049f63 --- /dev/null +++ b/.changeset/mcp-http-refusal.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +**Fix: an MCP server refusing a tool call with a 4xx HTTP response (for example Stripe's `422` when `stripe_context` is missing) surfaced as `Internal tool error [id]`.** When the body is a JSON object naming the problem, the call now returns a typed `mcp_tool_error` failure with the server's message and status, so the model can fix the arguments instead of reading an outage. diff --git a/packages/plugins/mcp/src/sdk/errors.ts b/packages/plugins/mcp/src/sdk/errors.ts index 3f4e8af3a6..73e59db64b 100644 --- a/packages/plugins/mcp/src/sdk/errors.ts +++ b/packages/plugins/mcp/src/sdk/errors.ts @@ -97,6 +97,17 @@ export class McpInvocationError extends Data.TaggedError("McpInvocationError")<{ readonly name: string; readonly code?: string | number; }; + /** The server answered `tools/call` with a non-2xx HTTP response whose + * body was a JSON object carrying a message (a validation refusal from a + * server that answers at the HTTP layer instead of with a JSON-RPC error, + * e.g. a 422 naming a missing field). Present only for 4xx statuses other + * than the auth walls (401/403), and only when the body parsed as JSON + * with a string `message`/`error`/`error.message` — a free-text body is + * never copied out of the transport error. */ + readonly httpRefusal?: { + readonly status: number; + readonly message: string; + }; }> {} export class McpOAuthReauthorizationRequired extends Data.TaggedError( diff --git a/packages/plugins/mcp/src/sdk/http-status.ts b/packages/plugins/mcp/src/sdk/http-status.ts index ce56baeff9..b410dbda3d 100644 --- a/packages/plugins/mcp/src/sdk/http-status.ts +++ b/packages/plugins/mcp/src/sdk/http-status.ts @@ -52,6 +52,44 @@ const statusFromNumericHttpCode = (cause: unknown): number | undefined => export const httpStatusFromCause = (cause: unknown): number | undefined => statusFromTypedTransportError(cause) ?? statusFromSsePostError(cause); +// A server that validates a call at the HTTP layer answers with a 4xx whose +// body is a JSON object naming the problem (Stripe's MCP: 422 for a missing +// `stripe_context`). The transport keeps that body on `SdkHttpError.data.text`. +// Read it STRUCTURALLY — parse, then pick a string message field — so a body +// that is not a JSON object (an HTML error page, a proxy banner) contributes +// nothing; only a message the server wrote for the caller comes out. +const JsonErrorBody = Schema.Union([ + Schema.Struct({ message: Schema.String }), + Schema.Struct({ error: Schema.String }), + Schema.Struct({ error: Schema.Struct({ message: Schema.String }) }), +]); +const decodeJsonErrorBody = Schema.decodeUnknownOption(JsonErrorBody); +const SdkHttpErrorText = Schema.Struct({ text: Schema.String }); +const decodeSdkHttpErrorText = Schema.decodeUnknownOption(SdkHttpErrorText); + +const parseJsonSafe = (text: string): unknown => { + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: classifying an untrusted upstream error body; a parse failure just means "not a JSON body" + try { + // oxlint-disable-next-line executor/no-json-parse -- boundary: the parsed value is only structurally decoded for a message field, never used as domain data + return JSON.parse(text) as unknown; + } catch { + return undefined; + } +}; + +/** The caller-facing message from a JSON error body the SDK's HTTP error + * carries, or `undefined` when there is none. */ +export const httpRefusalMessageFromCause = (cause: unknown): string | undefined => { + const sdk = mcpClientSdkIfLoaded(); + if (sdk === undefined || !sdk.client.SdkHttpError.isInstance(cause)) return undefined; + const text = Option.getOrUndefined(decodeSdkHttpErrorText(cause.data))?.text; + if (text === undefined) return undefined; + const body = Option.getOrUndefined(decodeJsonErrorBody(parseJsonSafe(text))); + if (body === undefined) return undefined; + if ("message" in body) return body.message; + return typeof body.error === "string" ? body.error : body.error.message; +}; + /** Connection handshakes may receive the SDK's SSE error, whose numeric code * is an HTTP status. Keep this connection-only: JSON-RPC invocation errors * also have numeric `code` fields which are not HTTP statuses. */ diff --git a/packages/plugins/mcp/src/sdk/invoke.ts b/packages/plugins/mcp/src/sdk/invoke.ts index a74e7a2ebb..ed67dd2312 100644 --- a/packages/plugins/mcp/src/sdk/invoke.ts +++ b/packages/plugins/mcp/src/sdk/invoke.ts @@ -33,7 +33,11 @@ import { import { McpConnectionError, McpInvocationError, McpOAuthReauthorizationRequired } from "./errors"; import type { McpConnection, McpConnector } from "./connection"; import type { McpConnectionPool } from "./connection-pool"; -import { httpStatusFromCause, insufficientScopeFromCause } from "./http-status"; +import { + httpRefusalMessageFromCause, + httpStatusFromCause, + insufficientScopeFromCause, +} from "./http-status"; // --------------------------------------------------------------------------- // Helpers @@ -161,6 +165,21 @@ const summarizeSdkFailure = (cause: unknown): { name: string; code?: string | nu return typeof code === "string" || typeof code === "number" ? { name, code } : { name }; }; +/** A 4xx other than the auth walls, with a JSON body that names the problem, + * is the server refusing THIS call (a validation failure at the HTTP layer) + * — not a dead transport. 401/403 keep their auth classification; 5xx and + * bodyless 4xx stay opaque, since there is nothing the caller can act on. */ +const httpRefusal = ( + status: number | undefined, + cause: unknown, +): { readonly httpRefusal: { readonly status: number; readonly message: string } } | {} => { + if (status === undefined || status < 400 || status >= 500 || status === 401 || status === 403) { + return {}; + } + const message = httpRefusalMessageFromCause(cause); + return message === undefined ? {} : { httpRefusal: { status, message } }; +}; + const asProtocolError = (cause: unknown): ProtocolError | undefined => { const sdk = mcpClientSdkIfLoaded(); if (sdk === undefined) return undefined; @@ -402,6 +421,7 @@ const useConnection = ( ...(status === 403 && insufficientScopeFromCause(cause) ? { insufficientScope: true } : {}), + ...httpRefusal(status, cause), }); }, }).pipe( diff --git a/packages/plugins/mcp/src/sdk/plugin.test.ts b/packages/plugins/mcp/src/sdk/plugin.test.ts index 22f249aae0..487874677b 100644 --- a/packages/plugins/mcp/src/sdk/plugin.test.ts +++ b/packages/plugins/mcp/src/sdk/plugin.test.ts @@ -1156,6 +1156,64 @@ describe("mcpPlugin", () => { ), ); + // Stripe's MCP validates the OAuth account context at the HTTP layer: a + // call without `stripe_context` gets a 422 whose JSON body names the missing + // field. That is the server refusing THIS call, so it must reach the caller + // as a typed failure carrying the server's message — the same treatment as + // a JSON-RPC invalid-params refusal — not scrub into an opaque defect. + it.effect("surfaces a 4xx JSON refusal from tools/call as a typed tool failure", () => + Effect.scoped( + Effect.gen(function* () { + const { executor, toolAddress } = yield* seedCallToolExecutor({ + slug: "call_http_422", + callTool: () => + HttpServerResponse.jsonUnsafe( + { message: "stripe_context is required for this tool" }, + { status: 422 }, + ), + }); + + const result = yield* executor.execute(toolAddress, {}, { onElicitation: "accept-all" }); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "mcp_tool_error", + message: "stripe_context is required for this tool", + status: 422, + retryable: false, + details: { upstream: { status: 422 } }, + }, + }); + expect(result).not.toMatchObject({ error: { details: { category: "authentication" } } }); + }), + ), + ); + + // A bodyless 4xx (or a body that is not a JSON object) has no message the + // caller can act on, so it keeps the opaque-defect path: nothing from the + // transport error text is copied out. + it.effect("keeps a 4xx without a JSON message opaque", () => + Effect.scoped( + Effect.gen(function* () { + const { executor, toolAddress } = yield* seedCallToolExecutor({ + slug: "call_http_422_text", + callTool: httpStatusCallTool(422), + }); + + const failure = yield* executor + .execute(toolAddress, {}, { onElicitation: "accept-all" }) + .pipe(Effect.flip); + expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true); + const error = failure as { readonly message: string; readonly cause?: unknown }; + expect(error).toMatchObject({ message: expect.not.stringContaining("do-not-leak") }); + const cause = error.cause as McpInvocationError; + expect(cause.status).toBe(422); + expect(cause.httpRefusal).toBeUndefined(); + }), + ), + ); + it.effect("does not classify JSON-RPC error codes as auth failures", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index e4e9d582da..079b89dce5 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -1820,6 +1820,20 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { }), ); } + // Same refusal, delivered at the HTTP layer: a 4xx with a JSON + // body naming the problem (Stripe answers a missing account context + // with a 422). The message is the server's answer to the caller. + if (error.httpRefusal !== undefined) { + return Effect.succeed( + ToolResult.fail({ + code: "mcp_tool_error", + message: error.httpRefusal.message, + status: error.httpRefusal.status, + retryable: false, + details: { upstream: { status: error.httpRefusal.status } }, + }), + ); + } return Effect.fail(error); }), Effect.withSpan("mcp.plugin.invoke_tool", {