From 7efbbb05cd235ecf0c080aff162e04dd8a8dd5c1 Mon Sep 17 00:00:00 2001 From: Dan Carter Date: Thu, 13 Aug 2026 10:39:44 -0400 Subject: [PATCH] Support large MCP portal catalogs --- packages/gatekeeper-mcp-portal/README.md | 58 ++++- .../__tests__/config.test.ts | 87 ++++++++ .../__tests__/configurator-ui.test.ts | 11 +- packages/gatekeeper-mcp-portal/src/config.ts | 75 ++++++- .../server-configurator-types.d.ts | 24 +- .../configurator/server-configurator-ui.tsx | 24 +- packages/gatekeeper-mcp-portal/src/portal.ts | 210 ++++++++++-------- .../__tests__/client-pagination.test.ts | 20 +- packages/mcp-shared/__tests__/portal.test.ts | 102 +++++---- packages/mcp-shared/__tests__/scope.test.ts | 9 + packages/mcp-shared/src/client.ts | 16 +- packages/mcp-shared/src/portal.ts | 113 ++++++---- 12 files changed, 512 insertions(+), 237 deletions(-) diff --git a/packages/gatekeeper-mcp-portal/README.md b/packages/gatekeeper-mcp-portal/README.md index e4e643c2f..d2d8265c1 100644 --- a/packages/gatekeeper-mcp-portal/README.md +++ b/packages/gatekeeper-mcp-portal/README.md @@ -28,11 +28,21 @@ every system the organization has connected in one click. pinning different tools of one upstream server share both the name and the endpoint, so the scope is the only thing that distinguishes them. -The session API — a typed method per described tool, plus `callTool`, `getActionResult`, and `listTools` — is -the same as [`gatekeeper-mcp`](../gatekeeper-mcp/README.md#what-it-provides). - -Scoping to one server also shrinks what the agent reads: a 57-tool portal generates 57 tool -signatures unscoped, and 12 when scoped to one server. +The session API — a typed method per described tool, plus `callTool`, `getActionResult`, and +`listTools` with progressive search/name options — is the same as +[`gatekeeper-mcp`](../gatekeeper-mcp/README.md#what-it-provides). + +A server-wide grant can cover more tools than one catalog describes, so its authority is not limited +to the ones with generated signatures. `listTools({ search })` searches beyond the bounded preview +and returns up to 20 compact matches within the shared 5,000-tool / 4 MiB discovery scan; +`listTools({ name })` loads one exact bounded definition, and `callTool` resolves that name under the +same bound before dispatch. Every path rejects names outside the +binding's scope before loading the catalog or contacting the endpoint. Discovery and read results are +recorded as observations; writes retain the ordinary approval flow. + +Scoping to one server also shrinks what the agent reads: only that server's tools are rendered as +signatures, and only as many as the budget describes. Additional tools are discovered on demand +within the explicit scan limits above; exceeding a limit fails rather than pretending a tool is absent. ## Configuration @@ -45,6 +55,10 @@ signatures unscoped, and 12 when scoped to one server. | `MCP_PORTAL_TRUST_ANNOTATIONS` | `true` to let upstream tool annotations drive auto-approval. Off by default; see below. | | `MCP_ALLOW_INSECURE` | `"true"` to disable the endpoint checks entirely: permits `http://` **and** private, loopback, link-local, and cloud-metadata hosts, for the portal and every OAuth URL discovered from it. Local dev only. | +The portal must expose upstream tools directly. Use a portal where Code Mode is off or opt-in, or +append `?codemode=off` when its policy is default-on. Enforced Code Mode is unsupported. Do not add +an `optimize_context` parameter or opt in to Code Mode on `MCP_PORTAL_URL`. + Only `MCP_ALLOW_INSECURE` is set in the repo's `wrangler.jsonc`, pinned to `"false"` so the default is explicit rather than merely absent. None of the others is, and a portal URL committed there would become the default for every deployment of this repo and would send their users' OAuth flows to @@ -91,10 +105,10 @@ pinned — which tools: ``` Server · Which server behind this portal to grant. Its tools appear next. -[ 🔍 GitHub ] 12 tools · 8 read-only, 4 need approval +[ 🔍 GitHub ] Tools · Choose how much of this server the Gadget may call. -(•) All tools Every tool this server offers (12 today), including ones it adds later. +(•) All tools Every tool this server offers, including ones it adds later. ( ) Choose tools Only the tools you tick. Anything else is refused, including tools added later. Allowed tools · Read-only tools return data straight away. The rest queue for your approval. @@ -130,17 +144,37 @@ recovered from two facts in the portal's documented contract: Detection is a capability probe — does the endpoint offer `portal_list_servers`? — not a hostname match, so it works for a custom portal hostname and for any other aggregator adopting the -convention. A *truncated* catalog counts as a portal whether or not the probe tool is in it: +convention. A *truncated* listing counts as a portal whether or not the probe tool is in it: `tools/list` is unordered, so concluding "not a portal" because the evidence fell past the cut would -fail open on the `portal_*` exclusion below. Truncation is reported by `listTools` rather than -inferred from the tool count, because either cap can stop it — `MAX_TOOLS_PER_SERVER` (200) or the -96 KiB UTF-8 catalog budget, and the latter can cut a catalog of verbose tools short while leaving an +fail open on the `portal_*` exclusion below. Truncation is reported by the client rather than +inferred from the tool count, because either cap can stop a listing — the count the caller asked for, +or the 96 KiB UTF-8 budget, and the latter can cut a listing of verbose tools short while leaving an array that looks complete. The byte budget leaves 32 KiB below Durable Object's per-value limit for the cache wrapper and serialization overhead; if storage nevertheless rejects the cache value, the fresh catalog is still used for that operation rather than turning a cache miss into a failure. +### Surveying a portal too large for one catalog + +The configurator normally gets server names directly from `portal_list_servers`, without surveying +every upstream tool. If that response is unavailable or only partly understood, it falls back to a +**name-only tool index** of up to 1,000 entries. The index detects the portal and recovers server +membership from tool-name prefixes; it carries no descriptions, schemas, or policy claims, so the +96 KiB result budget covers as many names as possible. A truncated fallback cannot establish the +complete server list and blocks the form rather than presenting a partial list as complete. + +After a server is selected, a separate filtered scan returns up to 200 compact summaries from that +server. Each summary carries its bounded title, description, and annotations, and is classified +through the shared `tools.ts` trust boundary before it becomes a read-versus-approval label. The +filter is applied before result budgets, so unrelated servers cannot crowd the selected one out. +Only the returned prefix is offered for an individual-tool grant; additional tools require the +server-wide grant, and a call resolves the full definition before approval or dispatch. + +Index entries are typed separately (`IndexedTool`) to distinguish name-only survey results from tool +definitions rendered into approval prompts or handed to an agent. + The server list is advisory: it supplies display names and ordering while tool-name prefixes remain -the authority on membership, and a failed call degrades to bare ids. The gatekeeper makes that call +the authority on membership, and a failed call degrades to bare ids recovered from tool prefixes. +The gatekeeper makes that call while building a form, so it does not pass through the approval queue. Failing to reach the portal at all is different, and blocks the grant rather than falling back to the bare endpoint — the configurator reports it and stays unsubmittable. diff --git a/packages/gatekeeper-mcp-portal/__tests__/config.test.ts b/packages/gatekeeper-mcp-portal/__tests__/config.test.ts index f8f70015b..caa5dea59 100644 --- a/packages/gatekeeper-mcp-portal/__tests__/config.test.ts +++ b/packages/gatekeeper-mcp-portal/__tests__/config.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from "vitest"; import { + isPortalToolGrantable, + portalCatalogValidationMode, portalResource, portalAuthRequiresReconnect, portalServer, @@ -7,6 +9,7 @@ import { portalTrust, readPortalConfig, requirePortalServerScope, + toolGrantOptions, } from "../src/config.js"; function env(overrides: Record = {}): Env { @@ -168,6 +171,38 @@ describe("requirePortalServerScope", () => { // Pinned-and-empty denies everything, which is fail-closed and fine to mint. expect(() => requirePortalServerScope({ serverId: "github", tools: [] })).not.toThrow(); }); + + it("rejects invalid names and cross-server tools before endpoint discovery", () => { + expect(() => requirePortalServerScope({ serverId: "" })).toThrow(/server id/i); + expect(() => requirePortalServerScope({ serverId: "x".repeat(600) })).toThrow(/server id/i); + expect(() => requirePortalServerScope({ serverId: "github", tools: [""] })) + .toThrow(/tool name/i); + expect(() => requirePortalServerScope({ serverId: "github", tools: ["jira_search"] })) + .toThrow(/does not belong/i); + expect(() => requirePortalServerScope({ + serverId: "portal", tools: ["portal_toggle_servers"], + })).toThrow(/portal management tool/i); + }); +}); + +describe("portal catalog validation", () => { + it("selects exact-tool validation for a non-empty pinned grant", () => { + expect(portalCatalogValidationMode( + { serverId: "github", tools: ["github_search"] }, [])).toBe("named-tools"); + }); + + it("uses the same server evidence for empty pinned and server-wide grants", () => { + for (const scope of [{ serverId: "github", tools: [] }, { serverId: "github" }]) { + expect(portalCatalogValidationMode(scope, [])).toBe("server-evidence"); + expect(portalCatalogValidationMode(scope, [{ id: "github" }])).toBe("reported-server"); + } + }); + + it("excludes portal-native and cross-server tools from portal grants", () => { + expect(isPortalToolGrantable("github_search", "github")).toBe(true); + expect(isPortalToolGrantable("linear_search", "github")).toBe(false); + expect(isPortalToolGrantable("portal_toggle_servers", "portal")).toBe(false); + }); }); describe("portalTokenFor", () => { @@ -223,3 +258,55 @@ describe("portalTokenFor", () => { expect(portalTokenFor(configured, "https://gw.example.com/mcp")).toBeNull(); }); }); + +describe("toolGrantOptions", () => { + const tools = [ + { name: "google_list_events", annotations: { readOnlyHint: true } }, + { name: "google_delete_event", annotations: { readOnlyHint: false } }, + { name: "google_send_mail" }, + ]; + + it("classifies each bounded summary from the annotation runtime policy uses", () => { + const options = toolGrantOptions({ + serverId: "google", + tools, + trust: "byo", + }); + expect(options.map(option => [option.value, option.meta])).toEqual([ + ["google_list_events", "read-only"], + ["google_delete_event", "needs approval"], + ["google_send_mail", "needs approval"], + ]); + }); + + it("uses summary text and degrades to the bare name without it", () => { + const options = toolGrantOptions({ + serverId: "google", + tools: [{ + name: "google_list_events", title: "List events", + description: "Lists calendar events.\nSecond line ignored.", + annotations: { readOnlyHint: true }, + }, ...tools.slice(1)], + trust: "byo", + }); + expect(options[0]).toEqual({ + value: "google_list_events", + title: "List events", + subtitle: "Lists calendar events.", + meta: "read-only", + }); + // No detail for this one: the prefix is still stripped, and no description is invented. + expect(options[1]).toMatchObject({ title: "delete_event", subtitle: undefined }); + }); + + it("does not let a portal's annotations drive auto-approval labels on an unvetted portal", () => { + // `meta` reports only read-versus-action, which is the distinction the person granting acts on. + // Auto-approval needs a vetted deployment as well, and is not something this form claims. + for (const trust of ["vetted", "byo"] as const) { + const options = toolGrantOptions({ serverId: "google", tools, trust }); + expect(options.map(option => option.meta)) + .toEqual(["read-only", "needs approval", "needs approval"]); + } + }); + +}); diff --git a/packages/gatekeeper-mcp-portal/__tests__/configurator-ui.test.ts b/packages/gatekeeper-mcp-portal/__tests__/configurator-ui.test.ts index 556ff0b4b..ed7560020 100644 --- a/packages/gatekeeper-mcp-portal/__tests__/configurator-ui.test.ts +++ b/packages/gatekeeper-mcp-portal/__tests__/configurator-ui.test.ts @@ -19,7 +19,7 @@ vi.mock("@gadgets/configurator-ui", () => ({ type Values = McpServerConfiguratorValues; -// The module caches tool lists at module scope, which is safe only because the host hands it a fresh +// The module caches the server list at module scope, which is safe only because the host hands it a fresh // iframe -- and so a fresh realm -- for every account and resource pattern. Re-importing per test // reproduces that; without it one test's cached list is served to the next. async function loadSpec() { @@ -88,7 +88,7 @@ describe("portal configurator", () => { expect(rendered).not.toContain("CheckboxList"); }); - it("keeps an empty portal ungrantable instead of serializing its future servers", async () => { + it("shows corrective guidance when the endpoint exposes no direct upstream tools", async () => { const ui = { getEndpoint: async () => "https://gw.example.com/mcp", listServerOptions: async () => [], @@ -99,11 +99,12 @@ describe("portal configurator", () => { }, ui); app.render(); - await vi.waitFor(() => expect(app.values.endpointKind).toBe("portal")); + await vi.waitFor(() => expect(app.values.endpointKind).toBe("empty")); expect(app.values.server).toBeNull(); expect(spec.isReady({ values: app.values })).toBe(false); - await expect(spec.resourceUrl({ values: app.values, ui } as never)) - .rejects.toThrow(/Choose a server/); + const rendered = JSON.stringify(app.render()); + expect(rendered).toContain("codemode=off"); + expect(rendered).not.toContain("Could not reach the portal"); }); it("shows every tool as a disabled preview for an all-tools grant", () => { diff --git a/packages/gatekeeper-mcp-portal/src/config.ts b/packages/gatekeeper-mcp-portal/src/config.ts index e57aa0018..2f5164fca 100644 --- a/packages/gatekeeper-mcp-portal/src/config.ts +++ b/packages/gatekeeper-mcp-portal/src/config.ts @@ -6,11 +6,14 @@ // connector instead of offering a dead end. See the README. import type { SupportedResource } from "@gadgets/workshop-shared/gatekeeper"; +import type { ConfiguratorUIOption } from "@gadgets/configurator-ui"; import type { ConnectedServer, ServerAuthKind } from "@gadgets/mcp-shared/account"; -import type { ToolScope } from "@gadgets/mcp-shared/scope"; +import { isValidToolName, type McpTool } from "@gadgets/mcp-shared/client"; +import { scopeAllows, type ToolScope } from "@gadgets/mcp-shared/scope"; import { fetchOptions } from "@gadgets/mcp-shared/fetch"; import { sameEndpoint } from "@gadgets/mcp-shared/scope"; -import type { ServerTrust } from "@gadgets/mcp-shared/tools"; +import { isPortalNativeTool, type PortalServer } from "@gadgets/mcp-shared/portal"; +import { classifyTool, type ServerTrust } from "@gadgets/mcp-shared/tools"; /** The configured portal, once the deployment's vars have been read and validated. */ export type PortalConfig = { @@ -86,13 +89,69 @@ export function readPortalConfig(env: Env): PortalConfig | null { * This is the enforcement, not the configurator. The form refuses to *emit* such a URL, but a * resource URL is not only ever produced by the form: an agent passes a concrete one to * `requestConnection`, and any URL under the portal's origin reaches `getGatekeeperClassFor`. A - * rule that lives only in the iframe is a suggestion; the facet is minted here. + * rule that lives only in the iframe is a suggestion; the facet is minted here. The assertion + * narrows the scope so callers do not need to repeat the invariant. */ -export function requirePortalServerScope(scope: ToolScope): void { - if (scope.serverId !== undefined) return; - throw new Error( - "A portal grant has to name one of the servers behind the portal. Granting the portal itself " + - "would cover every system connected to it, including ones added later."); +export function requirePortalServerScope( + scope: ToolScope, +): asserts scope is ToolScope & { serverId: string } { + if (scope.serverId === undefined) { + throw new Error( + "A portal grant has to name one of the servers behind the portal. Granting the portal itself " + + "would cover every system connected to it, including ones added later."); + } + if (!isValidToolName(scope.serverId)) throw new Error("Invalid portal server id."); + for (const name of scope.tools ?? []) { + if (!isValidToolName(name)) throw new Error("Invalid MCP tool name."); + if (isPortalNativeTool(name)) { + throw new Error(`Portal management tool "${name}" cannot be granted.`); + } + if (!isPortalToolGrantable(name, scope.serverId)) { + throw new Error(`Tool "${name}" does not belong to portal server "${scope.serverId}".`); + } + } +} + +/** Whether one tool may appear in a grant for this portal server. */ +export function isPortalToolGrantable(name: string, serverId: string): boolean { + return scopeAllows({ serverId }, name, true); +} + +/** Which catalog evidence is needed to validate one portal scope. */ +export function portalCatalogValidationMode( + scope: ToolScope & { serverId: string }, + reportedServers: readonly Pick[], +): "named-tools" | "reported-server" | "server-evidence" { + if ((scope.tools?.length ?? 0) > 0) return "named-tools"; + return reportedServers.some(server => server.id === scope.serverId) + ? "reported-server" + : "server-evidence"; +} + +/** + * Renders one upstream server's tools as choices on the grant form. + * + * The bounded summaries carry both display text and the annotations classification is decided from, + * so the picker cannot disagree with runtime policy merely because a full schema did not fit. + */ +export function toolGrantOptions(args: { + serverId: string; + tools: readonly McpTool[]; + trust: ServerTrust; +}): ConfiguratorUIOption[] { + return args.tools.map(tool => { + return { + value: tool.name, + // Within a chosen server the `{server_id}_` prefix is noise, so it is shown stripped while + // `value` keeps the wire name the grant is actually recorded with. + title: tool.title ?? tool.name.slice(args.serverId.length + 1), + subtitle: tool.description?.split(/\r?\n/)[0], + // Surfaced here so the person granting can see, per tool, whether calls will interrupt them. + meta: classifyTool(tool, args.trust).mode === "read" + ? "read-only" + : "needs approval", + }; + }); } /** The single resource type this connector offers, scoped to the configured portal's origin. */ diff --git a/packages/gatekeeper-mcp-portal/src/configurator/server-configurator-types.d.ts b/packages/gatekeeper-mcp-portal/src/configurator/server-configurator-types.d.ts index c6ce0a249..20444859b 100644 --- a/packages/gatekeeper-mcp-portal/src/configurator/server-configurator-types.d.ts +++ b/packages/gatekeeper-mcp-portal/src/configurator/server-configurator-types.d.ts @@ -6,8 +6,7 @@ import type { ConfiguratorUIOption } from "@gadgets/configurator-ui"; /** Values collected by the configurator and turned into a resource URL. */ export type McpServerConfiguratorValues = { /** - * The chosen upstream server id, when the endpoint is a portal fronting several servers. Null - * until one is picked; a plain MCP server never sets it. + * The chosen upstream server id. Null until one is picked. */ server?: string | null; /** @@ -28,16 +27,14 @@ export type McpServerConfiguratorValues = { /** * Whether the upstream server list has been retrieved yet, discovered after first paint. * - * Only three states, and there is deliberately no "this is a plain endpoint" one. Every grant - * this connector makes names one upstream server, so a listing that comes back empty is a portal - * with nothing grantable rather than a bare endpoint to grant whole. A fourth state meaning the - * latter is what previously let a portal with no current upstreams serialize to its bare URL, - * taking in every server added to it later. + * There is deliberately no "this is a plain endpoint" state. Every grant this connector makes + * names one upstream server, so an empty listing is ungrantable rather than a bare endpoint whose + * grant would silently include servers added later. * - * `"unavailable"` is failure, and is deliberately distinct from `"portal"`: not being able to ask - * which servers are behind the endpoint must block the grant. + * `"empty"` is a successful response with no direct upstream tools; `"unavailable"` is a failed + * request. Both block the grant, but they require different remediation. */ - endpointKind?: "unknown" | "portal" | "unavailable"; + endpointKind?: "unknown" | "portal" | "empty" | "unavailable"; }; /** Capability the configurator iframe is given, to describe the account's server. */ @@ -46,7 +43,7 @@ export interface McpServerConfiguratorRpc { getEndpoint(): Promise; /** - * The upstream servers behind the portal, with tool counts. + * The upstream servers behind the portal. * * Empty covers both "this endpoint is not a portal" and "it is a portal fronting nothing right * now". The form deliberately does not distinguish them: every grant here names one upstream @@ -58,8 +55,7 @@ export interface McpServerConfiguratorRpc { listServerOptions(): Promise; /** - * Tools the grant may cover, annotated with whether calls need approval. Narrowed to one - * upstream server when `serverId` is given. + * Tools the grant may cover on one upstream server, annotated with whether calls need approval. */ - listToolOptions(serverId?: string): Promise; + listToolOptions(serverId: string): Promise; } diff --git a/packages/gatekeeper-mcp-portal/src/configurator/server-configurator-ui.tsx b/packages/gatekeeper-mcp-portal/src/configurator/server-configurator-ui.tsx index c8c272d29..c3db06231 100644 --- a/packages/gatekeeper-mcp-portal/src/configurator/server-configurator-ui.tsx +++ b/packages/gatekeeper-mcp-portal/src/configurator/server-configurator-ui.tsx @@ -47,7 +47,7 @@ export default { }, isReady({ values }) { - if (values.endpointKind === "unavailable" || !values.server) return false; + if (values.endpointKind !== "portal" || !values.server) return false; return values.mode === "all" || (values.tools ?? "").split(",").some(name => name.trim().length > 0); }, @@ -69,7 +69,7 @@ export default { if (values.endpointKind === "unknown") { void serverOptions(ui).then( servers => setValues({ - endpointKind: "portal", + endpointKind: servers.length > 0 ? "portal" : "empty", server: servers.length === 1 ? servers[0].value : values.server, }), () => setValues({ endpointKind: "unavailable" }), @@ -83,7 +83,20 @@ export default { description={ "Could not reach the portal to list the servers behind it, so there is nothing to " + "grant yet. Close this and try again; if it keeps happening, ask an administrator to " + - "check the portal configuration." + "check the portal configuration and any context-optimization setting." + } + /> + ; + } + + if (values.endpointKind === "empty") { + return
+
; @@ -126,7 +139,8 @@ export default { value: "choose", title: "Choose tools", description: - "Only the tools you tick. Anything else is refused, including tools added later.", + "Only the tools you tick, from up to 200 shown. Anything else is refused, " + + "including tools added later.", }, ]} onChange={next => setValues({ mode: next })} @@ -145,7 +159,7 @@ export default { (await ui.listToolOptions(values.server ?? undefined)) + loadOptions={async () => (await ui.listToolOptions(serverKey)) .map(option => ({ ...option, value: encodeURIComponent(option.value) }))} allSelected={mode === "all"} disabled={mode === "all"} diff --git a/packages/gatekeeper-mcp-portal/src/portal.ts b/packages/gatekeeper-mcp-portal/src/portal.ts index efae83602..ea7374b7a 100644 --- a/packages/gatekeeper-mcp-portal/src/portal.ts +++ b/packages/gatekeeper-mcp-portal/src/portal.ts @@ -23,36 +23,30 @@ import { type SupportedResource, type VendorDescription, } from "@gadgets/workshop-shared/gatekeeper"; -import type { ToolCatalog } from "@gadgets/mcp-shared/client"; -import { - classifyTool, - MAX_TOOLS_PER_SERVER, - type ServerTrust, -} from "@gadgets/mcp-shared/tools"; +import { isValidToolName } 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"; import { generateSessionTypes, sessionTypeName } from "@gadgets/mcp-shared/schema-to-ts"; import { McpAccountBase, type ConnectedServer, type ConnectOutcome } from "@gadgets/mcp-shared/account"; import { generateNonce } from "@gadgets/mcp-shared/connect-nonce"; -import { fetchTools, withClient, type ConnectionAccount } from "@gadgets/mcp-shared/connection"; +import { withClient, type ConnectionAccount } from "@gadgets/mcp-shared/connection"; import { McpSessionBase } from "@gadgets/mcp-shared/session"; import { McpFacetBase } from "@gadgets/mcp-shared/facet"; import { - groupToolsByServer, looksLikePortal, parsePortalServers, - reconcilePortalServers, PORTAL_LIST_SERVERS_TOOL, + reconcilePortalServers, type PortalServer, + type PortalServerListing, } from "@gadgets/mcp-shared/portal"; import { endpointOfResourceUrl, endpointTag, parseToolScope, - requireCompleteCatalogForToolSelection, sameEndpoint, - scopeAllows, validateToolScopeAgainstCatalog, type ToolScope, } from "@gadgets/mcp-shared/scope"; @@ -70,12 +64,15 @@ import { } from "@gadgets/mcp-shared/user"; import { portalAuthRequiresReconnect, + portalCatalogValidationMode, portalResource, portalServer, portalTokenFor, portalTrust, readPortalConfig, requirePortalServerScope, + isPortalToolGrantable, + toolGrantOptions, } from "./config.js"; import type { ConfiguratorUIOption } from "@gadgets/configurator-ui"; import { MCP_BASE_TYPES } from "@gadgets/mcp-shared/base-types"; @@ -85,6 +82,13 @@ import type { McpServerConfiguratorRpc } from "./configurator/server-configurato const VENDOR_ID = "mcp_portal"; +// How many tools one survey of the portal may cover. +// +// Entries retain names only, maximizing how much of a large portal the 96 KiB listing budget can +// cover. This count cap independently bounds work when names are short; either cut is reported as +// `truncated`. +const MAX_PORTAL_TOOL_INDEX = 1000; + const logger = createLogger({ component: "gatekeeper.mcp-portal", vendorId: VENDOR_ID, }); @@ -104,22 +108,31 @@ function getBaseUrl(env: Env): string { return stripTrailingSlashes(env.BASE_URL ?? "http://localhost:8787/gatekeeper/mcp-portal"); } -async function fetchPortalServers( +async function listPortalServers( + env: Env, + account: DurableObjectStub, + endpoint: string, +): Promise { + const result = await withClient(env, account, endpoint, + client => client.callTool(PORTAL_LIST_SERVERS_TOOL, {})); + if (result.isError) throw new Error("The portal could not list its upstream servers."); + return parsePortalServers(result); +} + +async function tryListPortalServers( env: Env, account: DurableObjectStub, endpoint: string, -): Promise { +): Promise { try { - const result = await withClient(env, account, endpoint, - client => client.callTool(PORTAL_LIST_SERVERS_TOOL, {})); - return parsePortalServers(result); + return await listPortalServers(env, account, endpoint); } catch (err) { logger.warn("could not list portal servers", { event: "portal.servers.list.failed", serverHost: hostOf(endpoint), error: err, }); - return []; + return null; } } @@ -318,10 +331,36 @@ export class GatekeeperUserImpl // here rather than only in the form that normally builds these URLs. const scope = parseToolScope(requested); requirePortalServerScope(scope); - const [catalog, portalServers] = await Promise.all([ - fetchTools(this.env, this.#account(), server.endpoint), - fetchPortalServers(this.env, this.#account(), server.endpoint), - ]); + 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, @@ -333,7 +372,7 @@ export class GatekeeperUserImpl endpoint: server.endpoint, serverId: server.serverId, serverName: config.name, - scopeServerName: upstream?.name ?? scope.serverId!, + scopeServerName: upstream?.name ?? scope.serverId, scope, }; return { class: this.ctx.exports.McpGatekeeperImpl({ props }), resource }; @@ -372,7 +411,8 @@ export class McpPortalVerifier class McpServerConfiguratorUI extends RpcTarget implements McpServerConfiguratorRpc { #env: Env; #account: DurableObjectStub; - #toolsPromise: Promise | undefined; + #serverPromise: Promise | undefined; + #portalServersPromise: Promise | undefined; constructor(env: Env, account: DurableObjectStub) { super(); @@ -381,77 +421,60 @@ class McpServerConfiguratorUI extends RpcTarget implements McpServerConfigurator } async getEndpoint(): Promise { - return (await this.#account.getServer()).endpoint; + return (await this.#server()).endpoint; + } + + #server(): Promise { + return this.#serverPromise ??= this.#account.getServer(); } - // One `tools/list` per configurator frame, shared by every question the form asks. - #tools(): Promise { - this.#toolsPromise ??= (async () => { - const server = await this.#account.getServer(); - return await fetchTools(this.#env, this.#account, server.endpoint); + #portalServers(): Promise { + 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 this.#toolsPromise; } - // The upstream servers behind a portal, derived from the tool list so there is no separate probe - // to keep in step. Empty for a plain MCP server and for a portal currently fronting nothing; the - // form treats both as having nothing to grant rather than telling them apart. + // Ask the portal for its server index without first loading every upstream tool. Empty for an + // endpoint that does not implement the portal contract or currently fronts nothing; either case + // leaves the form unsubmittable. async listServerOptions(): Promise { - const { tools, truncated } = await this.#tools(); - if (!looksLikePortal(tools, { truncated, cap: MAX_TOOLS_PER_SERVER })) return []; - - const servers = reconcilePortalServers( - await this.#fetchPortalServers(), tools, truncated); - const trust = portalTrust(this.#env); - const grouped = groupToolsByServer(tools); - - return servers.map(upstream => { - const owned = grouped.get(upstream.id) ?? []; - const reads = owned.filter( - tool => classifyTool(tool, trust).mode === "read").length; - return { - value: upstream.id, - title: upstream.name, - subtitle: truncated - ? `Catalog truncated \u00b7 ${owned.length} shown, ${reads} shown read-only` - : `${owned.length} tool${owned.length === 1 ? "" : "s"} \u00b7 ` + - `${reads} read-only, ${owned.length - reads} need approval`, - // A server can be configured but switched off for this session, making a grant onto it valid - // but presently empty, which the person choosing should see. - meta: upstream.enabled ? undefined : "disabled in portal", - }; + return (await this.#portalServers()).map(upstream => ({ + value: upstream.id, + title: upstream.name, + // A server can be configured but switched off for this session, making a grant onto it valid + // but presently empty, which the person choosing should see. + meta: upstream.enabled ? undefined : "disabled in portal", + })); + } + + // Tools the grant may cover within one portal upstream server. The survey is checked before the + // detailed catalog is fetched, and `toolGrantOptions` decides what each source says. + async listToolOptions(serverId: string): Promise { + if (!isValidToolName(serverId)) return []; + if (!(await this.#portalServers()).some(server => server.id === serverId)) return []; + const server = await this.#server(); + const tools = await withClient(this.#env, this.#account, server.endpoint, + client => client.listMatchingToolSummaries( + MAX_TOOLS_PER_SERVER, + tool => isPortalToolGrantable(tool.name, serverId), + )); + return toolGrantOptions({ + serverId, + tools, + trust: portalTrust(this.#env), }); } - - // Asks the portal which upstream servers it fronts. A tool call, but the gatekeeper's own while - // building a configuration form rather than a Gadget's, so it does not pass through the approval - // queue. Advisory only: membership comes from tool-name prefixes, so a failure degrades to unnamed - // groups rather than an error. - async #fetchPortalServers(): Promise { - const server = await this.#account.getServer(); - return fetchPortalServers(this.#env, this.#account, server.endpoint); - } - - // Tools the grant may cover, narrowed to one portal upstream server when `serverId` is given. - async listToolOptions(serverId?: string): Promise { - const trust = portalTrust(this.#env); - const { tools, truncated } = await this.#tools(); - requireCompleteCatalogForToolSelection(truncated); - const scope: ToolScope = serverId ? { serverId } : {}; - const isPortal = looksLikePortal(tools, { truncated, cap: MAX_TOOLS_PER_SERVER }); - - return tools - .filter(tool => scopeAllows(scope, tool.name, isPortal)) - .map(tool => ({ - value: tool.name, - // Within a chosen server the `{server_id}_` prefix is noise, so it is shown stripped while - // `value` keeps the wire name the grant is actually recorded with. - title: tool.title ?? (serverId ? tool.name.slice(serverId.length + 1) : tool.name), - subtitle: tool.description?.split(/\r?\n/)[0], - // Surfaced here so the person granting can see, per tool, whether calls will interrupt them. - meta: classifyTool(tool, trust).mode === "read" ? "read-only" : "needs approval", - })); - } } // --------------------------------------------------------------------------- @@ -467,9 +490,8 @@ type McpGatekeeperImplProps = { serverName: string; // Optional for facets minted before catalog-backed validation persisted the upstream display name. scopeServerName?: string; - // How much of the endpoint this binding may call. Empty means the whole endpoint, including tools - // it publishes later. - scope: ToolScope; + // How much of one upstream server this binding may call. + scope: ToolScope & { serverId: string }; }; export class McpGatekeeperImpl @@ -517,11 +539,9 @@ export class McpGatekeeperImpl const snippet = scope.tools ? `${scope.tools.length} named MCP tool${scope.tools.length === 1 ? "" : "s"} on ` + `${label} \u2014 ${counts}. Other tools are refused.` - : scope.serverId - ? `All tools of the ${this.ctx.props.scopeServerName ?? scope.serverId} server on ` + - `${this.ctx.props.serverName}; ${tools.length} definition${plural} shown here ` + - `(${counts}). Other servers on it are refused.` - : `All ${tools.length} MCP tool${plural} on ${label} \u2014 ${counts}.`; + : `All tools of the ${this.ctx.props.scopeServerName ?? scope.serverId} server on ` + + `${this.ctx.props.serverName}; ${tools.length} tool definition${plural} shown here ` + + `(${counts}). Use listTools({ search }) for others. Other servers are refused.`; return { url: this.resourceUrl, @@ -541,7 +561,7 @@ export class McpGatekeeperImpl // URL. #bindingId(): string { const { scope, serverId } = this.ctx.props; - return scope.serverId ? `${serverId}-${scope.serverId}` : serverId; + return `${serverId}-${scope.serverId}`; } /** diff --git a/packages/mcp-shared/__tests__/client-pagination.test.ts b/packages/mcp-shared/__tests__/client-pagination.test.ts index f541e760f..1d82bfe1f 100644 --- a/packages/mcp-shared/__tests__/client-pagination.test.ts +++ b/packages/mcp-shared/__tests__/client-pagination.test.ts @@ -234,7 +234,8 @@ describe("McpClient.listTools", () => { }); it("can index far more tools than a catalog holds by retaining only names", async () => { - // A catalog of these would exhaust the byte budget long before 400 tools. The index keeps the + // A catalog of these would exhaust the byte budget long before 400 tools. The index drops + // descriptions, schemas, and annotations. stubPages([{ tools: Array.from({ length: 400 }, (_, i) => ({ name: `server_tool_${i}`, description: "x".repeat(1000), @@ -242,11 +243,12 @@ describe("McpClient.listTools", () => { annotations: { readOnlyHint: i % 2 === 0 }, })) }]); const client = new McpClient("https://mcp.example.com/mcp", async () => null); - const { tools, truncated } = await client.listMatchingToolIndex(500, () => true); + const { tools, truncated } = await client.listToolIndex(500); expect(tools).toHaveLength(400); expect(truncated).toBe(false); expect(tools[399]).toEqual({ name: "server_tool_399" }); - // The point of the index: no description or schema survives to be stored or shown. + // The point of the index: no policy or display metadata survives to consume the survey budget. + expect(tools[0]).not.toHaveProperty("annotations"); expect(tools[0]).not.toHaveProperty("description"); expect(tools[0]).not.toHaveProperty("inputSchema"); }); @@ -391,11 +393,19 @@ describe("McpClient.listTools", () => { }); const client = new McpClient("https://mcp.example.com/mcp", async () => null); - await expect(client.listMatchingToolIndex(1000, () => true)) - .resolves.toEqual({ tools: [], truncated: true }); + await expect(client.listToolIndex(1000)).resolves.toEqual({ tools: [], truncated: true }); expect(calls).toBe(5); }); + it("returns a truncated index when pagination reaches the page limit", async () => { + const calls = stubPages([{ tools: [], nextCursor: "more" }]); + const client = new McpClient("https://mcp.example.com/mcp", async () => null); + + await expect(client.listToolIndex(1000)) + .resolves.toEqual({ tools: [], truncated: true }); + expect(calls()).toBe(50); + }); + it("returns collected catalog matches when later pages exceed the scan budget", async () => { const pages = [ { tools: [{ name: "jira_search" }], nextCursor: "1" }, diff --git a/packages/mcp-shared/__tests__/portal.test.ts b/packages/mcp-shared/__tests__/portal.test.ts index a8004d6d5..4670a1a27 100644 --- a/packages/mcp-shared/__tests__/portal.test.ts +++ b/packages/mcp-shared/__tests__/portal.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from "vitest"; import { - groupToolsByServer, isPortalNativeTool, looksLikePortal, parsePortalServers, @@ -9,7 +8,6 @@ import { } from "../src/portal.js"; import { scopeAllows } from "../src/scope.js"; import { MAX_TOOLS_PER_SERVER } from "../src/tools.js"; -import type { McpTool } from "../src/client.js"; // What a caller passes when it fetched with the ordinary catalog cap and the listing completed. const COMPLETE = { truncated: false, cap: MAX_TOOLS_PER_SERVER }; @@ -77,9 +75,6 @@ describe("server id recovery", () => { expect(toolBelongsToServer(name, "search"), name).toBe(false); expect(toolBelongsToServer(name, ""), name).toBe(false); } - expect([...groupToolsByServer([ - { name: "search" }, { name: "_leading" }, { name: "trailing_" }, - ] as McpTool[]).keys()]).toEqual([]); }); it("matches membership exactly, not by string prefix", () => { @@ -97,21 +92,6 @@ describe("server id recovery", () => { }); }); -describe("groupToolsByServer", () => { - it("groups by prefix and drops portal-native and unprefixed tools", () => { - const grouped = groupToolsByServer([ - { name: "portal_list_servers" }, - { name: "gh_list_issues" }, - { name: "gh_create_issue" }, - { name: "linear_list_comments" }, - { name: "unprefixed" }, - ]); - expect([...grouped.keys()]).toEqual(["gh", "linear"]); - expect(grouped.get("gh")!.map(tool => tool.name)) - .toEqual(["gh_list_issues", "gh_create_issue"]); - }); -}); - // Captured verbatim from a live Cloudflare MCP server portal. The earlier parser expected JSON and // returned nothing for this, which is the whole reason it was rewritten. const REAL_PORTAL_REPLY = { @@ -129,12 +109,13 @@ describe("parsePortalServers", () => { // The heading and the trailing instruction are not bullet lines, and the instruction names a // portal tool, so neither may become a server. Each display name pairs with the id that // prefixes that server's tools, which is what a grant actually matches on. - const servers = parsePortalServers(REAL_PORTAL_REPLY); - expect(servers).toEqual([ + const listing = parsePortalServers(REAL_PORTAL_REPLY); + expect(listing).toEqual({ complete: true, servers: [ { id: "test", name: "Cloudflare documentation", enabled: true }, { id: "linear", name: "Linear", enabled: true }, - ]); - expect(toolBelongsToServer("test_search_cloudflare_documentation", servers[0].id)).toBe(true); + ] }); + expect(toolBelongsToServer( + "test_search_cloudflare_documentation", listing.servers[0].id)).toBe(true); }); it("reports a disabled server, and shows one whose wording it cannot read", () => { @@ -145,21 +126,65 @@ describe("parsePortalServers", () => { text: "- Linear (linear): \u2713 enabled\n- GitHub (github): \u2717 disabled" + "\n- Jira (jira): active", }], - })).toEqual([ + })).toEqual({ complete: true, servers: [ { id: "linear", name: "Linear", enabled: true }, { id: "github", name: "GitHub", enabled: false }, { id: "jira", name: "Jira", enabled: true }, - ]); + ] }); }); - it("reads structuredContent, and returns nothing rather than throwing otherwise", () => { - // Not a failure: `reconcilePortalServers` then names each group by the id from its tool prefixes. + it("reads structuredContent and distinguishes complete empty from unrecognized", () => { expect(parsePortalServers({ structuredContent: [{ id: "gh", name: "GitHub", enabled: false }], - })).toEqual([{ id: "gh", name: "GitHub", enabled: false }]); - expect(parsePortalServers({})).toEqual([]); - expect(parsePortalServers({ content: [{ type: "text", text: "no servers today" }] })).toEqual([]); - expect(parsePortalServers({ structuredContent: "a string" })).toEqual([]); + })).toEqual({ + complete: true, + servers: [{ id: "gh", name: "GitHub", enabled: false }], + }); + expect(parsePortalServers({ structuredContent: [] })) + .toEqual({ complete: true, servers: [] }); + expect(parsePortalServers({})).toEqual({ complete: false, servers: [] }); + expect(parsePortalServers({ content: [{ type: "text", text: "no servers today" }] })) + .toEqual({ complete: false, servers: [] }); + expect(parsePortalServers({ structuredContent: "a string" })) + .toEqual({ complete: false, servers: [] }); + }); + + it("falls back to valid text when structured content has an unrecognized shape", () => { + expect(parsePortalServers({ + structuredContent: { servers: [{ id: "ignored" }] }, + content: [{ type: "text", text: "- GitHub (gh): enabled" }], + })).toEqual({ + complete: true, + servers: [{ id: "gh", name: "GitHub", enabled: true }], + }); + expect(parsePortalServers({ + structuredContent: [{ type: "text", text: "not a server record" }], + content: [{ type: "text", text: "- Linear (linear): enabled" }], + })).toEqual({ + complete: true, + servers: [{ id: "linear", name: "Linear", enabled: true }], + }); + }); + + it("marks a partially malformed server list incomplete", () => { + expect(parsePortalServers({ + content: [ + { type: "text", text: "Available MCP Servers:" }, + { type: "text", text: "- Linear (linear): enabled" }, + { type: "text", text: "- malformed" }, + ], + })).toEqual({ + complete: false, + servers: [{ id: "linear", name: "Linear", enabled: true }], + }); + }); + + it("marks unrecognized list formats incomplete so tool prefixes can recover them", () => { + for (const entry of ["+ GitHub (gh): enabled", "1. GitHub (gh): enabled"]) { + expect(parsePortalServers({ + content: [{ type: "text", text: `Available MCP Servers:\n${entry}` }], + })).toEqual({ complete: false, servers: [] }); + } }); }); @@ -169,6 +194,7 @@ describe("reconcilePortalServers", () => { { name: "portal_list_servers" }, { name: "gh_list_issues" }, { name: "linear_list_comments" }, + { name: "unprefixed" }, ]; it("names groups from the reported list", () => { @@ -197,18 +223,6 @@ describe("reconcilePortalServers", () => { expect(servers).toEqual([{ id: "gh", name: "GitHub", enabled: true }]); }); - it("retains reported servers when the tool catalog is truncated", () => { - const servers = reconcilePortalServers( - [{ id: "gh", name: "GitHub", enabled: true }, { id: "jira", name: "Jira", enabled: false }], - [{ name: "gh_list_issues" }], - true, - ); - expect(servers).toEqual([ - { id: "gh", name: "GitHub", enabled: true }, - { id: "jira", name: "Jira", enabled: false }, - ]); - }); - it("sorts by display name", () => { expect(reconcilePortalServers( [{ id: "gh", name: "Zulip", enabled: true }, { id: "linear", name: "Asana", enabled: true }], diff --git a/packages/mcp-shared/__tests__/scope.test.ts b/packages/mcp-shared/__tests__/scope.test.ts index 9680ea642..f95b5b3b5 100644 --- a/packages/mcp-shared/__tests__/scope.test.ts +++ b/packages/mcp-shared/__tests__/scope.test.ts @@ -219,6 +219,15 @@ describe("validateToolScopeAgainstCatalog", () => { }); }); + it("uses current tool evidence to validate a pinned-empty grant", () => { + expect(validateToolScopeAgainstCatalog( + { serverId: "gh", tools: [] }, catalog, [])).toEqual({ + id: "gh", + name: "gh", + enabled: true, + }); + }); + it("rejects named tools outside or absent from the selected portal server", () => { expect(() => validateToolScopeAgainstCatalog( { serverId: "gh", tools: ["linear_list_comments"] }, catalog)) diff --git a/packages/mcp-shared/src/client.ts b/packages/mcp-shared/src/client.ts index b3bfad3d9..3d5dcfe79 100644 --- a/packages/mcp-shared/src/client.ts +++ b/packages/mcp-shared/src/client.ts @@ -45,13 +45,13 @@ export type ToolCatalog = { truncated: boolean; }; -/** One tool identity retained to validate an exact name against a large endpoint. */ +/** One tool identity retained to validate names or recover portal membership. */ export type IndexedTool = { /** Exact wire name advertised by the endpoint. */ name: string; }; -/** A bounded survey of endpoint tool identities without schemas or descriptions. */ +/** A bounded survey of endpoint tool identities without schemas, descriptions, or policy claims. */ export type ToolIndex = { /** Retained tool identities. */ tools: IndexedTool[]; @@ -318,8 +318,8 @@ function clampText(value: unknown, max: number): string | undefined { // large to render is dropped rather than clipped, so the generated method degrades to // `Record`. // Keeps only the hints this gatekeeper understands, so a server cannot attach unbounded text to a -// tool under `annotations` and have it stored, indexed, or rendered. Each retained field is a -// boolean or absent, which is what makes an index entry's size predictable. +// tool under `annotations` and have it stored or rendered. Each retained field is a boolean or +// absent. function clampAnnotations( annotations: McpToolAnnotations | undefined, ): McpToolAnnotations | undefined { @@ -570,6 +570,14 @@ export class McpClient { return this.#list(maxTools, include, clampToolDefinition); } + /** + * Surveys names without retaining descriptions, schemas, or policy annotations. Resolve a full + * definition with `findTool` before use. + */ + async listToolIndex(maxTools: number): Promise { + return this.#list(maxTools, undefined, indexTool); + } + /** Collects at most `maxTools` matching index entries without scanning later pages. */ async listMatchingToolIndex(maxTools: number, include: McpToolFilter): Promise { return this.#list(maxTools, include, indexTool, true); diff --git a/packages/mcp-shared/src/portal.ts b/packages/mcp-shared/src/portal.ts index 29c78bc02..74679b859 100644 --- a/packages/mcp-shared/src/portal.ts +++ b/packages/mcp-shared/src/portal.ts @@ -78,22 +78,6 @@ export function toolBelongsToServer(toolName: string, serverId: string): boolean return serverIdOfTool(toolName) === serverId; } -/** Groups a portal's tools by upstream server id, dropping the portal's own tools. */ -export function groupToolsByServer>( - tools: T[], -): Map { - const groups = new Map(); - for (const tool of tools) { - if (isPortalNativeTool(tool.name)) continue; - const serverId = serverIdOfTool(tool.name); - if (!serverId) continue; - const existing = groups.get(serverId); - if (existing) existing.push(tool); - else groups.set(serverId, [tool]); - } - return groups; -} - // `portal_list_servers` answers in prose, not JSON. A Cloudflare portal replies with bullet lines of // the form `- {display name} ({server id}): {status}`: // @@ -102,8 +86,9 @@ export function groupToolsByServer>( // - Cloudflare documentation (test): \u2713 enabled // - Linear (linear): \u2713 enabled // -// Display metadata only, so an unrecognized line is skipped rather than raised; -// `reconcilePortalServers` recovers any server the prose failed to describe. +// Display metadata only, so an unrecognized line is skipped rather than raised, but the enclosing +// listing is marked incomplete so a truncated tool index cannot mistake the partial result for all +// servers. A complete index can still recover the skipped id from tool-name prefixes. function parseServerLine(line: string): PortalServer | null { const bulletLine = line.trimStart(); if (bulletLine[0] !== "-" && bulletLine[0] !== "*" && bulletLine[0] !== "\u2022") return null; @@ -148,72 +133,110 @@ function parseServerLine(line: string): PortalServer | null { return null; } -function parseServerLines(text: string): PortalServer[] { +function parseServerLines(text: string): { + servers: PortalServer[]; + complete: boolean; + recognized: boolean; +} { const servers: PortalServer[] = []; const seen = new Set(); + let recognized = /available mcp servers\s*:/i.test(text); + let complete = true; for (const line of text.split(/\r?\n/)) { + const trimmed = line.trimStart(); + const first = trimmed[0]; + const entryLike = first === "-" || first === "*" || first === "+" || first === "\u2022" || + /^\d+[.)]\s/.test(trimmed); const server = parseServerLine(line); - if (!server || seen.has(server.id)) continue; + if (!server) { + if (entryLike) complete = false; + continue; + } + recognized = true; + if (seen.has(server.id)) continue; seen.add(server.id); servers.push(server); } - return servers; + return { servers, complete: recognized && complete, recognized }; } // A `structuredContent` payload, if a future portal version supplies one. Read literally as an array // of `{ id, name, enabled }`, with no guessing at alternative spellings. -function parseStructured(value: unknown): PortalServer[] { - if (!Array.isArray(value)) return []; +function parseStructured(value: unknown): { servers: PortalServer[]; complete: boolean } | null { + if (value === undefined) return null; + if (!Array.isArray(value)) return null; const servers: PortalServer[] = []; + let complete = true; for (const entry of value) { - if (typeof entry !== "object" || entry === null) continue; + if (typeof entry !== "object" || entry === null) { + complete = false; + continue; + } const record = entry as Record; const id = typeof record.id === "string" ? record.id.trim() : ""; - if (!id) continue; + if (!id) { + complete = false; + continue; + } servers.push({ id, name: typeof record.name === "string" && record.name.trim() ? record.name.trim() : id, enabled: record.enabled !== false, }); } - return servers; + return { servers, complete }; } +/** A parsed portal server list and whether every advertised entry was understood. */ +export type PortalServerListing = { + /** Valid upstream server entries recovered from the response. */ + servers: PortalServer[]; + /** Whether the complete response was recognized without dropping malformed entries. */ + complete: boolean; +}; + /** - * Parses the upstream server list out of a `portal_list_servers` result. Empty when nothing - * parseable is present; callers fall back to the ids recovered from tool-name prefixes. + * Parses the upstream server list out of a `portal_list_servers` result. + * + * `complete` is false when the response was absent, unrecognized, or only partly understood; a + * truncated tool index cannot safely use such a response as its authoritative list. * Typed by what it reads rather than as `McpToolCallResult`, which a result satisfies: this is an * untrusted reply and every field is re-checked here, so the loose type is the honest one. */ export function parsePortalServers( result: { structuredContent?: unknown; content?: unknown }, -): PortalServer[] { +): PortalServerListing { const structured = parseStructured(result.structuredContent); - if (structured.length > 0) return structured; - - for (const block of Array.isArray(result.content) ? result.content : []) { - const { type, text } = (block ?? {}) as { type?: unknown; text?: unknown }; - if (type !== "text" || typeof text !== "string") continue; - const servers = parseServerLines(text); - if (servers.length > 0) return servers; + if (structured?.complete) return structured; + + const combinedText = (Array.isArray(result.content) ? result.content : []) + .flatMap(block => { + const { type, text: blockText } = (block ?? {}) as { type?: unknown; text?: unknown }; + return type === "text" && typeof blockText === "string" ? [blockText] : []; + }) + .join("\n"); + if (combinedText) { + const listing = parseServerLines(combinedText); + if (listing.recognized) return { servers: listing.servers, complete: listing.complete }; } - return []; + return structured ?? { servers: [], complete: false }; } /** - * Merges the portal's reported servers with the ids present in the tool list. With a complete - * catalog, tool names are the authority and reported empty servers are dropped. With a truncated - * catalog, absence is not evidence, so reported servers are retained for server-wide grants. + * Merges the portal's reported servers with the ids present in a complete tool index. Tool names + * are the authority, so reported empty servers are dropped. */ export function reconcilePortalServers( - reported: PortalServer[], tools: Pick[], truncated = false, + reported: PortalServer[], tools: Pick[], ): PortalServer[] { - const grouped = groupToolsByServer(tools); const byId = new Map(reported.map(server => [server.id, server])); + const ids = new Set(); + for (const tool of tools) { + if (isPortalNativeTool(tool.name)) continue; + const id = serverIdOfTool(tool.name); + if (id) ids.add(id); + } const servers: PortalServer[] = []; - const ids = truncated - ? new Set([...reported.map(server => server.id), ...grouped.keys()]) - : grouped.keys(); for (const id of ids) { servers.push(byId.get(id) ?? { id, name: id, enabled: true }); }