NgTech-LTD/gns3-server
2
0
Fork 0
mirror of https://github.com/GNS3/gns3-server.git synced 2026-10-12 21:45:41 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(tools): merge same-device entries before device command execution

Browse Source 
A batch carrying the same device twice lost all but the last entry's
commands: the per-device command map kept only the last entry while
result processing iterated every original entry, so each row reported
the last entry's output under its own command list.

c3c78f99a consolidated the merge into the MCP template renderer, but
that only runs when a template is passed — the MCP direct-commands
path and gns3-copilot's direct _run path were left uncovered.
Normalize once in each tool's _run (single point covering all entry
paths, restoring the c42b3a59b coverage) and keep the renderer merge
as boundary normalization. Blocked-command info now also accumulates
across same-device entries instead of last-winning in the per-device
map.
This commit is contained in:
YueGuobin 2026-10-04 19:20:22 +08:00
parent d0b0cdbd38
commit e13e5f3804
No known key found for this signature in database
5 changed files with 300 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py
View File

@ -47,7 +47,6 @@ from nornir.core.task import AggregatedResult, Result, Task
from nornir_netmiko.tasks import netmiko_send_config
from gns3server.agent.gns3_copilot.gns3_client import get_gns3_server_host
# Import custom Netmiko device types for GNS3 emulation
# This registers gns3_huawei_telnet_ce and other custom device types
# NOTE: Must be imported BEFORE any Nornir operations to ensure device types are registered
@ -55,6 +54,9 @@ from gns3server.agent.gns3_copilot.utils import get_device_ports_from_topology
from gns3server.agent.gns3_copilot.utils.command_filter import (
filter_forbidden_commands,
)
from gns3server.agent.gns3_copilot.utils.device_configs import (
merge_duplicate_device_configs,
)
# Explicitly register custom device types to ensure they are available
# This is a safety measure in case the auto-registration on import doesn't work
@ -196,6 +198,9 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool):
if len(device_configs_list) > 0 and "error" in device_configs_list[0]:
return device_configs_list
# Merge same-device entries before anything keys by device_name
device_configs_list = merge_duplicate_device_configs(device_configs_list, commands_field="config_commands")
# Filter forbidden commands and store blocked commands info
device_configs_list, blocked_commands_map = self._filter_forbidden_commands_from_device_configs(
device_configs_list

7
gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py
View File

@ -47,7 +47,6 @@ from nornir.core.task import AggregatedResult, Result, Task
from nornir_netmiko.tasks import netmiko_multiline
from gns3server.agent.gns3_copilot.gns3_client import get_gns3_server_host
# Import custom Netmiko device types for GNS3 emulation
# This registers gns3_huawei_telnet_ce and other custom device types
# NOTE: Must be imported BEFORE any Nornir operations to ensure device types are registered
@ -55,6 +54,9 @@ from gns3server.agent.gns3_copilot.utils import get_device_ports_from_topology
from gns3server.agent.gns3_copilot.utils.command_filter import (
filter_forbidden_commands,
)
from gns3server.agent.gns3_copilot.utils.device_configs import (
merge_duplicate_device_configs,
)
# Explicitly register custom device types to ensure they are available
# This is a safety measure in case the auto-registration on import doesn't work
@ -199,6 +201,9 @@ class ExecuteMultipleDeviceCommands(BaseTool):
if len(device_configs_list) > 0 and "error" in device_configs_list[0]:
return device_configs_list
# Merge same-device entries before anything keys by device_name
device_configs_list = merge_duplicate_device_configs(device_configs_list, commands_field="commands")
# Filter forbidden commands and store blocked commands info
device_configs_list, blocked_commands_map = self._filter_forbidden_commands_from_device_configs(
device_configs_list

7
gns3server/agent/gns3_copilot/tools_v2/vpcs_tools_netmiko.py
View File

@ -47,11 +47,13 @@ from nornir.core.task import AggregatedResult, Result, Task
from nornir_netmiko.tasks import netmiko_multiline
from gns3server.agent.gns3_copilot.gns3_client import get_gns3_server_host
# Import custom Netmiko device types for GNS3 emulation
# This registers gns3_vpcs_telnet and other custom device types
# NOTE: Must be imported BEFORE any Nornir operations to ensure device types are registered
from gns3server.agent.gns3_copilot.utils import get_device_ports_from_topology
from gns3server.agent.gns3_copilot.utils.device_configs import (
merge_duplicate_device_configs,
)
# Explicitly register VPCS device type to ensure it is available
try:
@ -179,6 +181,9 @@ class VPCSCommands(BaseTool):
if len(device_configs_list) > 0 and "error" in device_configs_list[0]:
return device_configs_list
# Merge same-device entries before anything keys by device_name
device_configs_list = merge_duplicate_device_configs(device_configs_list, commands_field="commands")
# Create a mapping of device names to their commands
device_configs_map = self._configs_map(device_configs_list)

71
gns3server/agent/gns3_copilot/utils/device_configs.py Normal file
View File

@ -0,0 +1,71 @@
# SPDX-License-Identifier: GPL-3.0-or-later
#
# GNS3-Copilot - AI-powered Network Lab Assistant for GNS3
#
# This file is part of GNS3-Copilot project.
#
# GNS3-Copilot is free software: you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by the
# Free Software Foundation, either version 3 of the License, or (at your
# option) any later version.
#
# GNS3-Copilot is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
# Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with GNS3-Copilot. If not, see <https://www.gnu.org/licenses/>.
#
# Copyright (C) 2025 Yue Guobin (岳国宾)
# Author: Yue Guobin (岳国宾)
#
# Project Home: https://github.com/yueguobin/gns3-copilot
#
"""
Device config entry normalization shared by the tools_v2 executors.
Callers (LLM agents via MCP, or gns3-copilot directly) may send the same
device several times in one batch. Every downstream stage keys by
device_name, so duplicates must merge before execution, not after.
"""
from typing import Any
def merge_duplicate_device_configs(
device_configs: list[dict[str, Any]],
commands_field: str,
) -> list[dict[str, Any]]:
"""
Merge entries that share a device_name, concatenating their command lists.
Without this merge the per-device command map keeps only the last
entry's commands while result processing iterates every original entry —
duplicate devices would silently drop all but the last command batch and
every row would report that same output.
Args:
device_configs: Device config entries; each holds device_name and a
command list under commands_field.
commands_field: Field carrying the commands ("commands" or
"config_commands" depending on the tool).
Returns:
One entry per device_name, in first-appearance order. The first
entry's extra fields are preserved; later duplicates only contribute
their commands.
"""
merged: dict[str, dict[str, Any]] = {}
for config in device_configs:
name = config.get("device_name")
if not name:
continue
commands = list(config.get(commands_field, []))
if name in merged:
merged[name][commands_field].extend(commands)
else:
entry = dict(config)
entry[commands_field] = commands
merged[name] = entry
return list(merged.values())

213
tests/agent/mcp/test_device_tools.py
View File

@ -1,14 +1,24 @@
"""
Device config tool tests with mocked topology and Nornir layers.
Covers the VPCS node-type guard and the error contract shared by
device_config_send / device_show_run / vpcs_config_set.
Covers the VPCS node-type guard, the error contract shared by
device_config_send / device_show_run / vpcs_config_set, and the
same-device entry merging that keeps duplicate entries from
overwriting each other's commands and outputs.
"""
import json
from unittest.mock import MagicMock, patch
from gns3server.agent.gns3_copilot.utils.device_configs import (
merge_duplicate_device_configs,
)
VPCS_MOD = "gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko"
DISPLAY_MOD = "gns3server.agent.gns3_copilot.tools_v2.display_tools_nornir"
CONFIG_MOD = "gns3server.agent.gns3_copilot.tools_v2.config_tools_nornir"
PROJECT_ID = "0c0fde25-6ead-4413-a283-ea8fd2324291"
def _topology_ports(node_type):
@ -16,6 +26,26 @@ def _topology_ports(node_type):
return {"PC1": {"port": 5000, "node_type": node_type}}
def _topology_two_devices():
"""Mocked topology with two console-reachable devices."""
return {
"R1": {"port": 5000, "node_type": "iou"},
"R2": {"port": 5001, "node_type": "iou"},
}
def _aggregated_result(output_by_device):
"""Real AggregatedResult: device -> MultiResult with one successful Result."""
from nornir.core.task import AggregatedResult, MultiResult, Result
aggregated = AggregatedResult("task")
for name, output in output_by_device.items():
multi = MultiResult("task")
multi.append(Result(host=None, result=output, failed=False))
aggregated[name] = multi
return aggregated
class TestVPCSNodeTypeGuard:
def test_non_vpcs_node_is_rejected(self):
from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands
@ -161,3 +191,182 @@ class TestDeviceToolErrorContract:
assert len(result) == 1
assert result[0]["status"] == "failed"
assert "Template rendering failed" in result[0]["error"]
class TestDuplicateDeviceMerging:
"""
Same-device entries in one batch must merge before execution.
Regression: the per-device command map kept only the last entry's
commands while result processing iterated every original entry, so all
rows reported the last entry's output (first reproduced with the MCP
direct-commands path, no template).
"""
def test_show_run_merges_same_device_entries(self):
from gns3server.agent.gns3_copilot.tools_v2.display_tools_nornir import (
ExecuteMultipleDeviceCommands,
)
nornir = MagicMock()
nornir.run.return_value = _aggregated_result({"R1": "merged output", "R2": "r2 output"})
with (
patch(f"{DISPLAY_MOD}.get_device_ports_from_topology", return_value=_topology_two_devices()),
patch.object(ExecuteMultipleDeviceCommands, "_initialize_nornir", return_value=nornir),
):
result = ExecuteMultipleDeviceCommands()._run(
json.dumps(
{
"project_id": PROJECT_ID,
"device_configs": [
{"device_name": "R1", "commands": ["show ip route"]},
{"device_name": "R2", "commands": ["show version"]},
{"device_name": "R1", "commands": ["show ip interface brief"]},
],
}
)
)
# every command reached the execution layer, in order
assert nornir.run.call_args.kwargs["device_configs_map"] == {
"R1": ["show ip route", "show ip interface brief"],
"R2": ["show version"],
}
# one truthful row per device
assert [row["device_name"] for row in result] == ["R1", "R2"]
assert result[0]["status"] == "success"
assert result[0]["output"] == "merged output"
assert result[0]["diagnostic_commands"] == ["show ip route", "show ip interface brief"]
def test_config_send_merges_same_device_entries(self):
from gns3server.agent.gns3_copilot.tools_v2.config_tools_nornir import (
ExecuteMultipleDeviceConfigCommands,
)
nornir = MagicMock()
nornir.run.return_value = _aggregated_result({"R1": "merged output"})
with (
patch(f"{CONFIG_MOD}.get_device_ports_from_topology", return_value={"R1": {"port": 5000, "node_type": "iou"}}),
patch.object(ExecuteMultipleDeviceConfigCommands, "_initialize_nornir", return_value=nornir),
):
result = ExecuteMultipleDeviceConfigCommands()._run(
json.dumps(
{
"project_id": PROJECT_ID,
"device_configs": [
{"device_name": "R1", "config_commands": ["int lo0"]},
{"device_name": "R1", "config_commands": ["ip address 1.1.1.1 255.255.255.255", "exit"]},
],
}
)
)
assert nornir.run.call_args.kwargs["device_configs_map"] == {
"R1": ["int lo0", "ip address 1.1.1.1 255.255.255.255", "exit"],
}
assert len(result) == 1
assert result[0]["status"] == "success"
assert result[0]["config_commands"] == ["int lo0", "ip address 1.1.1.1 255.255.255.255", "exit"]
def test_vpcs_merges_same_device_entries(self):
from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands
nornir = MagicMock()
host_result = MagicMock(failed=False)
host_result.result = "OK"
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 = VPCSCommands()._run(
json.dumps(
{
"project_id": PROJECT_ID,
"device_configs": [
{"device_name": "PC1", "commands": ["ip 10.0.0.1/24"]},
{"device_name": "PC1", "commands": ["save"]},
],
}
)
)
assert nornir.run.call_args.kwargs["device_configs_map"] == {"PC1": ["ip 10.0.0.1/24", "save"]}
assert len(result) == 1
assert result[0]["status"] == "success"
assert result[0]["commands"] == ["ip 10.0.0.1/24", "save"]
def test_blocked_commands_accumulate_across_duplicate_entries(self):
"""Blocked-command info must not last-win between same-device entries."""
from gns3server.agent.gns3_copilot.tools_v2.display_tools_nornir import (
ExecuteMultipleDeviceCommands,
)
def fake_filter(commands):
blocked = {c: "matched forbidden pattern" for c in commands if c.startswith("debug")}
allowed = [c for c in commands if not c.startswith("debug")]
return allowed, blocked
nornir = MagicMock()
nornir.run.return_value = _aggregated_result({"R1": "ok"})
with (
patch(f"{DISPLAY_MOD}.get_device_ports_from_topology", return_value={"R1": {"port": 5000, "node_type": "iou"}}),
patch.object(ExecuteMultipleDeviceCommands, "_initialize_nornir", return_value=nornir),
patch(f"{DISPLAY_MOD}.filter_forbidden_commands", side_effect=fake_filter),
):
result = ExecuteMultipleDeviceCommands()._run(
json.dumps(
{
"project_id": PROJECT_ID,
"device_configs": [
{"device_name": "R1", "commands": ["show version", "debug ip routing"]},
{"device_name": "R1", "commands": ["debug ospf events", "show clock"]},
],
}
)
)
assert nornir.run.call_args.kwargs["device_configs_map"] == {"R1": ["show version", "show clock"]}
assert result[0]["status"] == "partial_success"
assert set(result[0]["blocked_commands"]) == {"debug ip routing", "debug ospf events"}
class TestMergeDuplicateDeviceConfigs:
"""Unit tests for the shared normalization helper."""
def test_merges_in_first_appearance_order(self):
merged = merge_duplicate_device_configs(
[
{"device_name": "R1", "commands": ["a"]},
{"device_name": "R2", "commands": ["b"]},
{"device_name": "R1", "commands": ["c", "d"]},
],
commands_field="commands",
)
assert merged == [
{"device_name": "R1", "commands": ["a", "c", "d"]},
{"device_name": "R2", "commands": ["b"]},
]
def test_preserves_first_entry_fields_without_mutating_input(self):
original = [
{"device_name": "R1", "commands": ["a"], "extra": 1},
{"device_name": "R1", "commands": ["b"]},
]
merged = merge_duplicate_device_configs(original, commands_field="commands")
assert merged == [{"device_name": "R1", "commands": ["a", "b"], "extra": 1}]
assert original[0]["commands"] == ["a"]
def test_entry_without_device_name_is_dropped(self):
merged = merge_duplicate_device_configs(
[{"commands": ["a"]}, {"device_name": "R1", "commands": ["b"]}],
commands_field="commands",
)
assert merged == [{"device_name": "R1", "commands": ["b"]}]
def test_missing_commands_field_defaults_to_empty(self):
merged = merge_duplicate_device_configs(
[{"device_name": "R1"}, {"device_name": "R1", "commands": ["b"]}],
commands_field="commands",
)
assert merged == [{"device_name": "R1", "commands": ["b"]}]