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.
Unified · Split
| @@ -0,0 +1,5 @@ | |||
| 1 | --- | ||
| 2 | "executor": patch | ||
| 3 | --- | ||
| 4 | |||
| 5 | **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. | ||
| @@ -97,6 +97,17 @@ export class McpInvocationError extends Data.TaggedError("McpInvocationError")<{ · 96 unmodified lines | |||
| 97 | readonly name: string; | 97 | readonly name: string; |
| 98 | readonly code?: string | number; | 98 | readonly code?: string | number; |
| 99 | }; | 99 | }; |
| 100 | /** The server answered `tools/call` with a non-2xx HTTP response whose | ||
| 101 | * body was a JSON object carrying a message (a validation refusal from a | ||
| 102 | * server that answers at the HTTP layer instead of with a JSON-RPC error, | ||
| 103 | * e.g. a 422 naming a missing field). Present only for 4xx statuses other | ||
| 104 | * than the auth walls (401/403), and only when the body parsed as JSON | ||
| 105 | * with a string `message`/`error`/`error.message` — a free-text body is | ||
| 106 | * never copied out of the transport error. */ | ||
| 107 | readonly httpRefusal?: { | ||
| 108 | readonly status: number; | ||
| 109 | readonly message: string; | ||
| 110 | }; | ||
| 100 | }> {} | 111 | }> {} |
| 101 | 112 | ||
| 102 | export class McpOAuthReauthorizationRequired extends Data.TaggedError( | 113 | export class McpOAuthReauthorizationRequired extends Data.TaggedError( |
| @@ -52,6 +52,44 @@ const statusFromNumericHttpCode = (cause: unknown): number | undefined => · 51 unmodified lines | |||
| 52 | export const httpStatusFromCause = (cause: unknown): number | undefined => | 52 | export const httpStatusFromCause = (cause: unknown): number | undefined => |
| 53 | statusFromTypedTransportError(cause) ?? statusFromSsePostError(cause); | 53 | statusFromTypedTransportError(cause) ?? statusFromSsePostError(cause); |
| 54 | 54 | ||
| 55 | // A server that validates a call at the HTTP layer answers with a 4xx whose | ||
| 56 | // body is a JSON object naming the problem (Stripe's MCP: 422 for a missing | ||
| 57 | // `stripe_context`). The transport keeps that body on `SdkHttpError.data.text`. | ||
| 58 | // Read it STRUCTURALLY — parse, then pick a string message field — so a body | ||
| 59 | // that is not a JSON object (an HTML error page, a proxy banner) contributes | ||
| 60 | // nothing; only a message the server wrote for the caller comes out. | ||
| 61 | const JsonErrorBody = Schema.Union([ | ||
| 62 | Schema.Struct({ message: Schema.String }), | ||
| 63 | Schema.Struct({ error: Schema.String }), | ||
| 64 | Schema.Struct({ error: Schema.Struct({ message: Schema.String }) }), | ||
| 65 | ]); | ||
| 66 | const decodeJsonErrorBody = Schema.decodeUnknownOption(JsonErrorBody); | ||
| 67 | const SdkHttpErrorText = Schema.Struct({ text: Schema.String }); | ||
| 68 | const decodeSdkHttpErrorText = Schema.decodeUnknownOption(SdkHttpErrorText); | ||
| 69 | |||
| 70 | const parseJsonSafe = (text: string): unknown => { | ||
| 71 | // 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" | ||
| 72 | try { | ||
| 73 | // 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 | ||
| 74 | return JSON.parse(text) as unknown; | ||
| 75 | } catch { | ||
| 76 | return undefined; | ||
| 77 | } | ||
| 78 | }; | ||
| 79 | |||
| 80 | /** The caller-facing message from a JSON error body the SDK's HTTP error | ||
| 81 | * carries, or `undefined` when there is none. */ | ||
| 82 | export const httpRefusalMessageFromCause = (cause: unknown): string | undefined => { | ||
| 83 | const sdk = mcpClientSdkIfLoaded(); | ||
| 84 | if (sdk === undefined || !sdk.client.SdkHttpError.isInstance(cause)) return undefined; | ||
| 85 | const text = Option.getOrUndefined(decodeSdkHttpErrorText(cause.data))?.text; | ||
| 86 | if (text === undefined) return undefined; | ||
| 87 | const body = Option.getOrUndefined(decodeJsonErrorBody(parseJsonSafe(text))); | ||
| 88 | if (body === undefined) return undefined; | ||
| 89 | if ("message" in body) return body.message; | ||
| 90 | return typeof body.error === "string" ? body.error : body.error.message; | ||
| 91 | }; | ||
| 92 | |||
| 55 | /** Connection handshakes may receive the SDK's SSE error, whose numeric code | 93 | /** Connection handshakes may receive the SDK's SSE error, whose numeric code |
| 56 | * is an HTTP status. Keep this connection-only: JSON-RPC invocation errors | 94 | * is an HTTP status. Keep this connection-only: JSON-RPC invocation errors |
| 57 | * also have numeric `code` fields which are not HTTP statuses. */ | 95 | * also have numeric `code` fields which are not HTTP statuses. */ |
| @@ -33,7 +33,11 @@ import { · 32 unmodified lines | |||
| 33 | import { McpConnectionError, McpInvocationError, McpOAuthReauthorizationRequired } from "./errors"; | 33 | import { McpConnectionError, McpInvocationError, McpOAuthReauthorizationRequired } from "./errors"; |
| 34 | import type { McpConnection, McpConnector } from "./connection"; | 34 | import type { McpConnection, McpConnector } from "./connection"; |
| 35 | import type { McpConnectionPool } from "./connection-pool"; | 35 | import type { McpConnectionPool } from "./connection-pool"; |
| 36 | import { httpStatusFromCause, insufficientScopeFromCause } from "./http-status"; | 36 | import { |
| 37 | httpRefusalMessageFromCause, | ||
| 38 | httpStatusFromCause, | ||
| 39 | insufficientScopeFromCause, | ||
| 40 | } from "./http-status"; | ||
| 37 | 41 | ||
| 38 | // --------------------------------------------------------------------------- | 42 | // --------------------------------------------------------------------------- |
| 39 | // Helpers | 43 | // Helpers |
| @@ -161,6 +165,21 @@ const summarizeSdkFailure = (cause: unknown): { name: string; code?: string | nu · 121 unmodified lines | |||
| 161 | return typeof code === "string" || typeof code === "number" ? { name, code } : { name }; | 165 | return typeof code === "string" || typeof code === "number" ? { name, code } : { name }; |
| 162 | }; | 166 | }; |
| 163 | 167 | ||
| 168 | /** A 4xx other than the auth walls, with a JSON body that names the problem, | ||
| 169 | * is the server refusing THIS call (a validation failure at the HTTP layer) | ||
| 170 | * — not a dead transport. 401/403 keep their auth classification; 5xx and | ||
| 171 | * bodyless 4xx stay opaque, since there is nothing the caller can act on. */ | ||
| 172 | const httpRefusal = ( | ||
| 173 | status: number | undefined, | ||
| 174 | cause: unknown, | ||
| 175 | ): { readonly httpRefusal: { readonly status: number; readonly message: string } } | {} => { | ||
| 176 | if (status === undefined || status < 400 || status >= 500 || status === 401 || status === 403) { | ||
| 177 | return {}; | ||
| 178 | } | ||
| 179 | const message = httpRefusalMessageFromCause(cause); | ||
| 180 | return message === undefined ? {} : { httpRefusal: { status, message } }; | ||
| 181 | }; | ||
| 182 | |||
| 164 | const asProtocolError = (cause: unknown): ProtocolError | undefined => { | 183 | const asProtocolError = (cause: unknown): ProtocolError | undefined => { |
| 165 | const sdk = mcpClientSdkIfLoaded(); | 184 | const sdk = mcpClientSdkIfLoaded(); |
| 166 | if (sdk === undefined) return undefined; | 185 | if (sdk === undefined) return undefined; |
| @@ -402,6 +421,7 @@ const useConnection = ( · 235 unmodified lines | |||
| 402 | ...(status === 403 && insufficientScopeFromCause(cause) | 421 | ...(status === 403 && insufficientScopeFromCause(cause) |
| 403 | ? { insufficientScope: true } | 422 | ? { insufficientScope: true } |
| 404 | : {}), | 423 | : {}), |
| 424 | ...httpRefusal(status, cause), | ||
| 405 | }); | 425 | }); |
| 406 | }, | 426 | }, |
| 407 | }).pipe( | 427 | }).pipe( |
| @@ -1156,6 +1156,64 @@ describe("mcpPlugin", () => { · 1155 unmodified lines | |||
| 1156 | ), | 1156 | ), |
| 1157 | ); | 1157 | ); |
| 1158 | 1158 | ||
| 1159 | // Stripe's MCP validates the OAuth account context at the HTTP layer: a | ||
| 1160 | // call without `stripe_context` gets a 422 whose JSON body names the missing | ||
| 1161 | // field. That is the server refusing THIS call, so it must reach the caller | ||
| 1162 | // as a typed failure carrying the server's message — the same treatment as | ||
| 1163 | // a JSON-RPC invalid-params refusal — not scrub into an opaque defect. | ||
| 1164 | it.effect("surfaces a 4xx JSON refusal from tools/call as a typed tool failure", () => | ||
| 1165 | Effect.scoped( | ||
| 1166 | Effect.gen(function* () { | ||
| 1167 | const { executor, toolAddress } = yield* seedCallToolExecutor({ | ||
| 1168 | slug: "call_http_422", | ||
| 1169 | callTool: () => | ||
| 1170 | HttpServerResponse.jsonUnsafe( | ||
| 1171 | { message: "stripe_context is required for this tool" }, | ||
| 1172 | { status: 422 }, | ||
| 1173 | ), | ||
| 1174 | }); | ||
| 1175 | |||
| 1176 | const result = yield* executor.execute(toolAddress, {}, { onElicitation: "accept-all" }); | ||
| 1177 | |||
| 1178 | expect(result).toMatchObject({ | ||
| 1179 | ok: false, | ||
| 1180 | error: { | ||
| 1181 | code: "mcp_tool_error", | ||
| 1182 | message: "stripe_context is required for this tool", | ||
| 1183 | status: 422, | ||
| 1184 | retryable: false, | ||
| 1185 | details: { upstream: { status: 422 } }, | ||
| 1186 | }, | ||
| 1187 | }); | ||
| 1188 | expect(result).not.toMatchObject({ error: { details: { category: "authentication" } } }); | ||
| 1189 | }), | ||
| 1190 | ), | ||
| 1191 | ); | ||
| 1192 | |||
| 1193 | // A bodyless 4xx (or a body that is not a JSON object) has no message the | ||
| 1194 | // caller can act on, so it keeps the opaque-defect path: nothing from the | ||
| 1195 | // transport error text is copied out. | ||
| 1196 | it.effect("keeps a 4xx without a JSON message opaque", () => | ||
| 1197 | Effect.scoped( | ||
| 1198 | Effect.gen(function* () { | ||
| 1199 | const { executor, toolAddress } = yield* seedCallToolExecutor({ | ||
| 1200 | slug: "call_http_422_text", | ||
| 1201 | callTool: httpStatusCallTool(422), | ||
| 1202 | }); | ||
| 1203 | |||
| 1204 | const failure = yield* executor | ||
| 1205 | .execute(toolAddress, {}, { onElicitation: "accept-all" }) | ||
| 1206 | .pipe(Effect.flip); | ||
| 1207 | expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true); | ||
| 1208 | const error = failure as { readonly message: string; readonly cause?: unknown }; | ||
| 1209 | expect(error).toMatchObject({ message: expect.not.stringContaining("do-not-leak") }); | ||
| 1210 | const cause = error.cause as McpInvocationError; | ||
| 1211 | expect(cause.status).toBe(422); | ||
| 1212 | expect(cause.httpRefusal).toBeUndefined(); | ||
| 1213 | }), | ||
| 1214 | ), | ||
| 1215 | ); | ||
| 1216 | |||
| 1159 | it.effect("does not classify JSON-RPC error codes as auth failures", () => | 1217 | it.effect("does not classify JSON-RPC error codes as auth failures", () => |
| 1160 | Effect.scoped( | 1218 | Effect.scoped( |
| 1161 | Effect.gen(function* () { | 1219 | Effect.gen(function* () { |
| @@ -1820,6 +1820,20 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { · 1819 unmodified lines | |||
| 1820 | }), | 1820 | }), |
| 1821 | ); | 1821 | ); |
| 1822 | } | 1822 | } |
| 1823 | // Same refusal, delivered at the HTTP layer: a 4xx with a JSON | ||
| 1824 | // body naming the problem (Stripe answers a missing account context | ||
| 1825 | // with a 422). The message is the server's answer to the caller. | ||
| 1826 | if (error.httpRefusal !== undefined) { | ||
| 1827 | return Effect.succeed( | ||
| 1828 | ToolResult.fail({ | ||
| 1829 | code: "mcp_tool_error", | ||
| 1830 | message: error.httpRefusal.message, | ||
| 1831 | status: error.httpRefusal.status, | ||
| 1832 | retryable: false, | ||
| 1833 | details: { upstream: { status: error.httpRefusal.status } }, | ||
| 1834 | }), | ||
| 1835 | ); | ||
| 1836 | } | ||
| 1823 | return Effect.fail(error); | 1837 | return Effect.fail(error); |
| 1824 | }), | 1838 | }), |
| 1825 | Effect.withSpan("mcp.plugin.invoke_tool", { | 1839 | Effect.withSpan("mcp.plugin.invoke_tool", { |