Fix fabric path search, read scope, orbit total ranking, and edge menu clamp.

Prefer BFS shortest paths, allow ne:read on path find, honor objective=total clearance trades, and keep context menus on-screen without a11y clock spam.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
oliver 2026-08-10 21:31:50 +08:00
parent 298859e608
commit e4a135ec16
13 changed files with 163 additions and 30 deletions

View file

@ -141,6 +141,9 @@ def required_scope_for_request(method: str, path: str) -> str | None:
return SCOPE_NE_READ return SCOPE_NE_READ
if p.startswith("/v1/topology"): if p.startswith("/v1/topology"):
# Read-only path search uses POST for a structured body.
if m == "POST" and p.rstrip("/").endswith("/fabric/paths"):
return SCOPE_NE_READ
if m in ("POST", "PUT", "PATCH", "DELETE"): if m in ("POST", "PUT", "PATCH", "DELETE"):
return SCOPE_NE_WRITE return SCOPE_NE_WRITE
return SCOPE_NE_READ return SCOPE_NE_READ

View file

@ -47,6 +47,9 @@ def validate_select_sql(sql: str, *, allowed_tables: set[str] | None = None) ->
low = cleaned.lower().lstrip() low = cleaned.lower().lstrip()
if not low.startswith(("select", "with")): if not low.startswith(("select", "with")):
raise HTTPException(status_code=400, detail="select_only") raise HTTPException(status_code=400, detail="select_only")
# Plain WITH CTE is allowed; RECURSIVE can explode into DoS.
if re.search(r"\bwith\s+recursive\b", cleaned, flags=re.IGNORECASE):
raise HTTPException(status_code=400, detail="with_recursive_not_allowed")
if _SQL_FORBIDDEN_RE.search(cleaned): if _SQL_FORBIDDEN_RE.search(cleaned):
raise HTTPException(status_code=400, detail="forbidden_keyword") raise HTTPException(status_code=400, detail="forbidden_keyword")
# Block obvious catalog / other-schema probes in the text. # Block obvious catalog / other-schema probes in the text.

View file

@ -2,6 +2,7 @@
from __future__ import annotations from __future__ import annotations
import re import re
from collections import deque
from datetime import datetime, timedelta from datetime import datetime, timedelta
from typing import Any from typing import Any
from uuid import uuid4 from uuid import uuid4
@ -509,25 +510,26 @@ def find_fabric_paths(
adj.setdefault(e.a_node_id, []).append((e.b_node_id, e.id)) adj.setdefault(e.a_node_id, []).append((e.b_node_id, e.id))
adj.setdefault(e.b_node_id, []).append((e.a_node_id, e.id)) adj.setdefault(e.b_node_id, []).append((e.a_node_id, e.id))
# DFS for simple paths (no repeated nodes), capped to avoid blow-up on large graphs. # BFS for simple paths so shorter hops are found first; cap expansions on dense graphs.
_EXPLORE_CAP = 100 _EXPLORE_CAP = 5000
all_paths: list[list[str]] = [] found: list[list[str]] = []
stack: list[tuple[str, list[str], set[str]]] = [(from_id, [], {from_id})] queue: deque[tuple[str, list[str], set[str]]] = deque([(from_id, [], {from_id})])
while stack and len(all_paths) < _EXPLORE_CAP: explored = 0
node, edge_path, visited = stack.pop() while queue and len(found) < max_paths and explored < _EXPLORE_CAP:
node, edge_path, visited = queue.popleft()
if len(edge_path) >= max_hops: if len(edge_path) >= max_hops:
continue continue
for nbr, eid in adj.get(node, []): for nbr, eid in adj.get(node, []):
if nbr in visited: if nbr in visited:
continue continue
explored += 1
new_path = edge_path + [eid] new_path = edge_path + [eid]
if nbr == to_id: if nbr == to_id:
all_paths.append(new_path) found.append(new_path)
if len(found) >= max_paths:
break
continue continue
stack.append((nbr, new_path, visited | {nbr})) queue.append((nbr, new_path, visited | {nbr}))
all_paths.sort(key=len)
found = all_paths[:max_paths]
node_ids = {from_id, to_id} node_ids = {from_id, to_id}
for p in found: for p in found:

View file

@ -26,6 +26,7 @@ from .topology_fabric import (
ensure_fabric_node_for_managed, ensure_fabric_node_for_managed,
ensure_fabric_node_for_ume, ensure_fabric_node_for_ume,
ensure_lldp_discovered_managed_ne, ensure_lldp_discovered_managed_ne,
find_fabric_paths,
get_fabric_neighborhood, get_fabric_neighborhood,
get_fabric_summary, get_fabric_summary,
list_fabric_edges, list_fabric_edges,

View file

@ -498,21 +498,22 @@ HTTP_MCP_TOOLS: list[dict[str, Any]] = [
"name": "findTopologyPaths", "name": "findTopologyPaths",
"description": ( "description": (
"Find up to max_paths simple paths between two fabric nodes for troubleshooting. " "Find up to max_paths simple paths between two fabric nodes for troubleshooting. "
"Accepts ume_ne_id (from UME alarms) or managed_ne_id — resolved to fabric node " "For each endpoint provide exactly one of ume_ne_id (from UME alarms) or "
"internally. Returns paths with node sequence + edge status (up/down)." "managed_ne_id — resolved to fabric node internally. Returns shortest paths "
"first with node sequence + edge status (up/down)."
), ),
"inputSchema": { "inputSchema": {
"type": "object", "type": "object",
"properties": { "properties": {
"from_ume_ne_id": {"type": "string", "description": "Source UME ne_id (from alarm ne_id)"}, "from_ume_ne_id": {"type": "string", "description": "Source UME ne_id (from alarm ne_id); mutually exclusive with from_managed_ne_id"},
"from_managed_ne_id": {"type": "string", "description": "Source managed NE id"}, "from_managed_ne_id": {"type": "string", "description": "Source managed NE id; mutually exclusive with from_ume_ne_id"},
"to_ume_ne_id": {"type": "string", "description": "Target UME ne_id"}, "to_ume_ne_id": {"type": "string", "description": "Target UME ne_id; mutually exclusive with to_managed_ne_id"},
"to_managed_ne_id": {"type": "string", "description": "Target managed NE id"}, "to_managed_ne_id": {"type": "string", "description": "Target managed NE id; mutually exclusive with to_ume_ne_id"},
"max_paths": {"type": "integer", "minimum": 1, "maximum": 10, "default": 3}, "max_paths": {"type": "integer", "minimum": 1, "maximum": 10, "default": 3},
"max_hops": {"type": "integer", "minimum": 1, "maximum": 12, "default": 6}, "max_hops": {"type": "integer", "minimum": 1, "maximum": 12, "default": 6},
"layer": {"type": "string", "default": "physical"}, "layer": {"type": "string", "default": "physical"},
}, },
"required": ["from_ume_ne_id", "to_ume_ne_id"], "required": [],
"additionalProperties": False, "additionalProperties": False,
}, },
}, },

View file

@ -489,10 +489,24 @@ def orbit_sweep_node(
scored.sort(key=_score_key) scored.sort(key=_score_key)
# Multi-objective re-rank: optimize total score, not just crossings # Multi-objective re-rank: optimize total score, not just crossings
if objective == "total" and len(scored) > 1: use_total = objective == "total" and len(scored) > 1
base_clearance_hits = 0
if use_total:
from netx_topology_mcp.layout_metrics import compute_edge_clearance
ec0 = compute_edge_clearance(pos, links, names=names, top_n=1)
base_clearance_hits = int(ec0.get("edge_clearance_hits") or 0)
scored = _rerank_by_total(scored, nid, pos, names, links, adj) scored = _rerank_by_total(scored, nid, pos, names, links, adj)
# Prefer improving moves; still return best even if none improve. # Prefer improving moves; still return best even if none improve.
improving = [c for c in scored if c["delta"]["global"] < 0] if use_total:
base_total = int(global0) + base_clearance_hits
improving = [
c
for c in scored
if int(c["crossings"]["global"]) + int(c.get("edge_clearance_hits", 0)) < base_total
]
else:
improving = [c for c in scored if c["delta"]["global"] < 0]
pool = improving if improving else scored pool = improving if improving else scored
top = _diversify_top(pool, k=max(1, int(top_k)), min_angle_sep=min_angle_sep) top = _diversify_top(pool, k=max(1, int(top_k)), min_angle_sep=min_angle_sep)
@ -582,8 +596,11 @@ def orbit_sweep_round(
protect_rigid: bool | str = "off", protect_rigid: bool | str = "off",
frozen_ids: set[str] | None = None, frozen_ids: set[str] | None = None,
focus_ids: list[str] | None = None, focus_ids: list[str] | None = None,
y_min: float | None = None,
y_max: float | None = None,
objective: str = "crossing",
) -> OpResult: ) -> OpResult:
"""Scan hot nodes; auto-apply each node's rank-1 if global crossings drop. """Scan hot nodes; auto-apply each node's rank-1 if the active objective improves.
Default ``protect_rigid=off`` (may move portals). Opt in with portals/all. Default ``protect_rigid=off`` (may move portals). Opt in with portals/all.
""" """
@ -653,6 +670,9 @@ def orbit_sweep_round(
min_angle_sep=min_angle_sep, min_angle_sep=min_angle_sep,
protect_rigid="off", # already applied frozen set protect_rigid="off", # already applied frozen set
frozen_ids=frozen, frozen_ids=frozen,
y_min=y_min,
y_max=y_max,
objective=objective,
) )
if not sweep.get("ok"): if not sweep.get("ok"):
trace.append({"node_id": nid, "skipped": sweep.get("error")}) trace.append({"node_id": nid, "skipped": sweep.get("error")})
@ -662,12 +682,14 @@ def orbit_sweep_round(
trace.append({"node_id": nid, "skipped": "no_candidates"}) trace.append({"node_id": nid, "skipped": "no_candidates"})
continue continue
best = cands[0] best = cands[0]
if int(best["delta"]["global"]) >= 0: # Skip when the active objective has no improving candidates.
if int(sweep.get("improving_n") or 0) <= 0:
trace.append( trace.append(
{ {
"node_id": nid, "node_id": nid,
"skipped": "no_global_gain", "skipped": "no_global_gain",
"best_delta": best["delta"], "best_delta": best["delta"],
"objective": objective,
} }
) )
continue continue

View file

@ -475,6 +475,9 @@ def run_layout_on_graph(
protect_rigid=protect, protect_rigid=protect,
frozen_ids=frozen, frozen_ids=frozen,
focus_ids=knobs.get("focus_ids"), focus_ids=knobs.get("focus_ids"),
y_min=knobs.get("y_min"),
y_max=knobs.get("y_max"),
objective=str(knobs.get("objective") or "crossing"),
) )
st = normalize_origin(op.state, base_params).state st = normalize_origin(op.state, base_params).state
fin = score_state(st) fin = score_state(st)

View file

@ -174,3 +174,37 @@ def test_orbit_default_protect_off_ignores_portal_freeze() -> None:
) )
assert frozen["ok"] is False assert frozen["ok"] is False
assert frozen.get("error") == "frozen" assert frozen.get("error") == "frozen"
def test_orbit_objective_total_ranks_clearance_trade() -> None:
"""objective=total may keep a crossing-up move if clearance improves enough."""
# Hub h near non-incident segment a—b; moving h right cuts clearance hits
# but can add a mild cross with c—d. Crossing-only ranking would reject it.
nodes = [
{"fabric_node_id": "a", "name": "AAAAAA-EN-1", "x": 0.0, "y": 0.0},
{"fabric_node_id": "b", "name": "BBBBBB-EN-2", "x": 400.0, "y": 0.0},
{"fabric_node_id": "c", "name": "CCCCCC-EN-3", "x": 200.0, "y": -200.0},
{"fabric_node_id": "d", "name": "DDDDDD-EN-4", "x": 200.0, "y": 200.0},
{"fabric_node_id": "h", "name": "HHHHHH-EN-5", "x": 200.0, "y": 20.0},
{"fabric_node_id": "t", "name": "TTTTTT-EN-6", "x": 200.0, "y": 300.0},
]
edges = [
{"a_node_id": "a", "b_node_id": "b"},
{"a_node_id": "c", "b_node_id": "d"},
{"a_node_id": "h", "b_node_id": "t"},
]
st = build_state_from_nodes_edges(nodes, edges)
st.positions = {n["fabric_node_id"]: (float(n["x"]), float(n["y"])) for n in nodes}
by_cross = orbit_sweep_node(st, "h", max_jump=400, nn_floor=20.0, objective="crossing")
by_total = orbit_sweep_node(st, "h", max_jump=400, nn_floor=20.0, objective="total")
assert by_cross["ok"] is True and by_total["ok"] is True
assert by_total.get("objective") == "total"
# y_band plumbing
banded = orbit_sweep_node(
st, "h", max_jump=400, nn_floor=20.0, objective="total", y_min=0.0, y_max=80.0
)
assert banded["ok"] is True
assert banded.get("y_band") == [0.0, 80.0]
for c in banded.get("candidates") or []:
assert 0.0 <= float(c["y"]) <= 80.0

View file

@ -51,17 +51,28 @@ class ScopeUnitTests(unittest.TestCase):
self.assertEqual(required_scope_for_request("GET", "/v1/webcrt/sessions"), SCOPE_WEBCRT) self.assertEqual(required_scope_for_request("GET", "/v1/webcrt/sessions"), SCOPE_WEBCRT)
self.assertEqual(required_scope_for_request("POST", "/v1/managed-ne/exec"), "ne:exec") self.assertEqual(required_scope_for_request("POST", "/v1/managed-ne/exec"), "ne:exec")
self.assertEqual(required_scope_for_request("POST", "/v1/managed-ne"), "ne:write") self.assertEqual(required_scope_for_request("POST", "/v1/managed-ne"), "ne:write")
self.assertEqual(required_scope_for_request("POST", "/v1/topology/fabric/paths"), "ne:read")
self.assertEqual(required_scope_for_request("POST", "/v1/topology/fabric/edges"), "ne:write")
class SqlGuardTests(unittest.TestCase): class SqlGuardTests(unittest.TestCase):
def test_allows_cte(self) -> None: def test_allows_cte(self) -> None:
# CTE (including WITH RECURSIVE) is now allowed; table whitelist still applies. # Plain WITH CTE is allowed; table whitelist still applies.
cleaned = validate_select_sql( cleaned = validate_select_sql(
"WITH x AS (SELECT * FROM ume_alarms_current) SELECT * FROM x", "WITH x AS (SELECT * FROM ume_alarms_current) SELECT * FROM x",
allowed_tables={"ume_alarms_current", "ume_inventory_ne"}, allowed_tables={"ume_alarms_current", "ume_inventory_ne"},
) )
self.assertTrue(cleaned.lower().startswith("with")) self.assertTrue(cleaned.lower().startswith("with"))
def test_rejects_recursive_cte(self) -> None:
with self.assertRaises(HTTPException) as ctx:
validate_select_sql(
"WITH RECURSIVE t AS (SELECT 1 AS n UNION ALL SELECT n+1 FROM t WHERE n < 100) "
"SELECT * FROM t",
allowed_tables={"ume_alarms_current", "ume_inventory_ne"},
)
self.assertEqual(ctx.exception.detail, "with_recursive_not_allowed")
def test_allows_cte_referencing_alias_in_from(self) -> None: def test_allows_cte_referencing_alias_in_from(self) -> None:
# CTE name appears in FROM but should not be flagged as unauthorized table. # CTE name appears in FROM but should not be flagged as unauthorized table.
cleaned = validate_select_sql( cleaned = validate_select_sql(

View file

@ -2495,6 +2495,62 @@ Management Addresses:
graph2 = svc.get_view_graph(self.db, view_id) graph2 = svc.get_view_graph(self.db, view_id)
self.assertFalse(any(e.id == edge_id for e in graph2.edges)) self.assertFalse(any(e.id == edge_id for e in graph2.edges))
def test_find_fabric_paths_prefers_shortest(self) -> None:
"""BFS should return the 1-hop path before a longer detour."""
suffix = uuid4().hex[:8]
nes = []
for label, ip_tail in (("A", 1), ("B", 2), ("C", 3)):
ne = ManagedNE(
id=f"path-{suffix}-{label}",
name=f"PATH-{label}-{suffix}",
vendor="Cisco",
device_type="cisco_ios",
ip_address=f"198.51.100.{ip_tail}",
)
self.db.add(ne)
nes.append(ne)
self.db.commit()
nodes = [svc.ensure_fabric_node_for_managed(self.db, ne) for ne in nes]
self.db.commit()
# Short: A—B ; Long: A—C—B
svc.upsert_fabric_edge(
self.db,
a_node_id=nodes[0].id,
b_node_id=nodes[1].id,
a_port="Gi0/0",
b_port="Gi0/0",
source="manual",
)
svc.upsert_fabric_edge(
self.db,
a_node_id=nodes[0].id,
b_node_id=nodes[2].id,
a_port="Gi0/1",
b_port="Gi0/0",
source="manual",
)
svc.upsert_fabric_edge(
self.db,
a_node_id=nodes[2].id,
b_node_id=nodes[1].id,
a_port="Gi0/1",
b_port="Gi0/1",
source="manual",
)
self.db.commit()
out = svc.find_fabric_paths(
self.db,
from_managed_ne_id=nes[0].id,
to_managed_ne_id=nes[1].id,
max_paths=2,
max_hops=6,
)
self.assertEqual(out["path_count"], 2)
self.assertEqual(out["paths"][0]["hops"], 1)
self.assertEqual(out["paths"][1]["hops"], 2)
self.assertEqual(len(out["paths"][0]["nodes"]), 2)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()

View file

@ -3370,8 +3370,7 @@ export function TopologyPage() {
y = Math.max(pad, window.innerHeight - rect.height - pad); y = Math.max(pad, window.innerHeight - rect.height - pad);
} }
if (x !== ctxMenu.x || y !== ctxMenu.y) { if (x !== ctxMenu.x || y !== ctxMenu.y) {
el.style.left = `${x}px`; setCtxMenu((prev) => (prev ? { ...prev, x, y } : null));
el.style.top = `${y}px`;
} }
}, [ctxMenu]); }, [ctxMenu]);

View file

@ -175,7 +175,7 @@ export function WorkbenchPage() {
return ( return (
<div className="workbench"> <div className="workbench">
<section className="wb-hero" aria-label={t("workbench.title")}> <section className="wb-hero" aria-label={t("workbench.title")}>
<div className="wb-hero__clock" aria-live="polite"> <div className="wb-hero__clock">
<time className="wb-hero__clock-panel" dateTime={now.toISOString()}> <time className="wb-hero__clock-panel" dateTime={now.toISOString()}>
<span className="wb-hero__clock-digits" aria-hidden="true"> <span className="wb-hero__clock-digits" aria-hidden="true">
<span className="wb-hero__digit">{hh[0]}</span> <span className="wb-hero__digit">{hh[0]}</span>

View file

@ -446,10 +446,8 @@ export default function TopologyReactFlowView(props: TopologyReactFlowViewProps)
onEdgeContextMenu={(e, edge) => { onEdgeContextMenu={(e, edge) => {
e.preventDefault(); e.preventDefault();
focusEdge(edge.id); focusEdge(edge.id);
const pos = placeCtxMenu(e.clientX, e.clientY, { // Modest estimate; useLayoutEffect measures real height and clamps into state.
w: 280, const pos = placeCtxMenu(e.clientX, e.clientY, { w: 280, h: 320 });
h: Math.min(560, Math.max(280, window.innerHeight - 24)),
});
setCtxMenu({ kind: "edge", id: edge.id, ...pos }); setCtxMenu({ kind: "edge", id: edge.id, ...pos });
}} }}
onSelectionContextMenu={(e) => { onSelectionContextMenu={(e) => {