From 48fbe30ebc69c8e23cc7ccd0ce0015ea34673d4a Mon Sep 17 00:00:00 2001 From: Aryan Date: Thu, 13 Aug 2026 14:17:42 +0100 Subject: [PATCH] fix(build): exclude global MCP ids from legacy-id detection (#2408) --- graphify/build.py | 30 +++++++++++++++++++++-- tests/test_build.py | 59 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index 3f7aa8fc..7f195fe2 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -668,6 +668,22 @@ def _semantic_id_remap(nodes: list, root: str | None) -> dict: return remap +# MCP node kinds whose ID is GLOBAL by design — deliberately shared across every +# config file that mentions them (`mcp_command_npx`, `mcp_package_...`, +# `env_var_...`), so it is not derived from ``source_file`` at all (#2408). The +# file-scoped kinds (`mcp_config_file`, `mcp_server`) ARE stem-derived and stay +# subject to legacy detection. +_MCP_GLOBAL_ID_KINDS = frozenset({"mcp_command", "mcp_package", "env_var"}) + + +def _has_global_id(node: dict) -> bool: + """Whether ``node``'s ID is global by construction rather than file-derived.""" + meta = node.get("metadata") + if not isinstance(meta, dict): + return False + return meta.get("mcp_kind") in _MCP_GLOBAL_ID_KINDS + + def graph_has_legacy_ids(nodes: list, root: str | Path | None = None, sample: int = 300) -> bool: """Whether a loaded graph still uses pre-#1504 node IDs (parent-dir / filename stem) rather than the full repo-relative path. Read-only consumers (query, @@ -677,8 +693,10 @@ def graph_has_legacy_ids(nodes: list, root: str | Path | None = None, sample: in inspected, because their ID is unambiguously the file stem. Symbol nodes are skipped — some extractors scope a symbol by package/directory (Go's ``_make_id(pkg_dir, name)`` → ``sub_thing``), which can coincide with an old - file-stem form and would otherwise false-positive. Returns True as soon as one - file node's ID matches an OLD stem form but not the canonical full-path form.""" + file-stem form and would otherwise false-positive. Nodes whose ID is global by + construction (see ``_MCP_GLOBAL_ID_KINDS``) are skipped for the same reason. + Returns True as soon as one file node's ID matches an OLD stem form but not the + canonical full-path form.""" from graphify.extractors.base import _file_stem _r = str(root) if root is not None else None checked = 0 @@ -687,6 +705,14 @@ def graph_has_legacy_ids(nodes: list, root: str | Path | None = None, sample: in continue if str(node.get("source_location") or "") != "L1": continue # only file-level nodes carry an unambiguous file-stem ID + if _has_global_id(node): + # #2408: MCP ingest stamps every node it emits with line 1 (JSON has no + # line info), so globally-scoped nodes slip past the L1 proxy for + # "file-level". For `sub/.mcp.json` the old bare stem is `mcp` while the + # canonical stem is `sub_mcp`, so a perfectly valid `mcp_command_npx` + # reads as a legacy `mcp_`-prefixed id and warns on every fresh build. + # (A root-level `.mcp.json` never tripped it: there `mcp` IS canonical.) + continue nid = node.get("id") sf = node.get("source_file") if not nid or not isinstance(nid, str) or not sf: diff --git a/tests/test_build.py b/tests/test_build.py index 416e17bc..eec628a0 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -1302,6 +1302,65 @@ def test_graph_has_legacy_ids_detects_old_scheme(): assert graph_has_legacy_ids(go_symbol, root=".") is False +# ── #2408: globally-scoped MCP node ids are not file-stem derived ────────────── + +@pytest.mark.parametrize("mcp_kind, nid", [ + ("mcp_command", "mcp_command_npx"), + ("mcp_package", "mcp_package_google_cloud_cloud_run_mcp"), + ("env_var", "env_var_google_cloud_project"), +]) +def test_graph_has_legacy_ids_ignores_global_mcp_ids(mcp_kind, nid): + """MCP ingest stamps every node with L1 (JSON has no line info), so global ids + would otherwise be read as file-level. Under `sub/.mcp.json` the old bare stem + is `mcp`, which these ids legitimately start with (#2408).""" + from graphify.build import graph_has_legacy_ids + node = { + "id": nid, + "source_file": "sub/.mcp.json", + "source_location": "L1", + "metadata": {"mcp_kind": mcp_kind}, + } + assert graph_has_legacy_ids([node], root=".") is False + + +def test_graph_has_legacy_ids_still_checks_file_scoped_mcp_nodes(): + """The exemption is narrow: file-derived MCP kinds stay under detection, and a + missing/malformed metadata blob doesn't exempt anything (or crash).""" + from graphify.build import graph_has_legacy_ids + for kind in ("mcp_config_file", "mcp_server"): + stale = {"id": "mcp_mcp_server_x", "source_file": "sub/.mcp.json", + "source_location": "L1", "metadata": {"mcp_kind": kind}} + assert graph_has_legacy_ids([stale], root=".") is True + for meta in (None, "not-a-dict", {}, {"mcp_kind": None}): + stale = {"id": "mcp_mcp_server_x", "source_file": "sub/.mcp.json", + "source_location": "L1", "metadata": meta} + assert graph_has_legacy_ids([stale], root=".") is True + + +@pytest.mark.parametrize("mcp_dir", ["", "sub"]) +def test_fresh_mcp_graph_is_not_flagged_legacy(tmp_path, monkeypatch, mcp_dir): + """End-to-end: a freshly extracted graph containing a .mcp.json — nested or at + the repo root — must not nudge the user to rebuild (#2408).""" + from graphify.build import graph_has_legacy_ids + from graphify.extract import extract + + (tmp_path / "main.py").write_text("def main():\n return 1\n") + mcp_parent = tmp_path / mcp_dir if mcp_dir else tmp_path + mcp_parent.mkdir(parents=True, exist_ok=True) + (mcp_parent / ".mcp.json").write_text(json.dumps({"mcpServers": {"cloud-run": { + "command": "npx", + "args": ["-y", "@google-cloud/cloud-run-mcp"], + "env": {"GOOGLE_CLOUD_PROJECT": "x"}, + }}})) + + monkeypatch.chdir(tmp_path) + rel = Path(mcp_dir, ".mcp.json") if mcp_dir else Path(".mcp.json") + result = extract([Path("main.py"), rel], root=Path("."), parallel=False) + ids = {n["id"] for n in result["nodes"]} + assert "mcp_command_npx" in ids # guard: the ingest actually ran + assert graph_has_legacy_ids(result["nodes"], root=".") is False + + def test_semantic_rekey_relative_vs_absolute_source_file(): """Re-key contract: a relative source_file is migrated; an absolute one is left untouched (it can't be relativized, so its on-disk path must not leak into IDs)."""