Merge pull request #2529 from NubsCarson/codex/2509-mcp-tool-input-params

fix(mcp): expose MCP tool input parameters to the agent

src/agent_loop.py

@@ -475,7 +475,7 @@ _API_HOSTS = frozenset([
     # schemas and the agent silently degrades to fenced-block parsing.
     "localhost", "127.0.0.1", "host.docker.internal",
 ])
-_MCP_KEYWORDS = frozenset(["browse", "browser", "website", "calendar", "event", "email",
+_MCP_KEYWORDS = frozenset(["mcp", "browse", "browser", "website", "calendar", "event", "email",
                            "gmail", "screenshot", "navigate", "click", "miniflux", "rss", "feed"])
 _ADMIN_SCHEMA_NAMES = frozenset([
     "manage_session", "manage_skills", "manage_tasks",

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

tests/test_agent_loop.py

@@ -38,6 +38,7 @@ try:
         _detect_admin_intent,
         _compute_final_metrics,
         _append_tool_results,
+        _MCP_KEYWORDS,
     )
     _IMPORTED_AGENT_LOOP = sys.modules.get("src.agent_loop")
 finally:
@@ -57,6 +58,10 @@ def test_import_stubs_do_not_leak_into_later_tests():
         assert sys.modules.get("src.agent_loop") is not _IMPORTED_AGENT_LOOP
 
 
+def test_mcp_keyword_gate_matches_literal_mcp_requests():
+    assert "mcp" in _MCP_KEYWORDS
+
+
 # ---------------------------------------------------------------------------
 # _detect_admin_intent
 # ---------------------------------------------------------------------------

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