feat(mcp): expose index freshness and relationship provenance

[vvenegasv]

What does this PR do?

This PR adds machine-readable freshness and provenance evidence to MCP responses, helping clients determine whether results come from a current index, a stale or partial index, direct source resolution, or heuristic inference.

Changes include:

• Persisting the indexed Git HEAD and file coverage metrics for each project.
• Extending index_status with additive evidence about indexed/current revisions, working tree state, freshness, coverage, and known limitations.
• Returning edge_evidence from trace_path and trace_call_path when relationship provenance is available.
• Preserving backward compatibility by returning unknown or unavailable values when older indexes do not contain the new metadata.

Why is this needed?

Consumers of the MCP need factual and inspectable signals to decide whether a result can be trusted as-is, whether the repository should be re-indexed, or whether the result should be verified directly against source code.

Rather than exposing an opaque reliability score, this change provides the underlying evidence needed for clients to make that decision.

Checklist

• Every commit is signed off (git commit -s)
• Tests pass locally (make -f Makefile.cbm test)
• Lint passes (make -f Makefile.cbm lint-ci)
• New behavior is covered by a test

Tracking

Closes #786

[DeusData]

Thanks @vvenegasv — this is the version we'll take for the index-freshness / relationship-provenance feature (the tested superset of #544, which I've now closed in its favor). The core is read-only, additive MCP output and reviewed clean.

One ask before merge: please split out the changes bundled in here that are unrelated to this feature —

• the cli.c ZIP-size cast,
• the cypher.c redundant NULL-check removal, and
• the git_capture "empty-output is success" refactor.

They're benign (and arguably correct), but they're out of scope for this PR — as a separate focused PR they're trivial to review and land. With those split out and CI green, this is good to go. 🙏

[vvenegasv]

Hi @DeusData, the requested changes have been applied and CI is now green. Please let me know if there’s anything else you’d like me to adjust. Best regards.

[DeusData]

Thanks for the freshness/provenance work. Because this adds MCP response fields and shells out through cbm_popen, please add focused tests showing repo paths are validated/quoted safely, including paths with spaces/quotes where applicable, and document the additive response fields for client compatibility. Please also link the tracking issue.

[vvenegasv]

Hi! @DeusData I rebased the branch on top of the latest upstream main, resolved the conflicts, and addressed the sanitizer leak reported by CI.

I also reran the relevant checks locally, including lint and the affected test suites, and the PR CI is now green.

Thanks for the review!

[DeusData]

Reviewed this in full — thank you, and apologies for how long it has waited. There is one genuinely good idea here and one finding you will want to know about regardless of what happens to the PR.

Start with the important one: the freshness half does not work on a real index, and the tests conceal it.

Both production pipelines — full (pipeline.c) and incremental (pipeline_incremental.c) — persist via cbm_gbuf_dump_to_sqlite, the direct writer. On that path this PR hardcodes indexed_git_head = NULL (the comment in sqlite_writer.c says "unavailable in the direct writer path") and zero counters. The only place the HEAD actually gets captured is cbm_store_upsert_project, and that is called solely from cbm_gbuf_flush_to_store — which neither pipeline uses.

So on any real index, freshness would report "unknown" forever. The tests pass because they drive the flush path that production does not take. That is a nasty shape to debug later, and it is worth knowing about even if none of this lands.

The coverage counters have the same problem from the other direction: pipeline.c passes (file_count, file_count, excluded_count, 0), so discovered is identical to indexed by construction and failed is always zero. That makes coverage_status report "partial" for any repository with normal exclusions and "complete" otherwise — misleading whichever value it produces.

There is also more overlap than the PR assumes, largely because main moved a long way since you branched: index_status now reports git context and a per-file coverage report with reasons, which is strictly richer than four aggregate counters; and detect_changes already answers "what changed since the index" at file, symbol and impact granularity — including untracked and staged-new files, which a HEAD-equality-plus-dirty-bit check ignores by design.

Now the good half — and I want to be clear that it is good.

The provenance work costs nothing in storage, which is the thing I checked first and expected to fail. It does not add a byte per edge: production already persists strategy, confidence and candidates into CALLS-edge properties in pass_calls.c, and the same on CONFIGURES edges. You are surfacing information we already pay for but never expose. At ~15.7M edges that distinction is the difference between a merge conversation and a non-starter, and you got it right. Reporting unavailable for edges that carry no such keys is also the honest default.

Two things I would want changed there: the classifier is a fairly brittle opinionated mapping, and the output is a parallel array that repeats from/to/type for every edge already present in callers/callees. That is real token bloat on deep traces, and it runs against the tree-format work that shipped on main since — which cut token cost substantially and would be the natural shape for this.

Two more things worth flagging:

• The PR bundles five unrelated refactors. One of them, in cypher.c, removes the start_line/end_line fallback branches in the node-refetch path — that is query-visible behaviour changed by a drive-by, which is exactly what our atomic-PR rule exists to prevent. Separately, your pass_pkgmap.c NULL-handling restructure looks like a genuine latent-UB fix (snprintf on NULL) and would be welcome as its own small PR.
• The branch is roughly 107 commits behind on mcp.c alone.

What happens next is the maintainer's call, and I have put it to him as close-and-distill versus split-and-rebase. I have deliberately not asked you to rebase: doing 107 commits of conflict resolution to land a half that does not function would be a poor use of your evening, and I would rather get you a direction first.

My own read, for what it is worth: the provenance idea deserves to exist, ideally as its own PR emitting into the tree format rather than a parallel array. Thank you for finding it — the observation that we already store this and never show it is the valuable part.

[DeusData]

Two corrections to my earlier comment, and one finding that matters more than either.

First, I was wrong about the cypher.c hunk. I described it as query-visible behaviour changed by a drive-by. It is not. node_prop() early-returns for start_line and end_line at cypher.c:2219-2226, so the fallback branches your diff deletes are unreachable — and I checked the merge base too, where the same early return sits at cypher.c:2090. They were already dead when you wrote the PR. It is still scope that was not asked for, but the deletion has zero behavioural effect and I should not have said otherwise.

Second, about the split. You were asked on 23 June to pull the unrelated changes out, and you replied on 25 June that it was done. The current head still contains them — the cli.c ZIP cast, the cypher.c hunks, the extract_defs.c change — riding in commit 0acb5af. I do not know whether the 6 July rebase reintroduced them or they were never removed, and I am not asking you to redo work you believe you delivered; I am flagging it so we are not talking past each other.

Now the finding, which is the important part: as written, this would break every index created by an older binary.

The new columns are added by ALTER TABLE inside init_schema. But init_schema only runs on read-write opens — main states this at src/store/store.c:320: "Read-only query opens skip init_schema and keep working." And the MCP server opens every project store read-only (mcp.c:2067, :2083, :2329, :7662).

Your widened SELECT in cbm_store_get_project and cbm_store_list_projects then fails at prepare time against an unmigrated database — no such column — and verify_project_indexed() (mcp.c:2531) interprets that failure as "project not indexed." So after upgrading the binary, every previously indexed project would report itself unindexed across get_graph_schema, trace_path and their siblings until re-indexed from scratch.

This is not a rebase artifact — both halves existed at your merge base. And it is invisible from CI because the migration test opens the store read-write, which is the one path the server never uses.

Related, and worth knowing before you invest more: the freshness half does not currently function on a real index. Both production pipelines persist through the direct SQLite writer (pipeline.c:1270, pipeline_incremental.c:653), and on that path build_project_record writes a hardcoded NULL — your own comment at sqlite_writer.c:800 says as much. The code that captures the HEAD lives in cbm_store_upsert_project, which the pipelines never reach. So freshness is always "unknown" and both snapshot_matches_* fields are always null in production. The tests pass because they call the upsert directly and then overwrite the value with raw SQL — they would pass unchanged even if nothing were ever recorded.

And the half that is genuinely good deserves saying clearly. Edge provenance is a real, unfilled gap: pass_calls.c:355 already stores {callee, confidence, strategy, candidates} on CALLS edges, cbm_store_traverse already reads them back, and no MCP tool exposes any of it — I checked main. An agent can see that A → B exists but not how it was resolved. Surfacing that costs zero extra storage and zero extra indexing work, because the indexer already pays to compute it. That idea is worth landing.

The complication is that bfs_to_json_array, the function your evidence block hooks, no longer exists — c5bffb7 rewrote trace_path into tree-format output with cursor pagination. So the provenance half needs re-implementing against the new emitter rather than rebasing, and it needs to respect the pagination window, which the current version bypasses.

The maintainer is deciding the shape now, including whether we distill the provenance work with Co-Authored-By credit rather than asking you to rewrite it. One piece I would like to take regardless: your pass_pkgmap.c NULL-handling restructure is a real latent-UB fix on a pre-existing path, and it stands on its own.

I will come back with the decision. Thank you for the patience, and for a PR whose good half is genuinely good.

[DeusData]

The maintainer has decided, and I want to give you the whole reasoning rather than a verdict.

We are taking your provenance idea, and I am closing this PR to do it. Those two things are one decision, not two, so let me be precise about what it means.

What we are taking. Edge provenance was the real find here. pass_calls.c:355 and :430 already persist {callee, confidence, strategy, candidates} into CALLS edge properties, cbm_store_traverse already reads them back, and the default trace_path edge set is CALLS — so most traversed edges genuinely carry that data. And no MCP tool exposes any of it. I checked main: grep -n confidence src/mcp/mcp.c returns nothing. An agent can see that A → B exists but not how it was resolved, and the indexer is already paying to compute the answer. "We already store this and never show it" is exactly the kind of observation that is hard to make from outside a codebase, and you made it.

That work will land with Co-Authored-By: vvenegasv on the commit. I will link the PR back here when it is up.

Why we are not asking you to rebase it instead. This is the part I want to be straight about, because "we'll do it ourselves" can read as a polite decline and it is not one here.

The function your evidence block hooks — bfs_to_json_array — no longer exists on main. c5bffb7 rewrote trace_path into tree-table output with cursor pagination and generation staleness checks; format:"json" is now a legacy escape hatch. Your branch is 601 commits behind, mcp.c alone moved +6066/−909 across 61 commits, and test_mcp.c moved +6195/−1118 across 45. So this is not a rebase with conflicts to resolve — it is a re-implementation against an emitter that did not exist when you wrote this, and asking you to do that after six weeks of waiting would be asking you to absorb the cost of our queue. About thirty-five lines of your mapping helpers survive the transition; the rest is new code against new machinery.

There is also a design point that has to be got right on the way in, and it is easier for us to carry: the current version calls trace_edges_to_json_array with the full traversal result rather than the paginated window, so it bypasses limit, the cursor, and the 5000-edge guard. Against a tree format that cut trace token cost by ~98.8%, that would be a visible regression. The distilled version will respect the window.

What I would genuinely like from you, as its own PR. Your pass_pkgmap.c NULL-handling restructure is a real latent-UB fix — snprintf("%s", name_copy) with name_copy == NULL on the pre-existing path — and it has nothing to do with anything above. It stands entirely on its own, it is small, and it should be yours. Please send it as a separate PR against current main and I will review it promptly; you have my word it will not sit for six weeks.

Why the freshness half is not coming with it. I described this in my previous comment and it is the reason the PR cannot land whole: both production pipelines persist through the direct SQLite writer, where build_project_record writes a hardcoded NULL — your own comment at sqlite_writer.c:800 says as much — so freshness is permanently "unknown" in production regardless of the reporting code. And the ALTER TABLE migration inside init_schema never runs for the MCP server, which opens every store read-only, so the widened SELECT would make every project indexed by an older binary report itself unindexed.

The underlying need is legitimate — your issue #786 has independent demand from other users — so it is being recorded as a design question rather than dropped. If it is built, it will need the HEAD captured on the cbm_gbuf_dump_to_sqlite path, stored where read-only opens can see it (store_meta, no schema widening), and gated behind verbose so index_status stays lean. Worth knowing too: detect_changes already answers a strictly larger question — what changed since the index, at file, symbol and impact granularity, including untracked and staged-new files.

Thank you. You responded within days every time you were asked something, you filed the issue before the PR, and you found something real that nobody had surfaced. That the queue moved the ground under you in the meantime is our failure, not yours, and I am sorry it cost you this much time. I hope you will send the pass_pkgmap.c fix — and I would be glad to see you back on the freshness design if we take it up.

[DeusData]

Quick correction before you spend any time on it — please do not open the pass_pkgmap.c PR.

I asked for it as a separate PR an hour ago. The maintainer has since decided to take it in the same distillation, so if you started on it you would be duplicating work we are about to land. Sorry for the whiplash; that one is on me for asking before checking.

So both pieces travel together: the edge-provenance surfacing and your pass_pkgmap.c NULL-handling fix, in one PR, with Co-Authored-By: vvenegasv on the commit. I will link it here when it is up.

Nothing else changes, and nothing is needed from you. Thank you again.

[DeusData]

One more correction, and this one is better news than it sounds.

The pass_pkgmap.c fix is already on main. I went to distill it and checked the code first: 632fd08 (10 July) landed the same guard independently while this PR was waiting. Main now reads

char *alt_entry = NULL;
if (name_copy) {
    snprintf(suffix, sizeof(suffix), "%s/__init__", name_copy);
    alt_entry = build_entry_path(rel_path, suffix);
}

which closes exactly the UB your version closed — the unguarded snprintf on a possibly-NULL name_copy — and it is leak-correct on both branches. Your restructure and main's fix are equivalent; main simply got there first.

So there is nothing to take from that file, and I am not going to open a no-op commit with your name on it. You were right about the bug — that is the part worth recording, and it is now in the review notes. It is the third piece of this PR that main reached independently during the wait, which says something uncomfortable about our queue rather than about your patch.

The distillation therefore covers the edge-provenance work only, still with Co-Authored-By: vvenegasv. Two design points the maintainer settled for it, in case you are curious how your idea is shipping:

• It goes behind an explicit include_evidence parameter rather than being on by default, so the default trace response stays lean.
• strategy ships as a closed enum with a test pinning it to what pass_calls.c actually emits, so internal resolver names cannot leak into the public contract and a future rename cannot silently change output.

I will link the PR here when it is up. Nothing needed from you — and again, sorry for the back-and-forth on that one file.

[DeusData]

As promised — the distillation is up: #1376, with Co-Authored-By: vvenegasv on the commit.

Your observation is what it is built on: we already store {callee, confidence, strategy, candidates} on every CALLS edge in pass_calls.c:355, cbm_store_traverse already reads it back, and no tool ever showed it. That is now surfaced on trace_path and trace_call_path.

Three things I changed from your version, and why:

It is behind include_evidence, default off. Two extra columns on every row is the kind of inflation the tree format exists to prevent, so the default response is byte-identical to before.

strategy publishes a class, not the raw name. Counting them, production emits about twenty internal strategy names — lsp_trait_dispatch, php_self_static, callee_suffix, service_pattern and so on — and the set grows with every language. Publishing those verbatim would make each internal resolver name public API by accident. They now map to lsp | language_rule | heuristic | unresolved, pinned by a test, so adding lsp_foo_dispatch maps automatically while a genuinely new kind of resolution fails loudly and forces a decision.

Evidence is read from the paginated view. Your version passed the full traversal result, which bypassed limit, the cursor and the 5000-edge guard — against a tree format that cut trace cost ~98.8%, that would have been a visible regression.

One incidental confirmation you might enjoy: while revert-checking, I bypassed the classifier to pass the raw strategy through, and ASan immediately caught a stack-use-after-return — the raw value lives in a stack buffer. So mapping to static class literals is load-bearing rather than cosmetic.

Thank you. This was a real gap and you found it from outside the codebase, which is the harder way to find anything.

[DeusData]

Landed on main as 73cfb61, merged via #1376 — with your co-authorship on the commit.

So the thing you spotted is now real: trace_path and trace_call_path can tell an agent how an edge was resolved, not just that it exists. That data was sitting in the store the whole time and nothing surfaced it, which is exactly what you said.

Thank you for the contribution and for the patience through a long queue. I hope we see you back.