fix: address review feedback on plugin system

- Fix ruff lint errors (import order, unused import, type annotation)
- Build tool→plugin map at registration time for O(1) dispatch
- Make plugin dispatch authoritative (first declaring plugin owns it)
- Track successfully started plugins; only shutdown those on exit

Author: bburda

src/ros2_medkit_mcp/mcp_app.py

@@ -14,7 +14,6 @@
 
 from ros2_medkit_mcp.client import SovdClient, SovdClientError
 from ros2_medkit_mcp.config import Settings
-from ros2_medkit_mcp.plugin import McpPlugin
 from ros2_medkit_mcp.models import (
     AppIdArgs,
     AreaComponentsArgs,
@@ -59,6 +58,7 @@
     UpdateExecutionArgs,
     filter_entities,
 )
+from ros2_medkit_mcp.plugin import McpPlugin
 
 logger = logging.getLogger(__name__)
 
@@ -631,14 +631,18 @@ async def download_rosbags_for_fault(
 }
 
 
-def register_tools(server: Server, client: SovdClient, plugins: list[McpPlugin] | None = None) -> None:
+def register_tools(
+    server: Server, client: SovdClient, plugins: list[McpPlugin] | None = None
+) -> None:
     """Register all MCP tools on the server.
 
     Args:
         server: The MCP server to register tools on.
         client: The SOVD client for making API calls.
         plugins: Optional list of plugins providing additional tools.
     """
+    # Tool name → plugin mapping, built during list_tools and used for dispatch
+    plugin_tool_map: dict[str, McpPlugin] = {}
 
     @server.list_tools()
     async def list_tools() -> list[Tool]:
@@ -1497,7 +1501,10 @@ async def list_tools() -> list[Tool]:
         if plugins:
             for plugin in plugins:
                 try:
-                    tools.extend(plugin.list_tools())
+                    plugin_tools = plugin.list_tools()
+                    tools.extend(plugin_tools)
+                    for t in plugin_tools:
+                        plugin_tool_map[t.name] = plugin
                 except Exception:
                     logger.exception("Failed to list tools from plugin: %s", plugin.name)
         return tools
@@ -1804,15 +1811,10 @@ async def call_tool(name: str, arguments: dict[str, Any]) -> list[TextContent]:
                 )
 
             else:
-                # Try plugins before reporting unknown tool
-                if plugins:
-                    for plugin in plugins:
-                        try:
-                            plugin_tool_names = {t.name for t in plugin.list_tools()}
-                            if normalized_name in plugin_tool_names:
-                                return await plugin.call_tool(normalized_name, arguments)
-                        except Exception:
-                            logger.exception("Plugin %s failed to handle tool %s", plugin.name, normalized_name)
+                # Check plugin tool map before reporting unknown tool
+                plugin = plugin_tool_map.get(normalized_name)
+                if plugin is not None:
+                    return await plugin.call_tool(normalized_name, arguments)
                 return format_error(f"Unknown tool: {name}")
 
         except SovdClientError as e:
@@ -1868,7 +1870,9 @@ async def read_resource(uri: str) -> list[TextContent]:
         raise ValueError(f"Unknown resource URI: {uri}")
 
 
-def setup_mcp_app(server: Server, settings: Settings, client: SovdClient, plugins: list[McpPlugin] | None = None) -> None:
+def setup_mcp_app(
+    server: Server, settings: Settings, client: SovdClient, plugins: list[McpPlugin] | None = None
+) -> None:
     """Set up the complete MCP application.
 
     Args:

src/ros2_medkit_mcp/server_http.py

@@ -79,6 +79,8 @@ async def health_check(_request: Request) -> JSONResponse:
             }
         )
 
+    started_plugins: list = []
+
     async def on_startup() -> None:
         """Application startup handler."""
         logger.info("ros2_medkit MCP server starting (HTTP transport)")
@@ -87,14 +89,15 @@ async def on_startup() -> None:
         for plugin in plugins:
             try:
                 await plugin.startup()
+                started_plugins.append(plugin)
                 logger.info("Plugin started: %s", plugin.name)
             except Exception:
                 logger.exception("Failed to start plugin: %s", plugin.name)
 
     async def on_shutdown() -> None:
         """Application shutdown handler."""
-        # Shutdown plugins
-        for plugin in plugins:
+        # Only shutdown plugins that started successfully
+        for plugin in started_plugins:
             try:
                 await plugin.shutdown()
             except Exception:

src/ros2_medkit_mcp/server_stdio.py

@@ -34,11 +34,13 @@ async def run_server() -> None:
     client = SovdClient(settings)
     plugins = discover_plugins()
 
+    started_plugins = []
     try:
         # Start plugins
         for plugin in plugins:
             try:
                 await plugin.startup()
+                started_plugins.append(plugin)
                 logger.info("Plugin started: %s", plugin.name)
             except Exception:
                 logger.exception("Failed to start plugin: %s", plugin.name)
@@ -52,8 +54,8 @@ async def run_server() -> None:
                 server.create_initialization_options(),
             )
     finally:
-        # Shutdown plugins
-        for plugin in plugins:
+        # Only shutdown plugins that started successfully
+        for plugin in started_plugins:
             try:
                 await plugin.shutdown()
             except Exception:

tests/test_plugin_discovery.py

@@ -1,8 +1,8 @@
 """Tests for MCP plugin discovery and integration."""
 
+from typing import Any
 from unittest.mock import MagicMock, patch
 
-import pytest
 from mcp.types import TextContent, Tool
 
 from ros2_medkit_mcp.plugin import discover_plugins
@@ -14,9 +14,15 @@ def name(self) -> str:
         return "fake"
 
     def list_tools(self) -> list[Tool]:
-        return [Tool(name="fake_tool", description="A fake tool", inputSchema={"type": "object", "properties": {}})]
-
-    async def call_tool(self, name: str, arguments: dict) -> list[TextContent]:
+        return [
+            Tool(
+                name="fake_tool",
+                description="A fake tool",
+                inputSchema={"type": "object", "properties": {}},
+            )
+        ]
+
+    async def call_tool(self, name: str, _arguments: dict[str, Any]) -> list[TextContent]:
         if name == "fake_tool":
             return [TextContent(type="text", text="fake result")]
         raise ValueError(f"Unknown tool: {name}")