Merge pull request #2776 from yueguobin/fix/mcp-template-update-kwargs-handling

Fix MCP tool parameter handling for nested kwargs

.claude/memory/MEMORY.md

@@ -30,3 +30,4 @@
 
 ### MCP Service
 - **[MCP Service Design](./mcp-service-design.md)** - MCP (Model Context Protocol) service architecture using FastMCP with SSE transport, JWT auth, 29 tools across 5 domains
+- **[MCP Tool Description Location](./mcp-tool-description-guide.md)** - Where to define MCP tool descriptions: in `@mcp.tool()` functions in `__init__.py`, not in `*_TOOLS` arrays

.claude/memory/mcp-tool-description-guide.md

@@ -0,0 +1,43 @@
+---
+name: mcp-tool-description-location
+description: Where to define MCP tool descriptions so AI can see them
+metadata:
+  type: reference
+---
+
+# MCP Tool Description Location
+
+## Key Point
+MCP tool descriptions are defined in `@mcp.tool()` decorator functions in `__init__.py`, NOT in the `*_TOOLS` arrays in individual module files.
+
+## Correct Location
+**File**: `gns3server/api/routes/mcp/__init__.py`
+
+**Example**:
+```python
+@mcp.tool()
+async def update_link(
+    project_id: Annotated[str, Field(description="UUID of the project")],
+    link_id: Annotated[str, Field(description="UUID of the link to update")],
+    **kwargs: Any,
+) -> list[dict[str, Any]]:
+    """Update a link's properties.
+    
+    Put detailed descriptions here, especially for complex parameters.
+    Include format requirements, ranges, and examples.
+    """
+    # implementation
+```
+
+## Wrong Location
+- ❌ `LINK_TOOLS` in `gns3server/api/routes/mcp/links.py`
+- ❌ `TEMPLATE_TOOLS` in `gns3server/api/routes/mcp/templates.py`
+
+## Activation
+**Must restart GNS3 server** for description updates to take effect.
+
+## Description Requirements
+- Be explicit about data formats (arrays vs single values)
+- Include parameter ranges and constraints
+- Provide usage examples
+- Prevent common errors in the description itself

gns3server/api/routes/mcp/__init__.py

@@ -391,9 +391,19 @@ async def create_link(
     project_id: Annotated[str, Field(description="UUID of the project")],
     nodes: Annotated[list, Field(description="List of node connections, e.g. [{\"node_id\": \"...\", \"adapter_number\": 0, \"port_number\": 0}]")],
     link_type: Annotated[str, Field(description="Link type - ethernet or serial")] = "ethernet",
-    filters: Annotated[dict, Field(description="Optional packet filters")] = None,
+    filters: Annotated[dict, Field(description="Optional packet filters (must use array format): frequency_drop: [N], packet_loss: [rate], delay: [ms, jitter], corrupt: [rate], bpf: [expression]")] = None,
 ) -> list[dict[str, Any]]:
-    """Create a link between two nodes in a project."""
+    """Create a link between two nodes in a project.
+
+    Filters must use array format:
+    - frequency_drop: [N] - Drop every Nth packet (N: -1 to 32767)
+    - packet_loss: [rate] - Packet loss percentage (rate: 0 to 100)
+    - delay: [ms, jitter] - Latency and jitter in milliseconds
+    - corrupt: [rate] - Packet corruption percentage (rate: 0 to 100)
+    - bpf: [expression] - Berkeley Packet Filter expression
+
+    Example: {"filters": {"delay": [100, 10], "packet_loss": [5]}}
+    """
     params = {"project_id": project_id, "nodes": nodes, "link_type": link_type}
     if filters:
         params["filters"] = filters
@@ -415,7 +425,23 @@ async def update_link(
     link_id: Annotated[str, Field(description="UUID of the link to update")],
     **kwargs: Any,
 ) -> list[dict[str, Any]]:
-    """Update a link's properties (suspend, filters, etc.)."""
+    """Update a link's properties (suspend, filters, etc.).
+
+    Supported kwargs:
+    - suspend: boolean - Suspend or resume the link
+    - filters: dict - Packet filters (must use array format):
+      * frequency_drop: [N] - Drop every Nth packet (N: -1 to 32767)
+      * packet_loss: [rate] - Packet loss percentage (rate: 0 to 100)
+      * delay: [ms, jitter] - Latency and jitter in milliseconds
+      * corrupt: [rate] - Packet corruption percentage (rate: 0 to 100)
+      * bpf: [expression] - Berkeley Packet Filter expression
+
+    Example filters:
+      {"filters": {"frequency_drop": [10]}}
+      {"filters": {"delay": [100, 10]}}
+      {"filters": {"packet_loss": [5]}}
+      {"filters": {"delay": [50, 5], "packet_loss": [2]}}
+    """
     params = {"project_id": project_id, "link_id": link_id, **kwargs}
     return await asyncio.to_thread(_run_handler_sync, update_link_handler, params)
 

gns3server/api/routes/mcp/links.py

@@ -94,7 +94,13 @@ def update_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
     if not project_id or not link_id:
         return {"error": "project_id and link_id are required"}
     conn = _get_connector(gns3_ctx)
-    update_data = {k: v for k, v in params.items() if k not in ("project_id", "link_id")}
+
+    # Extract update parameters - handle nested kwargs structure from MCP clients
+    if "kwargs" in params and isinstance(params["kwargs"], dict):
+        update_data = params["kwargs"]
+    else:
+        update_data = {k: v for k, v in params.items() if k not in ("project_id", "link_id", "kwargs")}
+
     url = f"{conn.base_url}/projects/{project_id}/links/{link_id}"
     return conn.http_call("put", url, json_data=update_data).json()
 
@@ -147,7 +153,10 @@ LINK_TOOLS = [
                     },
                 },
                 "link_type": {"type": "string", "description": "Link type: ethernet or serial (optional)"},
-                "filters": {"type": "object", "description": "Packet filters (optional)"},
+                "filters": {
+                    "type": "object",
+                    "description": "Packet filters (optional). Must use array format: frequency_drop: [N], packet_loss: [rate], delay: [ms, jitter], corrupt: [rate], bpf: [expression]"
+                },
             },
             "required": ["project_id", "nodes"],
         },
@@ -175,7 +184,10 @@ LINK_TOOLS = [
                 "project_id": {"type": "string", "description": "Project UUID"},
                 "link_id": {"type": "string", "description": "Link UUID"},
                 "suspend": {"type": "boolean", "description": "Suspend the link (optional)"},
-                "filters": {"type": "object", "description": "Packet filters (optional)"},
+                "filters": {
+                    "type": "object",
+                    "description": "Packet filters (optional). Must use array format: frequency_drop: [N], packet_loss: [rate], delay: [ms, jitter], corrupt: [rate], bpf: [expression]. Example: {\"frequency_drop\": [10], \"packet_loss\": [5]}"
+                },
             },
             "required": ["project_id", "link_id"],
         },

gns3server/api/routes/mcp/nodes.py

@@ -132,7 +132,13 @@ def update_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
     if not project_id or not node_id:
         return {"error": "project_id and node_id are required"}
     conn = _get_connector(gns3_ctx)
-    update_data = {k: v for k, v in params.items() if k not in ("project_id", "node_id")}
+
+    # Extract update parameters - handle nested kwargs structure from MCP clients
+    if "kwargs" in params and isinstance(params["kwargs"], dict):
+        update_data = params["kwargs"]
+    else:
+        update_data = {k: v for k, v in params.items() if k not in ("project_id", "node_id", "kwargs")}
+
     url = f"{conn.base_url}/projects/{project_id}/nodes/{node_id}"
     return conn.http_call("put", url, json_data=update_data).json()
 

gns3server/api/routes/mcp/templates.py

@@ -52,12 +52,16 @@ def list_templates_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) ->
 def get_template_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict[str, Any]:
     template_id = params.get("template_id")
     name = params.get("name")
+
     if not template_id and not name:
         return {"error": "template_id or name is required"}
+
     conn = _get_connector(gns3_ctx)
     template = conn.get_template(name=name, template_id=template_id)
+
     if template is None:
         return {"error": "Template not found"}
+
     return template
 
 
@@ -66,26 +70,43 @@ def create_template_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) ->
     template_type = params.get("template_type")
     if not name or not template_type:
         return {"error": "name and template_type are required"}
+
     conn = _get_connector(gns3_ctx)
-    return conn.create_template(**params)
+
+    # Handle nested kwargs structure from MCP clients
+    if "kwargs" in params and isinstance(params["kwargs"], dict):
+        create_params = params["kwargs"]
+    else:
+        create_params = params
+
+    return conn.create_template(**create_params)
 
 
 def update_template_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict[str, Any]:
     template_id = params.get("template_id")
     name = params.get("name")
+
     if not template_id and not name:
         return {"error": "template_id or name is required"}
+
     conn = _get_connector(gns3_ctx)
-    return conn.update_template(name=name, template_id=template_id, **{
-        k: v for k, v in params.items() if k not in ("template_id", "name")
-    })
+
+    # Extract update parameters - handle nested kwargs structure from MCP clients
+    if "kwargs" in params and isinstance(params["kwargs"], dict):
+        update_params = params["kwargs"]
+    else:
+        update_params = {k: v for k, v in params.items() if k not in ("template_id", "name", "kwargs")}
+
+    return conn.update_template(name=name, template_id=template_id, **update_params)
 
 
 def delete_template_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict[str, Any]:
     template_id = params.get("template_id")
     name = params.get("name")
+
     if not template_id and not name:
         return {"error": "template_id or name is required"}
+
     conn = _get_connector(gns3_ctx)
     conn.delete_template(name=name, template_id=template_id)
     return {"message": f"Template deleted"}