diff --git a/AGENTS.md b/AGENTS.md index a372ebb5a3..cd4210ffb0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,8 +22,8 @@ OpenEnv is a Python (`uv`) monorepo: a core library + `openenv` CLI in `src/open - `openenv serve` is a stub that just prints instructions; run the server module (or `uvicorn echo_env.server.app:app`) directly instead. - Interact via the client: `EchoEnv(base_url="http://localhost:8000").sync()` then `.reset()` / `.list_tools()` / `.step(CallToolAction(...))`. -### Optional Gradio web UI gotcha -- The debug web UI is off by default; enable with `ENABLE_WEB_INTERFACE=true` and open `/web/`. Reset works, but the auto-generated Playground "Step" form has a pre-existing bug for MCP `CallToolAction`: the `arguments` field is a plain textbox passed as a string, so dict-typed args fail validation. Use the Python client or the REST `/web/step` endpoint (with a real JSON dict) instead of the Playground form for MCP envs. +### Optional Gradio web UI +- The debug web UI is off by default; enable with `ENABLE_WEB_INTERFACE=true` and open `/web/`. ### Docker - Docker is not required for local dev/test of the core library or pure-Python envs, and is not installed by the update script. Tests marked `@pytest.mark.docker`/`network` are skipped without it. Only set Docker up if you specifically work on container build/run flows. diff --git a/src/openenv/core/env_server/serialization.py b/src/openenv/core/env_server/serialization.py index 9d777884e9..36c7acbf11 100644 --- a/src/openenv/core/env_server/serialization.py +++ b/src/openenv/core/env_server/serialization.py @@ -8,6 +8,7 @@ HTTP server and web interface implementations. """ +import json from typing import Any, Dict, Type from .mcp_types import CallToolAction, ListToolsAction @@ -77,6 +78,7 @@ def deserialize_action_with_preprocessing( Convert JSON dict to Action instance with preprocessing for special types. This version handles common type conversions needed for web interfaces: + - Converting JSON string arguments to dict for MCP call_tool actions - Converting lists/strings to tensors for 'tokens' field - Converting string action_id to int - Other custom preprocessing as needed @@ -93,7 +95,17 @@ def deserialize_action_with_preprocessing( Raises: `ValidationError`: If `action_data` is invalid for the action class. """ - mcp_action = _deserialize_mcp_action(action_data, action_cls) + mcp_data = action_data + if action_data.get("type") == "call_tool" and isinstance( + action_data.get("arguments"), str + ): + mcp_data = dict(action_data) + try: + mcp_data["arguments"] = json.loads(action_data["arguments"]) + except Exception: + pass + + mcp_action = _deserialize_mcp_action(mcp_data, action_cls) if mcp_action is not None: return mcp_action @@ -105,8 +117,6 @@ def deserialize_action_with_preprocessing( if isinstance(value, str): # If it's a string, try to parse it as a list of numbers try: - import json - value = json.loads(value) except Exception: # If parsing fails, treat as empty list diff --git a/tests/core/test_mcp/test_mcp_types.py b/tests/core/test_mcp/test_mcp_types.py index 28a692b6f7..ce1421a248 100644 --- a/tests/core/test_mcp/test_mcp_types.py +++ b/tests/core/test_mcp/test_mcp_types.py @@ -225,6 +225,15 @@ def test_non_mcp_action_uses_action_cls(self): assert isinstance(action, _DummyEnvAction) assert action.value == "world" + def test_call_tool_with_string_arguments_is_rejected_without_preprocessing(self): + data = { + "type": "call_tool", + "tool_name": "echo", + "arguments": '{"message": "hello"}', + } + with pytest.raises(ValidationError): + deserialize_action(data, Action) + def test_invalid_non_mcp_action_raises(self): data = {"nonexistent_field": 123} with pytest.raises(ValidationError): @@ -246,18 +255,68 @@ def test_non_mcp_cls_with_list_tools_type_falls_through(self): class TestDeserializeWithPreprocessingMCPRouting: - """Same MCP routing works in the preprocessing variant.""" + """Same MCP routing works in the preprocessing variant, with string argument decoding.""" def test_list_tools_bypasses_preprocessing(self): data = {"type": "list_tools"} action = deserialize_action_with_preprocessing(data, Action) assert isinstance(action, ListToolsAction) - def test_call_tool_bypasses_preprocessing(self): - data = {"type": "call_tool", "tool_name": "solve", "arguments": {}} - action = deserialize_action_with_preprocessing(data, Action) + @pytest.mark.parametrize("action_cls", [Action, CallToolAction]) + def test_call_tool_with_dict_arguments(self, action_cls): + data = {"type": "call_tool", "tool_name": "solve", "arguments": {"x": 1}} + action = deserialize_action_with_preprocessing(data, action_cls) assert isinstance(action, CallToolAction) assert action.tool_name == "solve" + assert action.arguments == {"x": 1} + + @pytest.mark.parametrize("action_cls", [Action, CallToolAction]) + def test_call_tool_with_valid_json_string_arguments(self, action_cls): + data = { + "type": "call_tool", + "tool_name": "echo", + "arguments": '{"message": "hello"}', + } + action = deserialize_action_with_preprocessing(data, action_cls) + assert isinstance(action, CallToolAction) + assert action.tool_name == "echo" + assert action.arguments == {"message": "hello"} + + def test_call_tool_string_preprocessing_does_not_mutate_input(self): + data = { + "type": "call_tool", + "tool_name": "echo", + "arguments": '{"message": "hello"}', + } + deserialize_action_with_preprocessing(data, Action) + assert data["arguments"] == '{"message": "hello"}' + + def test_call_tool_with_malformed_json_string_arguments_is_rejected(self): + data = { + "type": "call_tool", + "tool_name": "echo", + "arguments": '{"message": "hello"', + } + with pytest.raises(ValidationError): + deserialize_action_with_preprocessing(data, Action) + + def test_call_tool_with_json_array_string_arguments_is_rejected(self): + data = { + "type": "call_tool", + "tool_name": "echo", + "arguments": '["hello"]', + } + with pytest.raises(ValidationError): + deserialize_action_with_preprocessing(data, Action) + + def test_call_tool_with_scalar_json_string_arguments_is_rejected(self): + data = { + "type": "call_tool", + "tool_name": "echo", + "arguments": '"hello"', + } + with pytest.raises(ValidationError): + deserialize_action_with_preprocessing(data, Action) def test_non_mcp_still_preprocessed(self): data = {"value": "test"} diff --git a/tests/core/test_web_interface_mcp.py b/tests/core/test_web_interface_mcp.py new file mode 100644 index 0000000000..72b98da3de --- /dev/null +++ b/tests/core/test_web_interface_mcp.py @@ -0,0 +1,78 @@ +# SPDX-License-Identifier: BSD-3-Clause + +"""Tests for MCP action handling in the OpenEnv Gradio web interface.""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient +from openenv.core.env_server.interfaces import Environment +from openenv.core.env_server.mcp_types import CallToolAction, CallToolObservation +from openenv.core.env_server.types import State +from openenv.core.env_server.web_interface import create_web_interface_app + +pytest.importorskip("gradio", reason="gradio is not installed") + + +class _FakeMCPState(State): + """Minimal state for exercising the web wrapper.""" + + step_count: int = 0 + + +class FakeMCPEnvironment(Environment): + """Minimal environment that accepts CallToolAction for web testing.""" + + def __init__(self): + super().__init__() + self._state = _FakeMCPState() + + def reset(self) -> CallToolObservation: + self._state = _FakeMCPState(step_count=0) + return CallToolObservation(tool_name="init", result="ready") + + def step(self, action: CallToolAction) -> CallToolObservation: + self._state.step_count += 1 + return CallToolObservation( + tool_name=action.tool_name, + result={ + "echoed_arguments": action.arguments, + "is_dict": isinstance(action.arguments, dict), + }, + ) + + @property + def state(self) -> _FakeMCPState: + return self._state + + def close(self) -> None: + pass + + +def test_web_step_call_tool_parses_json_string_arguments() -> None: + """POST /web/step should decode JSON string arguments for CallToolAction.""" + app = create_web_interface_app( + FakeMCPEnvironment, + CallToolAction, + CallToolObservation, + ) + client = TestClient(app) + + reset_response = client.post("/web/reset") + assert reset_response.status_code == 200 + + step_response = client.post( + "/web/step", + json={ + "action": { + "type": "call_tool", + "tool_name": "echo", + "arguments": '{"message": "hello"}', + } + }, + ) + assert step_response.status_code == 200 + step_json = step_response.json() + result = step_json["observation"]["result"] + assert result["echoed_arguments"] == {"message": "hello"} + assert result["is_dict"] is True