fix(ai): kb_search scope-aware residency + AI-residency gate architectural drift - #354
Merged
Conversation
…chanism kb-promote completeness (#303 follow-up): - First-class kb_promote MCP tool with an explicit node_id param (was only reachable as an implicit, buffer-scoped command before) - SPC n p leader binding, in-app help docs, docs/KNOWLEDGE_BASE.md updates, and a "promoted from X on DATE" provenance line in the KB view header - Fix kb_migrate_stranded_federation_nodes's durability gap (in-memory-only removal); remove dead get_crdt_doc/update_crdt_doc trait methods - New detect_reimport_stale_files drift signal, surfaced via kb_id_audit - Full-lifecycle tests at the AI/MCP tool layer (not just core Editor methods) plus a two-peer CRDT sharing round-trip test - Fix a real bug found along the way: kb_share's primary-KB branch read from the federated query layer, silently bundling every other registered KB instance's nodes into a "primary" share payload Always-on AI guidance mechanism: - New ai_guidance_kb option naming a KB whose content is surfaced to AI agents at session start as standing practices - Shared mae_ai::guidance module (project-context + guidance-KB reading), deduplicating logic that only existed in the deprecated embedded chat path - Wired into mae-agent-cli's system prompt and the MCP initialize response's new `instructions` field Test-safe browser launching: - MAE_BROWSER env-var override at both xdg-open call sites (matches the existing MAE_DAP_*/MAE_LSP_* idiom) so cargo test never pops a real Firefox window Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ope/ranking (#350, #351) Fixes both filed bugs, plus the architectural gap they turned out to be symptoms of: - #351: kb_search's AI-residency check ignored its own `scope` argument, denying calls outright even when `scope` explicitly excluded the restricted KB. Fixed by resolving scope before checking residency. - #350: kb_search_context had no `scope` param at all (unlike kb_search), and its ranking never checked :ALIASES: despite the tool's own description claiming it does, so alias-only matches fell to the lowest scoring tier and ties broke arbitrarily by alphabetical node id. Fixed by routing through the same kb_federated_search_scoped mechanism kb_search already uses (closing a second, independently hand-rolled scan), scoring aliases/ids at proper weight, and breaking ties by body-match position. Investigating these two surfaced a bigger problem: the AI-residency gate (crates/mae/src/ai_residency.rs, ADR-048) was two hand-maintained arrays that any new kb_* tool could silently fall through unclassified -- and the default for an unlisted tool was Allow, not deny. Nine tools were found completely ungated this way, including kb_raw_query (arbitrary Datalog against the primary store -- a full content bypass) and kb_graph (an explicitly federated BFS walk). Replaced the two arrays with an exhaustive classification (SingleTarget / PrimaryOnly / ScopedFederatedScan / UnscopedFederatedContent / NonContent) that fails closed for any kb_*/help_open tool it doesn't recognize, backstopped by a CI test that enumerates every real registered tool and asserts each has an explicit classification -- so this class of bug can't silently recur. Also fixed kb_agenda's inverse bug (classified as federated-scan when its implementation is actually primary-only, so it over-blocked on unrelated restricted instances) and kb_links_to's wrong-entity check (was gated on the target node's home KB, not the KBs its aggregated backlinks actually scan). ADR-048 updated to match. 32 residency tests (9 pre-existing + 23 new), 10 kb_search_context tests (8 pre-existing + 2 new: position-based tie-break, scope filtering), all passing. Full workspace + daemon build/test/clippy/fmt clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
Live-validated this branch against a real 85-node federated KB (`dev-practices-kb`), scoped queries run against 3 separate registered instances on the same machine (~15 total scoped calls):
|
…ific notes (#357) execute_kb_search_context's score_node scored the whole query string as a single substring against title/alias/id/tag, so multi-word natural-language queries almost never matched -- nearly every candidate tied at the score=1 body-match fallback, collapsing the sort to alphabetical-by-id (unrelated to relevance) instead of trusting kb_federated_search_scoped's already-correct upstream order. That's why a hub/category node whose id happened to sort early could outrank the actual target note. - score_node now tokenizes the query and sums per-term field-weighted hits (matching search_ranked's own tokenization), with a whole-phrase bonus on top and a hub/meta down-weight (NodeKind::Category/Meta or :role: hub). - Sort now trusts stable-sort + upstream order for ties instead of re-deriving a worse id/position-based tiebreak. - body_match_position is repurposed to pick which paragraph excerpt_body starts from (RAG-specific value), not a sort key. - The same hub/meta down-weight is added to search_ranked itself (shared/kb/src/lib.rs) via a new kind_role_prior, composed with the existing namespace_prior, so kb_search and kb-find's large-KB path benefit too -- not just the AI RAG tool. - search_ranked also gains a soft-AND fallback: when strict per-term AND matching (every term must match somewhere) returns nothing, a single relaxed pass allows exactly one missing term, at a fixed score penalty. This is a bounded down payment on #357's "zero results" symptom for natural-language queries with one filler/unmatched word -- not a replacement for real fuzzy/FTS body search, which stays tracked under #81. - Corrected kb_search_context's tool docstring, which claimed a ranking contract ("same ranking kb_search uses... ties broken by body-match position") that didn't match the actual implementation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…359) kb-find, find-file/project-find-file, and switch-buffer all showed an arbitrary (alphabetical-by-id, alphabetical-by-path, or buffer-creation order) candidate list before the user typed anything, instead of recently used items first -- real friction since users mostly cycle between a handful of recent files/nodes while working. Two recency mechanisms already existed in the codebase (RecentFiles MRU, KB activity tracking) but neither was wired into these pickers' default ordering. - FilePicker::reorder_by_recency reuses RecentFiles, called after scan() in both find-file and project-find-file. Entries outside root or never scanned are silently skipped; never-opened files keep their prior alphabetical order after the MRU block. - switch-buffer: new Buffer::last_focused + Editor::buffer_focus_seq, bumped in the single sync_mode_to_buffer() choke point every focus change already routes through. Candidates sort by last_focused descending. - kb-find: empty-query snapshot now reuses the existing activity-sort comparator (extracted into a shared helper instead of duplicated), but only when kb_search_sort is still at its default ("relevance") -- an explicit alphabetical/activity/recency choice is left untouched, and typing a query restores today's behavior exactly. No new OptionRegistry option: this corrects a default that's meaningless for the empty-query case (relevance has nothing to rank against with zero terms), not a new opt-in flag. Typed-query behavior is unaffected in all three pickers since their filters fully re-derive from the base candidate array; reordering it only changes the empty-query default and the tie-break among equal-scored typed matches. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d Ctrl-J (Insert+LSP popup) (#360) Two confirmed readline/POSIX keybinding gaps in interactive prompts: - Ctrl-U (clear query) had no binding in Mode::CommandPalette (kb-find, switch-buffer, recent-files, project-switch, kb-insert-link, ...) or Mode::Search, unlike the analogous Mode::FilePicker which already had it. - Ctrl-J was unconditionally treated as "insert newline" in Mode::Insert even while the LSP completion popup was open, dismissing the popup as a side effect -- inconsistent with Ctrl-N/Ctrl-P (already wired to lsp_complete_next/prev when the popup is open) and with CommandPalette/FilePicker, where Ctrl-J already means move-selection-down. Checked and ruled out as a cause: no terminal/GUI key-decoding collision between Ctrl-J and Enter in either the TUI (crossterm raw-mode C0-control decoding) or GUI (winit-to-crossterm translation) backends. Also confirmed Mode::ConversationInput has no gap -- its Ctrl-U is already a correct readline kill-to-cursor, and it has no completion-list concept to need a Ctrl-J binding for. Fix: added Ctrl-U to CommandPalette (routes through kb_find_palette_query_changed() for correct large-KB lazy re-search) and Search (direct clear on search_input). Added a popup_open-guarded Ctrl-J arm in Insert mode before the existing unguarded newline arm, calling lsp_complete_next() -- the existing arm remains the correct fallback when no popup is open. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nto fix/kb-search-residency-scope
…358) MAE's AI-residency policy (ADR-048) gates AI tool access to KB content at whole-KB-instance granularity. Restricting `primary` to `local_models_only` (to protect a user's own locally-ingested notes) also blocked AI access to MAE's own seeded/built-in manual content living in the same instance -- compiled in at startup, identical on every install, never sensitive -- with no per-node exemption. This locked an AI agent out of MAE's own help system as an unintended side effect of a policy meant to protect private notes. A root-cause audit of all 20 kb_*/help_open AI tools across every ToolResidencyShape bucket grounds this fix's scope: exempt wherever a real Node is already in hand or free to get (Tier 1, implemented here); file one follow-up issue for tools that need deeper plumbing (Tier 2, #361); document why three tool shapes stay structurally unable to apply the exemption at all (Tier 3) rather than leaving them as a silent gap. - New crates/core/src/ai_residency.rs: is_residency_exempt (keys on the already-existing Node::source == Some(NodeSource::Seed), stamped once at startup -- no new tagging infrastructure) and filter_residency_exempt(_primary), the shared filter primitives. Live in mae-core rather than crates/mae (where the rest of ADR-048's gate lives) purely because the `mae` package has no [lib] target and is therefore unreachable from mae-ai's tool implementations -- a Rust crate-graph constraint, not a conceptual split. - SingleTarget tools (kb_get, kb_update, kb_delete, kb_add_link, kb_set_role, help_open, kb_preview_show): resolve_restricted_label now checks the already-resolved node's exemption before denying -- zero new plumbing. - kb_agenda, kb_search, kb_search_context: reclassified into two new shapes (PrimaryOnlyFilterable, ScopedFederatedScanFilterable) where the gate allows the call through unconditionally and the tool impl post-filters its own materialized results instead of denying the whole call outright. kb_raw_query/kb_view_query (arbitrary Datalog, no per-row node-identity to filter) stay in the original hard-deny PrimaryOnly bucket; kb_vector_search (a permanent stub with no real results yet) stays in the original ScopedFederatedScan bucket. - Threaded requester_provider: Option<&str> down to these three tool impls via a minimal-blast-radius wrapper split (execute_tool/execute_tool_with_requester, dispatch_tool, kb_exec::dispatch) -- only the one dispatch point that needs it changes signature; the other 7 category dispatchers are untouched. The embedded/delegate() path needed zero new plumbing (editor.ai.provider was already reachable); the external-MCP path's already-computed requester_provider, previously discarded right after the gate check, is now threaded through. - ADR-048 gains a new Decision §7 and Verification bullets documenting the exemption; ai_residency.rs's module doc documents why kb_raw_query/ kb_view_query/kb_id_audit/the two count-only graph-view tools stay unchanged, and cross-links #361 for the real remaining candidates. Tests: new crates/core/src/ai_residency.rs unit tests (every NodeSource variant, not just seed-vs-one-other); crates/mae/src/ai_residency.rs gains the critical negative case (a genuine user node in the same restricted primary as an allowed seed node must still be denied) plus rewritten/ retargeted gate-level tests reflecting the two tools that now defer to their own post-filter; crates/ai/src/tool_impls/kb.rs gains real adversarial positive/negative/local-provider-bypass/no-op-when-open coverage for all three post-filter tools, including a check that an all-filtered result set hits kb_search_context's low-result guidance branch rather than a mislabeled empty array. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
kb_search's AI-residency check ignored its ownscopeargument, over-blocking searches scoped to an open KB just because some other unrelated registered KB wasLocalModelsOnly.kb_search_contexthad noscopeparam at all, ignored:ALIASES:/id matches when ranking, and tie-broke arbitrarily on node ID instead of body-match position. Its hand-rolled dual-path scan is replaced with the canonicalkb_federated_search_scoped(the same mechanismkb_searchalready uses), which also makes the residency fix apply correctly.crates/mae/src/ai_residency.rs, ADR-048) was a hand-maintained two-array allowlist where any unlistedkb_*tool silently defaulted toAllow. The audit that followed found 9 previously-ungated tools, includingkb_raw_query(arbitrary Datalog against the primary store) andkb_graph— a real, live bypass oflocal_models_onlyprotection, not a hypothetical.What changed
ToolResidencyShapeclassification (SingleTarget/PrimaryOnly/ScopedFederatedScan/UnscopedFederatedContent/NonContent) covering all 55kb_*tools +help_open, with a fail-closed default for anything unrecognized.every_kb_tool_and_help_open_is_explicitly_classified— a CI-enforced exhaustiveness test that fails the build if a newkb_*tool is added without an explicit residency review, preventing this class of drift from recurring silently.kb_agenda's inverse over-blocking bug (was treated as federated-scan but only ever reads the primary store), andkb_links_to's wrong-entity-checked bug (was checking the target node's KB instead of the KBs its aggregated backlink sources actually come from).kb_search_context: addedscopeparam, fixedscore_nodeto weight alias/id matches, added body-match-position tie-break, consolidated its scan ontokb_federated_search_scoped.kb_agenda's scan behavior.resync_after_external_write) during the same sweep.Test plan
cargo test -p mae ai_residency— 32/32 passing (23 new: exhaustiveness test, fail-closed unclassified-tool test, per-shape scope/residency tests for every reclassified tool)cargo test -p mae-ai kb_search_context— 10/10 passing (2 new: tie-break, scope-filter)cargo test -p mae-ai score_node— 3/3 passingcargo build --workspace+cd daemon && cargo buildcargo test --workspace+cd daemon && cargo testcargo clippy --workspace --all-targets -- -D warnings(both workspaces)cargo fmt --checkmaeandmae-daemonlocally viamake install-upgrade, confirmed daemon reportsbuild="eb0ffcda22ee"(this branch's HEAD) and isactive🤖 Generated with Claude Code