From 864388f3d232329027c3f5eb369a58a1ea383721 Mon Sep 17 00:00:00 2001 From: Stefan Hudici Date: Sat, 4 Jul 2026 00:33:11 +0300 Subject: [PATCH] Attach graph freshness provenance to MCP tool responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents consuming graph tools have no way to tell whether an answer reflects the current tree: the graph may have been built hours ago, on a different branch, at a different commit. Every response now carries a compact `_graph` envelope: "_graph": { "updated_at": "2026-07-03T18:22:41", "age_seconds": 5121, "built_on_branch": "main", "built_at_sha": "b72413c9d0aa..." } - `graph_provenance()` (tools/_common.py) reads `last_updated`, `git_branch`, `git_head_sha` from the graph's metadata table via a read-only SQLite connection. The db path is escaped with `Path.as_uri()` so URI-significant characters (#, %) in repo paths cannot derail the SQLite URI parser. Best-effort by design: any failure (no graph, unreadable DB, missing metadata) returns None and never fails the tool call. - `with_provenance()` attaches the envelope to dict responses only, skips results that already carry `_graph`, and passes everything else through untouched. - All 27 graph-backed MCP tools in main.py wrap their returns. For the five async tools the wrap runs inside the asyncio.to_thread worker (the envelope read opens the graph DB, and the event loop must never touch disk — #46, #136). Excluded: get_docs_section_tool, list_repos_tool, cross_repo_search_tool (not backed by a single repo graph). 13 new tests covering metadata read, URI-hostile paths (% and # in the repo path), age clamping, unparseable timestamps, optional fields, all no-op paths, and one end-to-end registered-tool response. --- code_review_graph/main.py | 175 ++++++++++++++++------------ code_review_graph/tools/__init__.py | 2 + code_review_graph/tools/_common.py | 64 ++++++++++ tests/test_tools.py | 121 +++++++++++++++++++ 4 files changed, 288 insertions(+), 74 deletions(-) diff --git a/code_review_graph/main.py b/code_review_graph/main.py index 629b1674..e5e247ca 100644 --- a/code_review_graph/main.py +++ b/code_review_graph/main.py @@ -57,6 +57,7 @@ run_postprocess, semantic_search_nodes, traverse_graph_func, + with_provenance, ) logger = logging.getLogger(__name__) @@ -122,14 +123,20 @@ async def build_or_update_graph_tool( recurse_submodules: If True, include files from git submodules. When None (default), falls back to CRG_RECURSE_SUBMODULES env var. """ - return await asyncio.to_thread( - build_or_update_graph, - full_rebuild=full_rebuild, - repo_root=_resolve_repo_root(repo_root), - base=base, - postprocess=postprocess, - recurse_submodules=recurse_submodules, - ) + root = _resolve_repo_root(repo_root) + + def _run() -> dict: + # with_provenance runs inside the worker thread too: it opens the + # graph DB, and the event loop must never touch disk (#46, #136). + return with_provenance(build_or_update_graph( + full_rebuild=full_rebuild, + repo_root=root, + base=base, + postprocess=postprocess, + recurse_submodules=recurse_submodules, + ), root) + + return await asyncio.to_thread(_run) @mcp.tool() @@ -154,11 +161,15 @@ async def run_postprocess_tool( fts: Rebuild FTS index. Default: True. repo_root: Repository root path. Auto-detected if omitted. """ - return await asyncio.to_thread( - run_postprocess, - flows=flows, communities=communities, fts=fts, - repo_root=_resolve_repo_root(repo_root), - ) + root = _resolve_repo_root(repo_root) + + def _run() -> dict: + return with_provenance(run_postprocess( + flows=flows, communities=communities, fts=fts, + repo_root=root, + ), root) + + return await asyncio.to_thread(_run) @mcp.tool() @@ -180,10 +191,10 @@ def get_minimal_context_tool( repo_root: Repository root path. Auto-detected if omitted. base: Git ref for diff comparison. Default: HEAD~1. """ - return get_minimal_context( + return with_provenance(get_minimal_context( task=task, changed_files=changed_files, repo_root=_resolve_repo_root(repo_root), base=base, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -206,10 +217,10 @@ def get_impact_radius_tool( base: Git ref for auto-detecting changes. Default: HEAD~1. detail_level: "standard" for full output, "minimal" for compact summary. Default: standard. """ - return get_impact_radius( + return with_provenance(get_impact_radius( changed_files=changed_files, max_depth=max_depth, repo_root=_resolve_repo_root(repo_root), base=base, detail_level=detail_level, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -237,10 +248,10 @@ def query_graph_tool( repo_root: Repository root path. Auto-detected if omitted. detail_level: "standard" for full output, "minimal" for compact summary. Default: standard. """ - return query_graph( + return with_provenance(query_graph( pattern=pattern, target=target, repo_root=_resolve_repo_root(repo_root), detail_level=detail_level, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -268,11 +279,11 @@ def get_review_context_tool( detail_level: "standard" for full output, "minimal" for token-efficient summary. Default: standard. """ - return get_review_context( + return with_provenance(get_review_context( changed_files=changed_files, max_depth=max_depth, include_source=include_source, max_lines_per_file=max_lines_per_file, repo_root=_resolve_repo_root(repo_root), base=base, detail_level=detail_level, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -305,10 +316,10 @@ def semantic_search_nodes_tool( or "minimax". Must match the provider used during embed_graph. detail_level: "standard" for full output, "minimal" for compact summary. Default: standard. """ - return semantic_search_nodes( + return with_provenance(semantic_search_nodes( query=query, kind=kind, limit=limit, repo_root=_resolve_repo_root(repo_root), model=model, provider=provider, detail_level=detail_level, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -345,12 +356,16 @@ async def embed_graph_tool( CRG_OPENAI_MODEL env vars and accepts any OpenAI-compatible endpoint (real OpenAI, Azure, new-api, LiteLLM, vLLM, etc.). """ - return await asyncio.to_thread( - embed_graph, - repo_root=_resolve_repo_root(repo_root), - model=model, - provider=provider, - ) + root = _resolve_repo_root(repo_root) + + def _run() -> dict: + return with_provenance(embed_graph( + repo_root=root, + model=model, + provider=provider, + ), root) + + return await asyncio.to_thread(_run) @mcp.tool() @@ -365,7 +380,10 @@ def list_graph_stats_tool( Args: repo_root: Repository root path. Auto-detected if omitted. """ - return list_graph_stats(repo_root=_resolve_repo_root(repo_root)) + return with_provenance( + list_graph_stats(repo_root=_resolve_repo_root(repo_root)), + _resolve_repo_root(repo_root), + ) @mcp.tool() @@ -411,10 +429,10 @@ def find_large_functions_tool( limit: Maximum results. Default: 50. repo_root: Repository root path. Auto-detected if omitted. """ - return find_large_functions( + return with_provenance(find_large_functions( min_lines=min_lines, kind=kind, file_path_pattern=file_path_pattern, limit=limit, repo_root=_resolve_repo_root(repo_root), - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -439,10 +457,10 @@ def list_flows_tool( returns only name, criticality, and node_count per flow. repo_root: Repository root path. Auto-detected if omitted. """ - return list_flows( + return with_provenance(list_flows( repo_root=_resolve_repo_root(repo_root), sort_by=sort_by, limit=limit, kind=kind, detail_level=detail_level, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -465,10 +483,10 @@ def get_flow_tool( include_source: Include source code snippets for each step. Default: False. repo_root: Repository root path. Auto-detected if omitted. """ - return get_flow( + return with_provenance(get_flow( flow_id=flow_id, flow_name=flow_name, include_source=include_source, repo_root=_resolve_repo_root(repo_root), - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -488,9 +506,9 @@ def get_affected_flows_tool( base: Git ref for auto-detecting changes. Default: HEAD~1. repo_root: Repository root path. Auto-detected if omitted. """ - return get_affected_flows_func( + return with_provenance(get_affected_flows_func( changed_files=changed_files, base=base, repo_root=_resolve_repo_root(repo_root), - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -514,10 +532,10 @@ def list_communities_tool( per community. repo_root: Repository root path. Auto-detected if omitted. """ - return list_communities_func( + return with_provenance(list_communities_func( repo_root=_resolve_repo_root(repo_root), sort_by=sort_by, min_size=min_size, detail_level=detail_level, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -541,10 +559,10 @@ def get_community_tool( include_members: Include full member node details. Default: False. repo_root: Repository root path. Auto-detected if omitted. """ - return get_community_func( + return with_provenance(get_community_func( community_name=community_name, community_id=community_id, include_members=include_members, repo_root=_resolve_repo_root(repo_root), - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -565,10 +583,10 @@ def get_architecture_overview_tool( community pair (typical reduction: 600KB -> <5KB); "standard" returns full per-edge detail. """ - return get_architecture_overview_func( + return with_provenance(get_architecture_overview_func( repo_root=_resolve_repo_root(repo_root), detail_level=detail_level, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -599,12 +617,16 @@ async def detect_changes_tool( detail_level: "standard" for full output, "minimal" for token-efficient summary. Default: standard. """ - coro = asyncio.to_thread( - detect_changes_func, - base=base, changed_files=changed_files, - include_source=include_source, max_depth=max_depth, - repo_root=_resolve_repo_root(repo_root), detail_level=detail_level, - ) + root = _resolve_repo_root(repo_root) + + def _run() -> dict: + return with_provenance(detect_changes_func( + base=base, changed_files=changed_files, + include_source=include_source, max_depth=max_depth, + repo_root=root, detail_level=detail_level, + ), root) + + coro = asyncio.to_thread(_run) tool_timeout = int(os.environ.get("CRG_TOOL_TIMEOUT", "0")) if tool_timeout > 0: try: @@ -615,11 +637,12 @@ async def detect_changes_tool( "Reduce scope with CRG_MAX_CHANGED_FUNCS / CRG_MAX_TRANSITIVE_FRONTIER, " "or increase CRG_TOOL_TIMEOUT." ) - return { + error_response = { "status": "error", "error": message, "summary": message, } + return await asyncio.to_thread(with_provenance, error_response, root) return await coro @@ -653,10 +676,10 @@ def refactor_tool( file_pattern: (dead_code) Filter by file path substring. repo_root: Repository root path. Auto-detected if omitted. """ - return refactor_func( + return with_provenance(refactor_func( mode=mode, old_name=old_name, new_name=new_name, kind=kind, file_pattern=file_pattern, repo_root=_resolve_repo_root(repo_root), - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -683,10 +706,10 @@ def apply_refactor_tool( dry_run. Use this for a human-in-the-loop review before committing changes to disk. See: #176 """ - return apply_refactor_func( + return with_provenance(apply_refactor_func( refactor_id=refactor_id, repo_root=_resolve_repo_root(repo_root), dry_run=dry_run, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -708,11 +731,15 @@ async def generate_wiki_tool( repo_root: Repository root path. Auto-detected if omitted. force: If True, regenerate all pages even if content unchanged. Default: False. """ - return await asyncio.to_thread( - generate_wiki_func, - repo_root=_resolve_repo_root(repo_root), - force=force, - ) + root = _resolve_repo_root(repo_root) + + def _run() -> dict: + return with_provenance(generate_wiki_func( + repo_root=root, + force=force, + ), root) + + return await asyncio.to_thread(_run) @mcp.tool() @@ -729,9 +756,9 @@ def get_wiki_page_tool( community_name: Community name to look up. repo_root: Repository root path. Auto-detected if omitted. """ - return get_wiki_page_func( + return with_provenance(get_wiki_page_func( community_name=community_name, repo_root=_resolve_repo_root(repo_root), - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -748,9 +775,9 @@ def get_hub_nodes_tool( top_n: Number of top hubs to return. Default: 10. repo_root: Repository root path. Auto-detected if omitted. """ - return get_hub_nodes_func( + return with_provenance(get_hub_nodes_func( repo_root=_resolve_repo_root(repo_root), top_n=top_n, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -768,9 +795,9 @@ def get_bridge_nodes_tool( top_n: Number of top bridges to return. Default: 10. repo_root: Repository root path. Auto-detected if omitted. """ - return get_bridge_nodes_func( + return with_provenance(get_bridge_nodes_func( repo_root=_resolve_repo_root(repo_root), top_n=top_n, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -786,9 +813,9 @@ def get_knowledge_gaps_tool( Args: repo_root: Repository root path. Auto-detected if omitted. """ - return get_knowledge_gaps_func( + return with_provenance(get_knowledge_gaps_func( repo_root=_resolve_repo_root(repo_root), - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -806,9 +833,9 @@ def get_surprising_connections_tool( top_n: Number of top surprises to return. Default: 15. repo_root: Repository root path. Auto-detected if omitted. """ - return get_surprising_connections_func( + return with_provenance(get_surprising_connections_func( repo_root=_resolve_repo_root(repo_root), top_n=top_n, - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -824,9 +851,9 @@ def get_suggested_questions_tool( Args: repo_root: Repository root path. Auto-detected if omitted. """ - return get_suggested_questions_func( + return with_provenance(get_suggested_questions_func( repo_root=_resolve_repo_root(repo_root), - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() @@ -852,11 +879,11 @@ def traverse_graph_tool( Default: 2000. repo_root: Repository root path. Auto-detected if omitted. """ - return traverse_graph_func( + return with_provenance(traverse_graph_func( query=query, mode=mode, depth=depth, token_budget=token_budget, repo_root=_resolve_repo_root(repo_root) or "", - ) + ), _resolve_repo_root(repo_root)) @mcp.tool() diff --git a/code_review_graph/tools/__init__.py b/code_review_graph/tools/__init__.py index 385e289c..b220d5db 100644 --- a/code_review_graph/tools/__init__.py +++ b/code_review_graph/tools/__init__.py @@ -49,6 +49,7 @@ _BUILTIN_CALL_NAMES, _get_store, _validate_repo_root, + with_provenance, ) # -- analysis_tools --------------------------------------------------------- @@ -107,6 +108,7 @@ "_BUILTIN_CALL_NAMES", "_get_store", "_validate_repo_root", + "with_provenance", # build "build_or_update_graph", "run_postprocess", diff --git a/code_review_graph/tools/_common.py b/code_review_graph/tools/_common.py index a44fb0ca..35167cde 100644 --- a/code_review_graph/tools/_common.py +++ b/code_review_graph/tools/_common.py @@ -2,6 +2,8 @@ from __future__ import annotations +import sqlite3 +from datetime import datetime from pathlib import Path from typing import Any @@ -15,6 +17,68 @@ def _error_response( """Build a standardised error response dict.""" return {"status": status, "error": message, "summary": message, **extra} + +def graph_provenance(repo_root: str | None = None) -> dict[str, Any] | None: + """Freshness/provenance envelope for a repo's graph, or None. + + Reads build metadata (when the graph was last updated, and on which + git branch/commit it was built) so every tool response can carry the + context an agent needs to judge whether the answer is stale — a graph + built three days ago on another branch answers questions about *that* + tree, not the current one. + + Best-effort by design: any failure (no graph, unreadable DB, missing + metadata) returns None and must never fail the tool call itself. + """ + try: + root = _resolve_root(repo_root) + db_path = get_db_path(root) + if not db_path.exists(): + return None + # as_uri() percent-escapes URI-significant characters (#, %, ?) + # that a plain f-string would hand to SQLite's URI parser raw. + conn = sqlite3.connect(f"{db_path.resolve().as_uri()}?mode=ro", uri=True) + try: + rows = dict(conn.execute( + "SELECT key, value FROM metadata WHERE key IN " + "('last_updated', 'git_branch', 'git_head_sha')" + ).fetchall()) + finally: + conn.close() + updated_at = rows.get("last_updated") + if not updated_at: + return None + provenance: dict[str, Any] = {"updated_at": updated_at} + try: + # Stored via time.strftime("%Y-%m-%dT%H:%M:%S") in local time. + built = datetime.fromisoformat(updated_at) + provenance["age_seconds"] = max( + 0, int((datetime.now() - built).total_seconds()), + ) + except ValueError: + pass + if rows.get("git_branch"): + provenance["built_on_branch"] = rows["git_branch"] + if rows.get("git_head_sha"): + provenance["built_at_sha"] = rows["git_head_sha"] + return provenance + except Exception: + return None + + +def with_provenance(result: Any, repo_root: str | None = None) -> Any: + """Attach a ``_graph`` provenance envelope to a tool response dict. + + No-op for non-dict results, results that already carry ``_graph``, + and repos where provenance cannot be determined. + """ + if not isinstance(result, dict) or "_graph" in result: + return result + provenance = graph_provenance(repo_root) + if provenance: + result["_graph"] = provenance + return result + # Common JS/TS builtin method names filtered from callers_of results. # "Who calls .map()?" returns hundreds of hits and is never useful. # These are kept in the graph (callees_of still shows them) but excluded diff --git a/tests/test_tools.py b/tests/test_tools.py index 578536d4..6090a24b 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -1563,3 +1563,124 @@ def test_task_routing_refactor(self): task="refactor auth module", repo_root=str(self.root), ) assert "refactor" in result["next_tool_suggestions"] + + +class TestProvenance: + """graph_provenance / with_provenance freshness envelope (_common.py).""" + + def _make_repo(self, tmp_path, metadata=None): + repo = tmp_path / "repo" + repo.mkdir() + (repo / ".git").mkdir() + graph_dir = repo / ".code-review-graph" + graph_dir.mkdir() + store = GraphStore(graph_dir / "graph.db") + try: + store.upsert_node(NodeInfo( + kind="Function", name="handle", file_path="src/app.py", + line_start=1, line_end=3, language="python", + )) + for key, value in (metadata or {}).items(): + store.set_metadata(key, value) + store.commit() + finally: + store.close() + return repo + + def test_reads_build_metadata(self, tmp_path): + repo = self._make_repo(tmp_path, { + "last_updated": "2026-01-02T03:04:05", + "git_branch": "feature/x", + "git_head_sha": "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0", + }) + prov = common_module.graph_provenance(str(repo)) + assert prov["updated_at"] == "2026-01-02T03:04:05" + assert prov["built_on_branch"] == "feature/x" + assert prov["built_at_sha"] == "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0" + assert prov["age_seconds"] >= 0 + + def test_reads_metadata_from_uri_hostile_path(self, tmp_path): + # '%' and '#' are significant in SQLite URIs; the read-only + # connection must percent-escape the db path (via as_uri). + hostile = tmp_path / "repo %40 #frag" + hostile.mkdir() + (hostile / ".git").mkdir() + graph_dir = hostile / ".code-review-graph" + graph_dir.mkdir() + store = GraphStore(graph_dir / "graph.db") + try: + store.set_metadata("last_updated", "2026-01-02T03:04:05") + store.commit() + finally: + store.close() + prov = common_module.graph_provenance(str(hostile)) + assert prov is not None + assert prov["updated_at"] == "2026-01-02T03:04:05" + + def test_future_timestamp_clamps_age_to_zero(self, tmp_path): + repo = self._make_repo(tmp_path, {"last_updated": "2999-01-01T00:00:00"}) + prov = common_module.graph_provenance(str(repo)) + assert prov["age_seconds"] == 0 + + def test_unparseable_timestamp_omits_age(self, tmp_path): + repo = self._make_repo(tmp_path, {"last_updated": "not-a-date"}) + prov = common_module.graph_provenance(str(repo)) + assert prov["updated_at"] == "not-a-date" + assert "age_seconds" not in prov + + def test_branch_and_sha_optional(self, tmp_path): + repo = self._make_repo(tmp_path, {"last_updated": "2026-01-02T03:04:05"}) + prov = common_module.graph_provenance(str(repo)) + assert "built_on_branch" not in prov + assert "built_at_sha" not in prov + + def test_none_without_last_updated(self, tmp_path): + repo = self._make_repo(tmp_path) # fresh DB only has schema_version + assert common_module.graph_provenance(str(repo)) is None + + def test_none_without_graph_db(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + (repo / ".git").mkdir() + assert common_module.graph_provenance(str(repo)) is None + + def test_none_on_invalid_repo_root(self, tmp_path): + assert common_module.graph_provenance(str(tmp_path / "missing")) is None + + def test_with_provenance_attaches_envelope(self, tmp_path): + repo = self._make_repo(tmp_path, {"last_updated": "2026-01-02T03:04:05"}) + result = common_module.with_provenance({"status": "ok"}, str(repo)) + assert result["status"] == "ok" + assert result["_graph"]["updated_at"] == "2026-01-02T03:04:05" + + def test_with_provenance_keeps_result_without_provenance(self, tmp_path): + repo = self._make_repo(tmp_path) + result = common_module.with_provenance({"status": "ok"}, str(repo)) + assert result == {"status": "ok"} + + def test_with_provenance_ignores_non_dict_results(self, tmp_path): + repo = self._make_repo(tmp_path, {"last_updated": "2026-01-02T03:04:05"}) + assert common_module.with_provenance([1, 2], str(repo)) == [1, 2] + assert common_module.with_provenance(None, str(repo)) is None + + def test_with_provenance_does_not_overwrite_existing_envelope(self, tmp_path): + repo = self._make_repo(tmp_path, {"last_updated": "2026-01-02T03:04:05"}) + result = common_module.with_provenance( + {"status": "ok", "_graph": {"updated_at": "existing"}}, str(repo), + ) + assert result["_graph"] == {"updated_at": "existing"} + + def test_registered_tool_response_carries_envelope(self, tmp_path): + from code_review_graph.main import list_graph_stats_tool + + repo = self._make_repo(tmp_path, { + "last_updated": "2026-01-02T03:04:05", + "git_branch": "main", + }) + # @mcp.tool() may return a FunctionTool (callable on .fn) or the + # plain function depending on the FastMCP version — same fallback + # as test_main.TestLongRunningToolsAreAsync. + underlying = getattr(list_graph_stats_tool, "fn", None) or list_graph_stats_tool + result = underlying(repo_root=str(repo)) + assert result["_graph"]["updated_at"] == "2026-01-02T03:04:05" + assert result["_graph"]["built_on_branch"] == "main"