From f642cc94032646106e918054e9eb1f4bb63802f7 Mon Sep 17 00:00:00 2001 From: Anton Arapov Date: Fri, 29 May 2026 09:37:50 +0200 Subject: [PATCH] =?UTF-8?q?revert(icon):=20drop=20URL-form=20serverInfo.ic?= =?UTF-8?q?ons=20(#59)=20=E2=80=94=20not=20the=20icon=20mechanism?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #59 emitted a URL-form serverInfo.icons entry on the theory that Claude.ai's connector UI reads serverInfo.icons and was skipping our data: URI. Empirical investigation (loomiomcp vs capsulemcp log A/B) disproved that theory: - Claude.ai's connector icon comes from a favicon crawler (AWS-hosted, hourly, desktop+mobile Chrome UAs) that fetches /favicon.ico DIRECTLY. It never reads serverInfo.icons, and never parses the landing-page tags. - It only crawls CUSTOM DOMAINS, not Public-Suffix-List platform subdomains (*.run.app, *.vercel.app, *.herokuapp.com). That's why the sibling loomiomcp on mcp.openssl-communities.org shows an icon and capsulemcp on *.run.app doesn't. So #59 was built on a wrong hypothesis and is not load-bearing. It was harmless (spec-correct, would only ever matter IF Anthropic ships serverInfo.icons consumption per modelcontextprotocol#152), but it added a helper + refactor justified by a disproven premise. Reverting for cleanliness restores the simple `icons: ICONS` data-URI shape that predated #59: - build-icon.mjs re-emits the ICONS export; icon.ts regenerated. - src/icon-builder.ts + tests/icon-builder.test.ts deleted. - server.ts back to `import { ICONS }` / `icons: ICONS`. Kept: PR #60's landing page (independent DX merit — human-readable root + favicon-checker support), but its CHANGELOG entry is corrected to NOT claim it fixes Claude icon display, with a scope note documenting the real favicon-crawler + custom-domain mechanism. The actual fix for the connector icon (custom-domain mapping) is infra-side, not a code change in this repo. 533 → 529 tests (−4, icon-builder tests removed). Bundle 165.18 KB stdio / 193.05 KB http. HOWTO test count + bundle figures resynced (were stale at 523 / ~191 from earlier un-synced merges). Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 65 +++++++++++--------------------------- HOWTO.md | 4 +-- scripts/build-icon.mjs | 26 +++++++++------ src/icon-builder.ts | 65 -------------------------------------- src/icon.ts | 26 +++++++++------ src/server.ts | 8 ++--- tests/bundle-shape.test.ts | 2 +- tests/icon-builder.test.ts | 65 -------------------------------------- 8 files changed, 56 insertions(+), 205 deletions(-) delete mode 100644 src/icon-builder.ts delete mode 100644 tests/icon-builder.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8792ba9..6a7dfd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,57 +13,30 @@ versions adhere to [Semantic Versioning](https://semver.org). ### Added -- **`GET /` now serves a small HTML landing page** with proper +- **`GET /` now serves a small HTML landing page** — a short + human-readable body pointing at `/mcp` and the GitHub repo, plus `` and `` tags in the - head, plus a short human-readable body pointing at `/mcp` and - the GitHub repo. Closes the browser-style favicon-discovery gap: - before this change, hitting `/` returned Express's default 404 - HTML with an empty ``, so any client doing HTML-shaped - icon discovery (favicon checkers, some connector UIs) found no - hints and fell back to nothing. - - Pairs with the v1.6.6 URL-form `serverInfo.icons` fix so we now - cover BOTH icon-discovery channels: - - MCP protocol channel: `serverInfo.icons` returns a URL entry - first (data URI fallback second) — for clients that read the - MCP initialize response. - - HTML channel: `GET /` returns an HTML page with `` - tags pointing at `/icon.svg` — for clients that crawl the root - URL like a browser. - - Static bytes, no template interpolation, no XSS surface. 1h - cache. Generic content only (no deployment-specific URLs or - tenant info in the body). 6 new tests in `tests/http-app.test.ts` - cover: content-type, the two `` tags' shape, the `/mcp` - reference in the body, the cache-control header, and a - determinism check (two requests return identical bytes). + head. Before this, hitting the root URL in a browser returned + Express's default "Cannot GET /" — now a curious visitor (or a + favicon checker that parses HTML) gets something useful. + + Scope note: this does NOT change how Claude.ai's connector UI + displays the server icon. We empirically traced that mechanism + to a favicon crawler (AWS-hosted, hourly, browser user-agents) + that fetches `/favicon.ico` directly — it does not parse the + landing-page `` tags, and it only crawls **custom domains**, + not Public-Suffix-List platform subdomains like `*.run.app`. The + load-bearing route for that crawler (`/favicon.ico`) predates + this change; getting Claude.ai to show the icon requires mapping + the service to a custom domain, which is infra-side, not a code + change here. The landing page stands on its own DX merit. 6 new + tests in `tests/http-app.test.ts` cover content-type, the two + `` tags, the `/mcp` reference, cache-control, and a + determinism check. ### Fixed -- **`serverInfo.icons` now emits a URL-form entry first when - `PUBLIC_BASE_URL` is set (HTTP+OAuth deploys), with the existing - data-URI form retained as a second fallback entry.** Some client - UIs (observed: Claude.ai's connector list on the Cloud Run URL) - silently skip `data:` image srcs — plausibly a CSP / `img-src` - policy that disallows data URIs. A sibling MCP server fronted by - a custom domain rendered its icon correctly with the URL form - while capsulemcp on `*.run.app` didn't, even though the SVG bytes - were identical. The fix emits both shapes (URL first so clients - iterating the array pick it; data URI second for stdio + CSP- - tolerant clients). Stdio installs (no `PUBLIC_BASE_URL`) see no - behavioural change — still data-URI-only. - - Refactor: the icons array shape now lives in - `src/icon-builder.ts` (hand-edited) instead of inside the SVG - generator at `scripts/build-icon.mjs`. `src/icon.ts` is now pure - generated data (`ICON_SVG`, `ICON_DATA_URI`); the orchestration - (URL vs data URI ordering, sizes hints) is separate. The `ICONS` - named export from `src/icon.ts` is gone — replaced by - `buildIcons(publicBaseUrl?)` in `src/icon-builder.ts`. No external - consumer relied on `ICONS` (single internal call site in - `src/server.ts`). - - **Tool annotations now emit `{readOnlyHint, destructiveHint}` explicitly on every tool — never rely on MCP spec defaults.** Per spec, `destructiveHint` defaults to `true`. The pre-fix diff --git a/HOWTO.md b/HOWTO.md index 3d16753..dd89c61 100644 --- a/HOWTO.md +++ b/HOWTO.md @@ -11,7 +11,7 @@ npm install npm test ``` -523 tests, all mocked — no Capsule API calls happen, no token needed. The suite has three layers: +529 tests, all mocked — no Capsule API calls happen, no token needed. The suite has three layers: - **Per-tool unit tests** (e.g. `tests/parties.test.ts`): import the tool function, mock `undici.fetch`, assert on the URL, method, body, and response handling. Most tests live here. - **MCP-protocol integration tests** (`tests/mcp-integration.test.ts`): drive a real `McpServer` through the wire protocol via the SDK's in-memory transport pair, with `undici.fetch` still mocked. Catches the layer between "tool function works" and "MCP correctly registers and dispatches the tool". Includes the `get_attachment` content-type routing logic (which lives in `server.ts`, not the tool function). @@ -46,7 +46,7 @@ for the contributor-facing summary. npm run build ``` -Produces `dist/index.js` (stdio entry, ~165 KB, with `#!/usr/bin/env node` shebang and the executable bit set) and `dist/http.js` (HTTP entry, ~191 KB, no shebang). Each is fully self-contained — tsup runs as two separate configs so the stdio entry can be invoked directly via npx while the HTTP entry isn't a CLI. tsup target is Node 22 (undici 8 requires Node 22+ for the `webidl.util.markAsUncloneable` runtime API). +Produces `dist/index.js` (stdio entry, ~165 KB, with `#!/usr/bin/env node` shebang and the executable bit set) and `dist/http.js` (HTTP entry, ~193 KB, no shebang). Each is fully self-contained — tsup runs as two separate configs so the stdio entry can be invoked directly via npx while the HTTP entry isn't a CLI. tsup target is Node 22 (undici 8 requires Node 22+ for the `webidl.util.markAsUncloneable` runtime API). `npm run build` also chains `npm run build:icon` (`scripts/build-icon.mjs`), which regenerates `src/icon.ts` from the canonical `assets/icon.svg`. The TypeScript file is committed (so typecheck works without a build step) but is **generated** — edit the SVG, then run the build. A drift-guard test (`tests/icon-source.test.ts`) fails CI if the two ever fall out of sync. diff --git a/scripts/build-icon.mjs b/scripts/build-icon.mjs index 586b047..af2d893 100644 --- a/scripts/build-icon.mjs +++ b/scripts/build-icon.mjs @@ -43,23 +43,29 @@ const generated = `/** * edit this file directly** — edit the SVG and re-run \`npm run * build:icon\` (or \`npm run build\`, which chains it). * - * This file is generated DATA: the raw SVG plus its \`data:\` URI - * form. The \`serverInfo.icons\` ARRAY shape (URL form vs data URI - * form vs both, ordering, sizes hints) is hand-edited orchestration - * and lives in \`src/icon-builder.ts\` — kept out of this generator - * so the icon-array shape can evolve without touching the SVG. - * - * Exposed two ways at runtime: + * Exposed two ways: * - Embedded as a \`data:\` URI in the MCP \`serverInfo.icons\` array - * (spec-compliant; works without any HTTP route — stdio path). + * (spec-compliant; works without any HTTP route). * - Served at \`/icon.svg\` and \`/favicon.ico\` by the HTTP entry, - * so clients that prefer to fetch a URL get a real HTTPS resource - * (some UIs' CSP blocks \`data:\` image srcs — URL form survives). + * in case the consuming client prefers a URL it can fetch. */ export const ICON_SVG = \`${escapedSvg}\`; export const ICON_DATA_URI = \`data:image/svg+xml;base64,\${Buffer.from(ICON_SVG, "utf8").toString("base64")}\`; + +/** + * Shaped for MCP's \`serverInfo.icons\` field. Single 64x64 SVG that + * scales cleanly to any size; \`sizes: ["any"]\` tells the client it + * works at every render size. + */ +export const ICONS = [ + { + src: ICON_DATA_URI, + mimeType: "image/svg+xml", + sizes: ["64x64", "any"], + }, +]; `; writeFileSync(TS_PATH, generated, "utf8"); diff --git a/src/icon-builder.ts b/src/icon-builder.ts deleted file mode 100644 index 2a36ad6..0000000 --- a/src/icon-builder.ts +++ /dev/null @@ -1,65 +0,0 @@ -/** - * Builds the `serverInfo.icons` array shape for MCP's `initialize` - * response. Lives separately from the generated `src/icon.ts` so the - * shape can evolve (URL form, multiple sizes, fallback ordering) - * without touching the SVG generator. - * - * Why two forms (URL + data URI): - * - * - URL form (`https:///icon.svg`) — preferred by client UIs - * whose Content-Security-Policy blocks `data:` image srcs in - * `` tags. Empirically: a sibling MCP server (Discourse- - * flavored, served from a custom domain) renders its icon - * correctly in Claude.ai's connector list while capsulemcp on - * a `*.run.app` URL did not — the difference traced to URL-vs- - * data-URI src shape, not the icon bytes themselves. - * - Data URI form — host-independent fallback. The stdio entry - * has no HTTP route to serve the SVG from, so the data URI is - * the only viable shape there. Also useful for clients that - * prefer inline bytes (no extra fetch round-trip). - * - * Ordering rule: URL first when available, then data URI. Clients - * that iterate the array and pick the first usable entry get the - * URL form by default and never see the data URI; clients that - * filter by `mimeType` or `sizes` see both and can pick either. - * - * When `publicBaseUrl` is undefined (stdio invocation, or HTTP - * deploy that hasn't configured `PUBLIC_BASE_URL`), only the data - * URI entry is emitted — preserves the pre-v1.6.x behaviour exactly. - */ - -import { ICON_DATA_URI } from "./icon.js"; - -interface IconEntry { - src: string; - mimeType: string; - sizes: string[]; -} - -export function buildIcons(publicBaseUrl?: string): IconEntry[] { - const icons: IconEntry[] = []; - - if (publicBaseUrl) { - // Strip trailing slash so the URL doesn't render as - // `https://host//icon.svg` if the caller passes a base ending in /. - // (Express's iconHandler at /icon.svg responds to either form, but - // the wire shape should look clean to humans reading the - // initialize response.) - const base = publicBaseUrl.replace(/\/+$/, ""); - icons.push({ - src: `${base}/icon.svg`, - mimeType: "image/svg+xml", - sizes: ["any"], - }); - } - - // Data URI: host-independent fallback. Always present — covers - // stdio (no HTTP route) and clients that prefer inline bytes. - icons.push({ - src: ICON_DATA_URI, - mimeType: "image/svg+xml", - sizes: ["64x64", "any"], - }); - - return icons; -} diff --git a/src/icon.ts b/src/icon.ts index ad2780a..0804cdb 100644 --- a/src/icon.ts +++ b/src/icon.ts @@ -7,18 +7,11 @@ * edit this file directly** — edit the SVG and re-run `npm run * build:icon` (or `npm run build`, which chains it). * - * This file is generated DATA: the raw SVG plus its `data:` URI - * form. The `serverInfo.icons` ARRAY shape (URL form vs data URI - * form vs both, ordering, sizes hints) is hand-edited orchestration - * and lives in `src/icon-builder.ts` — kept out of this generator - * so the icon-array shape can evolve without touching the SVG. - * - * Exposed two ways at runtime: + * Exposed two ways: * - Embedded as a `data:` URI in the MCP `serverInfo.icons` array - * (spec-compliant; works without any HTTP route — stdio path). + * (spec-compliant; works without any HTTP route). * - Served at `/icon.svg` and `/favicon.ico` by the HTTP entry, - * so clients that prefer to fetch a URL get a real HTTPS resource - * (some UIs' CSP blocks `data:` image srcs — URL form survives). + * in case the consuming client prefers a URL it can fetch. */ export const ICON_SVG = ` @@ -43,3 +36,16 @@ export const ICON_SVG = ` { const httpKb = statSync(HTTP_PATH).size / 1024; // Floor catches "the bundler produced an empty file"; ceiling // catches "we accidentally inlined a giant dependency". The - // current values (~165 / ~191 KB) sit comfortably in the band. + // current values (~165 / ~193 KB) sit comfortably in the band. expect(stdioKb).toBeGreaterThan(MIN_KB); expect(stdioKb).toBeLessThan(MAX_KB); expect(httpKb).toBeGreaterThan(MIN_KB); diff --git a/tests/icon-builder.test.ts b/tests/icon-builder.test.ts deleted file mode 100644 index 8068dd4..0000000 --- a/tests/icon-builder.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -/** - * Tests for `buildIcons()` — the `serverInfo.icons` array - * constructor. - * - * Pins three contracts: - * 1. Stdio shape (no publicBaseUrl) is data-URI-only — preserves - * the pre-fix wire shape so stdio installs see no behavioural - * change. - * 2. HTTP shape (publicBaseUrl present) is URL-first, data-URI - * second — clients iterating the array and picking the first - * usable entry get the URL form (the goal of the fix). - * 3. Trailing slash on publicBaseUrl is tolerated — `${base}/icon.svg` - * doesn't render as `host//icon.svg`. - */ - -import { describe, expect, it } from "vitest"; -import { buildIcons } from "../src/icon-builder.js"; - -describe("buildIcons", () => { - it("stdio shape: undefined publicBaseUrl → data-URI-only entry", () => { - const icons = buildIcons(undefined); - - expect(icons).toHaveLength(1); - expect(icons[0]?.src.startsWith("data:image/svg+xml;base64,")).toBe(true); - expect(icons[0]?.mimeType).toBe("image/svg+xml"); - // Preserves the historic `sizes: ["64x64", "any"]` for stdio. - expect(icons[0]?.sizes).toEqual(["64x64", "any"]); - }); - - it("HTTP shape: URL-first, data-URI-second when publicBaseUrl is set", () => { - const icons = buildIcons("https://example.test"); - - expect(icons).toHaveLength(2); - - // URL entry comes first — clients picking the first array entry - // get the URL form (the fix's whole point). - expect(icons[0]?.src).toBe("https://example.test/icon.svg"); - expect(icons[0]?.mimeType).toBe("image/svg+xml"); - - // Data URI is the fallback for clients that can't reach the URL - // (CSP-restricted iframes, offline caches, etc.). - expect(icons[1]?.src.startsWith("data:image/svg+xml;base64,")).toBe(true); - expect(icons[1]?.mimeType).toBe("image/svg+xml"); - }); - - it("strips trailing slash on publicBaseUrl (no `host//icon.svg`)", () => { - const icons = buildIcons("https://example.test/"); - expect(icons[0]?.src).toBe("https://example.test/icon.svg"); - - // Multiple trailing slashes too — defensive against operator typos. - const icons2 = buildIcons("https://example.test///"); - expect(icons2[0]?.src).toBe("https://example.test/icon.svg"); - }); - - it("empty-string publicBaseUrl is treated as 'not set' (data-URI-only)", () => { - // Defensive: an unset env var sometimes shows up as "" instead - // of undefined depending on how it's read. Both should fall back - // to the stdio shape rather than producing a relative URL like - // `/icon.svg` that's not resolvable. - const icons = buildIcons(""); - - expect(icons).toHaveLength(1); - expect(icons[0]?.src.startsWith("data:")).toBe(true); - }); -});