diff --git a/poetry.lock b/poetry.lock index 37704c8..754413a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1244,13 +1244,13 @@ httpx = ">=0.25.0" [[package]] name = "ros2-medkit-client" -version = "0.6.0" +version = "0.7.0" description = "Async Python client for the ros2_medkit gateway" optional = false python-versions = ">=3.11" groups = ["main"] files = [ - {file = "ros2_medkit_client-0.6.0-py3-none-any.whl", hash = "sha256:36806724b570105332563f26005049d42252004267d27b1a1ca648c8eba7c203"}, + {file = "ros2_medkit_client-0.7.0-py3-none-any.whl", hash = "sha256:9c118ff83dbd4715088f715e2d5a0ebabf77203507b5400f5e1a130baf9c0ade"}, ] [package.dependencies] @@ -1263,7 +1263,7 @@ dev = ["pytest (>=8.0)", "pytest-asyncio (>=0.24)", "respx (>=0.22)", "ruff (>=0 [package.source] type = "url" -url = "https://github.com/selfpatch/ros2_medkit_clients/releases/download/py-v0.6.0/ros2_medkit_client-0.6.0-py3-none-any.whl" +url = "https://github.com/selfpatch/ros2_medkit_clients/releases/download/py-v0.7.0/ros2_medkit_client-0.7.0-py3-none-any.whl" [[package]] name = "rpds-py" @@ -1812,4 +1812,4 @@ files = [ [metadata] lock-version = "2.1" python-versions = "^3.11" -content-hash = "0a033fc8257acca535ed44168a80a7f2b842b0c4482d98b10f9c33d9d56b5db2" +content-hash = "a110aa558cf7d7aa7dc571d0e49775929bf70aa0b4133a9bfc7554f073972690" diff --git a/pyproject.toml b/pyproject.toml index 32ab2db..266bef0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "ros2-medkit-mcp" -version = "0.6.0" +version = "0.7.0" description = "MCP server adapter for ros2_medkit SOVD HTTP API" authors = ["bburda "] readme = "README.md" @@ -15,7 +15,7 @@ pydantic = "^2.10.0" uvicorn = { version = "^0.34.0", extras = ["standard"] } starlette = "^0.45.0" # Distributed via GitHub Releases wheel (no PyPI yet). Replace with version constraint when available. -ros2-medkit-client = {url = "https://github.com/selfpatch/ros2_medkit_clients/releases/download/py-v0.6.0/ros2_medkit_client-0.6.0-py3-none-any.whl"} +ros2-medkit-client = {url = "https://github.com/selfpatch/ros2_medkit_clients/releases/download/py-v0.7.0/ros2_medkit_client-0.7.0-py3-none-any.whl"} [tool.poetry.group.dev.dependencies] pytest = "^8.3.0" diff --git a/src/ros2_medkit_mcp/__init__.py b/src/ros2_medkit_mcp/__init__.py index 548d6dd..8b1a952 100644 --- a/src/ros2_medkit_mcp/__init__.py +++ b/src/ros2_medkit_mcp/__init__.py @@ -1,3 +1,3 @@ """ros2_medkit_mcp - MCP adapter for ros2_medkit SOVD HTTP API.""" -__version__ = "0.6.0" +__version__ = "0.7.0" diff --git a/src/ros2_medkit_mcp/client.py b/src/ros2_medkit_mcp/client.py index 5365316..45600b9 100644 --- a/src/ros2_medkit_mcp/client.py +++ b/src/ros2_medkit_mcp/client.py @@ -477,28 +477,28 @@ def _validate_relative_uri(uri: str) -> None: # force-shutdown); the generated modules use underscores. "lifecycle": { "get": { - "components": lifecycle.get_components_status, - "apps": lifecycle.get_apps_status, + "components": lifecycle.get_component_status, + "apps": lifecycle.get_app_status, }, "start": { - "components": lifecycle.put_components_status_start, - "apps": lifecycle.put_apps_status_start, + "components": lifecycle.put_component_status_start, + "apps": lifecycle.put_app_status_start, }, "restart": { - "components": lifecycle.put_components_status_restart, - "apps": lifecycle.put_apps_status_restart, + "components": lifecycle.put_component_status_restart, + "apps": lifecycle.put_app_status_restart, }, "force-restart": { - "components": lifecycle.put_components_status_force_restart, - "apps": lifecycle.put_apps_status_force_restart, + "components": lifecycle.put_component_status_force_restart, + "apps": lifecycle.put_app_status_force_restart, }, "shutdown": { - "components": lifecycle.put_components_status_shutdown, - "apps": lifecycle.put_apps_status_shutdown, + "components": lifecycle.put_component_status_shutdown, + "apps": lifecycle.put_app_status_shutdown, }, "force-shutdown": { - "components": lifecycle.put_components_status_force_shutdown, - "apps": lifecycle.put_apps_status_force_shutdown, + "components": lifecycle.put_component_status_force_shutdown, + "apps": lifecycle.put_app_status_force_shutdown, }, }, } @@ -1269,12 +1269,25 @@ async def execute_script( script_id: str, params: dict[str, Any] | None = None, entity_type: str = "components", + execution_type: str = "now", ) -> dict[str, Any]: + # The request body is ScriptExecutionRequest: `execution_type` says when to + # run and the script's own inputs live under `parameters`. The shipped + # backend accepts only `now`, which is why it defaults, but a ScriptProvider + # plugin defines its own vocabulary and answers 400 for a value it does not + # know, so the caller can name one. + # + # `is not None` rather than a truth test: `parameters` is forwarded to the + # provider untouched, so an explicitly empty object is a different request + # from an absent one and only the caller knows which they meant. + body: dict[str, Any] = {"execution_type": execution_type} + if params is not None: + body["parameters"] = params fn = _entity_func("scripts", "execute", entity_type) kwargs: dict[str, Any] = { _entity_id_kwarg(entity_type): entity_id, "script_id": script_id, - "body": params if params else {}, + "body": body, } return await self._call(fn, **kwargs) diff --git a/src/ros2_medkit_mcp/mcp_app.py b/src/ros2_medkit_mcp/mcp_app.py index c1cd7e6..9dfcd48 100644 --- a/src/ros2_medkit_mcp/mcp_app.py +++ b/src/ros2_medkit_mcp/mcp_app.py @@ -2071,7 +2071,11 @@ async def list_tools() -> list[Tool]: }, "params": { "type": "object", - "description": "Optional parameters to pass to the script execution", + "description": ( + "Optional parameters for the script, forwarded to the provider" + " untouched. The accepted shape is the script's own, from" + " parameters_schema on the script." + ), }, "entity_type": { "type": "string", @@ -2079,6 +2083,15 @@ async def list_tools() -> list[Tool]: "description": "Entity type", "default": "components", }, + "execution_type": { + "type": "string", + "description": ( + "When to run. The built-in script backend accepts only 'now'; a" + " provider plugin defines its own vocabulary and answers 400 for a" + " value it does not support." + ), + "default": "now", + }, }, "required": ["entity_id", "script_id"], }, @@ -2485,8 +2498,10 @@ async def list_tools() -> list[Tool]: "update_config": { "type": "object", "description": ( - "Update package configuration" - " (e.g., {'name': 'firmware-v2', 'version': '2.0.0'," + "Update package configuration. 'id' is required and becomes" + " the update's path segment; every other key is forwarded to" + " the update backend unchanged" + " (e.g., {'id': 'firmware-v2', 'version': '2.0.0'," " 'uri': 'https://...'})" ), }, @@ -3076,7 +3091,11 @@ async def call_tool(name: str, arguments: dict[str, Any]) -> list[TextContent]: elif normalized_name == "ros2_medkit_execute_script": args = ExecuteScriptArgs(**arguments) result = await client.execute_script( - args.entity_id, args.script_id, args.params, args.entity_type + args.entity_id, + args.script_id, + args.params, + args.entity_type, + args.execution_type, ) return format_json_response(result) diff --git a/src/ros2_medkit_mcp/models.py b/src/ros2_medkit_mcp/models.py index 67d5530..7a2b81a 100644 --- a/src/ros2_medkit_mcp/models.py +++ b/src/ros2_medkit_mcp/models.py @@ -1009,12 +1009,22 @@ class ExecuteScriptArgs(BaseModel): script_id: str = Field(..., description="The script identifier") params: dict[str, Any] | None = Field( default=None, - description="Optional parameters to pass to the script execution", + description=( + "Optional parameters for the script, forwarded to the provider untouched. " + "The accepted shape is the script's own, from parameters_schema on the script." + ), ) entity_type: str = Field( default="components", description="Entity type: 'components' or 'apps'", ) + execution_type: str = Field( + default="now", + description=( + "When to run. The built-in script backend accepts only 'now'; a provider " + "plugin defines its own vocabulary and answers 400 for a value it does not support." + ), + ) class GetScriptExecutionArgs(BaseModel): diff --git a/tests/test_new_tools.py b/tests/test_new_tools.py index d30f5dd..e0b5eb5 100644 --- a/tests/test_new_tools.py +++ b/tests/test_new_tools.py @@ -1,5 +1,7 @@ """Tests for new MCP tools (v0.2.0-v0.4.0 features).""" +import json + import httpx import pytest import respx @@ -235,6 +237,77 @@ async def test_execute_script(self, client: SovdClient) -> None: assert result["status"] == "running" await client.close() + @respx.mock + async def test_execute_script_sends_the_script_inputs_under_parameters( + self, client: SovdClient + ) -> None: + # The gateway reads the script's own inputs from `parameters` and refuses a + # body without `execution_type`, so what the wire carries is the whole point + # here - a 202 alone would pass on either shape. + route = respx.post( + "http://test-sovd:8080/api/v1/components/motor/scripts/s1/executions" + ).mock(return_value=httpx.Response(202, json=self.SCRIPT_EXECUTION)) + await client.execute_script("motor", "s1", {"iterations": 3}) + assert json.loads(route.calls.last.request.content) == { + "execution_type": "now", + "parameters": {"iterations": 3}, + } + await client.close() + + @respx.mock + async def test_execute_script_forwards_a_provider_execution_type( + self, client: SovdClient + ) -> None: + # A ScriptProvider plugin defines its own vocabulary, so the caller has to be + # able to name a value other than the built-in backend's only one. + route = respx.post("http://test-sovd:8080/api/v1/apps/talker/scripts/s1/executions").mock( + return_value=httpx.Response(202, json=self.SCRIPT_EXECUTION) + ) + await client.execute_script( + "talker", "s1", None, entity_type="apps", execution_type="scheduled" + ) + assert json.loads(route.calls.last.request.content) == {"execution_type": "scheduled"} + await client.close() + + @respx.mock + async def test_execute_script_forwards_an_execution_type_alongside_parameters( + self, client: SovdClient + ) -> None: + # The two fields are independent, and a body built as though they were - the + # caller's type honoured only when no parameters came with it - satisfies + # every other case here. + route = respx.post("http://test-sovd:8080/api/v1/apps/talker/scripts/s1/executions").mock( + return_value=httpx.Response(202, json=self.SCRIPT_EXECUTION) + ) + await client.execute_script( + "talker", + "s1", + {"iterations": 3}, + entity_type="apps", + execution_type="scheduled", + ) + assert json.loads(route.calls.last.request.content) == { + "execution_type": "scheduled", + "parameters": {"iterations": 3}, + } + await client.close() + + @respx.mock + async def test_execute_script_keeps_an_explicitly_empty_parameters_object( + self, client: SovdClient + ) -> None: + # `parameters` reaches the provider untouched, so an empty object is a + # different request from an absent one and only the caller knows which. + route = respx.post( + "http://test-sovd:8080/api/v1/components/motor/scripts/s1/executions" + ).mock(return_value=httpx.Response(202, json=self.SCRIPT_EXECUTION)) + await client.execute_script("motor", "s1", {}) + assert json.loads(route.calls.last.request.content) == { + "execution_type": "now", + "parameters": {}, + } + await client.close() + @respx.mock async def test_get_script_execution(self, client: SovdClient) -> None: respx.get( @@ -450,7 +523,7 @@ async def test_register_update(self, client: SovdClient) -> None: return_value=httpx.Response(201, json=self.UPDATE_RESPONSE) ) result = await client.register_update( - {"name": "firmware-v2", "version": "2.0.0", "uri": "https://example.com/fw.bin"} + {"id": "upd-1", "version": "2.0.0", "uri": "https://example.com/fw.bin"} ) assert result["id"] == "upd-1" assert result["status"] == "pending" @@ -571,7 +644,7 @@ async def test_set_status_routes_every_action( self, client: SovdClient, entity_type: str, entity_id: str, action: str ) -> None: # The action segment stays hyphenated on the wire even though the generated - # module name uses an underscore (put_apps_status_force_restart). + # module name uses an underscore (put_app_status_force_restart). route = respx.put( f"http://test-sovd:8080/api/v1/{entity_type}/{entity_id}/status/{action}" ).mock(return_value=httpx.Response(202)) @@ -688,49 +761,94 @@ async def test_get_status_renders_as_json(self, client: SovdClient) -> None: class TestDataDiscoveryTools: - """Tests for data discovery tools (categories and groups).""" + """Tests for data discovery tools (categories and groups). + + The gateway answers 501 on both resources for every entity type - the ROS 2 + data provider does not group or categorise its topics - so 501 is the only + status these tools see in production. A ScriptProvider-style plugin backend + may implement them, which is why the tools stay in the set; until one does, + what matters is that the gateway's reason reaches the caller intact rather + than being flattened into an unexpected-status. + """ + + NOT_IMPLEMENTED = { + "error_code": "not-implemented", + "message": "Data categories are not implemented for ROS 2", + "parameters": {"feature": "data-categories"}, + } @respx.mock - async def test_list_data_categories(self, client: SovdClient) -> None: + async def test_list_data_categories_reports_not_implemented(self, client: SovdClient) -> None: respx.get("http://test-sovd:8080/api/v1/components/motor/data-categories").mock( - return_value=httpx.Response( - 200, - json={"items": ["topics", "parameters"]}, - ) + return_value=httpx.Response(501, json=self.NOT_IMPLEMENTED) ) - result = await client.list_data_categories("motor") - assert result == ["topics", "parameters"] + with pytest.raises(SovdClientError) as excinfo: + await client.list_data_categories("motor") + assert "not-implemented" in str(excinfo.value) + assert "not implemented for ROS 2" in str(excinfo.value) await client.close() @respx.mock - async def test_list_data_categories_apps(self, client: SovdClient) -> None: + async def test_list_data_categories_apps_reports_not_implemented( + self, client: SovdClient + ) -> None: respx.get("http://test-sovd:8080/api/v1/apps/my_node/data-categories").mock( - return_value=httpx.Response(200, json={"items": ["topics"]}) + return_value=httpx.Response(501, json=self.NOT_IMPLEMENTED) ) - result = await client.list_data_categories("my_node", "apps") - assert result == ["topics"] + with pytest.raises(SovdClientError) as excinfo: + await client.list_data_categories("my_node", "apps") + assert "not-implemented" in str(excinfo.value) await client.close() @respx.mock - async def test_list_data_groups(self, client: SovdClient) -> None: + async def test_list_data_groups_reports_not_implemented(self, client: SovdClient) -> None: respx.get("http://test-sovd:8080/api/v1/components/motor/data-groups").mock( return_value=httpx.Response( - 200, - json={"items": [{"id": "sensor_data", "name": "Sensor Data"}]}, + 501, + json={ + "error_code": "not-implemented", + "message": "Data groups are not implemented for ROS 2", + "parameters": {"feature": "data-groups"}, + }, ) ) - result = await client.list_data_groups("motor") - assert len(result) == 1 - assert result[0]["id"] == "sensor_data" + with pytest.raises(SovdClientError) as excinfo: + await client.list_data_groups("motor") + assert "Data groups are not implemented" in str(excinfo.value) await client.close() @respx.mock - async def test_list_data_groups_apps(self, client: SovdClient) -> None: + async def test_list_data_groups_apps_reports_not_implemented(self, client: SovdClient) -> None: respx.get("http://test-sovd:8080/api/v1/apps/my_node/data-groups").mock( - return_value=httpx.Response(200, json={"items": []}) + return_value=httpx.Response( + 501, + json={ + "error_code": "not-implemented", + "message": "Data groups are not implemented for ROS 2", + "parameters": {"feature": "data-groups"}, + }, + ) ) - result = await client.list_data_groups("my_node", "apps") - assert result == [] + with pytest.raises(SovdClientError) as excinfo: + await client.list_data_groups("my_node", "apps") + assert "not-implemented" in str(excinfo.value) + await client.close() + + @respx.mock + async def test_list_data_categories_cannot_yet_read_a_backend_that_answers_200( + self, client: SovdClient + ) -> None: + # The route documents no 200, so the generated parser hands back no model and + # the wrapper reports unexpected-status. A provider plugin that implements the + # resource would hit exactly this, which is a gateway schema gap rather than a + # client one. Pinned so that adding a 200 to the schema fails here and someone + # revisits it, instead of the tool silently changing what it returns. + respx.get("http://test-sovd:8080/api/v1/components/motor/data-categories").mock( + return_value=httpx.Response(200, json={"items": ["topics", "parameters"]}) + ) + with pytest.raises(SovdClientError) as excinfo: + await client.list_data_categories("motor") + assert "unexpected-status" in str(excinfo.value) await client.close() @@ -918,14 +1036,18 @@ async def test_updates_dispatch_smoke(self, client: SovdClient) -> None: @respx.mock async def test_data_discovery_dispatch_smoke(self, client: SovdClient) -> None: + # Dispatch reaches the right route and carries the gateway's own reason out; + # the ROS 2 backend implements neither resource, so 501 is what it answers. respx.get("http://test-sovd:8080/api/v1/components/motor/data-categories").mock( return_value=httpx.Response( - 200, - json={"items": ["topics", "parameters"]}, + 501, + json={ + "error_code": "not-implemented", + "message": "Data categories are not implemented for ROS 2", + }, ) ) - result = await client.list_data_categories("motor") - formatted = format_json_response(result) - assert "topics" in formatted[0].text - assert "parameters" in formatted[0].text + with pytest.raises(SovdClientError) as excinfo: + await client.list_data_categories("motor") + assert "Data categories are not implemented" in str(excinfo.value) await client.close()