/** * Tool-schema portability lint (#1005). * * A tool's `inputSchema`/`outputSchema` can be a perfectly legal JSON Schema * and still be rejected outright by the client the server is actually meant to * run against. The reported case is Go's `jsonschema` package emitting a bare * `true` for an `interface{}` field: valid Draft 2020-12, accepted by the MCP * SDK's own parser, and refused by Claude Code with a bare `"Invalid input"` * pointing at `["tools", 7, "outputSchema", "properties", "data"]`. The * Inspector is where a server author looks *first*, so a construct that will * fail downstream should be named here rather than silently passed through. * * This is deliberately **not** a JSON Schema validator, for a measured reason: * a census of 617 public servers (14,804 tool schemas) reported on the issue * found **0** that fail the SDK's own `ListToolsResultSchema.safeParse` — so a * conformance check would report nothing on essentially every real server. * What bites instead is the narrower subset each consumer accepts, which is * what the rules below encode. Every rule is a construct that is legal JSON * Schema and is known to be refused or quietly mishandled by real MCP clients. * * Kept pure and dependency-free so all three clients share one verdict: the * CLI's `--strict` report, the TUI's tool detail pane, and the web Tools tab * must not disagree about whether a schema is portable. */ import type { Tool } from "@modelcontextprotocol/client"; /** Which of a tool's two schemas a finding was raised against. */ export type SchemaKind = "inputSchema" | "outputSchema"; /** * `error` — the construct is refused by at least one shipping MCP client, so * the tool is unusable there. `warning` — accepted unevenly, or degraded * (silently coerced, ignored) rather than rejected. */ export type SchemaLintSeverity = "error" | "warning"; /** Stable identifier for a lint rule, so a caller can filter or suppress. */ export type SchemaLintRule = | "boolean-schema" | "type-union" | "untyped-schema" | "remote-ref"; /** One problem found at one location in one schema. */ export interface SchemaFinding { rule: SchemaLintRule; severity: SchemaLintSeverity; /** Which schema the finding is in. */ schema: SchemaKind; /** * Location within that schema, in the dotted/bracketed form the report * prints — `properties.data`, `anyOf[1].items`. Empty string means the * schema root. */ path: string; /** What is wrong, phrased for a server author. */ issue: string; /** A concrete replacement, not a restatement of the problem. */ suggestion: string; } /** Every finding raised against one tool, in walk order. */ export interface ToolSchemaFindings { toolName: string; findings: SchemaFinding[]; } /** * Keywords whose value is a schema *or* a boolean by design. `true`/`false` * here is the idiomatic spelling ("allow anything else" / "allow nothing * else") and every client understands it, so the boolean-schema rule must not * fire on them — the issue's own suggested fix is `additionalProperties: true`. */ const BOOLEAN_VALUED_KEYWORDS: ReadonlySet = new Set([ "additionalProperties", "unevaluatedProperties", "additionalItems", "unevaluatedItems", ]); /** Keywords whose value is a single subschema. */ const SUBSCHEMA_KEYWORDS = [ "items", "contains", "not", "propertyNames", "if", "then", "else", "additionalProperties", "unevaluatedProperties", "additionalItems", "unevaluatedItems", // 2019-09/2020-12: the schema the *decoded* string content must satisfy. // A real schema position, so a bare `true` or a remote `$ref` under it is a // real finding. "contentSchema", ] as const; /** Keywords whose value is an array of subschemas. */ const SUBSCHEMA_ARRAY_KEYWORDS = [ "allOf", "anyOf", "oneOf", "prefixItems", ] as const; /** * Keywords whose value is a name → subschema map. * * `dependencies` is the pre-2019 form and may hold either a subschema or an * array of property names; the array case is not a schema position, and the * walk skips it because a non-object, non-boolean node is not a schema. */ const SUBSCHEMA_MAP_KEYWORDS = [ "properties", "patternProperties", "dependentSchemas", "dependencies", "$defs", "definitions", ] as const; /** * Keywords that actually constrain the instance being validated — the * assertion and applicator vocabularies, plus reference keywords. * * This is an **allowlist**, and that direction is load-bearing. JSON Schema * ignores keywords it does not recognize, so a schema is unconstrained unless * something it carries is known to constrain: a denylist of known-annotation * keywords would let `{"vendorHint": true}` — which accepts every value — * pass as constrained simply because the keyword is unfamiliar. * * Two categories are deliberately absent. **Annotations** (`title`, * `description`, `default`, `examples`, `deprecated`, `readOnly`, * `writeOnly`, `$comment`) assert nothing. So do the **definition-only * containers** `$defs` / `definitions` and the identifier keywords `$id`, * `$schema`, `$anchor`, `$vocabulary`: they hold or name subschemas for * *other* schemas to reference, and constrain the current instance not at all. * * One category is admitted despite being annotation-only in 2020-12's default * vocabulary: `format`, and the three content keywords (`contentMediaType`, * `contentEncoding`, `contentSchema`). A schema declaring any of them is * plainly an attempt to constrain, and reporting it as accepting anything * would be noise rather than a finding an author can act on. * * Defining the rule this way, rather than as "has no `type`", is also * deliberate: `{"properties": {…}}` with no `type` does constrain its input, * so calling it unconstrained would be false. The rule fires on `{}`, on * annotation-only schemas, and on `$defs`-only schemas — the object-literal * spellings of the same "any" that a bare `true` expresses. */ const CONSTRAINING_KEYWORDS: ReadonlySet = new Set([ // Assertions "type", "enum", "const", "multipleOf", "maximum", "exclusiveMaximum", "minimum", "exclusiveMinimum", "maxLength", "minLength", "pattern", "format", "contentMediaType", "contentEncoding", "contentSchema", "maxItems", "minItems", "uniqueItems", "maxContains", "minContains", "maxProperties", "minProperties", "required", "dependentRequired", // Applicators "allOf", "anyOf", "oneOf", "not", // `if` / `then` / `else` are deliberately absent — they only constrain as a // PAIR. `if` alone has no assertion effect, and `then` / `else` are ignored // without it, so `{"if": {"const": 1}}` accepts every value. The pairing is // checked separately in `constrainsInstance`. "items", "prefixItems", "contains", "additionalItems", "unevaluatedItems", "properties", "patternProperties", "additionalProperties", "unevaluatedProperties", "propertyNames", "dependentSchemas", "dependencies", // References "$ref", "$dynamicRef", "$recursiveRef", ]); /** * The seven type names JSON Schema recognizes. A `type` naming anything else * is malformed rather than unportable, which this module deliberately does not * report — see {@link isPortableTypeUnion}. */ const JSON_SCHEMA_TYPES: ReadonlySet = new Set([ "null", "boolean", "object", "array", "number", "string", "integer", ]); /** * Does this schema constrain the instance at all? * * Presence of a {@link CONSTRAINING_KEYWORDS} member, plus the one keyword * group that cannot be judged by presence alone: `if` asserts nothing without * a `then` or an `else`, and either of those is ignored without an `if`. So * `{"if": {"const": 1}}` and `{"then": {"type": "string"}}` both accept every * value, and counting the keyword's mere presence would let them pass as * constrained. */ function constrainsInstance(node: SchemaRecord): boolean { if (Object.keys(node).some((key) => CONSTRAINING_KEYWORDS.has(key))) { return true; } return "if" in node && ("then" in node || "else" in node); } /** * Is an array-valued `type` the *legal* union this rule is about? * * The rule reports a construct that is valid JSON Schema and unportable, and * its message says exactly that — so it must not fire on an array that is * simply malformed (`[]`, which matches nothing; `[3]`; `["bananas"]`; * `["string", "string"]`). Saying "this is legal JSON Schema, but…" of those * would be false, and the `anyOf` replacement it suggests would be invalid * too. Malformed schemas are the SDK parser's business, the same way a * non-object, non-boolean node is; this module stays quiet on them. */ function isPortableTypeUnion(type: readonly unknown[]): type is string[] { if (type.length === 0) return false; if (!type.every((t) => typeof t === "string" && JSON_SCHEMA_TYPES.has(t))) { return false; } return new Set(type).size === type.length; } /** * Depth cap for the walk. A `$ref`-free JSON document cannot be cyclic, but * this module is handed whatever a server sent, so bound the recursion rather * than trusting it. */ const MAX_DEPTH = 64; type SchemaRecord = Record; function isRecord(value: unknown): value is SchemaRecord { return typeof value === "object" && value !== null && !Array.isArray(value); } /** * Append a key to a path, quoting it when it is not a bare identifier so the * printed path stays unambiguous for an odd property name. */ function childPath(parent: string, key: string): string { if (/^[A-Za-z_$][A-Za-z0-9_$]*$/.test(key)) { return parent === "" ? key : `${parent}.${key}`; } return `${parent}[${JSON.stringify(key)}]`; } function indexPath(parent: string, index: number): string { return `${parent}[${index}]`; } /** Human label for a path, so the root reads as something rather than blank. */ export function describeSchemaPath(schema: SchemaKind, path: string): string { return path === "" ? schema : `${schema}.${path}`; } interface WalkContext { schema: SchemaKind; findings: SchemaFinding[]; /** * Whether this schema document declares an `$id` anywhere — see * {@link declaresAnyId}. Suppresses the `remote-ref` rule. */ hasEmbeddedIds: boolean; } /** * Does the document declare an `$id` anywhere in it? * * `$id` establishes a base URI, which changes what "points outside this * document" means: with a root `$id: "https://example.com/root"`, the `$ref` * `"https://example.com/root#/$defs/x"` resolves to *this* document and needs * no fetch at all. Classifying refs correctly in that case means full RFC 3986 * base-URI resolution against possibly-nested embedded resources, which this * module does not do — so when any `$id` is present it declines to classify * rather than risk a wrong finding with wrong remediation on a valid schema. * * A missed finding is the acceptable direction here, the same trade the walk * makes on a malformed `type` array. `$id` is vanishingly rare in a tool * schema, so this costs almost nothing in practice. If it ever matters, the * refinement is to resolve each `$ref` against the enclosing base and compare * with the declared ids — not to drop the guard. * * Descends through {@link forEachSubschema} — **subschema positions only**. * A scan over every object value would also reach instance data, where an * `$id` key declares nothing: `examples: [{"$id": "payload-field"}]` would * otherwise switch this rule off for the entire document. */ function declaresAnyId(node: unknown, depth = 0): boolean { if (depth > MAX_DEPTH || !isRecord(node)) return false; if (typeof node.$id === "string" && node.$id !== "") return true; let found = false; forEachSubschema(node, "", (child) => { if (!found) found = declaresAnyId(child, depth + 1); }); return found; } function add( ctx: WalkContext, rule: SchemaLintRule, severity: SchemaLintSeverity, path: string, issue: string, suggestion: string, ): void { ctx.findings.push({ rule, severity, schema: ctx.schema, path, issue, suggestion, }); } /** * Call `visit` once for every position under `node` that JSON Schema treats as * a **subschema** — and only those. * * The single traversal both {@link walk} and {@link declaresAnyId} run on, so * the two cannot disagree about what counts as a schema. That matters more * than the deduplication: an independent scan that descended through *every* * object value would also reach instance data — `examples: [{"$id": "x"}]`, * a `default`, a `const` — where `$id` is just a payload key and declares no * embedded resource. Reading one there would suppress `remote-ref` for the * whole document. */ function forEachSubschema( node: SchemaRecord, path: string, visit: (child: unknown, childPath: string, keyword: string) => void, ): void { for (const key of SUBSCHEMA_MAP_KEYWORDS) { const map = node[key]; if (!isRecord(map)) continue; for (const [name, sub] of Object.entries(map)) { visit(sub, childPath(childPath(path, key), name), key); } } for (const key of SUBSCHEMA_ARRAY_KEYWORDS) { const arr = node[key]; if (!Array.isArray(arr)) continue; arr.forEach((sub, i) => { visit(sub, indexPath(childPath(path, key), i), key); }); } for (const key of SUBSCHEMA_KEYWORDS) { if (!(key in node)) continue; const value = node[key]; // Draft-04 tuple form: `items` may be an array of schemas. if (key === "items" && Array.isArray(value)) { value.forEach((sub, i) => { visit(sub, indexPath(childPath(path, key), i), key); }); continue; } visit(value, childPath(path, key), key); } } /** * What to replace a bare `true` with — which depends on the keyword above it. * * `{}` and `true` are the same schema, so `{}` is always the *exact* rewrite. * It is normally not what to advise, because `{}` is the very shape * `untyped-schema` flags — hence the usual advice to declare a real type. * * Under `not` that reverses. `{"not": true}` rejects every value; swapping in * `{"type": "object", …}` leaves the parent rejecting only objects, so every * non-object now passes — the enclosing contract is **widened**, not narrowed. * `{}` is exempt from `untyped-schema` in that position for the same reason, * so it is both exact and safe to recommend there. * * Everywhere else the replacement is described as a deliberate *change* rather * than a narrowing: in an ordinary property position it does narrow, but under * a conditional (`if`) it redirects which branch applies, and a suggestion has * no business claiming an effect it cannot guarantee. */ function booleanTrueSuggestion(keyword: string | undefined): string { if (keyword === "not") { return "Use `{}`, the object form of the always-true schema — under `not` that is an exact replacement. (Declaring a concrete type here would instead let every value of another type through.)"; } return 'Declare what the value actually is — e.g. `{"type": "object", "additionalProperties": true}` for a free-form object. That is a deliberate change of contract, not an equivalent rewrite: `true` accepts any JSON value at all.'; } /** * Visit one schema position. `keyword` is the parent keyword this node sits * under (undefined at the root), which is what decides whether a bare boolean * is idiomatic here or a portability defect. */ function walk( node: unknown, path: string, keyword: string | undefined, depth: number, ctx: WalkContext, ): void { if (depth > MAX_DEPTH) return; if (typeof node === "boolean") { if (keyword !== undefined && BOOLEAN_VALUED_KEYWORDS.has(keyword)) return; add( ctx, "boolean-schema", "error", path, `Bare \`${String(node)}\` used where a schema object is expected.`, node ? booleanTrueSuggestion(keyword) : // `false` in a property position means the property is FORBIDDEN. // Deleting the entry is not the same thing: with `additionalProperties` // absent (default `true`) the property becomes allowed with any value, // so the suggestion has to preserve the always-false constraint. 'Use the object form of the always-false schema — `{"not": {}}`. (Deleting the entry is a different contract: under the default `additionalProperties` the property would then be allowed with any value, not forbidden.)', ); return; } // Anything that is neither a boolean nor an object is not a schema at all. // That is malformed rather than merely unportable, and the SDK's own parser // is the right place to reject it, so this lint stays quiet and stops here. if (!isRecord(node)) return; lintNode(node, path, keyword, ctx); forEachSubschema(node, path, (child, childPathValue, childKeyword) => { walk(child, childPathValue, childKeyword, depth + 1, ctx); }); } /** Rules that apply to a single schema object, ignoring its children. */ function lintNode( node: SchemaRecord, path: string, keyword: string | undefined, ctx: WalkContext, ): void { const type = node.type; if (Array.isArray(type) && isPortableTypeUnion(type)) { // The suggestion is always `anyOf`, never "drop it from `required`". // Accepting an explicit JSON `null` and permitting the property to be // absent are independent: on a required `["null","boolean"]` field, // un-requiring it would both stop accepting `null` and start accepting // omission — a different contract, not the same one spelled portably. // `anyOf` branches each carrying a single `type` are equivalent, and this // lint treats them as portable. add( ctx, "type-union", "warning", path, `\`type\` is an array (${JSON.stringify(type)}). The array form is legal JSON Schema, but several MCP clients read \`type\` as a single string and either reject the tool or drop the constraint.`, `Split it into \`anyOf\` branches, each with a single \`type\` — \`{"anyOf": [${type .map((t) => `{"type": "${t}"}`) .join( ", ", )}]}\`. (Making the property optional instead is a different contract: absent is not the same as \`null\`.)`, ); } const ref = node.$ref; if ( typeof ref === "string" && ref !== "" && !ref.startsWith("#") && !ctx.hasEmbeddedIds ) { add( ctx, "remote-ref", "warning", path, `\`$ref\` points outside this document (\`${ref}\`). Clients do not fetch remote schemas, so the constraint is dropped or the tool is rejected.`, "Inline the referenced schema, or move it into `$defs` and reference it as `#/$defs/`.", ); } const constrains = constrainsInstance(node); // Under `not`, an always-accepting subschema is the idiomatic spelling of // always-*reject* — `{"not": {}}` is the object form of `false`, and the one // this module's own `boolean-schema` suggestion recommends. Reporting it as // "accepts any value" would be the exact opposite of what it does, so the // rule stops at that position rather than contradicting itself. if (!constrains && keyword !== "not") { add( ctx, "untyped-schema", "warning", path, "Schema carries no validation keyword at all, so it accepts any value — the object-literal spelling of a bare `true`.", // "changes", not "narrows": in an ordinary property position it does // narrow, but under a conditional (`if`) a typed replacement redirects // which branch applies rather than tightening anything. 'Declare what the value actually is — e.g. `{"type": "object", "additionalProperties": true}` for a free-form object. That is a deliberate change of contract, which is the point: as written it constrains nothing.', ); } } /** * Lint one of a tool's schemas. * * There is deliberately **no root-level "inputSchema must be an object" rule**, * even though MCP does require that. The SDK's `ToolSchema` types `inputSchema` * with `type: literal("object")`, so a tool whose input root is anything else * fails `ListToolsResultSchema.safeParse`; `salvageListItems` then drops it * before `ManagedToolsState` hands the list to anyone. Verified against a live * server advertising `inputSchema: {type: "array"}` — `listAllTools()` returns * an empty array, so no client can ever pass such a tool to this module. * * A rule that cannot fire is worse than no rule: it makes the documentation * claim a check the tool does not perform. The condition is already reported * through the salvage path's malformed-items surface, which is where a dropped * tool legitimately belongs. */ function lintSchema( schema: unknown, kind: SchemaKind, findings: SchemaFinding[], ): void { walk(schema, "", undefined, 0, { schema: kind, findings, hasEmbeddedIds: declaresAnyId(schema), }); } /** * Lint both of a tool's schemas. Returns findings in walk order — inputSchema * first, then outputSchema — so a report reads top-down through the tool. */ export function lintToolSchemas(tool: Tool): SchemaFinding[] { const findings: SchemaFinding[] = []; if (tool.inputSchema !== undefined) { lintSchema(tool.inputSchema, "inputSchema", findings); } if (tool.outputSchema !== undefined) { lintSchema(tool.outputSchema, "outputSchema", findings); } return findings; } /** Lint a tool list, dropping the tools that came back clean. */ export function lintTools(tools: readonly Tool[]): ToolSchemaFindings[] { const results: ToolSchemaFindings[] = []; for (const tool of tools) { const findings = lintToolSchemas(tool); if (findings.length > 0) results.push({ toolName: tool.name, findings }); } return results; } /** Count findings by severity across a lint result. */ export function countFindings(results: readonly ToolSchemaFindings[]): { errors: number; warnings: number; } { let errors = 0; let warnings = 0; for (const { findings } of results) { for (const f of findings) { if (f.severity === "error") errors++; else warnings++; } } return { errors, warnings }; } /** `2 errors` / `1 warning` — one pluralized count, used in every surface. */ function plural(count: number, noun: string): string { return `${count} ${noun}${count === 1 ? "" : "s"}`; } /** * One-line summary of a lint result, shared by the CLI's non-strict hint, the * TUI's tools header and the web Tools tab so the three cannot disagree. */ export function summarizeFindings( results: readonly ToolSchemaFindings[], ): string { const { errors, warnings } = countFindings(results); return `${plural(errors, "error")}, ${plural(warnings, "warning")} across ${plural(results.length, "tool")}`; } /** * Severity breakdown for ONE tool's findings — "1 error, 3 warnings". * * Deliberately not "N errors" keyed off the highest severity present: a tool * with one error and three warnings is four findings, not four errors, and * labelling the total with the worst severity misreports every mixed result. * Categories that are empty are omitted, so a warning-only tool reads "3 * warnings" rather than "0 errors, 3 warnings". */ export function summarizeToolFindings( findings: readonly SchemaFinding[], ): string { const errors = findings.filter((f) => f.severity === "error").length; const warnings = findings.length - errors; const parts: string[] = []; if (errors > 0) parts.push(plural(errors, "error")); if (warnings > 0) parts.push(plural(warnings, "warning")); return parts.join(", "); } /** * Render a lint result as plain text — the CLI's `--strict` report, and the * body the TUI's detail pane prints. Shared so the two cannot describe the * same finding differently. */ export function formatSchemaLintReport( results: readonly ToolSchemaFindings[], ): string { const lines: string[] = []; for (const { toolName, findings } of results) { for (const f of findings) { lines.push( `${f.severity === "error" ? "Error" : "Warning"}: tool "${toolName}"`, ); lines.push(` Path: ${describeSchemaPath(f.schema, f.path)}`); lines.push(` Issue: ${f.issue}`); lines.push(` Suggestion: ${f.suggestion}`); lines.push(""); } } lines.push(`${summarizeFindings(results)}.`); return lines.join("\n"); }