diff --git a/.claude/memory/MEMORY.md b/.claude/memory/MEMORY.md index e02b12fbc..59456d279 100644 --- a/.claude/memory/MEMORY.md +++ b/.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 diff --git a/.claude/memory/mcp-tool-description-guide.md b/.claude/memory/mcp-tool-description-guide.md new file mode 100644 index 000000000..ca4ebcf42 --- /dev/null +++ b/.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 diff --git a/gns3server/api/routes/mcp/__init__.py b/gns3server/api/routes/mcp/__init__.py index ce77a48c7..39d19a501 100644 --- a/gns3server/api/routes/mcp/__init__.py +++ b/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) diff --git a/gns3server/api/routes/mcp/links.py b/gns3server/api/routes/mcp/links.py index 2b9278ba0..1d5a87da6 100644 --- a/gns3server/api/routes/mcp/links.py +++ b/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"], }, diff --git a/gns3server/api/routes/mcp/nodes.py b/gns3server/api/routes/mcp/nodes.py index 7fef57021..7a16a59cd 100644 --- a/gns3server/api/routes/mcp/nodes.py +++ b/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() diff --git a/gns3server/api/routes/mcp/templates.py b/gns3server/api/routes/mcp/templates.py index b5df86faf..e2cc71e76 100644 --- a/gns3server/api/routes/mcp/templates.py +++ b/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"}