Make MCP retrieval and writes graph aware - #64
blast-hardcheese wants to merge 15 commits into
Conversation
There was a problem hiding this comment.
Confidence Score: 1/5
Summary
Extends MCP retrieval and writes across project and global graphs, but global policy isolation and reference round-tripping have reproducible defects. Secondary-store error handling and grounding warnings also need correction.
Important Files Changed
| File | Overview |
|---|---|
| README.md | Documents cross-graph retrieval and write routing. |
| src/kindex/config.py | Preserves the user-level data directory before project overrides. |
| src/kindex/mcp_server.py | Adds cross-graph discovery and routing with policy-isolation and reference-handling defects. |
| src/kindex/store.py | Adds read-only connections; cleanup masks profile-mismatch errors. |
| src/kindex/tasks.py | Formats graph-qualified task references. |
| src/kindex/vectors.py | Checks existing vector compatibility without schema writes. |
| tests/test_mcp_cross_graph.py | Tests basic routing but misses policy isolation, aliases, dependency round-trips, and secondary-store failures. |
|
Independent review at head Does it fix the two-store defect? No.Reproduced from inside first. Three stores on this machine:
Mechanical cause, And the file that flips the switch is created by a different lane: At this PR's head:
Blocking1. Two harms. Ordering changes for all existing users with no flag and no migration — and the PR's risk note says reordering happens "relative to a single-graph search", which is the opposite of what it does. And the 2. Default writes still land in the store the reader will not read. 3. A git-tracked A cloned repository can destroy the additive-immutability invariant that High4. A stale global graph breaks project-only operations with a protocol error. 5. Profile mismatch raises Medium and below6. A routed global write permanently stamps the user's primary database ( 7. Retrieval containment changes with no opt-out: 8. Grounding verdicts dropped for global hits — 9. 10. Global task All six Adapt findings are real. I disagree with their severity ordering — they rated the Tests
Nothing would fail if a write went to the wrong store. Absent: a default What this gets rightThe diagnosis is correct and the missing concept is named in the right place: the MCP surface had one implicit store and no vocabulary for a second, and It refuses rather than fakes the impossible case. SQLite edges can't span databases, and instead of inventing a shadow edge table or silently dropping the link, the cross-store paths fail with an explicit message. That's the correct answer. The read-only secondary store is defensively right: The hard boundary is preserved — And the comment at Gate item 1 behind Verdict: CHANGES_REQUESTEDRight diagnosis, right primitive — but it doesn't fix the live defect, it silently changes search ranking for every existing user including single-graph ones, and it lets a cloned repo's Upstream fixes worth making regardless of this PR
|
|
@jmc-wander The correctness findings in your review have been addressed and pushed, including the broader read/write consistency issues beyond the six Adapt findings.
One proposed policy change is deliberately not adopted: the requester explicitly wants automatic repository Published head: Session-qualified refs remain session-bound authority, not persistent graph IDs. Sibling-worktree discovery and archive reconstruction remain separate scope. No release or deployment was performed. |
Summary
.kin/selection and selected-store default captures. Default MCP retrieval includes the selected project and effective configured global graph, including nondefault data directories. Explicit profiles remain isolated.source_refs, with global precedence for mixed evidence; cross-store links and task dependencies are rejected before mutation.graph="project"permits contained reads without opening a broken secondary.Default behavior and graph identity
Automatic local graph support remains intentional, per the requester's direction; this PR does not add an opt-in requirement. A source-free capture belongs to the selected store and is readable there through default MCP retrieval. Derivation cannot be inferred from prose: callers provide qualified evidence references when they intend source-based routing.
Qualified references authorize operations only within the current MCP store-selection session. They are not durable database identifiers. Broader sibling-worktree discovery, archive reconstruction, and persistent graph identity are outside this PR. SQLite edges and task dependencies cannot span stores.
Validation
c327820: 3,058 passed, 4 skipped, 9 subtests passed (Node 24; ambient Codex session identifiers cleared).kin policy check --event pre-commitandgit diff --checkpass.Regression coverage includes real
load_config/MCP round trips with a nondefault configured global directory, default capture/search/link, global-derived captures, duplicate IDs and title/alias collisions, contextual write prevalidation, task dependency round trips, native-ranking parity, scoped retrieval, grounding/trust warnings, stale schemas, profile mismatch cleanup, and edit-policy isolation.Delivery scope
No release, tag, deployment, or sibling-worktree discovery is included. The branch incorporates current main's already-landed release metadata and unrelated upstream fixes without changing their behavior.