diff --git a/crates/tinymemory-conformance/src/lib.rs b/crates/tinymemory-conformance/src/lib.rs index 727fe4d..9a66e06 100644 --- a/crates/tinymemory-conformance/src/lib.rs +++ b/crates/tinymemory-conformance/src/lib.rs @@ -45,7 +45,8 @@ pub use reference::{InMemoryProvider, REFERENCE_DRIVER_ID}; pub use suite::{ assert_awkward_content_round_trips, assert_capability_audit, assert_export_cursor_terminates, assert_export_import_round_trip, assert_forget_is_idempotent, assert_kv_round_trip, - assert_list_filters_narrow, assert_namespaces_are_isolated, assert_provider, + assert_list_filters_narrow, assert_namespaces_are_isolated, + assert_namespaces_preserve_their_section, assert_provider, assert_recall_respects_limit_and_namespace, assert_store_get_round_trip, assert_taint_is_preserved, assert_upsert_replaces_rather_than_duplicates, }; diff --git a/crates/tinymemory-conformance/src/suite/mod.rs b/crates/tinymemory-conformance/src/suite/mod.rs index 7ea359b..5c8650b 100644 --- a/crates/tinymemory-conformance/src/suite/mod.rs +++ b/crates/tinymemory-conformance/src/suite/mod.rs @@ -25,6 +25,7 @@ use std::sync::Arc; use tinymemory_api::capabilities::Capability; use tinymemory_api::error::MemoryError; +use tinymemory_api::namespace::Namespace; use tinymemory_api::provider::{audit_provider, ExportRecord, MemoryProvider, SourceScope}; use tinymemory_api::recall::OwnedRecallOpts; use tinymemory_api::types::{MemoryCategory, MemoryTaint}; @@ -61,6 +62,7 @@ pub async fn assert_provider(provider: Arc) { assert_list_filters_narrow(p).await; assert_taint_is_preserved(p).await; assert_recall_respects_limit_and_namespace(p).await; + assert_namespaces_preserve_their_section(p).await; assert_recall_respects_source_scope(p).await; assert_export_import_round_trip(p).await; assert_awkward_content_round_trips(p).await; @@ -489,6 +491,70 @@ pub async fn assert_recall_respects_limit_and_namespace(provider: &dyn MemoryPro cleanup(provider, &theirs, &["other"]).await; } +/// `namespaces()` reports a sectioned namespace back under the same +/// [`tinymemory_api::namespace::MemorySection`] the caller wrote it in. +/// +/// This is the regression the unified SQLite store's own storage-address +/// sanitiser taught us to check for: a driver whose on-disk address collapses +/// `:` to `_` (a real filesystem constraint) must still report the *logical* +/// namespace back through `namespaces()`, or `conversation:thread-8f21` +/// silently re-addresses out of the `conversation` section and every caller +/// enumerating a section's scopes sees nothing, even though the write itself +/// succeeded. +/// +/// # Panics +/// +/// Panics when no reported namespace parses to the same section as the one +/// that was written. +pub async fn assert_namespaces_preserve_their_section(provider: &dyn MemoryProvider) { + let who = provider.driver_id(); + let namespace = format!("conversation:{}", ns(provider, "section-thread")); + let written = Namespace::parse(&namespace) + .unwrap_or_else(|e| panic!("{who}: test fixture `{namespace}` failed to parse: {e}")); + + provider + .store( + &namespace, + "k", + "content", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .unwrap_or_else(|e| panic!("{who}: store failed: {e}")); + + let summaries = provider + .namespaces() + .await + .unwrap_or_else(|e| panic!("{who}: namespaces() failed: {e}")); + + let matching_scope = summaries.iter().find_map(|summary| { + Namespace::parse(&summary.namespace) + .ok() + .filter(|parsed| parsed.scope() == written.scope()) + }); + + match matching_scope { + Some(parsed) => assert_eq!( + parsed.section(), + written.section(), + "{who}: wrote `{namespace}` under section {:?}, but namespaces() reported \ + its scope back under section {:?} instead — a driver must not silently \ + re-address a sectioned namespace out of its section", + written.section(), + parsed.section(), + ), + None => panic!( + "{who}: namespaces() did not report any namespace with scope `{}` after \ + storing `{namespace}`; got {summaries:?}", + written.scope() + ), + } + + cleanup(provider, &namespace, &["k"]).await; +} + /// A present, empty source scope fails closed. /// /// # Panics diff --git a/crates/tinymemory-core/src/store/memory_trait.rs b/crates/tinymemory-core/src/store/memory_trait.rs index b8099b3..98c1dcf 100644 --- a/crates/tinymemory-core/src/store/memory_trait.rs +++ b/crates/tinymemory-core/src/store/memory_trait.rs @@ -300,8 +300,18 @@ impl UnifiedMemory { // be `'static` and cannot borrow the store. /// One `memory_docs` row as `get` selects it: -/// `(document_id, key, content, updated_at, category, taint, session_id)`. -type MemoryDocRow = (String, String, String, f64, String, String, Option); +/// `(document_id, key, content, updated_at, category, taint, session_id, +/// logical_namespace)`. +type MemoryDocRow = ( + String, + String, + String, + f64, + String, + String, + Option, + Option, +); impl UnifiedMemory { fn get_blocking( @@ -315,9 +325,18 @@ impl UnifiedMemory { // readers disagree about one record. The contract's round-trip // assertion catches exactly that (`tinymemory_conformance`), and it was // invisible until #18 §A3 let this store be bound as a driver at all. + // + // `logical_namespace` is selected too so the returned `MemoryEntry` + // reports the row's own logical name rather than the physical address + // this method happens to have been called with — see `list_blocking`'s + // doc comment for why that distinction matters. This is purely a + // labelling improvement: the row is still addressed by the physical + // `namespace` column alone (`WHERE namespace = ?1`), so two logical + // namespaces that sanitize to the same physical address are still + // one namespace here, same as before `logical_namespace` existed. let row: Option = conn .query_row( - "SELECT document_id, key, content, updated_at, category, taint, session_id + "SELECT document_id, key, content, updated_at, category, taint, session_id, logical_namespace FROM memory_docs WHERE namespace = ?1 AND key = ?2 LIMIT 1", params![ns, key], |row| { @@ -329,25 +348,41 @@ impl UnifiedMemory { row.get(4)?, row.get(5)?, row.get(6)?, + row.get(7)?, )) }, ) .optional()?; Ok(row.map( - |(id, key, content, updated_at, category, taint_str, session_id)| MemoryEntry { - id, - key, - content, - namespace: Some(ns.to_string()), - category: memory_category_from_stored(&category), - timestamp: timestamp_to_rfc3339(updated_at), - session_id, - score: None, - taint: crate::MemoryTaint::from_db_str(&taint_str), + |(id, key, content, updated_at, category, taint_str, session_id, row_logical)| { + MemoryEntry { + id, + key, + content, + namespace: Some(row_logical.unwrap_or_else(|| ns.to_string())), + category: memory_category_from_stored(&category), + timestamp: timestamp_to_rfc3339(updated_at), + session_id, + score: None, + taint: crate::MemoryTaint::from_db_str(&taint_str), + } }, )) } + /// List every row addressed to one physical namespace. + /// + /// Addressed by the physical `namespace` column only (`WHERE namespace = + /// ?1`) — exactly as before `logical_namespace` existed. Two logical + /// namespaces that sanitize to the same physical address (`a:b_c` and + /// `a_b:c` both sanitize to `a_b_c`) are still one namespace for this + /// call, and `sanitize_namespace` has always collapsed them that way; this + /// is pre-existing behaviour, not something this column changes. What + /// `logical_namespace` adds is purely the label: each returned entry's + /// `namespace` is the row's *own* logical name (falling back to the + /// physical address for pre-migration NULL rows) instead of the raw + /// sanitized address, so a sectioned namespace still reports its `:` + /// spelling back to a caller enumerating it. fn list_blocking( conn: &Arc>, ns: &str, @@ -356,16 +391,17 @@ impl UnifiedMemory { ) -> anyhow::Result> { let conn = conn.lock(); let mut stmt = conn.prepare( - "SELECT document_id, key, content, category, session_id, updated_at, taint + "SELECT document_id, key, content, category, session_id, updated_at, taint, logical_namespace FROM memory_docs WHERE namespace = ?1 ORDER BY updated_at DESC", )?; let rows = stmt.query_map(params![ns], |row| { let stored_category: String = row.get(3)?; + let row_logical: Option = row.get(7)?; Ok(MemoryEntry { id: row.get(0)?, key: row.get(1)?, content: row.get(2)?, - namespace: Some(ns.to_string()), + namespace: Some(row_logical.unwrap_or_else(|| ns.to_string())), category: memory_category_from_stored(&stored_category), session_id: row.get(4)?, timestamp: timestamp_to_rfc3339(row.get(5)?), @@ -402,11 +438,35 @@ impl UnifiedMemory { conn: &Arc>, ) -> anyhow::Result> { let conn = conn.lock(); + // `COALESCE(logical_namespace, namespace)` is the entire backfill + // story, deliberately: rows written before the `logical_namespace` + // column existed have it NULL and fall back to exactly today's + // sanitized value. A sanitized `_` cannot be reconstructed into + // whatever delimiter it replaced (a scope may legitimately contain + // `_`), so guessing would silently mislabel unrelated namespaces — + // NULL rows simply keep reporting their sanitized address. + // + // `GROUP BY namespace` (the storage address), not the logical name: + // every OTHER operation on this store — `get`, `list`, `forget`, + // `recall`, `clear_namespace` — addresses a row by its physical + // `namespace` column alone, so two logical names that sanitize to the + // same address (`conversation:x` and `conversation_x` both sanitize + // to `conversation_x`) are already treated as one namespace + // everywhere else. Grouping summaries by the logical name instead + // would report two summaries with two partial counts for what every + // other call still treats, and returns, as a single merged + // namespace — `list` on either reported name would return BOTH + // aliases' rows, double the count either summary claims. Grouping by + // the address keeps one summary per physical namespace with an + // accurate count; `MIN(logical_namespace)` (aggregate `MIN` ignores + // `NULL`) just picks a single, deterministic logical representative + // to report it under, so a sectioned namespace still enumerates + // under its `:` spelling instead of the sanitized `_` form. let mut stmt = conn.prepare( - "SELECT namespace, COUNT(*) AS n, MAX(updated_at) AS last + "SELECT COALESCE(MIN(logical_namespace), namespace) AS ns, COUNT(*) AS n, MAX(updated_at) AS last FROM memory_docs GROUP BY namespace - ORDER BY namespace", + ORDER BY ns", )?; let rows = stmt.query_map([], |row| { let ns: String = row.get(0)?; diff --git a/crates/tinymemory-core/src/store/memory_trait_tests.rs b/crates/tinymemory-core/src/store/memory_trait_tests.rs index cfa9e7e..12fd955 100644 --- a/crates/tinymemory-core/src/store/memory_trait_tests.rs +++ b/crates/tinymemory-core/src/store/memory_trait_tests.rs @@ -106,6 +106,249 @@ async fn namespace_summaries_counts_per_namespace() { assert!(alpha.last_updated.is_some()); } +/// A `
:` namespace (`tinymemory_bus::namespace`'s +/// convention) must survive `namespace_summaries()` byte-for-byte, even +/// though the on-disk address stays sanitized. Before the +/// `logical_namespace` column, `sanitize_namespace` collapsed `:` to `_` +/// and `namespace_summaries` read that sanitized value straight back out, +/// so every sectioned namespace looked unsectioned to a caller enumerating +/// namespaces. +#[tokio::test] +async fn namespace_summaries_reports_sectioned_namespace_verbatim() { + let (_tmp, mem) = fresh_mem(); + let namespace = "conversation:thread-8f21"; + mem.store(namespace, "k1", "hello there", MemoryCategory::Core, None) + .await + .unwrap(); + + let summaries = mem.namespace_summaries().await.unwrap(); + let found = summaries + .iter() + .find(|s| s.namespace == namespace) + .unwrap_or_else(|| panic!("expected `{namespace}` in {summaries:?}")); + assert_eq!(found.count, 1); + + // The storage address stays sanitized: the sectioned `:` is not a + // valid filesystem character, so the column and the on-disk directory + // must both still use the collapsed form. + let sanitized: String = { + let conn = mem.conn.lock(); + conn.query_row( + "SELECT namespace FROM memory_docs WHERE key = 'k1'", + [], + |row| row.get(0), + ) + .unwrap() + }; + assert_eq!(sanitized, "conversation_thread-8f21"); + assert!( + !sanitized.contains(':'), + "the memory_docs.namespace column must stay path-safe, got {sanitized}" + ); + + let dir = mem.namespace_dir(namespace); + assert!( + !dir.to_string_lossy().contains(':'), + "namespace_dir must never contain ':', got {}", + dir.display() + ); +} + +/// `get`/`forget`/`list`/`recall` must still address a sectioned +/// namespace by its original, unsanitized string — the `logical_namespace` +/// column is purely additive and must not disturb the sanitized lookup +/// path those methods already use. +#[tokio::test] +async fn sectioned_namespace_stays_addressable_by_its_original_string() { + let (_tmp, mem) = fresh_mem(); + let namespace = "conversation:thread-8f21"; + mem.store( + namespace, + "k1", + "we should ship on friday", + MemoryCategory::Core, + None, + ) + .await + .unwrap(); + + let got = mem.get(namespace, "k1").await.unwrap().unwrap(); + assert_eq!(got.content, "we should ship on friday"); + // The returned entry must report the row's own sectioned (logical) name, + // not the sanitized physical address (`conversation_thread-8f21`) it is + // actually stored under — a caller that fed this back into `get`/`list` + // must land on the same row, not a different, unsectioned one. + assert_eq!(got.namespace.as_deref(), Some(namespace)); + + let listed = mem.list(Some(namespace), None, None).await.unwrap(); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].key, "k1"); + assert_eq!(listed[0].namespace.as_deref(), Some(namespace)); + + let recalled = mem + .recall( + "ship on friday", + 5, + RecallOpts { + namespace: Some(namespace), + min_score: Some(0.0), + ..Default::default() + }, + ) + .await + .unwrap(); + assert!( + recalled.iter().any(|e| e.key == "k1"), + "recall must still find the row via the original sectioned namespace, got {recalled:#?}" + ); + + assert!(mem.forget(namespace, "k1").await.unwrap()); + assert!(mem.get(namespace, "k1").await.unwrap().is_none()); +} + +/// A row written before this migration has `logical_namespace = NULL`. +/// `namespace_summaries` must fall back to the sanitized `namespace` +/// column for those rows rather than erroring or hiding them — the +/// `COALESCE` is the entire backfill story, deliberately, because a +/// sanitized `_` cannot be un-collapsed back into the original delimiter. +#[tokio::test] +async fn namespace_summaries_falls_back_to_sanitized_namespace_when_logical_is_null() { + let (_tmp, mem) = fresh_mem(); + { + let conn = mem.conn.lock(); + conn.execute( + "INSERT INTO memory_docs ( + document_id, namespace, key, title, content, source_type, + priority, tags_json, metadata_json, category, session_id, + created_at, updated_at, markdown_rel_path + ) VALUES (?1, ?2, ?3, ?4, ?5, 'chat', 'medium', '[]', '{}', 'core', NULL, 0.0, 0.0, '')", + rusqlite::params![ + "pre-migration-doc", + "premigration_ns", + "k1", + "title", + "content" + ], + ) + .unwrap(); + } + + let summaries = mem.namespace_summaries().await.unwrap(); + let found = summaries + .iter() + .find(|s| s.namespace == "premigration_ns") + .unwrap_or_else(|| panic!("expected `premigration_ns` in {summaries:?}")); + assert_eq!(found.count, 1); +} + +/// A blank/whitespace namespace sanitizes to `GLOBAL_NAMESPACE` on the +/// storage address (`sanitize_namespace`); the logical column must land on +/// the same fallback rather than an empty string, or `COALESCE(logical_namespace, +/// namespace)` would report an empty-string namespace instead of `global`. +#[tokio::test] +async fn namespace_summaries_normalizes_blank_namespace_to_global() { + let (_tmp, mem) = fresh_mem(); + mem.store(" ", "k1", "content", MemoryCategory::Core, None) + .await + .unwrap(); + + let summaries = mem.namespace_summaries().await.unwrap(); + assert!( + summaries.iter().all(|s| !s.namespace.is_empty()), + "no summary should report an empty namespace, got {summaries:?}" + ); + let found = summaries + .iter() + .find(|s| s.namespace == GLOBAL_NAMESPACE) + .unwrap_or_else(|| panic!("expected `{GLOBAL_NAMESPACE}` in {summaries:?}")); + assert_eq!(found.count, 1); +} + +/// Two logical names that sanitize to the same physical namespace +/// (`conversation:x` and `conversation_x` both collapse to +/// `conversation_x`) must not split into two summaries with two partial +/// counts: every addressed call (`list`, `export`, ...) already merges +/// their rows into one physical namespace, so `namespace_summaries` must +/// report exactly one entry with the true, combined count. +/// +/// `sanitize_namespace` has always collapsed these two names onto one +/// physical address, and every operation on this store has always treated +/// them as one namespace — that is pre-existing behaviour, not something +/// `logical_namespace` changes. What `logical_namespace` adds is purely a +/// more informative label on the merged summary (a sectioned spelling +/// instead of the sanitized one), not isolation between the two names. +#[tokio::test] +async fn namespace_summaries_deduplicates_when_two_logical_names_alias_one_address() { + let (_tmp, mem) = fresh_mem(); + mem.store("conversation:x", "k1", "a", MemoryCategory::Core, None) + .await + .unwrap(); + mem.store("conversation_x", "k2", "b", MemoryCategory::Core, None) + .await + .unwrap(); + + let summaries = mem.namespace_summaries().await.unwrap(); + let matching: Vec<_> = summaries + .iter() + .filter(|s| s.namespace == "conversation:x" || s.namespace == "conversation_x") + .collect(); + assert_eq!( + matching.len(), + 1, + "expected exactly one summary for the aliased address, got {summaries:?}" + ); + assert_eq!(matching[0].count, 2); + + // Both aliases still address the same merged physical namespace. + let listed = mem.list(Some("conversation:x"), None, None).await.unwrap(); + assert_eq!(listed.len(), 2); +} + +/// `canonical_identifier`'s `[REDACTED_PII_*]` placeholder is valid storage +/// content but not a valid `Namespace` scope (`[`/`]` are rejected). A +/// sectioned namespace whose scope trips the strict PII gate must still +/// come back `Namespace::parse`-able and under its original section, or the +/// exact enumeration bug this column exists to fix reappears for precisely +/// PII-shaped scopes. +#[tokio::test] +async fn namespace_summaries_substitutes_brackets_in_pii_redacted_sectioned_namespace() { + use tinymemory_api::namespace::{MemorySection, Namespace}; + + let (_tmp, mem) = fresh_mem(); + let namespace = "conversation:ssn-123-45-6789"; + mem.store(namespace, "k1", "content", MemoryCategory::Core, None) + .await + .unwrap(); + + let summaries = mem.namespace_summaries().await.unwrap(); + let found = summaries + .iter() + .find(|s| s.namespace.starts_with("conversation:")) + .unwrap_or_else(|| panic!("expected a `conversation:` namespace in {summaries:?}")); + assert!( + !found.namespace.contains('[') && !found.namespace.contains(']'), + "logical namespace must stay Namespace-valid (no brackets), got {}", + found.namespace + ); + let parsed = Namespace::parse(&found.namespace) + .unwrap_or_else(|e| panic!("reported namespace `{}` must parse: {e}", found.namespace)); + assert_eq!(parsed.section(), Some(&MemorySection::Conversation)); + + // Address-equivalence: feeding the reported logical name straight back + // into an addressed call must find the row it names. Stripping the + // brackets instead of substituting `_` for them (matching + // `sanitize_namespace`'s own character mapping) would re-sanitize this + // name to a *different* physical namespace than the one actually + // written, so this call would silently return nothing. + let listed = mem.list(Some(&found.namespace), None, None).await.unwrap(); + assert_eq!( + listed.len(), + 1, + "listing the reported namespace `{}` must find the row stored under `{namespace}`", + found.namespace + ); +} + #[tokio::test] async fn legacy_namespace_migration_splits_and_is_idempotent() { use rusqlite::params; diff --git a/crates/tinymemory-core/src/store/namespace_store/documents.rs b/crates/tinymemory-core/src/store/namespace_store/documents.rs index 1f2f582..426c7e8 100644 --- a/crates/tinymemory-core/src/store/namespace_store/documents.rs +++ b/crates/tinymemory-core/src/store/namespace_store/documents.rs @@ -10,7 +10,7 @@ use std::collections::BTreeSet; use uuid::Uuid; use crate::store::safety; -use crate::store::types::{NamespaceDocumentInput, StoredMemoryDocument}; +use crate::store::types::{NamespaceDocumentInput, StoredMemoryDocument, GLOBAL_NAMESPACE}; use super::UnifiedMemory; @@ -29,6 +29,16 @@ impl UnifiedMemory { input: NamespaceDocumentInput, ) -> Result { let namespace = Self::sanitize_namespace(&input.namespace); + // The logical (delimiter-preserving) namespace, PII-redacted the same + // way `sanitize_namespace` redacts the storage address, so + // `namespace_summaries` can report `conversation:thread-8f21` back + // verbatim instead of the path-safe `conversation_thread-8f21`. Uses + // the same blank-input fallback as `sanitize_namespace` and strips the + // redaction placeholder's brackets so a PII-bearing sectioned + // namespace stays `Namespace::parse`-able -- see + // `canonical_logical_namespace`'s doc comment for both. + let logical_namespace = + safety::canonical_logical_namespace(&input.namespace, GLOBAL_NAMESPACE); let key = input.key.trim().to_string(); if key.is_empty() { return Err("document key cannot be empty".to_string()); @@ -110,9 +120,9 @@ impl UnifiedMemory { .map_err(|e| format!("begin tx: {e}"))?; tx.execute( "INSERT INTO memory_docs - (document_id, namespace, key, title, content, source_type, priority, tags_json, metadata_json, category, session_id, created_at, updated_at, markdown_rel_path, taint) + (document_id, namespace, key, title, content, source_type, priority, tags_json, metadata_json, category, session_id, created_at, updated_at, markdown_rel_path, taint, logical_namespace) VALUES - (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15) + (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16) ON CONFLICT(namespace, key) DO UPDATE SET title = excluded.title, content = excluded.content, @@ -124,7 +134,8 @@ impl UnifiedMemory { session_id = excluded.session_id, updated_at = excluded.updated_at, markdown_rel_path = excluded.markdown_rel_path, - taint = excluded.taint", + taint = excluded.taint, + logical_namespace = excluded.logical_namespace", params![ document_id, namespace, @@ -140,7 +151,8 @@ impl UnifiedMemory { created_at, updated_at, markdown_rel, - input.taint.as_db_str() + input.taint.as_db_str(), + logical_namespace ], ) .map_err(|e| format!("upsert memory_docs: {e}"))?; @@ -238,6 +250,10 @@ impl UnifiedMemory { input: NamespaceDocumentInput, ) -> Result { let namespace = Self::sanitize_namespace(&input.namespace); + // See `upsert_document_presanitized` — same delimiter-preserving, + // PII-redacted logical namespace, same reason. + let logical_namespace = + safety::canonical_logical_namespace(&input.namespace, GLOBAL_NAMESPACE); let key = input.key.trim().to_string(); if key.is_empty() { return Err("document key cannot be empty".to_string()); @@ -308,9 +324,9 @@ impl UnifiedMemory { let conn = self.conn.lock(); conn.execute( "INSERT INTO memory_docs - (document_id, namespace, key, title, content, source_type, priority, tags_json, metadata_json, category, session_id, created_at, updated_at, markdown_rel_path, taint) + (document_id, namespace, key, title, content, source_type, priority, tags_json, metadata_json, category, session_id, created_at, updated_at, markdown_rel_path, taint, logical_namespace) VALUES - (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15) + (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16) ON CONFLICT(namespace, key) DO UPDATE SET title = excluded.title, content = excluded.content, @@ -322,7 +338,8 @@ impl UnifiedMemory { session_id = excluded.session_id, updated_at = excluded.updated_at, markdown_rel_path = excluded.markdown_rel_path, - taint = excluded.taint", + taint = excluded.taint, + logical_namespace = excluded.logical_namespace", params![ document_id, namespace, @@ -338,7 +355,8 @@ impl UnifiedMemory { created_at, updated_at, markdown_rel, - input.taint.as_db_str() + input.taint.as_db_str(), + logical_namespace ], ) .map_err(|e| format!("upsert memory_docs: {e}"))?; @@ -457,38 +475,45 @@ impl UnifiedMemory { .next() .map_err(|e| format!("row load_documents_for_scope: {e}"))? { - let tags_json: String = row.get(7).map_err(|e| e.to_string())?; - let metadata_json: String = row.get(8).map_err(|e| e.to_string())?; - // The `taint` column has a NOT NULL DEFAULT 'internal' clause - // from the migration, so legacy rows that pre-date the column - // surface as "internal" string and round-trip back to - // `MemoryTaint::Internal`. Unknown / corrupted values fail - // closed to `MemoryTaint::ExternalSync` inside `from_db_str`, - // so a forward-rolled schema variant or a bad UPDATE can't - // silently downgrade a row to user-authored content. - let taint_str: String = row.get(14).map_err(|e| e.to_string())?; - let taint = crate::MemoryTaint::from_db_str(&taint_str); - docs.push(StoredMemoryDocument { - document_id: row.get(0).map_err(|e| e.to_string())?, - namespace: row.get(1).map_err(|e| e.to_string())?, - key: row.get(2).map_err(|e| e.to_string())?, - title: row.get(3).map_err(|e| e.to_string())?, - content: row.get(4).map_err(|e| e.to_string())?, - source_type: row.get(5).map_err(|e| e.to_string())?, - priority: row.get(6).map_err(|e| e.to_string())?, - tags: serde_json::from_str(&tags_json).unwrap_or_default(), - metadata: serde_json::from_str(&metadata_json).unwrap_or_else(|_| json!({})), - category: row.get(9).map_err(|e| e.to_string())?, - session_id: row.get(10).map_err(|e| e.to_string())?, - created_at: row.get(11).map_err(|e| e.to_string())?, - updated_at: row.get(12).map_err(|e| e.to_string())?, - markdown_rel_path: row.get(13).map_err(|e| e.to_string())?, - taint, - }); + docs.push(Self::row_to_stored_document(row)?); } Ok(docs) } + /// Map one `memory_docs` row, in the column order + /// [`Self::load_documents_for_scope`] selects it in, into a + /// [`StoredMemoryDocument`]. + fn row_to_stored_document(row: &rusqlite::Row<'_>) -> Result { + let tags_json: String = row.get(7).map_err(|e| e.to_string())?; + let metadata_json: String = row.get(8).map_err(|e| e.to_string())?; + // The `taint` column has a NOT NULL DEFAULT 'internal' clause + // from the migration, so legacy rows that pre-date the column + // surface as "internal" string and round-trip back to + // `MemoryTaint::Internal`. Unknown / corrupted values fail + // closed to `MemoryTaint::ExternalSync` inside `from_db_str`, + // so a forward-rolled schema variant or a bad UPDATE can't + // silently downgrade a row to user-authored content. + let taint_str: String = row.get(14).map_err(|e| e.to_string())?; + let taint = crate::MemoryTaint::from_db_str(&taint_str); + Ok(StoredMemoryDocument { + document_id: row.get(0).map_err(|e| e.to_string())?, + namespace: row.get(1).map_err(|e| e.to_string())?, + key: row.get(2).map_err(|e| e.to_string())?, + title: row.get(3).map_err(|e| e.to_string())?, + content: row.get(4).map_err(|e| e.to_string())?, + source_type: row.get(5).map_err(|e| e.to_string())?, + priority: row.get(6).map_err(|e| e.to_string())?, + tags: serde_json::from_str(&tags_json).unwrap_or_default(), + metadata: serde_json::from_str(&metadata_json).unwrap_or_else(|_| json!({})), + category: row.get(9).map_err(|e| e.to_string())?, + session_id: row.get(10).map_err(|e| e.to_string())?, + created_at: row.get(11).map_err(|e| e.to_string())?, + updated_at: row.get(12).map_err(|e| e.to_string())?, + markdown_rel_path: row.get(13).map_err(|e| e.to_string())?, + taint, + }) + } + /// List documents in a namespace, or across all namespaces when `None`. /// Returns `{ "documents": [...], "count": N }` JSON. pub async fn list_documents(&self, namespace: Option<&str>) -> Result { @@ -575,6 +600,17 @@ impl UnifiedMemory { /// Delete all documents, vector chunks, KV entries, and graph relations /// for the given namespace in a single transaction. Also removes the /// on-disk markdown directory (`namespaces/{ns}/docs/`). + /// + /// Scoped by the physical `namespace` column only, exactly as before + /// `logical_namespace` existed: `sanitize_namespace` has always collapsed + /// two differently-delimited names onto one physical address (`a:b_c` and + /// `a_b:c` both sanitize to `a_b_c`), and every operation on this store — + /// reads, writes, and this clear — has always treated that as one + /// namespace. This call is no exception; isolating aliasing logical + /// namespaces from each other is out of scope here (it would need + /// `logical_namespace` columns, and matching write-path support, on + /// `vector_chunks`, `kv_namespace`, and `graph_namespace` too, not just + /// `memory_docs`). pub async fn clear_namespace(&self, namespace: &str) -> Result<(), String> { let ns = Self::sanitize_namespace(namespace); log::debug!("[memory] clear_namespace: starting for namespace={ns}"); diff --git a/crates/tinymemory-core/src/store/namespace_store/documents_tests.rs b/crates/tinymemory-core/src/store/namespace_store/documents_tests.rs index 7b2f7ea..7716c69 100644 --- a/crates/tinymemory-core/src/store/namespace_store/documents_tests.rs +++ b/crates/tinymemory-core/src/store/namespace_store/documents_tests.rs @@ -1257,6 +1257,97 @@ async fn upsert_document_auto_sanitizes_pii_like_namespace() { ); } +/// The `logical_namespace` column carries the delimiter-preserving, +/// PII-**redacted** namespace, not the caller's raw string: the #5164 +/// PII-redaction step must apply to this column exactly as it does to the +/// sanitized `namespace` column, so a national ID never becomes a stored +/// address just because it round-trips through `namespaces()`. +#[tokio::test] +async fn upsert_document_redacts_pii_in_logical_namespace() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let doc_id = memory + .upsert_document(NamespaceDocumentInput { + namespace: "cliente-RFC-VECJ880326XK4".to_string(), + key: "k1".to_string(), + title: "Title".to_string(), + content: "Body".to_string(), + source_type: "doc".to_string(), + priority: "medium".to_string(), + tags: vec![], + metadata: json!({}), + category: "core".to_string(), + session_id: None, + document_id: None, + taint: crate::MemoryTaint::Internal, + }) + .await + .expect("PII-like namespace should be auto-sanitized, not rejected"); + + let logical_namespace: Option = { + let conn = memory.conn.lock(); + conn.query_row( + "SELECT logical_namespace FROM memory_docs WHERE document_id = ?1", + rusqlite::params![doc_id], + |row| row.get(0), + ) + .unwrap() + }; + let logical_namespace = + logical_namespace.expect("logical_namespace must be populated on a fresh write"); + assert!( + !logical_namespace.contains("VECJ880326XK4"), + "the national ID must not become the stored logical namespace, got: {logical_namespace}" + ); + assert!( + logical_namespace.contains("REDACTED_PII"), + "expected a redaction placeholder, got: {logical_namespace}" + ); +} + +/// A sectioned namespace's `:` delimiter must survive into +/// `logical_namespace` untouched — only the filesystem-hostile character +/// scrub (the sanitized `namespace` column) collapses it. +#[tokio::test] +async fn upsert_document_preserves_colon_in_logical_namespace() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let doc_id = memory + .upsert_document(NamespaceDocumentInput { + namespace: "conversation:thread-8f21".to_string(), + key: "k1".to_string(), + title: "Title".to_string(), + content: "Body".to_string(), + source_type: "doc".to_string(), + priority: "medium".to_string(), + tags: vec![], + metadata: json!({}), + category: "core".to_string(), + session_id: None, + document_id: None, + taint: crate::MemoryTaint::Internal, + }) + .await + .unwrap(); + + let (namespace, logical_namespace): (String, Option) = { + let conn = memory.conn.lock(); + conn.query_row( + "SELECT namespace, logical_namespace FROM memory_docs WHERE document_id = ?1", + rusqlite::params![doc_id], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .unwrap() + }; + assert_eq!(namespace, "conversation_thread-8f21"); + assert_eq!( + logical_namespace.as_deref(), + Some("conversation:thread-8f21") + ); +} + #[tokio::test] async fn upsert_document_metadata_only_auto_sanitizes_pii_like_key() { let tmp = TempDir::new().unwrap(); diff --git a/crates/tinymemory-core/src/store/namespace_store/init.rs b/crates/tinymemory-core/src/store/namespace_store/init.rs index 9010e0f..8d9ae3c 100644 --- a/crates/tinymemory-core/src/store/namespace_store/init.rs +++ b/crates/tinymemory-core/src/store/namespace_store/init.rs @@ -171,6 +171,7 @@ impl UnifiedMemory { updated_at REAL NOT NULL, markdown_rel_path TEXT NOT NULL, taint TEXT NOT NULL DEFAULT 'internal', + logical_namespace TEXT, UNIQUE(namespace, key) ); CREATE INDEX IF NOT EXISTS idx_memory_docs_ns_updated ON memory_docs(namespace, updated_at DESC); @@ -251,6 +252,19 @@ impl UnifiedMemory { "memory_docs", )?; + // Backfill the `logical_namespace` column on existing `memory_docs` + // databases. Fresh installs get this via the CREATE TABLE above. + // Nullable, no DEFAULT: existing rows get NULL rather than a guessed + // value, because a sanitized `_` cannot be reliably un-collapsed back + // into whatever delimiter it replaced (`namespace_summaries_blocking` + // falls back to the sanitized `namespace` column for those rows via + // `COALESCE`). + apply_additive_migration( + &conn, + "ALTER TABLE memory_docs ADD COLUMN logical_namespace TEXT", + "memory_docs", + )?; + // Create FTS5 episodic tables (episodic_log, episodic_fts, and their // triggers) so the Archivist can call episodic_insert immediately after // the store is initialised. diff --git a/crates/tinymemory-core/src/store/namespace_store/init_tests.rs b/crates/tinymemory-core/src/store/namespace_store/init_tests.rs index 6310870..4557137 100644 --- a/crates/tinymemory-core/src/store/namespace_store/init_tests.rs +++ b/crates/tinymemory-core/src/store/namespace_store/init_tests.rs @@ -181,6 +181,35 @@ fn additive_migration_surfaces_a_readonly_database() { ); } +/// The `logical_namespace` additive migration must be safe to run on +/// every boot: a fresh install gets the column from `CREATE TABLE`, and +/// reopening the same store must not fail with "duplicate column name". +#[test] +fn logical_namespace_migration_is_idempotent_across_reopen() { + fn has_logical_namespace_column(conn: &Connection) -> bool { + let mut stmt = conn.prepare("PRAGMA table_info(memory_docs)").unwrap(); + let found = stmt + .query_map([], |row| row.get::<_, String>(1)) + .unwrap() + .filter_map(Result::ok) + .any(|name| name == "logical_namespace"); + found + } + + let tmp = TempDir::new().unwrap(); + { + let mem = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + assert!( + has_logical_namespace_column(&mem.conn.lock()), + "a fresh install must get logical_namespace from CREATE TABLE" + ); + } + + // Reopening must not fail even though the column already exists. + let mem = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + assert!(has_logical_namespace_column(&mem.conn.lock())); +} + #[test] fn connection_has_busy_timeout_set() { let tmp = TempDir::new().unwrap(); diff --git a/crates/tinymemory-core/src/store/namespace_store/query.rs b/crates/tinymemory-core/src/store/namespace_store/query.rs index 8dee248..3e8d3f7 100644 --- a/crates/tinymemory-core/src/store/namespace_store/query.rs +++ b/crates/tinymemory-core/src/store/namespace_store/query.rs @@ -325,7 +325,7 @@ impl UnifiedMemory { hits.push(NamespaceMemoryHit { id: format!("episodic:{}", entry.id.unwrap_or(0)), kind: MemoryItemKind::Episodic, - namespace: ns.clone(), + namespace: ns.to_string(), key: format!("{}:{}", entry.session_id, entry.role), title: entry.lesson.clone(), content, diff --git a/crates/tinymemory-core/src/store/namespace_store/query_tests.rs b/crates/tinymemory-core/src/store/namespace_store/query_tests.rs index 9dc78e7..f4a94a3 100644 --- a/crates/tinymemory-core/src/store/namespace_store/query_tests.rs +++ b/crates/tinymemory-core/src/store/namespace_store/query_tests.rs @@ -1390,3 +1390,67 @@ async fn no_session_context_leaves_results_unchanged() { "a blank exclude_session_id must not filter anything" ); } + +// ── Sectioned-namespace context query (double-sanitization regression) ────── + +/// `query_namespace_context_data` (and the public `query_namespace` / +/// `query_documents` context API built on it) must find a row stored under a +/// sectioned namespace like `conversation:thread-9f11`, not silently return +/// empty. +/// +/// Kept as a regression test for a double-sanitization bug this path is prone +/// to: a caller derives a value from `namespace`, then hands the *sanitized* +/// form to a callee that derives from it again. Canonicalizing an +/// already-sanitized string is a no-op — no `:` survives to preserve — so the +/// second derivation silently produces a name no row holds, since the write +/// path derives from the ORIGINAL namespace. The shape recurs whenever a +/// physical and a logical form of the same namespace both travel through this +/// call chain, so the assertion is worth keeping even though the read filter +/// that first exposed it is gone. +#[tokio::test] +async fn query_namespace_context_data_finds_rows_in_a_sectioned_namespace() { + let tmp = TempDir::new().unwrap(); + let memory = UnifiedMemory::new(tmp.path(), Arc::new(NoopEmbedding), None).unwrap(); + + let namespace = "conversation:thread-9f11"; + memory + .upsert_document(NamespaceDocumentInput { + namespace: namespace.to_string(), + key: "decision".to_string(), + title: "Decision".to_string(), + content: "We decided to ship the rocket launch on Friday.".to_string(), + source_type: "chat".to_string(), + priority: "medium".to_string(), + tags: Vec::new(), + metadata: json!({}), + category: "core".to_string(), + session_id: None, + document_id: None, + taint: crate::MemoryTaint::Internal, + }) + .await + .unwrap(); + + let context = memory + .query_namespace_context_data(namespace, "rocket launch", 5) + .await + .unwrap(); + assert!( + context.hits.iter().any(|hit| hit.key == "decision"), + "query_namespace_context_data must find rows stored in a sectioned \ + namespace, not just an unsectioned one, got {:#?}", + context.hits + ); + + // `query_namespace_context` is the string-only convenience wrapper the + // public `query_namespace` client API calls — it must surface the same + // content, not just the structured hit list. + let text = memory + .query_namespace_context(namespace, "rocket launch", 5) + .await + .unwrap(); + assert!( + text.contains("rocket launch"), + "context text must include the sectioned namespace's own content, got: {text}" + ); +} diff --git a/crates/tinymemory-core/src/store/safety/mod.rs b/crates/tinymemory-core/src/store/safety/mod.rs index a3c1225..940bd53 100644 --- a/crates/tinymemory-core/src/store/safety/mod.rs +++ b/crates/tinymemory-core/src/store/safety/mod.rs @@ -50,6 +50,47 @@ pub fn canonical_identifier(value: &str) -> String { pii::redact_pii(value).value } +/// Canonical form of the delimiter-preserving *logical* namespace that +/// `namespace_summaries` reports back to callers (`COALESCE(logical_namespace, +/// namespace)`). +/// +/// Built on [`canonical_identifier`] so a PII-bearing namespace is redacted +/// the same way the storage address is (#5164), with two corrections +/// `canonical_identifier` alone does not make: +/// +/// * **Bracket substitution, not stripping.** The `[REDACTED_PII_*]` +/// placeholder is valid storage-address content but not a valid `Namespace` +/// scope — `Namespace::parse` rejects `[` and `]` — so a PII-bearing +/// sectioned namespace would round-trip through redaction and then fail to +/// parse back into its own section, reintroducing the exact enumeration gap +/// this column exists to close. The brackets are mapped to `_`, the exact +/// substitution `UnifiedMemory::sanitize_namespace` already performs on +/// every character outside its path-safe allow-list. That match matters: +/// removing the brackets instead (rather than substituting) would make the +/// logical name parse but no longer *address-equivalent* — re-sanitizing it +/// would produce a different physical namespace than the one the row was +/// actually written under, so a caller that fed the reported name back into +/// `list`/`get` would find nothing. +/// * **Blank fallback.** `UnifiedMemory::sanitize_namespace` maps blank / +/// whitespace-only input to `fallback` (in practice `GLOBAL_NAMESPACE`) so +/// the storage address is never an empty string. `canonical_identifier` +/// alone does not: trimmed-empty input canonicalizes to `""`, and +/// `COALESCE` treats an empty string as present, so the logical column +/// would silently diverge from the storage address for exactly the inputs +/// that column exists to shadow. Applying the same fallback here keeps them +/// in sync. +pub fn canonical_logical_namespace(raw: &str, fallback: &str) -> String { + let canonical: String = canonical_identifier(raw.trim()) + .chars() + .map(|ch| if ch == '[' || ch == ']' { '_' } else { ch }) + .collect(); + if canonical.is_empty() { + fallback.to_string() + } else { + canonical + } +} + /// Canonical storage form of a document key: the exact transform /// `upsert_document` / `upsert_document_metadata_only` apply before writing the /// `memory_docs.key` column (trim, then [`canonical_identifier`]). diff --git a/crates/tinymemory-tinycortex/src/engine/mod.rs b/crates/tinymemory-tinycortex/src/engine/mod.rs index 072419b..88e2f9d 100644 --- a/crates/tinymemory-tinycortex/src/engine/mod.rs +++ b/crates/tinymemory-tinycortex/src/engine/mod.rs @@ -259,12 +259,18 @@ impl MemoryHostConfig for EngineRuntimeConfig { /// reads it; it gates the host's LLM triage of Composio *triggers*, a path /// that never enters this crate. Carrying a value nobody reads would /// invite a reader to believe it does something here. + /// - `gmail_sync_query` stays `None` for the same reason as the two above: + /// [`EngineRuntimeConfig`] carries no field for it and nothing plumbs one + /// in from `ModuleConfig`, so `None` (the whole-inbox default) is the + /// only honest answer here rather than inventing a value this type was + /// never told. fn composio(&self) -> ComposioMode { ComposioMode { mode: self.composio_mode.clone(), entity_id: self.composio_entity_id.clone(), api_key: None, triage_disabled: false, + gmail_sync_query: None, } } fn memory_sources_json(&self) -> anyhow::Result { diff --git a/crates/tinymemory/src/sections/test.rs b/crates/tinymemory/src/sections/test.rs index 0e52771..e7b627a 100644 --- a/crates/tinymemory/src/sections/test.rs +++ b/crates/tinymemory/src/sections/test.rs @@ -563,7 +563,7 @@ async fn in_scope_allows_cross_session_on_the_conversation_section() { } #[tokio::test] -async fn in_scope_rejects_a_custom_alias_of_the_conversation_section_with_cross_session() { +async fn in_scope_allows_a_custom_alias_of_the_conversation_section_with_cross_session() { // `Custom("conversation")` and `MemorySection::Conversation` are the same // view (see `a_custom_section_spelling_a_known_prefix_is_the_same_view`), // so the cross_session guard must normalise before checking — it must diff --git a/docs/specs/memory-section-api.md b/docs/specs/memory-section-api.md index d739d3d..ea8ea30 100644 --- a/docs/specs/memory-section-api.md +++ b/docs/specs/memory-section-api.md @@ -153,6 +153,87 @@ Scores come from separate calls to one driver with one query. They are comparabl in practice on every bundled driver; the contract does not guarantee it, and the documentation says so rather than pretending otherwise. +### 4. The storage address and the logical namespace + +`UnifiedMemory` cannot store a `:` in the value it uses as a namespace: that +string becomes a filesystem directory via `namespace_dir()`, and +`sanitize_namespace` maps every character outside `[A-Za-z0-9\-_/]` to `_` as a +path-traversal defence. So `conversation:thread-8f21` was stored — and +enumerated — as `conversation_thread-8f21`, which `Namespace::parse` reads as +*unsectioned*. Every enumerating call on this surface therefore returned empty +against the production store, after writes that had succeeded. + +Widening that allow-list is not the fix. It is what keeps the address path-safe, +`:` is illegal in a Windows filename and denotes an NTFS alternate data stream, +and the sanitiser also performs the PII redaction that keeps a national ID from +becoming a storage address. + +So the address and the name are now separate columns. `memory_docs.namespace` +keeps exactly the characters it has today and remains what addresses the row and +names the directory. A new nullable `memory_docs.logical_namespace` carries +`canonical_identifier(namespace)` — the delimiter-preserving form, still +PII-redacted. `namespace_summaries` reports `COALESCE(MIN(logical_namespace), +namespace)`, and `get`/`list` populate `MemoryEntry.namespace` from the row's +own `logical_namespace` where the row query already selects it, falling back +to the physical address for a pre-migration row that has none. This is purely +a **labelling** fix: a sectioned namespace enumerates and round-trips under +its `:` spelling again, closing the actual goal of this change. + +The `COALESCE` is the entire backfill, deliberately. A row written before the +migration has `NULL` and keeps exactly its previous behaviour; the upsert clause +sets the column, so such a row heals when it is next written. No migration tries +to turn an old `_` back into a `:` — that mapping is not invertible, because a +scope may legitimately contain `_`, and guessing would silently relabel +unrelated namespaces into a section they were never written to. + +**This column does not make the physical address injective, and no operation +here isolates two logical names that sanitize to the same address.** `a:b_c` +and `a_b:c` both sanitize to `a_b_c`; `sanitize_namespace` has always +collapsed them onto that one physical address, and every operation on this +store — `get`, `list`, `forget`, `recall`, `clear_namespace` — has always +treated that address as a single namespace, addressing rows and deleting data +by it alone. That is unchanged here and is **explicitly out of scope**: it is +pre-existing behaviour this change restores rather than a regression this +change introduces. Concretely: + +- `list("a:b_c", ...)` and `list("a_b:c", ...)` both return the union of + whatever was written under either spelling — the same physical namespace, + same as before `logical_namespace` existed. +- `namespace_summaries` reports **one** summary for the physical address + (`GROUP BY namespace`), under a single logical representative + (`MIN(logical_namespace)`, falling back to the address when every row + predates the column) — not one summary per logical name. Only one of the + two colliding names is ever reported by enumeration; the other still + addresses the same merged data, but does not appear as its own entry. +- `clear_namespace` deletes the entire physical namespace's rows across + `memory_docs`, `vector_chunks`, `kv_namespace`, and `graph_namespace`, and + removes the whole on-disk markdown directory — regardless of which + colliding logical name is named. It does not, and cannot with this schema, + delete only "half" of a physically-merged namespace. +- `recall` and the hybrid query path (`query_namespace_hits`) score every + document under the physical address, whichever logical name was used to + reach it. + +Isolating two aliasing logical namespaces from each other — so that `list`, +`get`, `forget`, `recall`, and `clear_namespace` each treat `a:b_c` and +`a_b:c` as genuinely separate namespaces — was explored in earlier revisions +of this change and reverted. It requires every access path on every table +(`memory_docs`, `vector_chunks`, `kv_namespace`, `graph_namespace`) to filter +on the logical name, `kv_namespace` and `graph_namespace` would need their own +`logical_namespace` columns and write-path support (they currently have +neither), and the `UNIQUE(namespace, key)` constraint would still let two +colliding logical namespaces silently contend for one key even with read-side +filtering. That is real, scoped work with its own migration story — a +separate change, not a half-measure folded into this one. + +`assert_namespaces_preserve_their_section` in the conformance suite holds +every *retaining* driver to this: a namespace written in a section must be +reported back in that section. It is skipped for a driver that retains nothing, +like the rest of the storage assertions, and it says nothing about a row written +before this change and never rewritten — see the invariant below for the exact +scope. It is the assertion whose absence let the two bundled drivers disagree +unnoticed. + ## Invariants and constraints - A `SectionView` never reads or writes a namespace outside its own section. @@ -163,6 +244,17 @@ documentation says so rather than pretending otherwise. - An unusable section is an error, never an empty one: if a section's prefix fails validation, the enumerating calls fail rather than reporting no scopes, so they agree with the addressed calls about the same section. +- On a retaining driver, a namespace written or rewritten after this change is + reported back in the section it was written in. A driver may re-address a + namespace to suit its store, but it may not change which section the name + belongs to; `assert_namespaces_preserve_their_section` enforces it for every + retaining driver (`assert_provider` skips it, like the rest of the storage + assertions, for a driver that accepts writes and discards them). A row + written before this change and never rewritten keeps enumerating under its + sanitised, unsectioned name — see "The storage address and the logical + namespace" above for why that backfill is deliberately a no-op. +- A namespace never reaches the filesystem with a character the path allow-list + excludes, and the PII redaction on the storage address is unchanged. - `put` then `get` on the same `(scope, key)` round-trips on any retaining driver. - Every call succeeds on a driver that retains nothing, returning empty rather than an error — the surface has no capability-absent path. @@ -184,6 +276,24 @@ documentation says so rather than pretending otherwise. descending, reports `namespaces_searched`, and sets `truncated` only when the namespace cap skipped one. - `across_section` with `opts.namespace: Some(_)` returns `MemoryError::Invalid`. +- A sectioned write to the production `UnifiedMemory` store is enumerable + afterwards: `scopes()` reports it, proven by the tinycortex full-provider + conformance test against a real on-disk workspace rather than an in-memory + double. +- The storage address still contains no character outside the path allow-list, + and a PII-bearing namespace is still redacted in both columns. +- The `logical_namespace` migration is idempotent, and a row predating it still + enumerates under its sanitised name. +- Two logical namespaces that sanitize to the same physical address remain one + namespace for every operation — reads, writes, recall, and clearing — + exactly as before this change: `list`/`get`/`forget`/`recall` on either + spelling return the merged physical namespace's rows, `namespace_summaries` + reports one summary for it, and `clear_namespace` deletes it as one unit. + Only one of the two colliding logical names is reported by enumeration. + This is pre-existing behaviour and explicitly out of scope here — see "The + storage address and the logical namespace" above. +- The public `query_namespace` / `query_documents` context API finds rows + stored under a sectioned namespace, not just an unsectioned one. - The four contract commands pass, and rustdoc builds with `-D warnings`. ## Open questions