fix(build): exclude global MCP ids from legacy-id detection (#2408)

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:

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)."""