diff --git a/gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py b/gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py index 41ed58f9d..285e221dc 100644 --- a/gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py +++ b/gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py @@ -227,7 +227,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): ) except ValueError as e: logger.error("Failed to prepare device hosts data: %s", e) - return [{"error": str(e)}] + return [{"status": "failed", "error": str(e)}] # Check if any devices have errors (e.g., missing device_type tag) error_devices = { @@ -254,7 +254,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): dynamic_nr = self._initialize_nornir(hosts_data) except ValueError as e: logger.error("Failed to initialize Nornir: %s", e) - return [{"error": str(e)}] + return [{"status": "failed", "error": str(e)}] results = [] @@ -278,7 +278,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): logger.error( "Error executing configurations on all devices: %s", e ) - return [{"error": f"Execution error: {str(e)}"}] + return [{"status": "failed", "error": f"Execution error: {str(e)}"}] logger.info( "Multiple device configuration execution completed. Results: %s", @@ -368,7 +368,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): "Invalid JSON string received as tool input: %s", e ) return ( - [{"error": f"Invalid JSON string input from model: {e}"}], + [{"status": "failed", "error": f"Invalid JSON string input from model: {e}"}], None, ) else: @@ -389,7 +389,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): if not project_id: error_msg = "Missing required 'project_id' field in input" logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) if not self._validate_project_id(project_id): error_msg = ( @@ -397,13 +397,13 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): "Expected UUID format." ) logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) # Validate device_configs if not isinstance(device_configs, list): error_msg = "'device_configs' must be an array" logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) if not device_configs: logger.warning("Device configs list is empty.") @@ -426,7 +426,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool): f"{type(parsed_input).__name__}" ) logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) def _validate_project_id(self, project_id: str) -> bool: """ diff --git a/gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py b/gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py index 8fa056650..7c4f45622 100644 --- a/gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py +++ b/gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py @@ -223,7 +223,7 @@ class ExecuteMultipleDeviceCommands(BaseTool): ) except ValueError as e: logger.error("Failed to prepare device hosts data: %s", e) - return [{"error": str(e)}] + return [{"status": "failed", "error": str(e)}] # Check if any devices have errors (e.g., missing device_type tag) error_devices = { @@ -250,7 +250,7 @@ class ExecuteMultipleDeviceCommands(BaseTool): dynamic_nr = self._initialize_nornir(hosts_data) except ValueError as e: logger.error("Failed to initialize Nornir: %s", e) - return [{"error": str(e)}] + return [{"status": "failed", "error": str(e)}] results = [] @@ -272,7 +272,7 @@ class ExecuteMultipleDeviceCommands(BaseTool): except Exception as e: # Overall execution failed logger.error("Error executing display on all devices: %s", e) - return [{"error": f"Execution error: {str(e)}"}] + return [{"status": "failed", "error": f"Execution error: {str(e)}"}] logger.debug( "Multiple device display execution completed. Results: %s", @@ -370,7 +370,7 @@ class ExecuteMultipleDeviceCommands(BaseTool): "Invalid JSON string received as tool input: %s", e ) return ( - [{"error": f"Invalid JSON string input from model: {e}"}], + [{"status": "failed", "error": f"Invalid JSON string input from model: {e}"}], None, ) else: @@ -391,18 +391,18 @@ class ExecuteMultipleDeviceCommands(BaseTool): if not project_id: error_msg = "Missing required 'project_id' field in input" logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) if not self._validate_project_id(project_id): error_msg = f"Invalid project_id: {project_id}. Expected UUID." logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) # Validate device_configs if not isinstance(device_configs, list): error_msg = "'device_configs' must be an array" logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) if not device_configs: logger.warning("Device configs list is empty.") @@ -423,7 +423,7 @@ class ExecuteMultipleDeviceCommands(BaseTool): f"or legacy JSON array, got {type(parsed_input).__name__}" ) logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) def _validate_project_id(self, project_id: str) -> bool: """ diff --git a/gns3server/agent/gns3_copilot/tools_v2/vpcs_tools_netmiko.py b/gns3server/agent/gns3_copilot/tools_v2/vpcs_tools_netmiko.py index 4eda2d3fd..8339baefc 100644 --- a/gns3server/agent/gns3_copilot/tools_v2/vpcs_tools_netmiko.py +++ b/gns3server/agent/gns3_copilot/tools_v2/vpcs_tools_netmiko.py @@ -196,7 +196,7 @@ class VPCSCommands(BaseTool): ) except ValueError as e: logger.error("Failed to prepare device hosts data: %s", e) - return [{"error": str(e)}] + return [{"status": "failed", "error": str(e)}] # Check if any devices have errors (e.g., missing device) error_devices = { @@ -223,7 +223,7 @@ class VPCSCommands(BaseTool): dynamic_nr = self._initialize_nornir(hosts_data) except ValueError as e: logger.error("Failed to initialize Nornir: %s", e) - return [{"error": str(e)}] + return [{"status": "failed", "error": str(e)}] results = [] @@ -244,7 +244,7 @@ class VPCSCommands(BaseTool): except Exception as e: # Overall execution failed logger.error("Error executing commands on all VPCS devices: %s", e) - return [{"error": f"Execution error: {str(e)}"}] + return [{"status": "failed", "error": f"Execution error: {str(e)}"}] logger.debug( "VPCS command execution completed. Results: %s", @@ -338,7 +338,7 @@ class VPCSCommands(BaseTool): "Invalid JSON string received as tool input: %s", e ) return ( - [{"error": f"Invalid JSON string input from model: {e}"}], + [{"status": "failed", "error": f"Invalid JSON string input from model: {e}"}], None, ) else: @@ -357,18 +357,18 @@ class VPCSCommands(BaseTool): if not project_id: error_msg = "Missing required 'project_id' field in input" logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) if not self._validate_project_id(project_id): error_msg = f"Invalid project_id: {project_id}. Expected UUID." logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) # Validate device_configs if not isinstance(device_configs, list): error_msg = "'device_configs' must be an array" logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) if not device_configs: logger.warning("Device configs list is empty.") @@ -382,7 +382,7 @@ class VPCSCommands(BaseTool): f"got {type(parsed_input).__name__}" ) logger.error(error_msg) - return ([{"error": error_msg}], None) + return ([{"status": "failed", "error": error_msg}], None) def _validate_project_id(self, project_id: str) -> bool: """ @@ -578,8 +578,8 @@ class VPCSCommands(BaseTool): if device_name in hosts_data and "error" in hosts_data[device_name]: results.append({ "device_name": device_name, - "status": "error", - "output": hosts_data[device_name]["error"], + "status": "failed", + "error": hosts_data[device_name]["error"], "commands": device_config["commands"], }) continue @@ -593,8 +593,8 @@ class VPCSCommands(BaseTool): error_msg = str(host_result.result) if host_result.result else "Unknown error" results.append({ "device_name": device_name, - "status": "error", - "output": error_msg, + "status": "failed", + "error": error_msg, "commands": device_config["commands"], }) else: @@ -609,8 +609,8 @@ class VPCSCommands(BaseTool): # Device not in task result (shouldn't happen) results.append({ "device_name": device_name, - "status": "error", - "output": f"Device '{device_name}' not in task results", + "status": "failed", + "error": f"Device '{device_name}' not in task results", "commands": device_config["commands"], }) diff --git a/gns3server/agent/mcp/__init__.py b/gns3server/agent/mcp/__init__.py index 780c05e62..3b28b3cea 100644 --- a/gns3server/agent/mcp/__init__.py +++ b/gns3server/agent/mcp/__init__.py @@ -1471,6 +1471,10 @@ async def device_config_send( Devices must be started first (use node_start or node_start_all). Device type is auto-detected from the 'device_type:' tag on each node. Common device types: cisco_ios_telnet, cisco_xr_telnet, huawei_telnet, gns3_huawei_telnet_ce + + Error contract: every failure is reported in-band as an entry with + status "failed" and an "error" message (per-device entries also carry + device_name and commands). """ params = {"project_id": project_id, "device_configs": device_configs} if template is not None: @@ -1501,6 +1505,10 @@ async def device_show_run( (e.g. device_type:cisco_ios_telnet, device_type:gns3_huawei_telnet_ce). Nodes without this tag will fail with "device_type tag not found". Docker/Linux nodes are not supported (use node_console instead). + + Error contract: every failure is reported in-band as an entry with + status "failed" and an "error" message (per-device entries also carry + device_name and commands). """ params = {"project_id": project_id, "device_configs": device_configs} if template is not None: @@ -1519,6 +1527,8 @@ async def vpcs_config_set( Only VPCS nodes are accepted: any other node type in device_configs fails with a per-device error instead of typing VPCS syntax into its CLI. + Every failure is reported in-band as an entry with status "failed" + and an "error" message. VPCS-specific configuration commands: - ip
/ Set IP and gateway diff --git a/gns3server/agent/mcp/device_config.py b/gns3server/agent/mcp/device_config.py index 10d4b7634..990f4b097 100644 --- a/gns3server/agent/mcp/device_config.py +++ b/gns3server/agent/mcp/device_config.py @@ -51,7 +51,13 @@ def _render_template(template: str, device_configs: list[dict], commands_field: Args: commands_field: field name for the rendered commands, e.g. "config_commands", "commands" """ - jinja = JinjaTemplate(template) + try: + jinja = JinjaTemplate(template) + except JinjaError as e: + # a syntactically invalid template must not escape as a raw exception + error_msg = f"Template rendering failed: {e}" + log.error(error_msg) + return [{"status": "failed", "error": error_msg}] merged: dict[str, dict] = {} for dev in device_configs: name = dev.get("device_name") @@ -69,7 +75,7 @@ def _render_template(template: str, device_configs: list[dict], commands_field: except JinjaError as e: error_msg = f"Template rendering failed for '{name}': {e}" log.error(error_msg) - return [{"error": error_msg}] + return [{"status": "failed", "error": error_msg}] return list(merged.values()) @@ -81,7 +87,7 @@ def device_config_send_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) device_configs = params.get("device_configs") template = params.get("template") if not project_id or not device_configs: - return [{"error": "project_id and device_configs are required"}] + return [{"status": "failed", "error": "project_id and device_configs are required"}] if template: device_configs = _render_template(template, device_configs, commands_field="config_commands") @@ -108,7 +114,7 @@ def device_show_run_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> device_configs = params.get("device_configs") template = params.get("template") if not project_id or not device_configs: - return [{"error": "project_id and device_configs (list of {device_name, commands}) are required"}] + return [{"status": "failed", "error": "project_id and device_configs (list of {device_name, commands}) are required"}] if template: device_configs = _render_template(template, device_configs, commands_field="commands") @@ -134,7 +140,7 @@ def vpcs_config_set_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> project_id = params.get("project_id") device_configs = params.get("device_configs") if not project_id or not device_configs: - return [{"error": "project_id and device_configs are required"}] + return [{"status": "failed", "error": "project_id and device_configs are required"}] from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands diff --git a/tests/agent/mcp/test_device_tools.py b/tests/agent/mcp/test_device_tools.py index fc996332e..5c5cd8c63 100644 --- a/tests/agent/mcp/test_device_tools.py +++ b/tests/agent/mcp/test_device_tools.py @@ -63,3 +63,77 @@ class TestVPCSNodeTypeGuard: })) assert result[0]["status"] == "success" assert result[0]["output"] == "OK" + + def test_execution_failure_reports_failed_with_error(self): + from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands + + tool = VPCSCommands() + nornir = MagicMock() + host_result = MagicMock(failed=True) + host_result.result = "Command failed (ReadTimeout)" + nornir.run.return_value = {"PC1": host_result} + with patch(f"{VPCS_MOD}.get_device_ports_from_topology", + return_value=_topology_ports("vpcs")), \ + patch.object(VPCSCommands, "_initialize_nornir", return_value=nornir): + result = tool._run(json.dumps({ + "project_id": "0c0fde25-6ead-4413-a283-ea8fd2324291", + "device_configs": [{"device_name": "PC1", "commands": ["ip 10.0.0.1/24"]}], + })) + assert result[0]["status"] == "failed" + assert result[0]["error"] == "Command failed (ReadTimeout)" + assert "output" not in result[0] + + +class TestDeviceToolErrorContract: + """ + Every in-band error entry carries status "failed" plus an "error" + message, whether it is topology-level (no device) or per-device. + """ + + def test_topology_level_error_has_status(self): + from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands + + with patch(f"{VPCS_MOD}.get_device_ports_from_topology", + side_effect=ValueError("topology unreachable")): + result = VPCSCommands()._run(json.dumps({ + "project_id": "0c0fde25-6ead-4413-a283-ea8fd2324291", + "device_configs": [{"device_name": "PC1", "commands": ["ip 10.0.0.1/24"]}], + })) + assert result == [{"status": "failed", "error": "topology unreachable"}] + + def test_config_tool_topology_level_error_has_status(self): + from gns3server.agent.gns3_copilot.tools_v2.config_tools_nornir import ( + ExecuteMultipleDeviceConfigCommands, + ) + + with patch("gns3server.agent.gns3_copilot.tools_v2.config_tools_nornir" + ".get_device_ports_from_topology", + side_effect=ValueError("no valid devices")): + result = ExecuteMultipleDeviceConfigCommands()._run(json.dumps({ + "project_id": "0c0fde25-6ead-4413-a283-ea8fd2324291", + "device_configs": [{"device_name": "R1", "config_commands": ["int lo0"]}], + })) + assert result == [{"status": "failed", "error": "no valid devices"}] + + def test_mcp_handler_param_error_has_status(self, ctx=None): + from gns3server.agent.mcp.device_config import ( + device_config_send_handler, + device_show_run_handler, + vpcs_config_set_handler, + ) + + for handler in (device_config_send_handler, device_show_run_handler, vpcs_config_set_handler): + result = handler({}, {"server_url": "http://x", "jwt_token": "t"}) + assert result == [{ + "status": "failed", + "error": result[0]["error"], # message text may differ per handler + }] + assert "required" in result[0]["error"] + + def test_template_render_error_has_status(self): + from gns3server.agent.mcp.device_config import _render_template + + result = _render_template("{{ unclosed", [{"device_name": "R1", "vars": {"n": 1}}]) + assert len(result) == 1 + assert result[0]["status"] == "failed" + assert "Template rendering failed" in result[0]["error"]