fix(mcp): reject undeclared tool arguments instead of silently dropping them (#2709)

FastMCP validates tool arguments with a pydantic model whose extra-field
policy is 'ignore', so create_note(content=...) — a plausible near-miss
for body=, primed by add_task_log's content — ran successfully, stored
body: '', and left a record embedding/search cannot see. Two real notes
were persisted body-less before the pattern was noticed; create_task
only 'worked' because those calls happened to use the right name.

StrictArgsFastMCP rejects any tool call carrying arguments the tool does
not declare, before dispatch, with a did-you-mean hint when one is close
and the declared list when none is. Applied at the dispatch seam so
every tool gets the guarantee — an error the caller sees once beats data
half-written forever.

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

src/scribe/mcp/server.py

@@ -1,6 +1,8 @@
 """FastMCP instance + Quart mount-point. Tools are registered in mcp/tools/."""
 from __future__ import annotations
 
+import difflib
+
 from mcp.server.fastmcp import FastMCP
 from mcp.server.transport_security import TransportSecuritySettings
 from quart import Quart
@@ -165,6 +167,46 @@ def _body_calls_write_tool(body: bytes) -> bool:
     return False
 
 
+class StrictArgsFastMCP(FastMCP):
+    """A FastMCP that REJECTS tool calls carrying undeclared arguments.
+
+    FastMCP validates arguments with a pydantic model built from the tool
+    signature, and pydantic's default extra-field policy is "ignore" — so a
+    misnamed argument simply vanishes and the tool runs with that field's
+    default. On the create/update tools the default is "", which turns a
+    plausible near-miss (`content=` for `body=`, primed by add_task_log's
+    `content`) into SILENT DATA LOSS: the call reports success and stores an
+    empty body, leaving a record search cannot see (#2709). Two notes were
+    persisted body-less that way before anyone noticed.
+
+    An error the caller sees once is strictly better than data half-written
+    forever, so the policy is applied to every tool, not just the two that
+    bit: nothing here knows tool semantics, only that an argument nobody
+    declared cannot have been meant to be dropped.
+    """
+
+    async def call_tool(self, name, arguments):
+        try:
+            tool = self._tool_manager.get_tool(name)
+        except Exception:
+            tool = None  # unknown tool → let upstream produce its own error
+        if tool is not None:
+            declared = set((tool.parameters or {}).get("properties", {}))
+            unknown = sorted(set(arguments or {}) - declared)
+            if unknown:
+                hints = []
+                for arg in unknown:
+                    close = difflib.get_close_matches(arg, sorted(declared), n=1)
+                    suggestion = f" (did you mean '{close[0]}'?)" if close else ""
+                    hints.append(f"'{arg}'{suggestion}")
+                raise ValueError(
+                    f"{name} does not accept argument(s) {', '.join(hints)}. "
+                    f"It accepts: {', '.join(sorted(declared))}. Nothing was "
+                    "created or changed — retry with the declared names."
+                )
+        return await super().call_tool(name, arguments)
+
+
 def build_mcp_server() -> FastMCP:
     """Build the FastMCP instance with all tools registered.
 
@@ -185,7 +227,7 @@ def build_mcp_server() -> FastMCP:
     # every request self-contained (bearer-auth only), so a post-deploy
     # reconnect just works. Trade-off: no server-pushed list_changed stream,
     # which we don't use — tools are re-fetched on reconnect anyway.
-    mcp = FastMCP(
+    mcp = StrictArgsFastMCP(
         "scribe",
         instructions=_INSTRUCTIONS.strip(),
         stateless_http=True,

tests/test_mcp_strict_args.py

@@ -0,0 +1,75 @@
+"""Unknown tool arguments are rejected, never silently dropped (#2709).
+
+The failure this pins: FastMCP validates tool arguments with a pydantic model
+whose extra-field policy is "ignore", so `create_note(content=...)` — a
+plausible near-miss for `body=`, primed by add_task_log's `content` — ran
+successfully, stored `body: ""`, and left a record embedding/search cannot
+see. The call REPORTED SUCCESS. Two real notes were persisted body-less
+before the pattern was noticed.
+
+The fix is at the dispatch seam, not per-tool: StrictArgsFastMCP rejects any
+call carrying arguments the tool does not declare, with a did-you-mean when
+one is close. An error the caller sees once beats data half-written forever.
+"""
+import pytest
+
+from scribe.mcp.server import StrictArgsFastMCP
+
+
+def _echo_server() -> StrictArgsFastMCP:
+    mcp = StrictArgsFastMCP("strict-test")
+
+    @mcp.tool()
+    def echo(text: str = "") -> str:
+        return text
+
+    return mcp
+
+
+async def test_declared_arguments_still_dispatch():
+    mcp = _echo_server()
+    result = await mcp.call_tool("echo", {"text": "hi"})
+    assert "hi" in str(result)
+
+
+async def test_unknown_argument_is_an_error_not_a_silent_drop():
+    """The load-bearing property: the call must FAIL, because succeeding is
+    what turned a typo into data loss."""
+    mcp = _echo_server()
+    with pytest.raises(ValueError) as exc:
+        await mcp.call_tool("echo", {"txt": "hi"})
+    msg = str(exc.value)
+    assert "'txt'" in msg
+    assert "did you mean 'text'?" in msg          # difflib near-miss hint
+    assert "Nothing was created or changed" in msg
+
+
+async def test_empty_arguments_pass():
+    mcp = _echo_server()
+    assert await mcp.call_tool("echo", {}) is not None
+
+
+async def test_unknown_tool_keeps_the_upstream_error():
+    """The gate must not swallow or reshape 'no such tool' — that error path
+    belongs to FastMCP and clients already understand it."""
+    mcp = _echo_server()
+    with pytest.raises(Exception) as exc:
+        await mcp.call_tool("no_such_tool", {"text": "hi"})
+    assert "no_such_tool" in str(exc.value)
+
+
+async def test_the_original_regression_create_note_with_content():
+    """Pin #2709 itself against the REAL server: `content=` on create_note
+    must raise before dispatch — naming the bad argument and listing `body`
+    among the accepted ones — instead of creating a body-less note."""
+    from scribe.mcp.server import build_mcp_server
+
+    mcp = build_mcp_server()
+    assert isinstance(mcp, StrictArgsFastMCP)   # the guard is actually mounted
+    with pytest.raises(ValueError) as exc:
+        await mcp.call_tool(
+            "create_note", {"title": "t", "content": "the body text"}
+        )
+    msg = str(exc.value)
+    assert "'content'" in msg
+    assert "body" in msg