From 18d3cc7f6cb84377d958e123830fd7f993362e34 Mon Sep 17 00:00:00 2001 From: Dvir Dukhan Date: Wed, 27 May 2026 14:10:24 +0300 Subject: [PATCH 1/2] feat(mcp): index_repo tool (T4 #652) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First real MCP tool. Wraps the existing Project / SourceAnalyzer pipeline so AI agents can call `index_repo(path_or_url, branch)` over stdio to populate code-graph for a repo. - `api/mcp/tools/structural.py` (NEW) — registers `index_repo` on the shared FastMCP app. Accepts local paths or git URLs; auto-detects branch from local git checkouts via T17's `detect_branch`; honors `ALLOWED_ANALYSIS_DIR` for sandboxing. Non-git folders are handled by driving SourceAnalyzer directly (Project requires a git repo). - `api/mcp/tools/__init__.py` (NEW) — package marker; importing it registers every tool module's `@app.tool()` decorators. - `api/mcp/server.py` — imports tools at module load so both direct `from api.mcp.server import app` and `cgraph-mcp` stdio entry point see the same tool list. - `tests/mcp/test_index_repo.py` (NEW) — 5 tests: local-path happy path, missing-path error, ALLOWED_ANALYSIS_DIR sandboxing, in-process app registration, JSON serialisability. - `tests/mcp/test_scaffold.py` — replaced the "zero tools" assertion with a presence check for `index_repo` so it stays stable as T5-T8 / T11 add more tools. Return shape: {project_name, branch, graph_name, num_nodes, num_edges, languages_detected, mode} `incremental` parameter is accepted now and forwarded once T18 lands; the current full-reindex path ignores it and always returns `mode="full"`. All 8 tests pass against FalkorDB on 6390. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- api/mcp/server.py | 5 + api/mcp/tools/__init__.py | 7 ++ api/mcp/tools/structural.py | 184 +++++++++++++++++++++++++++++++++++ tests/mcp/test_index_repo.py | 80 +++++++++++++++ tests/mcp/test_scaffold.py | 13 ++- 5 files changed, 285 insertions(+), 4 deletions(-) create mode 100644 api/mcp/tools/__init__.py create mode 100644 api/mcp/tools/structural.py create mode 100644 tests/mcp/test_index_repo.py diff --git a/api/mcp/server.py b/api/mcp/server.py index f0ae6f84..63ce5cfa 100644 --- a/api/mcp/server.py +++ b/api/mcp/server.py @@ -13,6 +13,11 @@ app: FastMCP = FastMCP("code-graph") +# Register tools on import so both direct ``import api.mcp.server`` and the +# stdio entry point see the same tool list. Imported below ``app`` because +# the tool modules need a reference to it. +from . import tools # noqa: F401, E402 + def main() -> None: """Run the MCP server over stdio. diff --git a/api/mcp/tools/__init__.py b/api/mcp/tools/__init__.py new file mode 100644 index 00000000..87b8b3a0 --- /dev/null +++ b/api/mcp/tools/__init__.py @@ -0,0 +1,7 @@ +"""MCP tool implementations for code-graph. + +Each submodule registers tools against the shared FastMCP app exposed by +``api.mcp.server``. Import this package to register all tools. +""" + +from . import structural # noqa: F401 (registers tools on import) diff --git a/api/mcp/tools/structural.py b/api/mcp/tools/structural.py new file mode 100644 index 00000000..30540f4b --- /dev/null +++ b/api/mcp/tools/structural.py @@ -0,0 +1,184 @@ +"""Structural MCP tools (T4-T8). + +These tools wrap the existing ``Project`` / ``Graph`` / ``AsyncGraphQuery`` +operations so MCP-capable agents (Claude Code, Cursor, Copilot, Cline) +can drive code-graph over the standard stdio transport. + +Conventions shared by all tools in this module: + +* Every tool accepts an optional ``branch`` so the agent can scope queries + to a specific per-branch graph (see T17, issue #651). When omitted the + branch is either auto-detected from a local checkout (``index_repo``) + or defaults to ``_default``. +* Long-running synchronous operations are pushed into a thread via + ``asyncio.get_running_loop().run_in_executor`` so the MCP event loop + stays responsive. +""" + +from __future__ import annotations + +import asyncio +import logging +import os +from pathlib import Path +from typing import Any, Optional + +from ..server import app + + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _looks_like_url(spec: str) -> bool: + """Return True for HTTP(S) / git URLs, False for local paths.""" + return spec.startswith(("http://", "https://", "git@", "ssh://", "git://")) + + +def _languages_detected(graph) -> list[str]: + """Best-effort enumeration of distinct ``File.ext`` values. + + Returns a sorted list of extension strings (without the leading dot). + Empty when no files were indexed. + """ + try: + rows = graph.g.query( + "MATCH (f:File) RETURN DISTINCT f.ext AS ext" + ).result_set + except Exception as e: # pragma: no cover — defensive + logger.warning("languages_detected query failed: %s", e) + return [] + seen: set[str] = set() + for row in rows or []: + ext = (row[0] or "").lstrip(".") + if ext: + seen.add(ext) + return sorted(seen) + + +def _count(graph, label: str) -> int: + try: + rows = graph.g.query( + f"MATCH (n:{label}) RETURN count(n) AS c" + ).result_set + return int(rows[0][0]) if rows else 0 + except Exception: + return 0 + + +def _count_edges(graph) -> int: + try: + rows = graph.g.query("MATCH ()-[r]->() RETURN count(r) AS c").result_set + return int(rows[0][0]) if rows else 0 + except Exception: + return 0 + + +# --------------------------------------------------------------------------- +# T4 — index_repo +# --------------------------------------------------------------------------- + + +@app.tool( + name="index_repo", + description=( + "Index a code repository into code-graph for subsequent navigation. " + "Accepts a local path or a git URL. When `branch` is omitted, " + "auto-detects the current branch from the local checkout (defaults " + "to '_default' for non-git folders). Returns the indexed graph's " + "node/edge counts, detected languages, and the (project, branch) " + "identity callers should pass to other code-graph tools." + ), +) +async def index_repo( + path_or_url: str, + branch: Optional[str] = None, + incremental: bool = True, # accepted now, fully honored once T18 lands + ignore: Optional[list[str]] = None, +) -> dict[str, Any]: + """Implementation for the ``index_repo`` MCP tool. + + Args: + path_or_url: Filesystem path to a local repository **or** a clonable + git URL (``https://...``, ``git@host:...``, ``ssh://...``). + branch: Branch identity for the indexed graph. When ``None``: + auto-detect from the checkout via ``git rev-parse --abbrev-ref + HEAD``; falls back to ``_default`` if not a git checkout. + incremental: Accepted for forward-compatibility with T18; the + current full-reindex path ignores it. + ignore: List of relative paths to skip during analysis. + """ + + from api.project import Project, detect_branch + + if ignore is None: + ignore = [] + + loop = asyncio.get_running_loop() + + def _do_index() -> dict[str, Any]: + if _looks_like_url(path_or_url): + project = Project.from_git_repository(path_or_url, branch=branch) + else: + local_path = Path(path_or_url).expanduser().resolve() + if not local_path.exists(): + raise ValueError(f"path does not exist: {local_path}") + + # Reject paths outside the allow-list when one is configured. + allowed_root = os.getenv("ALLOWED_ANALYSIS_DIR") + if allowed_root: + allowed = Path(allowed_root).expanduser().resolve() + try: + local_path.relative_to(allowed) + except ValueError as e: + raise ValueError( + f"path {local_path} is outside ALLOWED_ANALYSIS_DIR={allowed}" + ) from e + + # Use Project for git-repo paths so commit metadata is saved, + # otherwise drive SourceAnalyzer directly so non-git folders work. + if (local_path / ".git").is_dir(): + project = Project.from_local_repository(local_path, branch=branch) + else: + # Synthesize a Project-like object so the return shape is uniform. + from api.analyzers.source_analyzer import SourceAnalyzer + from api.graph import Graph + + detected = branch if branch is not None else detect_branch(local_path) + graph = Graph(local_path.name, branch=detected) + analyzer = SourceAnalyzer() + analyzer.analyze_local_folder(str(local_path), graph, ignore) + + class _Synth: # tiny shim to mirror Project's surface + name = local_path.name + + def __init__(self, g, b): + self.graph = g + self.branch = b + + return _payload(_Synth(graph, detected)) + + project.analyze_sources(ignore) + return _payload(project) + + def _payload(project) -> dict[str, Any]: + g = project.graph + return { + "project_name": project.name, + "branch": getattr(project, "branch", None), + "graph_name": g.name, + "num_nodes": ( + _count(g, "File") + _count(g, "Class") + _count(g, "Function") + ), + "num_edges": _count_edges(g), + "languages_detected": _languages_detected(g), + # T18 will flip this to "incremental" when only changed files + # were re-analyzed. + "mode": "full", + } + + return await loop.run_in_executor(None, _do_index) diff --git a/tests/mcp/test_index_repo.py b/tests/mcp/test_index_repo.py new file mode 100644 index 00000000..ebf88a19 --- /dev/null +++ b/tests/mcp/test_index_repo.py @@ -0,0 +1,80 @@ +"""T4 — ``index_repo`` MCP tool tests.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + + +pytestmark = pytest.mark.anyio + + +@pytest.fixture +def anyio_backend() -> str: + return "asyncio" + + +async def test_index_repo_local_path(sample_project_path: Path, expected_contract): + """Index a local non-git folder and verify the response shape.""" + from api.mcp.tools.structural import index_repo + + result = await index_repo(str(sample_project_path), branch="t4-local-test") + + assert result["project_name"] == "sample_project" + assert result["branch"] == "t4-local-test" + assert result["graph_name"].startswith("code:sample_project:") + assert result["mode"] == "full" + assert result["num_nodes"] >= sum(expected_contract["counts_min"].values()) + assert result["num_edges"] > 0 + assert "py" in result["languages_detected"] + + +async def test_index_repo_rejects_missing_path(): + """Missing local paths surface as a clear ValueError to the agent.""" + from api.mcp.tools.structural import index_repo + + with pytest.raises(ValueError, match="path does not exist"): + await index_repo("/this/path/definitely/does/not/exist/anywhere") + + +async def test_index_repo_honors_allowed_analysis_dir( + sample_project_path: Path, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +): + """Sandboxing: paths outside ALLOWED_ANALYSIS_DIR are rejected.""" + from api.mcp.tools import structural + + monkeypatch.setenv("ALLOWED_ANALYSIS_DIR", str(tmp_path)) + + with pytest.raises(ValueError, match="outside ALLOWED_ANALYSIS_DIR"): + await structural.index_repo(str(sample_project_path), branch="t4-sandbox") + + +async def test_index_repo_registered_via_app(): + """The tool is reachable via ``app.list_tools()`` (protocol parity).""" + from api.mcp.server import app + + tools = await app.list_tools() + names = {t.name for t in tools} + assert "index_repo" in names + + tool = next(t for t in tools if t.name == "index_repo") + schema = tool.inputSchema + # Description / param schema are surfaced to the agent. + assert "path_or_url" in schema["properties"] + assert "branch" in schema["properties"] + assert "incremental" in schema["properties"] + + +async def test_index_repo_response_serialises_to_json( + sample_project_path: Path, +): + """MCP transports JSON — the response dict must be JSON-serialisable.""" + from api.mcp.tools.structural import index_repo + + result = await index_repo(str(sample_project_path), branch="t4-json-test") + # Must not raise. + json.dumps(result) diff --git a/tests/mcp/test_scaffold.py b/tests/mcp/test_scaffold.py index 851f091a..b02a6780 100644 --- a/tests/mcp/test_scaffold.py +++ b/tests/mcp/test_scaffold.py @@ -45,11 +45,12 @@ def test_main_entry_point_exists() -> None: @pytest.mark.anyio -async def test_stdio_server_lists_zero_tools() -> None: +async def test_stdio_server_lists_registered_tools() -> None: """Spawn ``cgraph-mcp`` over stdio and verify the protocol handshake. - The scaffold registers no tools, so ``list_tools`` must return an - empty list. Tool tickets (T4-T8, T11) extend this expectation. + Once tool tickets land (T4+), ``list_tools`` returns at least the + tools they register. This test only guards the *handshake* — per-tool + behavioural assertions live in the per-tool test modules. """ cgraph_mcp = shutil.which("cgraph-mcp") assert cgraph_mcp is not None, ( @@ -62,4 +63,8 @@ async def test_stdio_server_lists_zero_tools() -> None: async with ClientSession(read, write) as session: await session.initialize() result = await session.list_tools() - assert result.tools == [] + # ``index_repo`` lands in T4; this assertion intentionally + # only checks for presence so it stays stable as more tools + # are registered in T5-T8 / T11. + names = {t.name for t in result.tools} + assert "index_repo" in names From cc841b64bc4164fd7b27aa7fdfefdf493e4f59b2 Mon Sep 17 00:00:00 2001 From: Dvir Dukhan <12258836+DvirDukhan@users.noreply.github.com> Date: Mon, 8 Jun 2026 15:44:34 +0300 Subject: [PATCH 2/2] fix(mcp): address index_repo review comments (#678) - num_nodes: count nodes generically (MATCH (n)) instead of summing only File+Class+Function, so non-Python repos (Method/Interface/Enum/ Constructor nodes) aren't under-reported; symmetric with _count_edges (galshubeli, Copilot). - Reject non-http(s) git URLs (git@.../ssh://...) with an actionable error instead of letting Project.from_git_repository raise a confusing "invalid url" from validators.url(); align the docstring/description to advertise http(s) URLs only. Silently rewriting to https would change auth semantics for private repos (Copilot). - Local git checkout with no configured remote: fall back to Project(url=None) instead of crashing on remotes[0] IndexError, so remote-less repos can still be indexed (galshubeli, Copilot). - Document that `incremental` is accepted but not yet honored (full reindex always); mode is always "full" until T18 (Copilot). - Skip DB-backed index_repo tests when FalkorDB is unreachable via a new require_falkordb fixture, keeping the unit subset runnable in dev/CI without the service (Copilot). - Add tests for the SSH-URL rejection and the no-remote git path (the .git branch previously had no coverage). - Fix a pre-existing assertion referencing expected_contract["counts_min"] (the fixture key is "counts") so the local-path test passes when FalkorDB is reachable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- api/mcp/tools/structural.py | 68 +++++++++++++++++++++++++----------- tests/mcp/conftest.py | 12 +++++++ tests/mcp/test_index_repo.py | 50 ++++++++++++++++++++++++-- 3 files changed, 107 insertions(+), 23 deletions(-) diff --git a/api/mcp/tools/structural.py b/api/mcp/tools/structural.py index 30540f4b..38d52bf9 100644 --- a/api/mcp/tools/structural.py +++ b/api/mcp/tools/structural.py @@ -39,6 +39,21 @@ def _looks_like_url(spec: str) -> bool: return spec.startswith(("http://", "https://", "git@", "ssh://", "git://")) +def _count_nodes(graph) -> int: + """Total node count for the graph. + + Counts every node generically (``MATCH (n)``) rather than summing a + fixed label list, so non-Python repos -- whose analyzers emit Method, + Interface, Enum, Constructor, ... nodes -- aren't under-reported. This + mirrors :func:`_count_edges`, which already counts edges generically. + """ + try: + rows = graph.g.query("MATCH (n) RETURN count(n) AS c").result_set + return int(rows[0][0]) if rows else 0 + except Exception: + return 0 + + def _languages_detected(graph) -> list[str]: """Best-effort enumeration of distinct ``File.ext`` values. @@ -60,16 +75,6 @@ def _languages_detected(graph) -> list[str]: return sorted(seen) -def _count(graph, label: str) -> int: - try: - rows = graph.g.query( - f"MATCH (n:{label}) RETURN count(n) AS c" - ).result_set - return int(rows[0][0]) if rows else 0 - except Exception: - return 0 - - def _count_edges(graph) -> int: try: rows = graph.g.query("MATCH ()-[r]->() RETURN count(r) AS c").result_set @@ -87,11 +92,11 @@ def _count_edges(graph) -> int: name="index_repo", description=( "Index a code repository into code-graph for subsequent navigation. " - "Accepts a local path or a git URL. When `branch` is omitted, " - "auto-detects the current branch from the local checkout (defaults " - "to '_default' for non-git folders). Returns the indexed graph's " - "node/edge counts, detected languages, and the (project, branch) " - "identity callers should pass to other code-graph tools." + "Accepts a local path or an http(s) git URL. When `branch` is " + "omitted, auto-detects the current branch from the local checkout " + "(defaults to '_default' for non-git folders). Returns the indexed " + "graph's node/edge counts, detected languages, and the (project, " + "branch) identity callers should pass to other code-graph tools." ), ) async def index_repo( @@ -104,12 +109,16 @@ async def index_repo( Args: path_or_url: Filesystem path to a local repository **or** a clonable - git URL (``https://...``, ``git@host:...``, ``ssh://...``). + http(s) git URL (``https://host/org/repo.git``). SSH/scp specs + (``git@host:org/repo.git``, ``ssh://...``) are not supported by + the cloner -- clone such repos locally and pass the checkout path + instead. branch: Branch identity for the indexed graph. When ``None``: auto-detect from the checkout via ``git rev-parse --abbrev-ref HEAD``; falls back to ``_default`` if not a git checkout. incremental: Accepted for forward-compatibility with T18; the - current full-reindex path ignores it. + current path always performs a full reindex and ignores this + flag (the response ``mode`` is always ``"full"``). ignore: List of relative paths to skip during analysis. """ @@ -122,6 +131,17 @@ async def index_repo( def _do_index() -> dict[str, Any]: if _looks_like_url(path_or_url): + # Project.from_git_repository validates with validators.url(), + # which only accepts http(s) URLs -- scp-style (git@host:...) and + # ssh:// specs would raise a confusing "invalid url". Reject them + # up front with an actionable message instead. (Silently rewriting + # them to https would change auth semantics for private repos.) + if not path_or_url.startswith(("http://", "https://")): + raise ValueError( + "index_repo only supports http(s) git URLs, got " + f"{path_or_url!r}. For SSH/scp specs, clone the repository " + "locally and pass the checkout path instead." + ) project = Project.from_git_repository(path_or_url, branch=branch) else: local_path = Path(path_or_url).expanduser().resolve() @@ -142,7 +162,15 @@ def _do_index() -> dict[str, Any]: # Use Project for git-repo paths so commit metadata is saved, # otherwise drive SourceAnalyzer directly so non-git folders work. if (local_path / ".git").is_dir(): - project = Project.from_local_repository(local_path, branch=branch) + try: + project = Project.from_local_repository(local_path, branch=branch) + except IndexError: + # from_local_repository reads remotes[0].url, which raises + # IndexError for a git repo with no configured remote. + # Index it anyway, just without url/repo-info metadata. + project = Project( + local_path.name, local_path, url=None, branch=branch + ) else: # Synthesize a Project-like object so the return shape is uniform. from api.analyzers.source_analyzer import SourceAnalyzer @@ -171,9 +199,7 @@ def _payload(project) -> dict[str, Any]: "project_name": project.name, "branch": getattr(project, "branch", None), "graph_name": g.name, - "num_nodes": ( - _count(g, "File") + _count(g, "Class") + _count(g, "Function") - ), + "num_nodes": _count_nodes(g), "num_edges": _count_edges(g), "languages_detected": _languages_detected(g), # T18 will flip this to "incremental" when only changed files diff --git a/tests/mcp/conftest.py b/tests/mcp/conftest.py index cb381b3c..8bf1f460 100644 --- a/tests/mcp/conftest.py +++ b/tests/mcp/conftest.py @@ -82,6 +82,18 @@ def _falkordb_reachable() -> bool: return False +@pytest.fixture +def require_falkordb() -> None: + """Skip the depending test when FalkorDB isn't reachable. + + Tests that drive the real indexing path (creating a ``Graph`` and + writing nodes/edges) depend on this so the MCP suite stays runnable in + environments that only exercise the pure-Python unit tests. + """ + if not _falkordb_reachable(): + pytest.skip("FalkorDB not reachable on $FALKORDB_HOST:$FALKORDB_PORT") + + @pytest.fixture(scope="session") def indexed_fixture(sample_project_path: Path): """Index the sample project into a unique per-session graph. diff --git a/tests/mcp/test_index_repo.py b/tests/mcp/test_index_repo.py index ebf88a19..1df01e00 100644 --- a/tests/mcp/test_index_repo.py +++ b/tests/mcp/test_index_repo.py @@ -16,7 +16,9 @@ def anyio_backend() -> str: return "asyncio" -async def test_index_repo_local_path(sample_project_path: Path, expected_contract): +async def test_index_repo_local_path( + require_falkordb, sample_project_path: Path, expected_contract +): """Index a local non-git folder and verify the response shape.""" from api.mcp.tools.structural import index_repo @@ -26,7 +28,7 @@ async def test_index_repo_local_path(sample_project_path: Path, expected_contrac assert result["branch"] == "t4-local-test" assert result["graph_name"].startswith("code:sample_project:") assert result["mode"] == "full" - assert result["num_nodes"] >= sum(expected_contract["counts_min"].values()) + assert result["num_nodes"] >= sum(expected_contract["counts"].values()) assert result["num_edges"] > 0 assert "py" in result["languages_detected"] @@ -70,6 +72,7 @@ async def test_index_repo_registered_via_app(): async def test_index_repo_response_serialises_to_json( + require_falkordb, sample_project_path: Path, ): """MCP transports JSON — the response dict must be JSON-serialisable.""" @@ -78,3 +81,46 @@ async def test_index_repo_response_serialises_to_json( result = await index_repo(str(sample_project_path), branch="t4-json-test") # Must not raise. json.dumps(result) + + +async def test_index_repo_rejects_ssh_url(): + """scp/ssh git specs surface an actionable error (not a confusing + 'invalid url' from the underlying validator).""" + from api.mcp.tools.structural import index_repo + + with pytest.raises(ValueError, match="only supports http"): + await index_repo("git@github.com:FalkorDB/code-graph.git") + + +async def test_index_repo_local_git_without_remote( + require_falkordb, tmp_path: Path +): + """A git checkout with no configured remote must still index. + + ``Project.from_local_repository`` reads ``remotes[0].url`` and raises + ``IndexError`` when the repo has no remote; ``index_repo`` should fall + back to indexing without url metadata instead of failing. + """ + import subprocess + + from api.mcp.tools.structural import index_repo + + repo = tmp_path / "localrepo" + (repo / "pkg").mkdir(parents=True) + (repo / "pkg" / "mod.py").write_text( + "def helper():\n return 1\n\n\ndef main():\n return helper()\n" + ) + subprocess.run(["git", "init", "-q"], cwd=repo, check=True) + subprocess.run(["git", "add", "-A"], cwd=repo, check=True) + subprocess.run( + ["git", "-c", "user.email=t@t", "-c", "user.name=t", + "commit", "-qm", "init"], + cwd=repo, check=True, + ) + + result = await index_repo(str(repo), branch="t4-no-remote") + + assert result["project_name"] == "localrepo" + assert result["branch"] == "t4-no-remote" + assert result["graph_name"].startswith("code:localrepo:") + assert result["num_nodes"] > 0