feat(mcp): report truthful index freshness from checkout evidence - #1561
feat(mcp): report truthful index freshness from checkout evidence#1561tmonestudio wants to merge 6 commits into
Conversation
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
DeusData
added
bug
|
Thank you @tmonestudio. The underlying issue is confirmed on current This is a broad correctness change despite the focused product claim: 11 files and about 1,300 added lines across git subprocess handling, MCP output, pipeline publication, store migration, and tests. It therefore needs a full storage/schema and fail-closed review rather than a quick UI pass. The current head is mergeable, but Thank you for the reproduce-first coverage and legacy-database checks. The queue is full, so detailed feedback may take some time, but this is labeled and queued. |
|
Checking in — this has been quiet since 12 August and it is still open and still wanted, so here is where it stands with the friction removed. The lint blocker is six clang-format violations in two files:
And an honest note on the rest, so the delay does not read as all yours. Even with lint green, this is not a fast merge: 11 files and ~1,300 lines spanning git subprocess handling, MCP output, pipeline publication, store migration and tests. It needs a full storage/schema and fail-closed review, and that review is ours to do and has not happened yet. Clearing lint is what lets the main test matrix actually run — it was skipped entirely on the last CI pass, so nobody has seen this change tested. The underlying problem you identified is confirmed on current If you have moved on, say so and we will take it from here rather than leaving it to age. If not, clear the lint and the matrix will finally have something to say. |
A live checkout SHA is not proof of the generation that produced graph content, so the freshness verdict must come from the indexed-checkout identity recorded with the DB. Legacy DBs record no such identity: verbose index_status now exposes the graph generation separately (indexed_generation) and fails closed with verdict=unknown, indexed_checkout_sha=null and reason=indexed_checkout_unavailable. The default status call stays lean and the freshness block is report-only — it never triggers indexing. Add focused coverage for the fail-closed verdict and for its omission from the default output. Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
Record the frozen git HEAD at the same successful staged-generation boundary as graph coverage. cbm_coverage_meta_t gains indexed_checkout_sha; new writable DBs create a nullable indexed_checkout_sha column and existing writable DBs are migrated idempotently in init_schema, while read-only legacy DBs without the column still open, read and report no identity instead of erroring. coverage_replace_ex writes/updates the SHA inside its existing transaction, so failure/rollback retains the prior identity. Full, delta-incremental and legacy-incremental generation templates set it from the pipeline's refreshed git context (NULL/empty for non-git repos). index_status freshness now derives from the recorded identity: no indexed SHA -> unknown/indexed_checkout_unavailable; differs from the live git HEAD -> stale/indexed_checkout_mismatch; equal -> current. Emits indexed_generation, indexed_checkout_sha, checkout_sha, a stable reasons array and recommended_action; stays verbose-only and read-only. Add store round-trip/rollback coverage and MCP verdict tests for matching current and mismatched stale (git fixtures, platform-skipped on Windows). Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
Add cbm_git_worktree_status: parse git status --porcelain=v1 -z
--untracked-files=all record-by-record over a bounded chunk buffer
(4K chunks, 16K field cap). Counts advance for the whole stream while
only max_samples paths per class (tracked/untracked) are retained;
exceeding the cap sets *_truncated. Rename/copy records consume their
second NUL-separated source path without counting it, so one rename
counts once. available is true ONLY when git exited 0 and every record
parsed cleanly; any shell-unsafe path, spawn failure, nonzero exit,
malformed/oversized/trailing record leaves available=false with zero
counts - callers must never read that as a clean worktree.
Index the snapshot into the verbose freshness block (status_available,
tracked_changes{count,paths,truncated}, untracked_source{...}). Verdict
composition: clean matching SHA -> current; tracked changes or SHA
mismatch -> stale; untracked-only, unavailable status or missing live
HEAD -> unknown. Reasons array keeps every applicable code in stable
order with the dominant reason first; recommended_action derives from
the verdict. Report-only, verbose-only, read-only: no indexing or
mutation is ever triggered.
Owner gates corrected: restored missing #endif around canonical_root
tests; rename/copy detection checks both XY columns; introduced
cbm_pipeline_indexed_checkout_sha accessor because cbm_pipeline_t is
opaque in pipeline_incremental.c (HEAD accessed git_ctx.head_sha
directly and could not compile); Windows tests force git add -f so a
user's global excludes cannot silently drop the tracked fixture;
rollback test now mutates row detail so the shadow rebuild actually
executes before the replace is rolled back.
Tests now shell out through cbm_popen (isolated spawn) instead of
system(), so the git-backed freshness/status tests run on Windows too;
only a genuinely missing git skips. Prior SHA persistence (staged-
generation boundary) is the anchor the new worktree status refines.
Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
167783b to
9e9fb2f
Compare
Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
|
An update, and a decision that went in your favour. #1727 independently added a top-level Your object stands. The reasoning is that a caller which learns why the graph is not current, and what to do about it, is better served than one that gets a single word — your One idea from that PR worth folding into yours if you touch the docs: "a dirty tree is never treated as evidence in either direction". Your implementation already behaves that way — a dirty tree cannot reach Where this stands otherwise: the storage/schema and fail-closed review you were owed is done and it passes — the three-DB-state migration (fresh, legacy-writable, legacy read-only) and the verdict ladder that never reaches Thanks for your patience — this has been open a long time, and it is the design that is being kept. |
2 participants
What does this PR do?
Makes verbose
index_statusdistinguish the live checkout from the generation that was actually indexed.indexed_checkout_shaonly at the staged-generation publication boundarycurrent,stale, or fail-closedunknownThis is intentionally separate from #1181 and #1065.
Local verification
git_context mcp store_nodesexited 0unknown/indexed_checkout_unavailableChecklist
git commit -s)No runtime binary, cache, ACL, or corpus was modified.