refactor: address Gemini review feedback

- Collapse verbose if/continue search chain into single boolean (tool_proxy.py)
- Remove unnecessary isinstance(params, dict) guards in _schema_hash,
  _make_summary, and _format_parameters — register_tool() guarantees dict
- Replace fragile startswith("ha_find_tool") with _META_TOOLS constant set
- Remove dead _unwrap_proxy_error method (ToolError now propagates directly)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: kingpanther13

src/ha_mcp/tools/tool_proxy.py

@@ -93,26 +93,14 @@ def find_tools(self, query: str) -> list[dict[str, Any]]:
         results = []
 
         for name, tool in self._tools.items():
-            # Match on tool name
-            if query_lower in name.lower():
-                results.append(self._make_summary(tool))
-                continue
-
-            # Match on category
-            if query_lower in tool["category"].lower():
-                results.append(self._make_summary(tool))
-                continue
-
-            # Match on description keywords
-            if query_lower in tool["description"].lower():
-                results.append(self._make_summary(tool))
-                continue
-
-            # Match on annotation tags
             tags = tool["annotations"].get("tags", [])
-            if any(query_lower in tag.lower() for tag in tags):
+            if (
+                query_lower in name.lower()
+                or query_lower in tool["category"].lower()
+                or query_lower in tool["description"].lower()
+                or any(query_lower in tag.lower() for tag in tags)
+            ):
                 results.append(self._make_summary(tool))
-                continue
 
         return results
 
@@ -156,28 +144,24 @@ def _schema_hash(self, tool_name: str) -> str:
         if not tool:
             return ""
         params = tool["parameters"]
-        param_keys = sorted(params.get("properties", {}).keys()) if isinstance(params, dict) else []
-        required = sorted(params.get("required", [])) if isinstance(params, dict) else []
+        param_keys = sorted(params.get("properties", {}).keys())
+        required = sorted(params.get("required", []))
         fingerprint = f"{tool_name}:{','.join(param_keys)}:{','.join(required)}"
         return hashlib.md5(fingerprint.encode()).hexdigest()[:8]
 
     def _make_summary(self, tool: dict[str, Any]) -> dict[str, Any]:
         params = tool["parameters"]
-        props = params.get("properties", {}) if isinstance(params, dict) else {}
         return {
             "tool_name": tool["name"],
             "summary": tool["description"].strip().split("\n")[0],
             "category": tool["category"],
             "is_destructive": tool["annotations"].get("destructiveHint", False),
-            "parameters": list(props.keys()),
-            "required_parameters": params.get("required", []) if isinstance(params, dict) else [],
+            "parameters": list(params.get("properties", {}).keys()),
+            "required_parameters": params.get("required", []),
         }
 
     def _format_parameters(self, params: dict[str, Any]) -> list[dict[str, Any]]:
         """Format parameter schema into LLM-friendly list."""
-        if not isinstance(params, dict):
-            return []
-
         properties = params.get("properties", {})
         required = set(params.get("required", []))
         result = []

tests/src/e2e/conftest.py

@@ -336,6 +336,9 @@ async def mcp_server(
     # Server cleanup handled by server.close()
 
 
+_META_TOOLS = {"ha_find_tools", "ha_get_tool_details", "ha_execute_tool"}
+
+
 class _ProxyAwareClient:
     """Wraps a FastMCP Client to transparently route proxied tools.
 
@@ -356,9 +359,7 @@ async def call_tool(self, tool_name: str, args: dict[str, Any] | None = None) ->
             args = {}
 
         # Meta-tools and known non-proxied tools skip the proxy
-        if tool_name in self._direct_tools or tool_name.startswith("ha_find_tool") or tool_name in (
-            "ha_get_tool_details", "ha_execute_tool",
-        ):
+        if tool_name in self._direct_tools or tool_name in _META_TOOLS:
             return await self._client.call_tool(tool_name, args)
 
         # Discover whether this tool is proxied (result cached)
@@ -372,7 +373,7 @@ async def call_tool(self, tool_name: str, args: dict[str, Any] | None = None) ->
                 return await self._client.call_tool(tool_name, args)
             self._schema_cache[tool_name] = details["schema_hash"]
 
-        result = await self._client.call_tool(
+        return await self._client.call_tool(
             "ha_execute_tool",
             {
                 "tool_name": tool_name,
@@ -381,43 +382,6 @@ async def call_tool(self, tool_name: str, args: dict[str, Any] | None = None) ->
             },
         )
 
-        # Unwrap proxy error responses so tests see the original tool output.
-        # When a proxied tool raises an exception, ha_execute_tool catches it
-        # and wraps the response as {"error": {"message": "Tool execution
-        # failed: {original_json}"}}.  We extract the original JSON so tests
-        # don't need to know about the proxy layer.
-        return self._unwrap_proxy_error(result)
-
-    @staticmethod
-    def _unwrap_proxy_error(result: Any) -> Any:
-        """If *result* is a proxy-wrapped error containing the original tool
-        response as stringified JSON, return a synthetic result with the
-        unwrapped JSON so callers see the original response shape."""
-        try:
-            data = parse_mcp_result(result)
-        except Exception:
-            return result
-
-        error = data.get("error")
-        if not isinstance(error, dict):
-            return result
-
-        msg = error.get("message", "")
-        prefix = "Tool execution failed: "
-        if not msg.startswith(prefix):
-            return result
-
-        # Try to parse the original tool response out of the message
-        try:
-            original = json.loads(msg[len(prefix):])
-        except (json.JSONDecodeError, TypeError):
-            return result
-
-        # Rebuild as a synthetic MCP text-content result matching what
-        # the original tool would have returned directly.
-        from mcp.types import TextContent
-        return [TextContent(type="text", text=json.dumps(original))]
-
     # Forward everything else (list_tools, session, etc.) to the real client
     def __getattr__(self, name: str) -> Any:
         return getattr(self._client, name)