diff --git a/apps/web/src/mcp-server-name.test.ts b/apps/web/src/mcp-server-name.test.ts new file mode 100644 index 000000000..c734b57d4 --- /dev/null +++ b/apps/web/src/mcp-server-name.test.ts @@ -0,0 +1,190 @@ +import { describe, expect, test } from "bun:test"; + +import { + dedupeMcpHandle, + dedupeMcpName, + displayNameFromUrl, + handleFromUrl, + redirectUrlName, + suggestMcpServerName, +} from "./mcp-server-name"; + +describe("suggestMcpServerName", () => { + test("prefers the server's title over its name", () => { + expect( + suggestMcpServerName({ + url: "https://mcp.linear.app/mcp", + serverInfo: { name: "linear", title: "Linear (Corp)", version: "1.0" }, + }), + ).toEqual({ name: "Linear (Corp)", source: "server" }); + }); + + test("falls back to the server's name when there is no title", () => { + expect( + suggestMcpServerName({ + url: "https://mcp.linear.app/mcp", + serverInfo: { name: "linear-proxy", version: "1.0" }, + }), + ).toEqual({ name: "linear-proxy", source: "server" }); + }); + + test("trims the server's name and ignores a blank one", () => { + expect( + suggestMcpServerName({ + url: "https://mcp.exa.ai/mcp", + serverInfo: { name: " ", version: "1.0" }, + }), + ).toEqual({ name: "Exa", source: "url" }); + expect( + suggestMcpServerName({ + url: "https://mcp.exa.ai/mcp", + serverInfo: { name: " Exa Search " }, + }), + ).toEqual({ name: "Exa Search", source: "server" }); + }); + + test("ignores a non-string or missing server name", () => { + expect( + suggestMcpServerName({ + url: "https://mcp.exa.ai/mcp", + serverInfo: { name: 42, version: "1.0" }, + }), + ).toEqual({ name: "Exa", source: "url" }); + expect(suggestMcpServerName({ url: "https://mcp.exa.ai/mcp", serverInfo: {} })).toEqual({ + name: "Exa", + source: "url", + }); + expect(suggestMcpServerName({ url: "https://mcp.exa.ai/mcp" })).toEqual({ + name: "Exa", + source: "url", + }); + }); + + test("caps an overlong server name", () => { + const { name } = suggestMcpServerName({ + url: "https://mcp.example.com/mcp", + serverInfo: { name: ` ${"n".repeat(200)} ` }, + }); + expect(name.length).toBeLessThanOrEqual(60); + }); + + test("reads a display name off the URL host", () => { + expect(suggestMcpServerName({ url: "https://mcp.linear.app/mcp" })).toEqual({ + name: "Linear", + source: "url", + }); + expect(suggestMcpServerName({ url: " https://www.mcp.exa.ai:443/mcp " })).toEqual({ + name: "Exa", + source: "url", + }); + expect(suggestMcpServerName({ url: "https://my-cool-server.example.com/mcp" })).toEqual({ + name: "My Cool Server", + source: "url", + }); + }); + + test("names bare hosts and IPs whole", () => { + expect(suggestMcpServerName({ url: "http://localhost:3000/mcp" })).toEqual({ + name: "Localhost", + source: "url", + }); + expect(suggestMcpServerName({ url: "http://127.0.0.1:3000/mcp" })).toEqual({ + name: "127.0.0.1", + source: "url", + }); + }); + + test("reads an internationalized host back as its real name, not mojibake", () => { + expect(suggestMcpServerName({ url: "http://münchen.example.com/mcp" })).toEqual({ + name: "München", + source: "url", + }); + }); + + test("falls back when the URL is empty or invalid", () => { + expect(suggestMcpServerName({ url: "" })).toEqual({ name: "MCP server", source: "fallback" }); + expect(suggestMcpServerName({ url: " " })).toEqual({ + name: "MCP server", + source: "fallback", + }); + expect(suggestMcpServerName({ url: "not a url" })).toEqual({ + name: "MCP server", + source: "fallback", + }); + }); + + test("dedupes against connected servers case-insensitively", () => { + expect( + suggestMcpServerName({ + url: "https://mcp.linear.app/mcp", + existingNames: ["Linear"], + }), + ).toEqual({ name: "Linear 2", source: "url" }); + expect( + suggestMcpServerName({ + url: "https://mcp.linear.app/mcp", + serverInfo: { name: "Linear" }, + existingNames: ["linear", "Linear 2"], + }), + ).toEqual({ name: "Linear 3", source: "server" }); + }); +}); + +describe("displayNameFromUrl", () => { + test("returns null for empty and invalid input", () => { + expect(displayNameFromUrl("")).toBeNull(); + expect(displayNameFromUrl("::not a url::")).toBeNull(); + }); +}); + +describe("handleFromUrl", () => { + test("slugs the host's distinctive label", () => { + expect(handleFromUrl("https://mcp.linear.app/mcp")).toBe("linear"); + expect(handleFromUrl("https://www.mcp.exa.ai/mcp")).toBe("exa"); + expect(handleFromUrl("https://my-cool-server.example.com/mcp")).toBe("my-cool-server"); + }); + + test("trims and returns null for empty or invalid input", () => { + expect(handleFromUrl(" https://mcp.linear.app/mcp ")).toBe("linear"); + expect(handleFromUrl("")).toBeNull(); + expect(handleFromUrl("not a url")).toBeNull(); + }); + + test("gives an IPv6 loopback a meaningful handle instead of a degenerate one", () => { + expect(handleFromUrl("http://[::1]:3000/mcp")).toBe("localhost"); + expect(handleFromUrl("http://[::1]:3000/mcp")).not.toBe("1"); + }); + + test("keeps an internationalized handle slug ascii-safe", () => { + expect(handleFromUrl("http://münchen.example.com/mcp")).toBe("xn-mnchen-3ya"); + }); +}); + +describe("redirectUrlName", () => { + test("extracts a display name from a pasted URL and dedupes it", () => { + expect(redirectUrlName("https://mcp.linear.app/mcp", [])).toBe("Linear"); + expect(redirectUrlName(" https://mcp.linear.app/mcp ", ["Linear"])).toBe("Linear 2"); + }); + + test("leaves plain names, blanks, and non-URLs alone", () => { + expect(redirectUrlName("Linear", [])).toBeNull(); + expect(redirectUrlName("", [])).toBeNull(); + expect(redirectUrlName("not a url", [])).toBeNull(); + }); +}); + +describe("dedupeMcpName", () => { + test("leaves a unique name alone and numbers collisions", () => { + expect(dedupeMcpName("Exa", ["Linear"])).toBe("Exa"); + expect(dedupeMcpName("Exa", ["exa"])).toBe("Exa 2"); + expect(dedupeMcpName("Exa", ["Exa", "Exa 2"])).toBe("Exa 3"); + }); +}); + +describe("dedupeMcpHandle", () => { + test("leaves a unique handle alone and dashes collisions", () => { + expect(dedupeMcpHandle("mcp", ["linear"])).toBe("mcp"); + expect(dedupeMcpHandle("mcp", ["MCP"])).toBe("mcp-2"); + expect(dedupeMcpHandle("mcp", ["mcp", "mcp-2"])).toBe("mcp-3"); + }); +}); diff --git a/apps/web/src/mcp-server-name.ts b/apps/web/src/mcp-server-name.ts new file mode 100644 index 000000000..1ebca0964 --- /dev/null +++ b/apps/web/src/mcp-server-name.ts @@ -0,0 +1,226 @@ +// Pre-fills the MCP "add a server by URL" name field: the server's own +// reported name first, the pasted URL's host when the server says nothing, +// and a plain fallback when the URL does not parse either. Every suggestion +// is trimmed, capped, and deduped against the workspace catalog, and stays +// editable in the form — a suggestion never overwrites what was typed. +import { type } from "arktype"; + +/** What `serverInfo` may carry a display name under. Discovery returns it + * untyped (`Record`), so anything else is ignored rather + * than cast. `title` wins: it is the human-readable label, `name` the id. */ +const ServerInfoShape = type({ + "name?": "string", + "title?": "string", +}); + +export type McpNameSource = "server" | "url" | "fallback"; + +const FALLBACK_NAME = "MCP server"; +const MAX_NAME_LENGTH = 60; + +function cleanCandidate(value: string): string | null { + const trimmed = value.trim().slice(0, MAX_NAME_LENGTH).trim(); + return trimmed === "" ? null : trimmed; +} + +/** Host labels that name the protocol, not the server: skipped when reading a + * display name or slug off a URL, so `mcp.linear.app` reads as `linear`. */ +const GENERIC_HOST_LABELS = new Set([ + "api", + "app", + "apps", + "gateway", + "mcp", + "proxy", + "server", + "servers", +]); + +/** RFC 3492 Punycode decoding, kept inline because no punycode package ships + * in the browser bundle this module runs in. A URL parser already handed us an + * ASCII `xn--…` label, so a decoding failure (unreachable) falls back to the + * raw label rather than throwing — the label a caller sees is never worse than + * the codepoint garble it replaces. */ +function decodePunycodeLabel(label: string): string { + if (!label.startsWith("xn--")) return label; + const base = 36; + const tmin = 1; + const tmax = 26; + const skew = 38; + const damp = 700; + const initialBias = 72; + const initialN = 128; + const digitFor = (code: number): number => { + if (code >= 48 && code <= 57) return code - 22; // 0-9 + if (code >= 65 && code <= 90) return code - 65; // A-Z + if (code >= 97 && code <= 122) return code - 97; // a-z + throw new Error("not a punycode digit"); + }; + const adapt = (delta: number, numPoints: number, first: boolean): number => { + let d = first ? Math.floor(delta / damp) : delta >> 1; + d += Math.floor(d / numPoints); + let k = 0; + while (d > Math.floor(((base - tmin) * tmax) / 2)) { + d = Math.floor(d / (base - tmin)); + k += base; + } + return k + Math.floor(((base - tmin + 1) * d) / (d + skew)); + }; + try { + const payload = label.slice(4); + const hyphen = payload.lastIndexOf("-"); + let output = hyphen > 0 ? payload.slice(0, hyphen) : ""; + const tail = hyphen > 0 ? payload.slice(hyphen + 1) : payload; + let n = initialN; + let i = 0; + let bias = initialBias; + let pos = 0; + while (pos < tail.length) { + const oldi = i; + let w = 1; + for (let k = base; ; k += base) { + const digit = digitFor(tail.charCodeAt(pos)); + pos += 1; + i += digit * w; + const threshold = k <= bias ? tmin : k >= bias + tmax ? tmax : k - bias; + if (digit < threshold) break; + w *= base - threshold; + } + const outputLength = output.length; + bias = adapt(i - oldi, outputLength + 1, oldi === 0); + n += Math.floor(i / (outputLength + 1)); + i %= outputLength + 1; + output = output.slice(0, i) + String.fromCodePoint(n) + output.slice(i); + i += 1; + } + return output; + } catch { + return label; + } +} + +/** The host label that names the server: the first label, unless it is a + * generic prefix with a more distinctive label behind it. Bare hosts and + * literal IPs come back whole. Null when there is no host at all. */ +function distinctiveHostLabel(hostname: string): string | null { + const bare = hostname.replace(/^www\./i, ""); + if (bare === "") return null; + // `new URL().hostname` reports an IPv6 literal bracketed (`[::1]`); the + // loopback one is a local server a name can mean, so it reads as + // "localhost" instead of collapsing to a degenerate slug like "1". + if (bare === "[::1]" || bare === "::1") return "localhost"; + if (!bare.includes(".") || /^[\d.]+$/.test(bare) || bare.includes(":")) return bare; + const labels = bare.split("."); + const first = labels[0] ?? ""; + if (GENERIC_HOST_LABELS.has(first.toLocaleLowerCase()) && labels.length > 1) { + return labels[1] ?? first; + } + return first; +} + +function humanizeHostLabel(label: string): string | null { + // A URL parser hands international host names back as Punycode (`xn--…`), + // which would otherwise read as codepoint garble once split into words; + // decode before humanizing so the label is meaningful. + const readable = decodePunycodeLabel(label); + const words = readable + .toLocaleLowerCase() + .split(/[^\p{L}\p{N}]+/u) + .filter((word) => word !== ""); + if (words.length === 0) return null; + return words.map((word) => word.slice(0, 1).toLocaleUpperCase() + word.slice(1)).join(" "); +} + +/** A display name read off a URL's host: `mcp.linear.app` reads as `Linear`. + * Null when the URL is empty or does not parse — the caller falls back. */ +export function displayNameFromUrl(url: string): string | null { + const trimmed = url.trim(); + if (trimmed === "") return null; + let hostname: string; + try { + hostname = new URL(trimmed).hostname; + } catch { + return null; + } + const label = distinctiveHostLabel(hostname); + if (label === null) return null; + // Literal IPs name themselves whole; anything else reads as words. + if (/^[\d.]+$/.test(label) || label.includes(":")) return label; + return humanizeHostLabel(label); +} + +/** The storage slug a pasted URL's server is kept under: the host's + * distinctive label, lowercased and dashed. Null when the URL is empty or + * unparsable. */ +export function handleFromUrl(url: string): string | null { + const trimmed = url.trim(); + if (trimmed === "") return null; + try { + const label = distinctiveHostLabel(new URL(trimmed).hostname); + if (label === null) return null; + const slug = label + .toLocaleLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, ""); + return slug === "" ? null : slug; + } catch { + return null; + } +} + +/** A name no connected server already carries (case-insensitive): appends + * ` 2`, ` 3`, … until it is unique. */ +export function dedupeMcpName(base: string, existingNames: readonly string[]): string { + const taken = new Set(existingNames.map((name) => name.trim().toLocaleLowerCase())); + if (!taken.has(base.toLocaleLowerCase())) return base; + let next = 2; + while (taken.has(`${base} ${String(next)}`.toLocaleLowerCase())) next += 1; + return `${base} ${String(next)}`; +} + +/** The same dedupe for storage slugs, joined with a dash. */ +export function dedupeMcpHandle(base: string, existingHandles: readonly string[]): string { + const taken = new Set(existingHandles.map((handle) => handle.trim().toLocaleLowerCase())); + if (!taken.has(base.toLocaleLowerCase())) return base; + let next = 2; + while (taken.has(`${base}-${String(next)}`.toLocaleLowerCase())) next += 1; + return `${base}-${String(next)}`; +} + +function serverReportedName(serverInfo: unknown): string | null { + if (serverInfo === null || typeof serverInfo !== "object") return null; + const parsed = ServerInfoShape(serverInfo); + if (parsed instanceof type.errors) return null; + if (parsed.title !== undefined) { + const title = cleanCandidate(parsed.title); + if (title !== null) return title; + } + if (parsed.name !== undefined) return cleanCandidate(parsed.name); + return null; +} + +/** A typed name that is really a pasted URL, read back as a display name: + * the name field never saves a URL silently — the form says where the paste + * belongs and saves the extracted suggestion instead. Null when the typed + * text is not a URL at all. */ +export function redirectUrlName(typed: string, existingNames: readonly string[]): string | null { + const extracted = displayNameFromUrl(typed); + if (extracted === null) return null; + return dedupeMcpName(extracted, existingNames); +} + +/** The name the form should show for a pasted URL and, once looked up, the + * server's self-reported `serverInfo`. Untrusted on both sides: each is + * parsed, trimmed, and capped before it is ever displayed or stored. */ +export function suggestMcpServerName(args: { + readonly url: string; + readonly serverInfo?: unknown; + readonly existingNames?: readonly string[]; +}): { readonly name: string; readonly source: McpNameSource } { + const existing = args.existingNames ?? []; + const fromServer = serverReportedName(args.serverInfo); + if (fromServer !== null) return { name: dedupeMcpName(fromServer, existing), source: "server" }; + const fromUrl = displayNameFromUrl(args.url); + if (fromUrl !== null) return { name: dedupeMcpName(fromUrl, existing), source: "url" }; + return { name: dedupeMcpName(FALLBACK_NAME, existing), source: "fallback" }; +} diff --git a/apps/web/src/mcp-servers.test.ts b/apps/web/src/mcp-servers.test.ts new file mode 100644 index 000000000..a0cacc07c --- /dev/null +++ b/apps/web/src/mcp-servers.test.ts @@ -0,0 +1,92 @@ +// `probeMcpServer` hands the Tools page the server's self-report, not the +// hub discovery envelope it arrives in: POST /mcp/discover nests the report +// beside the negotiated protocol version, and the name suggestion reads the +// report's own `title`/`name`. +import { describe, expect, test } from "bun:test"; + +import { McpServerError, probeMcpServer } from "./mcp-servers"; +import { suggestMcpServerName } from "./mcp-server-name"; + +const TOOL = { + name: "acme.list_things", + description: "List things", + inputSchema: { type: "object", properties: {} }, +}; + +function discoverFetch(serverInfo: unknown): typeof fetch { + const stub = async (): Promise => + new Response(JSON.stringify({ data: { serverInfo, tools: [TOOL] } })); + // A bare stub never carries `fetch`'s static `preconnect` member. + return Object.assign(stub, { preconnect: () => {} }) as typeof fetch; +} + +describe("probeMcpServer", () => { + test("unwraps the hub envelope to the server's self-reported serverInfo", async () => { + const { serverInfo } = await probeMcpServer( + { tenantId: "tnt_workspace", url: "https://mcp.acme.test/mcp" }, + discoverFetch({ + protocolVersion: "2025-03-26", + serverInfo: { name: "acme", version: "1.0.0" }, + }), + ); + expect(serverInfo).toEqual({ name: "acme", version: "1.0.0" }); + expect(suggestMcpServerName({ url: "https://mcp.acme.test/mcp", serverInfo })).toEqual({ + name: "acme", + source: "server", + }); + }); + + test("passes a report-shaped answer through untouched", async () => { + const { serverInfo } = await probeMcpServer( + { tenantId: "tnt_workspace", url: "https://mcp.acme.test/mcp" }, + discoverFetch({ name: "acme", version: "1.0.0" }), + ); + expect(serverInfo).toEqual({ name: "acme", version: "1.0.0" }); + }); + + test("keeps an envelope with no inner report whole for the URL fallback", async () => { + const { serverInfo } = await probeMcpServer( + { tenantId: "tnt_workspace", url: "https://mcp.acme.test/mcp" }, + discoverFetch({ protocolVersion: "2025-03-26" }), + ); + expect(serverInfo).toEqual({ protocolVersion: "2025-03-26" }); + expect(suggestMcpServerName({ url: "https://mcp.acme.test/mcp", serverInfo })).toEqual({ + name: "Acme", + source: "url", + }); + }); + + test("marks a server that refused discovery as needing a bearer token", async () => { + const fetchImpl = async (): Promise => + new Response( + JSON.stringify({ + error: + "the MCP server at http://mcp.acme.test could not be discovered: the server refused the request", + }), + { status: 422 }, + ); + const error = await probeMcpServer( + { tenantId: "tnt_workspace", url: "https://mcp.acme.test/mcp" }, + Object.assign(fetchImpl, { preconnect: () => {} }) as typeof fetch, + ).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(McpServerError); + expect((error as McpServerError).needsBearer).toBe(true); + }); + + test("leaves an unreachable server unmarked as needing a bearer token", async () => { + const fetchImpl = async (): Promise => + new Response( + JSON.stringify({ + error: + "the MCP server at http://mcp.acme.test could not be discovered: the handshake failed", + }), + { status: 422 }, + ); + const error = await probeMcpServer( + { tenantId: "tnt_workspace", url: "https://mcp.acme.test/mcp" }, + Object.assign(fetchImpl, { preconnect: () => {} }) as typeof fetch, + ).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(McpServerError); + expect((error as McpServerError).needsBearer).toBe(false); + }); +}); diff --git a/apps/web/src/mcp-servers.ts b/apps/web/src/mcp-servers.ts index 5983617ff..ce9353c09 100644 --- a/apps/web/src/mcp-servers.ts +++ b/apps/web/src/mcp-servers.ts @@ -21,7 +21,18 @@ import { type } from "arktype"; export { MCP_SERVER_CATALOG }; export type { McpCatalogEntry } from "@corbits/worker/workflow-ids"; -export class McpServerError extends Error {} +export class McpServerError extends Error { + /** True when the hub said the server refused the request (typically a + * token-protected server answering discovery with a 401/403). Distinct from + * a server that simply cannot be reached, so the Tools page can tell "add a + * token" from "check the URL". */ + readonly needsBearer: boolean; + constructor(message: string, needsBearer = false) { + super(message); + this.name = "McpServerError"; + this.needsBearer = needsBearer; + } +} const ProviderShape = type({ id: "string", name: "string", plugin: "string" }); const ProvidersPage = type({ data: ProviderShape.array() }); @@ -237,12 +248,13 @@ async function deleteCredential( }); } -/** Read a server's catalog through the hub, which is the only side that may - * hold the bearer. `credentialId` names the stored secret to send. */ -export async function discoverMcpCatalog( +/** One POST to the hub's discovery route, shared by the add flow (which needs + * only the tools) and the name lookup (which needs the self-reported + * `serverInfo` too). Stores nothing. */ +async function postDiscovery( args: { readonly tenantId: string; readonly url: string; readonly credentialId?: string }, - fetchImpl: typeof fetch = fetch, -): Promise { + fetchImpl: typeof fetch, +): Promise<{ readonly serverInfo: Record; readonly tools: readonly McpTool[] }> { const response = await fetchImpl(tenantPath(args.tenantId, "/mcp/discover"), { method: "POST", headers: { "content-type": "application/json" }, @@ -254,13 +266,54 @@ export async function discoverMcpCatalog( if (!response.ok) { const body: unknown = await response.json().catch(() => undefined); const envelope = type({ error: "string" })(body); - throw new McpServerError( + const message = envelope instanceof type.errors ? `this server could not be reached (HTTP ${String(response.status)})` - : envelope.error, - ); + : envelope.error; + // The hub answers every discovery failure with a single 422 whose copy is + // the only sign of why: "refused the request" when a token-protected server + // answered with a 4xx, "the handshake failed" when it never answered. Read + // that sign here so the Tools page can tell "add a token" from "check the + // URL". (Mirrors the pinned @corbits/mcp wording.) + throw new McpServerError(message, message.includes("refused the request")); + } + return (await readJson(response, DiscoveryShape, "the MCP server's catalog")).data; +} + +/** Read a server's catalog through the hub, which is the only side that may + * hold the bearer. `credentialId` names the stored secret to send. */ +export async function discoverMcpCatalog( + args: { readonly tenantId: string; readonly url: string; readonly credentialId?: string }, + fetchImpl: typeof fetch = fetch, +): Promise { + return (await postDiscovery(args, fetchImpl)).tools; +} + +/** The hub nests the server's self-report beside the negotiated protocol + * version (`{ protocolVersion, serverInfo? }`); the name lookup wants the + * inner report itself. A report-shaped object passes through untouched, and + * an envelope with no inner report stays whole — callers fall back to the + * URL suggestion either way. */ +function unwrapDiscoveredName(discovered: Record): unknown { + if (typeof discovered["name"] === "string" || typeof discovered["title"] === "string") { + return discovered; } - return (await readJson(response, DiscoveryShape, "the MCP server's catalog")).data.tools; + const inner = discovered["serverInfo"]; + return inner !== null && typeof inner === "object" ? inner : discovered; +} + +/** Asks a server what it calls itself without storing anything: the Tools + * page pre-fills the name field from the returned `serverInfo` before the + * server is added. `serverInfo` stays unknown — callers parse what they + * display. Keyless servers answer directly; token-protected ones cannot + * answer until they hold a credential, so callers keep the URL suggestion + * when this rejects. */ +export async function probeMcpServer( + args: { readonly tenantId: string; readonly url: string }, + fetchImpl: typeof fetch = fetch, +): Promise<{ readonly serverInfo: unknown; readonly tools: readonly McpTool[] }> { + const { serverInfo, tools } = await postDiscovery(args, fetchImpl); + return { serverInfo: unwrapDiscoveredName(serverInfo), tools }; } export type AddMcpServerInput = { diff --git a/apps/web/src/pages/tools-page.tsx b/apps/web/src/pages/tools-page.tsx index 270119dd5..37fd013c0 100644 --- a/apps/web/src/pages/tools-page.tsx +++ b/apps/web/src/pages/tools-page.tsx @@ -26,8 +26,21 @@ import { QueryView } from "@/lib/api-query"; import { CircleNotch, MagnifyingGlass, Plugs, type Icon } from "@/lib/icons"; import { describeApiError } from "@/lib/api-query"; -import { MCP_SERVER_CATALOG, type McpCatalogEntry } from "../mcp-servers"; +import { + MCP_SERVER_CATALOG, + McpServerError, + probeMcpServer, + resolveWorkspaceTenantId, +} from "../mcp-servers"; +import type { McpCatalogEntry } from "../mcp-servers"; import { toolCountLabel } from "../tools/tool-count"; +import { + dedupeMcpHandle, + dedupeMcpName, + handleFromUrl, + redirectUrlName, + suggestMcpServerName, +} from "../mcp-server-name"; import { describeRedeployResult, useAddMcpServer, @@ -45,16 +58,12 @@ import { StageTopBar } from "../shell/stage-top-bar"; import { ConfirmButton } from "../components/confirm-button"; import { ListCard, ListFilter } from "./library-list"; -/** The handle a pasted URL's server is stored under; the host's first label - * reads better than a random id and is what a person would have typed. */ -function handleFromUrl(url: string): string { - const host = new URL(url).hostname.replace(/^www\./, ""); - const label = host.split(".")[0] ?? host; - return label.toLocaleLowerCase().replace(/[^a-z0-9]+/g, "-"); -} - function toolHost(url: string): string { - return new URL(url).hostname; + try { + return new URL(url).hostname; + } catch { + return url; + } } function ToolTile({ @@ -172,44 +181,124 @@ function PopularTile({ function AddByUrl({ onAdd, onSignIn, + onProbeName, + existingNames, + existingHandles, adding, waiting, }: { readonly onAdd: (input: { url: string; name: string; handle: string; token?: string }) => void; readonly onSignIn: (input: { url: string; name: string; handle: string }) => void; + /** The server's self-reported `serverInfo` for a pasted URL; rejects when + * the server cannot answer (bad URL, auth-gated, offline). */ + readonly onProbeName: (url: string) => Promise; + readonly existingNames: readonly string[]; + readonly existingHandles: readonly string[]; readonly adding: boolean; readonly waiting: boolean; }) { const [url, setUrl] = useState(""); const [token, setToken] = useState(""); + const [name, setName] = useState(""); + /** Typed text is never overwritten by a suggestion; clearing the field + * hands the suggestion back. */ + const [edited, setEdited] = useState(false); + /** The last looked-up `serverInfo` for this URL; cleared with the URL, so + * a suggestion never leaks across servers. */ + const [serverInfo, setServerInfo] = useState(undefined); + const [probing, setProbing] = useState(false); + + const hasUrl = url.trim() !== ""; + const suggestion = suggestMcpServerName({ url, serverInfo, existingNames }); + const shown = edited ? name : hasUrl ? suggestion.name : ""; + const typed = shown.trim(); + /** A URL pasted into the name field is never saved raw: the hint below + * says so, and the extracted suggestion is what gets stored. */ + const typedUrlName = edited ? redirectUrlName(typed, existingNames) : null; + const finalName = + typedUrlName ?? (typed === "" ? suggestion.name : dedupeMcpName(typed, existingNames)); + + let hint: string; + if (!hasUrl) { + hint = "Paste a server URL and the name fills in."; + } else if (!edited) { + hint = + suggestion.source === "server" + ? "From the server itself — edit as you like." + : "Suggested from the URL — edit as you like."; + } else if (typedUrlName !== null) { + hint = `Looks like a URL — will be saved as "${typedUrlName}".`; + } else if (typed === "") { + hint = `Will be saved as "${suggestion.name}".`; + } else if (finalName !== typed) { + hint = `"${typed}" is taken — will be saved as "${finalName}".`; + } else { + hint = "Custom name."; + } + + function changeUrl(value: string) { + setUrl(value); + setServerInfo(undefined); + } function parsedHandle(): string | null { - try { - return handleFromUrl(url); - } catch { + const base = handleFromUrl(url); + if (base === null) { toast("That doesn't look like a server URL."); return null; } + return dedupeMcpHandle(base, existingHandles); + } + + function reset() { + setUrl(""); + setToken(""); + setName(""); + setEdited(false); + setServerInfo(undefined); + } + + async function lookUp() { + const trimmed = url.trim(); + if (trimmed === "" || probing) return; + setProbing(true); + try { + const info = await onProbeName(trimmed); + setServerInfo(info); + // A suggestion never clobbers what was typed: only hand the name field + // back to the suggestion when the user had not already edited it. + if (!edited) setEdited(false); + } catch (cause) { + // The hub answers an auth-gated server's discovery with "refused the + // request" (it answered but needs a token), which reads differently from + // a server that never answered at all. + if (cause instanceof McpServerError && cause.needsBearer) { + toast("That server needs a token — add it below."); + } else { + toast("That server didn't answer — check the URL."); + } + } finally { + setProbing(false); + } } function signIn() { const handle = parsedHandle(); if (handle === null) return; - onSignIn({ url, handle, name: handle }); - setUrl(""); + onSignIn({ url: url.trim(), handle, name: finalName }); + reset(); } function submit() { const handle = parsedHandle(); if (handle === null) return; onAdd({ - url, + url: url.trim(), handle, - name: handle, + name: finalName, ...(token === "" ? {} : { token }), }); - setUrl(""); - setToken(""); + reset(); } return ( @@ -223,9 +312,36 @@ function AddByUrl({ placeholder="https://mcp.example.com/mcp" value={url} onChange={(event) => { - setUrl(event.target.value); + changeUrl(event.target.value); }} /> +
+ { + setName(event.target.value); + setEdited(true); + }} + /> +
+ + {hint} + + +
+
{ + const workspaceTenantId = await resolveWorkspaceTenantId(tenantId as string); + return (await probeMcpServer({ tenantId: workspaceTenantId, url: probeUrl })).serverInfo; + } + return stage(
@@ -442,6 +565,9 @@ export function ToolsPage({ tenantId }: { readonly tenantId: string | null }) { onSignIn={(server) => { signIn.start.mutate(server); }} + onProbeName={probeServerName} + existingNames={servers.map((server) => server.name)} + existingHandles={servers.map((server) => server.handle)} adding={add.isPending || signIn.busy} waiting={signIn.waitingHandle !== null} /> diff --git a/apps/web/src/tools/tools-page.css b/apps/web/src/tools/tools-page.css index d97744411..4709369e6 100644 --- a/apps/web/src/tools/tools-page.css +++ b/apps/web/src/tools/tools-page.css @@ -18,7 +18,7 @@ .tools-grid { display: grid; gap: 12px; - grid-template-columns: repeat(auto-fill, minmax(200px, 1fr)); + grid-template-columns: repeat(auto-fill, minmax(240px, 1fr)); } .tool-tile { @@ -139,10 +139,39 @@ align-self: flex-start; } +/* The name row's fields stay tightly bound while its long hint wraps on full + * width; both the name row and the footer row share the card's left edge. */ +.tool-name-group { + display: flex; + flex-direction: column; + gap: 8px; +} + +.tool-name-actions { + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: 8px; +} + +.tool-name-actions .tool-tile-hint { + flex: 1; + min-width: 0; + overflow-wrap: anywhere; +} + +/* The touch target for a secondary action: react-ui's "lg" button is 40px, + * so an explicit 44px floor keeps it on the app's minimum target size + * alongside its sibling actions in the card. */ +.tool-name-actions button { + min-height: 44px; + flex: none; +} + .tool-card { display: flex; flex-direction: column; - gap: 0.75rem; + gap: 16px; padding: 16px; } diff --git a/e2e/lib/first-run.ts b/e2e/lib/first-run.ts index 820ef952e..3366b9bca 100644 --- a/e2e/lib/first-run.ts +++ b/e2e/lib/first-run.ts @@ -38,8 +38,22 @@ export async function runFirstRunFlow(page: Page, origin: string): Promise // Connecting hands off to the new-workbench prompt (a workspace with no // workbenches); the first bench's worker is the first worker. - await page.waitForSelector("textarea", { timeout: STEP_TIMEOUT }); + // The composer stays disabled until workspace roles have loaded (CL-9780). + await page.waitForFunction( + `(() => { + const input = document.querySelector("textarea"); + return input instanceof HTMLTextAreaElement && !input.disabled; + })()`, + { timeout: STEP_TIMEOUT }, + ); await page.type("textarea", FIRST_WORKBENCH); + await page.waitForFunction( + `(() => { + const send = document.querySelector("button[aria-label='Start this workbench']"); + return send instanceof HTMLButtonElement && !send.disabled; + })()`, + { timeout: STEP_TIMEOUT }, + ); await page.click("button[aria-label='Start this workbench']"); // Creating the workbench navigates to its own page, which shows the prompt. diff --git a/e2e/mcp/server-name-prefill.test.ts b/e2e/mcp/server-name-prefill.test.ts new file mode 100644 index 000000000..b6cf3363b --- /dev/null +++ b/e2e/mcp/server-name-prefill.test.ts @@ -0,0 +1,195 @@ +// The Tools page's name pre-fill for "Add a server by URL": pasting a URL +// fills the name from the URL host, "Look up name" swaps in the server's own +// serverInfo, typed text is never overwritten (clearing falls back to the +// suggestion), a URL pasted into the name field is redirected rather than +// saved raw, a second identical add dedupes, and the stored names match. +// The fake server is keyless on loopback: it answers discovery with no +// bearer, so both the lookup and the keyless Add flow reach it. +import { afterAll, expect, test } from "bun:test"; +import { type } from "arktype"; +import { bootBrowserApp, browserGate } from "../lib/browser"; +import { clickText, runFirstRunFlow, STEP_TIMEOUT, waitForText } from "../lib/first-run"; + +const describeBrowser = browserGate(import.meta.path); + +const NAME_INPUT = "input[aria-label='Server name']"; +const URL_INPUT = "input[aria-label='Server URL']"; +const HINT = ".tool-name-actions .tool-tile-hint"; + +function startFakeKeylessMcpServer() { + const json = (body: unknown, status = 200) => + new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); + const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + async fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/mcp" && req.method === "POST") { + const rpc = (await req.json()) as { id?: number; method: string }; + if (rpc.id === undefined) return new Response(null, { status: 202 }); + if (rpc.method === "initialize") { + return json({ + jsonrpc: "2.0", + id: rpc.id, + result: { + protocolVersion: "2025-03-26", + capabilities: { tools: {} }, + serverInfo: { name: "acme", version: "1.0.0" }, + }, + }); + } + if (rpc.method === "tools/list") { + return json({ + jsonrpc: "2.0", + id: rpc.id, + result: { + tools: [ + { + name: "acme.list_things", + description: "List things", + inputSchema: { type: "object", properties: {} }, + annotations: { readOnlyHint: true }, + }, + ], + }, + }); + } + } + return new Response("not found", { status: 404 }); + }, + }); + return { server, origin: `http://127.0.0.1:${String(server.port)}` }; +} + +describeBrowser("mcp server name prefill", () => { + const fake = startFakeKeylessMcpServer(); + const app = bootBrowserApp(); + afterAll(async () => { + await fake.server.stop(true); + }); + + test("prefill → look-up → edit-stays → dedupe → stored name", async () => { + const { page, errors } = await app().newPage(); + try { + await runFirstRunFlow(page, app().origin); + // Capture the workspace catalog read once, rather than tracking unrelated later requests. + const [credentialsRequest] = await Promise.all([ + page.waitForRequest( + (request) => + request.method() === "GET" && + /\/api\/tenants\/[^/]+\/credentials$/.test(new URL(request.url()).pathname), + { timeout: STEP_TIMEOUT }, + ), + page.goto(`${app().origin}/tools`, { waitUntil: "domcontentloaded" }), + ]); + const credentialsPath = new URL(credentialsRequest.url()).pathname; + const serverUrl = `${fake.origin}/mcp`; + + const nameValue = () => + page.evaluate( + `document.querySelector(${JSON.stringify(NAME_INPUT)}).value`, + ) as Promise; + const hintText = () => + page.evaluate( + `document.querySelector(${JSON.stringify(HINT)}).textContent ?? ""`, + ) as Promise; + const storedNames = async () => + type("string[]").assert( + await page.evaluate(`(async () => { + const response = await fetch(${JSON.stringify(credentialsPath)}); + if (!response.ok) throw new Error("Listing credentials failed"); + const creds = await response.json(); + return creds.data + .filter((c) => c.metadata?.mcp?.url === ${JSON.stringify(serverUrl)}) + .map((c) => c.metadata.mcp.name); + })()`), + ); + async function clearName(): Promise { + await page.click(NAME_INPUT, { count: 3 }); + await page.keyboard.press("Backspace"); + } + + await page.waitForSelector(URL_INPUT, { timeout: STEP_TIMEOUT }); + + // Pasting the URL pre-fills the name from the host (loopback names itself whole). + await page.type(URL_INPUT, serverUrl); + await page.waitForFunction( + `document.querySelector(${JSON.stringify(NAME_INPUT)}).value === "127.0.0.1"`, + { timeout: STEP_TIMEOUT }, + ); + expect(await hintText()).toContain("Suggested from the URL"); + + // "Look up name" swaps in the server's self-reported serverInfo. + await clickText(page, ".tool-name-actions button", "Look up name"); + await page.waitForFunction( + `document.querySelector(${JSON.stringify(NAME_INPUT)}).value === "acme"`, + { timeout: STEP_TIMEOUT }, + ); + expect(await hintText()).toContain("From the server itself"); + + // Typed text is never overwritten by a suggestion. + await page.click(NAME_INPUT, { count: 3 }); + await page.type(NAME_INPUT, "Custom Acme"); + await Bun.sleep(500); + expect(await nameValue()).toBe("Custom Acme"); + expect(await hintText()).toBe("Custom name."); + + // Regression: typing then "Look up name" must keep the typed value, not + // flip back to the server-suggested name. + await clickText(page, ".tool-name-actions button", "Look up name"); + await page.waitForFunction( + `document.querySelector(${JSON.stringify(NAME_INPUT)}).value === "Custom Acme"`, + { timeout: STEP_TIMEOUT }, + ); + expect(await nameValue()).toBe("Custom Acme"); + + // Clearing falls back to the suggestion for the save. + await clearName(); + expect(await nameValue()).toBe(""); + expect(await hintText()).toBe('Will be saved as "acme".'); + + // A URL pasted into the name field is redirected, never saved raw. + await page.type(NAME_INPUT, serverUrl); + await page.waitForFunction(`document.body.innerText.includes("Looks like a URL")`, { + timeout: STEP_TIMEOUT, + }); + expect(await hintText()).toBe('Looks like a URL — will be saved as "127.0.0.1".'); + await clearName(); + expect(await hintText()).toBe('Will be saved as "acme".'); + + // Adding stores the suggestion; the tile shows it live. + await clickText(page, ".tool-add-actions button", "Add"); + await page.waitForFunction(`document.body.innerText.includes("1 tool live")`, { + timeout: 120_000, + polling: 500, + }); + await waitForText(page, "acme"); + expect(await storedNames()).toEqual(["acme"]); + + // Adding the same URL again dedupes against the stored name. + await page.type(URL_INPUT, serverUrl); + await page.waitForFunction( + `document.querySelector(${JSON.stringify(NAME_INPUT)}).value === "127.0.0.1"`, + { timeout: STEP_TIMEOUT }, + ); + await clickText(page, ".tool-name-actions button", "Look up name"); + await page.waitForFunction( + `document.querySelector(${JSON.stringify(NAME_INPUT)}).value === "acme 2"`, + { timeout: STEP_TIMEOUT }, + ); + await clickText(page, ".tool-add-actions button", "Add"); + await waitForText(page, "acme 2"); + expect((await storedNames()).sort()).toEqual(["acme", "acme 2"]); + } catch (cause) { + process.stderr.write( + `server-name-prefill failed at ${page.url()}\nconsole: ${errors.join(" | ")}\n`, + ); + throw cause; + } finally { + await page.close(); + } + }, 600_000); +});