Address code review feedback for MCP implementation

rfcs/RFC-003-implementation-journal.md

@@ -96,7 +96,7 @@ This journal tracks the implementation of RFC-003: MCP (Model Context Protocol)
 
 #### Echo Env Conversion
 - [x] Deprecate `src/envs/echo_env/models.py` (EchoAction deprecated)
-- [x] Create `src/envs/echo_env/server/mcp_tools.py`
+- [x] Create `src/envs/echo_env/server/mcp_server.py`
 - [x] Define `echo_message` tool using FastMCP
 - [x] Update `src/envs/echo_env/server/echo_environment.py`
 - [x] Update `src/envs/echo_env/server/app.py` to initialize MCP
@@ -146,7 +146,7 @@ This journal tracks the implementation of RFC-003: MCP (Model Context Protocol)
    - Added `mcp_server` parameter to `HTTPEnvServer` and `create_fastapi_app()`
 
 5. **Echo Env Conversion**:
-   - Created `mcp_tools.py` with `echo_message` tool using FastMCP decorators
+   - Created `mcp_server.py` with `echo_message` tool using FastMCP decorators
    - Rewrote `EchoEnvironment` to use MCP client instead of custom actions
    - Updated `app.py` to initialize MCP server, client, and wire them together
    - Deprecated `EchoAction` and `EchoObservation` in `models.py` with warnings

src/core/env_server/http_server.py

@@ -14,7 +14,6 @@
 from __future__ import annotations
 
 import asyncio
-import json
 import os
 from concurrent.futures import ThreadPoolExecutor
 from dataclasses import asdict
@@ -23,6 +22,7 @@
 from fastapi import Body, FastAPI, Request
 
 from .interfaces import Environment
+from .mcp_environment import MCPEnvironment
 from .types import Action, CallToolAction, ListToolsAction, Observation
 
 
@@ -148,17 +148,28 @@ async def mcp_endpoint(request: Request) -> Dict[str, Any]:
             Returns:
                 JSON-RPC 2.0 response
             """
-            if self.env.mcp_client is None:
+            if not hasattr(self.env, "mcp_client") or self.env.mcp_client is None:
                 return {
                     "jsonrpc": "2.0",
                     "error": {
-                        "code": -32600,
+                        "code": -32603,
                         "message": "MCP server not configured for this environment",
                     },
                     "id": None,
                 }
 
-            body = await request.json()
+            try:
+                body = await request.json()
+            except (ValueError, TypeError):
+                return {
+                    "jsonrpc": "2.0",
+                    "error": {
+                        "code": -32700,
+                        "message": "Parse error: Invalid JSON",
+                    },
+                    "id": None,
+                }
+
             method = body.get("method")
             params = body.get("params", {})
             request_id = body.get("id")
@@ -240,8 +251,11 @@ def _deserialize_action(self, action_data: Dict[str, Any]) -> Action:
             return ListToolsAction()
 
         elif action_type == "CallToolAction":
+            tool_name = action_data.get("tool_name")
+            if tool_name is None:
+                raise ValueError("Missing required field 'tool_name' for CallToolAction")
             return CallToolAction(
-                tool_name=action_data["tool_name"],
+                tool_name=tool_name,
                 parameters=action_data.get("parameters", {}),
             )
 

src/core/env_server/mcp_environment.py

@@ -67,7 +67,7 @@ def __init__(self, mcp_server: Any):
 
         self.mcp_server = mcp_server
         self.mcp_client = Client(mcp_server)
-        super().__init__(mcp_client=self.mcp_client)
+        super().__init__()
 
     def reset(self) -> Observation:
         """

src/core/pyproject.toml

@@ -44,7 +44,6 @@ packages = [
     "openenv_core.containers",
     "openenv_core.containers.runtime",
     "openenv_core.env_server",
-    "openenv_core.tools",
-    "openenv_core.mcp"
+    "openenv_core.tools"
 ]
 package-dir = {"openenv_core" = "."}

src/envs/echo_env/client.py

@@ -18,7 +18,6 @@
     from core.env_server.types import (
         CallToolAction,
         CallToolObservation,
-        ListToolsAction,
         ListToolsObservation,
         Observation,
         State,
@@ -29,7 +28,6 @@
     from openenv_core.env_server.types import (
         CallToolAction,
         CallToolObservation,
-        ListToolsAction,
         ListToolsObservation,
         Observation,
         State,
@@ -136,3 +134,35 @@ def _parse_state(self, payload: Dict) -> State:
             episode_id=payload.get("episode_id"),
             step_count=payload.get("step_count", 0),
         )
+
+    def list_tools(self) -> List[Dict[str, Any]]:
+        """
+        List available tools from the MCP server.
+
+        Returns:
+            List of tool dictionaries with name, description, and inputSchema
+        """
+        # Send action via step with correct payload format
+        payload = {"type": "ListToolsAction"}
+        response = self._session.post(f"{self.base_url}/step", json={"action": payload})
+        response.raise_for_status()
+        result = self._parse_result(response.json())
+        if isinstance(result.observation, ListToolsObservation):
+            return result.observation.tools
+        return []
+
+    def echo_message(self, message: str) -> Dict[str, Any]:
+        """
+        Call the echo_message tool.
+
+        Args:
+            message: The message to echo
+
+        Returns:
+            Dictionary containing the echoed message result
+        """
+        action = CallToolAction(tool_name="echo_message", parameters={"message": message})
+        result = self.step(action)
+        if isinstance(result.observation, CallToolObservation):
+            return result.observation.result or {}
+        return {}