bvandeusenandClaude Opus 5.5
ba0b56018e
Fourteen more test files: docstrings state the failure each guard defends as a present-tense shape instead of the incident that found it, and issue, milestone and rule numbers leave comments and assertion messages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
128 lines
4.6 KiB
Python
128 lines
4.6 KiB
Python
"""Unknown tool arguments are rejected, never silently dropped.
|
|
|
|
The failure this pins: MCPServer 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` — would
|
|
run, store `body: ""`, and leave a record embedding/search cannot see, while
|
|
REPORTING SUCCESS.
|
|
|
|
The rejection is at the dispatch seam, not per-tool: StrictArgsMCPServer 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 mcp.server.mcpserver.exceptions import ToolError, UnexpectedToolError
|
|
|
|
from scribe.mcp.server import StrictArgsMCPServer
|
|
|
|
|
|
def _echo_server() -> StrictArgsMCPServer:
|
|
mcp = StrictArgsMCPServer("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(ToolError) 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 MCPServer 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)
|
|
|
|
|
|
# --- a refusal's text reaches the agent ---------------------------------------
|
|
#
|
|
# SDK 2.x passes on only a ToolError's text; anything else is wrapped as
|
|
# "Error executing tool X" and its message is kept on the server. ValueError is
|
|
# how every Scribe tool refuses, so without this every refusal arrives bare.
|
|
|
|
|
|
def _raising_server() -> StrictArgsMCPServer:
|
|
mcp = StrictArgsMCPServer("refusal-test")
|
|
|
|
@mcp.tool()
|
|
def refuse() -> str:
|
|
raise ValueError("record 7 is not yours to change — open your own copy")
|
|
|
|
@mcp.tool()
|
|
def crash() -> str:
|
|
raise RuntimeError("password=hunter2 at db.internal:5432")
|
|
|
|
return mcp
|
|
|
|
|
|
async def test_a_refusal_reaches_the_caller_with_its_reason():
|
|
mcp = _raising_server()
|
|
with pytest.raises(ToolError) as exc:
|
|
await mcp.call_tool("refuse", {})
|
|
msg = str(exc.value)
|
|
assert "Error executing tool refuse" in msg
|
|
assert "record 7 is not yours to change — open your own copy" in msg
|
|
assert not isinstance(exc.value, UnexpectedToolError), (
|
|
"still typed as a crash, so the SDK's handler would log it as one")
|
|
|
|
|
|
async def test_a_crash_stays_masked():
|
|
"""Only the refusal convention is unmasked: a crash's text can carry
|
|
internals, and the SDK hides it for that reason."""
|
|
mcp = _raising_server()
|
|
with pytest.raises(UnexpectedToolError) as exc:
|
|
await mcp.call_tool("crash", {})
|
|
assert "hunter2" not in str(exc.value)
|
|
|
|
|
|
@pytest.mark.usefixtures("_bind_user")
|
|
async def test_a_real_tools_refusal_reaches_the_caller():
|
|
"""Against the real server, on a refusal that needs no database: the
|
|
message judge_menu writes for an empty verdict list."""
|
|
from scribe.mcp.server import build_mcp_server
|
|
|
|
with pytest.raises(ToolError) as exc:
|
|
await build_mcp_server().call_tool("judge_menu", {"log_id": 1, "verdicts": []})
|
|
assert "at least one verdict" in str(exc.value)
|
|
|
|
|
|
async def test_the_original_regression_create_note_with_content():
|
|
"""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, StrictArgsMCPServer) # the guard is actually mounted
|
|
with pytest.raises(ToolError) 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
|