diff --git a/AGENTS.md b/AGENTS.md index d07e84f4d..8ac65f05e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -98,7 +98,21 @@ v2/main/ │ │ # callback listener; also stripBrackets. Used across │ │ # clients/web/server, clients/cli, and core/auth/node — #1795) │ ├── react/ # React hooks over the state stores -│ └── storage/ # File I/O helpers (store-io.ts) used by OAuth persist backends +│ ├── storage/ # File I/O helpers (store-io.ts) used by OAuth persist backends +│ └── uri/ # RFC 6570 URI Template discovery/expansion/preview +│ # (uriTemplate.ts) — wraps the SDK's UriTemplate, keeping +│ # its parse/operators/separators and correcting where it +│ # departs from the RFC: value encoding is done here +│ # against the RFC 3986 sets, each non-query expression is +│ # rewritten to a synthetic variable (so multi-name +│ # expressions encode, and a name repeated under different +│ # operators encodes per occurrence), and the shapes it +│ # mishandles (`{}`, `;`) are declined. Shared by the web +│ # ResourceTemplatePanel, the TUI's uriTemplateToForm, +│ # and InspectorClient.readResourceFromTemplate, so a +│ # template cannot resolve differently per client — #1919. +│ # Gated by the web coverage `include`; tests live in +│ # clients/web/src/test/core/uri/. ├── test-servers/ # Composable MCP test servers + fixtures used by integration tests. │ ├── src/ # TypeScript sources. (modern-tasks.ts: SEP-2663 modern │ │ # Tasks extension runtime + tasks/* Express interceptor @@ -666,7 +680,7 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab - **The test tiers, shallowest first:** unit (`test`, per client) → web integration (`test:integration`, real transports/servers) → out-of-process (`clients/cli/__tests__/e2e.test.ts`, spawns the built binary) → smokes through the built launcher (`npm run smoke`) → Storybook play functions (`test:storybook`) → the published-tarball check (`npm run pack:verify`, local/release only — needs network). `validate` runs the per-client `test` scripts — so web **unit** plus cli's out-of-process `e2e.test.ts` (it's part of cli's `test`), but **not** web's integration project, which runs inside the `coverage` gate. Everything from `smoke` rightward is `npm run ci` only, and is described under [Mandatory pre-push gate](#mandatory-pre-push-gate). - The repo root has no aggregate `test` script — each client self-validates, so run `npm run validate` from the root (all clients, fast) or `cd clients/ && npm run validate` (one client). Each client still exposes its own `test` / `test:coverage` for quick iteration. - **`validate` is fast: it runs `test`, not `test:coverage`.** The coverage gate (slower — adds v8 instrumentation, and for web the integration project) is a **separate** top-level `npm run coverage` (and per-client `coverage:web` / `coverage:cli` / `coverage:tui` / `coverage:launcher`, each delegating to that client's `test:coverage`). Run `npm run coverage` when you want to reproduce the gate locally before pushing. **CI runs `coverage`** on every push (#1550): the per-file ≥90 gate is CI-enforced, so a PR that drops any file below 90 on lines/statements/functions/branches fails the job. CI runs `validate` (fast) for format/lint/build/unit tests, then `coverage` for the instrumented gate. Because web's `test:coverage` already runs the integration project, CI has no separate `test:integration` step — the integration paths are exercised inside the coverage gate. -- Each client's `test:coverage` enforces a **uniform per-file gate of ≥ 90 on all four dimensions** — lines, statements, functions, and branches — across `clients/web`, `clients/cli`, `clients/tui`, and `clients/launcher` (CI enforces this gate). This is the result of a codebase-wide audit: the branch floor was first lifted 50 → 70 for web (#1271), then the whole gate raised to 90 with real tests added for every outlier. Genuinely-unreachable branches are **not** waved through by lowering the gate — they are annotated at the source with a justified `/* v8 ignore … -- */` comment. Acceptable reasons are happy-dom-inherent paths (Mantine portal mount points, `useMediaQuery` fallbacks, `typeof window` SSR guards), React StrictMode effect-replay blocks, and provably-dead defensive guards (e.g. a `?? fallback` for a value the types guarantee non-null, or a `Select.onChange` receiving a value outside the allowed list). New code must clear 90 on every dimension; reach for a justified `v8 ignore` only when a branch is genuinely impossible to exercise. The web coverage `include` (in `clients/web/vite.config.ts`) covers the shared `core/` runtime consumed by the browser — `core/mcp`, `core/react`, `core/auth`, `core/storage`, `core/logging`, `core/node`, **`core/json`, and `core/client`** (the last two folded in by #1689). When adding a `core/json/*` or `core/client/*` module, its tests live under `clients/web/src/test/core/…` and are gated the same ≥90 way. +- Each client's `test:coverage` enforces a **uniform per-file gate of ≥ 90 on all four dimensions** — lines, statements, functions, and branches — across `clients/web`, `clients/cli`, `clients/tui`, and `clients/launcher` (CI enforces this gate). This is the result of a codebase-wide audit: the branch floor was first lifted 50 → 70 for web (#1271), then the whole gate raised to 90 with real tests added for every outlier. Genuinely-unreachable branches are **not** waved through by lowering the gate — they are annotated at the source with a justified `/* v8 ignore … -- */` comment. Acceptable reasons are happy-dom-inherent paths (Mantine portal mount points, `useMediaQuery` fallbacks, `typeof window` SSR guards), React StrictMode effect-replay blocks, and provably-dead defensive guards (e.g. a `?? fallback` for a value the types guarantee non-null, or a `Select.onChange` receiving a value outside the allowed list). New code must clear 90 on every dimension; reach for a justified `v8 ignore` only when a branch is genuinely impossible to exercise. The web coverage `include` (in `clients/web/vite.config.ts`) covers the shared `core/` runtime consumed by the browser — `core/mcp`, `core/react`, `core/auth`, `core/storage`, `core/logging`, `core/node`, **`core/json`, and `core/client`** (the last two folded in by #1689), plus **`core/uri`** (#1919). When adding a `core/json/*`, `core/client/*`, or `core/uri/*` module, its tests live under `clients/web/src/test/core/…` and are gated the same ≥90 way. - The **same per-file gate** is enforced for the CLI and TUI (#1484), not just web: - **CLI** (`clients/cli`): tests run **in-process** by importing `runCli()` (see `__tests__/helpers/cli-runner.ts`) so `clients/cli/src` is measured under v8 instrumentation. A thin out-of-process layer (`__tests__/e2e.test.ts` + `scripts/smoke-cli.mjs`) still spawns the built binary for the shebang/`process.exit` paths; `src/index.ts` (binary bootstrap) is the only coverage exclusion. `commander` uses `.exitOverride()` so a parse error throws instead of tearing down the test worker. - **TUI** (`clients/tui`): the gate now covers **all of `src/**`, React surface included** — the former interim exclusion of the Ink components, `App.tsx`, and `hooks/` was lifted in #1501. Components mount through `ink-testing-library` with the `ink-scroll-view` / `ink-form` passthrough doubles in `__tests__/helpers/`, `App.tsx` mounts against a controllable mock of the `@inspector/core` surface, and keypresses are driven through stdin. The **only** coverage exclusion left in `clients/tui/vitest.config.ts` is `src/tui-servers.ts` — a pure re-export + type alias of core's server resolver with no runtime statements of its own (the logic is measured in `core/` via the web suite; `tui-servers.test.ts` still exercises it behaviorally, and it's excluded only so it doesn't surface as a misleading 0/0 row). Any new logic under `clients/tui/src`, React or not, is held to the gate automatically. diff --git a/README.md b/README.md index cb57dd359..924c69923 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,8 @@ inspector/ │ ├── mcp/ # InspectorClient runtime, state stores, transports, config import │ ├── node/ # Node-only shared helpers: version reader, hostUrl (host normalize/canonicalize + all-interfaces/loopback detection) │ ├── react/ # React hooks over the state stores -│ └── storage/ # File I/O helpers for the OAuth persist backends +│ ├── storage/ # File I/O helpers for the OAuth persist backends +│ └── uri/ # RFC 6570 URI Template discovery/expansion/preview, shared by all clients ├── test-servers/ # Composable MCP test servers + fixtures used by integration tests ├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, verify-format-coverage, verify-dep-lockstep, pack:verify) ├── docs/ # Task-oriented guides (v1→v2 migration, server configuration, MCP App review, launcher/config plan) @@ -147,6 +148,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | `structured-output-http.json` | Tools tab: a result's `structuredContent` section | [#1908](https://github.com/modelcontextprotocol/inspector/issues/1908) | | `duplicate-tool-names-http.json` | A `tools/list` that repeats a tool name | [#1957](https://github.com/modelcontextprotocol/inspector/issues/1957) | | `nullable-fields-http.json` | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) | +| `rfc6570-templates-http.json` | Resources tab: RFC 6570 template expansion | [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) | | `advertised-extensions-http.json` | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) | | `logging-{legacy,modern}-http.json` | Logging, both eras | [#1629](https://github.com/modelcontextprotocol/inspector/issues/1629) | | `subscriptions-{legacy,modern}-http.json` | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) | @@ -237,6 +239,22 @@ Open the Tools tab and select `record_shipment`: `direction` must render as a ** The **TUI** had the same gap and is worth checking against the same server (`--tui`, then test `record_shipment`): `direction` is a select, `quantity` an integer field, `express` a boolean. Both clients now share one collapse step — `normalizeNullableUnion` in [`core/json/nullableUnion.ts`](./core/json/nullableUnion.ts) — precisely so they cannot drift on which schemas they can render. +#### RFC 6570 resource templates + +`rfc6570-templates-http.json` serves the two templates from [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) (preset `rfc6570_templates`): `foobar://events/{topic}` (simple expression) and `foobar://events{?topic}` (query expression). Each echoes back the `topic` it received and the URI that matched. Plain streamable-HTTP — connect with the **default (legacy)** protocol era. + +Open the Resources tab and select **events-by-path**, enter `foo/bar` for `topic`, and read it: the preview and the `resources/read` request must both show `foobar://events/foo%2Fbar`. On the broken build the web client substituted the value verbatim, producing `foobar://events/foo/bar` — a second path segment, which a spec-compliant matcher rejects with `-32602 Resource not found` (this server does exactly that, so the failure is visible rather than silent). + +Then select **events-by-query**: it must render a `topic` input at all. The old scan was `/\{(\w+)\}/g`, which sees only bare `{name}` expressions, so a query expression declared a variable the form never offered. + +All three clients now go through one shared helper, [`core/uri/uriTemplate.ts`](./core/uri/uriTemplate.ts) — the web panel, the TUI's form builder, and `InspectorClient.readResourceFromTemplate` — so a template cannot resolve differently depending on where it is driven from. + +It wraps the SDK's `UriTemplate`, keeping what that gets right (the parse, the operators and separators, which expressions appear at all) and correcting where it departs from RFC 6570: + +- **Value encoding** is done against the explicit RFC 3986 character sets. The SDK's `encodeURIComponent` leaves the sub-delimiters `!*'()` bare, its `encodeURI` escapes the gen-delims `[` and `]` that reserved expansion exists to pass through, and it double-encodes an existing percent triplet (`%41` → `%2541`). +- **Multi-name expressions** (`{a,b}`) take an SDK branch that skips both encoding and the operator, and a **name repeated under different operators** (`{+a}-{a}`) has to encode differently per occurrence — which values looked up by name cannot express. Each non-query expression is rewritten to its own synthetic variable so both work. +- Two shapes the SDK accepts but mishandles — an expression declaring no variable (`{}`), and the unimplemented `;` path-parameter operator — are **declined** rather than expanded into a knowingly invalid URI, so the panel withholds the request instead of sending it. + #### Advertised extensions `advertised-extensions-http.json` serves `echo` (always) and a `get_weather` tool **gated on the `io.modelcontextprotocol/tasks` extension** (`extensionGatedTools`): the tool is registered but starts disabled, and the server enables it on `notifications/initialized` only when the client declared that extension in its `capabilities.extensions`. diff --git a/clients/tui/__tests__/uriTemplateToForm.test.ts b/clients/tui/__tests__/uriTemplateToForm.test.ts index 1c522c659..a888b8cf4 100644 --- a/clients/tui/__tests__/uriTemplateToForm.test.ts +++ b/clients/tui/__tests__/uriTemplateToForm.test.ts @@ -23,13 +23,24 @@ describe("uriTemplateToForm", () => { }); it("logs and returns an empty form when the template cannot be parsed", () => { - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + // The shared core/uri helper warns and yields no names; this file no longer + // does its own try/catch, so the assertion is on that warning (#1919). + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); const form = uriTemplateToForm("file:///{unclosed", "broken"); - expect(errorSpy).toHaveBeenCalledWith( - "Failed to parse URI template:", - expect.any(Error), - ); + expect(warnSpy).toHaveBeenCalled(); expect(form.sections[0]!.fields).toEqual([]); }); + + // Shared with the web panel's field list: the old scan saw only bare + // `{name}` expressions, and a repeated name produced two identical fields. + it("creates a field for a variable inside a query expression", () => { + const form = uriTemplateToForm("foobar://events{?topic}", "events"); + expect(form.sections[0]!.fields.map((f) => f.name)).toEqual(["topic"]); + }); + + it("creates one field for a name repeated across expressions", () => { + const form = uriTemplateToForm("x://{a}/{b}/{a}", "repeat"); + expect(form.sections[0]!.fields.map((f) => f.name)).toEqual(["a", "b"]); + }); }); diff --git a/clients/tui/src/utils/uriTemplateToForm.ts b/clients/tui/src/utils/uriTemplateToForm.ts index c8a027e9c..b8d6a9424 100644 --- a/clients/tui/src/utils/uriTemplateToForm.ts +++ b/clients/tui/src/utils/uriTemplateToForm.ts @@ -3,7 +3,7 @@ */ import type { FormStructure, FormSection, FormField } from "ink-form"; -import { UriTemplate } from "@modelcontextprotocol/client"; +import { templateVariableNames } from "@inspector/core/uri/uriTemplate.js"; /** * Converts a URI Template to ink-form structure @@ -12,28 +12,18 @@ export function uriTemplateToForm( uriTemplate: string, templateName: string, ): FormStructure { - const fields: FormField[] = []; - - try { - const template = new UriTemplate(uriTemplate); - /* v8 ignore next -- UriTemplate.variableNames is a getter that always - returns a string[]; the `|| []` fallback is an unreachable guard. */ - const variableNames = template.variableNames || []; - - for (const variableName of variableNames) { - const field: FormField = { - name: variableName, - label: variableName, - type: "string", - required: false, // URI template variables are typically optional - }; - - fields.push(field); - } - } catch (error) { - // If parsing fails, return empty form - console.error("Failed to parse URI template:", error); - } + // Shared with the web panel's field list (#1919), so the two clients offer + // the same inputs for a given template — including the variables inside + // non-simple expressions, and one field (not two) for a repeated name. It + // does not throw: a malformed template yields no names, so the form is empty. + const fields: FormField[] = templateVariableNames(uriTemplate).map( + (variableName) => ({ + name: variableName, + label: variableName, + type: "string", + required: false, // URI template variables are typically optional + }), + ); const sections: FormSection[] = [ { diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.stories.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.stories.tsx index 8098dbf1a..c38c206a7 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.stories.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.stories.tsx @@ -46,6 +46,23 @@ export const WithAnnotations: Story = { }, }; +/** + * An RFC 6570 query expression. The variable lives inside `{?…}` rather than a + * bare `{…}`, so it only produces an input once discovery goes through a real + * RFC 6570 parser (#1919); the preview shows where the value lands in the + * query string. + */ +export const QueryExpression: Story = { + args: { + template: { + name: "Events", + uriTemplate: "foobar://events{?topic}", + description: + "Filter the event stream by topic. The value is percent-encoded into the query string.", + }, + }, +}; + export const NoDescription: Story = { args: { template: { diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx index 596a9340f..8d5ac88ba 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx @@ -1,7 +1,11 @@ import { describe, it, expect, vi } from "vitest"; import userEvent from "@testing-library/user-event"; import type { ResourceTemplateType as ResourceTemplate } from "@modelcontextprotocol/client"; -import { renderWithMantine, screen } from "../../../test/renderWithMantine"; +import { + renderWithMantine, + screen, + fireEvent, +} from "../../../test/renderWithMantine"; import { ResourceTemplatePanel } from "./ResourceTemplatePanel"; const singleVarTemplate: ResourceTemplate = { @@ -27,6 +31,18 @@ const noVarTemplate: ResourceTemplate = { uriTemplate: "file:///static.txt", }; +// #1919: a query expression declares a variable the old `/\{(\w+)\}/g` scan +// could not see, so it rendered no input at all. +const queryVarTemplate: ResourceTemplate = { + name: "Events", + uriTemplate: "foobar://events{?topic}", +}; + +const simpleVarTemplate: ResourceTemplate = { + name: "Events", + uriTemplate: "foobar://events/{topic}", +}; + describe("ResourceTemplatePanel", () => { it("renders the template title (or name) and description", () => { renderWithMantine( @@ -105,6 +121,102 @@ describe("ResourceTemplatePanel", () => { expect(screen.getByText("file:///users/bob/profile")).toBeInTheDocument(); }); + it("renders an input for a variable declared by a query expression", () => { + renderWithMantine( + , + ); + expect(screen.getByLabelText("topic")).toBeInTheDocument(); + }); + + it("expands a query expression per RFC 6570 when submitted", async () => { + const user = userEvent.setup(); + const onReadResource = vi.fn(); + renderWithMantine( + , + ); + await user.type(screen.getByLabelText("topic"), "weather"); + await user.click(screen.getByRole("button", { name: "Read Resource" })); + expect(onReadResource).toHaveBeenCalledWith( + "foobar://events?topic=weather", + ); + }); + + it("percent-encodes a reserved character rather than emitting a new path segment", async () => { + const user = userEvent.setup(); + const onReadResource = vi.fn(); + renderWithMantine( + , + ); + await user.type(screen.getByLabelText("topic"), "foo/bar"); + await user.click(screen.getByRole("button", { name: "Read Resource" })); + expect(onReadResource).toHaveBeenCalledWith("foobar://events/foo%2Fbar"); + }); + + it("previews the encoded value, not the raw input", async () => { + const user = userEvent.setup(); + renderWithMantine( + , + ); + await user.type(screen.getByLabelText("topic"), "a b"); + expect(screen.getByText("foobar://events/a%20b")).toBeInTheDocument(); + }); + + // The SDK refuses a value past its 1,000,000-character ceiling at expansion + // time, and the input has no matching limit. Withhold the request rather than + // send a URI we know is wrong. + it("keeps Read Resource disabled when the value cannot be expanded", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const onReadResource = vi.fn(); + renderWithMantine( + , + ); + // fireEvent, not user.type — typing a million characters key by key would + // take longer than the suite's timeout. + fireEvent.change(screen.getByLabelText("topic"), { + target: { value: "z".repeat(1_000_001) }, + }); + expect( + screen.getByRole("button", { name: "Read Resource" }), + ).toBeDisabled(); + expect(onReadResource).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + // `x://{}` parses in the SDK and reports no variables, so the panel renders + // no inputs and "every variable is filled" is vacuously true. Read Resource + // must stay disabled rather than submit a URI that is not the advertised + // template. + it("keeps Read Resource disabled for a template with an empty expression", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + renderWithMantine( + , + ); + expect( + screen.getByRole("button", { name: "Read Resource" }), + ).toBeDisabled(); + // And it shows the template as declared, not the `x://` the SDK expands to. + expect(screen.getByText("x://{}")).toBeInTheDocument(); + warn.mockRestore(); + }); + it("clears a variable via its Clear button (non-autocomplete branch)", async () => { const user = userEvent.setup(); renderWithMantine( diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx index 6b506228b..cd9d5b18f 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx @@ -14,6 +14,11 @@ import { useValueChange } from "../../../hooks/useValueChange"; import type { ResourceTemplateType as ResourceTemplate } from "@modelcontextprotocol/client"; import { AnnotationBadge } from "../../elements/AnnotationBadge/AnnotationBadge"; import { CopyButton } from "../../elements/CopyButton/CopyButton"; +import { + expandTemplate, + previewTemplate, + templateVariableNames, +} from "@inspector/core/uri/uriTemplate.js"; export interface ResourceTemplatePanelProps { template: ResourceTemplate; @@ -39,34 +44,6 @@ export interface ResourceTemplatePanelProps { const COMPLETION_DEBOUNCE_MS = 300; -function parseVariableNames(uriTemplate: string): string[] { - const names: string[] = []; - const regex = /\{(\w+)\}/g; - let match: RegExpExecArray | null; - - while ((match = regex.exec(uriTemplate)) !== null) { - names.push(match[1]); - } - - return names; -} - -function resolveUri( - uriTemplate: string, - variables: Record, -): string { - return uriTemplate.replace(/\{(\w+)\}/g, (_, key: string) => variables[key]); -} - -function previewUri( - uriTemplate: string, - variables: Record, -): string { - return uriTemplate.replace(/\{(\w+)\}/g, (match, key: string) => - variables[key]?.length > 0 ? variables[key] : match, - ); -} - const HeaderRow = Group.withProps({ justify: "space-between", wrap: "nowrap", @@ -109,7 +86,7 @@ export function ResourceTemplatePanel({ const { name, title, uriTemplate, description, annotations } = template; const variableNames = useMemo( - () => parseVariableNames(uriTemplate), + () => templateVariableNames(uriTemplate), [uriTemplate], ); @@ -232,13 +209,21 @@ export function ResourceTemplatePanel({ void runCompletion(varName, value, buildContext(varName)); } - const canSubmit = variableNames.every((n) => variables[n]?.length > 0); + const allFilled = variableNames.every((n) => variables[n]?.length > 0); + // `null` means the template or a value could not be expanded (a malformed + // template, or a value past the SDK's length ceiling). Withhold the request + // rather than send a URI we know is wrong. + const expandedUri = allFilled ? expandTemplate(uriTemplate, variables) : null; + const canSubmit = expandedUri !== null; function handleSubmit() { - onReadResource(resolveUri(uriTemplate, variables)); + /* v8 ignore next -- unreachable: the button is disabled unless + `expandedUri` is non-null. */ + if (expandedUri === null) return; + onReadResource(expandedUri); } - const preview = previewUri(uriTemplate, variables); + const preview = previewTemplate(uriTemplate, variables); return ( diff --git a/clients/web/src/test/core/uri/uriTemplate.test.ts b/clients/web/src/test/core/uri/uriTemplate.test.ts new file mode 100644 index 000000000..440951c76 --- /dev/null +++ b/clients/web/src/test/core/uri/uriTemplate.test.ts @@ -0,0 +1,470 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { + expandTemplate, + previewTemplate, + templateVariableNames, +} from "@inspector/core/uri/uriTemplate.js"; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** Suppress the console.warn a malformed template emits. */ +function silenceWarn() { + return vi.spyOn(console, "warn").mockImplementation(() => {}); +} + +describe("templateVariableNames", () => { + it("finds a simple variable", () => { + expect(templateVariableNames("foobar://events/{topic}")).toEqual(["topic"]); + }); + + it("finds a variable inside a query expression", () => { + expect(templateVariableNames("foobar://events{?topic}")).toEqual(["topic"]); + }); + + it.each([ + ["reserved", "x://{+path}", ["path"]], + ["fragment", "x://a{#frag}", ["frag"]], + ["path segment", "x://a{/seg}", ["seg"]], + ["label", "x://a{.ext}", ["ext"]], + ["query continuation", "x://a?x=1{&y}", ["y"]], + ])("finds a variable in a %s expression", (_label, template, expected) => { + expect(templateVariableNames(template)).toEqual(expected); + }); + + it("finds every variable in a multi-variable expression", () => { + expect(templateVariableNames("foobar://e{?a,b}")).toEqual(["a", "b"]); + }); + + it("returns a repeated name once", () => { + expect(templateVariableNames("x://{a}/{b}/{a}")).toEqual(["a", "b"]); + }); + + // The SDK's UriTemplate does not implement prefix modifiers: it treats + // `topic:3` as the whole variable name rather than a 3-char prefix of + // `topic`. Pinned here because that boundary is what the panel renders as a + // field label, and it is shared with the TUI and readResourceFromTemplate. + it("treats a prefix modifier as part of the variable name", () => { + expect(templateVariableNames("x://{topic:3}")).toEqual(["topic:3"]); + }); + + it("returns an empty list for a template with no expressions", () => { + expect(templateVariableNames("foobar://events")).toEqual([]); + }); + + it("returns an empty list for a malformed template", () => { + const warn = silenceWarn(); + expect(templateVariableNames("x://{unterminated")).toEqual([]); + expect(warn).toHaveBeenCalled(); + }); + + // The SDK *accepts* these: `new UriTemplate("x://{}")` parses, reports no + // variables, and expands to `x://`. Left alone, the panel would render no + // inputs, find its "all filled" check vacuously true, and submit a URI that + // is not the template the server advertised. + it.each([ + ["no name", "x://{}"], + ["a blank name", "x://{ }"], + ["only a separator", "x://{,}"], + ["a missing member", "x://{a,}"], + ["an operator and no name", "x://{?}"], + ])("rejects an expression with %s", (_label, template) => { + silenceWarn(); + expect(templateVariableNames(template)).toEqual([]); + expect(expandTemplate(template, { a: "1" })).toBeNull(); + }); +}); + +describe("expandTemplate", () => { + it("percent-encodes a reserved character in a simple variable", () => { + expect( + expandTemplate("foobar://events/{topic}", { topic: "foo/bar" }), + ).toBe("foobar://events/foo%2Fbar"); + }); + + it("expands a query expression", () => { + expect( + expandTemplate("foobar://events{?topic}", { topic: "foo/bar" }), + ).toBe("foobar://events?topic=foo%2Fbar"); + }); + + it.each([ + ["?", "x://e/%3F"], + ["#", "x://e/%23"], + ["%", "x://e/%25"], + [" ", "x://e/%20"], + ["café", "x://e/caf%C3%A9"], + ])("encodes %j", (value, expected) => { + expect(expandTemplate("x://e/{v}", { v: value })).toBe(expected); + }); + + it("leaves a reserved-expansion value's sub-delimiters intact", () => { + expect(expandTemplate("x://{+path}", { path: "a/b" })).toBe("x://a/b"); + }); + + // The SDK's own encoding is not RFC-conformant, and every case below is + // reachable from a plain text input. Simple and query expansions go through + // `encodeURIComponent`, which leaves these five bare; RFC 6570 §3.2.1 encodes + // everything outside the *unreserved* set. + describe("RFC 3986 character sets", () => { + it.each([ + ["!", "%21"], + ["*", "%2A"], + ["'", "%27"], + ["(", "%28"], + [")", "%29"], + ])("encodes %j in a simple expansion", (value, encoded) => { + expect(expandTemplate("x://{v}", { v: value })).toBe(`x://${encoded}`); + }); + + it("encodes them in a query expansion too", () => { + expect(expandTemplate("x://e{?v}", { v: "!*'()" })).toBe( + "x://e?v=%21%2A%27%28%29", + ); + }); + + // `+` and `#` use `encodeURI`, which escapes `[` and `]` — but those are + // gen-delims, exactly what reserved expansion exists to pass through. + it.each([ + ["reserved", "x://{+v}", "x://[a]"], + ["fragment", "x://{#v}", "x://#[a]"], + ])("keeps gen-delims in a %s expansion", (_label, template, expected) => { + expect(expandTemplate(template, { v: "[a]" })).toBe(expected); + }); + + it("keeps every reserved character under the + operator", () => { + const reserved = ":/?#[]@!$&'()*+,;="; + expect(expandTemplate("x://{+v}", { v: reserved })).toBe( + `x://${reserved}`, + ); + }); + + // A well-formed triplet is already pct-encoded; the SDK re-encoded it to + // `%2541`, which changes the value. + it("passes an existing pct-triplet through a reserved expansion", () => { + expect(expandTemplate("x://{+v}", { v: "%41" })).toBe("x://%41"); + }); + + it("encodes a bare percent that is not a triplet", () => { + expect(expandTemplate("x://{+v}", { v: "100%" })).toBe("x://100%25"); + expect(expandTemplate("x://{+v}", { v: "%zz" })).toBe("x://%25zz"); + }); + + // A simple expansion has no triplet passthrough — the value is literal. + it("encodes a percent in a simple expansion", () => { + expect(expandTemplate("x://{v}", { v: "%41" })).toBe("x://%2541"); + }); + + it("encodes a code point outside the BMP as its UTF-8 octets", () => { + expect(expandTemplate("x://{v}", { v: "😀" })).toBe("x://%F0%9F%98%80"); + }); + + // An unpaired surrogate has no UTF-8 encoding and makes + // `encodeURIComponent` throw `URIError`. Encoding runs outside the SDK's + // try/catch — and, through the preview, during render — so it must be + // reported rather than thrown. + it.each([ + ["a lone high surrogate", "\uD800"], + ["a lone low surrogate", "\uDC00"], + ["a surrogate among valid text", "ok\uD800ok"], + ])("declines %s instead of throwing", (_label, value) => { + silenceWarn(); + expect(() => expandTemplate("x://{v}", { v: value })).not.toThrow(); + expect(expandTemplate("x://{v}", { v: value })).toBeNull(); + }); + + it("previews a lone surrogate as the raw template instead of throwing", () => { + silenceWarn(); + expect(previewTemplate("x://{v}", { v: "\uD800" })).toBe("x://{v}"); + }); + }); + + // RFC 6570 allows one name in expressions with different operators, and each + // occurrence encodes per *its* operator. A single value keyed by name cannot + // express that, so each occurrence gets its own rendering. + describe("a name repeated under different operators", () => { + // `-` as the literal separator, so the slashes in the output are only ever + // the ones the expansion produced. + it("encodes each occurrence per its own operator", () => { + expect(expandTemplate("x://{+a}-{a}", { a: "/" })).toBe("x:///-%2F"); + }); + + it("still offers the repeated name as one field", () => { + expect(templateVariableNames("x://{+a}-{a}")).toEqual(["a"]); + }); + + it("handles a simple/query pair", () => { + expect(expandTemplate("x://{+a}{?a}", { a: "a/b" })).toBe( + "x://a/b?a=a%2Fb", + ); + }); + + it("handles a fragment/simple pair", () => { + expect(expandTemplate("x://{a}{#a}", { a: "[x]" })).toBe( + "x://%5Bx%5D#[x]", + ); + }); + }); + + // RFC 6570 distinguishes an *undefined* variable from one defined as the + // empty string, and the expansion must not collapse the two. + it("keeps a variable defined as the empty string", () => { + expect(expandTemplate("foobar://events{?topic}", { topic: "" })).toBe( + "foobar://events?topic=", + ); + }); + + it("omits a variable that is absent from the values entirely", () => { + expect(expandTemplate("foobar://e{?a,b}", { a: "1" })).toBe( + "foobar://e?a=1", + ); + }); + + // The SDK's multi-name branch skips both encoding and the operator, so these + // expressions are rewritten to a single array-valued variable, which takes + // the branch that applies them. + // + // Covered here rather than in the integration suite on purpose: the SDK's + // `UriTemplate.match` cannot match a multi-name expression *either* (it + // returns null for every URI), so an SDK-based server can never route one and + // there is no round trip to drive. What the client can do is emit the + // spec-correct URI, which is what a conforming server needs — so that is what + // these assert. + describe("multi-name expressions", () => { + it.each([ + ["simple", "x://{a,b}", "x://foo%2Fbar,x%20y"], + // `#`, like `+`, is a *reserved* expansion — `/` survives, a space does not. + ["fragment", "x://e{#a,b}", "x://e#foo/bar,x%20y"], + ["label", "x://e{.a,b}", "x://e.foo%2Fbar.x%20y"], + ["path segment", "x://e{/a,b}", "x://e/foo%2Fbar/x%20y"], + ])("encodes and applies the operator for a %s group", (_l, t, expected) => { + expect(expandTemplate(t, { a: "foo/bar", b: "x y" })).toBe(expected); + }); + + it("preserves reserved characters under the + operator", () => { + expect(expandTemplate("x://{+a,b}", { a: "foo/bar", b: "x y" })).toBe( + "x://foo/bar,x%20y", + ); + }); + + it("still expands a multi-name query expression correctly", () => { + expect(expandTemplate("x://e{?a,b}", { a: "foo/bar", b: "x y" })).toBe( + "x://e?a=foo%2Fbar&b=x%20y", + ); + }); + + it("drops an undefined member from the group", () => { + expect(expandTemplate("x://{a,b}", { b: "two" })).toBe("x://two"); + }); + + it("omits the whole expression when no member is defined", () => { + expect(expandTemplate("x://e{#a,b}", {})).toBe("x://e"); + }); + + // The projection copies every supplied variable, so a caller passing a key + // equal to the generated synthetic name must not have it stand in for the + // group — the SDK ignores undeclared variables, and so must this. + it("ignores a caller value keyed like the synthetic group name", () => { + expect( + expandTemplate("x://{a,b}", { __inspectorGroup0__: "injected" }), + ).toBe("x://"); + }); + + // `variableNames` strips a trailing `*`, so the form stores `a`. A group + // that kept `a*` would look up a key the form never sets and silently drop + // a filled value. + it("matches the SDK's name for an exploded member", () => { + expect(templateVariableNames("x://e{/a*,b}")).toEqual(["a", "b"]); + expect(expandTemplate("x://e{/a*,b}", { a: "one", b: "two" })).toBe( + "x://e/one/two", + ); + }); + + // A template may legitimately declare a variable named like the synthetic + // one; the rewrite must not overwrite the user's value with the group's. + it("does not collide with a variable named like the synthetic one", () => { + expect( + expandTemplate("x://{a,b}/{__inspectorGroup0__}", { + a: "one", + b: "two", + __inspectorGroup0__: "mine", + }), + ).toBe("x://one,two/mine"); + }); + }); + + // The SDK does not implement `;`: it reads `{;a}` as a variable literally + // named ";a" and expands it to the bare value, dropping the required `;a=`. + // No arrangement of its branches produces the right output, so decline rather + // than hand back a URI known to be invalid. + describe("the unsupported ; (path-parameter) operator", () => { + it.each([ + ["single-name", "x://e{;a}"], + ["multi-name", "x://e{;a,b}"], + ])("returns null for a %s expression", (_label, template) => { + silenceWarn(); + expect(expandTemplate(template, { a: "1", b: "2" })).toBeNull(); + }); + + it("previews it as the template the server declared", () => { + expect(previewTemplate("x://e{;a,b}", { a: "1" })).toBe("x://e{;a,b}"); + }); + + it("does not mistake a literal semicolon for the operator", () => { + expect(expandTemplate("x://e;q/{a}", { a: "1" })).toBe("x://e;q/1"); + }); + }); + + it("returns null when the template cannot be parsed", () => { + silenceWarn(); + expect(expandTemplate("x://{unterminated", { a: "1" })).toBeNull(); + }); + + // The rewrite replaces variable names with short synthetics, shrinking both + // the template and every name — so validating only the rewritten text would + // accept a template the SDK rejects, and disagree with + // `templateVariableNames`, which parses the original. + it.each([ + ["an over-long variable name", `x://{${"n".repeat(1_000_001)}}`], + ["an over-long template", `x://${"p".repeat(1_000_001)}/{a}`], + ])("returns null for %s, as discovery does", (_label, template) => { + silenceWarn(); + expect(templateVariableNames(template)).toEqual([]); + expect(expandTemplate(template, { a: "1" })).toBeNull(); + }); + + // Parsing succeeding does not mean expanding will — the SDK checks its + // per-value length ceiling at expansion time. + it("returns null when a value cannot be expanded", () => { + silenceWarn(); + expect(expandTemplate("x://{a}", { a: "z".repeat(1_000_001) })).toBeNull(); + }); +}); + +describe("previewTemplate", () => { + // The preview's own notion, deliberately different from expandTemplate's: a + // text input cannot express "defined but empty", so within the preview an + // empty string means "not entered yet". + it("treats an empty string as unfilled rather than as an empty expansion", () => { + expect(expandTemplate("x://e{?t}", { t: "" })).toBe("x://e?t="); + expect(previewTemplate("x://e{?t}", { t: "" })).toBe("x://e?t={t}"); + }); + + it("shows an unfilled simple variable as its expression", () => { + expect(previewTemplate("foobar://events/{topic}", { topic: "" })).toBe( + "foobar://events/{topic}", + ); + }); + + it("shows an unfilled query variable as its expression", () => { + expect(previewTemplate("foobar://events{?topic}", { topic: "" })).toBe( + "foobar://events?topic={topic}", + ); + }); + + it("shows a filled value encoded exactly as it will be sent", () => { + expect( + previewTemplate("foobar://events/{topic}", { topic: "foo/bar" }), + ).toBe("foobar://events/foo%2Fbar"); + }); + + it("mixes filled and unfilled variables", () => { + expect(previewTemplate("x://{a}/{b}", { a: "one", b: "" })).toBe( + "x://one/{b}", + ); + }); + + it("substitutes every occurrence of a repeated unfilled name", () => { + expect(previewTemplate("x://{a}/{b}/{a}", { a: "", b: "2" })).toBe( + "x://{a}/2/{a}", + ); + }); + + // The placeholder token for variable 1 must not be a prefix of the one for + // variable 11, or substituting the former would corrupt the latter. + it("keeps double-digit variable positions distinct", () => { + const names = Array.from({ length: 12 }, (_, i) => `v${i}`); + const template = `x://${names.map((n) => `{${n}}`).join("/")}`; + const empty = Object.fromEntries(names.map((n) => [n, ""])); + expect(previewTemplate(template, empty)).toBe(template); + }); + + // A prefix modifier does not truncate here — the SDK folds it into the + // variable name (see the discovery test above) — so the sentinel survives + // expansion intact and the placeholder is restored like any other. + it("restores the placeholder for a variable carrying a prefix modifier", () => { + expect(previewTemplate("x://{topic:3}", {})).toBe("x://{topic:3}"); + expect(previewTemplate("x://{?topic:3}", {})).toBe( + "x://?topic:3={topic:3}", + ); + }); + + // A filled value that happens to be the placeholder token must not be + // rewritten into a `{name}` — that would make the preview disagree with the + // URI actually submitted. + it("does not mistake a filled value for its own placeholder", () => { + expect( + previewTemplate("x://{a}/{b}", { a: "zzInspectorUnfilledzz1zz", b: "" }), + ).toBe("x://zzInspectorUnfilledzz1zz/{b}"); + }); + + it("does not mistake the template's own literal text for a placeholder", () => { + expect(previewTemplate("x://zzInspectorUnfilledzz0zz/{a}", { a: "" })).toBe( + "x://zzInspectorUnfilledzz0zz/{a}", + ); + }); + + it("shows a placeholder per member of a multi-name group", () => { + expect(previewTemplate("x://{a,b}", { a: "one", b: "" })).toBe( + "x://one,{b}", + ); + }); + + it("returns the raw template when it cannot be parsed", () => { + silenceWarn(); + expect(previewTemplate("x://{unterminated", {})).toBe("x://{unterminated"); + }); + + // Must not throw out of render — the panel expands its preview while + // rendering, so an escaping error would unmount the panel. + it("returns the raw template when a value cannot be expanded", () => { + silenceWarn(); + expect(previewTemplate("x://{a}", { a: "z".repeat(1_000_001) })).toBe( + "x://{a}", + ); + }); + + // A template holding the token followed by a long run of `z`s used to make + // every extended candidate collide in turn, rescanning the whole input each + // time. The base is now cleared in a single pass. + it("resolves a padded-run collision without rescanning", () => { + const template = `x://zzInspectorUnfilledzz${"z".repeat(5000)}/{a}`; + expect(previewTemplate(template, { a: "" })).toBe(template); + }); + + // Substitution is one regex pass over the expansion rather than one pass per + // variable — the SDK accepts a 1 MB template with up to 10,000 expressions, + // and a per-variable rescan is O(variables × length) during render. This also + // covers the multi-digit index boundary at scale. + it("substitutes many variables correctly", () => { + const names = Array.from({ length: 200 }, (_, i) => `v${i}`); + const template = `x://${names.map((n) => `{${n}}`).join("/")}`; + const values = Object.fromEntries( + names.map((n, i) => [n, i % 2 === 0 ? `val${i}` : ""]), + ); + const expected = `x://${names + .map((n, i) => (i % 2 === 0 ? `val${i}` : `{${n}}`)) + .join("/")}`; + expect(previewTemplate(template, values)).toBe(expected); + }); + + // The token starts and ends with `zz`, so it can overlap itself: this literal + // holds a second occurrence at index 19 whose trailing run is the longer one. + // A scan advancing by the token's length would miss it and pick a colliding + // placeholder, rewriting literal URI text into `{a}`. + it("measures an overlapping occurrence of the token", () => { + const template = "x://zzInspectorUnfilledzzInspectorUnfilledzzz0zz/{a}"; + expect(previewTemplate(template, { a: "" })).toBe(template); + }); +}); diff --git a/clients/web/src/test/integration/mcp/rfc6570-templates.test.ts b/clients/web/src/test/integration/mcp/rfc6570-templates.test.ts new file mode 100644 index 000000000..555395fc0 --- /dev/null +++ b/clients/web/src/test/integration/mcp/rfc6570-templates.test.ts @@ -0,0 +1,174 @@ +import { describe, it, expect, afterEach } from "vitest"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { createTransportNode } from "@inspector/core/mcp/node/transport.js"; +import { + expandTemplate, + templateVariableNames, +} from "@inspector/core/uri/uriTemplate.js"; +import { + createTestServerHttp, + type TestServerHttp, + createTestServerInfo, + loadConfig, + resolveConfig, +} from "@modelcontextprotocol/inspector-test-server"; + +/** + * Live coverage of `test-servers/configs/rfc6570-templates-http.json` — the + * documented manual reproduction for #1919. + * + * The helper's unit tests assert what `expandTemplate` *produces*; they cannot + * assert that the produced URI is what a spec-compliant server *accepts*. That + * second half is the whole bug: the old string substitution emitted a URI the + * Inspector was perfectly happy with and the server rejected. So this test + * drives both directions against a real server over a real transport — the + * encoded URI must resolve, and the unencoded one the old code produced must + * still be refused, so a regression cannot pass by loosening the server. + * + * The server is built by **resolving the checked-in config** rather than by + * calling the fixture factory, so a misspelt preset name in `preset-registry.ts` + * (or a config naming a preset that no longer exists) fails here instead of + * only when someone runs the repro by hand. + */ +describe("RFC 6570 resource templates over the wire (#1919)", () => { + let client: InspectorClient | null = null; + let server: TestServerHttp | null = null; + + const configPath = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../../../../../../test-servers/configs/rfc6570-templates-http.json", + ); + + afterEach(async () => { + if (client) { + try { + await client.disconnect(); + } catch { + // ignore + } + client = null; + } + if (server) { + try { + await server.stop(); + } catch { + // ignore + } + server = null; + } + }); + + /** + * Boot the showcase config. The harness picks the port rather than using the + * config's fixed one, so this cannot collide with a showcase server someone + * is running by hand. + */ + async function connectToShowcase(): Promise { + const resolved = resolveConfig(loadConfig(configPath)); + const started = createTestServerHttp({ + serverInfo: createTestServerInfo("rfc6570-templates-test", "1.0.0"), + resourceTemplates: resolved.resourceTemplates, + }); + await started.start(); + server = started; + + const connected = new InspectorClient( + { type: "streamable-http", url: started.url }, + { environment: { transport: createTransportNode } }, + ); + await connected.connect(); + client = connected; + return connected; + } + + /** + * Read the sole content block as JSON. `contents[]` is a text-or-blob union, + * so narrow rather than cast — a fixture that started returning a blob should + * fail here with a clear message, not at `JSON.parse(undefined)`. + */ + async function readJson( + connected: InspectorClient, + uri: string, + ): Promise { + const { result } = await connected.readResource(uri); + const [content] = result.contents; + expect(content).toBeDefined(); + if (!("text" in content)) { + throw new Error(`expected a text content block for ${uri}`); + } + return JSON.parse(content.text); + } + + it("resolves the preset the config names", () => { + const resolved = resolveConfig(loadConfig(configPath)); + expect(resolved.resourceTemplates?.map((t) => t.uriTemplate)).toEqual([ + "foobar://events/{topic}", + "foobar://events{?topic}", + ]); + }); + + it("advertises both templates, including the query expression", async () => { + const connected = await connectToShowcase(); + const { resourceTemplates } = await connected.listAllResourceTemplates(); + const byName = Object.fromEntries( + resourceTemplates.map((t) => [t.name, t.uriTemplate]), + ); + expect(byName["events-by-path"]).toBe("foobar://events/{topic}"); + expect(byName["events-by-query"]).toBe("foobar://events{?topic}"); + }); + + it("discovers a variable in each expression form", () => { + expect(templateVariableNames("foobar://events/{topic}")).toEqual(["topic"]); + expect(templateVariableNames("foobar://events{?topic}")).toEqual(["topic"]); + }); + + it("reads a reserved-character value through the simple expression", async () => { + const connected = await connectToShowcase(); + const uri = expandTemplate("foobar://events/{topic}", { topic: "foo/bar" }); + expect(uri).toBe("foobar://events/foo%2Fbar"); + if (uri === null) throw new Error("unreachable — asserted above"); + + expect(await readJson(connected, uri)).toEqual({ + topic: "foo%2Fbar", + matchedUri: uri, + }); + }); + + it("reads through the query expression", async () => { + const connected = await connectToShowcase(); + const uri = expandTemplate("foobar://events{?topic}", { topic: "weather" }); + expect(uri).toBe("foobar://events?topic=weather"); + if (uri === null) throw new Error("unreachable — asserted above"); + + expect(await readJson(connected, uri)).toMatchObject({ topic: "weather" }); + }); + + // `readResourceFromTemplate` is the path the TUI and CLI submit through, and + // it used to call the SDK directly — so the same template could resolve one + // way in the web panel and another here. Both now route through + // `core/uri/uriTemplate`; this asserts the client-level path end to end. + it("encodes correctly through readResourceFromTemplate", async () => { + const connected = await connectToShowcase(); + const invocation = await connected.readResourceFromTemplate( + "foobar://events/{topic}", + { topic: "foo/bar" }, + ); + expect(invocation.expandedUri).toBe("foobar://events/foo%2Fbar"); + // Same URI the panel's `expandTemplate` produces — the two cannot drift. + expect(invocation.expandedUri).toBe( + expandTemplate("foobar://events/{topic}", { topic: "foo/bar" }), + ); + }); + + // The old behavior, pinned from the server's side. If this ever starts + // succeeding, the repro server has stopped reproducing and the test above + // would keep passing while proving nothing. + it("rejects the unencoded URI the old string substitution produced", async () => { + const connected = await connectToShowcase(); + await expect( + connected.readResource("foobar://events/foo/bar"), + ).rejects.toThrow(/not found/i); + }); +}); diff --git a/clients/web/vite.config.ts b/clients/web/vite.config.ts index a31c01693..178e3bbe7 100644 --- a/clients/web/vite.config.ts +++ b/clients/web/vite.config.ts @@ -212,6 +212,7 @@ export default defineConfig(({ command }) => { path.join(repoRoot, "core/storage/**/*.{ts,tsx}"), path.join(repoRoot, "core/logging/**/*.{ts,tsx}"), path.join(repoRoot, "core/node/**/*.{ts,tsx}"), + path.join(repoRoot, "core/uri/**/*.{ts,tsx}"), ], exclude: [ "**/*.stories.{ts,tsx}", diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index a635582d0..e987f5344 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -190,7 +190,7 @@ import { convertToolParameters, convertPromptArguments, } from "../json/jsonUtils.js"; -import { UriTemplate } from "@modelcontextprotocol/client"; +import { expandTemplate } from "../uri/uriTemplate.js"; import { InspectorClientEventTarget, type TaskWithOptionalCreatedAt, @@ -4971,15 +4971,14 @@ export class InspectorClient extends InspectorClientEventTarget { const uriTemplateString = uriTemplate; - // Expand the template's uriTemplate using the provided params - let expandedUri: string; - try { - const uriTemplate = new UriTemplate(uriTemplateString); - expandedUri = uriTemplate.expand(params); - } catch (error) { + // Expand through the shared RFC 6570 helper rather than the SDK directly, + // so this path and the web panel's cannot resolve the same template to + // different URIs (#1919). `null` means the template or a value could not be + // expanded correctly — better to fail loudly than to read a wrong URI. + const expandedUri = expandTemplate(uriTemplateString, params); + if (expandedUri === null) { throw new Error( - `Failed to expand URI template "${uriTemplate}": ${error instanceof Error ? error.message : String(error)}`, - { cause: error }, + `Failed to expand URI template "${uriTemplateString}": the template or one of its values cannot be expanded per RFC 6570.`, ); } diff --git a/core/uri/uriTemplate.ts b/core/uri/uriTemplate.ts new file mode 100644 index 000000000..2adb37785 --- /dev/null +++ b/core/uri/uriTemplate.ts @@ -0,0 +1,552 @@ +/** + * RFC 6570 URI Template discovery, expansion, and preview — shared by every + * client so a template cannot resolve differently depending on where it is + * driven from. + * + * The web client used to discover variables with `/\{(\w+)\}/g` and expand them + * with a plain `String.replace`. That only ever saw simple expressions — a + * query expression like `{?topic}` produced no input at all — and it inserted + * values verbatim, so a `topic` of `foo/bar` silently became a second path + * segment instead of `foo%2Fbar` (#1919). + * + * These wrap the SDK's `UriTemplate`, keeping what it gets right — the parse, + * the operators, the separators, which expressions appear at all — and + * correcting where it departs from RFC 6570: + * + * - **value encoding** is done here instead, against the explicit RFC 3986 + * character sets; the SDK's `encodeURIComponent` / `encodeURI` leave `!*'()` + * bare, escape the gen-delims `[` `]` that reserved expansion should pass + * through, and double-encode an existing pct-triplet (see `encodeValue`); + * - **multi-name expressions** (`{a,b}`) take a branch that skips both encoding + * and the operator, and a **name repeated under different operators** cannot + * be encoded per-occurrence when values are looked up by name — both are + * handled by rewriting each non-query expression into its own synthetic + * variable (see `rewriteExpressions`); + * - two shapes it accepts but mishandles — an expression declaring no variable, + * and the unimplemented `;` operator — are **declined** rather than expanded + * into a knowingly wrong URI (see `unsupportedReason`). + * + * Living in `core/` is what makes the correction + * uniform: the web panel, the TUI's form builder, and + * `InspectorClient.readResourceFromTemplate` all route through here rather than + * calling the SDK directly, so web, CLI, and TUI agree on what a template's + * variables are and on how a value is encoded. + * + * Neither helper throws: a template the SDK rejects, or a value it refuses to + * expand, yields `null` from `expandTemplate` (so the caller can withhold the + * request) and the raw template from `previewTemplate` (which runs during + * render, where a throw would take the panel down with it). + */ +import { UriTemplate } from "@modelcontextprotocol/client"; + +/** + * Parses `uriTemplate` once so a caller can discover, expand, and preview + * without re-parsing. Returns `null` when the template is malformed (the SDK + * throws on, e.g., an unterminated expression) so callers can degrade to + * rendering the raw string rather than crashing the panel. + */ +function parseTemplate(uriTemplate: string): UriTemplate | null { + try { + return new UriTemplate(uriTemplate); + } catch (error) { + console.warn(`Failed to parse URI template "${uriTemplate}":`, error); + return null; + } +} + +/** Any `{…}` expression in the template, with its body captured. */ +const ANY_EXPRESSION = /\{([^{}]*)\}/g; + +/** + * Why this template cannot be handled, or `null` if it can. + * + * Checked on the template *as the server declared it*, before any rewriting — + * the multi-name grouping would otherwise mask an empty member by folding + * `{a,}` into a synthetic single-name expression. + * + * Two cases, both of which the SDK accepts and mishandles rather than + * rejecting: + * + * - **An expression declaring no variable.** `new UriTemplate("x://{}")` + * parses, reports no variable names, and expands to `x://`. That defeats the + * null-on-malformed contract in the most dangerous way: the panel renders no + * inputs, so its "every variable is filled" check is vacuously true and it + * submits a URI that is not the template the server advertised. `{ }`, + * `{,}`, and `{a,}` are the same defect with some members missing. + * - **The `;` path-parameter operator**, which the SDK does not implement: it + * reads `{;a}` as a variable literally named `";a"` and expands it to the + * bare value, dropping the required `;a=`. No arrangement of its own branches + * produces the right output. + */ +function unsupportedReason(uriTemplate: string): string | null { + for (const [, body] of uriTemplate.matchAll(ANY_EXPRESSION)) { + if (body.startsWith(";")) { + return "the ; (path-parameter) operator is unsupported"; + } + const names = body.replace(/^[+#./?&]/, "").split(","); + if (names.some((name) => name.trim().length === 0)) { + return "an expression declares no variable"; + } + } + return null; +} + +/** + * Expand, converting a throw into `null`. + * + * Parsing succeeding does not mean expanding will: the SDK also enforces a + * 1,000,000-character ceiling per *value*, which is checked at expansion time. + * The inputs have no matching limit, so a paste can reach it — and the preview + * expands during render, where an escaping throw unmounts the panel instead of + * showing a problem with the value. + */ +function tryExpand( + template: UriTemplate, + values: Record, +): string | null { + try { + return template.expand(values); + } catch (error) { + console.warn("Failed to expand URI template:", error); + return null; + } +} + +/** + * The variable names declared by `uriTemplate`, in declaration order — + * including those inside non-simple expressions (`{?topic}`, `{+path}`, + * `{#frag}`, `{/seg*}`, …), which the old regex missed entirely. + * + * One boundary is inherited from the SDK rather than chosen here: it does not + * implement prefix modifiers, so `{topic:3}` yields the name `"topic:3"` and + * expands without truncating. That behavior is shared with the TUI's form + * builder and `readResourceFromTemplate`, which parse through the same class. + */ +export function templateVariableNames(uriTemplate: string): string[] { + if (warnIfUnsupported(uriTemplate)) return []; + // Parsed only to reject what the SDK rejects (an unterminated expression); + // the names themselves come from the same scan the expander uses, so the + // form's fields and its lookups are one list by construction. + if (!parseTemplate(uriTemplate)) return []; + return rewriteExpressions(uriTemplate).order; +} + +/** Warn once with the reason, and report whether the template is unsupported. */ +function warnIfUnsupported(uriTemplate: string): boolean { + const reason = unsupportedReason(uriTemplate); + if (reason === null) return false; + console.warn(`Cannot handle URI template "${uriTemplate}": ${reason}.`); + return true; +} + +/** + * The name the SDK will parse out of an expression member, so the values handed + * back to it are keyed the way the form stores them. + * + * It strips a trailing explode modifier (`{a*}` → `a`) but *keeps* a prefix + * modifier (`{a:3}` → `a:3`, see `templateVariableNames`). Mirroring it exactly + * is the point: normalizing differently would silently drop a filled value. + */ +function memberName(raw: string): string { + return raw.trim().replace(/\*$/, ""); +} + +/** + * Name for the synthetic variable a rewritten group expands from, chosen so it + * cannot collide with a variable the template already declares — otherwise a + * template like `x://{a,b}/{__inspectorGroup0__}` would have the group's value + * overwrite the user's own, emitting the group twice. + */ +function groupName(prefix: string, index: number): string { + return `${prefix}${index}__`; +} + +const GROUP_PREFIX = "__inspectorGroup"; + +/** One expression of the template, as rewritten for the SDK. */ +interface Slot { + /** The real variable names it declares, in order. */ + names: string[]; + /** Its RFC 6570 operator (`""` for a simple expression). */ + operator: string; +} + +interface RewrittenTemplate { + /** The rewritten template string, safe to hand to the SDK. */ + text: string; + /** Synthetic variable name → the expression it stands for. */ + slots: Map; + /** Names left under their own key, because a query expression emits them. */ + queryNames: Set; + /** Every declared name, in template order, deduplicated. */ + order: string[]; +} + +/** + * Rewrite each **non-query** expression to a single synthetic variable. + * + * Two problems this solves at once, both stemming from the SDK looking values up + * by variable name: + * + * - a *multi-name* expression takes a branch that returns the values joined raw, + * skipping both `encodeValue` and the operator; giving the rewritten + * expression an **array** value routes it through the single-name branch, + * which applies them (`x://{a,b}` → `x://{__inspectorGroup0__}`); + * - a name **repeated under different operators** — `x://{+a}/{a}` — must be + * encoded differently per occurrence (`/` preserved by `+`, `%2F` by the + * simple expansion), which one value keyed by name cannot express. A synthetic + * name per *occurrence* gives each its own value, hence its own encoding. + * + * Query expressions are deliberately left alone: `?`/`&` emit the variable's + * **name** into the URI, so renaming `{?topic}` would produce `?__inspectorGroup0__=`. + * They need no rewrite anyway — their branch already encodes correctly, and + * `?` and `&` share one encoding, so every query occurrence of a name can share + * a single value keyed by that name. + */ +function rewriteExpressions(uriTemplate: string): RewrittenTemplate { + const slots = new Map(); + const queryNames = new Set(); + // A Set, so the order is the template's and each name appears once. + const order = new Set(); + // The prefix must not occur in the template, or a template declaring a + // variable of that name would have the slot overwrite the user's value. + const prefix = padPastCollisions(GROUP_PREFIX, "_", uriTemplate); + const text = uriTemplate.replace(ANY_EXPRESSION, (match, body: string) => { + const operator = /^[+#./?&]/.test(body) ? body[0] : ""; + const names = body.slice(operator.length).split(",").map(memberName); + for (const name of names) order.add(name); + if (operator === "?" || operator === "&") { + for (const name of names) queryNames.add(name); + return match; + } + const synthetic = groupName(prefix, slots.size); + slots.set(synthetic, { names, operator }); + return `{${operator}${synthetic}}`; + }); + return { text, slots, queryNames, order: [...order] }; +} + +/** RFC 3986 §2.3 unreserved: the set never percent-encoded, under any operator. */ +const UNRESERVED = /[A-Za-z0-9\-._~]/; + +/** RFC 3986 §2.2 reserved (gen-delims + sub-delims), allowed by `+` and `#`. */ +const RESERVED = /[:/?#[\]@!$&'()*+,;=]/; + +/** `encodeURIComponent` leaves these alone; RFC 6570 requires them encoded. */ +const UNDER_ENCODED_BY_ENCODE_URI_COMPONENT = /[!'()*]/g; + +/** + * Percent-encode `value` for an expression using `operator`, per RFC 6570 §3.2.1. + * + * The SDK's own encoding is not RFC-conformant in three ways, all verified + * against the pinned version, and all reachable from a plain text input: + * + * - simple and query expansions use `encodeURIComponent`, which leaves `!`, + * `*`, `'`, `(` and `)` bare — `{v}` with `!` gives `x://!` where the RFC + * requires `x://%21`; + * - `+` and `#` use `encodeURI`, which escapes `[` and `]` — but those are + * gen-delims, which reserved expansion is specifically meant to pass through; + * - `+` and `#` also re-encode an existing pct-triplet, turning `%41` into + * `%2541`, where the RFC keeps a well-formed triplet as-is. + * + * So this module owns value encoding and leaves *structure* — operators, + * separators, which expressions appear at all — to the SDK. See + * `expandWithPlaceholders` for how the two are combined. + */ +function encodeValue(value: string, operator: string): string | null { + const allowReserved = operator === "+" || operator === "#"; + let out = ""; + for (let at = 0; at < value.length; ) { + const char = value[at]; + if ( + allowReserved && + char === "%" && + /^[0-9A-Fa-f]{2}$/.test(value.slice(at + 1, at + 3)) + ) { + // A well-formed triplet is already pct-encoded; reserved expansion keeps it. + out += value.slice(at, at + 3); + at += 3; + continue; + } + if (UNRESERVED.test(char) || (allowReserved && RESERVED.test(char))) { + out += char; + at += 1; + continue; + } + // Encode a whole code point, so a surrogate pair yields its UTF-8 octets + // rather than two lone-surrogate errors. + const point = value.codePointAt(at) as number; + // An *unpaired* surrogate has no UTF-8 encoding, and `encodeURIComponent` + // throws `URIError` on it. Report it instead: this runs outside `tryExpand` + // and, through the preview, during React render — an escaping throw would + // take the panel down rather than disable its submit. + if (point >= 0xd800 && point <= 0xdfff) return null; + const codePoint = String.fromCodePoint(point); + out += encodeURIComponent(codePoint).replace( + UNDER_ENCODED_BY_ENCODE_URI_COMPONENT, + (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`, + ); + at += codePoint.length; + } + return out; +} + +/** + * The SDK's per-value ceiling, enforced here instead of by it. + * + * Because a value now reaches the SDK as a short sentinel and is substituted in + * afterwards, the SDK's own `validateLength` no longer sees it — so without this + * the helper would happily emit a URI the SDK itself refuses to build. Keeping + * the same bound preserves that guard, and with it the panel's behavior of + * withholding the request rather than sending something absurd. + */ +const MAX_VALUE_LENGTH = 1_000_000; + +/** + * Expand `uriTemplate`, letting the SDK build the *structure* while this module + * supplies each variable's *rendering*. + * + * Every variable that will appear is handed to the SDK as a sentinel token made + * only of unreserved characters, so it survives whichever encoding the SDK + * applies, and is swapped for its real rendering afterwards. That split is what + * lets the operators, separators, and omission rules stay the SDK's job while + * the encoding — which the SDK gets wrong (see `encodeValue`) — becomes ours. + * + * Renderings are per *occurrence*, not per name, which is what lets the same + * variable be encoded two ways in one template: `x://{+a}/{a}` with `a = "/"` + * must keep the `/` under the reserved operator and encode it as `%2F` in the + * simple expansion. + * + * `renderUnset` decides what an absent-or-empty variable becomes: `null` omits + * it (the wire behavior), while returning a string substitutes it (the preview's + * `{name}` placeholder). + * + * The substitution is a single regex pass over the expansion, not one pass per + * variable: the SDK accepts a 1 MB template with up to 10,000 expressions, and a + * per-variable rescan would be O(variables × length) on the render thread. + */ +function expandWithPlaceholders( + uriTemplate: string, + variables: Record, + renderUnset: (name: string) => string | null, +): string | null { + // Validate the template **as the server sent it**, before rewriting. The + // rewrite replaces variable names with short synthetics, which shrinks both + // the template and every name — so a template the SDK would reject for + // exceeding its length limits could otherwise slip through here while + // `templateVariableNames` (which parses the original) rejected it, leaving the + // form empty and the expansion happily producing some shorter URI. + if (!parseTemplate(uriTemplate)) return null; + + const { text, slots, queryNames, order } = rewriteExpressions(uriTemplate); + const template = parseTemplate(text); + /* v8 ignore next -- unreachable: the rewrite only ever shortens the template + and its names, so anything the original parse accepted parses here too. */ + if (!template) return null; + + for (const name of order) { + const value = variables[name]; + if (value !== undefined && value.length > MAX_VALUE_LENGTH) { + console.warn( + `Cannot expand URI template "${uriTemplate}": the value for "${name}" exceeds ${MAX_VALUE_LENGTH} characters.`, + ); + return null; + } + } + + const base = uncollidingBase(uriTemplate); + const renderings = new Map(); + const values: Record = {}; + let nextIndex = 0; + + // An unpaired surrogate has no encoding under any operator, so it fails the + // whole expansion rather than silently dropping one variable. Checked up + // front, which also lets `place` treat a null rendering as "omit this one". + for (const name of order) { + const value = variables[name]; + if (value !== undefined && encodeValue(value, "") === null) { + console.warn( + `Cannot expand URI template "${uriTemplate}": the value for "${name}" contains an unpaired surrogate.`, + ); + return null; + } + } + + /** Mint a sentinel for one occurrence, or report that it contributes nothing. */ + function place(name: string, operator: string): string | null { + const value = variables[name]; + const rendered = + value === undefined ? renderUnset(name) : encodeValue(value, operator); + if (rendered === null) return null; + const index = nextIndex++; + renderings.set(index, rendered); + return sentinelFor(base, index); + } + + for (const [synthetic, slot] of slots) { + const placed = slot.names + .map((name) => place(name, slot.operator)) + .filter((sentinel): sentinel is string => sentinel !== null); + // An expression with no defined variable contributes nothing (RFC 6570). + if (placed.length > 0) values[synthetic] = placed; + } + for (const name of queryNames) { + // `?` and `&` share one encoding, so every query occurrence of a name can + // share the single value the SDK will look up under that name. + const sentinel = place(name, "?"); + if (sentinel !== null) values[name] = sentinel; + } + + const expanded = tryExpand(template, values); + if (expanded === null) return null; + + return expanded.replace( + new RegExp(`${base}(\\d+)zz`, "g"), + /* v8 ignore next -- the `?? match` fallback is unreachable: every sentinel + in the expansion was minted from `renderings` just above. */ + (match, index: string) => renderings.get(Number(index)) ?? match, + ); +} + +/** + * Expands `uriTemplate` per RFC 6570, percent-encoding each value according to + * its expression's operator. Returns `null` when the template cannot be handled + * or a value cannot be expanded, so a caller can decline to issue the request + * rather than send a URI it knows is wrong. + * + * An absent variable is omitted, while one defined as the empty string is kept — + * preserving the spec's distinction between the two, so `{ topic: "" }` against + * `{?topic}` yields `?topic=`. Collapsing them would make a deliberately-empty + * value unexpressible. + */ +export function expandTemplate( + uriTemplate: string, + variables: Record, +): string | null { + if (warnIfUnsupported(uriTemplate)) return null; + return expandWithPlaceholders(uriTemplate, variables, () => null); +} + +/** + * Base of the token used to stand in for a variable the user hasn't filled yet, + * so the preview can show `{topic}` in its place instead of silently dropping it. + * + * Every character is RFC 3986 *unreserved*, so `expand` passes it through + * verbatim under every operator and it survives to be swapped back out. + * Percent-encoding can never *produce* this sequence either — it only emits + * `%` plus hex digits, and the base contains characters outside that set — so + * checking the raw inputs for a collision (below) is sufficient. + */ +const UNFILLED_SENTINEL = "zzInspectorUnfilledzz"; + +/** + * Pick a sentinel base that appears nowhere in the template's literal text. + * + * Only the template needs checking: values never reach the expansion — they are + * substituted in afterwards, in a single pass that does not rescan what it + * inserts — so a value equal to the token cannot be mistaken for one. + * + * Done in one pass rather than by extending the base until it stops colliding: + * the template is server-supplied and may be up to 1 MB, and a template holding + * the token followed by a long run of `z`s would make every extended candidate + * collide in turn, each rescanning the whole input — quadratic work on the + * render thread. Instead, measure the longest run of `z` that follows any + * occurrence and clear it by one, which no occurrence can then match. + */ +function uncollidingBase(uriTemplate: string) { + return padPastCollisions(UNFILLED_SENTINEL, "z", uriTemplate); +} + +/** + * Extend `token` with `pad` characters until it cannot occur in `haystack`. + * + * Done by measuring, in a single pass, the longest run of `pad` that follows + * any occurrence, then clearing it by one. The obvious `while + * (haystack.includes(candidate)) candidate += pad` is quadratic on exactly the + * input that motivates the check: a haystack holding the token followed by a + * long run of `pad` makes every successive candidate collide, each rescanning + * the whole string — and the template is server-supplied, up to 1 MB, scanned + * on the render thread. + * + * The scan advances one character at a time rather than by the token's length, + * because a token that begins and ends with the same characters can overlap + * itself: `zzInspectorUnfilledzzInspectorUnfilledzzz0zz` holds a second + * occurrence at index 19 whose trailing run is longer than the first's. Skipping + * it would choose a colliding token after all. + */ +function padPastCollisions( + token: string, + pad: string, + haystack: string, +): string { + let longestRun = -1; + for ( + let at = haystack.indexOf(token); + at !== -1; + at = haystack.indexOf(token, at + 1) + ) { + let run = 0; + let cursor = at + token.length; + while (haystack[cursor] === pad) { + run++; + cursor++; + } + if (run > longestRun) longestRun = run; + } + // -1 means the token is absent, so it needs no padding at all. + return token + pad.repeat(longestRun + 1); +} + +/** + * Keyed by the variable's position rather than its name: a name may legally + * contain characters (`%`-encoded triplets) that `expand` would re-encode, + * which would keep the sentinel from surviving the round trip. + * + * The trailing delimiter is load-bearing — without it index 1's token would be + * a prefix of index 11's, and substituting the first would corrupt the second. + */ +function sentinelFor(base: string, index: number): string { + return `${base}${index}zz`; +} + +/** + * The subset of `variables` the user has actually typed something into. + * + * This is a *preview* notion, not an RFC 6570 one — `expandTemplate` + * deliberately does not collapse `""` this way. The panel seeds every declared + * variable with `""` and a text input cannot express "defined but empty", so + * within the preview an empty string means "not entered yet" and earns a + * `{name}` placeholder rather than an empty expansion. + */ +function enteredValues( + variables: Record, +): Record { + return Object.fromEntries( + Object.entries(variables).filter(([, value]) => value.length > 0), + ); +} + +/** + * A human-readable rendering of the template with the values entered so far: + * filled variables are expanded (and encoded) exactly as they would be on the + * wire, while unfilled ones are shown as `{name}` so the shape of the URI stays + * legible while the form is still being completed. + * + * Falls back to the raw template when it cannot be parsed or expanded — this + * runs during render, so it must not throw. + */ +export function previewTemplate( + uriTemplate: string, + variables: Record, +): string { + // Nothing truthful to render for a template the expander declines, so show it + // as the server declared it. + if (unsupportedReason(uriTemplate) !== null) return uriTemplate; + return ( + expandWithPlaceholders( + uriTemplate, + enteredValues(variables), + (name) => `{${name}}`, + ) ?? uriTemplate + ); +} diff --git a/test-servers/configs/rfc6570-templates-http.json b/test-servers/configs/rfc6570-templates-http.json new file mode 100644 index 000000000..1e30477e0 --- /dev/null +++ b/test-servers/configs/rfc6570-templates-http.json @@ -0,0 +1,11 @@ +{ + "serverInfo": { + "name": "rfc6570-templates", + "version": "1.0.0" + }, + "resourceTemplates": [{ "preset": "rfc6570_templates" }], + "transport": { + "type": "streamable-http", + "port": 3143 + } +} diff --git a/test-servers/src/preset-registry.ts b/test-servers/src/preset-registry.ts index d78998ec6..34fd53ad7 100644 --- a/test-servers/src/preset-registry.ts +++ b/test-servers/src/preset-registry.ts @@ -62,6 +62,7 @@ import { createFileResourceTemplate, createUserResourceTemplate, createNumberedResourceTemplates, + createRfc6570ResourceTemplates, createSimplePrompt, createArgsPrompt, createNumberedPrompts, @@ -260,6 +261,8 @@ function resolveResourceTemplatePreset( return createUserResourceTemplate(); case "numbered_resource_templates": return createNumberedResourceTemplates(Number(get("count")) || 3); + case "rfc6570_templates": + return createRfc6570ResourceTemplates(); default: throw new Error(`Unknown resource template preset: ${name}`); } diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index 070b57506..5082e3e9e 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -1474,6 +1474,51 @@ export function createFileResourceTemplate( }; } +/** + * Create the pair of RFC 6570 templates from #1919: one simple expression whose + * value must be percent-encoded rather than injected verbatim, and one query + * expression — which a naive `/\{(\w+)\}/g` scan cannot see at all. + * + * Both echo the received variable back, so the rendered result shows whether the + * client encoded and routed the value the server actually expected. + */ +export function createRfc6570ResourceTemplates(): ResourceTemplateDefinition[] { + const describe = (uri: URL, topic: unknown) => ({ + contents: [ + { + uri: uri.toString(), + mimeType: "application/json", + text: JSON.stringify({ topic, matchedUri: uri.toString() }, null, 2), + }, + ], + }); + + return [ + { + name: "events-by-path", + uriTemplate: "foobar://events/{topic}", + description: + "Simple expression. A topic containing `/` must arrive percent-encoded, or it becomes a second path segment and no longer matches.", + inputSchema: { + topic: z.string().describe("Topic name — try `foo/bar`"), + }, + handler: async (uri: URL, params: Record) => + describe(uri, params.topic), + }, + { + name: "events-by-query", + uriTemplate: "foobar://events{?topic}", + description: + "Query expression. The variable is only discoverable through an RFC 6570 parser.", + inputSchema: { + topic: z.string().describe("Topic name"), + }, + handler: async (uri: URL, params: Record) => + describe(uri, params.topic), + }, + ]; +} + /** * Create a "user" resource template that returns user data by ID */