Merge upstream DO-reset retry and make the portal connector recoverable

packages/gatekeeper-mcp-portal/__tests__/reconnect.test.ts

@@ -0,0 +1,99 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+import { GatekeeperUserImpl } from "../src/portal.js";
+
+// Enough of `McpAccount` for `withClient` to run a `tools/call`, plus the base reconnect account
+// methods so a fallback to the inherited flow would be observable.
+function accountStub() {
+  return {
+    async getServer() {
+      return {
+        endpoint: "https://gw.example.com/mcp",
+        serverId: "gw-example",
+        serverName: "Portal",
+        provenance: "deployment" as const,
+        auth: "oauth" as const,
+      };
+    },
+    async getConnection() {
+      // A session id already on file skips `initialize`, so the stubbed `fetch` below only has to
+      // answer the one `tools/call` request this test cares about.
+      return { authorization: "token", sessionId: "session", generation: 1 };
+    },
+    async assertConnectionCurrent() {},
+    async setMcpSessionId() { return true; },
+    async noteCredentialsExpired() {},
+    prepareReconnect: vi.fn(async () => {}),
+    revoke: vi.fn(async () => {}),
+  };
+}
+
+function user(account: ReturnType<typeof accountStub>) {
+  const ctx = {
+    props: { accountObjectId: "account-1" },
+    exports: {
+      McpAccount: {
+        idFromString: (id: string) => id,
+        get: () => account,
+      },
+    },
+  };
+  return new GatekeeperUserImpl(ctx as never, {} as never);
+}
+
+function jsonRpcResult(id: unknown, result: unknown) {
+  return new Response(JSON.stringify({ jsonrpc: "2.0", id, result }), {
+    headers: { "Content-Type": "application/json" },
+  });
+}
+
+afterEach(() => vi.unstubAllGlobals());
+
+describe("reconnect", () => {
+  it("calls portal_toggle_servers with no arguments and returns the URL it finds", async () => {
+    let toolCallParams: unknown;
+    vi.stubGlobal("fetch", async (_input: unknown, init?: RequestInit) => {
+      const request = JSON.parse(String(init?.body));
+      toolCallParams = request.params;
+      return jsonRpcResult(request.id, {
+        content: [{ type: "text", text: "Re-authenticate: https://gw.example.com/reauth?x=1" }],
+      });
+    });
+
+    const { url } = await user(accountStub()).reconnect();
+
+    expect(url).toBe("https://gw.example.com/reauth?x=1");
+    expect(toolCallParams).toEqual({ name: "portal_toggle_servers", arguments: {} });
+  });
+
+  it("throws instead of falling back to the inherited own-OAuth reconnect when no URL is found", async () => {
+    // The base `McpGatekeeperUserBase.reconnect()` this overrides calls `account.prepareReconnect`.
+    // If this override ever fell back to it on failure, the bug it fixes would return: the user
+    // reconnects, the upstream authorization is untouched, and the error comes right back.
+    vi.stubGlobal("fetch", async (_input: unknown, init?: RequestInit) => {
+      const request = JSON.parse(String(init?.body));
+      return jsonRpcResult(request.id, {
+        content: [{ type: "text", text: "Nothing usable here." }],
+      });
+    });
+
+    const account = accountStub();
+    await expect(user(account).reconnect()).rejects.toThrow(
+      "Could not get a re-authentication URL from the portal.");
+    expect(account.prepareReconnect).not.toHaveBeenCalled();
+  });
+
+  it("throws on a tool-level error without leaking the response into the user-facing message", async () => {
+    vi.stubGlobal("fetch", async (_input: unknown, init?: RequestInit) => {
+      const request = JSON.parse(String(init?.body));
+      return jsonRpcResult(request.id, {
+        isError: true,
+        content: [{ type: "text", text: "internal detail https://leak.example.com/should-not-surface" }],
+      });
+    });
+
+    const error: Error = await user(accountStub()).reconnect().catch(err => err);
+    expect(error).toBeInstanceOf(Error);
+    expect(error.message).not.toContain("leak.example.com");
+  });
+});

packages/gatekeeper-mcp-portal/__tests__/stubs/cloudflare-workers.ts

@@ -0,0 +1,20 @@
+// Stand-in for the `cloudflare:workers` module, which only exists inside workerd.
+//
+// Modules here that define a Durable Object or an `RpcTarget` cannot be imported under plain vitest
+// without it. Only the base classes are provided, and only so that `import` and `extends` resolve --
+// anything that actually needs the runtime belongs in a Workers-pool test, not here. Mirrors
+// `@gadgets/mcp-shared`'s stub of the same name.
+
+export class DurableObject<E = unknown, P = unknown> {
+  constructor(readonly ctx: unknown, readonly env: E, readonly props?: P) {}
+}
+
+export class RpcTarget {}
+
+export class WorkerEntrypoint<E = unknown, P = unknown> {
+  constructor(readonly ctx: unknown, readonly env: E, readonly props?: P) {}
+}
+
+export class RpcStub<T> {
+  constructor(readonly target: T) {}
+}

packages/gatekeeper-mcp-portal/src/portal.ts

@@ -23,7 +23,7 @@ import {
   type SupportedResource,
   type VendorDescription,
 } from "@gadgets/workshop-shared/gatekeeper";
-import { isValidToolName } from "@gadgets/mcp-shared/client";
+import { isValidToolName, type ToolIndex } from "@gadgets/mcp-shared/client";
 import { MAX_TOOLS_PER_SERVER, type ServerTrust } from "@gadgets/mcp-shared/tools";
 import { bindingNameFragment, hostOf } from "@gadgets/mcp-shared/util";
 import type { McpLog, McpLogFields } from "@gadgets/mcp-shared/log";
@@ -36,8 +36,10 @@ import { McpSessionBase } from "@gadgets/mcp-shared/session";
 import { McpFacetBase } from "@gadgets/mcp-shared/facet";
 import {
   looksLikePortal,
+  parsePortalReconnectUrl,
   parsePortalServers,
   PORTAL_LIST_SERVERS_TOOL,
+  PORTAL_TOGGLE_SERVERS_TOOL,
   reconcilePortalServers,
   type PortalServer,
   type PortalServerListing,
@@ -136,6 +138,125 @@ async function tryListPortalServers(
   }
 }
 
+/**
+ * Validates one portal-scoped resource URL, returning the scope it grants together with the upstream
+ * server that scope names, when the portal reported one. Throws if the scope is not grantable.
+ *
+ * The fragment records how much of the portal this binding may call; see `scope.ts`. A grant that
+ * names no upstream server would reach every system behind the portal, so it is refused here rather
+ * than only in the form that normally builds these URLs.
+ *
+ * The server-list result is advisory metadata, so failing to obtain it is not fatal on its own. But
+ * the endpoint still has to prove it implements the portal capability before a portal-scoped binding
+ * can be minted, which is what the `findTool` probe below establishes.
+ *
+ * Each mode then fetches only the names validation still needs. Named grants prove each selected
+ * name. A reported server needs no catalog scan; an unreported server needs one prefixed tool as
+ * fallback evidence.
+ */
+async function validatePortalScope(
+  env: Env,
+  account: DurableObjectStub<McpAccount>,
+  endpoint: string,
+  requested: URL,
+): Promise<{ scope: ToolScope & { serverId: string }; upstream: PortalServer | undefined }> {
+  const scope = parseToolScope(requested);
+  requirePortalServerScope(scope);
+
+  const listing = await tryListPortalServers(env, account, endpoint);
+  if (listing === null) {
+    const portalTool = await withClient(env, account, endpoint,
+      client => client.findTool(PORTAL_LIST_SERVERS_TOOL));
+    if (!portalTool) {
+      throw new Error("The configured MCP endpoint does not expose the portal server-list tool.");
+    }
+  }
+
+  const servers = listing?.servers ?? [];
+  const requestedTools = new Set(scope.tools ?? []);
+  let catalog: ToolIndex;
+  switch (portalCatalogValidationMode(scope, servers)) {
+    case "named-tools":
+      catalog = await withClient(env, account, endpoint,
+        client => client.listMatchingToolIndex(
+          requestedTools.size,
+          tool => requestedTools.has(tool.name),
+        ));
+      break;
+    case "reported-server":
+      catalog = { tools: [], truncated: false };
+      break;
+    case "server-evidence":
+      catalog = await withClient(env, account, endpoint,
+        client => client.listMatchingToolIndex(
+          1,
+          tool => isPortalToolGrantable(tool.name, scope.serverId),
+        ));
+      break;
+  }
+  return { scope, upstream: validateToolScopeAgainstCatalog(scope, catalog, servers) };
+}
+
+/**
+ * The servers behind the portal, for the configurator's picker. Returns an empty list when the
+ * endpoint is not a portal at all, but throws when it is one whose server list could not be read
+ * completely: an incomplete picker would silently hide servers the user is entitled to grant.
+ */
+async function listAvailablePortalServers(
+  env: Env,
+  account: DurableObjectStub<McpAccount>,
+  endpoint: string,
+): Promise<PortalServer[]> {
+  const reported = await tryListPortalServers(env, account, endpoint);
+  if (reported?.complete) return reported.servers;
+
+  const index = await withClient(env, account, endpoint,
+    client => client.listToolIndex(MAX_PORTAL_TOOL_INDEX));
+  if (!looksLikePortal(
+    index.tools, { truncated: index.truncated, cap: MAX_PORTAL_TOOL_INDEX })) return [];
+  if (index.truncated) {
+    throw new Error("Could not retrieve the portal's complete server list. Try again.");
+  }
+  return reconcilePortalServers(reported?.servers ?? [], index.tools);
+}
+
+/**
+ * Opens the portal's own re-authentication page, via `portal_toggle_servers`. Unlike
+ * `getSupportedResources`'s reconnect story, this is not the deployment's OAuth with Cloudflare
+ * Access; it is the portal's on-behalf authorization to an upstream server, and
+ * `portal_toggle_servers` is the only path Cloudflare's MCP Server Portals expose to recover it --
+ * there is no dashboard alternative.
+ *
+ * Called with no arguments: the tool is documented only as "opens a URL-based server selection
+ * page", with nothing suggesting a required argument, and the sibling `portal_list_servers` takes
+ * none either.
+ *
+ * The response shape is undocumented, so the URL is recovered by the same permissive prose scan as
+ * `portal_list_servers` rather than trusting a guessed structured field. When no URL can be found,
+ * the full response is logged for later inspection but never folded into the thrown message --
+ * its shape is unverified, so it must not reach the user-facing error.
+ */
+async function reconnectPortal(
+  env: Env,
+  account: DurableObjectStub<McpAccount>,
+  endpoint: string,
+): Promise<string> {
+  const result = await withClient(env, account, endpoint,
+    client => client.callTool(PORTAL_TOGGLE_SERVERS_TOOL, {}));
+  const url = result.isError ? null : parsePortalReconnectUrl(result);
+  if (!url) {
+    // The raw response shape is unverified, so it goes through `error` (logged, never surfaced)
+    // rather than into the thrown message below, which reaches the user.
+    logger.warn("could not recover a reconnect URL from the portal", {
+      event: "portal.reconnect.url.missing",
+      serverHost: hostOf(endpoint),
+      error: JSON.stringify(result),
+    });
+    throw new Error("Could not get a re-authentication URL from the portal.");
+  }
+  return url;
+}
+
 // HTTP handler. There is no page asking which server to connect, since the endpoint is configured,
 // so the only browser round trip is the portal's own authorization.
 
@@ -326,46 +447,9 @@ export class GatekeeperUserImpl
       throw new Error(`"${url}" does not match this connection's resource type.`);
     }
 
-    // The fragment records how much of the portal this binding may call; see `scope.ts`. A grant
-    // that names no upstream server would reach every system behind the portal, so it is refused
-    // here rather than only in the form that normally builds these URLs.
-    const scope = parseToolScope(requested);
-    requirePortalServerScope(scope);
     const account = this.#account();
-    const listedServers = await tryListPortalServers(this.env, account, server.endpoint);
-    if (listedServers === null) {
-      // The server-list result is advisory metadata, but the endpoint still has to prove it implements
-      // the portal capability before a portal-scoped binding can be minted.
-      const portalTool = await withClient(this.env, account, server.endpoint,
-        client => client.findTool(PORTAL_LIST_SERVERS_TOOL));
-      if (!portalTool) {
-        throw new Error("The configured MCP endpoint does not expose the portal server-list tool.");
-      }
-    }
-    const portalServers = listedServers?.servers ?? [];
-
-    // Fetch only the names validation still needs. Named grants prove each selected name. A reported
-    // server needs no catalog scan; an unreported server needs one prefixed tool as fallback evidence.
-    const requestedTools = new Set(scope.tools ?? []);
-    const validationMode = portalCatalogValidationMode(scope, portalServers);
-    const catalog = validationMode === "named-tools"
-      ? await withClient(this.env, account, server.endpoint,
-        client => client.listMatchingToolIndex(
-          requestedTools.size,
-          tool => requestedTools.has(tool.name),
-        ))
-      : validationMode === "reported-server"
-      ? { tools: [], truncated: false }
-      : await withClient(this.env, account, server.endpoint,
-        client => client.listMatchingToolIndex(
-          1,
-          tool => isPortalToolGrantable(tool.name, scope.serverId),
-        ));
-    const upstream = validateToolScopeAgainstCatalog(
-      scope,
-      catalog,
-      portalServers,
-    );
+    const { scope, upstream } = await validatePortalScope(
+      this.env, account, server.endpoint, requested);
 
     const props: McpGatekeeperImplProps = {
       accountObjectId: this.ctx.props.accountObjectId,
@@ -378,6 +462,22 @@ export class GatekeeperUserImpl
     return { class: this.ctx.exports.McpGatekeeperImpl({ props }), resource };
   }
 
+  /**
+   * Overrides the inherited `reconnect()`: that base implementation only restarts this
+   * gatekeeper's own OAuth with Cloudflare Access, and never touches the portal's on-behalf
+   * authorization to whichever upstream server is failing. When an upstream token lapses, the
+   * Gadget-facing error tells the user to "Call the portal_toggle_servers tool to
+   * re-authenticate" -- something only this Worker, not the Gadget, is positioned to do. So this
+   * calls that tool itself. Falling back to the inherited flow on failure would reproduce the bug
+   * this fixes: the user reconnects, nothing about the upstream authorization changes, and the
+   * error returns on the next call.
+   */
+  async reconnect(): Promise<{ url: string }> {
+    const server = await this.#account().getServer();
+    const url = await reconnectPortal(this.env, this.#account(), server.endpoint);
+    return { url };
+  }
+
   async startResourceConfigurator(_resourceUrlPattern: string): Promise<ResourceConfiguratorFrame> {
     return {
       iframeHtml: MCP_SERVER_CONFIGURATOR_HTML,
@@ -431,17 +531,7 @@ class McpServerConfiguratorUI extends RpcTarget implements McpServerConfigurator
   #portalServers(): Promise<PortalServer[]> {
     return this.#portalServersPromise ??= (async () => {
       const server = await this.#server();
-      const reported = await tryListPortalServers(this.#env, this.#account, server.endpoint);
-      if (reported?.complete) return reported.servers;
-
-      const index = await withClient(this.#env, this.#account, server.endpoint,
-        client => client.listToolIndex(MAX_PORTAL_TOOL_INDEX));
-      if (!looksLikePortal(
-        index.tools, { truncated: index.truncated, cap: MAX_PORTAL_TOOL_INDEX })) return [];
-      if (index.truncated) {
-        throw new Error("Could not retrieve the portal's complete server list. Try again.");
-      }
-      return reconcilePortalServers(reported?.servers ?? [], index.tools);
+      return listAvailablePortalServers(this.#env, this.#account, server.endpoint);
     })();
   }
 

packages/gatekeeper-mcp-portal/tsconfig.json

@@ -10,7 +10,6 @@
     "types": ["./worker-configuration.d.ts"],
     "paths": {
       "@gadgets/configurator-ui": ["../configurator-ui/src/index.ts"],
-      "@gadgets/mcp-shared/*": ["../mcp-shared/src/*"],
       "@gadgets/workshop-shared/*": ["../workshop-shared/src/*"]
     }
   },

packages/gatekeeper-mcp-portal/vitest.config.ts

@@ -0,0 +1,17 @@
+import { fileURLToPath } from "node:url";
+import capnwebValidate from "capnweb-validate/vite";
+import { defineConfig } from "vitest/config";
+
+export default defineConfig({
+  plugins: [capnwebValidate()],
+  test: {
+    include: ["__tests__/*.test.ts"],
+    environment: "node",
+    alias: {
+      // Lets `src/portal.ts` -- which declares a Durable Object, a `WorkerEntrypoint`, and an
+      // `RpcTarget` -- be imported at all. See the stub for what it does and does not provide.
+      "cloudflare:workers": fileURLToPath(
+        new URL("./__tests__/stubs/cloudflare-workers.ts", import.meta.url)),
+    },
+  },
+});

packages/gatekeeper-mcp/tsconfig.json

@@ -10,7 +10,6 @@
     "types": ["./worker-configuration.d.ts"],
     "paths": {
       "@gadgets/configurator-ui": ["../configurator-ui/src/index.ts"],
-      "@gadgets/mcp-shared/*": ["../mcp-shared/src/*"],
       "@gadgets/workshop-shared/*": ["../workshop-shared/src/*"]
     }
   },