Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mcp-http-refusal.md
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions packages/plugins/mcp/src/sdk/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
38 changes: 38 additions & 0 deletions packages/plugins/mcp/src/sdk/http-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
22 changes: 21 additions & 1 deletion packages/plugins/mcp/src/sdk/invoke.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -402,6 +421,7 @@ const useConnection = (
...(status === 403 && insufficientScopeFromCause(cause)
? { insufficientScope: true }
: {}),
...httpRefusal(status, cause),
});
},
}).pipe(
Expand Down
58 changes: 58 additions & 0 deletions packages/plugins/mcp/src/sdk/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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* () {
Expand Down
14 changes: 14 additions & 0 deletions packages/plugins/mcp/src/sdk/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down
Loading