mirror of
https://github.com/akitaonrails/ai-memory.git
synced 2026-10-02 03:24:46 +08:00
fix(mcp): union _global for single-project queries, not just unscoped ones
memory_query gated the reserved _global preferences union on 'no named workspace/project'. The routing doctrine tells static MCP clients to pass workspace+project on every call, which set that gate false, so static clients never received global_scope_hits despite the documented contract (#930). The union now keys on single-project resolution (scopes empty); an explicit multi-scopes set is the only opt-out, and global=true/as_of are unaffected. The double-search guard now resolves the queried project (named or active) rather than the active-project default. The union remains keyed strictly to the reserved _global scope and never leaks another project's pages (adversarial test + security-boundaries row 1b). Closes #930 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MDbhmszrjG9s5MrPrTuNtm
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
85c9ba36d7
commit
954548ffe1
@@ -8,6 +8,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- `memory_query` now returns `global_scope_hits` (standing `_global` user/team
|
||||
preferences) for a single-project query whose project is named explicitly
|
||||
with `workspace`+`project`, not only when scope is omitted. The routing
|
||||
doctrine tells static MCP clients to pass `workspace`+`project` on every call,
|
||||
which set the old gate's "no named scope" condition to false, so those clients
|
||||
never received global preferences despite the documented contract. The union
|
||||
now keys on single-project resolution (`scopes` empty); only an explicit
|
||||
multi-`scopes` set opts out, and `global=true`/`as_of` are unaffected. The
|
||||
reserved-scope union is still keyed strictly to `_global` and never leaks
|
||||
another project's pages. (#930)
|
||||
- `[capture] ignore_paths` now covers shell commands. Shell tools (`Bash`,
|
||||
`shell`, `execute_bash`, `terminal`, …) were classified as non-file and always
|
||||
kept, so `cat docs/adr/*.md` stored the ignored file's full text in the
|
||||
|
||||
@@ -198,10 +198,11 @@ developer, user, and canonical project instructions.\n\
|
||||
to propose architecture (always check first). Defaults to the \
|
||||
current project; pass `scopes` to search named sibling projects, \
|
||||
or `global=true` to search EVERY project at once when you don't \
|
||||
know where the knowledge lives. Default-scoped calls also return \
|
||||
`global_scope_hits` — standing user/team preferences from the \
|
||||
reserved `_global` scope; treat them as context that applies to \
|
||||
every project. Expired pages are hidden by default; use \
|
||||
know where the knowledge lives. Single-project calls — whether the \
|
||||
project is left implicit or named explicitly with `workspace`+`project` \
|
||||
— also return `global_scope_hits`: standing user/team preferences from \
|
||||
the reserved `_global` scope; treat them as context that applies to \
|
||||
every project. Only an explicit multi-`scopes` set opts out. Expired pages are hidden by default; use \
|
||||
`include_expired=true` only when the user explicitly wants to inspect \
|
||||
expired historical memory. Superseded (older) page versions are hidden \
|
||||
by default; pass `include_superseded=true` when the user wants a page's \
|
||||
@@ -2317,9 +2318,11 @@ impl AiMemoryServer {
|
||||
If compiled wiki search misses in default/project/`scopes` mode, \
|
||||
`raw_hits` contains bounded raw observation fallback matches; \
|
||||
`global=true` searches compiled wiki pages only and returns no raw \
|
||||
fallback. Default-scoped calls also return \
|
||||
fallback. Single-project calls (project implicit or named with \
|
||||
`workspace`+`project`) also return \
|
||||
`global_scope_hits`: standing user/team preferences from the \
|
||||
reserved `_global` scope that apply across projects. Set \
|
||||
reserved `_global` scope that apply across projects; only an explicit \
|
||||
multi-`scopes` set opts out. Set \
|
||||
`global=true` to search EVERY \
|
||||
project at once (cross-project) when you don't know which project \
|
||||
holds the knowledge — each hit then carries its workspace + \
|
||||
@@ -2569,25 +2572,32 @@ impl AiMemoryServer {
|
||||
// Default-scoped queries (no workspace/project/scopes/global args)
|
||||
// also union the reserved `_global` preferences scope, so standing
|
||||
// user/team context travels into every project without the caller
|
||||
// knowing a magic project name (issue #154). Explicit scoping means
|
||||
// the caller asked for exactly those scopes — leave it alone. One
|
||||
// extra scoped search when the scope exists; zero cost when it
|
||||
// doesn't.
|
||||
let default_scoped = args.scopes.is_empty()
|
||||
&& args
|
||||
.workspace
|
||||
.as_deref()
|
||||
.is_none_or(|s| s.trim().is_empty())
|
||||
&& args.project.as_deref().is_none_or(|s| s.trim().is_empty());
|
||||
let global_scope_hits = if default_scoped {
|
||||
// knowing a magic project name (issue #154). This unions for any query
|
||||
// that resolves to a single project — whether the project is left
|
||||
// implicit (active-project pointer) or named explicitly with
|
||||
// `workspace`+`project`. Naming the current project is exactly what the
|
||||
// static-client routing doctrine tells callers to do, so gating the
|
||||
// union on absent `workspace`/`project` silently denied global
|
||||
// preferences to every static client (#930). Only an explicit,
|
||||
// deliberately-narrowed multi-scope set (`scopes`) opts out — the
|
||||
// caller asked for exactly those scopes. One extra scoped search when
|
||||
// the reserved scope exists; zero cost when it doesn't.
|
||||
let single_project_scoped = args.scopes.is_empty();
|
||||
let global_scope_hits = if single_project_scoped {
|
||||
match ai_memory_store::lookup_global_scope(&self.reader).await {
|
||||
Ok(Some(scope)) => {
|
||||
// If the current project IS the reserved scope (e.g. the
|
||||
// actor's active-project pointer lands there after a
|
||||
// global write), `hits` already covers it — don't search
|
||||
// it twice.
|
||||
// If the project this query resolves to IS the reserved
|
||||
// scope (e.g. the caller named it, or the actor's
|
||||
// active-project pointer lands there after a global write),
|
||||
// `hits` already covers it — don't search it twice. Resolve
|
||||
// through the query's own `workspace`/`project` so a named
|
||||
// project is compared, not the active-project default.
|
||||
let current = self
|
||||
.effective_ids_for_read_args_with_actor(None, None, &aps_actor)
|
||||
.effective_ids_for_read_args_with_actor(
|
||||
args.workspace.as_deref(),
|
||||
args.project.as_deref(),
|
||||
&aps_actor,
|
||||
)
|
||||
.await?;
|
||||
if current == scope.as_tuple() {
|
||||
Vec::new()
|
||||
@@ -8309,7 +8319,7 @@ mod tests {
|
||||
// Issue #154: default-scoped queries union the reserved `_global`
|
||||
// preferences scope; explicitly scoped queries do not.
|
||||
#[tokio::test]
|
||||
async fn default_query_unions_global_scope_and_explicit_scope_skips_it() {
|
||||
async fn single_project_query_unions_global_scope_and_multi_scope_skips_it() {
|
||||
let (_tmp, store, server, _ws, _proj) = setup_server().await;
|
||||
let global = ai_memory_store::create_global_scope(&store.writer)
|
||||
.await
|
||||
@@ -8393,6 +8403,9 @@ mod tests {
|
||||
"the default query's reserved global-scope union must keep its explanation"
|
||||
);
|
||||
|
||||
// A query that names its single project explicitly with
|
||||
// `workspace`+`project` — exactly what the static-client routing
|
||||
// doctrine requires — still unions the reserved global scope (#930).
|
||||
let result = server
|
||||
.memory_query(
|
||||
Parameters(query(Some("default"), Some("scratch"))),
|
||||
@@ -8401,9 +8414,121 @@ mod tests {
|
||||
.await
|
||||
.unwrap();
|
||||
let text = result.content.first().and_then(|c| c.as_text()).unwrap();
|
||||
assert!(
|
||||
text.text.contains("global_scope_hits") && text.text.contains("preferences/style.md"),
|
||||
"a query naming its single project must still union the global scope (#930): {}",
|
||||
text.text
|
||||
);
|
||||
|
||||
// A deliberately-narrowed multi-scope set (`scopes`) is the one form
|
||||
// that opts out: the caller asked for exactly those scopes.
|
||||
let mut scoped = query(None, None);
|
||||
scoped.scopes = vec![MemoryScopeArg {
|
||||
workspace: "default".into(),
|
||||
project: "scratch".into(),
|
||||
}];
|
||||
let result = server
|
||||
.memory_query(Parameters(scoped), OptionalParts(test_parts_default()))
|
||||
.await
|
||||
.unwrap();
|
||||
let text = result.content.first().and_then(|c| c.as_text()).unwrap();
|
||||
assert!(
|
||||
!text.text.contains("preferences/style.md"),
|
||||
"explicitly scoped queries must not union the global scope: {}",
|
||||
"an explicit multi-scope query must not union the global scope: {}",
|
||||
text.text
|
||||
);
|
||||
}
|
||||
|
||||
// Adversarial (invariant #16): the reserved-scope union must surface ONLY
|
||||
// the `_global` scope's pages — never a different real project's pages.
|
||||
// Broadening the union to single-project queries (#930) must not become a
|
||||
// cross-project read. This fails if the union is ever mis-keyed to a real
|
||||
// project instead of the reserved global scope.
|
||||
#[tokio::test]
|
||||
async fn global_union_never_leaks_a_foreign_projects_pages() {
|
||||
let (_tmp, store, server, ws, _proj) = setup_server().await;
|
||||
|
||||
let global = ai_memory_store::create_global_scope(&store.writer)
|
||||
.await
|
||||
.unwrap();
|
||||
store
|
||||
.writer
|
||||
.upsert_page(NewPage {
|
||||
workspace_id: global.workspace_id,
|
||||
project_id: global.project_id,
|
||||
path: PagePath::new("preferences/sharedterm.md").unwrap(),
|
||||
title: "Reserved".into(),
|
||||
body: "sharedterm reserved global preference".into(),
|
||||
tier: Tier::Semantic,
|
||||
frontmatter_json: serde_json::json!({}),
|
||||
pinned: false,
|
||||
links: Vec::new(),
|
||||
author_id: None,
|
||||
expires_at: None,
|
||||
entities: Vec::new(),
|
||||
evidence: Vec::new(),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// A different real project in the same workspace, NOT the queried one
|
||||
// and NOT the reserved scope.
|
||||
let foreign = store
|
||||
.writer
|
||||
.get_or_create_project(ws, "foreign", None)
|
||||
.await
|
||||
.unwrap();
|
||||
store
|
||||
.writer
|
||||
.upsert_page(NewPage {
|
||||
workspace_id: ws,
|
||||
project_id: foreign,
|
||||
path: PagePath::new("leak.md").unwrap(),
|
||||
title: "Foreign".into(),
|
||||
body: "sharedterm foreign project must not leak".into(),
|
||||
tier: Tier::Semantic,
|
||||
frontmatter_json: serde_json::json!({}),
|
||||
pinned: false,
|
||||
links: Vec::new(),
|
||||
author_id: None,
|
||||
expires_at: None,
|
||||
entities: Vec::new(),
|
||||
evidence: Vec::new(),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let result = server
|
||||
.memory_query(
|
||||
Parameters(QueryArgs {
|
||||
query: "sharedterm".into(),
|
||||
limit: Some(10),
|
||||
project: Some("scratch".into()),
|
||||
scopes: Vec::new(),
|
||||
workspace: Some("default".into()),
|
||||
global: None,
|
||||
include_expired: None,
|
||||
include_superseded: None,
|
||||
pin_first: None,
|
||||
explain: None,
|
||||
as_of: None,
|
||||
answer: None,
|
||||
reasoning: None,
|
||||
}),
|
||||
OptionalParts(test_parts_default()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let text = result.content.first().and_then(|c| c.as_text()).unwrap();
|
||||
assert!(
|
||||
text.text.contains("global_scope_hits")
|
||||
&& text.text.contains("preferences/sharedterm.md"),
|
||||
"the reserved global page must surface for a single-project query: {}",
|
||||
text.text
|
||||
);
|
||||
assert!(
|
||||
!text.text.contains("leak.md"),
|
||||
"a foreign project's page must never surface via the global union: {}",
|
||||
text.text
|
||||
);
|
||||
}
|
||||
|
||||
@@ -440,7 +440,7 @@ invariants below.
|
||||
|
||||
| Tool | Hint | Purpose |
|
||||
|---|---|---|
|
||||
| `memory_query` | read-only | FTS5 + entity-match + graph RRF + optional vector RRF search, followed by bounded kind/tier/pinned/tag authority adjustment and raw fallback. Bumps access counters for page hits. Defaults to the current project; default-scoped calls also union the reserved `_global` preferences scope as `global_scope_hits`; `scopes` searches named sibling projects; `global=true` searches every project at once (each hit annotated with its workspace + project). With `AI_MEMORY_RERANKER=llm`, project/scopes candidate pools are fused before at most one final LLM relevance pass; query/title/snippet data is bounded and JSON-encoded, and any timeout, provider error, invalid/incomplete score set, or four-call concurrency saturation preserves the adjusted order. The distinct `global=true` FTS-only ranker and supplemental global-preference hits are not reranked. `explain=true` attaches per-hit `score_details` (per-stream ranks, matched entities, raw FTS/cosine/entity inverse-frequency scores, RRF contributions, graph provenance including the typed edge kind (`causes`/`fixes`/`contradicts`) a neighbour was reached by, the page's evidence count, authority multiplier, and optional rerank score) to project/scopes hits plus a top-level `streams_active` list. The global FTS-only ranker reports its active stream without per-hit details. `include_expired=true` also returns TTL-expired pages. `include_superseded=true` also returns superseded (non-latest) page versions across the FTS/entity/vector/graph streams, each hit labelled `superseded: true` (the current version is never marked); default-off is byte-identical to the latest-only behaviour, and `global=true` / `as_of` are unaffected. `pin_first=true` prepends the project's bounded pinned latest pages (`ReaderPool::list_pinned_pages`, cap 10) ahead of the fused hits, deduped by page id (a pinned page that also matches appears once, marked `pinned: true`) and re-truncated to the requested limit; it applies to single-project searches (default or `workspace`+`project`), is ignored on `scopes`/`global`/`as_of`, and default-off is byte-identical. `answer=true` (opt-in, off by default) additionally synthesizes a cited natural-language answer over the top hits via the configured LLM provider (`complete_structured`, JSON-schema `{ answer, citations }`), attached as `answer: { text, citations }`; with no provider configured it returns the hits plus an `answer_unavailable` note instead of erroring, and with `answer` unset/`false` no provider is accessed and the response is byte-identical (invariant #13). It applies to the normal single-project/`scopes` path; `global`/`as_of` ignore it. Answer quality is not yet eval-validated. An optional `reasoning` tier (`minimal` (default) / `low` / `medium` / `high` / `max`) tunes the synthesis effort: `ChatRequest` carries no per-request reasoning field (the provider-level `reasoning_effort` is fixed at construction from config), so the tier maps to a per-tier max-token budget scaled off the path's base (answer base 2 000; `minimal` = 1x = byte-identical, `low` 1.5x, `medium` 2x, `high` 3x, `max` 4x). The tier is inert unless the `answer` LLM path runs (invariant #13); an unknown value is rejected by the schema (invariant #7). |
|
||||
| `memory_query` | read-only | FTS5 + entity-match + graph RRF + optional vector RRF search, followed by bounded kind/tier/pinned/tag authority adjustment and raw fallback. Bumps access counters for page hits. Defaults to the current project; single-project calls (project implicit or named with `workspace`+`project`) also union the reserved `_global` preferences scope as `global_scope_hits`, and only an explicit multi-`scopes` set opts out (#930); `scopes` searches named sibling projects; `global=true` searches every project at once (each hit annotated with its workspace + project). With `AI_MEMORY_RERANKER=llm`, project/scopes candidate pools are fused before at most one final LLM relevance pass; query/title/snippet data is bounded and JSON-encoded, and any timeout, provider error, invalid/incomplete score set, or four-call concurrency saturation preserves the adjusted order. The distinct `global=true` FTS-only ranker and supplemental global-preference hits are not reranked. `explain=true` attaches per-hit `score_details` (per-stream ranks, matched entities, raw FTS/cosine/entity inverse-frequency scores, RRF contributions, graph provenance including the typed edge kind (`causes`/`fixes`/`contradicts`) a neighbour was reached by, the page's evidence count, authority multiplier, and optional rerank score) to project/scopes hits plus a top-level `streams_active` list. The global FTS-only ranker reports its active stream without per-hit details. `include_expired=true` also returns TTL-expired pages. `include_superseded=true` also returns superseded (non-latest) page versions across the FTS/entity/vector/graph streams, each hit labelled `superseded: true` (the current version is never marked); default-off is byte-identical to the latest-only behaviour, and `global=true` / `as_of` are unaffected. `pin_first=true` prepends the project's bounded pinned latest pages (`ReaderPool::list_pinned_pages`, cap 10) ahead of the fused hits, deduped by page id (a pinned page that also matches appears once, marked `pinned: true`) and re-truncated to the requested limit; it applies to single-project searches (default or `workspace`+`project`), is ignored on `scopes`/`global`/`as_of`, and default-off is byte-identical. `answer=true` (opt-in, off by default) additionally synthesizes a cited natural-language answer over the top hits via the configured LLM provider (`complete_structured`, JSON-schema `{ answer, citations }`), attached as `answer: { text, citations }`; with no provider configured it returns the hits plus an `answer_unavailable` note instead of erroring, and with `answer` unset/`false` no provider is accessed and the response is byte-identical (invariant #13). It applies to the normal single-project/`scopes` path; `global`/`as_of` ignore it. Answer quality is not yet eval-validated. An optional `reasoning` tier (`minimal` (default) / `low` / `medium` / `high` / `max`) tunes the synthesis effort: `ChatRequest` carries no per-request reasoning field (the provider-level `reasoning_effort` is fixed at construction from config), so the tier maps to a per-tier max-token budget scaled off the path's base (answer base 2 000; `minimal` = 1x = byte-identical, `low` 1.5x, `medium` 2x, `high` 3x, `max` 4x). The tier is inert unless the `answer` LLM path runs (invariant #13); an unknown value is rejected by the schema (invariant #7). |
|
||||
| `memory_recent` | read-only | Most-recently-updated `is_latest=1` pages. |
|
||||
| `memory_read_page` | read-only | Fetch the FULL body of a single wiki page by `path` or by top FTS5 hit for a `query`; optional `workspace` + `project` targets a named sibling workspace/project. Use when an agent needs more than the 24-word snippets from `memory_query`. `include_related=true` also walks the link graph outward from the page (bounded BFS reusing the `page_links` primitive per node: default 1 hop, hard cap 3, global visited set for dedup/cycle-safety, total-node cap 50, cross-project aware) and returns a `related` array of reachable pages, each labelled with its `depth` (hop distance) and `direction` (`link`/`backlink`); default-off is byte-identical (no `related` field). |
|
||||
| `memory_read_session_observations` | read-only | Page through ONE session's raw hook observations (`ObservationRecord` with full sanitized body, capped per row by `body_max_chars`), restricted to the rows that landed in the resolved scope and to sessions the caller may see; `total` and `elided_other_scope` report the in-scope count and the rows the session left in another project. `session_id` omitted reads the latest completed visible session. |
|
||||
|
||||
@@ -23,6 +23,7 @@ boundary not yet built.
|
||||
| # | Boundary | Enforcing code | Adversarial test(s) | Coverage |
|
||||
|---|----------|----------------|---------------------|----------|
|
||||
| 1 | Per-project isolation (3-tuple) | `ai-memory-store/src/scope.rs` `ScopeResolver::resolve_read_args`/`resolve_write_args`, no-create `lookup_existing_scope`; reader queries filter by `(workspace_id, project_id)` | `store` `scope.rs` read-resolution table tests; `tests/suite/multi_session.rs` | STRONG |
|
||||
| 1b | Reserved `_global` scope union never leaks a foreign project (#930) | `ai-memory-mcp/src/server.rs` `memory_query` — the union runs for single-project queries (`scopes` empty) and searches **only** `lookup_global_scope`'s reserved `(workspace_id, project_id)`, never an arbitrary project; the double-search guard resolves the *queried* project (named or active) so it can't be tricked into skipping | `server.rs` `global_union_never_leaks_a_foreign_projects_pages` (a third real project's page must not surface via the union; reserved page must), `single_project_query_unions_global_scope_and_multi_scope_skips_it` | STRONG |
|
||||
| 2 | Workspace isolation | same as #1; same-named project → distinct ids per workspace | `scope.rs` cross-workspace resolution rows; `multi_scope` dedup/validate | STRONG |
|
||||
| 3 | Multi-user auth ladder | `ai-memory-core/src/actor.rs` `AuthLevel::authorize`; `ai-memory-mcp/src/auth.rs` middleware; `admin.rs` `require_root_for_multiuser_admin` / `require_root` | `admin.rs` `multiuser_admin_routes_reject_db_user_tier` / `…reject_anonymous` / `create_user_as_user_tier_returns_403`; `auth.rs` unknown-bearer 401; `actor.rs` `skip_admission_chain_rejects_db_users` | STRONG |
|
||||
| 4 | Handoff single-claim / no-steal | `ai-memory-store/src/ops.rs` `accept_handoff_in_transaction` (metadata `state='open'` guard + atomic CAS) | `multi_session.rs` `a_second_accept_cannot_steal_an_accepted_handoff`; `handoff_ownership.rs` `another_operator_cannot_claim_the_handoff` | STRONG |
|
||||
|
||||
Reference in New Issue
Block a user