fix(web): expand resource templates per RFC 6570
The Resources screen discovered and substituted template variables with a
bare `/\{(\w+)\}/g` regex, which is wrong two ways: it cannot see an
expression carrying an operator, so `foobar://events{?topic}` rendered no
input at all; and it splices the raw value in, so a `/`, `?`, `#`, `%`,
space or non-ASCII character in a simple `{topic}` landed unencoded and
changed the URI's structure -- `foo/bar` produced an extra path segment
that a conforming matcher rejects with `-32602 Resource not found`.
Delegate expansion to the SDK's `UriTemplate`, the same implementation
`InspectorClient.readResourceFromTemplate` (and so the TUI) already
expands through, so the two clients cannot disagree about what a template
means. The new `utils/uriTemplate` supplies only what that class does
not: which variables to render an input for, which of them a read cannot
proceed without, and a partially-expanded preview.
Required-ness follows the operator. Under `?`, `&`, `.` or `/` the whole
expression is omitted when the variable is undefined, so those fields are
marked Optional and reading with them blank is a legitimate request for
the unfiltered resource; under `""`, `+` or `#` the variable sits mid-URI,
so it stays required. Blank fields are dropped before expanding so an
untouched optional field reads as undefined rather than as the empty
string, which would expand to a valueless `?topic=`.
Adds the `rfc6570_templates` preset and a `rfc6570-templates-http.json`
showcase server serving the two templates from the issue.
Closes #1919
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
8dd0fdf28b
commit
83658db2dd
2 changed files with 578 additions and 34 deletions
|
|
@ -27,6 +27,11 @@ const noVarTemplate: ResourceTemplate = {
|
|||
uriTemplate: "file:///static.txt",
|
||||
};
|
||||
|
||||
const queryTemplate: ResourceTemplate = {
|
||||
name: "Events",
|
||||
uriTemplate: "foobar://events{?topic}",
|
||||
};
|
||||
|
||||
describe("ResourceTemplatePanel", () => {
|
||||
it("renders the template title (or name) and description", () => {
|
||||
renderWithMantine(
|
||||
|
|
@ -154,6 +159,92 @@ describe("ResourceTemplatePanel", () => {
|
|||
).not.toBeDisabled();
|
||||
});
|
||||
|
||||
describe("RFC 6570 expansion (#1919)", () => {
|
||||
it("renders an input for a query expression the old regex could not see", () => {
|
||||
renderWithMantine(
|
||||
<ResourceTemplatePanel
|
||||
template={queryTemplate}
|
||||
onReadResource={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByLabelText("topic")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("percent-encodes a reserved character in a simple variable", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onReadResource = vi.fn();
|
||||
renderWithMantine(
|
||||
<ResourceTemplatePanel
|
||||
template={singleVarTemplate}
|
||||
onReadResource={onReadResource}
|
||||
/>,
|
||||
);
|
||||
await user.type(screen.getByLabelText("userId"), "foo/bar");
|
||||
await user.click(screen.getByRole("button", { name: "Read Resource" }));
|
||||
expect(onReadResource).toHaveBeenCalledWith(
|
||||
"file:///users/foo%2Fbar/profile",
|
||||
);
|
||||
});
|
||||
|
||||
it("builds an encoded query expression for {?topic}", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onReadResource = vi.fn();
|
||||
renderWithMantine(
|
||||
<ResourceTemplatePanel
|
||||
template={queryTemplate}
|
||||
onReadResource={onReadResource}
|
||||
/>,
|
||||
);
|
||||
await user.type(screen.getByLabelText("topic"), "foo/bar");
|
||||
await user.click(screen.getByRole("button", { name: "Read Resource" }));
|
||||
expect(onReadResource).toHaveBeenCalledWith(
|
||||
"foobar://events?topic=foo%2Fbar",
|
||||
);
|
||||
});
|
||||
|
||||
it("marks a query variable Optional and does not gate the read on it", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onReadResource = vi.fn();
|
||||
renderWithMantine(
|
||||
<ResourceTemplatePanel
|
||||
template={queryTemplate}
|
||||
onReadResource={onReadResource}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("Optional")).toBeInTheDocument();
|
||||
const button = screen.getByRole("button", { name: "Read Resource" });
|
||||
expect(button).not.toBeDisabled();
|
||||
// Left blank, the whole expression drops out per RFC 6570.
|
||||
await user.click(button);
|
||||
expect(onReadResource).toHaveBeenCalledWith("foobar://events");
|
||||
});
|
||||
|
||||
it("does not mark a required simple variable Optional", () => {
|
||||
renderWithMantine(
|
||||
<ResourceTemplatePanel
|
||||
template={singleVarTemplate}
|
||||
onReadResource={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(screen.queryByText("Optional")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("previews the query expression verbatim until it is filled", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<ResourceTemplatePanel
|
||||
template={queryTemplate}
|
||||
onReadResource={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("foobar://events{?topic}")).toBeInTheDocument();
|
||||
await user.type(screen.getByLabelText("topic"), "news");
|
||||
expect(
|
||||
screen.getByText("foobar://events?topic=news"),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe("completions", () => {
|
||||
it("fires a completion immediately on focus before any keystroke", async () => {
|
||||
const user = userEvent.setup();
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
expandUriTemplate,
|
||||
previewUriTemplate,
|
||||
templateVariables,
|
||||
} from "../../../utils/uriTemplate";
|
||||
|
||||
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, string>,
|
||||
): string {
|
||||
return uriTemplate.replace(/\{(\w+)\}/g, (_, key: string) => variables[key]);
|
||||
}
|
||||
|
||||
function previewUri(
|
||||
uriTemplate: string,
|
||||
variables: Record<string, string>,
|
||||
): 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",
|
||||
|
|
@ -108,10 +85,16 @@ export function ResourceTemplatePanel({
|
|||
}: ResourceTemplatePanelProps) {
|
||||
const { name, title, uriTemplate, description, annotations } = template;
|
||||
|
||||
const variableNames = useMemo(
|
||||
() => parseVariableNames(uriTemplate),
|
||||
// Every variable the template declares, with the operator it appears under
|
||||
// and whether omitting it would change the URI's shape (see `utils/uriTemplate`).
|
||||
const declaredVariables = useMemo(
|
||||
() => templateVariables(uriTemplate),
|
||||
[uriTemplate],
|
||||
);
|
||||
const variableNames = useMemo(
|
||||
() => declaredVariables.map((v) => v.name),
|
||||
[declaredVariables],
|
||||
);
|
||||
|
||||
const [variables, setVariables] = useState<Record<string, string>>(() =>
|
||||
Object.fromEntries(variableNames.map((n) => [n, ""])),
|
||||
|
|
@ -232,13 +215,18 @@ export function ResourceTemplatePanel({
|
|||
void runCompletion(varName, value, buildContext(varName));
|
||||
}
|
||||
|
||||
const canSubmit = variableNames.every((n) => variables[n]?.length > 0);
|
||||
// Only the variables whose absence would change the URI's shape gate the
|
||||
// read; an unfilled `{?topic}` is a legitimate request for the unfiltered
|
||||
// resource, and RFC 6570 drops the whole expression for it.
|
||||
const canSubmit = declaredVariables.every(
|
||||
(v) => !v.required || variables[v.name]?.length > 0,
|
||||
);
|
||||
|
||||
function handleSubmit() {
|
||||
onReadResource(resolveUri(uriTemplate, variables));
|
||||
onReadResource(expandUriTemplate(uriTemplate, variables));
|
||||
}
|
||||
|
||||
const preview = previewUri(uriTemplate, variables);
|
||||
const preview = previewUriTemplate(uriTemplate, variables);
|
||||
|
||||
return (
|
||||
<Stack gap="md">
|
||||
|
|
@ -251,13 +239,17 @@ export function ResourceTemplatePanel({
|
|||
</HeaderRow>
|
||||
{description && <DescriptionText>{description}</DescriptionText>}
|
||||
<Stack gap="sm">
|
||||
{variableNames.map((varName) => {
|
||||
{declaredVariables.map(({ name: varName, required }) => {
|
||||
/* v8 ignore next -- `?? ""` fallback unreachable: `variables` is seeded with every declared variable, so the key is always present. */
|
||||
const fieldValue = variables[varName] ?? "";
|
||||
// RFC 6570 omits an undefined variable under a query/path-segment
|
||||
// operator entirely, so those fields are genuinely optional.
|
||||
const description = required ? undefined : "Optional";
|
||||
return useAutocomplete ? (
|
||||
<Autocomplete
|
||||
key={varName}
|
||||
label={varName}
|
||||
description={description}
|
||||
placeholder={`Enter ${varName}`}
|
||||
value={fieldValue}
|
||||
data={completions[varName] ?? []}
|
||||
|
|
@ -273,6 +265,7 @@ export function ResourceTemplatePanel({
|
|||
<TextInput
|
||||
key={varName}
|
||||
label={varName}
|
||||
description={description}
|
||||
placeholder={`Enter ${varName}`}
|
||||
value={fieldValue}
|
||||
onChange={(e) =>
|
||||
|
|
|
|||
Loading…
Reference in a new issue