test: guard tool descriptions structurally instead of locking them (#537)

The schema golden deliberately excludes tool descriptions, which left the one
description failure mode that matters uncovered.

Descriptions are the highest-leverage field the server publishes: 81 of the 91
carry their handler's full Args: block, ~50k characters, and that prose decides
which of 91 tools the model picks. A bad input schema fails loudly at call
time; a degraded description causes quiet wrong-tool selection.

But the two ways one can change are not symmetric:

  * A human edits a docstring — already visible in that file's own diff.
    Locking it in the golden adds no information and costs a regeneration
    roughly every five days (104 commits touched handlers/ in six months, ~32
    of them docstring prose). A golden that moves weekly stops being read,
    which would cost the schema half its whole point.
  * The SDK changes how it *derives* descriptions — publishes only the summary
    line, strips Args:, caps length. That silently rewrites all 91 with no diff
    anywhere in the repo, and nothing else in the suite sees it.

Only the second is worth a test, and it does not need the prose locked — just
its shape asserted. Three guards, all mutation-tested against simulated SDK
regressions (first-line-only, empty, capped at 200 chars):

  - every tool publishes a non-empty description
  - every parameterised tool still publishes an Args: block, with a one-entry
    allowlist for master_album, whose parameters are genuinely undocumented
  - no description is truncated

The dump script now emits descriptions and takes --golden to narrow output to
the golden file's fields, so one dump serves both consumers. The golden itself
regenerates byte-identical.

Verified on both SDK lines: 20 passed on 1.28.1 and 2.0.0, and the full dump
including descriptions is byte-identical between them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: bitwize-music

CHANGELOG.md

@@ -14,7 +14,8 @@ This project uses [Conventional Commits](https://conventionalcommits.org/) and [
 - **The MCP server runs on `mcp` 2.x, and still runs on 1.x** ([#537](https://github.com/bitwize-music-studio/claude-ai-music-skills/issues/537)) — `mcp` 2.0.0 moved `mcp.server.fastmcp`'s `FastMCP` to `mcp.server.mcpserver`'s `MCPServer` with no compat shim. `server.py` now imports whichever is present, preferring 2.x, and `requirements.txt` pins `mcp[cli]==2.0.0`. Supporting both is the point rather than a hedge: the plugin updates through the marketplace but the venv is updated by hand, so a hard cutover would have killed the server on every existing install the moment the plugin updated, with no action by the user. `check_venv_health` reports the pin drift and people upgrade on their own schedule. The install advice and all four readiness probes accept either line — a probe naming only one module calls a perfectly working install broken, which is the same false verdict in the opposite direction from the one #542 fixed.
   - **The migration is smaller than it looked, and that was measured rather than assumed.** The concern on the issue was that `_shared.install_error_boundary` monkey-patches `mcp.tool` across all 91 tools and could keep working while silently changing every generated schema. It does not: both SDKs build schemas from the handler signature via `inspect.signature(fn, eval_str=True)`, every registration site is a bare `@mcp.tool()` with no keyword arguments, and `MCPServer` takes the same constructor name, `.tool()` decorator and `.run(transport="stdio")`. Dumped side by side, all 91 tools produce byte-identical `tools/list` wire schemas on 1.28.1 and 2.0.0, and a real stdio handshake returns the same protocol version and capabilities on both.
   - **`serverInfo.version` now reports the plugin version on 2.x.** 1.x has no `version` parameter and hardcodes the SDK's own version — telling a client `1.28.1`, which describes the SDK rather than this server — while 2.x would otherwise default it to the empty string. It is passed only where it is accepted, so 1.x behaviour is unchanged.
-  - **A golden file locks the 91 generated schemas** (`tests/fixtures/tool_schemas.json`, checked by `tests/unit/state/test_tool_schema_parity.py`). This is the schema-parity check the issue asked for, and it keeps earning its place after the migration: drop the `@functools.wraps` out of the error boundary and every tool collapses to a single `async_wrapper` taking `(*args, **kwargs)` — handlers keep working, the rest of the suite stays green, and clients lose every parameter. That is [#443](https://github.com/bitwize-music-studio/claude-ai-music-skills/issues/443)'s failure mode, and nothing else in the suite sees it. Verified by mutation: removing the decorator turns the parity test red. Tool *descriptions* are deliberately outside the golden so ordinary docstring edits do not force a regeneration.
+  - **A golden file locks the 91 generated schemas** (`tests/fixtures/tool_schemas.json`, checked by `tests/unit/state/test_tool_schema_parity.py`). This is the schema-parity check the issue asked for, and it keeps earning its place after the migration: drop the `@functools.wraps` out of the error boundary and every tool collapses to a single `async_wrapper` taking `(*args, **kwargs)` — handlers keep working, the rest of the suite stays green, and clients lose every parameter. That is [#443](https://github.com/bitwize-music-studio/claude-ai-music-skills/issues/443)'s failure mode, and nothing else in the suite sees it. Verified by mutation: removing the decorator turns the parity test red.
+  - **Tool descriptions are guarded structurally rather than locked in the golden.** They are the highest-leverage field the server publishes — 81 of the 91 carry their handler's full `Args:` block, ~50k characters in total, and that prose is what decides which of 91 tools gets picked; a bad input schema fails loudly at call time, a degraded description just causes quiet wrong-tool selection. But the two ways one can change are not symmetric. A human editing a docstring is already visible in that file's own diff, and locking it would force a regeneration roughly every five days at this repo's rate of handler churn — a golden that moves weekly stops being read, which would cost the *schema* half its whole point. An SDK changing how it derives descriptions — publishing only the summary line, stripping `Args:`, capping length — silently rewrites all 91 with no diff anywhere. Only the second needs a test, and it does not need the prose locked, just its shape asserted: every tool has a description, every parameterised tool still publishes an `Args:` block (with a one-entry allowlist for `master_album`, whose parameters are undocumented), and nothing is truncated. All three were mutation-tested against simulated SDK regressions. Descriptions are byte-identical on 1.28.1 and 2.0.0 today; this is what keeps that true.
   - **A `MCP Server Boot (mcp 1.x fallback)` CI job covers the fallback branch.** Every other job installs `requirements.txt` and therefore only ever exercises 2.x, which would leave the 1.x path uncovered from the day it was written — the way compat shims rot. The job installs the pinned set, downgrades mcp alone to the 1.28.1 floor, fails loudly if the downgrade did not take, boots the server through the same `mcp-launch` launcher `.mcp.json` uses, and re-checks the schema golden. That last step is what holds the cross-SDK guarantee: the golden is generated on one line and must reproduce byte-for-byte on the other. Linux-only, since the variable under test is the SDK and not the OS.
 - **Dependency bumps that the grouped PR could not deliver: `ruff` 0.16.0 → 0.16.2, `boto3` 1.43.56 → 1.43.69, `playwright` 1.61.0 → 1.62.0** — these three were safe the whole time, but rode in a `pip-all` group PR alongside two upgrades that cannot land, so all three sat unmerged. Split out and verified on their own. `ruff` matters most of the three: it is one of the exactly-pinned gate tools from [#532](https://github.com/bitwize-music-studio/claude-ai-music-skills/issues/532), so its verdict changes on unchanged code — 0.16.2 was run against the full tree (`tools/`, `servers/`, `hooks/`, plus the scoped `PLW1514` preview pass) and reports no new findings.
 - **`librosa` 1.0+ is blocked in `.github/dependabot.yml`, on the same Python-floor grounds as `scipy` and `numpy`** — `librosa` 1.0.0 declares `Requires-Python >=3.12` and the plugin supports 3.11, so pip cannot resolve it at all: `No matching distribution found for librosa==1.0.0`. That is worse than a failing test. Every job that installs `requirements.txt` dies at the install step, `pip-audit` included, which is why the group PR carrying it failed 11 checks rather than the 6 that mcp alone accounts for. It joins the existing ignore block, whose comment now also records *why* this class of upgrade is blocked rather than merely which packages are affected. ([#532](https://github.com/bitwize-music-studio/claude-ai-music-skills/issues/532)) — `requirements.txt` pinned all 16 runtime deps with `==`, but every entry in `requirements-test.txt` used `>=`, so `ruff`, `mypy` and `bandit` resolved to whatever was newest on PyPI at the moment CI ran. Those three decide the Lint and Security Scan verdicts, and unlike a test runner they change their answer on unchanged code — a new rule or a widened check reddens a commit nobody touched, and re-running an old green build no longer reproduces it. The drift was already visible: the file read `ruff>=0.15.21` while CI had been installing `0.16.0`, which is why Dependabot closed #522 as redundant. `ruff`, `mypy` and `bandit` are now `==` pins (`cache: 'pip'` never mitigated this — it caches wheels, but pip still resolves to newest). The `pytest` stack stays on `>=`: it changes what runs, not what counts as a violation. A parametrized test in `tests/unit/shared/test_pinned_dependencies.py` keeps the three from silently loosening again.

tests/unit/state/_dump_tool_schemas.py

@@ -7,7 +7,7 @@
     diffs the output against the committed golden file;
   * by a maintainer regenerating that golden after an intentional tool change::
 
-        python3 tests/unit/state/_dump_tool_schemas.py > tests/fixtures/tool_schemas.json
+        python3 tests/unit/state/_dump_tool_schemas.py --golden > tests/fixtures/tool_schemas.json
 
 Why a subprocess rather than an in-process import: the ~24 test modules that
 exercise ``server.py`` install a fake ``mcp.server.fastmcp`` into ``sys.modules``
@@ -51,22 +51,32 @@ def _isolate_state_cache(tmp: Path) -> None:
     indexer.LOCK_FILE = tmp / "state.lock"
 
 
+# The fields the committed golden file locks. `description` is deliberately NOT
+# among them: it comes from the handler docstring, so locking it would turn every
+# prose edit into a regeneration — roughly one every five days at this repo's rate
+# of handler churn. A golden that moves weekly stops being read, which would cost
+# the schema half of this file its whole point. Descriptions are still *dumped*,
+# and guarded structurally instead — see test_tool_schema_parity.py.
+GOLDEN_FIELDS = ("name", "inputSchema", "outputSchema")
+
+
+def golden_projection(tool: dict[str, object]) -> dict[str, object]:
+    """Narrow a dumped tool to the fields the golden file locks."""
+    return {field: tool.get(field) for field in GOLDEN_FIELDS}
+
+
 def collect() -> list[dict[str, object]]:
-    """Return each tool's name and generated schemas, sorted by name."""
+    """Return each tool's description and generated schemas, sorted by name."""
     import server
 
     tools = asyncio.run(server.mcp.list_tools())
     dumped = [t.model_dump(mode="json", by_alias=True, exclude_none=True) for t in tools]
 
-    # `description` is deliberately excluded. It comes from the handler docstring,
-    # so including it would turn every prose edit into a golden-file regeneration —
-    # real friction for contributors, and not what this file guards. The generated
-    # schemas are the SDK-dependent part (#537) and the part the error boundary
-    # could silently change (#443).
     return sorted(
         (
             {
                 "name": t["name"],
+                "description": t.get("description") or "",
                 "inputSchema": t.get("inputSchema"),
                 "outputSchema": t.get("outputSchema"),
             }
@@ -77,9 +87,14 @@ def collect() -> list[dict[str, object]]:
 
 
 def main() -> None:
+    """Dump all tools; ``--golden`` narrows output to the golden file's fields."""
+    golden_only = "--golden" in sys.argv[1:]
     with tempfile.TemporaryDirectory() as tmp:
         _isolate_state_cache(Path(tmp))
-        json.dump(collect(), sys.stdout, indent=2, sort_keys=True)
+        tools = collect()
+        if golden_only:
+            tools = [golden_projection(t) for t in tools]
+        json.dump(tools, sys.stdout, indent=2, sort_keys=True)
         sys.stdout.write("\n")
 
 

tests/unit/state/test_tool_schema_parity.py

@@ -19,10 +19,13 @@
    passing, and clients lose every parameter. That failure mode is exactly what a
    golden file catches and nothing else does.
 
+Tool *descriptions* are guarded differently — see the description tests at the
+bottom of this file for why they are not in the golden.
+
 Regenerate after an intentional tool change (new tool, renamed or retyped
 parameter) and review the diff as part of the change::
 
-    python3 tests/unit/state/_dump_tool_schemas.py > tests/fixtures/tool_schemas.json
+    python3 tests/unit/state/_dump_tool_schemas.py --golden > tests/fixtures/tool_schemas.json
 """
 
 import json
@@ -36,7 +39,13 @@
 DUMP_SCRIPT = PROJECT_ROOT / "tests" / "unit" / "state" / "_dump_tool_schemas.py"
 GOLDEN = PROJECT_ROOT / "tests" / "fixtures" / "tool_schemas.json"
 
-REGENERATE = f"python3 {DUMP_SCRIPT.relative_to(PROJECT_ROOT)} > {GOLDEN.relative_to(PROJECT_ROOT)}"
+REGENERATE = (
+    f"python3 {DUMP_SCRIPT.relative_to(PROJECT_ROOT)} --golden "
+    f"> {GOLDEN.relative_to(PROJECT_ROOT)}"
+)
+
+# Fields the golden locks — kept in sync with _dump_tool_schemas.GOLDEN_FIELDS.
+GOLDEN_FIELDS = ("name", "inputSchema", "outputSchema")
 
 
 def _dump_live_schemas() -> list[dict]:
@@ -66,7 +75,10 @@ def _dump_live_schemas() -> list[dict]:
 @pytest.mark.unit
 def test_tool_schemas_match_golden() -> None:
     """Every registered tool's generated schemas match the committed golden."""
-    live = _dump_live_schemas()
+    live = [
+        {field: tool.get(field) for field in GOLDEN_FIELDS}
+        for tool in _dump_live_schemas()
+    ]
     golden = json.loads(GOLDEN.read_text(encoding="utf-8"))
 
     live_names = [t["name"] for t in live]
@@ -129,3 +141,91 @@ def test_golden_covers_every_tool_with_a_schema() -> None:
         f"{len(no_params)} of {len(golden)} tools take no parameters. That is the "
         f"shape of collapsed (*args, **kwargs) introspection, not a real API."
     )
+
+
+# --- tool descriptions --------------------------------------------------------
+#
+# Descriptions are the highest-leverage field the server publishes: 81 of the 91
+# carry their handler's full `Args:` block, ~50k characters in total, and that
+# prose is what the model reads when choosing between 91 tools. A bad input schema
+# fails loudly at call time; a degraded description just causes quiet wrong-tool
+# selection that reads as the model being dim.
+#
+# They are still kept OUT of the golden file, because the two ways a description
+# can change are not symmetric:
+#
+#   * A human edits a docstring — already visible in that file's own diff. Locking
+#     it in the golden adds no information and costs a regeneration every few days.
+#   * The SDK changes how it *derives* descriptions — starts at the summary line,
+#     strips `Args:`, truncates, dedents differently. That silently rewrites all 91
+#     with no diff anywhere in the repo, and nothing else in the suite would see it.
+#
+# Only the second is worth a guard, and it does not need the prose locked — just
+# its shape asserted. That is what these tests do: SDK-drift coverage with zero
+# churn on ordinary docstring edits.
+
+# Parameterised tools whose docstring legitimately has no `Args:` block. Keep this
+# list short — an entry is a tool whose parameters are undocumented to the model.
+TOOLS_WITHOUT_ARGS_BLOCK = {"master_album"}
+
+
+@pytest.mark.unit
+def test_every_tool_has_a_description() -> None:
+    """An empty description leaves the model picking a tool by name alone."""
+    live = _dump_live_schemas()
+
+    missing = [t["name"] for t in live if not (t.get("description") or "").strip()]
+    assert not missing, (
+        f"{len(missing)} tool(s) publish no description: {missing[:10]}. Either the "
+        f"handler lost its docstring, or the SDK stopped deriving descriptions from "
+        f"one — check `mcp.server.*`'s tool registration before assuming the former."
+    )
+
+
+@pytest.mark.unit
+def test_parameterised_tools_document_their_arguments() -> None:
+    """A tool's parameters must still reach the model, not just its summary line.
+
+    This is the SDK-drift canary. If a future mcp version publishes only the
+    docstring's first line — a plausible and entirely silent change — every
+    parameterised tool keeps working while the model loses the text telling it what
+    the parameters mean. Nothing else in the suite notices.
+    """
+    live = _dump_live_schemas()
+
+    undocumented = sorted(
+        t["name"]
+        for t in live
+        if (t.get("inputSchema") or {}).get("properties")
+        and t["name"] not in TOOLS_WITHOUT_ARGS_BLOCK
+        and "Args:" not in (t.get("description") or "")
+    )
+    assert not undocumented, (
+        f"{len(undocumented)} parameterised tool(s) publish no `Args:` block: "
+        f"{undocumented[:10]}.\n\nIf many tools regressed at once, suspect the SDK's "
+        f"docstring handling rather than the handlers — that is the failure this "
+        f"test exists for. If a single new tool is listed, give it an `Args:` block "
+        f"or add it to TOOLS_WITHOUT_ARGS_BLOCK with a reason."
+    )
+
+
+@pytest.mark.unit
+def test_descriptions_are_not_truncated() -> None:
+    """Catch an SDK that starts capping description length.
+
+    A cap would not empty any description, so the two tests above would both pass
+    while the model silently lost the tail of every long one.
+    """
+    live = _dump_live_schemas()
+    descriptions = {t["name"]: (t.get("description") or "") for t in live}
+
+    elided = sorted(n for n, d in descriptions.items() if d.rstrip().endswith(("...", "…")))
+    assert not elided, f"description(s) end in an ellipsis, i.e. truncated: {elided[:10]}"
+
+    # The longest description is ~2.3k characters. If the maximum collapses to a
+    # round-ish number, something upstream is capping it.
+    longest = max(descriptions.values(), key=len)
+    assert len(longest) > 1000, (
+        f"the longest tool description is only {len(longest)} characters. These run "
+        f"to ~2.3k, so a ceiling this low means something upstream is truncating them."
+    )