diff --git a/crates/khive-pack-brain/src/handlers.rs b/crates/khive-pack-brain/src/handlers.rs index ca4496b5..9d6374b7 100644 --- a/crates/khive-pack-brain/src/handlers.rs +++ b/crates/khive-pack-brain/src/handlers.rs @@ -878,19 +878,19 @@ impl BrainPack { } }; - // Validate target_id in the PRIMARY namespace only. A visible-only - // (foreign) record must not be targeted by a mutation (feedback event). - // NotFound (not Forbidden) per ADR-007:215-219. + // Resolve target_id over the caller's VISIBLE set — the same visibility recall + // uses. Feedback targets are recall outputs; a record recall can surface must be + // feedback-targetable, or the feedback-discipline loop breaks for every memory + // stamped with an actor-id namespace (ADR-007 Rev 6). NotFound (not Forbidden) + // when the id is neither owned nor visible. let resolved = self .runtime - .resolve_primary(token, target) + .resolve(token, target) .await .map_err(|e| RuntimeError::InvalidInput(e.to_string()))?; if resolved.is_none() { return Err(RuntimeError::NotFound(format!( - "target_id {:?} not found in namespace {:?}", - target, - token.namespace().as_str() + "target_id {target:?} not found or not visible to caller" ))); } @@ -1608,7 +1608,7 @@ pub(crate) async fn resolve_auto_feedback_target( } if raw.len() >= 8 && raw.chars().all(|c| c.is_ascii_hexdigit()) { return runtime - .resolve_prefix(token, raw) + .resolve_prefix_visible(token, raw) .await .map_err(|e| RuntimeError::InvalidInput(e.to_string()))? .ok_or_else(|| { diff --git a/crates/khive-pack-brain/src/tests.rs b/crates/khive-pack-brain/src/tests.rs index 656b65ad..1d0fac90 100644 --- a/crates/khive-pack-brain/src/tests.rs +++ b/crates/khive-pack-brain/src/tests.rs @@ -3952,6 +3952,270 @@ async fn feedback_accepts_short_prefix_target_id() { assert_eq!(result["signal"], json!("useful"), "signal must round-trip"); } +// ── #391: feedback target resolution over the caller's VISIBLE set ────────── +// +// Root cause: ADR-007 Rev 6 stamps EPISODIC memory notes with the caller's +// actor-id namespace (memory pack `remember.rs`), but `brain.feedback` / +// `brain.auto_feedback` resolved targets against the caller's PRIMARY +// namespace only (`resolve_primary` / `resolve_prefix`). Every episodic +// memory written after the Rev-6 rollout became feedback-unreachable. The +// fix widens resolution to the caller's full visible set (`resolve` / +// `resolve_prefix_visible`), mirroring how `memory.recall` already surfaces +// records across the visible set. Strict `resolve_primary` / `resolve_prefix` +// remain untouched for mutation-validation callers (gtd depends_on, link +// endpoints, and non-KG prefix validators). KG by-ID CRUD +// (get/update/delete/merge) is the OTHER policy in this fix: its prefix +// resolution intentionally moved to `resolve_uuid_unfiltered` — no namespace +// boundary at all, matching the full-UUID by-ID contract — see +// `khive-pack-gtd/tests/dependencies.rs::resolve_primary_rejects_visible_only_*` +// and `khive-runtime/tests/integration.rs` visible-set tests. +// +// khive-pack-brain has no dependency on khive-pack-memory, so these tests +// create notes directly via `rt.create_note(token, "memory", ...)` to +// reproduce the actor-ns-stamped shape without pulling in the memory pack. +// Note-kind validation is a no-op on a bare `KhiveRuntime::memory()` runtime +// (no packs installed → `validate_note_kind` allows any kind), so "memory" +// here behaves exactly like a real episodic memory row for resolution +// purposes — the bug is about namespace visibility, not note kind. + +/// Shape 1 (baseline, must stay green): a note in the caller's own `local` +/// primary namespace resolves via `brain.feedback` today and after the fix. +#[tokio::test] +async fn test_391_feedback_resolves_note_in_local_namespace_baseline() { + let (pack, rt) = make_pack(); + let registry = empty_registry(); + let token = rt.authorize(Namespace::local()).unwrap(); + + let note = rt + .create_note( + &token, + "memory", + None, + "local baseline memory", + None, + None, + vec![], + ) + .await + .expect("create baseline memory note"); + + let result = pack + .dispatch( + "brain.feedback", + json!({ + "target_id": note.id.to_string(), + "signal": "useful", + "served_by_profile_id": "balanced-recall-v1" + }), + ®istry, + &token, + ) + .await + .expect("baseline: feedback on a local-namespace memory note must resolve"); + assert_eq!(result["emitted"], json!(true)); +} + +/// Shape 2, full-UUID form (leg C): an episodic-shaped note stamped with the +/// caller's actor-id namespace (`lambda:test-391`, not `local`) must resolve +/// via `brain.feedback`'s exact-UUID path once the caller's visible set +/// includes that namespace. Before the fix (`resolve_primary`, primary-only) +/// this returned NotFound; after the fix (`resolve`, visible-set-aware) it +/// must succeed. +#[tokio::test] +async fn test_391_feedback_resolves_actor_namespace_note_full_uuid() { + let (pack, rt) = make_pack(); + let registry = empty_registry(); + + let actor_ns = Namespace::parse("lambda:test-391").unwrap(); + let writer_token = rt.authorize(actor_ns.clone()).unwrap(); + let note = rt + .create_note( + &writer_token, + "memory", + None, + "actor-ns memory", + None, + None, + vec![], + ) + .await + .expect("create actor-ns memory note"); + + // Caller's primary is `local`; visible set additionally covers the actor ns + // (ADR-007 Rev 6 visible set = {local} ∪ {actor.id} ∪ {actor.visible_namespaces}). + let caller_token = rt + .authorize_with_visibility(Namespace::local(), vec![actor_ns]) + .unwrap(); + + let result = pack + .dispatch( + "brain.feedback", + json!({ + "target_id": note.id.to_string(), + "signal": "useful", + "served_by_profile_id": "balanced-recall-v1" + }), + ®istry, + &caller_token, + ) + .await + .expect( + "#391 leg C: feedback on an actor-ns memory note visible to the caller must resolve", + ); + assert_eq!(result["emitted"], json!(true)); +} + +/// Shape 2, 8-char-prefix form (leg B, cascading into leg C): the same +/// actor-ns note, targeted by hex prefix via `brain.auto_feedback` (the +/// `memory.recall` → `brain.auto_feedback` calling convention). Exercises +/// `resolve_auto_feedback_target`'s prefix branch (`resolve_prefix_visible`) +/// feeding into `handle_feedback`'s visible-aware `resolve`. +#[tokio::test] +async fn test_391_auto_feedback_resolves_actor_namespace_note_prefix() { + let (pack, rt) = make_pack(); + let registry = empty_registry(); + + let actor_ns = Namespace::parse("lambda:test-391b").unwrap(); + let writer_token = rt.authorize(actor_ns.clone()).unwrap(); + let note = rt + .create_note( + &writer_token, + "memory", + None, + "actor-ns memory (prefix)", + None, + None, + vec![], + ) + .await + .expect("create actor-ns memory note"); + let full_id = note.id.to_string(); + let prefix = &full_id[..8]; + + let caller_token = rt + .authorize_with_visibility(Namespace::local(), vec![actor_ns]) + .unwrap(); + + let result = pack + .dispatch( + "brain.auto_feedback", + json!({ + "query": "#391 actor-ns prefix regression", + "results": [{ "id": prefix }] + }), + ®istry, + &caller_token, + ) + .await + .expect( + "#391 leg B: auto_feedback prefix-resolving an actor-ns memory note visible to the caller must resolve", + ); + assert_eq!(result["emitted"], json!(true)); + assert_eq!( + result["target_id"].as_str().unwrap_or(""), + full_id, + "resolved target_id must match the full actor-ns note UUID" + ); +} + +/// Shape 3: a note under an explicit namespace override (a cross-agent grant +/// via `authorize_with_visibility`'s `extra_visible`, not the caller's own +/// actor id) must also resolve — proving the fix widens to the general +/// visible set, not just the actor-id special case. +#[tokio::test] +async fn test_391_feedback_resolves_note_under_explicit_namespace_override() { + let (pack, rt) = make_pack(); + let registry = empty_registry(); + + let shared_ns = Namespace::parse("explicit-shared-ns-391").unwrap(); + let writer_token = rt.authorize(shared_ns.clone()).unwrap(); + let note = rt + .create_note( + &writer_token, + "memory", + None, + "explicit-ns memory", + None, + None, + vec![], + ) + .await + .expect("create explicit-namespace memory note"); + + let caller_token = rt + .authorize_with_visibility(Namespace::local(), vec![shared_ns]) + .unwrap(); + + let result = pack + .dispatch( + "brain.feedback", + json!({ + "target_id": note.id.to_string(), + "signal": "useful", + "served_by_profile_id": "balanced-recall-v1" + }), + ®istry, + &caller_token, + ) + .await + .expect( + "#391 shape 3: feedback on a note under an explicit visible-namespace override must resolve", + ); + assert_eq!(result["emitted"], json!(true)); +} + +/// Negative: a note in a namespace that is NOT in the caller's visible set +/// (neither primary nor an extra-visible grant) must still fail `NotFound`. +/// The fix widens resolution to the visible set — it must not widen to every +/// namespace in the database. +#[tokio::test] +async fn test_391_feedback_rejects_note_outside_visible_set() { + let (pack, rt) = make_pack(); + let registry = empty_registry(); + + let hidden_ns = Namespace::parse("invisible-ns-391").unwrap(); + let writer_token = rt.authorize(hidden_ns).unwrap(); + let note = rt + .create_note( + &writer_token, + "memory", + None, + "hidden memory", + None, + None, + vec![], + ) + .await + .expect("create hidden-namespace memory note"); + + // Caller's visible set covers local + a DIFFERENT extra namespace — not the + // note's namespace. + let caller_token = rt + .authorize_with_visibility( + Namespace::local(), + vec![Namespace::parse("some-other-visible-ns-391").unwrap()], + ) + .unwrap(); + + let err = pack + .dispatch( + "brain.feedback", + json!({ + "target_id": note.id.to_string(), + "signal": "useful", + "served_by_profile_id": "balanced-recall-v1" + }), + ®istry, + &caller_token, + ) + .await + .unwrap_err(); + assert!( + matches!(err, RuntimeError::NotFound(_)), + "#391: feedback on a note outside the caller's visible set must return NotFound, got {err:?}" + ); +} + // ── #354: brain.register_adapter ───────────────────────────────────────────── // ACCEPT: matching base_model_revision registers the adapter and persists the artifact. diff --git a/crates/khive-pack-brain/tests/dispatch_hook.rs b/crates/khive-pack-brain/tests/dispatch_hook.rs index cd57638a..263239da 100644 --- a/crates/khive-pack-brain/tests/dispatch_hook.rs +++ b/crates/khive-pack-brain/tests/dispatch_hook.rs @@ -320,21 +320,37 @@ async fn cold_hook_signal_applies_on_top_of_persisted_snapshot() { ); } -// ---- Finding 3 regression: brain.feedback target_id must be primary-only ---- +// ---- #391 supersedes "Finding 3" (PR #127): brain.feedback resolves over ---- +// ---- the caller's VISIBLE set, not primary-only ---- -/// `brain.feedback` must reject a `target_id` that lives in a visible (non-primary) -/// namespace. A visible-only record must not be the target of a feedback mutation. +/// `brain.feedback` must ACCEPT a `target_id` that lives in a visible +/// (non-primary) namespace. /// -/// This tests that the fix from `resolve` to `resolve_primary` is in effect: -/// a record the caller can READ but not MUTATE returns NotFound on feedback. +/// PR #127 originally locked feedback to primary-only resolution +/// (`resolve_primary`) under a general "mutation reference validation must be +/// primary-only" principle, reasoning that feedback mutates its target. +/// Issue #391 (Leo-ruled, 2026-07-01) corrected that framing: `brain.feedback` +/// appends a NEW ledger event that *references* the target — it does not +/// mutate the target row — so target resolution is read-class, the same +/// class as `memory.recall`. Under ADR-007 Rev 6, EPISODIC memory notes are +/// stamped with the caller's actor-id namespace, so primary-only resolution +/// made every episodic memory feedback-unreachable the moment it was written +/// under an actor-id namespace different from the caller's session-primary — +/// a fleet-wide feedback-discipline outage. The fix moved feedback's target +/// resolution from `resolve_primary` to `resolve` (visible-set-aware), +/// mirroring the visibility recall already uses. This test (formerly +/// `brain_feedback_rejects_visible_only_target_id`) now asserts the +/// corrected contract; see `khive-pack-brain/src/tests.rs` `test_391_*` for +/// the full 3-shape + negative regression suite (including the still-strict +/// `resolve_primary` contract used by mutation-validation callers elsewhere). #[tokio::test] -async fn brain_feedback_rejects_visible_only_target_id() { +async fn brain_feedback_accepts_visible_only_target_id() { let rt = KhiveRuntime::memory().expect("in-memory runtime"); let ns_primary = Namespace::parse("brain-primary-ns").unwrap(); let ns_foreign = Namespace::parse("brain-foreign-ns").unwrap(); - // Create a KG entity in the foreign namespace. + // Create a KG entity in the foreign (visible-only) namespace. let tok_foreign = rt.authorize(ns_foreign.clone()).unwrap(); let foreign_entity = rt .create_entity( @@ -362,7 +378,8 @@ async fn brain_feedback_rejects_visible_only_target_id() { "visible-set token must be able to read the foreign entity; got: {found:?}" ); - // brain.feedback with the foreign entity as target_id must fail NotFound. + // brain.feedback with the foreign entity as target_id must now succeed: + // feedback is downstream of recall and follows recall's visibility. let brain = BrainPack::new(rt.clone()); let empty_registry = VerbRegistryBuilder::new().build().unwrap(); @@ -372,7 +389,7 @@ async fn brain_feedback_rejects_visible_only_target_id() { .await .expect("promote primary namespace"); - let err = brain + let result = brain .dispatch( "brain.feedback", json!({ "target_id": foreign_id, "signal": "useful" }), @@ -380,11 +397,7 @@ async fn brain_feedback_rejects_visible_only_target_id() { &tok_vis, ) .await - .unwrap_err(); + .expect("#391: brain.feedback with a visible-only target_id must resolve"); - let msg = err.to_string(); - assert!( - msg.to_lowercase().contains("not found"), - "brain.feedback with visible-only target_id must return NotFound; got: {msg}" - ); + assert_eq!(result["emitted"], json!(true), "emitted must be true"); } diff --git a/crates/khive-pack-kg/src/handlers/common.rs b/crates/khive-pack-kg/src/handlers/common.rs index d3a1ec5e..4725b113 100644 --- a/crates/khive-pack-kg/src/handlers/common.rs +++ b/crates/khive-pack-kg/src/handlers/common.rs @@ -260,7 +260,14 @@ pub(crate) async fn resolve_uuid_async( resolve_name_async(s, runtime, token).await } -pub(crate) async fn resolve_uuid_including_deleted( +/// By-ID contract (ADR-007 Rev 6): UUID resolution for get/update/delete/merge +/// is namespace-agnostic — the Gate is the authz seam, not storage-layer +/// filtering. Full-UUID inputs were already unfiltered (`resolve_by_id`); this +/// closes the gap for the *prefix* form, which previously fell through to the +/// primary-namespace-only `resolve_prefix` and was invisible for any row +/// stamped with a non-primary namespace (#391 §3). Exact copy of +/// `resolve_uuid_async` except the prefix branch. +pub(crate) async fn resolve_uuid_unfiltered( s: &str, runtime: &KhiveRuntime, token: &NamespaceToken, @@ -269,7 +276,32 @@ pub(crate) async fn resolve_uuid_including_deleted( return Ok(uuid); } if s.len() >= 8 && s.chars().all(|c| c.is_ascii_hexdigit()) { - match runtime.resolve_prefix_including_deleted(token, s).await { + match runtime.resolve_prefix_unfiltered(s).await { + Ok(Some(uuid)) => return Ok(uuid), + Ok(None) => { + return Err(RuntimeError::InvalidInput(format!( + "no record matches prefix: {s:?}" + ))) + } + Err(e) => return Err(e), + } + } + resolve_name_async(s, runtime, token).await +} + +/// `resolve_uuid_unfiltered`, including soft-deleted rows — used by the +/// hard-delete by-ID path (#391 §3). Exact copy of +/// `resolve_uuid_including_deleted` except the prefix branch. +pub(crate) async fn resolve_uuid_unfiltered_including_deleted( + s: &str, + runtime: &KhiveRuntime, + token: &NamespaceToken, +) -> Result { + if let Ok(uuid) = Uuid::from_str(s) { + return Ok(uuid); + } + if s.len() >= 8 && s.chars().all(|c| c.is_ascii_hexdigit()) { + match runtime.resolve_prefix_unfiltered_including_deleted(s).await { Ok(Some(uuid)) => return Ok(uuid), Ok(None) => { return Err(RuntimeError::InvalidInput(format!( diff --git a/crates/khive-pack-kg/src/handlers/get.rs b/crates/khive-pack-kg/src/handlers/get.rs index 8e783b9e..37cd1e54 100644 --- a/crates/khive-pack-kg/src/handlers/get.rs +++ b/crates/khive-pack-kg/src/handlers/get.rs @@ -11,7 +11,7 @@ use khive_types::EventKind; use super::common::{ deser, flatten_get_result, normalize_entity_timestamps, normalize_event_timestamps, - remap_note_status, resolve_uuid_async, to_json, GetParams, + remap_note_status, resolve_uuid_unfiltered, to_json, GetParams, }; use crate::KgPack; @@ -25,9 +25,11 @@ impl KgPack { ) -> Result { let p: GetParams = deser(params)?; - let id = if let Ok(id) = resolve_uuid_async(&p.id, &self.runtime, graph_token).await { + // By-ID resolution (including the hex-prefix form) is namespace-agnostic + // (ADR-007 Rev 6 / #391 §3) — the Gate is the authz seam, not this lookup. + let id = if let Ok(id) = resolve_uuid_unfiltered(&p.id, &self.runtime, graph_token).await { id - } else if let Ok(id) = resolve_uuid_async(&p.id, &self.runtime, token).await { + } else if let Ok(id) = resolve_uuid_unfiltered(&p.id, &self.runtime, token).await { id } else { if let Some(payload_val) = self.try_get_proposal_payload(token, &p.id).await? { diff --git a/crates/khive-pack-kg/src/handlers/merge.rs b/crates/khive-pack-kg/src/handlers/merge.rs index bf8fa2b1..23831c3e 100644 --- a/crates/khive-pack-kg/src/handlers/merge.rs +++ b/crates/khive-pack-kg/src/handlers/merge.rs @@ -6,7 +6,8 @@ use khive_runtime::{NamespaceToken, RuntimeError, VerbRegistry}; use super::common::{ deser, ensure_entity_kind, ensure_note_kind, immutable_event_error, parse_content_strategy, - parse_entity_policy, resolve_kind_spec, resolve_uuid_async, to_json, KindSpec, MergeParams, + parse_entity_policy, resolve_kind_spec, resolve_uuid_unfiltered, to_json, KindSpec, + MergeParams, }; use crate::KgPack; @@ -18,8 +19,10 @@ impl KgPack { registry: &VerbRegistry, ) -> Result { let p: MergeParams = deser(params)?; - let into_id = resolve_uuid_async(&p.into_id, &self.runtime, token).await?; - let from_id = resolve_uuid_async(&p.from_id, &self.runtime, token).await?; + // By-ID resolution (including the hex-prefix form) is namespace-agnostic + // (ADR-007 Rev 6 / #391 §3) — the Gate is the authz seam, not this lookup. + let into_id = resolve_uuid_unfiltered(&p.into_id, &self.runtime, token).await?; + let from_id = resolve_uuid_unfiltered(&p.from_id, &self.runtime, token).await?; let raw_kind = p.kind.as_deref().unwrap_or("entity"); let spec = resolve_kind_spec(raw_kind, registry)?; let policy = parse_entity_policy(p.strategy.as_deref().unwrap_or("prefer_into"))?; diff --git a/crates/khive-pack-kg/src/handlers/update.rs b/crates/khive-pack-kg/src/handlers/update.rs index 31277790..4fb8f722 100644 --- a/crates/khive-pack-kg/src/handlers/update.rs +++ b/crates/khive-pack-kg/src/handlers/update.rs @@ -9,8 +9,9 @@ use khive_runtime::{ use super::common::{ description_patch, deser, immutable_event_error, normalize_entity_timestamps, - optional_string_patch, parse_relation, resolve_kind_spec, resolve_uuid_async, - resolve_uuid_including_deleted, string_value, to_json, DeleteParams, KindSpec, UpdateParams, + optional_string_patch, parse_relation, resolve_kind_spec, resolve_uuid_unfiltered, + resolve_uuid_unfiltered_including_deleted, string_value, to_json, DeleteParams, KindSpec, + UpdateParams, }; use crate::KgPack; @@ -166,7 +167,9 @@ impl KgPack { } else { None }; - let id = resolve_uuid_async(&p.id, &self.runtime, token).await?; + // By-ID resolution (including the hex-prefix form) is namespace-agnostic + // (ADR-007 Rev 6 / #391 §3) — the Gate is the authz seam, not this lookup. + let id = resolve_uuid_unfiltered(&p.id, &self.runtime, token).await?; let spec: KindSpec = match explicit_spec { Some(s) => s, None => match self.infer_kind_from_uuid(token, id, &p.id).await { @@ -260,10 +263,12 @@ impl KgPack { } else { None }; + // By-ID resolution (including the hex-prefix form) is namespace-agnostic + // (ADR-007 Rev 6 / #391 §3) — the Gate is the authz seam, not this lookup. let id = if hard { - resolve_uuid_including_deleted(&p.id, &self.runtime, token).await? + resolve_uuid_unfiltered_including_deleted(&p.id, &self.runtime, token).await? } else { - resolve_uuid_async(&p.id, &self.runtime, token).await? + resolve_uuid_unfiltered(&p.id, &self.runtime, token).await? }; let spec: Option = match explicit_spec { Some(s) => Some(s), diff --git a/crates/khive-pack-kg/tests/integration.rs b/crates/khive-pack-kg/tests/integration.rs index 3ef27306..6555dbe2 100644 --- a/crates/khive-pack-kg/tests/integration.rs +++ b/crates/khive-pack-kg/tests/integration.rs @@ -6280,6 +6280,279 @@ async fn hard_delete_soft_deleted_note_cross_namespace_succeeds() { ); } +// ---- #391 §3: by-ID CRUD prefix resolution is fully unfiltered ---- +// +// The `*_cross_namespace_succeeds` tests above already lock in that a FULL +// UUID by-ID lookup is namespace-agnostic (ADR-007 Rev 2). Before #391, the +// *prefix* form of the same by-ID lookup fell through to the +// primary-namespace-only `resolve_prefix`, so a caller in a different +// namespace than the record got `NotFound` for a prefix that would have +// resolved fine as a full UUID — the outage's by-ID-adjacent half. These +// tests lock in the fix: prefix resolution is now unfiltered too, matching +// the full-UUID contract. Distinct from feedback's visible-set policy +// (`brain_feedback_accepts_visible_only_target_id` in +// `khive-pack-brain/tests/dispatch_hook.rs`) — by-ID CRUD has no visibility +// boundary at all; the Gate is the authz seam. + +/// `get` by short prefix from a caller in a DIFFERENT namespace than the +/// record must now succeed (was `NotFound` pre-#391). +#[tokio::test] +async fn get_entity_by_prefix_cross_namespace_succeeds() { + let f = pack(); + + let created = f + .dispatch( + "create", + json!({"kind": "concept", "name": "PrefixCrossNsGet"}), + ) + .await + .expect("create must succeed"); + let full_id = created["id"].as_str().unwrap().to_string(); + let prefix = &full_id[..8]; + + let result = f + .dispatch("get", json!({"id": prefix, "namespace": "ns-caller"})) + .await; + assert!( + result.is_ok(), + "get by prefix from a different namespace must succeed (#391 §3); got: {result:?}" + ); + assert_eq!(result.unwrap()["id"].as_str(), Some(full_id.as_str())); +} + +/// `get` by full UUID from a different namespace still succeeds — guards the +/// already-working path this fix must not regress. +#[tokio::test] +async fn get_entity_by_full_uuid_cross_namespace_still_succeeds() { + let f = pack(); + + let created = f + .dispatch( + "create", + json!({"kind": "concept", "name": "FullUuidCrossNsGet"}), + ) + .await + .expect("create must succeed"); + let full_id = created["id"].as_str().unwrap().to_string(); + + let result = f + .dispatch("get", json!({"id": full_id, "namespace": "ns-caller"})) + .await; + assert!( + result.is_ok(), + "get by full UUID from a different namespace must still succeed; got: {result:?}" + ); +} + +/// `update` by short prefix from a different namespace must now succeed. +#[tokio::test] +async fn update_entity_by_prefix_cross_namespace_succeeds() { + let f = pack(); + + let created = f + .dispatch( + "create", + json!({"kind": "concept", "name": "PrefixCrossNsUpdate"}), + ) + .await + .expect("create must succeed"); + let full_id = created["id"].as_str().unwrap().to_string(); + let prefix = &full_id[..8]; + + let result = f + .dispatch( + "update", + json!({"id": prefix, "namespace": "ns-caller", "description": "updated via prefix"}), + ) + .await; + assert!( + result.is_ok(), + "update by prefix from a different namespace must succeed (#391 §3); got: {result:?}" + ); +} + +/// `delete` (soft) by short prefix from a different namespace must now succeed. +#[tokio::test] +async fn soft_delete_entity_by_prefix_cross_namespace_succeeds() { + let f = pack(); + + let created = f + .dispatch( + "create", + json!({"kind": "concept", "name": "PrefixCrossNsSoftDelete"}), + ) + .await + .expect("create must succeed"); + let full_id = created["id"].as_str().unwrap().to_string(); + let prefix = &full_id[..8]; + + let result = f + .dispatch("delete", json!({"id": prefix, "namespace": "ns-caller"})) + .await; + assert!( + result.is_ok(), + "soft delete by prefix from a different namespace must succeed (#391 §3); got: {result:?}" + ); +} + +/// `delete(hard=true)` by short prefix from a different namespace must now +/// succeed — mirrors `hard_delete_soft_deleted_entity_cross_namespace_succeeds` +/// but resolves via prefix instead of full UUID. +#[tokio::test] +async fn hard_delete_entity_by_prefix_cross_namespace_succeeds() { + let f = pack(); + + let created = f + .dispatch( + "create", + json!({"kind": "concept", "name": "PrefixCrossNsHardDelete"}), + ) + .await + .expect("create must succeed"); + let full_id = created["id"].as_str().unwrap().to_string(); + let prefix = &full_id[..8]; + + f.dispatch("delete", json!({"id": full_id})) + .await + .expect("soft delete must succeed"); + + let result = f + .dispatch( + "delete", + json!({"id": prefix, "hard": true, "namespace": "ns-caller"}), + ) + .await; + assert!( + result.is_ok(), + "hard delete by prefix from a different namespace must succeed (#391 §3); got: {result:?}" + ); + assert_eq!( + result.unwrap().get("deleted").and_then(Value::as_bool), + Some(true) + ); +} + +/// `merge` resolves both `into_id` and `from_id` by short prefix even when the +/// caller is in a DIFFERENT namespace than the records — id resolution is no +/// longer the blocker (pre-#391 this failed at resolution with +/// `InvalidInput("no record matches prefix...")`). `merge_entity` separately +/// enforces its OWN namespace-ownership check deep in the merge transaction +/// (`curation.rs::merge_entity`, `belongs to namespace` — entirely downstream +/// of and unrelated to id resolution). That is a deliberate, pre-existing +/// mutation-safety constraint, not part of the #391 outage, and out of scope +/// here (SPEC's own "merge if cheap" hedge: this is not cheap, it is a +/// distinct design question). This test locks in exactly that boundary: +/// resolution now succeeds, the ownership check still legitimately blocks the +/// cross-namespace merge, and the failure mode is the ownership error, not a +/// resolution error. +#[tokio::test] +async fn merge_entities_by_prefix_cross_namespace_resolves_then_hits_ownership_check() { + let f = pack(); + + let into_full = f + .dispatch( + "create", + json!({"kind": "concept", "name": "PrefixCrossNsMergeInto"}), + ) + .await + .expect("create into-entity must succeed")["id"] + .as_str() + .unwrap() + .to_string(); + let from_full = f + .dispatch( + "create", + json!({"kind": "concept", "name": "PrefixCrossNsMergeFrom"}), + ) + .await + .expect("create from-entity must succeed")["id"] + .as_str() + .unwrap() + .to_string(); + + let into_prefix = &into_full[..8]; + let from_prefix = &from_full[..8]; + + let err = f + .dispatch( + "merge", + json!({"into_id": into_prefix, "from_id": from_prefix, "namespace": "ns-caller"}), + ) + .await + .expect_err( + "cross-namespace merge must still fail (ownership check), just NOT at id resolution", + ); + + let msg = err.to_string(); + assert!( + !msg.contains("no record matches prefix"), + "#391 §3: prefix resolution must no longer be the failure — got a resolution-layer error: {msg}" + ); + assert!( + msg.contains("belongs to namespace"), + "expected the deeper merge ownership check to fail instead; got: {msg}" + ); +} + +/// `merge` by short prefix within the records' own namespace still succeeds — +/// guards the already-working path. +#[tokio::test] +async fn merge_entities_by_prefix_same_namespace_succeeds() { + let f = pack(); + + let into_full = f + .dispatch( + "create", + json!({"kind": "concept", "name": "PrefixSameNsMergeInto"}), + ) + .await + .expect("create into-entity must succeed")["id"] + .as_str() + .unwrap() + .to_string(); + let from_full = f + .dispatch( + "create", + json!({"kind": "concept", "name": "PrefixSameNsMergeFrom"}), + ) + .await + .expect("create from-entity must succeed")["id"] + .as_str() + .unwrap() + .to_string(); + + let into_prefix = &into_full[..8]; + let from_prefix = &from_full[..8]; + + let result = f + .dispatch( + "merge", + json!({"into_id": into_prefix, "from_id": from_prefix}), + ) + .await; + assert!( + result.is_ok(), + "merge by prefix within the records' own namespace must succeed; got: {result:?}" + ); +} + +/// Negative: a hex prefix matching nothing resolves to `NotFound` even though +/// resolution is now unfiltered — unfiltered means "no namespace predicate", +/// not "match anything" (#391 §3). +#[tokio::test] +async fn get_by_prefix_with_no_match_returns_not_found() { + let f = pack(); + + let err = f + .dispatch("get", json!({"id": "deadbeef"})) + .await + .unwrap_err(); + assert!( + matches!(err, RuntimeError::NotFound(_)), + "unmatched prefix must return NotFound; got: {err:?}" + ); +} + /// Soft-delete an edge, then hard-delete it WITHOUT supplying explicit kind — must /// succeed and leave the row physically gone (ADR-002: public delete auto-detects). #[tokio::test] diff --git a/crates/khive-runtime/src/operations.rs b/crates/khive-runtime/src/operations.rs index 2f0b0e91..32086bb4 100644 --- a/crates/khive-runtime/src/operations.rs +++ b/crates/khive-runtime/src/operations.rs @@ -2747,15 +2747,17 @@ impl KhiveRuntime { /// Resolve a short UUID prefix (8+ hex chars) to a full UUID. /// /// Searches entities, notes, and edges tables for a UUID starting with the - /// given prefix, scoped to the caller's namespace. Returns `Ok(Some(uuid))` - /// if exactly one match is found, `Ok(None)` if no matches, or an error if - /// ambiguous (multiple matches). + /// given prefix, scoped to the caller's primary namespace only. Returns + /// `Ok(Some(uuid))` if exactly one match is found, `Ok(None)` if no + /// matches, or an error if ambiguous (multiple matches). pub async fn resolve_prefix( &self, token: &NamespaceToken, prefix: &str, ) -> RuntimeResult> { - self.resolve_prefix_inner(token, prefix, false).await + let namespaces = [token.namespace().as_str().to_owned()]; + self.resolve_prefix_inner(Some(&namespaces), prefix, false) + .await } pub async fn resolve_prefix_including_deleted( @@ -2763,18 +2765,76 @@ impl KhiveRuntime { token: &NamespaceToken, prefix: &str, ) -> RuntimeResult> { - self.resolve_prefix_inner(token, prefix, true).await + let namespaces = [token.namespace().as_str().to_owned()]; + self.resolve_prefix_inner(Some(&namespaces), prefix, true) + .await } - async fn resolve_prefix_inner( + /// Resolve a short UUID prefix (8+ hex chars) to a full UUID, scanning the + /// caller's full **visible** namespace set (`{local} ∪ {actor.id} ∪ + /// {actor.visible_namespaces}`, i.e. `token.visible_namespaces()`) rather + /// than the primary namespace alone. + /// + /// Feedback targets are recall outputs, and recall is visible-set-aware; + /// this keeps prefix resolution on the same visibility recall uses so + /// that any record recall can surface is also feedback-targetable + /// (#391). Only the non-deleted variant is provided — auto_feedback's + /// prefix path never needs to resolve soft-deleted rows. + pub async fn resolve_prefix_visible( &self, token: &NamespaceToken, prefix: &str, + ) -> RuntimeResult> { + let namespaces: Vec = token + .visible_namespaces() + .iter() + .map(|ns| ns.as_str().to_owned()) + .collect(); + self.resolve_prefix_inner(Some(&namespaces), prefix, false) + .await + } + + /// Resolve a short UUID prefix (8+ hex chars) to a full UUID with NO + /// namespace filter at all — mirrors `resolve_by_id`'s by-ID contract + /// (ADR-007 Rev 6: by-ID resolution is namespace-agnostic; the Gate, not + /// storage-layer filtering, is the authz seam). Used by the four by-ID + /// CRUD verbs (get/update/delete/merge) so their prefix path matches + /// their already-unfiltered full-UUID path (#391 §3). No token param: + /// unlike `resolve_prefix`/`resolve_prefix_visible`, there is no + /// namespace to derive from one. + pub async fn resolve_prefix_unfiltered(&self, prefix: &str) -> RuntimeResult> { + self.resolve_prefix_inner(None, prefix, false).await + } + + /// `resolve_prefix_unfiltered`, including soft-deleted rows — used by the + /// hard-delete by-ID path (#391 §3). + pub async fn resolve_prefix_unfiltered_including_deleted( + &self, + prefix: &str, + ) -> RuntimeResult> { + self.resolve_prefix_inner(None, prefix, true).await + } + + /// Shared prefix-scan implementation over an explicit namespace set. + /// + /// `namespaces` selects the scan scope: `Some(&[ns])` reproduces the + /// historical primary-only behaviour (`resolve_prefix` / + /// `resolve_prefix_including_deleted`); `Some(&visible_set)` scans the + /// caller's full visible set (`resolve_prefix_visible`); `None` applies + /// no namespace predicate at all (`resolve_prefix_unfiltered*`). + /// Ambiguity (a prefix matching more than one UUID, even across + /// different namespaces in the set, or across all namespaces when + /// unfiltered) is still an error: UUIDs are globally unique, so two + /// distinct rows sharing a prefix always requires caller disambiguation — + /// no cross-namespace dedup is needed or performed. + async fn resolve_prefix_inner( + &self, + namespaces: Option<&[String]>, + prefix: &str, include_deleted: bool, ) -> RuntimeResult> { use khive_storage::types::{SqlStatement, SqlValue}; - let ns = token.namespace().as_str().to_owned(); let pattern = format!("{}%", prefix); let tables = [ @@ -2784,6 +2844,11 @@ impl KhiveRuntime { ("graph_edges", false), ]; + let ns_clause = namespaces.map(|ns| { + let placeholders: Vec = (0..ns.len()).map(|i| format!("?{}", i + 2)).collect(); + format!(" AND namespace IN ({})", placeholders.join(", ")) + }); + let mut matches: Vec = Vec::new(); let mut reader = self.sql().reader().await.map_err(RuntimeError::Storage)?; @@ -2793,14 +2858,16 @@ impl KhiveRuntime { } else { "" }; + let mut params = vec![SqlValue::Text(pattern.clone())]; + if let Some(ns) = namespaces { + params.extend(ns.iter().map(|n| SqlValue::Text(n.clone()))); + } let sql = SqlStatement { sql: format!( - "SELECT id FROM {table} WHERE id LIKE ?1 AND namespace = ?2{deleted_filter} LIMIT 2" + "SELECT id FROM {table} WHERE id LIKE ?1{ns_clause}{deleted_filter} LIMIT 2", + ns_clause = ns_clause.as_deref().unwrap_or("") ), - params: vec![ - SqlValue::Text(pattern.clone()), - SqlValue::Text(ns.clone()), - ], + params, label: Some("resolve_prefix".into()), }; match reader.query_all(sql).await { diff --git a/tests/khive-contract/tests/test_namespace_isolation.py b/tests/khive-contract/tests/test_namespace_isolation.py index 05c9abbe..7b6919ff 100644 --- a/tests/khive-contract/tests/test_namespace_isolation.py +++ b/tests/khive-contract/tests/test_namespace_isolation.py @@ -32,8 +32,10 @@ def test_read_isolation_between_namespaces( Contract verified here: - get(id, namespace=beta) where entity lives in alpha: SUCCEEDS — full-UUID by-ID is namespace-agnostic (ADR-007 Rev 6 Rule 2, SHIPPED PR-A1 commit 2607e263). - - 8-char prefix get from beta: FAILS with not-found — prefix expansion is a namespace-scoped - lookup, distinct from the full-UUID by-ID path; ADR-007 Rule 2 covers "by UUID" only. + - 8-char prefix get from beta: SUCCEEDS — prefix expansion for by-ID ops is unfiltered + (issue #391 fix): the prefix path now matches the full-UUID by-ID contract instead of + silently narrowing it to the caller namespace. Ambiguity across namespaces errors; + a unique prefix resolves regardless of namespace. - list(namespace=beta): alpha entity ABSENT — multi-record namespace scoping survives. - search(namespace=beta): alpha entity ABSENT — multi-record namespace scoping survives. - get(id, namespace=alpha): SUCCEEDS — control path confirming same-namespace access. @@ -102,22 +104,25 @@ def test_read_isolation_between_namespaces( f"Entity name mismatch: {fetched}" ) - # 8-char prefix get from beta: prefix expansion is namespace-scoped (distinct from full-UUID - # by-ID path). ADR-007 Rule 2 covers "by UUID" only; prefix resolution expands via a - # namespace-scoped query and correctly returns not-found for a cross-namespace prefix. + # 8-char prefix get from beta: prefix expansion for by-ID CRUD is unfiltered (issue #391 + # fix). The prior contract asserted not-found here, but that namespace-scoped prefix filter + # silently narrowed by-ID lookups to a boundary the full-UUID path never had (ADR-007 Rev 6 + # Rule 2). Prefix resolution now matches the full-UUID by-ID contract: a unique prefix + # resolves regardless of namespace; an ambiguous prefix errors. prefix8 = full_id[:8] envelope_prefix = khive_session.request_batch([{ "tool": "get", "args": {"id": prefix8, "namespace": ns_beta}, }]) first_prefix = envelope_prefix["results"][0] - assert not first_prefix.get("ok", False), ( - "8-char prefix get from beta namespace must not resolve alpha entity: " - "prefix expansion is namespace-scoped (not the full-UUID by-ID path of ADR-007 Rule 2)" + assert first_prefix.get("ok", False), ( + "8-char prefix get from beta namespace must resolve the alpha entity: by-ID prefix " + "expansion is unfiltered, matching the full-UUID by-ID contract " + f"(ADR-007 Rev 6 Rule 2, issue #391), got: {first_prefix}" ) - err_prefix = first_prefix.get("error", "").lower() - assert "not found" in err_prefix or "no record" in err_prefix, ( - f"Expected not-found prefix error from beta, got: {first_prefix.get('error')!r}" + prefix_result = first_prefix.get("result", {}) + assert prefix_result.get("name") == "AlphaEntity", ( + f"prefix get from beta must return AlphaEntity, got: {first_prefix}" )