[codex] fix MCP OAuth popup close handling

apps/marketing/src/components/tweet.tsx

@@ -1,9 +1,9 @@
-import { Tweet } from 'react-tweet'
+import { Tweet } from "react-tweet";
 
 export function TweetEmbed({ id }: { id: string }) {
   return (
     <div className="flex justify-center" data-theme="light">
       <Tweet id={id} />
     </div>
-  )
+  );
 }

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

@@ -1084,3 +1084,51 @@ describe("graphqlPlugin", () => {
     }),
   );
 });
+
+describe("graphqlPlugin detect URL-token fallback", () => {
+  // Port 1 connection-refuses immediately, so introspection always
+  // fails and the URL-token fallback is the only thing that can
+  // produce a candidate.
+  it.effect("returns low-confidence candidate when path has /graphql segment", () =>
+    Effect.gen(function* () {
+      const executor = yield* createExecutor(
+        makeTestConfig({ plugins: [graphqlPlugin()] as const }),
+      );
+      const results = yield* executor.sources.detect("http://127.0.0.1:1/api/graphql");
+      const gql = results.find((r) => r.kind === "graphql");
+      expect(gql).toBeDefined();
+      expect(gql?.confidence).toBe("low");
+    }),
+  );
+
+  it.effect("matches graphql on hostname label", () =>
+    Effect.gen(function* () {
+      const executor = yield* createExecutor(
+        makeTestConfig({ plugins: [graphqlPlugin()] as const }),
+      );
+      const results = yield* executor.sources.detect("http://graphql.127.0.0.1.nip.io:1/");
+      const gql = results.find((r) => r.kind === "graphql");
+      expect(gql?.confidence).toBe("low");
+    }),
+  );
+
+  it.effect("does not match graphql as a substring", () =>
+    Effect.gen(function* () {
+      const executor = yield* createExecutor(
+        makeTestConfig({ plugins: [graphqlPlugin()] as const }),
+      );
+      const results = yield* executor.sources.detect("http://127.0.0.1:1/graphqlite");
+      expect(results.find((r) => r.kind === "graphql")).toBeUndefined();
+    }),
+  );
+
+  it.effect("returns null when no token match and introspection fails", () =>
+    Effect.gen(function* () {
+      const executor = yield* createExecutor(
+        makeTestConfig({ plugins: [graphqlPlugin()] as const }),
+      );
+      const results = yield* executor.sources.detect("http://127.0.0.1:1/api/v1");
+      expect(results.find((r) => r.kind === "graphql")).toBeUndefined();
+    }),
+  );
+});

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

@@ -128,6 +128,16 @@ export interface GraphqlUpdateSourceInput {
 // Helpers
 // ---------------------------------------------------------------------------
 
+/** Match `token` as a separator-bounded run inside a URL hostname or path,
+ *  used as a low-confidence detection hint when introspection fails.
+ *  Boundary chars are everything non-alphanumeric, so `/api/graphql`,
+ *  `graphql.example.com`, `graphql-api`, and `graphql_v2` all match while
+ *  `graphqlserver` and `/graphqlite` do not. */
+const urlMatchesToken = (url: URL, token: string): boolean => {
+  const re = new RegExp(`(?:^|[^a-z0-9])${token}(?:$|[^a-z0-9])`, "i");
+  return re.test(url.hostname) || re.test(url.pathname);
+};
+
 /** Derive a namespace from an endpoint URL */
 const namespaceFromEndpoint = (endpoint: string): string => {
   // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: URL construction throws; this helper intentionally falls back to the stable default namespace
@@ -1144,16 +1154,34 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => {
           Effect.catch(() => Effect.succeed(false)),
         );
 
-        if (!ok) return null;
-
         const name = namespaceFromEndpoint(trimmed);
-        return new SourceDetectionResult({
-          kind: "graphql",
-          confidence: "high",
-          endpoint: trimmed,
-          name,
-          namespace: name,
-        });
+
+        if (ok) {
+          return new SourceDetectionResult({
+            kind: "graphql",
+            confidence: "high",
+            endpoint: trimmed,
+            name,
+            namespace: name,
+          });
+        }
+
+        // Low-confidence URL-token fallback. Introspection can fail for
+        // many reasons (auth, CORS, the endpoint disabled introspection
+        // in production, transport errors). When the URL itself
+        // strongly implies GraphQL, surface a candidate so the user
+        // can still pick it from the detect dropdown.
+        if (urlMatchesToken(parsed.value, "graphql")) {
+          return new SourceDetectionResult({
+            kind: "graphql",
+            confidence: "low",
+            endpoint: trimmed,
+            name,
+            namespace: name,
+          });
+        }
+
+        return null;
       }),
 
     routes: () => GraphqlGroup,

packages/plugins/mcp/src/react/AddMcpSource.tsx

@@ -298,6 +298,7 @@ export default function AddMcpSource(props: {
   const oauth = useOAuthPopupFlow<OAuthCompletionPayload>({
     popupName: "mcp-oauth",
     popupBlockedMessage: "OAuth popup was blocked",
+    detectPopupClosed: false,
     startErrorMessage: "Failed to start OAuth",
   });
 

packages/plugins/mcp/src/react/McpSignInButton.tsx

@@ -72,6 +72,7 @@ export default function McpSignInButton(props: { sourceId: string }) {
         bindings ?? [],
       )}
       isConnected={isConnected}
+      detectPopupClosed={false}
       onConnected={async (nextConnectionId) => {
         await setBinding({
           params: { scopeId: userScopeId },

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

@@ -17,7 +17,7 @@ import {
   type SecretProvider,
 } from "@executor-js/sdk";
 
-import { mcpPlugin } from "./plugin";
+import { mcpPlugin, userFacingProbeMessage } from "./plugin";
 import { MCP_OAUTH_CONNECTION_SLOT } from "./types";
 import { extractManifestFromListToolsResult, deriveMcpNamespace, joinToolPath } from "./manifest";
 import { serveMcpServer } from "../testing";
@@ -863,3 +863,89 @@ describe("MCP destructiveHint → requiresApproval", () => {
     }),
   );
 });
+
+describe("userFacingProbeMessage", () => {
+  it("turns auth-required into a credentials-asking message", () => {
+    const message = userFacingProbeMessage({
+      kind: "not-mcp",
+      category: "auth-required",
+      reason: "401 without Bearer WWW-Authenticate — not an MCP auth challenge",
+    });
+    expect(message).toMatch(/requires authentication/i);
+    expect(message).toMatch(/credentials/i);
+  });
+
+  it("turns wrong-shape into a 'not an MCP server' message", () => {
+    const message = userFacingProbeMessage({
+      kind: "not-mcp",
+      category: "wrong-shape",
+      reason: "2xx POST body is not a JSON-RPC envelope",
+    });
+    expect(message).toMatch(/doesn't appear to host an MCP server/i);
+  });
+
+  it("turns unreachable into a connectivity message", () => {
+    const message = userFacingProbeMessage({
+      kind: "unreachable",
+      reason: "ECONNREFUSED",
+    });
+    expect(message).toMatch(/couldn't reach/i);
+  });
+
+  it("never surfaces the raw probe reason verbatim", () => {
+    const reasons = [
+      "401 without Bearer WWW-Authenticate — not an MCP auth challenge",
+      "2xx POST body is not a JSON-RPC envelope",
+      "GET response is not an SSE stream",
+      "unexpected status 418 for initialize",
+    ] as const;
+    for (const reason of reasons) {
+      const auth = userFacingProbeMessage({ kind: "not-mcp", category: "auth-required", reason });
+      const wrong = userFacingProbeMessage({ kind: "not-mcp", category: "wrong-shape", reason });
+      expect(auth).not.toContain(reason);
+      expect(wrong).not.toContain(reason);
+    }
+  });
+});
+
+describe("mcpPlugin detect URL-token fallback", () => {
+  // Port 1 connection-refuses immediately, so wire-shape detection
+  // returns `unreachable` and the URL-token fallback is the only thing
+  // that can produce a candidate.
+  it.effect("returns low-confidence candidate when path has /mcp segment", () =>
+    Effect.gen(function* () {
+      const executor = yield* createExecutor(makeTestConfig({ plugins: [mcpPlugin()] as const }));
+      const results = yield* executor.sources.detect("http://127.0.0.1:1/api/mcp");
+      const mcp = results.find((r) => r.kind === "mcp");
+      expect(mcp).toBeDefined();
+      expect(mcp?.confidence).toBe("low");
+    }),
+  );
+
+  it.effect("matches mcp on hostname label", () =>
+    Effect.gen(function* () {
+      const executor = yield* createExecutor(makeTestConfig({ plugins: [mcpPlugin()] as const }));
+      const results = yield* executor.sources.detect("http://mcp.127.0.0.1.nip.io:1/");
+      const mcp = results.find((r) => r.kind === "mcp");
+      expect(mcp?.confidence).toBe("low");
+    }),
+  );
+
+  it.effect("does not match mcp as a substring", () =>
+    Effect.gen(function* () {
+      const executor = yield* createExecutor(makeTestConfig({ plugins: [mcpPlugin()] as const }));
+      // `/mcpstore` is a substring containing `mcp` but `mcp` is not a
+      // separator-bounded run, so the URL-token fallback must not fire.
+      const results = yield* executor.sources.detect("http://127.0.0.1:1/mcpstore");
+      expect(results.find((r) => r.kind === "mcp")).toBeUndefined();
+    }),
+  );
+
+  it.effect("returns null when no token match and no wire-shape match", () =>
+    Effect.gen(function* () {
+      const executor = yield* createExecutor(makeTestConfig({ plugins: [mcpPlugin()] as const }));
+      const results = yield* executor.sources.detect("http://127.0.0.1:1/api/v1");
+      expect(results.find((r) => r.kind === "mcp")).toBeUndefined();
+    }),
+  );
+});

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

@@ -42,7 +42,7 @@ import { discoverTools } from "./discover";
 import { McpConnectionError, McpInvocationError, McpToolDiscoveryError } from "./errors";
 import { invokeMcpTool } from "./invoke";
 import { deriveMcpNamespace, type McpToolManifest, type McpToolManifestEntry } from "./manifest";
-import { probeMcpEndpointShape } from "./probe-shape";
+import { probeMcpEndpointShape, type McpShapeProbeResult } from "./probe-shape";
 import {
   MCP_HEADER_AUTH_SLOT,
   MCP_OAUTH_CLIENT_ID_SLOT,
@@ -195,6 +195,34 @@ const toBinding = (entry: McpToolManifestEntry): McpToolBinding =>
 
 const MCP_PLUGIN_ID = "mcp";
 
+/** Match `token` as a separator-bounded run inside a URL hostname or path,
+ *  used as a low-confidence detection hint when wire-shape detection fails.
+ *  Boundary chars are everything non-alphanumeric, so `/api/mcp`,
+ *  `mcp.example.com`, `mcp-server`, and `mcp_v1` all match while
+ *  `mcphost.com` and `/mcpstore` do not. */
+const urlMatchesToken = (url: URL, token: string): boolean => {
+  const re = new RegExp(`(?:^|[^a-z0-9])${token}(?:$|[^a-z0-9])`, "i");
+  return re.test(url.hostname) || re.test(url.pathname);
+};
+
+/** Translate a non-MCP probe outcome into a message a user can act on.
+ *  The technical `reason` (`401 without Bearer WWW-Authenticate — not an
+ *  MCP auth challenge`, etc.) stays in telemetry via the probe span; the
+ *  user gets a sentence pointing at their next step. Exported for tests. */
+export const userFacingProbeMessage = (
+  shape: Extract<McpShapeProbeResult, { kind: "not-mcp" } | { kind: "unreachable" }>,
+): string => {
+  if (shape.kind === "unreachable") {
+    return "Couldn't reach this URL. Check the address, your network, and that the server is running.";
+  }
+  switch (shape.category) {
+    case "auth-required":
+      return "This server requires authentication. Add credentials (Authorization header, query parameter, or API key) below and retry.";
+    case "wrong-shape":
+      return "This URL doesn't appear to host an MCP server. Double-check the address, including the path.";
+  }
+};
+
 const scopeRanks = (ctx: PluginCtx<McpBindingStore>): ReadonlyMap<string, number> =>
   new Map(ctx.scopes.map((scope, index) => [String(scope.id), index]));
 
@@ -1042,10 +1070,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => {
           if (shape.kind !== "mcp") {
             return yield* new McpConnectionError({
               transport: "remote",
-              message:
-                shape.kind === "not-mcp"
-                  ? `Endpoint does not look like an MCP server: ${shape.reason}`
-                  : `Could not reach endpoint: ${shape.reason}`,
+              message: userFacingProbeMessage(shape),
             });
           }
 
@@ -1075,7 +1100,8 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => {
 
           return yield* new McpConnectionError({
             transport: "remote",
-            message: "MCP server requires authentication but OAuth discovery failed",
+            message:
+              "This server requires authentication, but OAuth metadata wasn't found. Add credentials (Authorization header, query parameter, or API key) below and retry.",
           });
         }).pipe(
           Effect.withSpan("mcp.plugin.probe_endpoint", {
@@ -1662,30 +1688,41 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => {
           });
         }
 
-        // host publishes RFC 9728 + 8414 metadata) would be classified
-        // as MCP whenever the cross-plugin detector fans out to us.
+        // The shape probe inspects the JSON-RPC `initialize` response
+        // and only classifies as MCP when the wire shape is
+        // unambiguous (2xx + JSON-RPC body, 2xx SSE, or 401 + Bearer +
+        // JSON-RPC error envelope). That body-shape gate is what
+        // separates real MCP servers — including those that
+        // authenticate with static API keys and publish no OAuth
+        // metadata — from unrelated OAuth-protected services whose
+        // host happens to expose RFC 9728/8414 documents.
         const shape = yield* probeMcpEndpointShape(trimmed, { httpClientLayer });
-        if (shape.kind !== "mcp") return null;
-
-        // Confirm OAuth metadata is actually reachable. The shape
-        // probe already found a Bearer challenge; the core OAuth
-        // service's probe verifies the AS metadata resolves so we
-        // don't classify endpoints that challenge but have no
-        // discovery.
-        const probeOk = yield* ctx.oauth.probe({ endpoint: trimmed }).pipe(
-          Effect.map(() => true),
-          Effect.catch(() => Effect.succeed(false)),
-          Effect.withSpan("mcp.plugin.probe_oauth"),
-        );
-        if (!probeOk) return null;
+        if (shape.kind === "mcp") {
+          return new SourceDetectionResult({
+            kind: "mcp",
+            confidence: "high",
+            endpoint: trimmed,
+            name,
+            namespace,
+          });
+        }
 
-        return new SourceDetectionResult({
-          kind: "mcp",
-          confidence: "high",
-          endpoint: trimmed,
-          name,
-          namespace,
-        });
+        // Low-confidence URL-token fallback. When wire-shape detection
+        // can't confirm MCP (server unreachable, behind unusual auth,
+        // returns a non-canonical body, etc.) but the URL itself is a
+        // strong hint, surface a candidate so the user can still pick
+        // it from the detect dropdown rather than getting nothing.
+        if (urlMatchesToken(parsed.value, "mcp")) {
+          return new SourceDetectionResult({
+            kind: "mcp",
+            confidence: "low",
+            endpoint: trimmed,
+            name,
+            namespace,
+          });
+        }
+
+        return null;
       }).pipe(
         Effect.catch(() => Effect.succeed(null)),
         Effect.withSpan("mcp.plugin.detect", {