fix(mcp): expose MCP tool input parameters to the agent
MCP server tools were presented to the agent with only their name and a truncated description: get_tool_descriptions_for_prompt() emitted "- name: description" and get_all_tools() dropped input_schema entirely. On the fenced-block tool path (used by Ollama models), the agent could not see a tool's declared inputs and guessed argument names from the description alone, so tool calls failed (issue #2509). MCP inspector showed the schemas fine, confirming the loss was on our side. - get_all_tools() now carries each tool's input_schema. - get_tool_descriptions_for_prompt() renders a compact args hint (parameter names, coarse types, required-ness) via a new _format_mcp_params() helper, matching the "Args (JSON): {...}" style the built-in tool descriptions already use. Fixes #2509
This commit is contained in:
@@ -30,6 +30,33 @@ def _format_mcp_connection_error(name: str, command: str = "", args: Optional[Li
|
|||||||
return raw_error
|
return raw_error
|
||||||
|
|
||||||
|
|
||||||
|
def _format_mcp_params(input_schema: Any) -> str:
|
||||||
|
"""Render an MCP tool's JSON-Schema inputs as a compact prompt hint.
|
||||||
|
|
||||||
|
Without this the agent only sees a tool's name + description and has to
|
||||||
|
guess its arguments (issue #2509). Produces e.g.
|
||||||
|
` Args (JSON): {"path": string (required), "limit": integer}` — names,
|
||||||
|
coarse types, and required-ness, kept short so it stays prompt-friendly.
|
||||||
|
Returns "" when there are no parameters.
|
||||||
|
"""
|
||||||
|
if not isinstance(input_schema, dict):
|
||||||
|
return ""
|
||||||
|
props = input_schema.get("properties")
|
||||||
|
if not isinstance(props, dict) or not props:
|
||||||
|
return ""
|
||||||
|
required = set(input_schema.get("required") or [])
|
||||||
|
parts = []
|
||||||
|
for pname, pinfo in props.items():
|
||||||
|
pinfo = pinfo if isinstance(pinfo, dict) else {}
|
||||||
|
ptype = pinfo.get("type") or "any"
|
||||||
|
if isinstance(ptype, list):
|
||||||
|
ptype = "|".join(str(x) for x in ptype)
|
||||||
|
tag = f'"{pname}": {ptype}'
|
||||||
|
if pname in required:
|
||||||
|
tag += " (required)"
|
||||||
|
parts.append(tag)
|
||||||
|
return " Args (JSON): {" + ", ".join(parts) + "}"
|
||||||
|
|
||||||
|
|
||||||
class McpManager:
|
class McpManager:
|
||||||
"""Manages MCP server connections and tool routing."""
|
"""Manages MCP server connections and tool routing."""
|
||||||
@@ -376,6 +403,7 @@ class McpManager:
|
|||||||
"name": tool["name"],
|
"name": tool["name"],
|
||||||
"qualified_name": f"mcp__{server_id}__{tool['name']}",
|
"qualified_name": f"mcp__{server_id}__{tool['name']}",
|
||||||
"description": tool.get("description", ""),
|
"description": tool.get("description", ""),
|
||||||
|
"input_schema": tool.get("input_schema") or {},
|
||||||
"is_disabled": tool["name"] in disabled,
|
"is_disabled": tool["name"] in disabled,
|
||||||
})
|
})
|
||||||
return result
|
return result
|
||||||
@@ -439,7 +467,11 @@ class McpManager:
|
|||||||
for t in server_tools:
|
for t in server_tools:
|
||||||
# Truncate long descriptions
|
# Truncate long descriptions
|
||||||
desc = t['description'][:120] + '...' if len(t['description']) > 120 else t['description']
|
desc = t['description'][:120] + '...' if len(t['description']) > 120 else t['description']
|
||||||
lines.append(f" - {t['qualified_name']}: {desc}")
|
# Include the tool's declared inputs so the model calls it with
|
||||||
|
# real argument names instead of guessing from the description
|
||||||
|
# alone (issue #2509).
|
||||||
|
args_hint = _format_mcp_params(t.get("input_schema"))
|
||||||
|
lines.append(f" - {t['qualified_name']}: {desc}{args_hint}")
|
||||||
|
|
||||||
result = "\n".join(lines)
|
result = "\n".join(lines)
|
||||||
self._cached_prompt_desc = result
|
self._cached_prompt_desc = result
|
||||||
|
|||||||
68
tests/test_mcp_tool_params_in_prompt.py
Normal file
68
tests/test_mcp_tool_params_in_prompt.py
Normal file
@@ -0,0 +1,68 @@
|
|||||||
|
"""Regression for issue #2509 — MCP tools must expose their input parameters.
|
||||||
|
|
||||||
|
``McpManager.get_tool_descriptions_for_prompt()`` previously emitted only
|
||||||
|
``- name: description`` per MCP tool, so agents (notably on the fenced-block
|
||||||
|
tool path used by Ollama models) never saw a tool's declared inputs and guessed
|
||||||
|
argument names from the description alone. ``get_all_tools()`` also dropped the
|
||||||
|
``input_schema`` entirely. These tests pin that the inputs now reach both
|
||||||
|
surfaces.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from src.mcp_manager import McpManager
|
||||||
|
|
||||||
|
|
||||||
|
def _mgr_with_tool() -> McpManager:
|
||||||
|
mgr = McpManager()
|
||||||
|
mgr._tools = {
|
||||||
|
"srv1": [
|
||||||
|
{
|
||||||
|
"name": "fetch_doc",
|
||||||
|
"description": "Fetch a document by path.",
|
||||||
|
"input_schema": {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"path": {"type": "string", "description": "file path"},
|
||||||
|
"limit": {"type": "integer"},
|
||||||
|
},
|
||||||
|
"required": ["path"],
|
||||||
|
},
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
mgr._connections = {"srv1": {"status": "connected", "name": "Files", "identity": ""}}
|
||||||
|
return mgr
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_all_tools_carries_input_schema():
|
||||||
|
tools = _mgr_with_tool().get_all_tools()
|
||||||
|
assert tools and tools[0]["input_schema"]["properties"]["path"]["type"] == "string"
|
||||||
|
|
||||||
|
|
||||||
|
def test_prompt_descriptions_surface_param_names_and_required():
|
||||||
|
text = _mgr_with_tool().get_tool_descriptions_for_prompt()
|
||||||
|
assert "mcp__srv1__fetch_doc" in text
|
||||||
|
assert "path" in text and "limit" in text # inputs are surfaced to the model
|
||||||
|
assert "required" in text # required-ness is surfaced
|
||||||
|
|
||||||
|
|
||||||
|
def test_format_mcp_params_handles_no_params():
|
||||||
|
from src.mcp_manager import _format_mcp_params
|
||||||
|
|
||||||
|
assert _format_mcp_params({}) == ""
|
||||||
|
assert _format_mcp_params(None) == ""
|
||||||
|
assert _format_mcp_params({"type": "object", "properties": {}}) == ""
|
||||||
|
|
||||||
|
|
||||||
|
def test_format_mcp_params_marks_required_and_types():
|
||||||
|
from src.mcp_manager import _format_mcp_params
|
||||||
|
|
||||||
|
out = _format_mcp_params(
|
||||||
|
{
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"q": {"type": "string"}, "n": {"type": "integer"}},
|
||||||
|
"required": ["q"],
|
||||||
|
}
|
||||||
|
)
|
||||||
|
assert '"q": string (required)' in out
|
||||||
|
assert '"n": integer' in out
|
||||||
|
assert '"n": integer (required)' not in out # optional param not marked required
|
||||||
Reference in New Issue
Block a user