fix: keep submission order and unify status in MCP batch handlers

Batch node/link creation collected results with as_completed, so the
response order followed completion rather than the submitted array and
callers could not correlate entries. Collect in submission order via
pool.map, and report batch deletes as status=success like every other
batch action (the message still says what was deleted).
This commit is contained in:
YueGuobin 2026-08-25 21:32:09 +08:00
parent c9bc635996
commit 57b5baed7f
No known key found for this signature in database
2 changed files with 56 additions and 13 deletions

View File

@ -48,7 +48,7 @@ pulls the request-scoped user JWT from the context variables.
"""
from typing import Any
from concurrent.futures import ThreadPoolExecutor, as_completed
from concurrent.futures import ThreadPoolExecutor
import hashlib
import logging
@ -315,7 +315,6 @@ def create_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
if not isinstance(nodes, list) or not nodes:
return {"error": "nodes must be a non-empty array"}
default_tid = params.get("template_id")
results = []
conn = _get_connector(gns3_ctx)
def _create_one(node_data):
tid = node_data.get("template_id", default_tid)
@ -336,10 +335,9 @@ def create_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
except Exception as e:
return {"template_id": tid, "status": "error", "error": str(e)}
with ThreadPoolExecutor(max_workers=min(len(nodes), BATCH_MAX_WORKERS)) as pool:
futures = {pool.submit(_create_one, n): n for n in nodes}
for future in as_completed(futures):
results.append(future.result())
return results
# pool.map keeps the submission order, so callers can correlate
# results with the nodes they sent regardless of completion order
return list(pool.map(_create_one, nodes))
# Single mode
template_id = params.get("template_id")
@ -371,7 +369,7 @@ def delete_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
def _del(nid):
try:
conn.http_call("delete", f"{conn.base_url}/projects/{project_id}/nodes/{nid}")
return {"node_id": nid, "status": "deleted"}
return {"node_id": nid, "status": "success", "message": f"Node {nid} deleted"}
except Exception as e:
return {"node_id": nid, "status": "error", "error": str(e)}
with ThreadPoolExecutor(max_workers=min(len(node_ids), BATCH_MAX_WORKERS)) as pool:
@ -656,7 +654,6 @@ def create_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
if links is not None:
if not isinstance(links, list) or not links:
return {"error": "links must be a non-empty array"}
results = []
conn = _get_connector(gns3_ctx)
def _create_one(link_data):
raw_nodes = link_data.get("nodes")
@ -676,10 +673,9 @@ def create_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
except Exception as e:
return {"status": "error", "error": str(e)}
with ThreadPoolExecutor(max_workers=min(len(links), BATCH_MAX_WORKERS)) as pool:
futures = {pool.submit(_create_one, link): link for link in links}
for future in as_completed(futures):
results.append(future.result())
return results
# pool.map keeps the submission order, so callers can correlate
# results with the links they sent regardless of completion order
return list(pool.map(_create_one, links))
# Single mode
nodes = params.get("nodes")
@ -710,7 +706,7 @@ def delete_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic
def _del(lid):
try:
conn.http_call("delete", f"{conn.base_url}/projects/{project_id}/links/{lid}")
return {"link_id": lid, "status": "deleted"}
return {"link_id": lid, "status": "success", "message": f"Link {lid} deleted"}
except Exception as e:
return {"link_id": lid, "status": "error", "error": str(e)}
with ThreadPoolExecutor(max_workers=min(len(link_ids), BATCH_MAX_WORKERS)) as pool:

View File

@ -182,6 +182,25 @@ class TestNode:
}, ctx)
assert result[0]["status"] == "success"
def test_create_batch_preserves_submission_order(self, ctx):
import time
from gns3server.agent.gns3_copilot.gns3_client.api_handlers import create_node_handler
with patch(f"{AH}._get_connector") as m:
conn = _mock_conn()
def _http_call(method, url, json_data=None, **kwargs):
# first submissions sleep longest so completion order is reversed
time.sleep({"slow": 0.25, "mid": 0.1}.get(json_data.get("name"), 0.0))
resp = MagicMock()
resp.json.return_value = {"node_id": "n1", "name": json_data["name"]}
return resp
conn.http_call.side_effect = _http_call
m.return_value = conn
result = create_node_handler({
"project_id": "p1", "template_id": "t1",
"nodes": [{"name": "slow"}, {"name": "mid"}, {"name": "fast"}],
}, ctx)
assert [r["node"]["name"] for r in result] == ["slow", "mid", "fast"]
def test_create_missing_project_id(self, ctx):
from gns3server.agent.gns3_copilot.gns3_client.api_handlers import create_node_handler
assert create_node_handler({}, ctx) == {"error": "project_id is required"}
@ -192,6 +211,8 @@ class TestNode:
m.return_value = _mock_conn({})
result = delete_node_handler({"project_id": "p1", "node_ids": ["n1", "n2"]}, ctx)
assert len(result) == 2
# same status vocabulary as create/start/stop batches
assert all(r["status"] == "success" for r in result)
def test_start_batch(self, ctx):
from gns3server.agent.gns3_copilot.gns3_client.api_handlers import start_node_handler
@ -291,6 +312,32 @@ class TestLink:
m.return_value = _mock_conn({})
result = delete_link_handler({"project_id": "p1", "link_ids": ["l1", "l2"]}, ctx)
assert len(result) == 2
# same status vocabulary as create batches
assert all(r["status"] == "success" for r in result)
def test_create_batch_preserves_submission_order(self, ctx):
import time
from gns3server.agent.gns3_copilot.gns3_client.api_handlers import create_link_handler
with patch(f"{AH}._get_connector") as m:
conn = _mock_conn()
def _http_call(method, url, json_data=None, **kwargs):
# first submission sleeps longest so completion order is reversed
first_node = json_data["nodes"][0]["node_id"]
time.sleep(0.25 if first_node == "n1" else 0.0)
resp = MagicMock()
resp.json.return_value = {"link_id": f"link-{first_node}"}
return resp
conn.http_call.side_effect = _http_call
m.return_value = conn
result = create_link_handler({
"project_id": "p1",
"links": [
{"nodes": ["n1", 0, 0, "n2", 0, 0]},
{"nodes": ["n3", 0, 0, "n4", 0, 0]},
],
"fields": ["link_id"],
}, ctx)
assert [r["link"]["link_id"] for r in result] == ["link-n1", "link-n3"]
def test_update(self, ctx):
from gns3server.agent.gns3_copilot.gns3_client.api_handlers import update_link_handler