mcp-doctor — a static-analysis linter for MCP server quality (feedback welcome) #3322

I found a mixed-coverage edge case in --diff-against at dd03b2c: switching from a FastMCP function to a raw Tool(inputSchema=...) reports a removed parameter even when both source fixtures declare the same required string key.

Minimal pair, in separate before/server.py and after/server.py directories:

# before/server.py
from mcp.server.fastmcp import FastMCP
mcp = FastMCP("synthetic")

@mcp.tool()
def lookup(key: str) -> str:
    """Look up a synthetic value.

    Args:
        key: Key to look up.
    """
    return key

# after/server.py
from mcp.types import Tool
lookup_tool = Tool(
    name="lookup",
    description="Look up a synthetic value.",
    inputSchema={
        "type": "object",
        "properties": {"key": {"type": "string", "description": "Key to look up."}},
        "required": ["key"],
    },
)

python -m mcp_doctor.cli before --json > baseline.json
python -m mcp_doctor.cli after --json --diff-against baseline.json --fail-on-breaking-change

The actual analyzer → JSON snapshot → CLI path produces param_removed for key and exits 1. Both findings have param_count=1, but the raw-schema snapshot has empty param_names / required_param_names. Reversing the comparison produces param_newly_required; each fixture compared with itself produces no changes. No --fail-under was used.

I saw the documented raw-schema → raw-schema limitation; this is the cross-style case where unknown coverage is interpreted as known absence. A separate “parameter schema inspected” flag, with parameter-level comparison only when both snapshots have coverage, would avoid conflating those states. Legacy snapshots missing these fields need the same consideration.

Scope: reproduced on Python 3.12.3 against the pinned analyzer and CLI, using synthetic sources. No MCP SDK/server was run, so this is not a runtime wire-schema equivalence test.

Disclosure: I co-founded Mnemoverse. Codex performed the source analysis, synthetic reproduction, and wrote this comment on my behalf.

Hi Edward, thanks for the minimal repro — that made this trivial to confirm and fix.

You found the real root cause: _find_lowlevel_tools (the raw Tool(inputSchema=...) constructor style) never populated param_names/required_param_names at all, so --diff-against silently read "coverage not attempted" as "genuinely no parameters." Diffing across two different Python registration styles surfaced it; comparing a style against itself couldn't.

Fixed at the source rather than adding a "schema inspected" flag: the analyzer now statically resolves param_names from the schema's properties keys and required_param_names from its required list — the same information the raw-schema style always had available but never extracted. Your exact before/after pair now produces zero schema changes, and a genuine cross-style parameter removal is still caught correctly.

Kept it honest about the remaining edge: if any property name in the schema isn't a string literal (a dynamic key, or an unresolved **spread_call()), param_names is incomplete, so required_param_names is dropped back to empty for that tool rather than risk misreading a still-present property as newly required.

Shipped in v0.9.3: vishalhabib99/mcp-doctor@45a52c5, with your repro as a permanent regression test. Attributed to you in the README's changelog and the known-limitations section.

Nice tool — the diff-against fix above is a good example of exactly the kind of thing static checks should be catching.

One check worth adding on the annotation side (separate from the parameter-coverage checks discussed above): the spec's own doc comment on destructiveHint says it's "meaningful only when readOnlyHint == false" (schema.ts, ToolAnnotations). So a tool that declares readOnlyHint: true and explicitly sets destructiveHint: true is self-contradictory per the spec, but nothing in the JSON Schema itself enforces that — they're just two independent optional booleans, so nothing stops both from being set. That combination is an easy copy-paste mistake when a new tool is cloned from an existing write-tool's annotation block and only readOnlyHint gets flipped to true.

A lint rule flagging readOnlyHint === true && destructiveHint !== undefined (the property has no defined meaning in that case at all, regardless of value) seems like a cheap, high-signal check to add alongside the description/parameter coverage you're already doing.