Make MCP retrieval and writes graph aware

[blast-hardcheese]

Summary

• Search the implicitly selected repository graph and effective configured global graph together. Include relevant global tasks while retaining the explicit-profile isolation boundary.
• Return session-bound graph-qualified references, route follow-on reads and writes to their source, route mixed-source derived writes to the configured global graph, and reject cross-store links and dependencies.
• Construct the secondary Store from the user-level config snapshot, keeping repository edit_policy and other local overrides out of the global graph. Secondary reads use SQLite read-only mode and never stamp the global database.
• Use the canonical title/alias resolver for bare cross-graph collision checks, qualify global task dependencies in structured responses, and surface grounding warnings from each contributing graph.
• Turn secondary schema initialization failures into typed memory-unavailable results with migration guidance; preserve profile-mismatch errors during read-only cleanup.

Graph identity boundary

Repository identity can group related worktrees for future discovery, but it does not authorize writes to a sibling worktree graph. Broader discovery is outside this PR. A future feature needs persistent database identity plus a live worktree or configured-global binding. Git archive cannot recreate an untracked .kin/local database.

Validation

• Regression tests reproduce each of the six Adapt findings, including case-insensitive title and alias collisions, dependency round-trips, a configured nondefault global directory, a stale global schema, and profile mismatch cleanup.
• Full tests/ suite on the final code: 2,914 passed, 4 skipped, 9 subtests passed. Focused runs covered the cross-graph/profile regressions (70 passed) and the existing title-collision contract (22 passed). Tests used Node 24 with the ambient Codex session ID cleared.
• kin policy check --event pre-commit and git diff --check pass.

Risk notes

• Cross-graph search ranking can reorder hits relative to a single-graph search. Qualified refs intentionally expire across MCP server restarts.
• SQLite edges and task dependencies cannot span stores. No release or deployment is included.

[adaptcom]

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.

↻ Re-run review · View in Adapt

[jmc-wander]

Independent review at head 6dd9ba2, executed in a throwaway worktree. I came to this PR from the live defect it appears to target, so let me lead with that.

Does it fix the two-store defect? No.

Reproduced from inside first. Three stores on this machine:

store	nodes
~/Personal/Conv/kindex.db (configured data_dir)	27,770
<repo>/.kin/local/kindex/kindex.db	1
~/.kindex/kindex.db (package default)	0

search reads the first, add/link/status write the second, so search hands back node ids that link then cannot find.

Mechanical cause, src/kindex/config.py:1003-1027 (identical on main and this head): store selection keys on the mere file presence of <git-root>/.kin/local/*.db, and presence outranks the user's configured data_dir. Proven on main:

A) plain                  -> …/repoA/.kin/local/kindex     # global data_dir silently discarded
B) .kin/config data_dir:  -> …/repoA/.kin/local/kindex     # explicit project config ALSO loses
C) KIN_PROFILE=personal   -> …/HOME-GRAPH                  # the only lever that pins it

And the file that flips the switch is created by a different lane: src/kindex/integrations.py:70-99 (modern_codebase_store) unconditionally mkdirs <repo>/.kin/local/kindex and opens a Store there. A separate MCP server's side effect silently re-points this one. That is the one-authority-per-fact break — store selection is decided by an artifact nobody announced.

At this PR's head:

ADD  -> Created node: aac7c6cd0d39      in local store? True   in global? False
LINK bare -> Target node not found: 418b002e7145          # the live symptom, unchanged
LINK qual -> Error: cross-graph links are not supported   # new permanent refusal

add with default arguments still writes to the repo-local store (_derived_write_store, mcp_server.py:381-390, defaults graph="project"). The PR makes global nodes visible to search, which raises the odds an agent finds one and then discovers it cannot link its capture to it at all.

Blocking

1. search silently re-ranks for every user, including single-graph users. mcp_server.py:666-687. The merge block is unconditional — it runs even when home is None, which is the majority configuration. It replaces hybrid RRF ordering with 0.35*confidence + 0.30/(rank+1) + 0.35*literal_term_coverage (:675), while the displayed score at :718 still prints rrf_score when home is None. Executed, single store, no global graph involved:

=== MAIN ===                     === PR64 ===
1. Release checklist  (0.786)    1. Release checklist  (0.786)
2. Deploy guide       (0.429)    2. Release release…   (0.383)  <- up
3. Release release…   (0.383)    3. Deploy guide       (0.429)  <- down

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 0.35*coverage term systematically demotes vector and graph-BFS hits carrying none of the query's literal tokens, which is exactly what hybrid retrieval exists to surface. Displayed scores are now non-monotonic, so the output looks broken.

2. Default writes still land in the store the reader will not read. mcp_server.py:381-390, 744/793, 1610, 2497. Routing to global requires the caller to pass graph="global" or source_refs="global:<id>"; nothing enforces it, and the only mechanism is a sentence in the instruction blob at :72-73. Concretely: an agent starts in a repo, captures a decision before searching (the documented workflow — "capture as you go, don't batch"), the write lands in the 1-node store, and the next session's search shows 27,770 nodes none of which link to it. The routed path genuinely works when used, but this is a behavioural contract on an LLM, not a mechanism — and it cannot cover the first write of a session, because no source_refs exists yet. link has no graph parameter at all.

3. A git-tracked .kin/config from a cloned repo now governs writes into the personal graph. mcp_server.py:301 does home_config = config.model_copy(deep=True), copying the project-merged config including edit_policy — which isn't in _PROJECT_LAYER_UNTRUSTED_KEYS (config.py:874-880) because until now it could only govern that repo's own store. Executed:

.kin/config:  edit_policy: {decision: editable, constraint: editable}
  -> ignored_project_keys == []
  -> edit("global:<decision-id>", content="REWRITTEN by the repo")  => succeeded
  baseline: "Error: Node type 'decision' is additive — use supersede"

A cloned repository can destroy the additive-immutability invariant that supersede and the whole decision/constraint model rest on, in the user's global graph. Same class this codebase already defends against in project_store.tracked_store_refusal ("files a clone delivers are not a local trusted database"). Build home_config from user-level layers, not config.model_copy.

High

4. A stale global graph breaks project-only operations with a protocol error. store.py:386-390 raises SchemaMigrationPending (a RuntimeError); callers wrap only except ValueError; _safe_output re-raises. With home at schema_version 3, both edit(<project node>) and search("local") raise RuntimeError. The collision probe in _routed_ref (:356-367) opens the global store for any bare id matching locally, so a second store the user never asked to involve takes down the primary. _tool's own docstring promises "a broken store turns into a typed tool result on every tool, never a protocol error."

5. Profile mismatch raises AttributeError instead of the real error. store.py:392-395 calls self._conn.close() in cleanup, but _check_profile_stamp (:450-451) already set _conn = None. The non-read-only path at :417-419 gets this right. Result: RuntimeError: 'NoneType' object has no attribute 'close' instead of the profile-stamp message.

Medium and below

6. A routed global write permanently stamps the user's primary database (:309-311 clears _stamp_on_open only for reads); first add(graph="global") writes meta.kin_profile, after which any other profile name hard-refuses. Irreversible without SQL.

7. Retrieval containment changes with no opt-out: search (:590) and task_list (:2533) gained no graph parameter while the write tools did, and ties prefer global (:677). Once any repo has a .kin/local, every search there merges the personal graph in — asymmetric with the write API, and it removes the separation the repo store exists to provide.

8. Grounding verdicts dropped for global hits — :631-634 omits grounding=grounding, so a search returning 100% global results prints no [grounding: uncalibrated] banner precisely when nothing was evaluated.

9. context (:996), ask (:1519), prime, list_nodes, suggest, graph_stats, status, changelog are not graph-aware. prime is the SessionStart primer — so search reports 27,770 nodes while prime reports 1, and an agent trusting prime concludes memory is empty.

10. Global task dependencies returned unqualified (:2698-2703, :2733-2738), so a follow-on task_get(dep) resolves against the wrong graph. 11. The collision guard (:356-367) probes with peek_node + exact-title SQL while the write uses resolve_node_for_write → _nodes_named (title or alias), so the guard has false negatives.

All six Adapt findings are real. I disagree with their severity ordering — they rated the edit_policy leak as a config-construction nit; it's a clone-controlled trust-boundary break. They missed items 1, 2, 6, 7 and 9, including the single-graph reordering that affects every user.

Tests

tests/test_mcp_cross_graph.py (209 lines, 13 tests) does not test store selection. The fixture (:13-30) monkeypatches server._store and server._config directly, bypassing load_config — it asserts behaviour given a correctly-chosen pair of stores, which is the one thing that isn't broken in the field. The only selection test (:191-209) asserts the current defective outcome: cfg.data_path == local_dir.

Nothing would fail if a write went to the wrong store. Absent: a default add() followed by a read that must find it; link(<new>, <bare id existing only in global>); single-graph ordering parity with main (would have caught 1); a stale or profile-mismatched global store (4, 5); edit_policy isolation (3); context/ask/prime parity (9).

What this gets right

The 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 global:<id> is the right primitive. Once a reference carries its authority, show, edit, supersede, verify, invalidate, link, graph_merge, watch_resolve, lock_*, stale_check --rebind and task_* all route consistently — and the PR wires every one, not a convenient subset.

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: store.py:377-396 uses a real mode=ro URI plus PRAGMA query_only=ON — enforcement, not convention. It refuses to migrate, stamp, or create a missing database (:318); vectors.ensure_vec_table gets a matching guard (vectors.py:615-630) so a search can't rebuild vector state in a graph it's only visiting; and store.py:2061 suppresses the last_accessed write so reading the global graph doesn't perturb its decay signal. The test at :58-76 proves all of it, including that the connection rejects an UPDATE.

The hard boundary is preserved — _global_store returns None when an explicit profile is active (:298-300), so anyone who deliberately sequestered their graphs sees byte-identical behaviour.

And the comment at :315-317 — "Fail visibly if an existing secondary graph is unreadable; silently returning only project results would recreate the original false negative" — shows the author understood the actual hazard class. The intent is right even where the implementation overshoots.

Gate item 1 behind if home is not None, build home_config from user-level layers, return typed results for 4 and 5, and default add to the graph search is reading, and this is a good PR.

Verdict: CHANGES_REQUESTED

Right 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 .kin/config override mutability policy on the user's personal graph.

Upstream fixes worth making regardless of this PR

1. Presence must stop being an authority. config.py:1003-1027 should require an explicit opt-in — a store: project key in .kin/config, or KIN_PROJECT_STORE=1 — before a repo-local database outranks the configured data_dir. A file created by a different lane is not a user decision.
2. integrations.modern_codebase_store must not create a store that silently re-points another lane.
3. Make the write default follow the read. If search merges two graphs, add with no graph must not silently pick the one with 1 node.
4. Add the missing test: default add, then search/link for that node, across the full load_config path — not a monkeypatched store pair.