Merge branch 'v2/main' into v2/fix/1941-cli-vitest-teardown-crash
This commit is contained in:
commit
6fdc12424f
25 changed files with 4078 additions and 333 deletions
|
|
@ -213,4 +213,173 @@ describe("schemaToForm", () => {
|
|||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({ initialValue: "hi" });
|
||||
});
|
||||
|
||||
// #2015 (the TUI twin of #1928): an argument declared with Zod's `.nullish()`
|
||||
// has no top-level `type`/`enum` — they sit on the surviving `anyOf` branch —
|
||||
// so before normalization every one of these degraded to a plain text field.
|
||||
describe("nullable unions", () => {
|
||||
it("renders a select for an anyOf string-enum|null argument", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
direction: {
|
||||
anyOf: [
|
||||
{ type: "string", enum: ["envio", "recebimento"] },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
"nullableEnum",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({
|
||||
name: "direction",
|
||||
type: "select",
|
||||
options: [
|
||||
{ label: "envio", value: "envio" },
|
||||
{ label: "recebimento", value: "recebimento" },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("maps the remaining nullable scalar branches to their typed fields", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
reference: { anyOf: [{ type: "string" }, { type: "null" }] },
|
||||
quantity: {
|
||||
anyOf: [
|
||||
{ type: "integer", minimum: 1, maximum: 9 },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
ratio: { anyOf: [{ type: "number" }, { type: "null" }] },
|
||||
express: { anyOf: [{ type: "boolean" }, { type: "null" }] },
|
||||
},
|
||||
},
|
||||
"nullableScalars",
|
||||
);
|
||||
const fields = form.sections[0]!.fields;
|
||||
expect(fields.map((f) => ({ name: f.name, type: f.type }))).toEqual([
|
||||
{ name: "reference", type: "string" },
|
||||
{ name: "quantity", type: "integer" },
|
||||
{ name: "ratio", type: "float" },
|
||||
{ name: "express", type: "boolean" },
|
||||
]);
|
||||
// The branch's own constraints are hoisted along with its type.
|
||||
expect(fields[1]).toMatchObject({ min: 1, max: 9 });
|
||||
});
|
||||
|
||||
it("renders a select for a nullable array-of-enum argument", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
tags: {
|
||||
anyOf: [
|
||||
{ type: "array", items: { enum: ["a", "b"] } },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
"nullableArrayEnum",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({
|
||||
type: "select",
|
||||
options: [
|
||||
{ label: "a", value: "a" },
|
||||
{ label: "b", value: "b" },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
// With the null left in the list, the all-strings check would reject the
|
||||
// enum and this would degrade to a plain text field.
|
||||
it("renders a select for a type: [string, null] enum, null stripped", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
direction: {
|
||||
type: ["string", "null"],
|
||||
enum: ["envio", "recebimento", null],
|
||||
},
|
||||
},
|
||||
},
|
||||
"typeArrayNullableEnum",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({
|
||||
type: "select",
|
||||
options: [
|
||||
{ label: "envio", value: "envio" },
|
||||
{ label: "recebimento", value: "recebimento" },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("collapses the type: [T, null] encoding too", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
note: { type: ["string", "null"] },
|
||||
flag: { type: ["boolean", "null"] },
|
||||
},
|
||||
},
|
||||
"typeArrayNull",
|
||||
);
|
||||
expect(
|
||||
form.sections[0]!.fields.map((f) => ({ name: f.name, type: f.type })),
|
||||
).toEqual([
|
||||
{ name: "note", type: "string" },
|
||||
{ name: "flag", type: "boolean" },
|
||||
]);
|
||||
});
|
||||
|
||||
// ink-form hands a select's stringified value straight back, so a numeric
|
||||
// enum routed to a select would submit "1" for 1. The typed field loses the
|
||||
// enum constraint but keeps the value's type — the safer loss.
|
||||
it("routes a nullable numeric enum to its typed field, not a select", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
level: {
|
||||
anyOf: [{ type: "integer", enum: [1, 2] }, { type: "null" }],
|
||||
},
|
||||
ratio: {
|
||||
anyOf: [{ type: "number", enum: [0.5, 1.5] }, { type: "null" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
"numericEnum",
|
||||
);
|
||||
expect(
|
||||
form.sections[0]!.fields.map((f) => ({ name: f.name, type: f.type })),
|
||||
).toEqual([
|
||||
{ name: "level", type: "integer" },
|
||||
{ name: "ratio", type: "float" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("routes a plain numeric enum to its typed field too", () => {
|
||||
const form = schemaToForm(
|
||||
{ properties: { level: { type: "integer", enum: [1, 2] } } },
|
||||
"plainNumericEnum",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({ type: "integer" });
|
||||
});
|
||||
|
||||
it("leaves a union of two real types as a plain string field", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
mixed: { anyOf: [{ type: "string" }, { type: "number" }] },
|
||||
},
|
||||
},
|
||||
"realUnion",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({
|
||||
name: "mixed",
|
||||
type: "string",
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -3,10 +3,19 @@
|
|||
*/
|
||||
|
||||
import type { FormStructure, FormSection, FormField } from "ink-form";
|
||||
import {
|
||||
isStringEnum,
|
||||
normalizeNullableUnion,
|
||||
} from "@inspector/core/json/nullableUnion.js";
|
||||
|
||||
/** Minimal JSON Schema property shape used when building tool parameter forms */
|
||||
interface JsonSchemaProperty {
|
||||
type?: string;
|
||||
/**
|
||||
* An array here is the `["string", "null"]` nullable encoding, which
|
||||
* {@link normalizeNullableUnion} collapses to a single name before the
|
||||
* dispatch below reads it.
|
||||
*/
|
||||
type?: string | string[];
|
||||
title?: string;
|
||||
enum?: unknown[];
|
||||
/** Non-standard legacy support: titles for enum values */
|
||||
|
|
@ -15,6 +24,8 @@ interface JsonSchemaProperty {
|
|||
minimum?: number;
|
||||
maximum?: number;
|
||||
default?: unknown;
|
||||
/** Present on a nullable union; see {@link normalizeNullableUnion}. */
|
||||
anyOf?: readonly unknown[];
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -68,9 +79,16 @@ export function schemaToForm(
|
|||
// `properties` values are `unknown` (the SDK schema admits anything), so
|
||||
// guard before treating a value as a schema object — a malformed server
|
||||
// schema with e.g. `properties: { foo: null }` must not throw on `.title`.
|
||||
const property = (
|
||||
typeof prop === "object" && prop !== null ? prop : {}
|
||||
) as JsonSchemaProperty;
|
||||
// Flatten a nullable union (`anyOf: [X, {type:"null"}]`, `type: [X,"null"]`)
|
||||
// before dispatching. Every branch below reads a single `type` string, and
|
||||
// for an enum the `enum` keyword sits on the union's surviving branch — so
|
||||
// without this an argument declared with Zod's `.nullish()` loses its
|
||||
// select and degrades to a plain text field (#2015, the TUI twin of #1928).
|
||||
const property = normalizeNullableUnion(
|
||||
(typeof prop === "object" && prop !== null
|
||||
? prop
|
||||
: {}) as JsonSchemaProperty,
|
||||
);
|
||||
const baseField = {
|
||||
name: key,
|
||||
label: property.title || key,
|
||||
|
|
@ -90,12 +108,17 @@ export function schemaToForm(
|
|||
...baseField,
|
||||
options: toSelectOptions(property.items.enum, property.items.enumNames),
|
||||
} as FormField;
|
||||
} else if (property.enum) {
|
||||
// Single select
|
||||
} else if (isStringEnum(property.enum)) {
|
||||
// Single select. Gated on the members being strings because
|
||||
// `toSelectOptions` stringifies them and ink-form hands the string
|
||||
// straight back: a numeric `enum: [1, 2]` would submit `"1"` and violate
|
||||
// the schema. A typed non-string enum falls through to its typed field
|
||||
// below, which loses the enum constraint but keeps the value's type —
|
||||
// the safer of the two losses.
|
||||
field = {
|
||||
type: "select",
|
||||
...baseField,
|
||||
options: toSelectOptions(property.enum, property.enumNames),
|
||||
options: toSelectOptions(property.enum ?? [], property.enumNames),
|
||||
} as FormField;
|
||||
} else {
|
||||
// Map JSON Schema types to ink-form types
|
||||
|
|
|
|||
|
|
@ -162,7 +162,9 @@ The dev/prod backend guards every `/api/*` route with `x-mcp-remote-auth: Bearer
|
|||
|
||||
## Host binding & the origin allow-list
|
||||
|
||||
Both the prod backend (`server/web-server-config.ts`) and the dev Vite server (`vite.config.ts`) resolve their bind host through one shared guard, `server/resolve-bind-host.ts`. It binds `localhost` by default and **refuses an all-interfaces host** (`0.0.0.0`, `::`, empty, or any equivalent spelling — `0`, `0x0`, `0.0`, `::0`, `::ffff:0.0.0.0`, … are all folded to the wildcard and refused) — which would expose the process-spawning backend to the whole network, the exposure DNS-rebinding attacks target — unless `DANGEROUSLY_BIND_ALL_INTERFACES=true` is set. The Docker image sets that flag (a container must bind `0.0.0.0` to be reachable through `-p`); a bare `HOST=0.0.0.0` anywhere else exits with an actionable error.
|
||||
Both the prod backend (`server/web-server-config.ts`) and the dev Vite server (`vite.config.ts`) resolve their bind host through one shared guard, `server/resolve-bind-host.ts`. It binds **`127.0.0.1`** by default and **refuses an all-interfaces host** (`0.0.0.0`, `::`, empty, or any equivalent spelling — `0`, `0x0`, `0.0`, `::0`, `::ffff:0.0.0.0`, … are all folded to the wildcard and refused) — which would expose the process-spawning backend to the whole network, the exposure DNS-rebinding attacks target — unless `DANGEROUSLY_BIND_ALL_INTERFACES=true` is set. The Docker image sets that flag (a container must bind `0.0.0.0` to be reachable through `-p`); a bare `HOST=0.0.0.0` anywhere else exits with an actionable error.
|
||||
|
||||
**The default is the IPv4 loopback _address_, deliberately not the name `localhost`** (#1951). `server.listen(port, host)` resolves a name through `dns.lookup` and binds the **single** address it gets back; since Node 17 the default result order is `verbatim`, so on a glibc Linux host — whose `/etc/hosts` maps `localhost` to both `127.0.0.1` and `::1` — that first address is `::1`. `HOST=localhost` there binds IPv6 loopback **only** and refuses every IPv4 client. On a desktop this is invisible (the browser resolves `localhost` the same way and lands on `::1` too); it breaks wherever the connection is made by something that pins `127.0.0.1` — a VS Code dev container's port forwarder, `ssh -L 6274:127.0.0.1:6274`, a container healthcheck. Binding the address takes the resolver out of the decision. Browsing to `http://localhost:PORT` still works, since browsers fall back across address families and the loopback allow-list covers all three forms. An explicit `HOST=localhost` is still honored as typed — the single-family bind is then your choice.
|
||||
|
||||
The backend's `/api/*` routes also enforce an **origin allow-list** (`allowedOrigins`) as DNS-rebinding protection. When left to default on a loopback host, it expands to all three interchangeable loopback origin forms for the port — `http://localhost:PORT`, `http://127.0.0.1:PORT`, and `http://[::1]:PORT` — because `localhost` resolves to either IPv4 or IPv6 loopback and Node/Vite may bind the IPv6 form, so the browser can legitimately arrive at `http://[::1]:PORT`. Set `ALLOWED_ORIGINS` (comma-separated) to override; entries are canonicalized (`new URL(o).origin`), so a trailing slash / uppercase host / explicit `:80` still match. **Each entry must include the scheme** — `http://localhost:6274`, not `localhost:6274` (a scheme-less value is dropped with a warning). `ALLOWED_ORIGINS` **replaces** the default list (it does not merge), so **list every origin you'll browse from, including the loopback forms** you still want (`http://localhost:PORT`, `http://127.0.0.1:PORT`, `http://[::1]:PORT`) — otherwise local access stops working. A blank `ALLOWED_ORIGINS` does **not** disable the check — it falls back to the default (fail closed); there is no env knob to turn origin validation off.
|
||||
|
||||
|
|
@ -176,7 +178,7 @@ The guard blocks only the **wildcard** all-interfaces addresses. Binding a **spe
|
|||
|
||||
The bind-host guard and the `ALLOWED_ORIGINS` allow-list apply to both the prod server and `--dev`. Note that in **`--dev`** the Vite dev server _additionally_ enforces its own `server.allowedHosts` Host-header check, whose default accepts loopback and IP-literal hosts. The host you **bind** is auto-allowed (Vite adds the resolved `server.host` — which this config sets from `HOST` — to the allow-list), so `HOST=<hostname>` works out of the box under `--dev` too. What needs an explicit `server.allowedHosts` entry is reaching the dev server at a **different** name than the one bound — e.g. a wildcard bind reached by hostname, or a reverse-proxy domain. For those, prefer the prod server (`mcp-inspector --web`) or add the host to `server.allowedHosts`.
|
||||
|
||||
**MCP Apps caveats.** The MCP Apps sandbox runs on a **separate** port (`MCP_SANDBOX_PORT`, dynamic by default). For the Apps tab to work off loopback, that sandbox port must be independently reachable from the browser — pin it with `MCP_SANDBOX_PORT` and expose/forward it too (the Docker image `EXPOSE`s only `6274`). (Under a `0.0.0.0` wildcard bind the sandbox URL is advertised as `localhost`, which is reachable — a wildcard bind serves loopback — so only the port needs handling.) Also note the sandbox iframe is gated by a `frame-ancestors` CSP, and **a bracketed IPv6 literal is not a valid CSP host-source** — so MCP Apps requires browsing the app at a name or IPv4 (`localhost`, `127.0.0.1`, a hostname, a LAN IPv4), **not** a bare `http://[::1]:…` address. Finally, the sandbox URL is always `http://` — so **behind TLS** (an `https://` app page) the browser blocks the `http://…/sandbox` iframe as mixed content and MCP Apps can't render; the Apps tab needs a plain-`http` app origin today.
|
||||
**MCP Apps caveats.** The MCP Apps sandbox runs on a **separate** port — `MCP_SANDBOX_PORT`, defaulting to a fixed **`6275`** (#2008; it was OS-assigned before, which meant it changed every run and so could never be named in a `forwardPorts` / `-p` / tunnel config written ahead of time). For the Apps tab to work off loopback, that sandbox port must be independently reachable from the browser — expose/forward `6275` alongside `6274`, or set `MCP_SANDBOX_PORT` to pick another. If the port is already taken the sandbox falls back to an OS-assigned one and warns, so a second Inspector still gets a working Apps tab locally — but the forwarded port is then wrong, which is what the warning tells you. (Under a `0.0.0.0` wildcard bind the sandbox URL is advertised as `localhost`, which is reachable — a wildcard bind serves loopback — so only the port needs handling.) Also note the sandbox iframe is gated by a `frame-ancestors` CSP, and **a bracketed IPv6 literal is not a valid CSP host-source** — so MCP Apps requires browsing the app at a name or IPv4 (`localhost`, `127.0.0.1`, a hostname, a LAN IPv4), **not** a bare `http://[::1]:…` address. Finally, the sandbox URL is always `http://` — so **behind TLS** (an `https://` app page) the browser blocks the `http://…/sandbox` iframe as mixed content and MCP Apps can't render; the Apps tab needs a plain-`http` app origin today.
|
||||
|
||||
In every case, exposing the Inspector beyond loopback also means anyone who can reach it can drive its backend — keep authentication on (do **not** set `DANGEROUSLY_OMIT_AUTH`) and prefer a specific bind address over the wildcard.
|
||||
|
||||
|
|
|
|||
|
|
@ -26,21 +26,46 @@ function isEnabled(value: string | undefined): boolean {
|
|||
return v === "true" || v === "1";
|
||||
}
|
||||
|
||||
/**
|
||||
* The default bind host: the IPv4 loopback **address**, deliberately not the
|
||||
* name `localhost`.
|
||||
*
|
||||
* `server.listen(port, host)` resolves a name through `dns.lookup` and binds the
|
||||
* **single** address it gets back. Since Node 17 the default result order is
|
||||
* `verbatim`, so Node takes whatever the resolver returns first rather than
|
||||
* preferring IPv4 — and on a glibc Linux host with the stock `/etc/hosts`
|
||||
* (`127.0.0.1 localhost` *and* `::1 localhost`) that is `::1`. `HOST=localhost`
|
||||
* therefore binds IPv6 loopback **only**, and every IPv4 client is refused.
|
||||
*
|
||||
* That is invisible on a desktop, where the browser resolves `localhost` the
|
||||
* same way and lands on `::1` too. It breaks wherever the connection is made by
|
||||
* something that pins IPv4 `127.0.0.1` instead — a VS Code dev container's port
|
||||
* forwarder (#1951), an `ssh -L …:127.0.0.1:…` tunnel, a container healthcheck.
|
||||
* Binding the address directly removes the resolver from the decision, and the
|
||||
* loopback origin allow-list already covers `localhost`, `127.0.0.1`, and
|
||||
* `[::1]` alike, so browsing to `http://localhost:PORT` still works (browsers
|
||||
* fall back across address families; a Node `listen` does not).
|
||||
*/
|
||||
export const DEFAULT_BIND_HOST = "127.0.0.1";
|
||||
|
||||
/**
|
||||
* Resolve the bind hostname from `env` (default `process.env`), defaulting to
|
||||
* `localhost`. Refuses an all-interfaces host (`0.0.0.0` / `::` / empty / their
|
||||
* legacy spellings) unless {@link BIND_ALL_INTERFACES_ENV} is explicitly
|
||||
* enabled — the published Docker image sets it, since a container must bind
|
||||
* `0.0.0.0` to be reachable through `-p`. Throws (fail fast, loudly) rather than
|
||||
* silently binding wide open. The returned value is trimmed and de-bracketed
|
||||
* (an IPv6 literal is returned bare, e.g. `HOST=[::1]` → `::1`) so detection,
|
||||
* `listen()`, and the origin list all consume the same value; `formatHostForUrl`
|
||||
* re-adds the brackets wherever a URL is built.
|
||||
* {@link DEFAULT_BIND_HOST}. Refuses an all-interfaces host (`0.0.0.0` / `::` /
|
||||
* empty / their legacy spellings) unless {@link BIND_ALL_INTERFACES_ENV} is
|
||||
* explicitly enabled — the published Docker image sets it, since a container
|
||||
* must bind `0.0.0.0` to be reachable through `-p`. Throws (fail fast, loudly)
|
||||
* rather than silently binding wide open. The returned value is trimmed and
|
||||
* de-bracketed (an IPv6 literal is returned bare, e.g. `HOST=[::1]` → `::1`) so
|
||||
* detection, `listen()`, and the origin list all consume the same value;
|
||||
* `formatHostForUrl` re-adds the brackets wherever a URL is built.
|
||||
*
|
||||
* An explicit `HOST=localhost` is honored as typed — the single-family bind
|
||||
* described on {@link DEFAULT_BIND_HOST} is then the caller's choice, not ours.
|
||||
*/
|
||||
export function resolveBindHostname(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): string {
|
||||
const host = (env.HOST ?? "localhost").trim();
|
||||
const host = (env.HOST ?? DEFAULT_BIND_HOST).trim();
|
||||
if (isAllInterfacesHost(host) && !isEnabled(env[BIND_ALL_INTERFACES_ENV])) {
|
||||
// Show the resolved address when it differs from the typed spelling — the
|
||||
// guard now catches forms the resolver folds to the wildcard (a fullwidth
|
||||
|
|
|
|||
|
|
@ -11,13 +11,14 @@ import {
|
|||
canonicalUrlHost,
|
||||
isAllInterfacesHost,
|
||||
} from "../../../core/node/hostUrl.ts";
|
||||
import { DEFAULT_BIND_HOST } from "./resolve-bind-host.js";
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
export interface SandboxControllerOptions {
|
||||
/** Port to bind (0 = dynamic). */
|
||||
port: number;
|
||||
/** Host to bind (default localhost). */
|
||||
/** Host to bind (default {@link DEFAULT_BIND_HOST}). */
|
||||
host?: string;
|
||||
/**
|
||||
* The backend's origin allow-list (the embedder origins). Used to build the
|
||||
|
|
@ -95,10 +96,28 @@ function parseListenPort(raw: string | undefined): number | undefined {
|
|||
}
|
||||
|
||||
/**
|
||||
* Resolve sandbox port from env: MCP_SANDBOX_PORT → SERVER_PORT → 0 (dynamic).
|
||||
* An invalid value falls through rather than crashing the boot; a set-but-invalid
|
||||
* MCP_SANDBOX_PORT (the dedicated knob) is warned so the fall-through isn't
|
||||
* silent — matching the warn-and-drop precedent for ALLOWED_ORIGINS.
|
||||
* The default sandbox listen port — **fixed**, deliberately not `0`.
|
||||
*
|
||||
* A `0` default lets the OS assign, which is fine on a desktop (the browser
|
||||
* learns the port from `/api/config`) but unusable anywhere the port has to be
|
||||
* declared *before* the process starts: a dev container's `forwardPorts`, a
|
||||
* `docker run -p`, a k8s service, an SSH tunnel. The port changed on every run,
|
||||
* so it could never be named in that config and the Apps tab was unreachable
|
||||
* (#2008). `CLIENT_PORT` has always defaulted to a fixed `6274` for the same
|
||||
* reason; this is its sandbox counterpart.
|
||||
*
|
||||
* `6275` sits next to the web port so the pair is easy to forward together. It
|
||||
* is not a v1 carry-over — v2 has no proxy server on it.
|
||||
*/
|
||||
export const DEFAULT_SANDBOX_PORT = 6275;
|
||||
|
||||
/**
|
||||
* Resolve sandbox port from env: MCP_SANDBOX_PORT → SERVER_PORT →
|
||||
* {@link DEFAULT_SANDBOX_PORT}. An invalid value falls through rather than
|
||||
* crashing the boot; a set-but-invalid MCP_SANDBOX_PORT (the dedicated knob) is
|
||||
* warned so the fall-through isn't silent — matching the warn-and-drop
|
||||
* precedent for ALLOWED_ORIGINS. An explicit `0` is still honored as "let the
|
||||
* OS assign" ({@link parseListenPort} accepts it).
|
||||
*/
|
||||
export function resolveSandboxPort(): number {
|
||||
const fromSandbox = parseListenPort(process.env.MCP_SANDBOX_PORT);
|
||||
|
|
@ -107,13 +126,21 @@ export function resolveSandboxPort(): number {
|
|||
`Ignoring invalid MCP_SANDBOX_PORT="${process.env.MCP_SANDBOX_PORT}" (need an integer 0–65535); falling back.`,
|
||||
);
|
||||
}
|
||||
return fromSandbox ?? parseListenPort(process.env.SERVER_PORT) ?? 0;
|
||||
return (
|
||||
fromSandbox ??
|
||||
parseListenPort(process.env.SERVER_PORT) ??
|
||||
DEFAULT_SANDBOX_PORT
|
||||
);
|
||||
}
|
||||
|
||||
export function createSandboxController(
|
||||
options: SandboxControllerOptions,
|
||||
): SandboxController {
|
||||
const { port, host = "localhost", allowedOrigins } = options;
|
||||
// Defaulted to the same address the web server binds, never the name
|
||||
// `localhost` — a name resolves to one address family and would reintroduce
|
||||
// the #1951 split (web on IPv4, sandbox on IPv6) for any future call site
|
||||
// that omits `host`. Both call sites pass `config.sandboxHost` today.
|
||||
const { port, host = DEFAULT_BIND_HOST, allowedOrigins } = options;
|
||||
let server: Server | null = null;
|
||||
let sandboxUrl: string | null = null;
|
||||
|
||||
|
|
@ -177,8 +204,28 @@ export function createSandboxController(
|
|||
});
|
||||
res.end(sandboxHtml);
|
||||
});
|
||||
// A taken port used to mean "no sandbox at all", which was tolerable
|
||||
// only because the default was dynamic and therefore never collided.
|
||||
// Now that it's fixed (see DEFAULT_SANDBOX_PORT), a second Inspector —
|
||||
// or anything else already on 6275 — would take the Apps tab down, so
|
||||
// retry once on an OS-assigned port. The retry is announced loudly:
|
||||
// whoever pinned MCP_SANDBOX_PORT to forward it needs to know the pin
|
||||
// didn't take, and a silently-moved port is exactly the failure this
|
||||
// default was introduced to fix.
|
||||
let retriedDynamic = false;
|
||||
server.on("error", (err: NodeJS.ErrnoException) => {
|
||||
if (err.code === "EADDRINUSE") {
|
||||
if (!retriedDynamic && port !== 0) {
|
||||
retriedDynamic = true;
|
||||
console.warn(
|
||||
`Sandbox: port ${port} in use; falling back to an OS-assigned port. ` +
|
||||
`The MCP Apps tab will work locally, but the sandbox is no longer ` +
|
||||
`on a predictable port — set MCP_SANDBOX_PORT to a free one if you ` +
|
||||
`need to forward it (container / SSH tunnel / reverse proxy).`,
|
||||
);
|
||||
server!.listen(0, host);
|
||||
return;
|
||||
}
|
||||
console.error(
|
||||
`Sandbox: port ${port || "dynamic"} in use. MCP Apps tab may not work.`,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -173,4 +173,30 @@ describe("AppDetailPanel", () => {
|
|||
rerender(<AppDetailPanel {...baseProps} tool={numberFieldTool("app_b")} />);
|
||||
expect(input().value).toBe("");
|
||||
});
|
||||
|
||||
// #2020: `hasMissingRequiredFields` sees a field with no value, but text a
|
||||
// field could not turn into a value looks identical to it — both report
|
||||
// `undefined` — so an optional argument was dropped from the call with no
|
||||
// gate. The form reports draft validity for this reason.
|
||||
it("disables Open App while a field holds text it cannot send", async () => {
|
||||
const user = userEvent.setup();
|
||||
const jsonFieldTool: Tool = {
|
||||
name: "chart_app",
|
||||
title: "Chart App",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
properties: { series: { type: "array", title: "Series" } },
|
||||
},
|
||||
};
|
||||
renderWithMantine(<AppDetailPanel {...baseProps} tool={jsonFieldTool} />);
|
||||
const openApp = screen.getByRole("button", { name: /open app/i });
|
||||
expect(openApp).not.toBeDisabled();
|
||||
|
||||
const jsonInput = screen.getByLabelText(/Series/);
|
||||
await user.type(jsonInput, "x");
|
||||
expect(openApp).toBeDisabled();
|
||||
|
||||
await user.clear(jsonInput);
|
||||
expect(openApp).not.toBeDisabled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { Button, Divider, ScrollArea, Stack, Text } from "@mantine/core";
|
||||
import { useState } from "react";
|
||||
import { MdPlayArrow } from "react-icons/md";
|
||||
import type { Tool } from "@modelcontextprotocol/client";
|
||||
import { SchemaForm } from "../SchemaForm/SchemaForm";
|
||||
|
|
@ -58,7 +59,12 @@ export function AppDetailPanel({
|
|||
/* v8 ignore next -- unreachable: Tool.inputSchema is always an object */
|
||||
const formSchema = toFormSchema(inputSchema) ?? {};
|
||||
const hasErrors = hasMissingRequiredFields(formSchema, formValues);
|
||||
const disabled = isOpening || hasErrors;
|
||||
// A field holding text it could not turn into a value (unparseable JSON, an
|
||||
// unrepresentable number) reports `undefined`, which is indistinguishable
|
||||
// from empty once it reaches `formValues` — so the form reports it directly
|
||||
// and an optional argument can no longer be dropped silently (#2020).
|
||||
const [hasInvalidDraft, setHasInvalidDraft] = useState(false);
|
||||
const disabled = isOpening || hasErrors || hasInvalidDraft;
|
||||
const hasFields = hasInputFields(tool);
|
||||
|
||||
return (
|
||||
|
|
@ -82,6 +88,7 @@ export function AppDetailPanel({
|
|||
// tool's name to drop another app's in-progress field text. See
|
||||
// SchemaFormProps.resetKey.
|
||||
resetKey={tool.name}
|
||||
onValidityChange={setHasInvalidDraft}
|
||||
/>
|
||||
|
||||
<OpenAppButton
|
||||
|
|
|
|||
|
|
@ -126,4 +126,32 @@ describe("ElicitationFormPanel", () => {
|
|||
await user.type(screen.getByLabelText("Host"), "l");
|
||||
expect(onChange).toHaveBeenCalledWith({ host: "l" });
|
||||
});
|
||||
|
||||
// #2020: text a field cannot turn into a value reports `undefined`, so it is
|
||||
// indistinguishable from an empty field in `values` — Submit stayed enabled
|
||||
// and the answer went out without the property.
|
||||
it("disables Submit while a field holds text it cannot send", async () => {
|
||||
const user = userEvent.setup();
|
||||
const numberRequest: ElicitRequestFormParams = {
|
||||
message: "Please provide the port.",
|
||||
requestedSchema: {
|
||||
type: "object" as const,
|
||||
properties: { port: { type: "number" as const, title: "Port number" } },
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<ElicitationFormPanel {...baseProps} request={numberRequest} />,
|
||||
);
|
||||
const submit = screen.getByRole("button", { name: "Submit" });
|
||||
expect(submit).toBeEnabled();
|
||||
|
||||
// Past MAX_SAFE_INTEGER the field reports no value rather than a rounded
|
||||
// one, so the answer would go out without the property.
|
||||
const input = screen.getByLabelText(/Port number/);
|
||||
await user.type(input, "90071992547409910");
|
||||
expect(submit).toBeDisabled();
|
||||
|
||||
await user.clear(input);
|
||||
expect(submit).toBeEnabled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import {
|
|||
Stack,
|
||||
Text,
|
||||
} from "@mantine/core";
|
||||
import { useState } from "react";
|
||||
import type { ElicitRequestFormParams } from "@modelcontextprotocol/client";
|
||||
import type { PendingRequestOrigin } from "@inspector/core/mcp/types.js";
|
||||
import {
|
||||
|
|
@ -88,8 +89,13 @@ export function ElicitationFormPanel({
|
|||
busy = false,
|
||||
}: ElicitationFormPanelProps) {
|
||||
const requestedSchema = request.requestedSchema as InspectorFormSchema;
|
||||
// Text a field could not turn into a value never reaches `values`, so the
|
||||
// form has to report it for Submit to see it at all (#2020).
|
||||
const [hasInvalidDraft, setHasInvalidDraft] = useState(false);
|
||||
const submitDisabled =
|
||||
busy || hasMissingRequiredFields(requestedSchema, values);
|
||||
busy ||
|
||||
hasMissingRequiredFields(requestedSchema, values) ||
|
||||
hasInvalidDraft;
|
||||
return (
|
||||
<Stack gap="md">
|
||||
<QuotedMessage>{formatQuoted(request.message)}</QuotedMessage>
|
||||
|
|
@ -101,6 +107,7 @@ export function ElicitationFormPanel({
|
|||
values={values}
|
||||
onChange={onChange}
|
||||
disabled={busy}
|
||||
onValidityChange={setHasInvalidDraft}
|
||||
/>
|
||||
</FieldScroll>
|
||||
<WarningAlert>{formatWarning(serverName)}</WarningAlert>
|
||||
|
|
|
|||
|
|
@ -161,4 +161,70 @@ describe("InlineElicitationRequest", () => {
|
|||
);
|
||||
expect(screen.getByText("Host")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// #2020: this inline form had no submit gate at all — neither the missing
|
||||
// required answer that `ElicitationFormPanel` already checked, nor text a
|
||||
// field could not turn into a value (which never reaches `values`).
|
||||
describe("submit gating (#2020)", () => {
|
||||
const requiredRequest = {
|
||||
message: "Please provide your name.",
|
||||
requestedSchema: {
|
||||
type: "object" as const,
|
||||
properties: { name: { type: "string" as const, title: "Name" } },
|
||||
required: ["name"],
|
||||
},
|
||||
} satisfies ElicitRequest["params"];
|
||||
|
||||
const numberRequest = {
|
||||
message: "Please provide the port.",
|
||||
requestedSchema: {
|
||||
type: "object" as const,
|
||||
properties: { port: { type: "number" as const, title: "Port number" } },
|
||||
},
|
||||
} satisfies ElicitRequest["params"];
|
||||
|
||||
it("disables Submit while a required answer is missing", () => {
|
||||
renderWithMantine(
|
||||
<InlineElicitationRequest
|
||||
{...baseProps}
|
||||
request={requiredRequest}
|
||||
values={{}}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
|
||||
});
|
||||
|
||||
it("enables Submit once the required answer is given", () => {
|
||||
renderWithMantine(
|
||||
<InlineElicitationRequest
|
||||
{...baseProps}
|
||||
request={requiredRequest}
|
||||
values={{ name: "Ada" }}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
|
||||
});
|
||||
|
||||
it("disables Submit while a field holds text it cannot send", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<InlineElicitationRequest
|
||||
{...baseProps}
|
||||
request={numberRequest}
|
||||
values={{}}
|
||||
/>,
|
||||
);
|
||||
const submit = screen.getByRole("button", { name: "Submit" });
|
||||
expect(submit).toBeEnabled();
|
||||
|
||||
// Past MAX_SAFE_INTEGER the field reports no value rather than a rounded
|
||||
// one, so the answer would go out without the property.
|
||||
const input = screen.getByLabelText(/Port number/);
|
||||
await user.type(input, "90071992547409910");
|
||||
expect(submit).toBeDisabled();
|
||||
|
||||
await user.clear(input);
|
||||
expect(submit).toBeEnabled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,7 +13,11 @@ import type {
|
|||
ElicitRequestFormParams,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { PendingRequestOrigin } from "@inspector/core/mcp/types.js";
|
||||
import type { InspectorFormSchema } from "../../../utils/jsonUtils";
|
||||
import { useState } from "react";
|
||||
import {
|
||||
hasMissingRequiredFields,
|
||||
type InspectorFormSchema,
|
||||
} from "../../../utils/jsonUtils";
|
||||
import { SchemaForm } from "../SchemaForm/SchemaForm";
|
||||
import { MrtrOriginNote } from "../../elements/MrtrOriginNote/MrtrOriginNote";
|
||||
|
||||
|
|
@ -86,6 +90,19 @@ export function InlineElicitationRequest({
|
|||
onSubmit,
|
||||
onCancel,
|
||||
}: InlineElicitationRequestProps) {
|
||||
// Same gate as `ElicitationFormPanel`, which this is the inline form of: a
|
||||
// missing required answer, or a field holding text it could not turn into a
|
||||
// value, must not be submittable — neither is visible in `values` (#2020).
|
||||
const [hasInvalidDraft, setHasInvalidDraft] = useState(false);
|
||||
const formValues = values ?? {};
|
||||
const submitDisabled =
|
||||
isFormMode(request) &&
|
||||
(hasMissingRequiredFields(
|
||||
request.requestedSchema as InspectorFormSchema,
|
||||
formValues,
|
||||
) ||
|
||||
hasInvalidDraft);
|
||||
|
||||
return (
|
||||
<RequestContainer>
|
||||
<Stack gap="sm">
|
||||
|
|
@ -100,8 +117,9 @@ export function InlineElicitationRequest({
|
|||
{isFormMode(request) && (
|
||||
<SchemaForm
|
||||
schema={request.requestedSchema as InspectorFormSchema}
|
||||
values={values ?? {}}
|
||||
values={formValues}
|
||||
onChange={onChange}
|
||||
onValidityChange={setHasInvalidDraft}
|
||||
/>
|
||||
)}
|
||||
|
||||
|
|
@ -120,7 +138,7 @@ export function InlineElicitationRequest({
|
|||
<ActionsRow>
|
||||
<CompactButton onClick={onCancel}>Cancel</CompactButton>
|
||||
{isFormMode(request) && (
|
||||
<Button size="xs" onClick={onSubmit}>
|
||||
<Button size="xs" onClick={onSubmit} disabled={submitDisabled}>
|
||||
Submit
|
||||
</Button>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { useState } from "react";
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import type { InspectorFormSchema } from "../../../utils/jsonUtils";
|
||||
import { toFormSchema } from "../../../utils/jsonUtils";
|
||||
import {
|
||||
fireEvent,
|
||||
renderWithMantine,
|
||||
|
|
@ -737,7 +738,52 @@ describe("SchemaForm", () => {
|
|||
expect(lastCall.config).toEqual([1, 2]);
|
||||
});
|
||||
|
||||
it("falls back to passing raw string to onChange when JSON is invalid in JsonInput", async () => {
|
||||
// The JSON field used to store unparseable text back as the *value*, which
|
||||
// the next render re-stringified — so each keystroke added a layer of
|
||||
// escaping (`[` → `"["` → `"\"[\""`). That compounding escape is #1928's
|
||||
// original symptom, and it lived here rather than in the dispatch. It matters
|
||||
// more since #2007, whose fix deliberately routes object unions to this
|
||||
// editor: a fallback nobody can type into is not a fallback.
|
||||
it("shows an error while the JSON draft does not parse", async () => {
|
||||
const user = userEvent.setup();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
config: { type: "array", title: "Config" },
|
||||
},
|
||||
};
|
||||
function Harness() {
|
||||
const [values, setValues] = useState<Record<string, unknown>>({});
|
||||
return (
|
||||
<SchemaForm schema={schema} values={values} onChange={setValues} />
|
||||
);
|
||||
}
|
||||
renderWithMantine(<Harness />);
|
||||
|
||||
const jsonInput = screen.getByLabelText(/Config/) as HTMLTextAreaElement;
|
||||
await user.type(jsonInput, "[[1,");
|
||||
// Invalid text yields no value, so without this the field would submit as
|
||||
// absent while the user is still looking at what they typed.
|
||||
expect(screen.getByText(/Not valid JSON/)).toBeInTheDocument();
|
||||
|
||||
await user.type(jsonInput, "2]");
|
||||
expect(screen.queryByText(/Not valid JSON/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows no error for an empty optional field", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
config: { type: "array", title: "Config" },
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.queryByText(/Not valid JSON/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("reports no value, not raw text, while the JSON is mid-edit", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
|
|
@ -751,9 +797,59 @@ describe("SchemaForm", () => {
|
|||
);
|
||||
const jsonInput = screen.getByLabelText(/Config/) as HTMLTextAreaElement;
|
||||
await user.type(jsonInput, "x");
|
||||
expect(onChange).toHaveBeenCalled();
|
||||
const lastCall = onChange.mock.calls[onChange.mock.calls.length - 1][0];
|
||||
expect(typeof lastCall.config).toBe("string");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ config: undefined });
|
||||
});
|
||||
|
||||
it("lets an array literal be typed one character at a time", async () => {
|
||||
const user = userEvent.setup();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
config: { type: "array", title: "Config" },
|
||||
},
|
||||
};
|
||||
// Drive the real controlled loop: each onChange feeds straight back in as
|
||||
// `values`, which is what turned the old handler's raw-text write into a
|
||||
// compounding re-escape.
|
||||
function Harness() {
|
||||
const [values, setValues] = useState<Record<string, unknown>>({});
|
||||
return (
|
||||
<SchemaForm schema={schema} values={values} onChange={setValues} />
|
||||
);
|
||||
}
|
||||
renderWithMantine(<Harness />);
|
||||
|
||||
const jsonInput = screen.getByLabelText(/Config/) as HTMLTextAreaElement;
|
||||
// `[` is a userEvent keyboard descriptor, so it is escaped as `[[`.
|
||||
await user.type(jsonInput, '[[1,"a"]');
|
||||
|
||||
// The box shows exactly what was typed — no injected quotes or backslashes.
|
||||
expect(jsonInput.value).toBe('[1,"a"]');
|
||||
});
|
||||
|
||||
it("keeps an in-progress draft visible instead of rewriting it", async () => {
|
||||
const user = userEvent.setup();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
config: { type: "array", title: "Config" },
|
||||
},
|
||||
};
|
||||
function Harness() {
|
||||
const [values, setValues] = useState<Record<string, unknown>>({});
|
||||
return (
|
||||
<SchemaForm schema={schema} values={values} onChange={setValues} />
|
||||
);
|
||||
}
|
||||
renderWithMantine(<Harness />);
|
||||
|
||||
const jsonInput = screen.getByLabelText(/Config/) as HTMLTextAreaElement;
|
||||
await user.type(jsonInput, "[[1,");
|
||||
// Unparseable so far, and it must survive the re-render untouched.
|
||||
expect(jsonInput.value).toBe("[1,");
|
||||
|
||||
await user.type(jsonInput, "2]");
|
||||
expect(jsonInput.value).toBe("[1,2]");
|
||||
});
|
||||
|
||||
it("uses default values when value is undefined", () => {
|
||||
|
|
@ -811,3 +907,573 @@ describe("SchemaForm", () => {
|
|||
expect(container.firstChild).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// #1928: "optional AND explicitly nullable" (Zod's `.nullish()`) compiles to a
|
||||
// nullable union rather than a plain type. Before the normalization step these
|
||||
// matched no branch and fell through to the raw-JSON fallback, where every
|
||||
// keystroke re-escaped the value.
|
||||
describe("SchemaForm nullable unions", () => {
|
||||
it("renders a Select for an anyOf string-enum|null field", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
type: {
|
||||
title: "Type",
|
||||
anyOf: [
|
||||
{ type: "string", enum: ["envio", "recebimento"] },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||
);
|
||||
await user.click(screen.getByRole("textbox", { name: "Type" }));
|
||||
await user.click(
|
||||
await screen.findByRole("option", { name: "envio", hidden: true }),
|
||||
);
|
||||
expect(onChange).toHaveBeenCalledWith({ type: "envio" });
|
||||
});
|
||||
|
||||
it("clears a nullable enum back to null", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
type: {
|
||||
title: "Type",
|
||||
anyOf: [{ type: "string", enum: ["envio"] }, { type: "null" }],
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm
|
||||
schema={schema}
|
||||
values={{ type: "envio" }}
|
||||
onChange={onChange}
|
||||
/>,
|
||||
);
|
||||
// Mantine marks its combobox clear button `aria-hidden` (it is mouse-only,
|
||||
// `tabIndex={-1}`), so it is only reachable with `hidden: true`.
|
||||
await user.click(screen.getByRole("button", { hidden: true }));
|
||||
expect(onChange).toHaveBeenCalledWith({ type: null });
|
||||
});
|
||||
|
||||
it("offers no clear affordance on a non-nullable enum", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
format: { type: "string", title: "Format", enum: ["json"] },
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm
|
||||
schema={schema}
|
||||
values={{ format: "json" }}
|
||||
onChange={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(
|
||||
screen.queryByRole("button", { hidden: true }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
// The other supported nullable encoding: keywords stay at the top level, so
|
||||
// the null sentinel sits inside the enum list rather than on a branch.
|
||||
it("renders a Select for a type: [string, null] enum, without a null option", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
// Built through `toFormSchema`, the same narrowing boundary every
|
||||
// production call site uses, rather than cast into `InspectorFormSchema`.
|
||||
// A `null` member is valid JSON Schema but outside that type's `string[]`
|
||||
// `enum`, and this fixture *is* wire data — so the honest way to introduce
|
||||
// it is the wire→form narrow, not a double cast that erases the mismatch.
|
||||
const schema = toFormSchema({
|
||||
type: "object",
|
||||
properties: {
|
||||
direction: {
|
||||
title: "Direction",
|
||||
type: ["string", "null"],
|
||||
enum: ["envio", "recebimento", null],
|
||||
},
|
||||
},
|
||||
});
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema!} values={{}} onChange={onChange} />,
|
||||
);
|
||||
await user.click(screen.getByRole("textbox", { name: "Direction" }));
|
||||
const options = await screen.findAllByRole("option", { hidden: true });
|
||||
expect(options.map((option) => option.textContent)).toEqual([
|
||||
"envio",
|
||||
"recebimento",
|
||||
]);
|
||||
await user.click(options[0]);
|
||||
expect(onChange).toHaveBeenCalledWith({ direction: "envio" });
|
||||
});
|
||||
|
||||
it("renders a TextInput for a type: [string, null] field", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
note: { type: ["string", "null"], title: "Note" },
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||
);
|
||||
await user.type(screen.getByLabelText(/Note/), "a");
|
||||
expect(onChange).toHaveBeenCalledWith({ note: "a" });
|
||||
});
|
||||
|
||||
it("renders a checkbox for an anyOf boolean|null field", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
enabled: {
|
||||
title: "Enabled",
|
||||
anyOf: [{ type: "boolean" }, { type: "null" }],
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||
);
|
||||
await user.click(screen.getByLabelText("Enabled"));
|
||||
expect(onChange).toHaveBeenCalledWith({ enabled: true });
|
||||
});
|
||||
|
||||
it("renders a MultiSelect for an anyOf array-of-enum|null field", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
tags: {
|
||||
title: "Tags",
|
||||
anyOf: [
|
||||
{ type: "array", items: { type: "string", enum: ["a", "b"] } },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||
);
|
||||
await user.click(screen.getByRole("textbox", { name: "Tags" }));
|
||||
await user.click(
|
||||
await screen.findByRole("option", { name: "a", hidden: true }),
|
||||
);
|
||||
expect(onChange).toHaveBeenCalledWith({ tags: ["a"] });
|
||||
});
|
||||
|
||||
it("renders nested fields for an anyOf object|null field", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
profile: {
|
||||
title: "Profile",
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: { nick: { type: "string", title: "Nick" } },
|
||||
},
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||
);
|
||||
await user.type(screen.getByLabelText(/Nick/), "z");
|
||||
expect(onChange).toHaveBeenCalledWith({ profile: { nick: "z" } });
|
||||
});
|
||||
|
||||
// #2007: `z.array(z.union([z.object(…), z.object(…)]))` gives an `items.anyOf`
|
||||
// whose branches carry no top-level `const`, so every MultiSelect option was
|
||||
// the empty string — and Mantine *throws* on duplicate option values, greying
|
||||
// out the whole tool panel. The nullable form below is reachable only because
|
||||
// this PR now collapses it into `type: "array"`, so it has to be safe too.
|
||||
it("falls back to the JSON input for an array of object-union items", () => {
|
||||
const objectBranch = (name: string): InspectorFormSchema => ({
|
||||
type: "object",
|
||||
properties: { type: { type: "string", const: name } },
|
||||
required: ["type"],
|
||||
});
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
items: {
|
||||
title: "Items",
|
||||
type: "array",
|
||||
items: { anyOf: [objectBranch("A"), objectBranch("B")] },
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByLabelText(/Items/).tagName).toBe("TEXTAREA");
|
||||
});
|
||||
|
||||
it("falls back to the JSON input for a nullable array of object-union items", () => {
|
||||
const objectBranch = (name: string): InspectorFormSchema => ({
|
||||
type: "object",
|
||||
properties: { type: { type: "string", const: name } },
|
||||
required: ["type"],
|
||||
});
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
items: {
|
||||
title: "Items",
|
||||
anyOf: [
|
||||
{ type: "array", items: { anyOf: [objectBranch("A")] } },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByLabelText(/Items/).tagName).toBe("TEXTAREA");
|
||||
});
|
||||
|
||||
// Mantine's select is string-valued, so a numeric const would submit "1" for
|
||||
// 1 — the same wrong-type-on-the-wire problem that keeps a numeric enum off
|
||||
// the select path.
|
||||
it("falls back to the JSON input for non-string anyOf consts", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
items: {
|
||||
title: "Items",
|
||||
type: "array",
|
||||
items: { anyOf: [{ const: 1 }, { const: 2 }] },
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByLabelText(/Items/).tagName).toBe("TEXTAREA");
|
||||
});
|
||||
|
||||
it("falls back to the JSON input when two anyOf branches share a const", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
items: {
|
||||
title: "Items",
|
||||
type: "array",
|
||||
items: { anyOf: [{ const: "dup" }, { const: "dup" }] },
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByLabelText(/Items/).tagName).toBe("TEXTAREA");
|
||||
});
|
||||
|
||||
it("falls back to a text input for a string oneOf with no consts", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
mode: {
|
||||
title: "Mode",
|
||||
type: "string",
|
||||
oneOf: [{ type: "string" }, { type: "string" }],
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByLabelText(/Mode/).tagName).toBe("INPUT");
|
||||
});
|
||||
|
||||
it("still renders a MultiSelect when every anyOf branch has a distinct const", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
items: {
|
||||
title: "Items",
|
||||
type: "array",
|
||||
items: { anyOf: [{ const: "a", title: "Alpha" }, { const: "b" }] },
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByRole("textbox", { name: "Items" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Reachable only since the collapse landed: the nullable wrapper had no
|
||||
// top-level `type` before, so it fell to the JSON editor rather than into a
|
||||
// string-valued MultiSelect that would submit ["1"] for [1].
|
||||
it("falls back to the JSON input for a nullable array of numeric item enums", () => {
|
||||
// Wire data again — a numeric `enum` is valid JSON Schema but outside
|
||||
// `InspectorFormSchema`'s `string[]`, so it comes in through the narrow.
|
||||
const schema = toFormSchema({
|
||||
type: "object",
|
||||
properties: {
|
||||
levels: {
|
||||
title: "Levels",
|
||||
anyOf: [{ type: "array", items: { enum: [1, 2] } }, { type: "null" }],
|
||||
},
|
||||
},
|
||||
});
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema!} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByLabelText(/Levels/).tagName).toBe("TEXTAREA");
|
||||
});
|
||||
|
||||
// The sibling enum rules null out even though the type list names it, so the
|
||||
// field must not get a clear button that emits a value the schema rejects.
|
||||
it("offers no clear button when a sibling enum excludes null", () => {
|
||||
const schema = toFormSchema({
|
||||
type: "object",
|
||||
properties: {
|
||||
direction: {
|
||||
title: "Direction",
|
||||
type: ["string", "null"],
|
||||
enum: ["envio", "recebimento"],
|
||||
},
|
||||
},
|
||||
});
|
||||
renderWithMantine(
|
||||
<SchemaForm
|
||||
schema={schema!}
|
||||
values={{ direction: "envio" }}
|
||||
onChange={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(
|
||||
screen.queryByRole("button", { hidden: true }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("still falls back to the JSON input for a union of two real types", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
mixed: {
|
||||
title: "Mixed",
|
||||
anyOf: [{ type: "string" }, { type: "number" }],
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByLabelText(/Mixed/).tagName).toBe("TEXTAREA");
|
||||
});
|
||||
});
|
||||
|
||||
// #2020: a field holding text it cannot turn into a value reports `undefined`,
|
||||
// which is exactly what an *empty* field reports. That makes the two states
|
||||
// indistinguishable to the caller, so invalid text in an optional field was
|
||||
// submittable and simply arrived at the server absent. The form reports draft
|
||||
// validity directly so a submit gate can see what `values` cannot.
|
||||
describe("SchemaForm draft validity (#2020)", () => {
|
||||
const jsonSchema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
config: { type: "array", title: "Config" },
|
||||
},
|
||||
};
|
||||
|
||||
function ValidityHarness({
|
||||
schema,
|
||||
onValidityChange,
|
||||
resetKey,
|
||||
}: {
|
||||
schema: InspectorFormSchema;
|
||||
onValidityChange: (hasInvalidDraft: boolean) => void;
|
||||
resetKey?: string;
|
||||
}) {
|
||||
const [values, setValues] = useState<Record<string, unknown>>({});
|
||||
return (
|
||||
<SchemaForm
|
||||
schema={schema}
|
||||
values={values}
|
||||
onChange={setValues}
|
||||
resetKey={resetKey}
|
||||
onValidityChange={onValidityChange}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
it("reports an empty optional field as valid", () => {
|
||||
const onValidityChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<ValidityHarness
|
||||
schema={jsonSchema}
|
||||
onValidityChange={onValidityChange}
|
||||
/>,
|
||||
);
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(false);
|
||||
});
|
||||
|
||||
it("reports an unparseable JSON draft, then clears it once the text parses", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onValidityChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<ValidityHarness
|
||||
schema={jsonSchema}
|
||||
onValidityChange={onValidityChange}
|
||||
/>,
|
||||
);
|
||||
|
||||
const jsonInput = screen.getByLabelText(/Config/) as HTMLTextAreaElement;
|
||||
await user.type(jsonInput, "[[1,");
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(true);
|
||||
|
||||
await user.type(jsonInput, "2]");
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(false);
|
||||
});
|
||||
|
||||
it("reports a number this client cannot send exactly, and says so on the field", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onValidityChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
divisor: { type: "number", title: "Divisor" },
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<ValidityHarness schema={schema} onValidityChange={onValidityChange} />,
|
||||
);
|
||||
|
||||
// Past MAX_SAFE_INTEGER the field reports no value rather than a rounded
|
||||
// one, so — like the JSON editor — the text on screen would otherwise be
|
||||
// submitted as an absent argument.
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
await user.type(input, "90071992547409910");
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(true);
|
||||
expect(screen.getByText(/this field will be omitted/)).toBeInTheDocument();
|
||||
|
||||
await user.clear(input);
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(false);
|
||||
expect(
|
||||
screen.queryByText(/this field will be omitted/),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("stays invalid while any one field is invalid", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onValidityChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
first: { type: "array", title: "First" },
|
||||
second: { type: "array", title: "Second" },
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<ValidityHarness schema={schema} onValidityChange={onValidityChange} />,
|
||||
);
|
||||
|
||||
await user.type(screen.getByLabelText(/First/), "x");
|
||||
await user.type(screen.getByLabelText(/Second/), "y");
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(true);
|
||||
|
||||
// One of the two recovering is not enough.
|
||||
await user.clear(screen.getByLabelText(/First/));
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(true);
|
||||
|
||||
await user.clear(screen.getByLabelText(/Second/));
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(false);
|
||||
});
|
||||
|
||||
it("surfaces a nested object's invalid draft through the outer form", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onValidityChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
outer: {
|
||||
type: "object",
|
||||
title: "Outer",
|
||||
properties: {
|
||||
config: { type: "array", title: "Config" },
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<ValidityHarness schema={schema} onValidityChange={onValidityChange} />,
|
||||
);
|
||||
|
||||
await user.type(screen.getByLabelText(/Config/), "x");
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(true);
|
||||
});
|
||||
|
||||
it("clears a stale invalid draft when the form moves to another entity", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onValidityChange = vi.fn();
|
||||
const { rerender } = renderWithMantine(
|
||||
<ValidityHarness
|
||||
schema={jsonSchema}
|
||||
onValidityChange={onValidityChange}
|
||||
resetKey="first_tool"
|
||||
/>,
|
||||
);
|
||||
|
||||
await user.type(screen.getByLabelText(/Config/), "x");
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(true);
|
||||
|
||||
// Switching entities remounts the field, discarding its draft — so text
|
||||
// typed for the previous one must not keep the new one blocked.
|
||||
rerender(
|
||||
<ValidityHarness
|
||||
schema={jsonSchema}
|
||||
onValidityChange={onValidityChange}
|
||||
resetKey="second_tool"
|
||||
/>,
|
||||
);
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(false);
|
||||
});
|
||||
|
||||
it("reports valid when the form unmounts holding an invalid draft", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onValidityChange = vi.fn();
|
||||
const { unmount } = renderWithMantine(
|
||||
<ValidityHarness
|
||||
schema={jsonSchema}
|
||||
onValidityChange={onValidityChange}
|
||||
/>,
|
||||
);
|
||||
|
||||
await user.type(screen.getByLabelText(/Config/), "x");
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(true);
|
||||
|
||||
unmount();
|
||||
expect(onValidityChange).toHaveBeenLastCalledWith(false);
|
||||
});
|
||||
|
||||
it("renders without a validity callback", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={jsonSchema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
await user.type(screen.getByLabelText(/Config/), "x");
|
||||
expect(screen.getByText(/Not valid JSON/)).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,10 +8,17 @@ import {
|
|||
Text,
|
||||
TextInput,
|
||||
} from "@mantine/core";
|
||||
import { useState } from "react";
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { ClearButton } from "../../elements/ClearButton/ClearButton";
|
||||
import { useValueChange } from "../../../hooks/useValueChange";
|
||||
import type { InspectorFormSchema } from "../../../utils/jsonUtils";
|
||||
import type {
|
||||
InspectorFormSchema,
|
||||
JsonSchemaConst,
|
||||
} from "../../../utils/jsonUtils";
|
||||
import {
|
||||
isStringEnum,
|
||||
normalizeNullableUnion,
|
||||
} from "@inspector/core/json/nullableUnion.js";
|
||||
|
||||
const FieldLabel = Text.withProps({
|
||||
fw: 500,
|
||||
|
|
@ -51,6 +58,58 @@ function toEnumData(
|
|||
return values;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build `Select`/`MultiSelect` option data from a list of `oneOf`/`anyOf`
|
||||
* branches that are expected to be constants.
|
||||
*
|
||||
* Returns `null` when the branches are not usable as options, which sends the
|
||||
* field to the JSON fallback instead. Four ways that happens:
|
||||
*
|
||||
* - **A non-string `const`.** Mantine's select is string-valued, so
|
||||
* `anyOf: [{ const: 1 }, { const: 2 }]` would submit `["1"]` where the
|
||||
* schema says `[1]` — the same wrong-type-on-the-wire problem that keeps a
|
||||
* numeric `enum` off the select path. An inspector must not misreport what
|
||||
* it sends, so this stays on the JSON editor, where the value keeps its type.
|
||||
* - **No `const` at all.** An `anyOf` of *object* schemas — what
|
||||
* `z.array(z.union([z.object(…), z.object(…)]))` compiles to — has no
|
||||
* top-level `const` on any branch, so every option would be the empty
|
||||
* string. Mantine **throws** on duplicate option values, which greys out the
|
||||
* whole tool panel rather than degrading (#2007).
|
||||
* - **Duplicate values.** Two branches sharing a `const` throw the same way.
|
||||
* - **An empty option value**, which Mantine cannot render as selectable.
|
||||
*
|
||||
* A union of object shapes has no faithful dropdown anyway, so the JSON editor
|
||||
* is the honest widget for it rather than a lossy or crashing one.
|
||||
*/
|
||||
function toConstOptions(
|
||||
branches: (InspectorFormSchema | JsonSchemaConst)[],
|
||||
): { value: string; label: string }[] | null {
|
||||
const options: { value: string; label: string }[] = [];
|
||||
const seen = new Set<string>();
|
||||
for (const branch of branches) {
|
||||
if (typeof branch.const !== "string") {
|
||||
return null;
|
||||
}
|
||||
const value = branch.const;
|
||||
if (value === "" || seen.has(value)) {
|
||||
return null;
|
||||
}
|
||||
seen.add(value);
|
||||
options.push({ value, label: branch.title ?? value });
|
||||
}
|
||||
return options.length > 0 ? options : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether an enum `Select` should offer a clear affordance. Only a schema that
|
||||
* admits `null` does: clearing emits `null`, and an enum without a null branch
|
||||
* would reject it. Without this a nullable enum is a one-way door — once a
|
||||
* value is picked there is no way back to "no answer".
|
||||
*/
|
||||
function isClearable(fieldSchema: InspectorFormSchema): boolean {
|
||||
return fieldSchema.nullable === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Interpret whatever Mantine's `NumberInput` reported as the JSON value for the
|
||||
* field. Anything that is not a finite number becomes `undefined`, which is how
|
||||
|
|
@ -90,7 +149,162 @@ function toNumericValue(raw: string | number): number | undefined {
|
|||
return Number.isSafeInteger(Math.trunc(parsed)) ? parsed : undefined;
|
||||
}
|
||||
|
||||
/** Parse editor text, reporting `undefined` for anything that is not JSON yet. */
|
||||
function parseJsonDraft(text: string): unknown {
|
||||
if (text.trim() === "") {
|
||||
return undefined;
|
||||
}
|
||||
try {
|
||||
return JSON.parse(text);
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/** Structural equality for the draft/value re-sync, via canonical JSON. */
|
||||
function isSameJson(a: unknown, b: unknown): boolean {
|
||||
if (a === undefined || b === undefined) {
|
||||
return a === b;
|
||||
}
|
||||
return serializeJson(a) === serializeJson(b);
|
||||
}
|
||||
|
||||
/**
|
||||
* How a field tells the enclosing `SchemaForm` whether the text it is holding
|
||||
* can be sent. Keyed by field name rather than closed over per field so the
|
||||
* reporter is referentially stable, which is what keeps the effect below from
|
||||
* re-subscribing on every render.
|
||||
*/
|
||||
type DraftValidityReporter = (fieldName: string, isValid: boolean) => void;
|
||||
|
||||
/**
|
||||
* Publish a field's draft validity to the enclosing `SchemaForm`.
|
||||
*
|
||||
* Reported from an effect rather than from the change handler because validity
|
||||
* also moves when the *parent* replaces the value — a cleared form or a loaded
|
||||
* example re-syncs the draft and makes invalid text valid again without the
|
||||
* user touching the field. Reporting during that render instead would mean
|
||||
* updating another component mid-render, which React rejects.
|
||||
*
|
||||
* The cleanup reports the field as valid again, so a field that goes away —
|
||||
* unmounted with the panel, or remounted under a new `resetKey` when the form
|
||||
* moves to another entity — cannot leave a stale draft blocking submission.
|
||||
*/
|
||||
function useDraftValidity(
|
||||
fieldName: string,
|
||||
isValid: boolean,
|
||||
onValidityChange: DraftValidityReporter,
|
||||
): void {
|
||||
useEffect(() => {
|
||||
onValidityChange(fieldName, isValid);
|
||||
return () => onValidityChange(fieldName, true);
|
||||
}, [fieldName, isValid, onValidityChange]);
|
||||
}
|
||||
|
||||
interface SchemaJsonFieldProps {
|
||||
fieldName: string;
|
||||
label: string;
|
||||
description?: string;
|
||||
withAsterisk: boolean;
|
||||
disabled: boolean;
|
||||
value: unknown;
|
||||
onChange: (value: unknown) => void;
|
||||
onValidityChange: DraftValidityReporter;
|
||||
}
|
||||
|
||||
/**
|
||||
* A `JsonInput` that keeps the text the user is typing, not just the value it
|
||||
* currently parses to.
|
||||
*
|
||||
* This field used to drive the box straight off the parent's value and, when
|
||||
* the text failed `JSON.parse`, store the **raw text** back as the value — which
|
||||
* the next render re-`JSON.stringify`d, adding a layer of escaping per
|
||||
* keystroke. Typing `[` gave `"["`, then `"\"[\""`, and the field was unusable
|
||||
* within a few characters. That compounding escape is the original #1928
|
||||
* symptom, and it lived in this fallback rather than in the dispatch.
|
||||
*
|
||||
* Routing nullable fields to real widgets removed the common way of *landing*
|
||||
* here, but it did not fix the editor — and #2007's fix deliberately sends a
|
||||
* union of object shapes to it, so the editor itself has to be typeable. Hence
|
||||
* the same split `SchemaNumberInput` uses: the raw text is the source of truth
|
||||
* for what is *displayed*, while the parent only ever sees parsed JSON.
|
||||
*
|
||||
* While the text is mid-edit and does not parse, the parent is told
|
||||
* `undefined` rather than handed the text. An inspector must not report a value
|
||||
* it cannot send, and `undefined` is how "no value supplied" is spelled
|
||||
* everywhere else in this form.
|
||||
*
|
||||
* That leaves invalid text and an empty field indistinguishable *to the value*,
|
||||
* so the field reports the difference two other ways: it says so on itself via
|
||||
* `error`, and it reports draft validity up through `onValidityChange` so the
|
||||
* enclosing form's caller can disable Execute/Open App/Submit (#2020). Without
|
||||
* the second channel an **optional** field's invalid text is simply dropped
|
||||
* from a submission the user was allowed to make.
|
||||
*/
|
||||
function SchemaJsonField({
|
||||
fieldName,
|
||||
value,
|
||||
onChange,
|
||||
onValidityChange,
|
||||
...inputProps
|
||||
}: SchemaJsonFieldProps) {
|
||||
const [draft, setDraft] = useState(() =>
|
||||
value === undefined ? "" : serializeJson(value),
|
||||
);
|
||||
|
||||
// Re-sync only when the parent's value genuinely diverges from what the draft
|
||||
// parses to, which leaves an external reset (a cleared form, a loaded
|
||||
// example) working while an in-progress `[` — whose parse is `undefined`,
|
||||
// matching the `undefined` we just emitted — is left alone.
|
||||
useValueChange(value, (next) => {
|
||||
if (!isSameJson(parseJsonDraft(draft), next)) {
|
||||
setDraft(next === undefined ? "" : serializeJson(next));
|
||||
}
|
||||
});
|
||||
|
||||
// Text that is present but does not parse yields no value, so the field would
|
||||
// otherwise submit as absent while the user is still looking at what they
|
||||
// typed. Saying so on the field keeps that from being silent; reporting it
|
||||
// upward is what keeps it from being submittable.
|
||||
const hasInvalidDraft =
|
||||
draft.trim() !== "" && parseJsonDraft(draft) === undefined;
|
||||
useDraftValidity(fieldName, !hasInvalidDraft, onValidityChange);
|
||||
|
||||
return (
|
||||
<SchemaJsonInput
|
||||
{...inputProps}
|
||||
value={draft}
|
||||
error={
|
||||
hasInvalidDraft
|
||||
? "Not valid JSON — this field will be omitted"
|
||||
: undefined
|
||||
}
|
||||
onChange={(text) => {
|
||||
setDraft(text);
|
||||
onChange(parseJsonDraft(text));
|
||||
}}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a number field's draft is text that cannot be sent.
|
||||
*
|
||||
* Anything non-empty that `toNumericValue` declines is invalid: a half-typed
|
||||
* `"-"`, and — the case that matters — an integer past `MAX_SAFE_INTEGER`,
|
||||
* which is deliberately dropped rather than silently rounded (see
|
||||
* `toNumericValue`). Both would otherwise submit as an absent argument while
|
||||
* the text sits in the box.
|
||||
*/
|
||||
function hasInvalidNumericDraft(draft: string | number): boolean {
|
||||
if (typeof draft === "number") {
|
||||
return false;
|
||||
}
|
||||
return draft.trim() !== "" && toNumericValue(draft) === undefined;
|
||||
}
|
||||
|
||||
interface SchemaNumberInputProps {
|
||||
fieldName: string;
|
||||
label: string;
|
||||
description?: string;
|
||||
withAsterisk: boolean;
|
||||
|
|
@ -100,6 +314,7 @@ interface SchemaNumberInputProps {
|
|||
max?: number;
|
||||
allowDecimal: boolean;
|
||||
onChange: (value: number | undefined) => void;
|
||||
onValidityChange: DraftValidityReporter;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -126,8 +341,10 @@ interface SchemaNumberInputProps {
|
|||
* prop for the case it covers.
|
||||
*/
|
||||
function SchemaNumberInput({
|
||||
fieldName,
|
||||
value,
|
||||
onChange,
|
||||
onValidityChange,
|
||||
...inputProps
|
||||
}: SchemaNumberInputProps) {
|
||||
const [draft, setDraft] = useState<string | number>(value ?? "");
|
||||
|
|
@ -138,10 +355,19 @@ function SchemaNumberInput({
|
|||
}
|
||||
});
|
||||
|
||||
// Same split as the JSON field: text this client cannot send reports no value,
|
||||
// so it has to say so on the field and report it upward, or the argument is
|
||||
// dropped from a submission the user was allowed to make (#2020).
|
||||
const invalidDraft = hasInvalidNumericDraft(draft);
|
||||
useDraftValidity(fieldName, !invalidDraft, onValidityChange);
|
||||
|
||||
return (
|
||||
<NumberInput
|
||||
{...inputProps}
|
||||
value={draft}
|
||||
error={
|
||||
invalidDraft ? "Not a number — this field will be omitted" : undefined
|
||||
}
|
||||
onChange={(next) => {
|
||||
setDraft(next);
|
||||
onChange(toNumericValue(next));
|
||||
|
|
@ -174,6 +400,21 @@ export interface SchemaFormProps {
|
|||
* substitute — callers rebuild it every render, so its identity is unstable.
|
||||
*/
|
||||
resetKey?: string;
|
||||
/**
|
||||
* Called with `true` while any field is holding text that cannot be sent —
|
||||
* unparseable JSON, or a number this client cannot represent exactly. Both
|
||||
* report their value as `undefined`, which is indistinguishable from an empty
|
||||
* field once it reaches `values`, so a caller cannot derive this from the
|
||||
* values it already has (#2020).
|
||||
*
|
||||
* Gate the submit action on it: `hasMissingRequiredFields` covers a field with
|
||||
* *no* value, and this covers a field whose visible text produced none. An
|
||||
* empty optional field is unaffected — it stays valid and submittable.
|
||||
*
|
||||
* The callback is read through a ref, so an inline closure is fine — it is
|
||||
* called when the answer changes, not on every render.
|
||||
*/
|
||||
onValidityChange?: (hasInvalidDraft: boolean) => void;
|
||||
}
|
||||
|
||||
function getDefaultValue(fieldSchema: InspectorFormSchema): unknown {
|
||||
|
|
@ -199,15 +440,70 @@ export function SchemaForm({
|
|||
onChange,
|
||||
disabled = false,
|
||||
resetKey,
|
||||
onValidityChange,
|
||||
}: SchemaFormProps) {
|
||||
const properties = schema.properties ?? {};
|
||||
const requiredFields = schema.required ?? [];
|
||||
|
||||
// The names of fields currently holding unsendable text. Held here rather
|
||||
// than in each field because only the form sees them all, and only the form
|
||||
// knows when the last one cleared.
|
||||
const [invalidFields, setInvalidFields] = useState<ReadonlySet<string>>(
|
||||
() => new Set(),
|
||||
);
|
||||
|
||||
// Stable so a field's reporting effect subscribes once, not per render. The
|
||||
// updater returns the previous set unchanged when nothing moved, which is
|
||||
// what makes a nested form's inline callback safe to call repeatedly.
|
||||
const reportFieldValidity = useCallback<DraftValidityReporter>(
|
||||
(fieldName, isValid) => {
|
||||
setInvalidFields((previous) => {
|
||||
if (previous.has(fieldName) === !isValid) {
|
||||
return previous;
|
||||
}
|
||||
const next = new Set(previous);
|
||||
if (isValid) {
|
||||
next.delete(fieldName);
|
||||
} else {
|
||||
next.add(fieldName);
|
||||
}
|
||||
return next;
|
||||
});
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
// Read through a ref so the callback's identity is not a dependency. It has
|
||||
// to be one or the other, and a *stable* dependency is what this needs: a
|
||||
// nested form is handed a fresh closure every render, and re-running the
|
||||
// effect for that alone would toggle the parent's state (cleanup, then
|
||||
// re-report), re-render this form, and loop forever.
|
||||
const notifyValidity = useRef(onValidityChange);
|
||||
useEffect(() => {
|
||||
notifyValidity.current = onValidityChange;
|
||||
});
|
||||
|
||||
// The cleanup mirrors the one in `useDraftValidity`: a form that is no longer
|
||||
// rendered holds no drafts, so an unmounted nested form must not leave the
|
||||
// outer one blocked. It runs in the same commit as the re-report, so the
|
||||
// transient `false` is never rendered.
|
||||
const hasInvalidDraft = invalidFields.size > 0;
|
||||
useEffect(() => {
|
||||
notifyValidity.current?.(hasInvalidDraft);
|
||||
return () => notifyValidity.current?.(false);
|
||||
}, [hasInvalidDraft]);
|
||||
|
||||
function handleFieldChange(fieldName: string, fieldValue: unknown) {
|
||||
onChange({ ...values, [fieldName]: fieldValue });
|
||||
}
|
||||
|
||||
function renderField(fieldName: string, fieldSchema: InspectorFormSchema) {
|
||||
function renderField(fieldName: string, rawSchema: InspectorFormSchema) {
|
||||
// Flatten a nullable union (`anyOf: [X, {type:"null"}]`, `type: [X,"null"]`)
|
||||
// before dispatching. Every branch below tests a single `type` string, so
|
||||
// without this an "optional and explicitly nullable" field — what Zod's
|
||||
// `.nullish()` emits — matches nothing and falls through to the raw-JSON
|
||||
// fallback, which is unusable for a value the user has to type (#1928).
|
||||
const fieldSchema = normalizeNullableUnion(rawSchema);
|
||||
const isRequired = requiredFields.includes(fieldName);
|
||||
const label = fieldSchema.title ?? fieldName;
|
||||
const description = fieldSchema.description;
|
||||
|
|
@ -223,6 +519,7 @@ export function SchemaForm({
|
|||
withAsterisk={isRequired}
|
||||
disabled={disabled}
|
||||
data={toEnumData(fieldSchema.enum, fieldSchema.enumNames)}
|
||||
clearable={isClearable(fieldSchema)}
|
||||
value={(rawValue as string) ?? null}
|
||||
onChange={(val) => handleFieldChange(fieldName, val)}
|
||||
/>
|
||||
|
|
@ -230,11 +527,11 @@ export function SchemaForm({
|
|||
}
|
||||
|
||||
// string with oneOf
|
||||
if (fieldSchema.type === "string" && fieldSchema.oneOf) {
|
||||
const data = fieldSchema.oneOf.map((item) => ({
|
||||
value: String(item.const ?? ""),
|
||||
label: item.title ?? String(item.const ?? ""),
|
||||
}));
|
||||
const oneOfData = fieldSchema.oneOf
|
||||
? toConstOptions(fieldSchema.oneOf)
|
||||
: null;
|
||||
if (fieldSchema.type === "string" && oneOfData) {
|
||||
const data = oneOfData;
|
||||
return (
|
||||
<Select
|
||||
key={fieldName}
|
||||
|
|
@ -243,6 +540,7 @@ export function SchemaForm({
|
|||
withAsterisk={isRequired}
|
||||
disabled={disabled}
|
||||
data={data}
|
||||
clearable={isClearable(fieldSchema)}
|
||||
value={(rawValue as string) ?? null}
|
||||
onChange={(val) => handleFieldChange(fieldName, val)}
|
||||
/>
|
||||
|
|
@ -281,6 +579,7 @@ export function SchemaForm({
|
|||
// The only field holding local state, so the only one that has to be
|
||||
// remounted when `resetKey` says the form moved to another entity.
|
||||
key={resetKey === undefined ? fieldName : `${resetKey}:${fieldName}`}
|
||||
fieldName={fieldName}
|
||||
label={label}
|
||||
description={description}
|
||||
withAsterisk={isRequired}
|
||||
|
|
@ -292,6 +591,7 @@ export function SchemaForm({
|
|||
// accepting a value the schema forbids.
|
||||
allowDecimal={fieldSchema.type === "number"}
|
||||
onChange={(val) => handleFieldChange(fieldName, val)}
|
||||
onValidityChange={reportFieldValidity}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -312,12 +612,14 @@ export function SchemaForm({
|
|||
);
|
||||
}
|
||||
|
||||
// array of enum values (multi-select)
|
||||
if (fieldSchema.type === "array" && fieldSchema.items?.enum) {
|
||||
const data = toEnumData(
|
||||
fieldSchema.items.enum,
|
||||
fieldSchema.items.enumNames,
|
||||
);
|
||||
// array of enum values (multi-select). Gated on the members being strings
|
||||
// for the same reason `toConstOptions` is: `MultiSelect` is string-valued,
|
||||
// so `items: { enum: [1, 2] }` would submit `["1"]` where the schema says
|
||||
// `[1]`. Reachable from a nullable array since the collapse landed, which
|
||||
// is what makes the guard load-bearing rather than theoretical.
|
||||
const itemsEnum = fieldSchema.items?.enum;
|
||||
if (fieldSchema.type === "array" && isStringEnum(itemsEnum)) {
|
||||
const data = toEnumData(itemsEnum, fieldSchema.items?.enumNames);
|
||||
return (
|
||||
<MultiSelect
|
||||
key={fieldName}
|
||||
|
|
@ -333,11 +635,11 @@ export function SchemaForm({
|
|||
}
|
||||
|
||||
// array with items having anyOf
|
||||
if (fieldSchema.type === "array" && fieldSchema.items?.anyOf) {
|
||||
const data = fieldSchema.items.anyOf.map((item) => ({
|
||||
value: String(item.const ?? ""),
|
||||
label: item.title ?? String(item.const ?? ""),
|
||||
}));
|
||||
const itemsAnyOfData = fieldSchema.items?.anyOf
|
||||
? toConstOptions(fieldSchema.items.anyOf)
|
||||
: null;
|
||||
if (fieldSchema.type === "array" && itemsAnyOfData) {
|
||||
const data = itemsAnyOfData;
|
||||
return (
|
||||
<MultiSelect
|
||||
key={fieldName}
|
||||
|
|
@ -368,6 +670,11 @@ export function SchemaForm({
|
|||
disabled={disabled}
|
||||
// Sub-fields belong to the same entity, so they reset with it.
|
||||
resetKey={resetKey}
|
||||
// A nested form's invalid draft is the outer form's invalid draft,
|
||||
// so it reports through the same channel under this field's name.
|
||||
onValidityChange={(nestedInvalid) =>
|
||||
reportFieldValidity(fieldName, !nestedInvalid)
|
||||
}
|
||||
/>
|
||||
</IndentedStack>
|
||||
</Stack>
|
||||
|
|
@ -376,20 +683,18 @@ export function SchemaForm({
|
|||
|
||||
// fallback: JsonInput for complex schemas
|
||||
return (
|
||||
<SchemaJsonInput
|
||||
key={fieldName}
|
||||
<SchemaJsonField
|
||||
// Holds local draft state, so — like the number field — it has to be
|
||||
// remounted when `resetKey` says the form moved to another entity.
|
||||
key={resetKey === undefined ? fieldName : `${resetKey}:${fieldName}`}
|
||||
fieldName={fieldName}
|
||||
label={label}
|
||||
description={description}
|
||||
withAsterisk={isRequired}
|
||||
disabled={disabled}
|
||||
value={rawValue !== undefined ? serializeJson(rawValue) : ""}
|
||||
onChange={(val) => {
|
||||
try {
|
||||
handleFieldChange(fieldName, JSON.parse(val));
|
||||
} catch {
|
||||
handleFieldChange(fieldName, val);
|
||||
}
|
||||
}}
|
||||
value={rawValue}
|
||||
onChange={(val) => handleFieldChange(fieldName, val)}
|
||||
onValidityChange={reportFieldValidity}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -218,7 +218,13 @@ describe("ToolDetailPanel", () => {
|
|||
});
|
||||
|
||||
it("renders the Execute Tool button enabled when not executing", () => {
|
||||
renderWithMantine(<ToolDetailPanel {...baseProps} tool={simpleTool} />);
|
||||
renderWithMantine(
|
||||
<ToolDetailPanel
|
||||
{...baseProps}
|
||||
tool={simpleTool}
|
||||
formValues={{ message: "hi" }}
|
||||
/>,
|
||||
);
|
||||
const button = screen.getByRole("button", { name: "Execute Tool" });
|
||||
expect(button).toBeInTheDocument();
|
||||
expect(button).not.toBeDisabled();
|
||||
|
|
@ -231,6 +237,7 @@ describe("ToolDetailPanel", () => {
|
|||
<ToolDetailPanel
|
||||
{...baseProps}
|
||||
tool={simpleTool}
|
||||
formValues={{ message: "hi" }}
|
||||
onExecute={onExecute}
|
||||
/>,
|
||||
);
|
||||
|
|
@ -502,4 +509,61 @@ describe("ToolDetailPanel", () => {
|
|||
).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
// #2020: Execute used to be gated on `isExecuting` alone — this panel never
|
||||
// called `hasMissingRequiredFields`, unlike the App and elicitation panels —
|
||||
// so arguments that could not be sent were executed anyway and simply arrived
|
||||
// absent at the server.
|
||||
describe("submit gating (#2020)", () => {
|
||||
const optionalJsonTool: Tool = {
|
||||
name: "record_shipment",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
// An array of no declared item type renders through the JSON editor,
|
||||
// which is where a draft can fail to parse.
|
||||
properties: { payload: { type: "array", title: "Payload" } },
|
||||
},
|
||||
};
|
||||
|
||||
it("disables Execute while a required argument has no value", () => {
|
||||
renderWithMantine(
|
||||
<ToolDetailPanel {...baseProps} tool={simpleTool} formValues={{}} />,
|
||||
);
|
||||
expect(
|
||||
screen.getByRole("button", { name: "Execute Tool" }),
|
||||
).toBeDisabled();
|
||||
});
|
||||
|
||||
it("enables Execute once the required argument is filled", () => {
|
||||
renderWithMantine(
|
||||
<ToolDetailPanel
|
||||
{...baseProps}
|
||||
tool={simpleTool}
|
||||
formValues={{ message: "hi" }}
|
||||
/>,
|
||||
);
|
||||
expect(
|
||||
screen.getByRole("button", { name: "Execute Tool" }),
|
||||
).not.toBeDisabled();
|
||||
});
|
||||
|
||||
it("disables Execute while an optional field holds text it cannot send", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<ToolDetailPanel {...baseProps} tool={optionalJsonTool} />,
|
||||
);
|
||||
const execute = screen.getByRole("button", { name: "Execute Tool" });
|
||||
expect(execute).not.toBeDisabled();
|
||||
|
||||
// The draft never reaches `formValues` — the field reports `undefined`
|
||||
// for text it cannot parse — so only the form's validity channel makes
|
||||
// this visible to the gate.
|
||||
const jsonInput = screen.getByLabelText(/Payload/);
|
||||
await user.type(jsonInput, "x");
|
||||
expect(execute).toBeDisabled();
|
||||
|
||||
await user.clear(jsonInput);
|
||||
expect(execute).not.toBeDisabled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -19,7 +19,10 @@ import type {
|
|||
ToolAnnotations,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import { resolveDisplayLabel } from "../../../utils/toolUtils";
|
||||
import { toFormSchema } from "../../../utils/jsonUtils";
|
||||
import {
|
||||
hasMissingRequiredFields,
|
||||
toFormSchema,
|
||||
} from "../../../utils/jsonUtils";
|
||||
import { getMirroredHeaderParams } from "@inspector/core/json/xMcpHeader.js";
|
||||
import { AnnotationBadge } from "../../elements/AnnotationBadge/AnnotationBadge";
|
||||
import { ProgressDisplay } from "../../elements/ProgressDisplay/ProgressDisplay";
|
||||
|
|
@ -213,6 +216,10 @@ export function ToolDetailPanel({
|
|||
// a prior tool's hidden state doesn't carry over — mirrors how ToolsScreen
|
||||
// clears formValues on change.
|
||||
const [descriptionOpen, setDescriptionOpen] = useState(true);
|
||||
// Text a field is holding that it could not turn into a value — unparseable
|
||||
// JSON, or a number this client cannot send exactly. Reported by the form
|
||||
// because the draft lives inside the field and never reaches `formValues`.
|
||||
const [hasInvalidDraft, setHasInvalidDraft] = useState(false);
|
||||
const [prevToolName, setPrevToolName] = useState(name);
|
||||
if (name !== prevToolName) {
|
||||
setPrevToolName(name);
|
||||
|
|
@ -245,6 +252,15 @@ export function ToolDetailPanel({
|
|||
(taskSupport === "required" ||
|
||||
((taskSupport === "optional" || modernTasks) && runAsTask));
|
||||
|
||||
// Two distinct reasons the arguments aren't sendable, and this panel used to
|
||||
// check neither — Execute was gated on `isExecuting` alone, so a required
|
||||
// argument left empty and a field full of unparseable text were both
|
||||
// executable, and simply arrived at the server absent (#2020).
|
||||
const executeDisabled =
|
||||
isExecuting ||
|
||||
hasMissingRequiredFields(formSchema, formValues) ||
|
||||
hasInvalidDraft;
|
||||
|
||||
return (
|
||||
<PanelStack>
|
||||
<PinnedHeader>
|
||||
|
|
@ -321,6 +337,7 @@ export function ToolDetailPanel({
|
|||
// so the form needs the tool name to drop another tool's
|
||||
// in-progress field text. See SchemaFormProps.resetKey.
|
||||
resetKey={name}
|
||||
onValidityChange={setHasInvalidDraft}
|
||||
/>
|
||||
|
||||
{progress && <ProgressDisplay params={progress} />}
|
||||
|
|
@ -341,7 +358,7 @@ export function ToolDetailPanel({
|
|||
<Button
|
||||
size="md"
|
||||
onClick={() => onExecute(effectiveRunAsTask)}
|
||||
disabled={isExecuting}
|
||||
disabled={executeDisabled}
|
||||
loading={isExecuting}
|
||||
>
|
||||
Execute Tool
|
||||
|
|
|
|||
|
|
@ -0,0 +1,127 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import type { JSONRPCMessage, Transport } from "@modelcontextprotocol/client";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import {
|
||||
MODERN_PROTOCOL_VERSION,
|
||||
eraToVersionNegotiation,
|
||||
} from "@inspector/core/mcp/types.js";
|
||||
|
||||
/**
|
||||
* Regression coverage for #1990: the connect-time `initialLoggingLevel` must be
|
||||
* gated on the negotiated era, not on the server capability alone.
|
||||
*
|
||||
* `logging/setLevel` is a legacy-era method, and the modern wire era rejects it
|
||||
* outright ("Method 'logging/setLevel' is not supported by the negotiated
|
||||
* protocol version"). Because the call sat in `connect()`'s post-handshake
|
||||
* sequence, that rejection failed the *connection* — so the CLI (the one caller
|
||||
* that passes `initialLoggingLevel`, hardcoded to "debug" in
|
||||
* `clients/cli/src/cli.ts`) could not run *any* method against a modern server
|
||||
* that advertised `logging`, logging-related or not.
|
||||
*
|
||||
* The two cases below are the era split: legacy still sends the request with the
|
||||
* configured level, modern connects cleanly and never puts the method on the
|
||||
* wire. The modern half asserts on the sent methods rather than on connect
|
||||
* succeeding alone — a server that happens to answer the method would hide the
|
||||
* regression behind a green connect.
|
||||
*
|
||||
* A fake transport carries both. The modern leg needs no `initialize` at all:
|
||||
* the SDK negotiates with a `server/discover` probe and adopts that result's
|
||||
* capabilities directly, which is also what lets one small class serve both eras
|
||||
* by answering whichever handshake arrives.
|
||||
*/
|
||||
class CapabilityAdvertisingTransport implements Transport {
|
||||
onmessage?: (message: JSONRPCMessage) => void;
|
||||
onclose?: () => void;
|
||||
onerror?: (error: Error) => void;
|
||||
|
||||
/** Every method this client put on the wire, in order. */
|
||||
readonly sentMethods: string[] = [];
|
||||
|
||||
/** The params of the `logging/setLevel` request, if one was sent. */
|
||||
loggingSetLevelParams?: Record<string, unknown>;
|
||||
|
||||
async start(): Promise<void> {}
|
||||
|
||||
async close(): Promise<void> {
|
||||
this.onclose?.();
|
||||
}
|
||||
|
||||
async send(message: JSONRPCMessage): Promise<void> {
|
||||
if (!("method" in message)) return;
|
||||
this.sentMethods.push(message.method);
|
||||
|
||||
// The modern era's handshake: one probe, answered with the capabilities the
|
||||
// client then treats as the server's.
|
||||
if (message.method === "server/discover" && "id" in message) {
|
||||
this.reply(message.id, {
|
||||
supportedVersions: [MODERN_PROTOCOL_VERSION],
|
||||
capabilities: { logging: {} },
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// The legacy era's handshake.
|
||||
if (message.method === "initialize" && "id" in message) {
|
||||
const params = message.params as { protocolVersion: string };
|
||||
this.reply(message.id, {
|
||||
protocolVersion: params.protocolVersion,
|
||||
capabilities: { logging: {} },
|
||||
serverInfo: { name: "logging-server", version: "1.0.0" },
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (message.method === "logging/setLevel" && "id" in message) {
|
||||
this.loggingSetLevelParams = message.params as Record<string, unknown>;
|
||||
this.reply(message.id, {});
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
private reply(id: string | number, result: Record<string, unknown>): void {
|
||||
// The SDK resolves the request from its own microtask; delivering
|
||||
// synchronously from inside `send()` is the earliest any server could.
|
||||
this.onmessage?.({ jsonrpc: "2.0", id, result });
|
||||
}
|
||||
}
|
||||
|
||||
async function connectWith(
|
||||
era: "legacy" | "modern",
|
||||
): Promise<CapabilityAdvertisingTransport> {
|
||||
const transport = new CapabilityAdvertisingTransport();
|
||||
const client = new InspectorClient(
|
||||
{ type: "streamable-http", url: "https://mcp.example/mcp" },
|
||||
{
|
||||
environment: { transport: () => ({ transport }) },
|
||||
versionNegotiation: eraToVersionNegotiation(era),
|
||||
initialLoggingLevel: "debug",
|
||||
},
|
||||
);
|
||||
|
||||
await client.connect();
|
||||
expect(client.getProtocolEra()).toBe(era);
|
||||
await client.disconnect();
|
||||
|
||||
return transport;
|
||||
}
|
||||
|
||||
describe("InspectorClient connect() initial logging level, by era (#1990)", () => {
|
||||
it("sends logging/setLevel on a legacy connection", async () => {
|
||||
const transport = await connectWith("legacy");
|
||||
|
||||
expect(transport.sentMethods).toContain("logging/setLevel");
|
||||
expect(transport.loggingSetLevelParams).toMatchObject({ level: "debug" });
|
||||
});
|
||||
|
||||
it("never sends logging/setLevel on a modern connection", async () => {
|
||||
const transport = await connectWith("modern");
|
||||
|
||||
// Modern has no session-scoped level: the equivalent is the per-request
|
||||
// `io.modelcontextprotocol/logLevel` `_meta` opt-in, driven by the
|
||||
// `modernLogLevel` server setting rather than by `initialLoggingLevel`.
|
||||
expect(transport.sentMethods).not.toContain("logging/setLevel");
|
||||
// The server did advertise logging, so the capability check alone would
|
||||
// have let the call through — the era gate is what stops it.
|
||||
expect(transport.sentMethods).toContain("server/discover");
|
||||
});
|
||||
});
|
||||
734
clients/web/src/test/core/nullableUnion.test.ts
Normal file
734
clients/web/src/test/core/nullableUnion.test.ts
Normal file
|
|
@ -0,0 +1,734 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
admitsNull,
|
||||
normalizeNullableUnion,
|
||||
} from "@inspector/core/json/nullableUnion.js";
|
||||
|
||||
// The two JSON Schema encodings of "this type, or null". Both form builders —
|
||||
// the web `SchemaForm` and the TUI's `schemaToForm` — dispatch on a single
|
||||
// `type` string, so this collapse is what keeps a nullable field renderable
|
||||
// (#1928 in the web client, #2015 in the TUI).
|
||||
describe("normalizeNullableUnion", () => {
|
||||
it("normalizes anyOf string|null", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "string" }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes anyOf boolean|null", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "boolean" }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "boolean",
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes anyOf number|null", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "number" }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "number",
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes anyOf integer|null", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "integer" }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "integer",
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes anyOf array|null", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "array" }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "array",
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes type array string|null", () => {
|
||||
expect(normalizeNullableUnion({ type: ["string", "null"] })).toEqual({
|
||||
type: "string",
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes type array boolean|null", () => {
|
||||
expect(normalizeNullableUnion({ type: ["boolean", "null"] })).toEqual({
|
||||
type: "boolean",
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes type array number|null", () => {
|
||||
expect(normalizeNullableUnion({ type: ["number", "null"] })).toEqual({
|
||||
type: "number",
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes type array integer|null", () => {
|
||||
expect(normalizeNullableUnion({ type: ["integer", "null"] })).toEqual({
|
||||
type: "integer",
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("returns schema unchanged when no union pattern matches", () => {
|
||||
const schema = { type: "string" as const };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("ignores anyOf with more than two members", () => {
|
||||
const schema = {
|
||||
anyOf: [
|
||||
{ type: "string" as const },
|
||||
{ type: "null" as const },
|
||||
{ type: "number" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("ignores a two-member anyOf with no null branch", () => {
|
||||
const schema = {
|
||||
anyOf: [{ type: "string" as const }, { type: "number" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("ignores an anyOf whose non-null branch has no renderable type (#1928)", () => {
|
||||
const schema = {
|
||||
anyOf: [{ const: "only" }, { type: "null" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
// #1928: the enum lives on the surviving branch, and hoisting it is what makes
|
||||
// the field render as a Select instead of the raw-JSON fallback.
|
||||
it("hoists enum out of an anyOf string-enum|null branch", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
description: "Direction",
|
||||
anyOf: [
|
||||
{ type: "string", enum: ["envio", "recebimento"] },
|
||||
{ type: "null" },
|
||||
],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
description: "Direction",
|
||||
enum: ["envio", "recebimento"],
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
// JSON Schema's `enum` is untyped, so a typeless branch only implies strings
|
||||
// when every member is one. Guessing otherwise would hand a number to a
|
||||
// renderer that declared the option list `string[]`.
|
||||
it("does not infer string for a typeless non-string enum branch", () => {
|
||||
const schema = { anyOf: [{ enum: [1, 2] }, { type: "null" as const }] };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("does not infer string for a typeless mixed enum branch", () => {
|
||||
const schema = { anyOf: [{ enum: ["a", 2] }, { type: "null" as const }] };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("does not infer string for a typeless empty enum branch", () => {
|
||||
const schema = { anyOf: [{ enum: [] }, { type: "null" as const }] };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
// An explicit `type` is authoritative — the enum members are the server's
|
||||
// problem at that point, not an inference this function is making.
|
||||
it("still collapses a non-string enum branch that declares its type", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "number", enum: [1, 2] }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "number",
|
||||
enum: [1, 2],
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("infers string for a typeless enum branch", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ enum: ["a", "b"] }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
enum: ["a", "b"],
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("hoists items out of an anyOf array|null branch", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [
|
||||
{ type: "array", items: { type: "string", enum: ["a", "b"] } },
|
||||
{ type: "null" },
|
||||
],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "array",
|
||||
items: { type: "string", enum: ["a", "b"] },
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("hoists properties out of an anyOf object|null branch", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [
|
||||
{ type: "object", properties: { a: { type: "string" } } },
|
||||
{ type: "null" },
|
||||
],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "object",
|
||||
properties: { a: { type: "string" } },
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes anyOf object|null regardless of branch order", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "null" }, { type: "object" }],
|
||||
}),
|
||||
).toEqual({ type: "object", anyOf: undefined, nullable: true });
|
||||
});
|
||||
|
||||
it("normalizes type array object|null", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
type: ["object", "null"],
|
||||
properties: { a: { type: "string" } },
|
||||
}),
|
||||
).toEqual({
|
||||
type: "object",
|
||||
properties: { a: { type: "string" } },
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
// The `type: [T, "null"]` encoding keeps its keywords at the top level, so a
|
||||
// nullable enum written that way carries the null *inside* the list. Leaving
|
||||
// it there hands `null` to Mantine as option data, and makes the TUI's
|
||||
// all-strings check reject the enum and fall back to a text field.
|
||||
it("strips the null sentinel from a type-array nullable enum", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
type: ["string", "null"],
|
||||
enum: ["envio", "recebimento", null],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
enum: ["envio", "recebimento"],
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("strips a null sentinel hoisted off an anyOf branch too", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "string", enum: ["a", null] }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
enum: ["a"],
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
// Declining is the point: emitting `enum: undefined` would turn a schema that
|
||||
// permits *only* `null` into a plain string field accepting arbitrary text,
|
||||
// inviting values the schema forbids. Left uncollapsed it renders through the
|
||||
// JSON editor, which represents it honestly.
|
||||
it("declines to collapse an enum that held nothing but null", () => {
|
||||
const schema = { type: ["string", "null"], enum: [null] };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("declines the same way on the anyOf encoding", () => {
|
||||
const schema = {
|
||||
anyOf: [{ type: "string", enum: [null] }, { type: "null" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
// The parallel `enumNames` is filtered by the same indices. Both renderers
|
||||
// discard labels outright on a length mismatch, so stripping one without the
|
||||
// other would silently lose every label, not just the dropped one's.
|
||||
it("keeps enumNames aligned when stripping the null member", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
type: ["string", "null"],
|
||||
enum: ["a", null, "b"],
|
||||
enumNames: ["Alpha", "None", "Beta"],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
enum: ["a", "b"],
|
||||
enumNames: ["Alpha", "Beta"],
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
// Flattening still happens — only the nullability claim is withheld, because
|
||||
// the sibling enum rules null out even though the type list names it.
|
||||
it("flattens but does not mark nullable when a sibling enum excludes null", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
type: ["string", "null"],
|
||||
enum: ["envio", "recebimento"],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
enum: ["envio", "recebimento"],
|
||||
anyOf: undefined,
|
||||
nullable: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("still marks nullable when the enum lives on an anyOf branch", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
anyOf: [{ type: "string", enum: ["envio"] }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
enum: ["envio"],
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
// Keeping a mismatched list is not neutral: filtering shortens `enum`, so a
|
||||
// list both renderers had correctly *ignored* can come out the same length
|
||||
// and start labelling — `enum: [null, "b"]` + `enumNames: ["None"]` would
|
||||
// otherwise label `b` as "None". Dropping it preserves the ignore.
|
||||
it("drops a mismatched enumNames rather than letting filtering align it", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
type: ["string", "null"],
|
||||
enum: [null, "b"],
|
||||
enumNames: ["None"],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
enum: ["b"],
|
||||
enumNames: undefined,
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("drops a mismatched enumNames that stays mismatched too", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
type: ["string", "null"],
|
||||
enum: ["a", null],
|
||||
enumNames: ["Alpha"],
|
||||
}),
|
||||
).toEqual({
|
||||
type: "string",
|
||||
enum: ["a"],
|
||||
enumNames: undefined,
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
// An empty enum permits nothing at all, so it reaches the same conclusion as
|
||||
// `[null]`: no value a dropdown could offer, so do not build one.
|
||||
it("declines to collapse an empty enum", () => {
|
||||
const typeArray = { type: ["string", "null"], enum: [] };
|
||||
expect(normalizeNullableUnion(typeArray)).toBe(typeArray);
|
||||
const union = {
|
||||
anyOf: [{ type: "string", enum: [] }, { type: "null" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(union)).toBe(union);
|
||||
});
|
||||
|
||||
// `$ref` applies alongside its siblings and is never resolved here, so a
|
||||
// branch carrying one can be unsatisfiable.
|
||||
it("treats $ref as opaque wherever it appears", () => {
|
||||
const onBranch = {
|
||||
anyOf: [
|
||||
{ type: "string", $ref: "#/$defs/intOnly" },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(onBranch)).toBe(onBranch);
|
||||
const onTypeArray = { type: ["string", "null"], $ref: "#/$defs/intOnly" };
|
||||
expect(normalizeNullableUnion(onTypeArray)).toBe(onTypeArray);
|
||||
expect(admitsNull({ type: ["string", "null"], $ref: "#/$defs/x" })).toBe(
|
||||
false,
|
||||
);
|
||||
expect(admitsNull({ anyOf: [{ type: "null", $ref: "#/$defs/x" }] })).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
// The type-array path keeps the schema's own keywords, but `collapsed` clears
|
||||
// `anyOf` and the renderers ignore the other applicators — so a compound
|
||||
// schema would be widened into an unconstrained field rather than collapsed.
|
||||
it("declines a type-array nullable that also carries a sibling anyOf", () => {
|
||||
const schema = {
|
||||
type: ["string", "null"],
|
||||
anyOf: [{ const: "a" }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("declines a type-array nullable that carries an opaque applicator", () => {
|
||||
const withNot = { type: ["string", "null"], not: { const: "a" } };
|
||||
expect(normalizeNullableUnion(withNot)).toBe(withNot);
|
||||
const withOneOf = { type: ["string", "null"], oneOf: [{ const: "a" }] };
|
||||
expect(normalizeNullableUnion(withOneOf)).toBe(withOneOf);
|
||||
const withAllOf = { type: ["string", "null"], allOf: [{ minLength: 2 }] };
|
||||
expect(normalizeNullableUnion(withAllOf)).toBe(withAllOf);
|
||||
});
|
||||
|
||||
it("normalizes type array array|null", () => {
|
||||
expect(normalizeNullableUnion({ type: ["array", "null"] })).toEqual({
|
||||
type: "array",
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps a type array of two non-null members unchanged", () => {
|
||||
const schema = { type: ["string", "number"] as const };
|
||||
expect(
|
||||
normalizeNullableUnion({ ...schema, type: [...schema.type] }),
|
||||
).toEqual({
|
||||
type: ["string", "number"],
|
||||
});
|
||||
});
|
||||
|
||||
// A server can put anything in `anyOf`; a non-object member must not be read
|
||||
// as a branch (nor throw), it simply means no nullable union was recognized.
|
||||
it("ignores an anyOf holding a non-object member", () => {
|
||||
const schema = { anyOf: ["string", { type: "null" as const }] };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("ignores an anyOf holding an array member", () => {
|
||||
const schema = { anyOf: [["string"], { type: "null" as const }] };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("ignores a type array whose non-null member has no widget", () => {
|
||||
const schema = { type: ["null", "null"] };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
// The hoist is a spread, but JSON Schema applies sibling keywords
|
||||
// conjunctively — so a branch keyword replacing the wrapper's would *widen*
|
||||
// the field. `enum: ["a"]` around a branch `enum: ["a","b"]` means "a", yet
|
||||
// the spread would offer "b" in the dropdown and submit it.
|
||||
it("declines when the branch would drop a conflicting wrapper enum", () => {
|
||||
const schema = {
|
||||
enum: ["a"],
|
||||
anyOf: [{ type: "string", enum: ["a", "b"] }, { type: "null" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("declines when wrapper and branch disagree on type", () => {
|
||||
const schema = {
|
||||
type: "string",
|
||||
anyOf: [{ type: "integer" }, { type: "null" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("declines when wrapper and branch disagree on bounds", () => {
|
||||
const schema = {
|
||||
minimum: 5,
|
||||
anyOf: [{ type: "integer", minimum: 1 }, { type: "null" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
// Deliberately stricter than value-equality: a wrapper restating its branch's
|
||||
// `type` also declines. That costs a dropdown on a rare redundant shape and
|
||||
// buys a rule with no gap, which is the better trade for a module that cannot
|
||||
// evaluate JSON Schema.
|
||||
it("declines even when wrapper and branch agree on a shared keyword", () => {
|
||||
const schema = {
|
||||
type: "string",
|
||||
anyOf: [{ type: "string", enum: ["a"] }, { type: "null" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
// The hoist *drops* a nested union: the branch's `anyOf` overwrites the
|
||||
// wrapper's in the spread and is then cleared, so the constraint vanishes and
|
||||
// the field widens into an unconstrained nullable string.
|
||||
it("declines when the surviving branch carries its own anyOf", () => {
|
||||
const schema = {
|
||||
anyOf: [
|
||||
{ type: "string", anyOf: [{ const: "a" }] },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("declines when the surviving branch carries an opaque applicator", () => {
|
||||
const withNot = {
|
||||
anyOf: [
|
||||
{ type: "string", not: { const: "a" } },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(withNot)).toBe(withNot);
|
||||
const withAllOf = {
|
||||
anyOf: [
|
||||
{ type: "string", allOf: [{ minLength: 2 }] },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(withAllOf)).toBe(withAllOf);
|
||||
const withOneOf = {
|
||||
anyOf: [
|
||||
{ type: "string", oneOf: [{ const: "a" }] },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(withOneOf)).toBe(withOneOf);
|
||||
});
|
||||
|
||||
it("declines when the wrapper carries an applicator the module cannot read", () => {
|
||||
const schema = {
|
||||
allOf: [{ minLength: 2 }],
|
||||
anyOf: [{ type: "string" }, { type: "null" as const }],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("treats wrapper metadata as safe to keep alongside branch constraints", () => {
|
||||
expect(
|
||||
normalizeNullableUnion({
|
||||
title: "Direction",
|
||||
description: "Which way",
|
||||
default: "envio",
|
||||
anyOf: [{ type: "string", enum: ["envio"] }, { type: "null" }],
|
||||
}),
|
||||
).toEqual({
|
||||
title: "Direction",
|
||||
description: "Which way",
|
||||
default: "envio",
|
||||
type: "string",
|
||||
enum: ["envio"],
|
||||
anyOf: undefined,
|
||||
nullable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores a schema with no union keywords at all", () => {
|
||||
const schema = { type: "object" as const, properties: {} };
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
});
|
||||
|
||||
// `admitsNull` answers a *validity* question and is deliberately decoupled from
|
||||
// the collapse, which answers a narrower *rendering* one. A schema can admit
|
||||
// null while being unrenderable as a single widget, and a form that conflated
|
||||
// the two would reject a value its own schema accepts.
|
||||
describe("admitsNull", () => {
|
||||
it("recognizes every encoding the collapse handles", () => {
|
||||
expect(admitsNull({ anyOf: [{ type: "string" }, { type: "null" }] })).toBe(
|
||||
true,
|
||||
);
|
||||
expect(admitsNull({ type: ["string", "null"] })).toBe(true);
|
||||
expect(admitsNull({ type: "null" })).toBe(true);
|
||||
expect(admitsNull({ nullable: true })).toBe(true);
|
||||
});
|
||||
|
||||
it("recognizes a null branch the collapse declines to flatten", () => {
|
||||
// Three members, so `normalizeNullableUnion` leaves this alone and it
|
||||
// renders through the JSON fallback — where a user can still enter `null`.
|
||||
const wide = {
|
||||
anyOf: [{ type: "string" }, { type: "number" }, { type: "null" }],
|
||||
};
|
||||
expect(normalizeNullableUnion(wide)).toBe(wide);
|
||||
expect(admitsNull(wide)).toBe(true);
|
||||
});
|
||||
|
||||
it("recognizes a null branch declared as a nested type array", () => {
|
||||
expect(admitsNull({ anyOf: [{ type: ["string", "null"] }] })).toBe(true);
|
||||
});
|
||||
|
||||
// Applicators this module does not evaluate. `oneOf` requires *exactly one*
|
||||
// branch to match, so a null branch does not by itself mean null validates;
|
||||
// `not` and `allOf` can rule it out outright. Under-claiming costs a clear
|
||||
// button, over-claiming lets the form emit a value the schema rejects.
|
||||
it("declines to claim nullability for applicators it cannot evaluate", () => {
|
||||
expect(admitsNull({ oneOf: [{ type: "string" }, { type: "null" }] })).toBe(
|
||||
false,
|
||||
);
|
||||
expect(
|
||||
admitsNull({ not: { type: "null" }, anyOf: [{ type: "null" }] }),
|
||||
).toBe(false);
|
||||
expect(
|
||||
admitsNull({ allOf: [{ type: "string" }], anyOf: [{ type: "null" }] }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
// A branch can name null and still admit nothing.
|
||||
it("ignores an unsatisfiable null branch", () => {
|
||||
expect(
|
||||
admitsNull({ anyOf: [{ type: "string" }, { type: "null", const: "x" }] }),
|
||||
).toBe(false);
|
||||
expect(
|
||||
admitsNull({
|
||||
anyOf: [{ type: "string" }, { type: "null", enum: ["x"] }],
|
||||
}),
|
||||
).toBe(false);
|
||||
expect(
|
||||
admitsNull({
|
||||
anyOf: [{ type: "string" }, { type: "null", not: { type: "null" } }],
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores a null branch made unsatisfiable by its own nested anyOf", () => {
|
||||
expect(
|
||||
admitsNull({
|
||||
anyOf: [
|
||||
{ type: "string" },
|
||||
{ type: "null", anyOf: [{ type: "string" }] },
|
||||
],
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("still accepts a plain null branch alongside an unsatisfiable one", () => {
|
||||
expect(
|
||||
admitsNull({
|
||||
anyOf: [{ type: "null", const: "x" }, { type: "null" }],
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("is false for a schema that does not permit null", () => {
|
||||
expect(admitsNull({ type: "string" })).toBe(false);
|
||||
expect(admitsNull({ type: ["string", "number"] })).toBe(false);
|
||||
expect(
|
||||
admitsNull({ anyOf: [{ type: "string" }, { type: "number" }] }),
|
||||
).toBe(false);
|
||||
expect(admitsNull({})).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores non-object anyOf members instead of throwing", () => {
|
||||
expect(admitsNull({ anyOf: ["null", ["null"], 7] })).toBe(false);
|
||||
});
|
||||
|
||||
// JSON Schema keywords at one level are conjunctive, so a syntactic null does
|
||||
// not by itself mean the schema accepts null. `{ type: ["string","null"],
|
||||
// enum: ["envio"] }` names "null" and still rejects it.
|
||||
it("is false when a sibling enum excludes null", () => {
|
||||
expect(admitsNull({ type: ["string", "null"], enum: ["envio"] })).toBe(
|
||||
false,
|
||||
);
|
||||
expect(
|
||||
admitsNull({
|
||||
anyOf: [{ type: "string" }, { type: "null" }],
|
||||
enum: ["a"],
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("is true when a sibling enum offers null", () => {
|
||||
expect(
|
||||
admitsNull({ type: ["string", "null"], enum: ["envio", null] }),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("is false when a sibling const pins a non-null value", () => {
|
||||
expect(admitsNull({ type: ["string", "null"], const: "envio" })).toBe(
|
||||
false,
|
||||
);
|
||||
expect(admitsNull({ type: ["string", "null"], const: null })).toBe(true);
|
||||
});
|
||||
|
||||
// A branch's enum is scoped to that branch, not a sibling of the union — this
|
||||
// is the #1928 shape, and reading it as a sibling would call it non-nullable.
|
||||
// An explicit `type` is conjunctive with the union, so it decides rather than
|
||||
// merely offering another way to say yes. A `{ type: "null" }` branch cannot
|
||||
// override a top-level `type: "string"`.
|
||||
it("lets an explicit non-null type override a null branch", () => {
|
||||
expect(
|
||||
admitsNull({
|
||||
type: "string",
|
||||
anyOf: [{ type: "string", enum: ["a"] }, { type: "null" }],
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("still honors an explicit type that does admit null", () => {
|
||||
expect(admitsNull({ type: ["string", "null"] })).toBe(true);
|
||||
expect(admitsNull({ type: "null" })).toBe(true);
|
||||
});
|
||||
|
||||
// A sibling union is conjunctive with the `type`, so it can reject null even
|
||||
// when the type list names it. It is not evaluated here, so its mere presence
|
||||
// withholds the claim.
|
||||
it("withholds the claim when a sibling anyOf sits beside an explicit type", () => {
|
||||
expect(
|
||||
admitsNull({ type: ["string", "null"], anyOf: [{ type: "string" }] }),
|
||||
).toBe(false);
|
||||
// Unsatisfiable: names null, admits nothing.
|
||||
expect(admitsNull({ type: "null", anyOf: [{ type: "string" }] })).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("is unaffected by an enum that lives inside an anyOf branch", () => {
|
||||
expect(
|
||||
admitsNull({
|
||||
anyOf: [{ type: "string", enum: ["envio"] }, { type: "null" }],
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
148
clients/web/src/test/integration/mcp/nullable-fields.test.ts
Normal file
148
clients/web/src/test/integration/mcp/nullable-fields.test.ts
Normal file
|
|
@ -0,0 +1,148 @@
|
|||
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 { normalizeNullableUnion } from "@inspector/core/json/nullableUnion.js";
|
||||
import { toFormSchema } from "../../../utils/jsonUtils";
|
||||
import type { InspectorFormSchema } from "../../../utils/jsonUtils";
|
||||
import {
|
||||
createTestServerHttp,
|
||||
type TestServerHttp,
|
||||
createTestServerInfo,
|
||||
loadConfig,
|
||||
resolveConfig,
|
||||
} from "@modelcontextprotocol/inspector-test-server";
|
||||
|
||||
/**
|
||||
* Live coverage of `test-servers/configs/nullable-fields-http.json` — the
|
||||
* documented manual reproduction for #1928.
|
||||
*
|
||||
* The unit tests on both form builders construct the `anyOf` shape by hand,
|
||||
* which verifies the *renderers* but assumes the premise: that Zod's
|
||||
* `.nullish()` actually emits `anyOf: [<branch>, { type: "null" }]` with the
|
||||
* `enum` on the branch. That premise is the whole reason the fix exists, and
|
||||
* nothing else pins it — a Zod change in how it compiles a nullish enum would
|
||||
* leave every unit test green while the showcase server quietly stopped
|
||||
* reproducing the bug.
|
||||
*
|
||||
* So the server is built by **resolving the checked-in config**, not by calling
|
||||
* the fixture factory directly. That is deliberate and is the difference
|
||||
* between covering the wiring and merely asserting the factory: going through
|
||||
* `loadConfig` → `resolveConfig` means a misspelt preset name in
|
||||
* `preset-registry.ts`, or a config that names a preset that no longer exists,
|
||||
* fails here rather than only when someone runs the repro by hand.
|
||||
*/
|
||||
describe("nullable argument schemas over the wire (#1928)", () => {
|
||||
let client: InspectorClient | null = null;
|
||||
let server: TestServerHttp | null = null;
|
||||
|
||||
const configPath = path.resolve(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
"../../../../../../test-servers/configs/nullable-fields-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<InspectorClient> {
|
||||
const resolved = resolveConfig(loadConfig(configPath));
|
||||
const started = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo("nullable-fields-test", "1.0.0"),
|
||||
tools: resolved.tools,
|
||||
});
|
||||
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;
|
||||
}
|
||||
|
||||
async function recordShipmentSchema(): Promise<InspectorFormSchema> {
|
||||
const connected = await connectToShowcase();
|
||||
const { tools } = await connected.listAllTools();
|
||||
const tool = tools.find((entry) => entry.name === "record_shipment");
|
||||
expect(tool).toBeDefined();
|
||||
const schema = toFormSchema(tool?.inputSchema);
|
||||
expect(schema).not.toBeNull();
|
||||
return schema as InspectorFormSchema;
|
||||
}
|
||||
|
||||
it("resolves the preset the config names", () => {
|
||||
// Fails on a misspelt registry case or a config naming a dead preset —
|
||||
// neither of which the hand-built unit schemas can see.
|
||||
const resolved = resolveConfig(loadConfig(configPath));
|
||||
expect(resolved.tools?.map((tool) => tool.name)).toEqual([
|
||||
"record_shipment",
|
||||
"get_temp",
|
||||
]);
|
||||
});
|
||||
|
||||
it("emits an anyOf-with-null for every nullish argument, enum on the branch", async () => {
|
||||
const schema = await recordShipmentSchema();
|
||||
const properties = schema.properties ?? {};
|
||||
expect(Object.keys(properties).sort()).toEqual([
|
||||
"direction",
|
||||
"express",
|
||||
"quantity",
|
||||
"reference",
|
||||
]);
|
||||
|
||||
// The premise of the whole fix: no top-level `type`, and the `enum` sits on
|
||||
// the surviving branch rather than beside it.
|
||||
const direction = properties.direction;
|
||||
expect(direction?.type).toBeUndefined();
|
||||
expect(direction?.enum).toBeUndefined();
|
||||
expect(direction?.anyOf).toHaveLength(2);
|
||||
expect(direction?.anyOf).toEqual(
|
||||
expect.arrayContaining([expect.objectContaining({ type: "null" })]),
|
||||
);
|
||||
});
|
||||
|
||||
it("collapses each argument to the type its widget dispatches on", async () => {
|
||||
const schema = await recordShipmentSchema();
|
||||
const collapsed = Object.fromEntries(
|
||||
Object.entries(schema.properties ?? {}).map(([name, propertySchema]) => [
|
||||
name,
|
||||
normalizeNullableUnion(propertySchema),
|
||||
]),
|
||||
);
|
||||
|
||||
expect(collapsed.direction?.type).toBe("string");
|
||||
expect(collapsed.direction?.enum).toEqual(["envio", "recebimento"]);
|
||||
expect(collapsed.reference?.type).toBe("string");
|
||||
expect(collapsed.quantity?.type).toBe("integer");
|
||||
expect(collapsed.express?.type).toBe("boolean");
|
||||
|
||||
for (const propertySchema of Object.values(collapsed)) {
|
||||
expect(propertySchema?.nullable).toBe(true);
|
||||
expect(propertySchema?.anyOf).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -1,12 +1,24 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
BIND_ALL_INTERFACES_ENV,
|
||||
DEFAULT_BIND_HOST,
|
||||
resolveBindHostname,
|
||||
} from "../../../../server/resolve-bind-host.js";
|
||||
|
||||
describe("resolveBindHostname", () => {
|
||||
it("defaults to localhost when HOST is unset", () => {
|
||||
expect(resolveBindHostname({})).toBe("localhost");
|
||||
// Regression guard for #1951. The default must be an *address*, never the
|
||||
// name `localhost`: `listen()` resolves a name and binds the single address
|
||||
// the resolver returns first, which on glibc Linux is `::1` — leaving every
|
||||
// IPv4 client (a dev container's port forwarder, an `ssh -L` to 127.0.0.1, a
|
||||
// container healthcheck) refused. See DEFAULT_BIND_HOST.
|
||||
it("defaults to the IPv4 loopback address, not the name localhost", () => {
|
||||
expect(resolveBindHostname({})).toBe("127.0.0.1");
|
||||
expect(resolveBindHostname({})).toBe(DEFAULT_BIND_HOST);
|
||||
expect(resolveBindHostname({})).not.toBe("localhost");
|
||||
});
|
||||
|
||||
it("still honors an explicit HOST=localhost as typed", () => {
|
||||
expect(resolveBindHostname({ HOST: "localhost" })).toBe("localhost");
|
||||
});
|
||||
|
||||
it("returns a loopback HOST unchanged", () => {
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { createServer, type Server } from "node:http";
|
||||
import { EventEmitter } from "node:events";
|
||||
import {
|
||||
createSandboxController,
|
||||
DEFAULT_SANDBOX_PORT,
|
||||
resolveSandboxPort,
|
||||
sandboxFrameAncestors,
|
||||
} from "../../../../server/sandbox-controller.js";
|
||||
|
|
@ -75,7 +77,17 @@ describe("resolveSandboxPort", () => {
|
|||
else process.env.SERVER_PORT = envSnapshot.server;
|
||||
});
|
||||
|
||||
it("returns 0 when no env vars are set", () => {
|
||||
// Regression guard for #2008: the default must be a FIXED port. An
|
||||
// OS-assigned one changes every run, so it can never be named in a dev
|
||||
// container's `forwardPorts`, a `docker run -p`, or an SSH tunnel — leaving
|
||||
// the Apps tab unreachable in every such environment.
|
||||
it("returns a fixed default port when no env vars are set", () => {
|
||||
expect(resolveSandboxPort()).toBe(DEFAULT_SANDBOX_PORT);
|
||||
expect(resolveSandboxPort()).not.toBe(0);
|
||||
});
|
||||
|
||||
it("still honors an explicit MCP_SANDBOX_PORT=0 as OS-assigned", () => {
|
||||
process.env.MCP_SANDBOX_PORT = "0";
|
||||
expect(resolveSandboxPort()).toBe(0);
|
||||
});
|
||||
|
||||
|
|
@ -102,19 +114,19 @@ describe("resolveSandboxPort", () => {
|
|||
expect(resolveSandboxPort()).toBe(9100);
|
||||
});
|
||||
|
||||
it("returns 0 when SERVER_PORT is non-numeric and no MCP_SANDBOX_PORT", () => {
|
||||
it("falls back to the default when SERVER_PORT is non-numeric and no MCP_SANDBOX_PORT", () => {
|
||||
process.env.SERVER_PORT = "not-a-port";
|
||||
expect(resolveSandboxPort()).toBe(0);
|
||||
expect(resolveSandboxPort()).toBe(DEFAULT_SANDBOX_PORT);
|
||||
});
|
||||
|
||||
it("ignores empty-string SERVER_PORT", () => {
|
||||
process.env.SERVER_PORT = "";
|
||||
expect(resolveSandboxPort()).toBe(0);
|
||||
expect(resolveSandboxPort()).toBe(DEFAULT_SANDBOX_PORT);
|
||||
});
|
||||
|
||||
it("ignores negative values", () => {
|
||||
process.env.MCP_SANDBOX_PORT = "-1";
|
||||
expect(resolveSandboxPort()).toBe(0);
|
||||
expect(resolveSandboxPort()).toBe(DEFAULT_SANDBOX_PORT);
|
||||
});
|
||||
|
||||
it("rejects a partial-parse value (6274abc) rather than binding 6274", () => {
|
||||
|
|
@ -125,7 +137,7 @@ describe("resolveSandboxPort", () => {
|
|||
|
||||
it("rejects an out-of-range value (70000) so it can't crash listen", () => {
|
||||
process.env.MCP_SANDBOX_PORT = "70000";
|
||||
expect(resolveSandboxPort()).toBe(0);
|
||||
expect(resolveSandboxPort()).toBe(DEFAULT_SANDBOX_PORT);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -135,7 +147,8 @@ describe("createSandboxController", () => {
|
|||
try {
|
||||
const { url, port } = await controller.start();
|
||||
expect(port).toBeGreaterThan(0);
|
||||
expect(url).toBe(`http://localhost:${port}/sandbox`);
|
||||
// Default host is now the IPv4 loopback address, not the name (#1951).
|
||||
expect(url).toBe(`http://127.0.0.1:${port}/sandbox`);
|
||||
expect(controller.getUrl()).toBe(url);
|
||||
|
||||
const res = await fetch(url);
|
||||
|
|
@ -312,11 +325,11 @@ describe("createSandboxController", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("resolves with empty values when listen fails (EADDRINUSE)", async () => {
|
||||
// Bind a placeholder HTTP server to claim a port, then point a sandbox
|
||||
// controller at the same port to force EADDRINUSE. The Vite plugin awaits
|
||||
// start() in configureServer; if start() ever stops resolving, the entire
|
||||
// dev backend hangs. This test pins down the resolve-on-error contract.
|
||||
/** Claim a port so a subsequent listen on it fails with EADDRINUSE. */
|
||||
async function claimPort(): Promise<{
|
||||
port: number;
|
||||
release: () => Promise<void>;
|
||||
}> {
|
||||
const blocker: Server = createServer();
|
||||
await new Promise<void>((resolve) =>
|
||||
blocker.listen(0, "127.0.0.1", () => resolve()),
|
||||
|
|
@ -327,21 +340,135 @@ describe("createSandboxController", () => {
|
|||
? addr.port
|
||||
: 0;
|
||||
expect(port).toBeGreaterThan(0);
|
||||
return {
|
||||
port,
|
||||
release: () =>
|
||||
new Promise<void>((resolve) => blocker.close(() => resolve())),
|
||||
};
|
||||
}
|
||||
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
it("falls back to an OS-assigned port when the fixed port is taken", async () => {
|
||||
// #2008 made the default port fixed, which introduced a collision the
|
||||
// dynamic default never had: a second Inspector, or anything else already
|
||||
// on 6275, would have taken the Apps tab down entirely. Retrying on a
|
||||
// dynamic port keeps it working locally.
|
||||
const { port, release } = await claimPort();
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const controller = createSandboxController({ port, host: "127.0.0.1" });
|
||||
try {
|
||||
const result = await controller.start();
|
||||
expect(result).toEqual({ port: 0, url: "" });
|
||||
expect(controller.getUrl()).toBeNull();
|
||||
// close() must be a no-op since the server never bound.
|
||||
await expect(controller.close()).resolves.toBeUndefined();
|
||||
expect(errorSpy).toHaveBeenCalledWith(
|
||||
expect(result.port).toBeGreaterThan(0);
|
||||
expect(result.port).not.toBe(port);
|
||||
expect(result.url).toBe(`http://127.0.0.1:${result.port}/sandbox`);
|
||||
expect(controller.getUrl()).toBe(result.url);
|
||||
// The sandbox must actually be serving on the fallback port, not merely
|
||||
// reporting one.
|
||||
expect((await fetch(result.url)).status).toBe(200);
|
||||
// Loud, not silent: a pinned-and-forwarded port that quietly moved is the
|
||||
// very failure the fixed default exists to prevent.
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining(`Sandbox: port ${port} in use`),
|
||||
);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("MCP_SANDBOX_PORT"),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
await controller.close();
|
||||
await release();
|
||||
}
|
||||
});
|
||||
|
||||
it("resolves with empty values when even the dynamic retry fails", async () => {
|
||||
// The Vite plugin awaits start() in configureServer; if start() ever stops
|
||||
// resolving, the entire dev backend hangs. This pins the resolve-on-error
|
||||
// contract for the case the retry can't rescue — an explicit `port: 0` that
|
||||
// fails has nowhere left to fall back to, so it degrades as before.
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
const controller = createSandboxController({ port: 0, host: "127.0.0.1" });
|
||||
try {
|
||||
// Force the listen to fail the way a dynamic listen can (an unusable bind
|
||||
// address), since port 0 cannot collide.
|
||||
const broken = createSandboxController({
|
||||
port: 0,
|
||||
host: "203.0.113.1", // TEST-NET-3: not assigned to any local interface
|
||||
});
|
||||
const result = await broken.start();
|
||||
expect(result).toEqual({ port: 0, url: "" });
|
||||
expect(broken.getUrl()).toBeNull();
|
||||
// close() must be a no-op since the server never bound.
|
||||
await expect(broken.close()).resolves.toBeUndefined();
|
||||
expect(errorSpy).toHaveBeenCalledWith(
|
||||
"Sandbox server error:",
|
||||
expect.objectContaining({ code: "EADDRNOTAVAIL" }),
|
||||
);
|
||||
} finally {
|
||||
errorSpy.mockRestore();
|
||||
await new Promise<void>((resolve) => blocker.close(() => resolve()));
|
||||
await controller.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("gives up after one retry when the dynamic fallback also fails", async () => {
|
||||
// Guards the `retriedDynamic` latch. A real `listen(0)` effectively always
|
||||
// succeeds, so claiming a port only ever produces ONE EADDRINUSE — under
|
||||
// which this test would pass with the latch deleted. Mock `node:http` so
|
||||
// every listen fails with EADDRINUSE: the first trips the retry, the second
|
||||
// must be caught by the latch and settle. Without the latch this recurses
|
||||
// until the test times out rather than resolving.
|
||||
vi.resetModules();
|
||||
const listenCalls: number[] = [];
|
||||
vi.doMock("node:http", async () => {
|
||||
const actual =
|
||||
await vi.importActual<typeof import("node:http")>("node:http");
|
||||
return {
|
||||
...actual,
|
||||
createServer: () => {
|
||||
const emitter = new EventEmitter();
|
||||
return Object.assign(emitter, {
|
||||
listen: (p: number) => {
|
||||
listenCalls.push(p);
|
||||
setImmediate(() =>
|
||||
emitter.emit(
|
||||
"error",
|
||||
Object.assign(new Error("in use"), { code: "EADDRINUSE" }),
|
||||
),
|
||||
);
|
||||
},
|
||||
address: () => null,
|
||||
close: (cb?: () => void) => cb?.(),
|
||||
});
|
||||
},
|
||||
};
|
||||
});
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
try {
|
||||
const mod = await import("../../../../server/sandbox-controller.js");
|
||||
const controller = mod.createSandboxController({
|
||||
port: 6275,
|
||||
host: "127.0.0.1",
|
||||
});
|
||||
// Settles rather than looping — the contract the Vite plugin depends on.
|
||||
const result = await controller.start();
|
||||
expect(result).toEqual({ port: 0, url: "" });
|
||||
expect(controller.getUrl()).toBeNull();
|
||||
// Exactly two listens: the fixed port, then one dynamic retry. A third
|
||||
// would mean the latch failed to bound the loop.
|
||||
expect(listenCalls).toEqual([6275, 0]);
|
||||
expect(
|
||||
warnSpy.mock.calls.filter((c) =>
|
||||
String(c[0]).includes("falling back to an OS-assigned port"),
|
||||
),
|
||||
).toHaveLength(1);
|
||||
// The second failure degrades loudly instead of retrying again.
|
||||
expect(errorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Sandbox: port 6275 in use"),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
errorSpy.mockRestore();
|
||||
vi.doUnmock("node:http");
|
||||
vi.resetModules();
|
||||
}
|
||||
});
|
||||
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue