mcp: surface auth-gated failures

e2e/selfhost/mcp-auth-required-add.test.ts

@@ -0,0 +1,88 @@
+// Regression guard for the add-MCP dead-end on a server that gates on auth
+// without a spec-compliant MCP challenge: the user should reach the auth-method
+// editor, not a "requires authentication, add credentials below" error with no
+// editor rendered below it.
+//
+// Selfhost-only because the probe must shape-probe a loopback server: the
+// selfhost instance runs with EXECUTOR_ALLOW_LOCAL_NETWORK so its outbound
+// probe can reach the loopback test server. Video is the artifact.
+import { randomBytes } from "node:crypto";
+
+import { expect } from "@effect/vitest";
+import { Effect } from "effect";
+import { HttpServerResponse } from "effect/unstable/http";
+import { composePluginApi } from "@executor-js/api/server";
+import { deriveMcpNamespace } from "@executor-js/plugin-mcp";
+import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api";
+import { IntegrationSlug } from "@executor-js/sdk/shared";
+import { serveTestHttpApp } from "@executor-js/sdk/testing";
+
+import { scenario } from "../src/scenario";
+import { Api, Browser, Target } from "../src/services";
+
+const api = composePluginApi([mcpHttpPlugin()] as const);
+
+scenario(
+  "Auth methods · a non-spec-compliant 401 still gets the auth editor (no dead-end)",
+  {},
+  Effect.scoped(
+    Effect.gen(function* () {
+      const target = yield* Target;
+      const browser = yield* Browser;
+      const { client: makeApiClient } = yield* Api;
+      // Auth-gated shape: a 401 with no Bearer WWW-Authenticate, no RFC 9728
+      // protected-resource metadata (the .well-known probe 404s), and a body
+      // that is neither JSON-RPC nor an OAuth error envelope.
+      const server = yield* serveTestHttpApp((request) =>
+        Effect.succeed(
+          (request.url ?? "").includes("/.well-known/")
+            ? HttpServerResponse.text("missing", { status: 404 })
+            : HttpServerResponse.jsonUnsafe({ message: "Unauthorized" }, { status: 401 }),
+        ),
+      );
+      const endpoint = server.url("/mcp");
+      // The raw 401 server reports no server name, so the probe can't seed a
+      // unique identity. Selfhost identities share one tenant, so name the
+      // integration uniquely to keep the derived slug from colliding across
+      // runs.
+      const name = `auth-gated-401-${randomBytes(3).toString("hex")}`;
+      const slug = IntegrationSlug.make(deriveMcpNamespace({ name }));
+      const identity = yield* target.newIdentity();
+      const client = yield* makeApiClient(api, identity);
+
+      yield* Effect.gen(function* () {
+        yield* browser.session(identity, async ({ page, step }) => {
+          await step("Open the add-MCP flow pointed at the auth-gated server", async () => {
+            await page.goto(`/integrations/add/mcp?url=${encodeURIComponent(endpoint)}`, {
+              waitUntil: "networkidle",
+            });
+            // Before the fix this dead-ended on a red "add credentials below"
+            // error with no editor. Now the auth-method editor renders.
+            await page.getByText("How does this server authenticate?").waitFor();
+          });
+
+          await step("The probe seeded a detected Bearer-header method", async () => {
+            await page.getByText("Method 1 · Detected").waitFor();
+            // The preview card flags the gate rather than failing the probe.
+            await page.getByText("Auth required").first().waitFor();
+          });
+
+          await step("Add the source with the declared method", async () => {
+            await page.getByPlaceholder("e.g. Linear").fill(name);
+            await page.getByRole("button", { name: "Add source" }).click();
+            // onComplete routes to the new integration's detail hub.
+            await page.waitForURL(/\/integrations\/(?!add\b)[^/?]+$/, { timeout: 30_000 });
+            const landedSlug = new URL(page.url()).pathname.split("/").filter(Boolean).at(-1);
+            expect(landedSlug, "the add flow lands on the created integration").toBe(String(slug));
+            await page.getByText("Connections").first().waitFor();
+          });
+
+          await step("The declared API key method is connectable", async () => {
+            await page.getByRole("button", { name: "Add connection" }).first().click();
+            await page.getByRole("tab", { name: "API key (Authorization)" }).waitFor();
+          });
+        });
+      }).pipe(Effect.ensuring(client.mcp.removeServer({ params: { slug } }).pipe(Effect.ignore)));
+    }),
+  ),
+);

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

@@ -3,7 +3,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js";
 import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js";
 import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";
 import { CfWorkerJsonSchemaValidator } from "@modelcontextprotocol/sdk/validation/cfworker";
-import { Effect } from "effect";
+import { Effect, Predicate } from "effect";
 
 // NOTE: `StdioClientTransport` is NOT imported eagerly. The upstream module
 // (`@modelcontextprotocol/sdk/client/stdio.js`) touches `node:child_process`
@@ -14,7 +14,7 @@ import { Effect } from "effect";
 // stdio branch of `createMcpConnector`.
 
 import type { McpRemoteIntegrationConfig, McpStdioIntegrationConfig } from "./types";
-import { McpConnectionError } from "./errors";
+import { McpConnectionError, McpOAuthReauthorizationRequired } from "./errors";
 
 // ---------------------------------------------------------------------------
 // Connection type
@@ -25,7 +25,10 @@ export type McpConnection = {
   readonly close: () => Promise<void>;
 };
 
-export type McpConnector = Effect.Effect<McpConnection, McpConnectionError>;
+export type McpConnector = Effect.Effect<
+  McpConnection,
+  McpConnectionError | McpOAuthReauthorizationRequired
+>;
 
 // ---------------------------------------------------------------------------
 // Connector input — extends stored source data with resolved auth
@@ -77,21 +80,29 @@ const connectionFromClient = (client: Client): McpConnection => ({
   close: () => client.close(),
 });
 
+const connectionFailure = (
+  transport: string,
+  message: string,
+  cause: unknown,
+): McpConnectionError | McpOAuthReauthorizationRequired => {
+  if (Predicate.isTagged(cause, "McpOAuthReauthorizationRequired")) {
+    return new McpOAuthReauthorizationRequired({ message: "MCP OAuth re-authorization required" });
+  }
+  return new McpConnectionError({ transport, message });
+};
+
 const connectClient = (input: {
   transport: string;
   createTransport: () => Parameters<Client["connect"]>[0];
-}): Effect.Effect<McpConnection, McpConnectionError> =>
+}): Effect.Effect<McpConnection, McpConnectionError | McpOAuthReauthorizationRequired> =>
   Effect.gen(function* () {
     const client = createClient();
     const transportInstance = input.createTransport();
 
     yield* Effect.tryPromise({
       try: () => client.connect(transportInstance),
-      catch: () =>
-        new McpConnectionError({
-          transport: input.transport,
-          message: `Failed connecting via ${input.transport}`,
-        }),
+      catch: (cause) =>
+        connectionFailure(input.transport, `Failed connecting via ${input.transport}`, cause),
     }).pipe(
       Effect.withSpan("plugin.mcp.connection.handshake", {
         attributes: { "plugin.mcp.transport": input.transport },
@@ -170,6 +181,12 @@ export const createMcpConnector = (input: ConnectorInput): McpConnector => {
   if (remoteTransport === "streamable-http") return connectStreamableHttp;
   if (remoteTransport === "sse") return connectSse;
 
-  // auto — try streamable-http first, fall back to SSE
-  return connectStreamableHttp.pipe(Effect.catch(() => connectSse));
+  // auto: try streamable-http first, fall back to SSE for transport failures.
+  return connectStreamableHttp.pipe(
+    Effect.catch((error) =>
+      Predicate.isTagged(error, "McpOAuthReauthorizationRequired")
+        ? Effect.fail(error)
+        : connectSse,
+    ),
+  );
 };

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

@@ -1,9 +1,7 @@
-// MCP plugin tagged errors. Each carries an `HttpApiSchema` annotation so
-// it can be `.addError(...)` directly on the API group — handlers return
-// these and HttpApi encodes them as 4xx responses with a typed body. No
-// per-handler sanitisation step.
+// MCP plugin tagged errors. API-facing errors carry `HttpApiSchema`
+// annotations so they can be `.addError(...)` directly on the API group.
 
-import { Schema } from "effect";
+import { Data, Schema } from "effect";
 
 export class McpConnectionError extends Schema.TaggedErrorClass<McpConnectionError>()(
   "McpConnectionError",
@@ -23,14 +21,20 @@ export class McpToolDiscoveryError extends Schema.TaggedErrorClass<McpToolDiscov
   { httpApiStatus: 400 },
 ) {}
 
-export class McpInvocationError extends Schema.TaggedErrorClass<McpInvocationError>()(
-  "McpInvocationError",
-  {
-    toolName: Schema.String,
-    message: Schema.String,
-  },
-  { httpApiStatus: 400 },
-) {}
+// Internal only: core wraps non-auth failures as ToolInvocationError.cause, so
+// this must carry only sanitized invocation metadata. Raw SDK causes can contain
+// upstream bodies/challenges and should not leave the invoke catch block.
+export class McpInvocationError extends Data.TaggedError("McpInvocationError")<{
+  readonly toolName: string;
+  readonly message: string;
+  readonly status?: number;
+}> {}
+
+export class McpOAuthReauthorizationRequired extends Data.TaggedError(
+  "McpOAuthReauthorizationRequired",
+)<{
+  readonly message: string;
+}> {}
 
 export class McpOAuthError extends Schema.TaggedErrorClass<McpOAuthError>()(
   "McpOAuthError",

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

@@ -30,11 +30,7 @@ export { migrateMcpAuthConfig } from "./migrate-config";
 // Request-shaped authoring: `headers: { Authorization: ["Bearer ", variable("token")] }`.
 export { variable, type ApiKeyAuthTemplate } from "@executor-js/sdk/http-auth";
 
-export {
-  McpConnectionError,
-  McpToolDiscoveryError,
-  McpInvocationError,
-  McpOAuthError,
-} from "./errors";
+// Only the API-facing errors; the internal Data.TaggedError ones stay private.
+export { McpConnectionError, McpToolDiscoveryError, McpOAuthError } from "./errors";
 
 export { deriveMcpNamespace, joinToolPath, extractManifestFromListToolsResult } from "./manifest";