diff --git a/src/mcp_manager.py b/src/mcp_manager.py index 811094f..7cd9740 100644 --- a/src/mcp_manager.py +++ b/src/mcp_manager.py @@ -30,6 +30,33 @@ def _format_mcp_connection_error(name: str, command: str = "", args: Optional[Li 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: """Manages MCP server connections and tool routing.""" @@ -376,6 +403,7 @@ class McpManager: "name": tool["name"], "qualified_name": f"mcp__{server_id}__{tool['name']}", "description": tool.get("description", ""), + "input_schema": tool.get("input_schema") or {}, "is_disabled": tool["name"] in disabled, }) return result @@ -439,7 +467,11 @@ class McpManager: for t in server_tools: # Truncate long descriptions 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) self._cached_prompt_desc = result diff --git a/tests/test_mcp_tool_params_in_prompt.py b/tests/test_mcp_tool_params_in_prompt.py new file mode 100644 index 0000000..c3149c5 --- /dev/null +++ b/tests/test_mcp_tool_params_in_prompt.py @@ -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