From e0a8738980965411f514f4a62c09f941efdea90c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 11:43:55 +0300 Subject: [PATCH 01/19] fix(memory): guard chunk tag rewrites --- src/memory/store/content/tags.rs | 28 +++++++++++++++++++++++ src/memory/store/content/tags_tests.rs | 31 ++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/memory/store/content/tags.rs b/src/memory/store/content/tags.rs index 0a171dc..1f15ad9 100644 --- a/src/memory/store/content/tags.rs +++ b/src/memory/store/content/tags.rs @@ -35,6 +35,8 @@ pub fn update_chunk_tags(abs_path: &Path, tags: &[String]) -> anyhow::Result<()> let new_bytes = rewrite_tags(&old_bytes, &augmented) .map_err(|e| anyhow::anyhow!("rewrite_tags {:?}: {e}", abs_path))?; + ensure_tag_rewrite_preserves_body(&old_bytes, &new_bytes, abs_path)?; + let parent = abs_path.parent().unwrap_or_else(|| Path::new(".")); let tmp_name = format!(".tmp_tags_{}.md", crate_temp_id()); let tmp_path = parent.join(&tmp_name); @@ -57,6 +59,32 @@ pub fn update_chunk_tags(abs_path: &Path, tags: &[String]) -> anyhow::Result<()> Ok(()) } +/// Reject a front-matter rewrite unless both files parse and their bodies are +/// byte-identical. +/// +/// The body hash is persisted separately from the markdown file, so silently +/// accepting an invalid or changed body would corrupt later retrieval. +fn ensure_tag_rewrite_preserves_body( + old_bytes: &[u8], + new_bytes: &[u8], + abs_path: &Path, +) -> anyhow::Result<()> { + let body = |bytes: &[u8]| -> Option { + std::str::from_utf8(bytes) + .ok() + .and_then(split_front_matter) + .map(|(_, body)| body.to_owned()) + }; + + match (body(old_bytes), body(new_bytes)) { + (Some(old), Some(new)) if old == new => Ok(()), + _ => Err(anyhow::anyhow!( + "tag rewrite would mutate or invalidate the body for {:?}", + abs_path + )), + } +} + /// Slugify an entity kind string for an Obsidian hierarchical tag. /// /// Output: lowercase, non-alphanumeric → `-`, collapsed, trimmed. diff --git a/src/memory/store/content/tags_tests.rs b/src/memory/store/content/tags_tests.rs index cd8bec1..1cac1e5 100644 --- a/src/memory/store/content/tags_tests.rs +++ b/src/memory/store/content/tags_tests.rs @@ -88,6 +88,37 @@ fn update_chunk_tags_is_noop_for_missing_file() { assert!(update_chunk_tags(&path, &["p/X".into()]).is_ok()); } +#[test] +fn update_chunk_tags_rejects_unparseable_front_matter_without_overwriting() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("malformed.md"); + let original = b"not front matter\nBODY"; + std::fs::write(&path, original).unwrap(); + + let result = update_chunk_tags(&path, &["person/Alice".into()]); + + assert!(result.is_err()); + assert_eq!(std::fs::read(&path).unwrap(), original); +} + +#[test] +fn body_guard_accepts_front_matter_only_changes() { + let path = Path::new("chunk.md"); + let old = b"---\ntags: []\n---\nBODY"; + let new = b"---\ntags:\n - person/Alice\n---\nBODY"; + + assert!(ensure_tag_rewrite_preserves_body(old, new, path).is_ok()); +} + +#[test] +fn body_guard_rejects_body_drift() { + let path = Path::new("chunk.md"); + let old = b"---\ntags: []\n---\nBODY"; + let new = b"---\ntags:\n - person/Alice\n---\nDIFFERENT"; + + assert!(ensure_tag_rewrite_preserves_body(old, new, path).is_err()); +} + #[test] fn slugify_tag_kind_examples() { assert_eq!(slugify_tag_kind("Person"), "person"); From 300ef71c93b10d716d3822b594fa695501b28368 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 18:16:49 +0300 Subject: [PATCH 02/19] refactor(memory): relocate goals PII/secret guards out of types.rs --- src/memory/goals/store.rs | 22 ++++++++++++++++++++++ src/memory/goals/types.rs | 19 +++++++------------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/memory/goals/store.rs b/src/memory/goals/store.rs index 6ca436f..eecc6ca 100644 --- a/src/memory/goals/store.rs +++ b/src/memory/goals/store.rs @@ -30,9 +30,31 @@ use parking_lot::Mutex; use crate::memory::config::MemoryConfig; use crate::memory::error::{MemoryEngineResult, MemoryError}; +use crate::memory::store::safety::{ + has_likely_email, has_likely_pii, has_likely_secret, sanitize_text, +}; use super::types::GoalsDoc; +/// Detect likely PII in goal text (email addresses, generic PII patterns, or +/// content the sanitizer would otherwise redact). Backs +/// [`super::types::GoalsDoc`]'s text validation. +/// +/// Lives here rather than in `types.rs` so that the value types +/// (`GoalItem`/`GoalsDoc`) stay free of the `regex`-backed `safety` module — +/// a dependency-light module tree is a prerequisite for those types moving +/// into a future standalone API crate. +pub(super) fn has_goal_pii(text: &str) -> bool { + has_likely_email(text) || has_likely_pii(text) || sanitize_text(text).report.pii_redactions > 0 +} + +/// Detect likely secrets (API keys, tokens, …) in goal text. Backs +/// [`super::types::GoalsDoc`]'s text validation; see [`has_goal_pii`] for why +/// this lives here instead of `types.rs`. +pub(super) fn has_goal_secret(text: &str) -> bool { + has_likely_secret(text) +} + /// File name of the goals document inside the workspace. pub const GOALS_FILE: &str = "MEMORY_GOALS.md"; diff --git a/src/memory/goals/types.rs b/src/memory/goals/types.rs index 4a60a02..e200336 100644 --- a/src/memory/goals/types.rs +++ b/src/memory/goals/types.rs @@ -6,21 +6,16 @@ //! tools. Each item carries a stable short id so edit/delete operations can //! address a specific line without depending on ordering. //! -//! This module is pure: it owns parse/render and the in-memory mutation logic -//! (`add` / `edit` / `delete`) plus their validation rules. The cap-enforcing -//! persistence layer lives in [`super::store`]; the reflection apply/dedupe -//! logic lives in [`super::reflect()`]. +//! This module owns parse/render and the in-memory mutation logic +//! (`add` / `edit` / `delete`) plus their validation rules; the PII/secret +//! predicates those rules call out to live in [`super::store`] instead of +//! here so this module stays free of the `regex`-backed safety machinery. +//! The cap-enforcing persistence layer lives in [`super::store`]; the +//! reflection apply/dedupe logic lives in [`super::reflect()`]. use serde::{Deserialize, Serialize}; use crate::memory::error::MemoryError; -use crate::memory::store::safety::{ - has_likely_email, has_likely_pii, has_likely_secret, sanitize_text, -}; - -fn has_goal_pii(text: &str) -> bool { - has_likely_email(text) || has_likely_pii(text) || sanitize_text(text).report.pii_redactions > 0 -} /// Markdown header rendered at the top of `MEMORY_GOALS.md`. pub(crate) const HEADER: &str = "# Long-term Goals"; @@ -146,7 +141,7 @@ impl GoalsDoc { "goal text must be a single line".to_string(), )); } - if has_likely_secret(text) || has_goal_pii(text) { + if super::store::has_goal_secret(text) || super::store::has_goal_pii(text) { return Err(MemoryError::Invalid( "goal text must not contain secrets or PII".to_string(), )); From 3fec74c9f516536b709e95b419c38a3cbbff0253 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 19:54:38 +0300 Subject: [PATCH 03/19] refactor(memory): extract stable contracts into the tinycortex-api crate --- Cargo.lock | 12 ++++++++++++ Cargo.toml | 18 ++++++++++++++++++ api/Cargo.toml | 18 ++++++++++++++++++ {src/memory => api/src}/error.rs | 0 api/src/lib.rs | 18 ++++++++++++++++++ {src/memory => api/src}/traits.rs | 0 {src/memory => api/src}/types.rs | 4 ++-- {src/memory => api/src}/types_tests.rs | 0 src/memory/mod.rs | 9 ++++++--- 9 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 api/Cargo.toml rename {src/memory => api/src}/error.rs (100%) create mode 100644 api/src/lib.rs rename {src/memory => api/src}/traits.rs (100%) rename {src/memory => api/src}/types.rs (99%) rename {src/memory => api/src}/types_tests.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index cf96fd2..03a6d77 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1582,6 +1582,7 @@ dependencies = [ "tempfile", "thiserror", "tinyagents", + "tinycortex-api", "tokio", "toml", "tracing", @@ -1590,6 +1591,17 @@ dependencies = [ "wiremock", ] +[[package]] +name = "tinycortex-api" +version = "0.1.1" +dependencies = [ + "anyhow", + "async-trait", + "serde", + "serde_json", + "thiserror", +] + [[package]] name = "tinystr" version = "0.8.3" diff --git a/Cargo.toml b/Cargo.toml index af298a2..7e0d5b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,3 +1,17 @@ +# The engine crate is also the workspace root so `api/` is never an orphan +# package: `cargo fmt --all`, `cargo check`, and `cargo test` from this +# directory cover the stable-contract crate too. `vendor/` is excluded so the +# nested TinyAgents submodule (reached through the `[patch.crates-io]` path +# below) is not pulled in as an implicit member. +[workspace] +members = [".", "api"] +# `default-members` would otherwise fall back to the root package alone (this is +# a root-package workspace, not a virtual one), so a bare `cargo test` here +# would silently skip `api/`. Naming both members keeps the contract crate in +# every default build/test. +default-members = [".", "api"] +exclude = ["vendor"] + [package] name = "tinycortex" version = "0.1.1" @@ -69,6 +83,10 @@ thiserror = "2" # `.cargo/config.toml`; embedding hosts may supply their own crates.io patch so # the host and TinyCortex share one `EmbeddingModel` trait identity. tinyagents = "2.1" +# The stable contract crate (value types, error enum, `Memory` trait). Declared +# as a path dependency, not a registry requirement, so embedding hosts resolve +# it through this checkout without needing their own `[patch.crates-io]` entry. +tinycortex-api = { path = "api" } toml = "0.8" uuid = { version = "1", features = ["serde", "v4"] } walkdir = "2" diff --git a/api/Cargo.toml b/api/Cargo.toml new file mode 100644 index 0000000..e3adf08 --- /dev/null +++ b/api/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "tinycortex-api" +version = "0.1.1" +edition = "2021" +license = "MIT" +repository = "https://github.com/tinyhumansai/tinycortex" +description = "Stable public contracts for the TinyCortex memory system" + +# Deliberately dependency-light: this crate is the stable contract surface that +# hosts compile against, so it must stay free of native, async-runtime, and +# storage dependencies. Anything heavier belongs in the `tinycortex` engine +# crate, never here. +[dependencies] +anyhow = "1" +async-trait = "0.1" +serde = { version = "1", features = ["derive"] } +serde_json = "1" +thiserror = "2" diff --git a/src/memory/error.rs b/api/src/error.rs similarity index 100% rename from src/memory/error.rs rename to api/src/error.rs diff --git a/api/src/lib.rs b/api/src/lib.rs new file mode 100644 index 0000000..a0ed61e --- /dev/null +++ b/api/src/lib.rs @@ -0,0 +1,18 @@ +//! Stable public contracts for the TinyCortex memory system. +//! +//! This crate holds the value types, error enum, and storage trait that both +//! the `tinycortex` engine and its embedding hosts compile against. It is +//! deliberately dependency-light (serde / anyhow / thiserror / async-trait +//! only) so depending on the contract never drags in SQLite, git2, reqwest, or +//! an async runtime. +//! +//! The engine crate re-exports these modules as `tinycortex::memory::{types, +//! error, traits}`, so existing paths keep resolving unchanged. +//! +//! - [`types`]: pure data contracts (entries, hits, taint, namespaces). +//! - [`error`]: the typed [`error::MemoryError`] enum and its result alias. +//! - [`traits`]: the [`traits::Memory`] storage-backend trait. + +pub mod error; +pub mod traits; +pub mod types; diff --git a/src/memory/traits.rs b/api/src/traits.rs similarity index 100% rename from src/memory/traits.rs rename to api/src/traits.rs diff --git a/src/memory/types.rs b/api/src/types.rs similarity index 99% rename from src/memory/types.rs rename to api/src/types.rs index 5943cae..e5914c9 100644 --- a/src/memory/types.rs +++ b/api/src/types.rs @@ -73,7 +73,7 @@ impl MemoryTaint { /// # Examples /// /// ``` - /// use tinycortex::memory::types::MemoryTaint; + /// use tinycortex_api::types::MemoryTaint; /// /// assert_eq!(MemoryTaint::Internal.as_db_str(), "internal"); /// assert_eq!(MemoryTaint::ExternalSync.as_db_str(), "external_sync"); @@ -97,7 +97,7 @@ impl MemoryTaint { /// # Examples /// /// ``` - /// use tinycortex::memory::types::MemoryTaint; + /// use tinycortex_api::types::MemoryTaint; /// /// assert_eq!(MemoryTaint::from_db_str("internal"), MemoryTaint::Internal); /// assert_eq!(MemoryTaint::from_db_str("external_sync"), MemoryTaint::ExternalSync); diff --git a/src/memory/types_tests.rs b/api/src/types_tests.rs similarity index 100% rename from src/memory/types_tests.rs rename to api/src/types_tests.rs diff --git a/src/memory/mod.rs b/src/memory/mod.rs index 1bf6ef8..531c56e 100644 --- a/src/memory/mod.rs +++ b/src/memory/mod.rs @@ -48,9 +48,12 @@ // ── Shared contracts ──────────────────────────────────────────────────────── pub mod config; -pub mod error; -pub mod traits; -pub mod types; +/// The stable value types, error enum, and storage trait now live in the +/// dependency-light `tinycortex-api` crate. Re-exporting the modules (rather +/// than the individual items) keeps every `memory::types::…` / +/// `super::types::…` path inside this crate — and in embedding hosts — +/// resolving exactly as before. +pub use tinycortex_api::{error, traits, types}; // ── Storage primitives ────────────────────────────────────────────────────── pub mod store; From 289313a912f6356c7d316311ee0ba42ffddf17cd Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 20:16:12 +0300 Subject: [PATCH 04/19] refactor(memory): move the remaining inert DTOs into tinycortex-api --- Cargo.lock | 3 + api/Cargo.toml | 18 ++++ .../chunks/types.rs => api/src/chunks.rs | 2 +- .../types_tests.rs => api/src/chunks_tests.rs | 0 src/memory/goals/types.rs => api/src/goals.rs | 78 +++------------ api/src/goals_tests.rs | 22 +++++ api/src/lib.rs | 23 ++++- .../types.rs => api/src/tool_memory.rs | 9 +- .../src/tool_memory_tests.rs | 0 .../tree/runtime/types.rs => api/src/tree.rs | 2 +- .../types_tests.rs => api/src/tree_tests.rs | 0 gitbooks/goals-and-tool-memory.md | 12 ++- src/memory/chunks/mod.rs | 9 +- src/memory/goals/mod.rs | 12 ++- .../{types_tests.rs => mutations_tests.rs} | 19 +--- src/memory/goals/reflect.rs | 1 + src/memory/goals/store.rs | 97 +++++++++++++++++-- src/memory/score/signals/source_weight.rs | 22 ++++- src/memory/tool_memory/mod.rs | 7 +- src/memory/tree/runtime/mod.rs | 7 +- 20 files changed, 225 insertions(+), 118 deletions(-) rename src/memory/chunks/types.rs => api/src/chunks.rs (99%) rename src/memory/chunks/types_tests.rs => api/src/chunks_tests.rs (100%) rename src/memory/goals/types.rs => api/src/goals.rs (59%) create mode 100644 api/src/goals_tests.rs rename src/memory/tool_memory/types.rs => api/src/tool_memory.rs (95%) rename src/memory/tool_memory/types_tests.rs => api/src/tool_memory_tests.rs (100%) rename src/memory/tree/runtime/types.rs => api/src/tree.rs (99%) rename src/memory/tree/runtime/types_tests.rs => api/src/tree_tests.rs (100%) rename src/memory/goals/{types_tests.rs => mutations_tests.rs} (81%) diff --git a/Cargo.lock b/Cargo.lock index 03a6d77..ef1d040 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1597,9 +1597,12 @@ version = "0.1.1" dependencies = [ "anyhow", "async-trait", + "chrono", "serde", "serde_json", + "sha2 0.10.9", "thiserror", + "uuid", ] [[package]] diff --git a/api/Cargo.toml b/api/Cargo.toml index e3adf08..458bf7a 100644 --- a/api/Cargo.toml +++ b/api/Cargo.toml @@ -10,9 +10,27 @@ description = "Stable public contracts for the TinyCortex memory system" # hosts compile against, so it must stay free of native, async-runtime, and # storage dependencies. Anything heavier belongs in the `tinycortex` engine # crate, never here. +# +# The full set is intentionally small and pure-Rust. Beyond the +# serde/error/async-trait baseline it carries exactly three additions, each +# pulled in by a value type that has to keep behaving identically after the +# move out of the engine crate: +# +# - `chrono` — timestamps on chunk/tree nodes; the `serde` feature backs +# `chunks::Metadata`'s `chrono::serde::ts_milliseconds`. +# - `sha2` — the deterministic `chunks::chunk_id`. +# - `uuid` — `tool_memory::ToolMemoryRule::generate_id` (v4 bytes, nibble +# encoded). Only the `v4` feature is needed here; the engine +# crate additionally enables `serde`. +# +# Nothing here may pull in `rusqlite`, `git2`, `reqwest`, `regex`, or an async +# runtime — guard with `cargo tree -i -p tinycortex-api`. [dependencies] anyhow = "1" async-trait = "0.1" +chrono = { version = "0.4", features = ["serde"] } serde = { version = "1", features = ["derive"] } serde_json = "1" +sha2 = "0.10" thiserror = "2" +uuid = { version = "1", features = ["v4"] } diff --git a/src/memory/chunks/types.rs b/api/src/chunks.rs similarity index 99% rename from src/memory/chunks/types.rs rename to api/src/chunks.rs index f8bbe8f..7770adb 100644 --- a/src/memory/chunks/types.rs +++ b/api/src/chunks.rs @@ -412,5 +412,5 @@ mod time_range_serde { } #[cfg(test)] -#[path = "types_tests.rs"] +#[path = "chunks_tests.rs"] mod tests; diff --git a/src/memory/chunks/types_tests.rs b/api/src/chunks_tests.rs similarity index 100% rename from src/memory/chunks/types_tests.rs rename to api/src/chunks_tests.rs diff --git a/src/memory/goals/types.rs b/api/src/goals.rs similarity index 59% rename from src/memory/goals/types.rs rename to api/src/goals.rs index e200336..697a867 100644 --- a/src/memory/goals/types.rs +++ b/api/src/goals.rs @@ -2,21 +2,20 @@ //! //! Goals are a small, ordered list of durable objectives the agent holds when //! interacting with the user. They are persisted as a compact markdown document -//! (`MEMORY_GOALS.md`) — see [`super::store`] — and surfaced over RPC + agent -//! tools. Each item carries a stable short id so edit/delete operations can -//! address a specific line without depending on ordering. +//! (`MEMORY_GOALS.md`) by the engine crate's `memory::goals::store` and +//! surfaced over RPC + agent tools. Each item carries a stable short id so +//! edit/delete operations can address a specific line without depending on +//! ordering. //! -//! This module owns parse/render and the in-memory mutation logic -//! (`add` / `edit` / `delete`) plus their validation rules; the PII/secret -//! predicates those rules call out to live in [`super::store`] instead of -//! here so this module stays free of the `regex`-backed safety machinery. -//! The cap-enforcing persistence layer lives in [`super::store`]; the -//! reflection apply/dedupe logic lives in [`super::reflect()`]. +//! This module is **pure data**: it owns the shape, parse, and render only. +//! The validating mutation surface (`add` / `edit` / `delete`) lives next to +//! the `regex`-backed PII/secret predicates it calls, in the engine crate's +//! `memory::goals::store::GoalsDocMutations` trait, so the value types stay +//! free of the safety machinery and of `regex`. The cap-enforcing persistence +//! layer and the reflection apply/dedupe logic live in the engine crate too. use serde::{Deserialize, Serialize}; -use crate::memory::error::MemoryError; - /// Markdown header rendered at the top of `MEMORY_GOALS.md`. pub(crate) const HEADER: &str = "# Long-term Goals"; @@ -125,63 +124,8 @@ impl GoalsDoc { pub fn contains_id(&self, id: &str) -> bool { self.items.iter().any(|i| i.id == id) } - - /// Validate that `text` is a non-empty, single-line goal body. A - /// newline-bearing goal would inject extra `- [..]` list lines on reload, - /// corrupting the stored shape — so it is rejected outright. - fn validate_text(text: &str) -> Result<&str, MemoryError> { - let text = text.trim(); - if text.is_empty() { - return Err(MemoryError::Invalid( - "goal text must not be empty".to_string(), - )); - } - if text.contains('\n') || text.contains('\r') { - return Err(MemoryError::Invalid( - "goal text must be a single line".to_string(), - )); - } - if super::store::has_goal_secret(text) || super::store::has_goal_pii(text) { - return Err(MemoryError::Invalid( - "goal text must not contain secrets or PII".to_string(), - )); - } - Ok(text) - } - - /// Append a new goal, returning the assigned id. Text is trimmed; empty or - /// multi-line text is rejected. - pub fn add(&mut self, text: &str) -> Result { - let text = Self::validate_text(text)?; - let id = self.next_id(); - self.items.push(GoalItem::new(&id, text)); - Ok(id) - } - - /// Replace the text of the goal with `id`. Returns an error if the id is - /// unknown or the new text is empty/multi-line. - pub fn edit(&mut self, id: &str, text: &str) -> Result<(), MemoryError> { - let text = Self::validate_text(text)?; - let item = self - .items - .iter_mut() - .find(|i| i.id == id) - .ok_or_else(|| MemoryError::NotFound(format!("no goal with id '{id}'")))?; - item.text = text.to_string(); - Ok(()) - } - - /// Delete the goal with `id`. Returns an error if the id is unknown. - pub fn delete(&mut self, id: &str) -> Result<(), MemoryError> { - let before = self.items.len(); - self.items.retain(|i| i.id != id); - if self.items.len() == before { - return Err(MemoryError::NotFound(format!("no goal with id '{id}'"))); - } - Ok(()) - } } #[cfg(test)] -#[path = "types_tests.rs"] +#[path = "goals_tests.rs"] mod tests; diff --git a/api/src/goals_tests.rs b/api/src/goals_tests.rs new file mode 100644 index 0000000..e67410c --- /dev/null +++ b/api/src/goals_tests.rs @@ -0,0 +1,22 @@ +//! Unit tests for [`super::GoalsDoc`] parse/render — the pure-data half. +//! +//! The validating mutation tests (`add` / `edit` / `delete`, including the +//! secret/PII rejection cases) live in the engine crate next to the +//! `GoalsDocMutations` trait that owns them: `memory::goals::mutations_tests`. + +use super::*; + +#[test] +fn render_starts_with_header() { + let doc = GoalsDoc::default(); + assert!(doc.render().starts_with("# Long-term Goals")); +} + +#[test] +fn parse_ignores_non_item_lines() { + let body = "# Long-term Goals\n\nsome stray prose\n- [g1] real goal\n- malformed line\n"; + let doc = GoalsDoc::parse(body); + assert_eq!(doc.items.len(), 1); + assert_eq!(doc.items[0].id, "g1"); + assert_eq!(doc.items[0].text, "real goal"); +} diff --git a/api/src/lib.rs b/api/src/lib.rs index a0ed61e..89df466 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -2,17 +2,30 @@ //! //! This crate holds the value types, error enum, and storage trait that both //! the `tinycortex` engine and its embedding hosts compile against. It is -//! deliberately dependency-light (serde / anyhow / thiserror / async-trait -//! only) so depending on the contract never drags in SQLite, git2, reqwest, or -//! an async runtime. +//! deliberately dependency-light (serde / serde_json / chrono / sha2 / anyhow / +//! thiserror / async-trait only) so depending on the contract never drags in +//! SQLite, git2, reqwest, regex, or an async runtime. //! -//! The engine crate re-exports these modules as `tinycortex::memory::{types, -//! error, traits}`, so existing paths keep resolving unchanged. +//! The engine crate aliases these modules back into their historical paths +//! (`tinycortex::memory::{types, error, traits}`, +//! `tinycortex::memory::chunks::types`, `tinycortex::memory::tree::runtime::types`, +//! `tinycortex::memory::tool_memory::types`, `tinycortex::memory::goals::types`), +//! so every existing path keeps resolving unchanged. //! //! - [`types`]: pure data contracts (entries, hits, taint, namespaces). //! - [`error`]: the typed [`error::MemoryError`] enum and its result alias. //! - [`traits`]: the [`traits::Memory`] storage-backend trait. +//! - [`chunks`]: the persisted chunk model ([`chunks::Chunk`], [`chunks::Metadata`], +//! [`chunks::SourceRef`], …) and the deterministic [`chunks::chunk_id`]. +//! - [`tree`]: the markdown summary-tree node model ([`tree::TreeNode`], +//! [`tree::NodeLevel`], [`tree::TreeStatus`], …). +//! - [`tool_memory`]: tool-scoped rule contracts ([`tool_memory::ToolMemoryRule`], …). +//! - [`goals`]: the long-term goals document ([`goals::GoalsDoc`], [`goals::GoalItem`]). +pub mod chunks; pub mod error; +pub mod goals; +pub mod tool_memory; pub mod traits; +pub mod tree; pub mod types; diff --git a/src/memory/tool_memory/types.rs b/api/src/tool_memory.rs similarity index 95% rename from src/memory/tool_memory/types.rs rename to api/src/tool_memory.rs index 095d2e5..8699d6d 100644 --- a/src/memory/tool_memory/types.rs +++ b/api/src/tool_memory.rs @@ -150,14 +150,13 @@ impl ToolMemoryRule { /// reason about the namespace without ambiguity. Always build the /// namespace through this helper — never hard-code the `tool-` format. /// -/// [`ToolMemoryStore::put_rule`] applies the same normalization to the stored -/// rule so namespace and display/grouping identity cannot diverge. -/// -/// [`ToolMemoryStore::put_rule`]: super::store::ToolMemoryStore::put_rule +/// The engine crate's `ToolMemoryStore::put_rule` applies the same +/// normalization to the stored rule so namespace and display/grouping identity +/// cannot diverge. pub fn tool_memory_namespace(tool_name: &str) -> String { format!("tool-{}", tool_name.trim().to_lowercase()) } #[cfg(test)] -#[path = "types_tests.rs"] +#[path = "tool_memory_tests.rs"] mod tests; diff --git a/src/memory/tool_memory/types_tests.rs b/api/src/tool_memory_tests.rs similarity index 100% rename from src/memory/tool_memory/types_tests.rs rename to api/src/tool_memory_tests.rs diff --git a/src/memory/tree/runtime/types.rs b/api/src/tree.rs similarity index 99% rename from src/memory/tree/runtime/types.rs rename to api/src/tree.rs index 197c27b..068dc00 100644 --- a/src/memory/tree/runtime/types.rs +++ b/api/src/tree.rs @@ -201,5 +201,5 @@ pub fn node_id_to_path(node_id: &str) -> PathBuf { } #[cfg(test)] -#[path = "types_tests.rs"] +#[path = "tree_tests.rs"] mod tests; diff --git a/src/memory/tree/runtime/types_tests.rs b/api/src/tree_tests.rs similarity index 100% rename from src/memory/tree/runtime/types_tests.rs rename to api/src/tree_tests.rs diff --git a/gitbooks/goals-and-tool-memory.md b/gitbooks/goals-and-tool-memory.md index e4d966b..3353033 100644 --- a/gitbooks/goals-and-tool-memory.md +++ b/gitbooks/goals-and-tool-memory.md @@ -56,9 +56,13 @@ hand-edited file degrades gracefully instead of erroring. All writes go through `load → mutate → save` in `goals::store`, which re-enforces every invariant on each write: -- **Single-line text only.** `GoalsDoc::validate_text` trims the text and - rejects it if empty or if it contains `\n`/`\r`. A newline-bearing goal would - inject extra `- [..]` list lines on reload and corrupt the stored shape. +- **Single-line text only.** `goals::store::validate_goal_text` trims the text + and rejects it if empty or if it contains `\n`/`\r`. A newline-bearing goal + would inject extra `- [..]` list lines on reload and corrupt the stored shape. + It backs the `GoalsDocMutations` trait (`add` / `edit` / `delete`), which + lives in `goals::store` — beside the `regex`-backed guards it calls — rather + than on `GoalsDoc` itself, so the value types can live in the + dependency-light `tinycortex-api` contract crate. - **PII / secret rejection.** The same validator rejects text where `has_likely_secret` or `has_likely_pii` (from `memory::store::safety`) fires — goals must stay free of credentials and personal data. @@ -118,7 +122,7 @@ decides *how* it is applied. `reflect(config, context, generator)`: `first_run`). 4. Applies them via `apply_mutations`: additions are de-duplicated by **normalized** text (trim, lowercase, collapse internal whitespace); - edits/deletes pass through `GoalsDoc::edit`/`delete`. Every rejected mutation + edits/deletes pass through `GoalsDocMutations::edit`/`delete`. Every rejected mutation (duplicate add, unknown id, invalid text) is counted as `skipped`. 5. `save`s with cap enforcement only if anything was applied; otherwise re-loads on-disk truth to avoid a needless rewrite. diff --git a/src/memory/chunks/mod.rs b/src/memory/chunks/mod.rs index 167e4d9..8e56230 100644 --- a/src/memory/chunks/mod.rs +++ b/src/memory/chunks/mod.rs @@ -58,8 +58,13 @@ mod store_delete; mod store_list; #[path = "store_sources.rs"] mod store_sources; -#[path = "types.rs"] -mod types; + +/// The persisted chunk value types now live in the dependency-light +/// `tinycortex-api` crate. Aliasing the whole module (rather than importing +/// the individual items) keeps every `super::types::…` path inside this module +/// tree resolving exactly as before, at the same private visibility the local +/// `mod types;` had. +use tinycortex_api::chunks as types; #[cfg(test)] #[path = "store_conn_tests.rs"] diff --git a/src/memory/goals/mod.rs b/src/memory/goals/mod.rs index fed1869..8acb687 100644 --- a/src/memory/goals/mod.rs +++ b/src/memory/goals/mod.rs @@ -42,13 +42,19 @@ pub mod reflect; pub mod store; -pub mod types; +/// The goals value types now live in the dependency-light `tinycortex-api` +/// crate. Aliasing the whole module (rather than the individual items) keeps +/// every `super::types::…` / `crate::memory::goals::types::…` path — inside +/// this crate and in embedding hosts — resolving exactly as before. The +/// validating mutation surface stayed behind in [`store::GoalsDocMutations`], +/// next to the `regex`-backed PII/secret guards it calls. +pub use tinycortex_api::goals as types; pub use reflect::{ build_prompt, reflect, GoalMutation, GoalsGenerator, NoopGenerator, ReflectOutcome, }; pub use store::{ - add, add_for, delete, delete_for, edit, edit_for, goals_path, list_for, load, save, GOALS_FILE, - GOALS_FILE_MAX_CHARS, GOALS_MAX_ITEMS, + add, add_for, delete, delete_for, edit, edit_for, goals_path, list_for, load, save, + GoalsDocMutations, GOALS_FILE, GOALS_FILE_MAX_CHARS, GOALS_MAX_ITEMS, }; pub use types::{GoalItem, GoalsDoc}; diff --git a/src/memory/goals/types_tests.rs b/src/memory/goals/mutations_tests.rs similarity index 81% rename from src/memory/goals/types_tests.rs rename to src/memory/goals/mutations_tests.rs index 5871943..849ebdc 100644 --- a/src/memory/goals/types_tests.rs +++ b/src/memory/goals/mutations_tests.rs @@ -1,7 +1,9 @@ -//! Unit tests for [`super::GoalsDoc`] parse/render and in-memory mutation. +//! Unit tests for the validating [`super::GoalsDocMutations`] surface +//! (`add` / `edit` / `delete`) and its secret/PII guards. //! Ported from OpenHuman `memory_goals/types.rs`. use super::*; +use crate::memory::goals::types::GoalItem; #[test] fn parse_round_trips_render() { @@ -13,21 +15,6 @@ fn parse_round_trips_render() { assert_eq!(doc, reparsed); } -#[test] -fn render_starts_with_header() { - let doc = GoalsDoc::default(); - assert!(doc.render().starts_with("# Long-term Goals")); -} - -#[test] -fn parse_ignores_non_item_lines() { - let body = "# Long-term Goals\n\nsome stray prose\n- [g1] real goal\n- malformed line\n"; - let doc = GoalsDoc::parse(body); - assert_eq!(doc.items.len(), 1); - assert_eq!(doc.items[0].id, "g1"); - assert_eq!(doc.items[0].text, "real goal"); -} - #[test] fn add_assigns_unique_ids() { let mut doc = GoalsDoc::default(); diff --git a/src/memory/goals/reflect.rs b/src/memory/goals/reflect.rs index a50aa1a..4f66599 100644 --- a/src/memory/goals/reflect.rs +++ b/src/memory/goals/reflect.rs @@ -24,6 +24,7 @@ use crate::memory::config::MemoryConfig; use crate::memory::error::MemoryEngineResult; use super::store; +use super::store::GoalsDocMutations; use super::types::GoalsDoc; /// A single proposed change to the goals list, emitted by a [`GoalsGenerator`]. diff --git a/src/memory/goals/store.rs b/src/memory/goals/store.rs index eecc6ca..647b0dc 100644 --- a/src/memory/goals/store.rs +++ b/src/memory/goals/store.rs @@ -34,27 +34,104 @@ use crate::memory::store::safety::{ has_likely_email, has_likely_pii, has_likely_secret, sanitize_text, }; -use super::types::GoalsDoc; +use super::types::{GoalItem, GoalsDoc}; /// Detect likely PII in goal text (email addresses, generic PII patterns, or /// content the sanitizer would otherwise redact). Backs -/// [`super::types::GoalsDoc`]'s text validation. +/// [`GoalsDocMutations`]'s text validation. /// -/// Lives here rather than in `types.rs` so that the value types -/// (`GoalItem`/`GoalsDoc`) stay free of the `regex`-backed `safety` module — -/// a dependency-light module tree is a prerequisite for those types moving -/// into a future standalone API crate. +/// Lives here rather than beside the value types so that `GoalItem`/`GoalsDoc` +/// stay free of the `regex`-backed `safety` module — that dependency-light +/// module tree is what lets those types live in the standalone +/// `tinycortex-api` contract crate. pub(super) fn has_goal_pii(text: &str) -> bool { has_likely_email(text) || has_likely_pii(text) || sanitize_text(text).report.pii_redactions > 0 } /// Detect likely secrets (API keys, tokens, …) in goal text. Backs -/// [`super::types::GoalsDoc`]'s text validation; see [`has_goal_pii`] for why -/// this lives here instead of `types.rs`. +/// [`GoalsDocMutations`]'s text validation; see [`has_goal_pii`] for why this +/// lives here rather than beside the value types. pub(super) fn has_goal_secret(text: &str) -> bool { has_likely_secret(text) } +/// Validate that `text` is a non-empty, single-line goal body. A +/// newline-bearing goal would inject extra `- [..]` list lines on reload, +/// corrupting the stored shape — so it is rejected outright. +fn validate_goal_text(text: &str) -> Result<&str, MemoryError> { + let text = text.trim(); + if text.is_empty() { + return Err(MemoryError::Invalid( + "goal text must not be empty".to_string(), + )); + } + if text.contains('\n') || text.contains('\r') { + return Err(MemoryError::Invalid( + "goal text must be a single line".to_string(), + )); + } + if has_goal_secret(text) || has_goal_pii(text) { + return Err(MemoryError::Invalid( + "goal text must not contain secrets or PII".to_string(), + )); + } + Ok(text) +} + +/// In-memory, validating mutation surface for [`GoalsDoc`]. +/// +/// These were inherent methods on `GoalsDoc` until the value types moved into +/// the dependency-light `tinycortex-api` crate. They live here, next to the +/// `regex`-backed [`has_goal_secret`] / [`has_goal_pii`] guards they call, so +/// the contract crate stays free of that machinery. The trait is implemented +/// for `GoalsDoc` and re-exported from [`super`], so call sites keep using +/// method syntax (`doc.add(text)`) with identical semantics. +/// +/// None of these persist; pair them with [`save`] (or use the `add` / `edit` / +/// `delete` free functions in this module, which do both under the mutation +/// lock). +pub trait GoalsDocMutations { + /// Append a new goal, returning the assigned id. Text is trimmed; empty, + /// multi-line, or secret/PII-bearing text is rejected. + fn add(&mut self, text: &str) -> Result; + + /// Replace the text of the goal with `id`. Returns an error if the id is + /// unknown or the new text is empty/multi-line/secret-or-PII-bearing. + fn edit(&mut self, id: &str, text: &str) -> Result<(), MemoryError>; + + /// Delete the goal with `id`. Returns an error if the id is unknown. + fn delete(&mut self, id: &str) -> Result<(), MemoryError>; +} + +impl GoalsDocMutations for GoalsDoc { + fn add(&mut self, text: &str) -> Result { + let text = validate_goal_text(text)?; + let id = self.next_id(); + self.items.push(GoalItem::new(&id, text)); + Ok(id) + } + + fn edit(&mut self, id: &str, text: &str) -> Result<(), MemoryError> { + let text = validate_goal_text(text)?; + let item = self + .items + .iter_mut() + .find(|i| i.id == id) + .ok_or_else(|| MemoryError::NotFound(format!("no goal with id '{id}'")))?; + item.text = text.to_string(); + Ok(()) + } + + fn delete(&mut self, id: &str) -> Result<(), MemoryError> { + let before = self.items.len(); + self.items.retain(|i| i.id != id); + if self.items.len() == before { + return Err(MemoryError::NotFound(format!("no goal with id '{id}'"))); + } + Ok(()) + } +} + /// File name of the goals document inside the workspace. pub const GOALS_FILE: &str = "MEMORY_GOALS.md"; @@ -221,3 +298,7 @@ pub fn delete_for(config: &MemoryConfig, id: &str) -> MemoryEngineResult f32 { if let Some(ds) = infer_data_source(meta) { return weight_for(ds); } - // Fallback: kind-level defaults consistent with per-provider averages. - match meta.source_kind { + kind_default(meta.source_kind) +} + +/// Kind-level default weight, consistent with the per-provider averages +/// below. Used when no [`DataSource`] can be inferred from the chunk tags. +fn kind_default(kind: SourceKind) -> f32 { + match kind { SourceKind::Email => 0.75, SourceKind::Document => 0.7, SourceKind::Chat => 0.5, @@ -48,8 +53,15 @@ pub fn score(meta: &Metadata) -> f32 { /// Per-[`DataSource`] base weight in `[0.0, 1.0]`, hand-tuned per the /// rationale in the module docs (scoped/directed communication scores -/// higher than broadcast-style channels). Exhaustive match — adding a new -/// `DataSource` variant is a compile error here until a weight is assigned. +/// higher than broadcast-style channels). +/// +/// Every variant that exists today is listed explicitly. `DataSource` is +/// `#[non_exhaustive]` and now lives in the separate `tinycortex-api` contract +/// crate, so the compiler can no longer prove this match exhaustive and a +/// wildcard is required; the wildcard is unreachable today and only fires if a +/// provider is added to the contract crate without a weight here. It degrades +/// to the same [`kind_default`] the no-provider path uses rather than +/// panicking. fn weight_for(ds: DataSource) -> f32 { match ds { // Personal email providers score high — typically small, directed audiences @@ -66,6 +78,8 @@ fn weight_for(ds: DataSource) -> f32 { DataSource::Notion => 0.75, DataSource::DriveDocs => 0.6, DataSource::MeetingNotes => 0.85, + // Unreachable today — see the doc comment above. + _ => kind_default(ds.kind()), } } diff --git a/src/memory/tool_memory/mod.rs b/src/memory/tool_memory/mod.rs index 58dd149..2f2ef2d 100644 --- a/src/memory/tool_memory/mod.rs +++ b/src/memory/tool_memory/mod.rs @@ -41,7 +41,12 @@ pub mod render; pub mod store; -pub mod types; +/// The tool-memory rule value types now live in the dependency-light +/// `tinycortex-api` crate. Aliasing the whole module (rather than the +/// individual items) keeps every `super::types::…` / +/// `crate::memory::tool_memory::types::…` path — inside this crate and in +/// embedding hosts — resolving exactly as before. +pub use tinycortex_api::tool_memory as types; #[cfg(test)] pub mod test_helpers; diff --git a/src/memory/tree/runtime/mod.rs b/src/memory/tree/runtime/mod.rs index bdd87d6..e8f2068 100644 --- a/src/memory/tree/runtime/mod.rs +++ b/src/memory/tree/runtime/mod.rs @@ -12,7 +12,12 @@ pub mod engine; mod fold; mod rebuild_fs; pub mod store; -pub mod types; +/// The summary-tree node value types now live in the dependency-light +/// `tinycortex-api` crate. Aliasing the whole module (rather than the +/// individual items) keeps every `super::types::…` / +/// `crate::memory::tree::runtime::types::…` path — inside this crate and in +/// embedding hosts — resolving exactly as before. +pub use tinycortex_api::tree as types; pub use engine::{ discover_active_namespaces, rebuild_tree, rebuild_tree_observed, run_summarization, From e0e848ea622aab46f70eb3846cc1b1cc85200f41 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 20:57:28 +0300 Subject: [PATCH 05/19] test(memory): guard every DataSource against a missing source weight --- api/Cargo.toml | 9 +++++++- src/memory/score/signals/source_weight.rs | 22 +++++++++++++++---- .../score/signals/source_weight_tests.rs | 22 +++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/api/Cargo.toml b/api/Cargo.toml index 458bf7a..cf4a1e8 100644 --- a/api/Cargo.toml +++ b/api/Cargo.toml @@ -24,7 +24,14 @@ description = "Stable public contracts for the TinyCortex memory system" # crate additionally enables `serde`. # # Nothing here may pull in `rusqlite`, `git2`, `reqwest`, `regex`, or an async -# runtime — guard with `cargo tree -i -p tinycortex-api`. +# runtime. Guard with the FORWARD form, which is scoped to this package: +# +# cargo tree -p tinycortex-api -e normal,build --prefix none \ +# | grep -Ei 'rusqlite|libsqlite|git2|reqwest|regex|tokio' # expect no match +# +# Do NOT use `cargo tree -i -p tinycortex-api`: `-i` discards the `-p` +# scope and prints the whole-workspace inverse tree, so it exits 0 and looks +# clean even when this crate is the one pulling the dependency in. [dependencies] anyhow = "1" async-trait = "0.1" diff --git a/src/memory/score/signals/source_weight.rs b/src/memory/score/signals/source_weight.rs index 86c7695..770017c 100644 --- a/src/memory/score/signals/source_weight.rs +++ b/src/memory/score/signals/source_weight.rs @@ -62,8 +62,20 @@ fn kind_default(kind: SourceKind) -> f32 { /// provider is added to the contract crate without a weight here. It degrades /// to the same [`kind_default`] the no-provider path uses rather than /// panicking. +/// +/// The lost compile-time exhaustiveness is recovered as a test: see +/// `every_data_source_has_an_explicit_weight` in `source_weight_tests.rs`, +/// which walks [`DataSource::all`] and asserts [`weight_for_explicit`] answers +/// for every variant. Adding a provider to the contract crate without a weight +/// here therefore fails the suite instead of silently degrading. fn weight_for(ds: DataSource) -> f32 { - match ds { + weight_for_explicit(ds).unwrap_or_else(|| kind_default(ds.kind())) +} + +/// The hand-tuned weight table proper. Returns `None` for a provider that has +/// no explicit weight assigned — which is what the guard test detects. +fn weight_for_explicit(ds: DataSource) -> Option { + let weight = match ds { // Personal email providers score high — typically small, directed audiences DataSource::Gmail => 0.8, DataSource::OtherEmail => 0.7, @@ -78,9 +90,11 @@ fn weight_for(ds: DataSource) -> f32 { DataSource::Notion => 0.75, DataSource::DriveDocs => 0.6, DataSource::MeetingNotes => 0.85, - // Unreachable today — see the doc comment above. - _ => kind_default(ds.kind()), - } + // Unreachable today — see the doc comment above. `None` routes the + // caller to `kind_default` and trips the guard test. + _ => return None, + }; + Some(weight) } #[cfg(test)] diff --git a/src/memory/score/signals/source_weight_tests.rs b/src/memory/score/signals/source_weight_tests.rs index 427720e..f1f70a9 100644 --- a/src/memory/score/signals/source_weight_tests.rs +++ b/src/memory/score/signals/source_weight_tests.rs @@ -40,3 +40,25 @@ fn all_data_sources_bounded() { assert!((0.0..=1.0).contains(&w)); } } + +/// Recovers the compile-time exhaustiveness guarantee that `DataSource` lost +/// when it moved into the `tinycortex-api` contract crate. +/// +/// `#[non_exhaustive]` is inert inside its defining crate but binding across a +/// crate boundary, so `weight_for_explicit`'s match now needs a wildcard and +/// the compiler can no longer prove every provider has a hand-tuned weight. +/// Without this test, adding a provider to the contract crate would silently +/// degrade it to the coarse `kind_default` instead of failing the build. +#[test] +fn every_data_source_has_an_explicit_weight() { + let missing: Vec<&'static str> = DataSource::all() + .iter() + .filter(|ds| weight_for_explicit(**ds).is_none()) + .map(|ds| ds.as_str()) + .collect(); + assert!( + missing.is_empty(), + "these DataSource variants fall through to kind_default instead of a \ + hand-tuned weight; add them to weight_for_explicit: {missing:?}" + ); +} From 1d92b1e49021916b38a187ccac85ee82246d8fbd Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 21:48:58 +0300 Subject: [PATCH 06/19] feat(api): add OwnedRecallOpts, the serde-derived recall contract form RecallOpts<'a> is named both in the mandatory MemoryRecall::recall signature and as a JSON body field on POST /v1/memory/recall. It can be neither: it derives no serde impls and its borrow lifetime cannot travel through an object-safe #[async_trait] method. Add OwnedRecallOpts with the same five fields owned and serde-derived, plus From impls both ways. Both impls destructure exhaustively, so a field added to one form and not the other is a compile error; a runtime round-trip test catches a field that is merely dropped in conversion. Both forms move into a new recall module so the pair is edited together, and are re-exported from types so every historical types::RecallOpts path (including the engine's tinycortex::memory::types:: alias) keeps resolving. --- api/src/lib.rs | 26 +++++-- api/src/recall.rs | 157 ++++++++++++++++++++++++++++++++++++++++ api/src/recall_tests.rs | 118 ++++++++++++++++++++++++++++++ api/src/types.rs | 22 ++---- 4 files changed, 302 insertions(+), 21 deletions(-) create mode 100644 api/src/recall.rs create mode 100644 api/src/recall_tests.rs diff --git a/api/src/lib.rs b/api/src/lib.rs index 89df466..89cc6b7 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -1,10 +1,20 @@ //! Stable public contracts for the TinyCortex memory system. //! -//! This crate holds the value types, error enum, and storage trait that both -//! the `tinycortex` engine and its embedding hosts compile against. It is -//! deliberately dependency-light (serde / serde_json / chrono / sha2 / anyhow / -//! thiserror / async-trait only) so depending on the contract never drags in -//! SQLite, git2, reqwest, regex, or an async runtime. +//! This crate holds the value types, error enum, and storage trait that both the `tinycortex` engine and its embedding hosts +//! compile against. It is deliberately dependency-light (serde / serde_json / +//! chrono / sha2 / anyhow / thiserror / async-trait / uuid only) so depending on +//! the contract never drags in SQLite, git2, reqwest, regex, or an async +//! runtime. +//! +//! ## Self-contained by design +//! +//! Nothing here names a host type. A third-party memory driver must be able to +//! depend on this crate alone, and the *generic* subsystem/driver vocabulary of +//! the OpenHuman kernel (`Driver`, `DriverClass`, `SubsystemRegistry`, the +//! policy `Guard`) must not be inherited from a *memory* crate by whichever +//! subsystem is cut over next. +//! +//! ## The engine's historical paths still resolve //! //! The engine crate aliases these modules back into their historical paths //! (`tinycortex::memory::{types, error, traits}`, @@ -12,7 +22,12 @@ //! `tinycortex::memory::tool_memory::types`, `tinycortex::memory::goals::types`), //! so every existing path keeps resolving unchanged. //! +//! ## Module map +//! //! - [`types`]: pure data contracts (entries, hits, taint, namespaces). +//! - [`recall`]: the borrowed [`recall::RecallOpts`] and owned, serde-derived +//! [`recall::OwnedRecallOpts`] recall filters (both re-exported from +//! [`types`]). //! - [`error`]: the typed [`error::MemoryError`] enum and its result alias. //! - [`traits`]: the [`traits::Memory`] storage-backend trait. //! - [`chunks`]: the persisted chunk model ([`chunks::Chunk`], [`chunks::Metadata`], @@ -25,6 +40,7 @@ pub mod chunks; pub mod error; pub mod goals; +pub mod recall; pub mod tool_memory; pub mod traits; pub mod tree; diff --git a/api/src/recall.rs b/api/src/recall.rs new file mode 100644 index 0000000..593194a --- /dev/null +++ b/api/src/recall.rs @@ -0,0 +1,157 @@ +//! Recall filter contracts — the borrowed engine form and the owned +//! contract/wire form, kept side by side so they cannot drift. +//! +//! ## Why there are two +//! +//! [`RecallOpts`] is the historical, engine-facing shape: it borrows its string +//! filters so a hot retrieval path allocates nothing. That makes it unusable as +//! a contract type in two independent ways — it derives no serde impls, so it +//! cannot be a `POST /v1/memory/recall` request body, and its lifetime +//! parameter would have to be threaded through every `#[async_trait]` recall +//! method, which destroys the object safety the whole driver model rests on. +//! +//! [`OwnedRecallOpts`] is the answer: the same five fields, owned, serde- +//! derived. Contract and wire paths use the owned form; the engine path keeps +//! the borrowed one and converts at the boundary via +//! `RecallOpts::from(&owned)`, which is zero-copy for the string fields. +//! +//! ## Field parity is the contract +//! +//! A field added to one form and not the other is a silent contract hole: the +//! wire would accept a filter the engine never applies, or the engine would +//! offer a filter no remote driver can be told about. Two defences are in +//! place, and both must stay: +//! +//! 1. Both [`From`] impls **exhaustively destructure** their source, so adding +//! a field to either struct without handling it fails to compile. +//! 2. `owned_and_borrowed_recall_opts_have_identical_fields` in +//! `recall_tests.rs` round-trips a fully non-default value through both +//! directions, so a field that is merely *dropped* during conversion fails +//! the test. +//! +//! Both types live in this module (rather than in `types.rs`) precisely so the +//! pair is read and edited together. They are re-exported from +//! [`crate::types`], so every historical `types::RecallOpts` path — including +//! the engine crate's `tinycortex::memory::types::` alias — keeps resolving. + +use serde::{Deserialize, Serialize}; + +use crate::types::MemoryCategory; + +/// Optional filters for recall — the **borrowed, engine-facing** form. +/// +/// Borrows its string filters so an engine call path can pass slices of a +/// caller-owned request without allocating. It is deliberately *not* +/// serializable and deliberately *not* used in the driver contract: a lifetime +/// parameter cannot travel through an object-safe `#[async_trait]` method, and +/// a borrowed struct cannot be a request body. +/// +/// Use [`OwnedRecallOpts`] for anything that crosses a trait object or the +/// wire, and convert at the boundary with the [`From`] impl below. The two +/// types carry the same fields; a field added to one and not the other is a +/// silent contract hole, which +/// `owned_and_borrowed_recall_opts_have_identical_fields` exists to catch. +#[derive(Debug, Default, Clone)] +pub struct RecallOpts<'a> { + /// Restrict recall to this namespace; `None` falls back to [`crate::types::GLOBAL_NAMESPACE`]. + pub namespace: Option<&'a str>, + /// Restrict recall to entries of this category. + pub category: Option, + /// Restrict recall to entries scoped to this session. + pub session_id: Option<&'a str>, + /// Drop hits scoring below this threshold (typically 0.0–1.0). + pub min_score: Option, + /// When `true`, include conversational hits from other sessions in the same + /// workspace alongside the namespace recall. + pub cross_session: bool, +} + +/// Optional filters for recall — the **owned, contract-facing** form. +/// +/// This is the type the driver contract and the JSON wire protocol use. It +/// exists because [`RecallOpts`] cannot serve either role: +/// +/// - it derives no `Serialize`/`Deserialize`, so it cannot be a +/// `POST /v1/memory/recall` request body; +/// - it carries a borrow lifetime, which would have to be threaded through +/// every `#[async_trait]` recall method and destroys object safety at the +/// `dyn` boundary the whole driver model rests on. +/// +/// The borrowed form stays for engine-internal use so the embedded driver's +/// hot path allocates nothing: build the owned value once at the contract +/// boundary, then hand `RecallOpts::from(&owned)` down. +/// +/// Every field is `#[serde(default)]` so a minimal request body — even `{}` — +/// deserializes to the same value as [`Default::default`]. +#[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] +pub struct OwnedRecallOpts { + /// Restrict recall to this namespace; `None` falls back to [`crate::types::GLOBAL_NAMESPACE`]. + #[serde(default)] + pub namespace: Option, + /// Restrict recall to entries of this category. + #[serde(default)] + pub category: Option, + /// Restrict recall to entries scoped to this session. + #[serde(default)] + pub session_id: Option, + /// Drop hits scoring below this threshold (typically 0.0–1.0). + #[serde(default)] + pub min_score: Option, + /// When `true`, include conversational hits from other sessions in the same + /// workspace alongside the namespace recall. + #[serde(default)] + pub cross_session: bool, +} + +impl<'a> From<&'a OwnedRecallOpts> for RecallOpts<'a> { + /// Borrows the owned form for an engine call. Zero-copy for the two string + /// fields; [`MemoryCategory`] is cloned because it owns a `String` in its + /// [`MemoryCategory::Custom`] variant and [`RecallOpts`] holds it by value. + /// + /// Exhaustively destructures the source so adding a field to + /// [`OwnedRecallOpts`] without handling it here is a compile error. + fn from(owned: &'a OwnedRecallOpts) -> Self { + let OwnedRecallOpts { + namespace, + category, + session_id, + min_score, + cross_session, + } = owned; + RecallOpts { + namespace: namespace.as_deref(), + category: category.clone(), + session_id: session_id.as_deref(), + min_score: *min_score, + cross_session: *cross_session, + } + } +} + +impl From> for OwnedRecallOpts { + /// Takes ownership of a borrowed form — the direction a transport adapter + /// needs when turning an engine-shaped call into a request body. + /// + /// Exhaustively destructures the source for the same reason as the inverse + /// impl. + fn from(borrowed: RecallOpts<'_>) -> Self { + let RecallOpts { + namespace, + category, + session_id, + min_score, + cross_session, + } = borrowed; + OwnedRecallOpts { + namespace: namespace.map(str::to_string), + category, + session_id: session_id.map(str::to_string), + min_score, + cross_session, + } + } +} + +#[cfg(test)] +#[path = "recall_tests.rs"] +mod tests; diff --git a/api/src/recall_tests.rs b/api/src/recall_tests.rs new file mode 100644 index 0000000..c008482 --- /dev/null +++ b/api/src/recall_tests.rs @@ -0,0 +1,118 @@ +//! Unit tests for the recall filter contracts in [`super`]. +//! +//! The load-bearing test here is +//! `owned_and_borrowed_recall_opts_have_identical_fields`: it is the runtime +//! half of the field-parity defence described in the module docs (the compile +//! half being the exhaustive destructuring inside both `From` impls). + +use super::*; +use serde_json::json; + +/// Every field set to a non-default value, so a conversion that silently drops +/// one is visible. +fn fully_populated_owned() -> OwnedRecallOpts { + OwnedRecallOpts { + namespace: Some("projects".to_string()), + category: Some(MemoryCategory::Custom("field_notes".to_string())), + session_id: Some("session-42".to_string()), + min_score: Some(0.75), + cross_session: true, + } +} + +#[test] +fn owned_and_borrowed_recall_opts_have_identical_fields() { + let owned = fully_populated_owned(); + + // Owned → borrowed. Destructured exhaustively so a new field on + // `RecallOpts` fails to compile here rather than silently going unchecked. + let borrowed = RecallOpts::from(&owned); + let RecallOpts { + namespace, + category, + session_id, + min_score, + cross_session, + } = borrowed.clone(); + assert_eq!(namespace, Some("projects")); + assert_eq!(category, Some(MemoryCategory::Custom("field_notes".into()))); + assert_eq!(session_id, Some("session-42")); + assert_eq!(min_score, Some(0.75)); + assert!(cross_session); + + // Borrowed → owned, and back to the value we started from. A field dropped + // in either direction fails this equality. + let round_tripped = OwnedRecallOpts::from(borrowed); + assert_eq!(round_tripped, owned); +} + +#[test] +fn borrowed_view_is_zero_copy_over_the_owned_strings() { + let owned = fully_populated_owned(); + let borrowed = RecallOpts::from(&owned); + + // The borrowed form points *into* the owned value rather than at a copy; + // that is the whole reason the borrowed form survives. + assert_eq!( + borrowed.namespace.unwrap().as_ptr(), + owned.namespace.as_deref().unwrap().as_ptr() + ); + assert_eq!( + borrowed.session_id.unwrap().as_ptr(), + owned.session_id.as_deref().unwrap().as_ptr() + ); +} + +#[test] +fn owned_recall_opts_defaults_match_borrowed_defaults() { + let owned = OwnedRecallOpts::default(); + let borrowed = RecallOpts::from(&owned); + + assert!(borrowed.namespace.is_none()); + assert!(borrowed.category.is_none()); + assert!(borrowed.session_id.is_none()); + assert!(borrowed.min_score.is_none()); + assert!(!borrowed.cross_session); + + // And the borrowed default converts back to the owned default. + assert_eq!(OwnedRecallOpts::from(RecallOpts::default()), owned); +} + +#[test] +fn owned_recall_opts_serde_round_trips_every_field() { + let owned = fully_populated_owned(); + let encoded = serde_json::to_value(&owned).unwrap(); + + assert_eq!( + encoded, + json!({ + "namespace": "projects", + "category": "custom:field_notes", + "session_id": "session-42", + "min_score": 0.75, + "cross_session": true + }) + ); + + let decoded: OwnedRecallOpts = serde_json::from_value(encoded).unwrap(); + assert_eq!(decoded, owned); +} + +#[test] +fn empty_recall_body_deserializes_to_the_default() { + // A minimal `POST /v1/memory/recall` body must be accepted: every field is + // `#[serde(default)]`. + let decoded: OwnedRecallOpts = serde_json::from_value(json!({})).unwrap(); + assert_eq!(decoded, OwnedRecallOpts::default()); +} + +#[test] +fn partial_recall_body_leaves_unmentioned_fields_at_default() { + let decoded: OwnedRecallOpts = + serde_json::from_value(json!({ "namespace": "global" })).unwrap(); + assert_eq!(decoded.namespace.as_deref(), Some("global")); + assert!(decoded.category.is_none()); + assert!(decoded.session_id.is_none()); + assert!(decoded.min_score.is_none()); + assert!(!decoded.cross_session); +} diff --git a/api/src/types.rs b/api/src/types.rs index e5914c9..b683b2f 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -26,6 +26,12 @@ use serde::{Deserialize, Serialize}; +/// The recall filter contracts live in [`crate::recall`] so the borrowed and +/// owned forms sit next to each other and cannot drift, and are re-exported +/// here so every historical `types::RecallOpts` path — including the engine +/// crate's `tinycortex::memory::types::` alias — keeps resolving unchanged. +pub use crate::recall::{OwnedRecallOpts, RecallOpts}; + /// Default namespace used when a caller passes no explicit namespace. pub const GLOBAL_NAMESPACE: &str = "global"; @@ -204,22 +210,6 @@ pub struct MemoryEntry { pub taint: MemoryTaint, } -/// Optional filters for recall. -#[derive(Debug, Default, Clone)] -pub struct RecallOpts<'a> { - /// Restrict recall to this namespace; `None` falls back to [`GLOBAL_NAMESPACE`]. - pub namespace: Option<&'a str>, - /// Restrict recall to entries of this category. - pub category: Option, - /// Restrict recall to entries scoped to this session. - pub session_id: Option<&'a str>, - /// Drop hits scoring below this threshold (typically 0.0–1.0). - pub min_score: Option, - /// When `true`, include conversational hits from other sessions in the same - /// workspace alongside the namespace recall. - pub cross_session: bool, -} - /// Summary row for agent-side namespace discovery. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct NamespaceSummary { From 11de07c80482b55826dc8264c586106fb2c0d494 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 21:49:17 +0300 Subject: [PATCH 07/19] feat(api): add the thirteen Capability families and the Capabilities set Capabilities are negotiated once at bind time; an unadvertised family means the RPC methods are unregistered and the agent tools absent, not present-and-failing. Capability carries exactly the thirteen contract families. Capabilities is a bitset that serialises as a JSON array of stable snake_case strings (the handshake capabilities[] field), never discriminant integers, so inserting a variant mid-enum cannot silently re-map an already-deployed driver's set. Capabilities::validate() encodes the mandatory rule in code: core, recall, and portability must all be present, and a missing one is reported structurally via MissingMandatoryCapabilities so the bind site can surface which. The enum is deliberately NOT #[non_exhaustive]: adding a family is a minor contract bump and should break every host match that filters registration by family - that compile error is what guarantees the family gets wired. --- api/src/capabilities.rs | 384 ++++++++++++++++++++++++++++++++++ api/src/capabilities_tests.rs | 303 +++++++++++++++++++++++++++ api/src/lib.rs | 13 +- 3 files changed, 698 insertions(+), 2 deletions(-) create mode 100644 api/src/capabilities.rs create mode 100644 api/src/capabilities_tests.rs diff --git a/api/src/capabilities.rs b/api/src/capabilities.rs new file mode 100644 index 0000000..1d67575 --- /dev/null +++ b/api/src/capabilities.rs @@ -0,0 +1,384 @@ +//! Capability families a memory driver may advertise, and the set type used to +//! negotiate them. +//! +//! ## Why capabilities exist +//! +//! A memory driver is not required to implement the whole surface. The kernel +//! asks a driver which families it supports **once**, at bind time, caches the +//! answer, and then unregisters the RPC methods and omits the agent tools that +//! belong to an unadvertised family. Absence beats a registered handler that +//! returns "not implemented": a present-but-failing method teaches a model that +//! the capability exists and makes it retry. +//! +//! Calling an unadvertised capability is therefore a *kernel* bug, not a driver +//! error. [`crate::error::MemoryError::Unsupported`] exists for the one case the +//! kernel cannot pre-empt: an out-of-process driver that answers `501` for a +//! family its handshake claimed. +//! +//! ## Mandatory families +//! +//! [`Capability::Core`], [`Capability::Recall`], and [`Capability::Portability`] +//! are mandatory. Without core and recall a driver is not a memory backend at +//! all; without portability a user cannot leave it, which makes the binding a +//! one-way door. [`Capabilities::validate`] is the single place that rule is +//! encoded — call it at bind time and refuse the bind on `Err`. +//! +//! ## Wire stability +//! +//! The set crosses the process boundary in the driver handshake +//! (`POST /v1/handshake` → `{ contract_version, driver_id, capabilities[] }`), +//! so the serialized form is a JSON **array of stable snake_case strings**, not +//! discriminant integers — inserting a variant in the middle of the enum must +//! not silently re-map an already-deployed driver's advertised set. +//! [`Capability::as_str`] is the authority for those strings and is pinned +//! against the serde derive by a test. +//! +//! ## Deliberately not `#[non_exhaustive]` +//! +//! Adding a family is a [`crate::CONTRACT_VERSION`] **minor** bump and should +//! break every exhaustive `match` in every host that filters registration by +//! family — that compile error is the mechanism which guarantees the new family +//! is actually wired somewhere. Marking this enum `#[non_exhaustive]` would +//! convert that compile-time guarantee into a silent fall-through at the crate +//! boundary (the failure mode recorded for `DataSource` during the M0 +//! carve-out). If a future family must be added without breaking downstream +//! matches, bump the **major** version instead. + +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +use crate::error::MemoryError; + +/// One capability family a memory driver may advertise. +/// +/// The variants are exactly the thirteen families of the memory contract. Each +/// maps to a trait family in the contract, a group of RPC methods, and a group +/// of agent tools; a driver that does not advertise a family simply has that +/// surface absent. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum Capability { + /// Store / get / forget / list / namespaces. **Mandatory.** + Core, + /// Ranked retrieval for a query. **Mandatory.** + Recall, + /// Document and chat ingestion — the driver owns chunking and embedding. + Ingest, + /// The namespace-document tier: put / get / query documents. + Documents, + /// Summary-tree query, drill-down, seal, and cascade. + Tree, + /// Entity index, entity edges, and hotness. + Entities, + /// Key/value graph read and write. + Graph, + /// Snapshot capture and change computation. + Diff, + /// Goal extraction and goal records. + Goals, + /// Per-tool learned memory. + ToolMemory, + /// Accepting synced source items; the host still owns credentials and + /// scheduling. + Sources, + /// Re-embed, compact, consolidate ("dream"), and doctor. + Maintenance, + /// Export and import of the whole store as a stream. **Mandatory.** + Portability, +} + +impl Capability { + /// Every family, in declaration order. + /// + /// Declaration order is also bit order in [`Capabilities`] and iteration + /// order in its serialized form, so this slice is the single ordering + /// authority for the whole module. + pub const ALL: [Capability; 13] = [ + Capability::Core, + Capability::Recall, + Capability::Ingest, + Capability::Documents, + Capability::Tree, + Capability::Entities, + Capability::Graph, + Capability::Diff, + Capability::Goals, + Capability::ToolMemory, + Capability::Sources, + Capability::Maintenance, + Capability::Portability, + ]; + + /// The families a driver must advertise to be bindable at all. + /// + /// See the module docs for why these three and not others. + pub const MANDATORY: [Capability; 3] = [ + Capability::Core, + Capability::Recall, + Capability::Portability, + ]; + + /// Every family, in declaration order. Slice form of [`Self::ALL`], for + /// callers that want to iterate without naming the array length. + pub fn all() -> &'static [Capability] { + &Self::ALL + } + + /// Stable snake_case identifier used on the wire, in config, and in logs. + /// + /// This is the authority for the serialized form; the serde derive is + /// pinned against it by `capability_as_str_matches_serde_representation`. + /// Changing a string here is a breaking change for every already-deployed + /// driver and requires a [`crate::CONTRACT_VERSION`] major bump. + pub fn as_str(self) -> &'static str { + match self { + Self::Core => "core", + Self::Recall => "recall", + Self::Ingest => "ingest", + Self::Documents => "documents", + Self::Tree => "tree", + Self::Entities => "entities", + Self::Graph => "graph", + Self::Diff => "diff", + Self::Goals => "goals", + Self::ToolMemory => "tool_memory", + Self::Sources => "sources", + Self::Maintenance => "maintenance", + Self::Portability => "portability", + } + } + + /// Parse back from the on-wire form. + /// + /// # Errors + /// + /// Returns the unrecognised input in an error message. An unknown string is + /// expected in practice: a driver speaking a newer minor contract version + /// may advertise a family this build has never heard of. Callers + /// negotiating a handshake should **skip** unknown families rather than + /// fail the bind — an unknown family is one this kernel would never call. + pub fn parse(raw: &str) -> Result { + Self::ALL + .iter() + .copied() + .find(|cap| cap.as_str() == raw) + .ok_or_else(|| format!("unknown memory capability: {raw}")) + } + + /// Whether this family is mandatory for every driver. + pub fn is_mandatory(self) -> bool { + Self::MANDATORY.contains(&self) + } + + /// Position of this family in [`Self::ALL`]; also its bit index in + /// [`Capabilities`]. + fn index(self) -> u16 { + match self { + Self::Core => 0, + Self::Recall => 1, + Self::Ingest => 2, + Self::Documents => 3, + Self::Tree => 4, + Self::Entities => 5, + Self::Graph => 6, + Self::Diff => 7, + Self::Goals => 8, + Self::ToolMemory => 9, + Self::Sources => 10, + Self::Maintenance => 11, + Self::Portability => 12, + } + } + + /// Single-bit mask for this family within a [`Capabilities`] set. + fn bit(self) -> u16 { + 1 << self.index() + } +} + +impl std::fmt::Display for Capability { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +impl std::str::FromStr for Capability { + type Err = String; + + fn from_str(raw: &str) -> Result { + Self::parse(raw) + } +} + +/// A driver's advertised capability set. +/// +/// Internally a bitset, so `contains` is a single mask test on the hot path and +/// the type is `Copy`. Externally it serializes as a JSON array of +/// [`Capability::as_str`] strings in [`Capability::ALL`] order — duplicates in +/// the input collapse, and ordering in the input is not preserved, because a +/// set has neither. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] +pub struct Capabilities { + bits: u16, +} + +impl Capabilities { + /// The empty set. Advertised by the `null` driver. + pub const fn empty() -> Self { + Self { bits: 0 } + } + + /// Every family. Advertised by the embedded `tinycortex` driver. + pub fn all() -> Self { + Capability::ALL.into_iter().collect() + } + + /// Exactly the mandatory families — the minimum bindable set. + pub fn mandatory() -> Self { + Capability::MANDATORY.into_iter().collect() + } + + /// Whether `capability` is advertised. + pub fn contains(&self, capability: Capability) -> bool { + self.bits & capability.bit() != 0 + } + + /// Whether every family in `other` is advertised here. + pub fn contains_all(&self, other: Capabilities) -> bool { + self.bits & other.bits == other.bits + } + + /// Adds `capability` in place. Idempotent. + pub fn insert(&mut self, capability: Capability) { + self.bits |= capability.bit(); + } + + /// Removes `capability` in place. Idempotent. + pub fn remove(&mut self, capability: Capability) { + self.bits &= !capability.bit(); + } + + /// Builder form of [`Self::insert`]. + pub fn with(mut self, capability: Capability) -> Self { + self.insert(capability); + self + } + + /// Builder form of [`Self::remove`]. + pub fn without(mut self, capability: Capability) -> Self { + self.remove(capability); + self + } + + /// Advertised families in [`Capability::ALL`] order. + pub fn iter(&self) -> impl Iterator + '_ { + Capability::ALL + .into_iter() + .filter(move |cap| self.contains(*cap)) + } + + /// Number of advertised families. + pub fn len(&self) -> usize { + self.bits.count_ones() as usize + } + + /// Whether no family is advertised. + pub fn is_empty(&self) -> bool { + self.bits == 0 + } + + /// Mandatory families this set is missing, in [`Capability::ALL`] order. + /// Empty when the set is bindable. + pub fn missing_mandatory(&self) -> Vec { + Capability::MANDATORY + .into_iter() + .filter(|cap| !self.contains(*cap)) + .collect() + } + + /// Rejects a set that is missing any mandatory family. + /// + /// Call this at bind time; on `Err` refuse the bind and fall back to the + /// embedded default rather than binding a driver a user could not leave. + /// + /// # Errors + /// + /// Returns [`MissingMandatoryCapabilities`] listing **every** missing + /// mandatory family, not just the first, so the operator sees the whole gap + /// in one message. + pub fn validate(&self) -> Result<(), MissingMandatoryCapabilities> { + let missing = self.missing_mandatory(); + if missing.is_empty() { + Ok(()) + } else { + Err(MissingMandatoryCapabilities { missing }) + } + } +} + +impl FromIterator for Capabilities { + fn from_iter>(iter: I) -> Self { + let mut set = Self::empty(); + for capability in iter { + set.insert(capability); + } + set + } +} + +impl Extend for Capabilities { + fn extend>(&mut self, iter: I) { + for capability in iter { + self.insert(capability); + } + } +} + +impl Serialize for Capabilities { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.collect_seq(self.iter()) + } +} + +impl<'de> Deserialize<'de> for Capabilities { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let families = Vec::::deserialize(deserializer)?; + Ok(families.into_iter().collect()) + } +} + +/// A driver advertised a capability set missing at least one mandatory family. +/// +/// Carries the missing families rather than a formatted string so the caller +/// can report them structurally (status RPC, bind-failure event) as well as in +/// a log line. +#[derive(Debug, Clone, PartialEq, Eq, Error)] +#[error( + "memory driver advertises an incomplete capability set; missing mandatory families: {}", + .missing.iter().map(|c| c.as_str()).collect::>().join(", ") +)] +pub struct MissingMandatoryCapabilities { + /// Mandatory families absent from the advertised set, in + /// [`Capability::ALL`] order. Never empty. + pub missing: Vec, +} + +impl From for MemoryError { + /// An incomplete advertised set is a caller/config error, not an + /// unsupported call: the driver said something invalid about itself, which + /// is why this maps to [`MemoryError::Invalid`] and not + /// [`MemoryError::Unsupported`]. + fn from(value: MissingMandatoryCapabilities) -> Self { + MemoryError::Invalid(value.to_string()) + } +} + +#[cfg(test)] +#[path = "capabilities_tests.rs"] +mod tests; diff --git a/api/src/capabilities_tests.rs b/api/src/capabilities_tests.rs new file mode 100644 index 0000000..4f4002e --- /dev/null +++ b/api/src/capabilities_tests.rs @@ -0,0 +1,303 @@ +//! Unit tests for the capability vocabulary in [`super`]. +//! +//! Three properties are load-bearing and each has its own test: +//! +//! 1. the enum has exactly the thirteen contract families and no more; +//! 2. the serialized form is stable snake_case **strings**, never discriminant +//! integers — a driver deployed against an older build must keep advertising +//! the same set after a variant is inserted mid-enum; +//! 3. [`super::Capabilities::validate`] rejects a set missing **any** of the +//! three mandatory families, checked one family at a time. + +use super::*; +use serde_json::json; + +#[test] +fn capability_has_exactly_the_thirteen_contract_families() { + assert_eq!(Capability::ALL.len(), 13); + assert_eq!(Capability::all().len(), 13); + + let names: Vec<&str> = Capability::ALL.iter().map(|c| c.as_str()).collect(); + assert_eq!( + names, + vec![ + "core", + "recall", + "ingest", + "documents", + "tree", + "entities", + "graph", + "diff", + "goals", + "tool_memory", + "sources", + "maintenance", + "portability", + ] + ); +} + +#[test] +fn capability_all_has_no_duplicates() { + let mut seen = std::collections::BTreeSet::new(); + for capability in Capability::ALL { + assert!( + seen.insert(capability.as_str()), + "duplicate capability in ALL: {capability}" + ); + } +} + +#[test] +fn capability_as_str_matches_serde_representation() { + // The wire form is the stable contract; `as_str` is the authority and the + // derive must agree with it for every variant. + for capability in Capability::ALL { + assert_eq!( + serde_json::to_value(capability).unwrap(), + json!(capability.as_str()), + "serde form drifted from as_str for {capability}" + ); + } +} + +#[test] +fn capability_serializes_as_a_string_not_an_integer() { + // Guards the specific regression the string form exists to prevent: + // inserting a variant must not re-map an already-deployed driver's set. + for capability in Capability::ALL { + assert!( + serde_json::to_value(capability).unwrap().is_string(), + "{capability} did not serialize as a string" + ); + } +} + +#[test] +fn capability_parse_round_trips_every_variant() { + for capability in Capability::ALL { + assert_eq!(Capability::parse(capability.as_str()), Ok(capability)); + assert_eq!( + capability.as_str().parse::(), + Ok(capability), + "FromStr disagreed with parse for {capability}" + ); + let decoded: Capability = + serde_json::from_value(json!(capability.as_str())).expect("known family decodes"); + assert_eq!(decoded, capability); + } +} + +#[test] +fn capability_parse_rejects_unknown_family() { + let err = Capability::parse("quantum_recall").expect_err("unknown family must not parse"); + assert!(err.contains("quantum_recall"), "unhelpful error: {err}"); +} + +#[test] +fn mandatory_families_are_core_recall_and_portability() { + assert_eq!( + Capability::MANDATORY, + [ + Capability::Core, + Capability::Recall, + Capability::Portability + ] + ); + for capability in Capability::ALL { + assert_eq!( + capability.is_mandatory(), + matches!( + capability, + Capability::Core | Capability::Recall | Capability::Portability + ), + "wrong mandatory classification for {capability}" + ); + } +} + +#[test] +fn capabilities_all_contains_every_family() { + let all = Capabilities::all(); + assert_eq!(all.len(), Capability::ALL.len()); + for capability in Capability::ALL { + assert!(all.contains(capability), "all() is missing {capability}"); + } + assert!(!all.is_empty()); +} + +#[test] +fn capabilities_empty_contains_nothing() { + let none = Capabilities::empty(); + assert!(none.is_empty()); + assert_eq!(none.len(), 0); + for capability in Capability::ALL { + assert!(!none.contains(capability)); + } + // The `null` driver's set is the default. + assert_eq!(Capabilities::default(), none); +} + +#[test] +fn capabilities_insert_and_remove_are_idempotent() { + let mut set = Capabilities::empty(); + set.insert(Capability::Tree); + set.insert(Capability::Tree); + assert_eq!(set.len(), 1); + assert!(set.contains(Capability::Tree)); + assert!(!set.contains(Capability::Graph)); + + set.remove(Capability::Tree); + set.remove(Capability::Tree); + assert!(set.is_empty()); +} + +#[test] +fn capabilities_builder_forms_mirror_insert_and_remove() { + let set = Capabilities::empty() + .with(Capability::Core) + .with(Capability::Recall) + .without(Capability::Recall); + assert!(set.contains(Capability::Core)); + assert!(!set.contains(Capability::Recall)); +} + +#[test] +fn capabilities_contains_all_checks_subsets() { + let full = Capabilities::all(); + let mandatory = Capabilities::mandatory(); + + assert!(full.contains_all(mandatory)); + assert!(!mandatory.contains_all(full)); + assert!(mandatory.contains_all(mandatory)); + assert!(full.contains_all(Capabilities::empty())); +} + +#[test] +fn capabilities_iterates_in_declaration_order() { + let set: Capabilities = [ + Capability::Portability, + Capability::Core, + Capability::Tree, + Capability::Recall, + ] + .into_iter() + .collect(); + + assert_eq!( + set.iter().collect::>(), + vec![ + Capability::Core, + Capability::Recall, + Capability::Tree, + Capability::Portability + ] + ); +} + +#[test] +fn capabilities_serde_round_trips_and_uses_a_string_array() { + let set = Capabilities::mandatory().with(Capability::ToolMemory); + let encoded = serde_json::to_value(set).unwrap(); + + // Declaration order, snake_case strings — the `capabilities[]` handshake + // field. + assert_eq!( + encoded, + json!(["core", "recall", "tool_memory", "portability"]) + ); + + let decoded: Capabilities = serde_json::from_value(encoded).unwrap(); + assert_eq!(decoded, set); +} + +#[test] +fn capabilities_full_set_serde_round_trips() { + let all = Capabilities::all(); + let encoded = serde_json::to_string(&all).unwrap(); + let decoded: Capabilities = serde_json::from_str(&encoded).unwrap(); + assert_eq!(decoded, all); +} + +#[test] +fn capabilities_deserialization_collapses_duplicates_and_ignores_order() { + let decoded: Capabilities = + serde_json::from_value(json!(["portability", "core", "core", "recall"])).unwrap(); + assert_eq!(decoded, Capabilities::mandatory()); + assert_eq!(decoded.len(), 3); +} + +#[test] +fn capabilities_deserialization_rejects_an_unknown_family() { + // The set type itself is strict; a handshake parser that wants to *skip* + // unknown families must decode the array element-wise via + // `Capability::parse`, which is why that function returns a `Result`. + let err = serde_json::from_value::(json!(["core", "warp_drive"])) + .expect_err("unknown family must not silently decode"); + assert!(err.to_string().contains("warp_drive"), "{err}"); +} + +#[test] +fn validate_accepts_the_minimum_bindable_set() { + assert_eq!(Capabilities::mandatory().validate(), Ok(())); + assert_eq!(Capabilities::all().validate(), Ok(())); +} + +#[test] +fn validate_rejects_a_set_missing_core() { + let set = Capabilities::all().without(Capability::Core); + let err = set.validate().expect_err("missing core must be rejected"); + assert_eq!(err.missing, vec![Capability::Core]); + assert!(err.to_string().contains("core"), "{err}"); +} + +#[test] +fn validate_rejects_a_set_missing_recall() { + let set = Capabilities::all().without(Capability::Recall); + let err = set.validate().expect_err("missing recall must be rejected"); + assert_eq!(err.missing, vec![Capability::Recall]); + assert!(err.to_string().contains("recall"), "{err}"); +} + +#[test] +fn validate_rejects_a_set_missing_portability() { + // Portability is mandatory because without it a bind is a one-way door. + let set = Capabilities::all().without(Capability::Portability); + let err = set + .validate() + .expect_err("missing portability must be rejected"); + assert_eq!(err.missing, vec![Capability::Portability]); + assert!(err.to_string().contains("portability"), "{err}"); +} + +#[test] +fn validate_reports_every_missing_mandatory_family_at_once() { + let err = Capabilities::empty() + .validate() + .expect_err("the null set must be rejected"); + assert_eq!( + err.missing, + vec![ + Capability::Core, + Capability::Recall, + Capability::Portability + ] + ); +} + +#[test] +fn missing_mandatory_converts_to_an_invalid_memory_error() { + let err = Capabilities::empty().validate().unwrap_err(); + let message = err.to_string(); + let converted: MemoryError = err.into(); + // An incomplete advertised set is a bad claim about the driver, not an + // unsupported call. + assert!(matches!(converted, MemoryError::Invalid(ref m) if *m == message)); +} + +#[test] +fn missing_mandatory_is_empty_for_a_valid_set() { + assert!(Capabilities::mandatory().missing_mandatory().is_empty()); + assert!(Capabilities::all().missing_mandatory().is_empty()); +} diff --git a/api/src/lib.rs b/api/src/lib.rs index 89cc6b7..5862f2e 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -1,6 +1,7 @@ //! Stable public contracts for the TinyCortex memory system. //! -//! This crate holds the value types, error enum, and storage trait that both the `tinycortex` engine and its embedding hosts +//! This crate holds the value types, error enum, capability vocabulary, and +//! storage trait that both the `tinycortex` engine and its embedding hosts //! compile against. It is deliberately dependency-light (serde / serde_json / //! chrono / sha2 / anyhow / thiserror / async-trait / uuid only) so depending on //! the contract never drags in SQLite, git2, reqwest, regex, or an async @@ -12,7 +13,12 @@ //! depend on this crate alone, and the *generic* subsystem/driver vocabulary of //! the OpenHuman kernel (`Driver`, `DriverClass`, `SubsystemRegistry`, the //! policy `Guard`) must not be inherited from a *memory* crate by whichever -//! subsystem is cut over next. +//! subsystem is cut over next. So the contract carries its own capability +//! vocabulary, and the host's memory adapter converts at the boundary. +//! +//! Driver *class* (embedded / external / null) is deliberately **absent**: that +//! is a host configuration fact about how a driver was bound, not something a +//! driver reports about itself. //! //! ## The engine's historical paths still resolve //! @@ -28,6 +34,8 @@ //! - [`recall`]: the borrowed [`recall::RecallOpts`] and owned, serde-derived //! [`recall::OwnedRecallOpts`] recall filters (both re-exported from //! [`types`]). +//! - [`capabilities`]: the thirteen [`capabilities::Capability`] families and +//! the [`capabilities::Capabilities`] set negotiated at bind time. //! - [`error`]: the typed [`error::MemoryError`] enum and its result alias. //! - [`traits`]: the [`traits::Memory`] storage-backend trait. //! - [`chunks`]: the persisted chunk model ([`chunks::Chunk`], [`chunks::Metadata`], @@ -37,6 +45,7 @@ //! - [`tool_memory`]: tool-scoped rule contracts ([`tool_memory::ToolMemoryRule`], …). //! - [`goals`]: the long-term goals document ([`goals::GoalsDoc`], [`goals::GoalItem`]). +pub mod capabilities; pub mod chunks; pub mod error; pub mod goals; From e1f76c0b66b436709ff487a47ae9c55be9b9df15 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 21:49:32 +0300 Subject: [PATCH 08/19] feat(api): add MemoryError::Unsupported for the driver contract The 501 -> Unsupported mapping the wire contract specifies had no target: the existing variants are NotFound/Invalid/BudgetExceeded/PathEscape/Io/Serde/Other. The payload is an owned String, not a Capability and not a &'static str, because the transport adapter constructs this from a wire response where the family is a runtime value that may not be a known Capability at all - a driver speaking a newer minor contract version, a vendor extension, or a typo. A Capability field would force the adapter to drop that information or fail parsing; a &'static str cannot be produced from a runtime value without leaking. MemoryError::unsupported(Capability) yields the canonical spelling for the known case, unsupported_raw() preserves the wire string otherwise. Adding a variant is safe: nothing in the engine or the host matches MemoryError exhaustively (construction and matches! only). --- api/src/error.rs | 60 ++++++++++++++++++++++++++++++++++++++++++ api/src/error_tests.rs | 59 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 api/src/error_tests.rs diff --git a/api/src/error.rs b/api/src/error.rs index f04ce10..c9e6405 100644 --- a/api/src/error.rs +++ b/api/src/error.rs @@ -11,9 +11,16 @@ //! [`MemoryError::Invalid`], [`MemoryError::BudgetExceeded`], //! [`MemoryError::PathEscape`]) are constructed explicitly by callers that want //! matchable, typed failure — they are never inferred from a foreign error. +//! +//! [`MemoryError::Unsupported`] is the one variant that belongs to the *driver +//! contract* rather than the engine: it is what a caller gets when a bound +//! driver does not implement the capability family a call needs. See its docs +//! for why that should be rare. use thiserror::Error; +use crate::capabilities::Capability; + /// Errors surfaced by the memory engine. #[derive(Debug, Error)] pub enum MemoryError { @@ -35,10 +42,63 @@ pub enum MemoryError { /// Serialization / deserialization failure. #[error("serde error: {0}")] Serde(#[from] serde_json::Error), + /// The bound driver does not implement the named capability family. + /// + /// This should be **rare**, because capabilities are negotiated once at + /// bind time and the kernel unregisters the RPC methods and omits the agent + /// tools of every unadvertised family. Reaching this variant means one of: + /// + /// - an out-of-process driver answered `501` for a family its handshake + /// claimed (the case [`crate::capabilities`] cannot pre-empt); + /// - a caller bypassed the capability filter — a kernel bug. + /// + /// ## Why the payload is an owned `String` and not a [`Capability`] + /// + /// The transport adapter constructs this from a wire response, where the + /// family is a runtime string that may not be a known [`Capability`] at all + /// — a driver speaking a newer minor contract version, a vendor extension, + /// or simply a typo in a third-party backend. A `Capability` field would + /// force the adapter to drop that information or fail parsing, and a + /// `&'static str` cannot be produced from a runtime value without leaking + /// memory. An owned `String` is the only representation that round-trips + /// every case. + /// + /// Construct it with [`MemoryError::unsupported`] when the family is known + /// (that path yields the canonical [`Capability::as_str`] spelling) and + /// with [`MemoryError::unsupported_raw`] when it came off the wire. + #[error("unsupported capability: {capability}")] + Unsupported { + /// Wire name of the capability family that is not supported — + /// [`Capability::as_str`] when known, otherwise the raw string the + /// driver reported. + capability: String, + }, /// Catch-all wrapping an opaque lower-level error. #[error(transparent)] Other(#[from] anyhow::Error), } +impl MemoryError { + /// Builds [`MemoryError::Unsupported`] for a family this build knows, + /// using its canonical [`Capability::as_str`] spelling. + pub fn unsupported(capability: Capability) -> Self { + Self::Unsupported { + capability: capability.as_str().to_string(), + } + } + + /// Builds [`MemoryError::Unsupported`] from a family name that came off the + /// wire and may not correspond to any known [`Capability`]. + pub fn unsupported_raw(capability: impl Into) -> Self { + Self::Unsupported { + capability: capability.into(), + } + } +} + /// Convenience result alias for engine-level fallible operations. pub type MemoryEngineResult = Result; + +#[cfg(test)] +#[path = "error_tests.rs"] +mod tests; diff --git a/api/src/error_tests.rs b/api/src/error_tests.rs new file mode 100644 index 0000000..75d72bc --- /dev/null +++ b/api/src/error_tests.rs @@ -0,0 +1,59 @@ +//! Unit tests for [`super::MemoryError`], focused on the `Unsupported` variant +//! added for the driver contract. The older variants are exercised where they +//! are constructed, in the engine crate. + +use super::*; +use crate::capabilities::Capability; + +#[test] +fn unsupported_from_a_known_capability_uses_the_canonical_wire_name() { + for capability in Capability::ALL { + let err = MemoryError::unsupported(capability); + match err { + MemoryError::Unsupported { + capability: ref got, + } => { + assert_eq!(got, capability.as_str()); + } + other => panic!("expected Unsupported, got {other:?}"), + } + } +} + +#[test] +fn unsupported_raw_preserves_a_family_this_build_does_not_know() { + // The reason the payload is an owned `String`: a driver speaking a newer + // minor contract version can name a family that is not a `Capability` here, + // and the adapter must be able to report it verbatim. + let err = MemoryError::unsupported_raw("holographic_recall"); + match err { + MemoryError::Unsupported { ref capability } => { + assert_eq!(capability, "holographic_recall"); + assert!(Capability::parse(capability).is_err()); + } + other => panic!("expected Unsupported, got {other:?}"), + } +} + +#[test] +fn unsupported_display_names_the_capability() { + assert_eq!( + MemoryError::unsupported(Capability::Tree).to_string(), + "unsupported capability: tree" + ); + assert_eq!( + MemoryError::unsupported(Capability::ToolMemory).to_string(), + "unsupported capability: tool_memory" + ); +} + +#[test] +fn unsupported_is_distinguishable_from_the_other_variants() { + // A transport adapter maps `501` to `Unsupported` and everything else + // elsewhere, so the variant must not collide with `Invalid` / `NotFound`. + let unsupported = MemoryError::unsupported(Capability::Diff); + assert!(matches!(unsupported, MemoryError::Unsupported { .. })); + + let invalid = MemoryError::Invalid("diff".to_string()); + assert!(!matches!(invalid, MemoryError::Unsupported { .. })); +} From 50738e3db14af9705e8f338aeb425f77a9d64b3e Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 21:49:46 +0300 Subject: [PATCH 09/19] feat(api): pin CONTRACT_VERSION and encode the compatibility rule in code CONTRACT_VERSION = (1, 0), re-exported at the crate root. Minor bump = a capability was added; major bump = an existing signature changed. is_compatible() puts the bind rule in code rather than prose: compatible exactly when the major halves match. A minor difference is accepted in both directions because capability negotiation already covers it - a remote ahead advertises families this build skips as unknown, a remote behind simply does not advertise families this build knows, which is the ordinary degradation path. Refusing on a minor difference would reject a usable driver and make adding a family a fleet-wide breaking change. --- api/src/lib.rs | 4 +++ api/src/version.rs | 75 ++++++++++++++++++++++++++++++++++++++++ api/src/version_tests.rs | 63 +++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+) create mode 100644 api/src/version.rs create mode 100644 api/src/version_tests.rs diff --git a/api/src/lib.rs b/api/src/lib.rs index 5862f2e..d9b4a8e 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -36,6 +36,7 @@ //! [`types`]). //! - [`capabilities`]: the thirteen [`capabilities::Capability`] families and //! the [`capabilities::Capabilities`] set negotiated at bind time. +//! - [`version`]: [`CONTRACT_VERSION`] and the [`is_compatible`] bind rule. //! - [`error`]: the typed [`error::MemoryError`] enum and its result alias. //! - [`traits`]: the [`traits::Memory`] storage-backend trait. //! - [`chunks`]: the persisted chunk model ([`chunks::Chunk`], [`chunks::Metadata`], @@ -54,3 +55,6 @@ pub mod tool_memory; pub mod traits; pub mod tree; pub mod types; +pub mod version; + +pub use version::{is_compatible, CONTRACT_VERSION}; diff --git a/api/src/version.rs b/api/src/version.rs new file mode 100644 index 0000000..281482d --- /dev/null +++ b/api/src/version.rs @@ -0,0 +1,75 @@ +//! The memory contract version and the compatibility rule that governs it. +//! +//! Re-exported at the crate root, so the canonical paths are +//! [`crate::CONTRACT_VERSION`] and [`crate::is_compatible`]. +//! +//! ## The rule +//! +//! `CONTRACT_VERSION` is `(major, minor)`: +//! +//! - **Minor bump — a capability was added.** A new [`crate::capabilities::Capability`] +//! family, or a new method inside an existing family that older drivers may +//! simply not implement. Nothing that already compiled stops compiling. +//! - **Major bump — an existing signature changed.** A method's parameters or +//! return type moved, a mandatory family was added or removed, or a wire +//! string changed. Anything an existing driver already implements now means +//! something different. +//! +//! ## Why only the major half gates the bind +//! +//! An out-of-process driver reports the version it speaks in its handshake +//! (`POST /v1/handshake` → `{ contract_version, driver_id, capabilities[] }`). +//! **A major mismatch refuses the bind**; a minor difference in either +//! direction is accepted, because capability negotiation already covers it: +//! +//! - remote minor > local minor — the driver advertises families this build has +//! never heard of. Unknown family strings are skipped during handshake +//! parsing, so this kernel simply never calls them. +//! - remote minor < local minor — the driver is missing families this build +//! knows about. It does not advertise them, so the corresponding RPC methods +//! are unregistered and the agent tools are absent. That is the ordinary +//! degradation path, not an error. +//! +//! Refusing on a minor difference would therefore reject a driver that is +//! perfectly usable, and would make adding a family a fleet-wide breaking +//! change — which is exactly what the major/minor split exists to avoid. +//! +//! Encoding the rule here rather than in prose means a caller cannot get it +//! subtly wrong: the bind path calls [`is_compatible`], never compares tuples +//! by hand. + +/// Version of the memory contract this crate defines, as `(major, minor)`. +/// +/// See the module docs for the bump rule. Bump the **minor** half when a +/// capability family or an additive method lands; bump the **major** half — and +/// reset the minor to `0` — when an existing signature, mandatory family, or +/// wire string changes. +pub const CONTRACT_VERSION: (u16, u16) = (1, 0); + +/// Whether a driver speaking `remote` can be bound against this build. +/// +/// Compatible exactly when the major halves match. See the module docs for why +/// the minor half is informational. +/// +/// # Examples +/// +/// ``` +/// use tinycortex_api::{is_compatible, CONTRACT_VERSION}; +/// +/// // The version this build speaks is always compatible with itself. +/// assert!(is_compatible(CONTRACT_VERSION)); +/// +/// // A minor difference in either direction is fine — capability negotiation +/// // covers the delta. +/// assert!(is_compatible((CONTRACT_VERSION.0, CONTRACT_VERSION.1 + 7))); +/// +/// // A major mismatch refuses the bind. +/// assert!(!is_compatible((CONTRACT_VERSION.0 + 1, 0))); +/// ``` +pub fn is_compatible(remote: (u16, u16)) -> bool { + remote.0 == CONTRACT_VERSION.0 +} + +#[cfg(test)] +#[path = "version_tests.rs"] +mod tests; diff --git a/api/src/version_tests.rs b/api/src/version_tests.rs new file mode 100644 index 0000000..b9819fb --- /dev/null +++ b/api/src/version_tests.rs @@ -0,0 +1,63 @@ +//! Unit tests for the contract version rule in [`super`]. +//! +//! The rule these pin is the one from the kernel design: a **minor** bump means +//! a capability was added and stays compatible; a **major** mismatch refuses +//! the bind. + +use super::*; + +#[test] +fn contract_version_starts_at_one_zero() { + assert_eq!(CONTRACT_VERSION, (1, 0)); +} + +#[test] +fn own_version_is_compatible_with_itself() { + assert!(is_compatible(CONTRACT_VERSION)); +} + +#[test] +fn a_minor_bump_stays_compatible_in_both_directions() { + let (major, minor) = CONTRACT_VERSION; + + // Remote ahead: it advertises families this build does not know. Unknown + // family strings are skipped during handshake parsing. + assert!(is_compatible((major, minor + 1))); + assert!(is_compatible((major, minor + 25))); + assert!(is_compatible((major, u16::MAX))); + + // Remote behind: it lacks families this build knows. Those simply are not + // advertised, so the surface degrades — the ordinary path, not an error. + assert!(is_compatible((major, minor.saturating_sub(1)))); + assert!(is_compatible((major, 0))); +} + +#[test] +fn a_major_mismatch_refuses_the_bind() { + let (major, minor) = CONTRACT_VERSION; + + // Remote ahead by a major: an existing signature changed under us. + assert!(!is_compatible((major + 1, 0))); + assert!(!is_compatible((major + 1, minor))); + assert!(!is_compatible((major + 1, u16::MAX))); + + // Remote behind by a major: same reasoning, other direction. A newer minor + // does not rescue an older major. + assert!(!is_compatible((major - 1, u16::MAX))); + assert!(!is_compatible((0, 0))); +} + +#[test] +fn compatibility_depends_only_on_the_major_half() { + let (major, _) = CONTRACT_VERSION; + for minor in [0u16, 1, 2, 7, 999, u16::MAX] { + assert!( + is_compatible((major, minor)), + "minor {minor} should not affect compatibility" + ); + assert!( + !is_compatible((major + 1, minor)), + "minor {minor} must not rescue a major mismatch" + ); + } +} From f2653d8378f8bfee91afbde634588422be936d88 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 21:50:01 +0300 Subject: [PATCH 10/19] feat(api): add MemoryHealth, the api-local driver liveness state The contract crate cannot name the kernel's generic DriverHealth: a third-party driver must be able to depend on tinycortex-api without pulling in the OpenHuman host, and the next subsystem cut over must not inherit generic kernel vocabulary from a memory crate. So the contract carries its own MemoryHealth and the host's memory adapter converts. The conversion is trivial and lossless by construction: a small closed enum with a reason string, shaped one-for-one against the kernel's Ready | Degraded { reason } | Down { reason }, not a free-form struct that would need field-by-field mapping and would drift. Driver class stays out of the api crate deliberately - embedded/external/null is a host configuration fact about how a driver was bound, not something a driver reports about itself. --- api/src/health.rs | 119 ++++++++++++++++++++++++++++++++++++++++ api/src/health_tests.rs | 90 ++++++++++++++++++++++++++++++ api/src/lib.rs | 7 ++- 3 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 api/src/health.rs create mode 100644 api/src/health_tests.rs diff --git a/api/src/health.rs b/api/src/health.rs new file mode 100644 index 0000000..248238a --- /dev/null +++ b/api/src/health.rs @@ -0,0 +1,119 @@ +//! Liveness state a memory driver reports about itself. +//! +//! ## Why this lives in the contract crate and not in the host +//! +//! The OpenHuman kernel has (or will have) a *generic* subsystem-agnostic +//! `DriverHealth` shared by memory, inference, channels, and sandbox. This crate +//! cannot name that type: `tinycortex-api` is the contract a third-party driver +//! compiles against, and a driver must be able to depend on it without pulling +//! in the OpenHuman host — nor should the next subsystem cut over inherit +//! generic kernel vocabulary from a *memory* crate. +//! +//! So the contract carries its own [`MemoryHealth`], and the host's memory +//! adapter converts. The conversion is deliberately trivial and lossless: this +//! is a **small closed enum with a reason string**, shaped one-for-one against +//! the kernel's `Ready | Degraded { reason } | Down { reason }`, not a +//! free-form struct that would need field-by-field mapping and would drift. +//! Keep it that way — if a driver needs to report something richer, it belongs +//! in a driver-specific status payload, not here. +//! +//! ## Wire form +//! +//! Serializes as an internally-tagged object with a stable snake_case `status` +//! discriminant, which is also the shape of the transport adapter's +//! `GET /v1/health` → `{ status, detail }` response: +//! +//! ```json +//! { "status": "ready" } +//! { "status": "degraded", "reason": "vector index rebuilding" } +//! { "status": "down", "reason": "connection refused" } +//! ``` + +use serde::{Deserialize, Serialize}; + +/// Health of a bound memory driver, as the driver reports it. +/// +/// The three states are ordered by severity and mean different things to the +/// kernel: +/// +/// - [`MemoryHealth::Ready`] — serve traffic normally. +/// - [`MemoryHealth::Degraded`] — still serve traffic, but surface the reason +/// in status output; results may be incomplete or slow. +/// - [`MemoryHealth::Down`] — do not serve traffic; the bind should be surfaced +/// as failed and, per the fallback rule, the embedded default rebound. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "status", rename_all = "snake_case")] +pub enum MemoryHealth { + /// The driver is reachable and serving requests normally. + Ready, + /// The driver is serving requests, but something is wrong and the caller + /// should surface it. Results may be incomplete, stale, or slow. + Degraded { + /// Operator-facing explanation. Must not contain credentials, tokens, + /// or user memory content — this string is logged and shown in status + /// output. + reason: String, + }, + /// The driver cannot serve requests at all. + Down { + /// Operator-facing explanation, subject to the same redaction rule as + /// [`MemoryHealth::Degraded::reason`]. + reason: String, + }, +} + +impl MemoryHealth { + /// Convenience constructor for [`MemoryHealth::Degraded`]. + pub fn degraded(reason: impl Into) -> Self { + Self::Degraded { + reason: reason.into(), + } + } + + /// Convenience constructor for [`MemoryHealth::Down`]. + pub fn down(reason: impl Into) -> Self { + Self::Down { + reason: reason.into(), + } + } + + /// Stable snake_case discriminant, matching the serialized `status` field. + pub fn as_str(&self) -> &'static str { + match self { + Self::Ready => "ready", + Self::Degraded { .. } => "degraded", + Self::Down { .. } => "down", + } + } + + /// The operator-facing reason, when there is one. `None` for + /// [`MemoryHealth::Ready`]. + pub fn reason(&self) -> Option<&str> { + match self { + Self::Ready => None, + Self::Degraded { reason } | Self::Down { reason } => Some(reason.as_str()), + } + } + + /// Whether the kernel should route traffic to this driver. + /// + /// True for [`MemoryHealth::Ready`] and [`MemoryHealth::Degraded`] — a + /// degraded driver is still the bound driver — and false for + /// [`MemoryHealth::Down`]. + pub fn is_usable(&self) -> bool { + !matches!(self, Self::Down { .. }) + } +} + +impl std::fmt::Display for MemoryHealth { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self.reason() { + Some(reason) => write!(f, "{}: {reason}", self.as_str()), + None => f.write_str(self.as_str()), + } + } +} + +#[cfg(test)] +#[path = "health_tests.rs"] +mod tests; diff --git a/api/src/health_tests.rs b/api/src/health_tests.rs new file mode 100644 index 0000000..a31c965 --- /dev/null +++ b/api/src/health_tests.rs @@ -0,0 +1,90 @@ +//! Unit tests for [`super::MemoryHealth`]. +//! +//! These pin the two properties the host's memory adapter depends on: the +//! variant set is closed and small enough for a lossless `match` into the +//! kernel's generic `DriverHealth`, and the wire form carries a stable +//! `status` discriminant plus a `reason`. + +use super::*; +use serde_json::json; + +#[test] +fn ready_has_no_reason_and_is_usable() { + let health = MemoryHealth::Ready; + assert_eq!(health.as_str(), "ready"); + assert_eq!(health.reason(), None); + assert!(health.is_usable()); + assert_eq!(health.to_string(), "ready"); +} + +#[test] +fn degraded_carries_a_reason_and_is_still_usable() { + let health = MemoryHealth::degraded("vector index rebuilding"); + assert_eq!(health.as_str(), "degraded"); + assert_eq!(health.reason(), Some("vector index rebuilding")); + // A degraded driver is still the bound driver. + assert!(health.is_usable()); + assert_eq!(health.to_string(), "degraded: vector index rebuilding"); +} + +#[test] +fn down_carries_a_reason_and_is_not_usable() { + let health = MemoryHealth::down("connection refused"); + assert_eq!(health.as_str(), "down"); + assert_eq!(health.reason(), Some("connection refused")); + assert!(!health.is_usable()); + assert_eq!(health.to_string(), "down: connection refused"); +} + +#[test] +fn health_serializes_with_a_stable_status_discriminant() { + assert_eq!( + serde_json::to_value(MemoryHealth::Ready).unwrap(), + json!({ "status": "ready" }) + ); + assert_eq!( + serde_json::to_value(MemoryHealth::degraded("slow")).unwrap(), + json!({ "status": "degraded", "reason": "slow" }) + ); + assert_eq!( + serde_json::to_value(MemoryHealth::down("gone")).unwrap(), + json!({ "status": "down", "reason": "gone" }) + ); +} + +#[test] +fn health_round_trips_through_serde() { + for health in [ + MemoryHealth::Ready, + MemoryHealth::degraded("reindexing"), + MemoryHealth::down("auth expired"), + ] { + let encoded = serde_json::to_string(&health).unwrap(); + let decoded: MemoryHealth = serde_json::from_str(&encoded).unwrap(); + assert_eq!(decoded, health); + } +} + +#[test] +fn health_constructors_match_their_variants() { + assert_eq!( + MemoryHealth::degraded("x"), + MemoryHealth::Degraded { + reason: "x".to_string() + } + ); + assert_eq!( + MemoryHealth::down("y"), + MemoryHealth::Down { + reason: "y".to_string() + } + ); +} + +#[test] +fn degraded_without_a_reason_is_rejected_on_the_wire() { + // `reason` is mandatory: a degraded/down driver that explains nothing is + // useless in status output, so the contract refuses to decode it. + assert!(serde_json::from_value::(json!({ "status": "degraded" })).is_err()); + assert!(serde_json::from_value::(json!({ "status": "down" })).is_err()); +} diff --git a/api/src/lib.rs b/api/src/lib.rs index d9b4a8e..d40d5ad 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -13,8 +13,9 @@ //! depend on this crate alone, and the *generic* subsystem/driver vocabulary of //! the OpenHuman kernel (`Driver`, `DriverClass`, `SubsystemRegistry`, the //! policy `Guard`) must not be inherited from a *memory* crate by whichever -//! subsystem is cut over next. So the contract carries its own capability -//! vocabulary, and the host's memory adapter converts at the boundary. +//! subsystem is cut over next. So the contract carries its own identity, +//! capability, and health vocabulary, and the host's memory adapter converts at +//! the boundary — see [`health`] for the shape that conversion relies on. //! //! Driver *class* (embedded / external / null) is deliberately **absent**: that //! is a host configuration fact about how a driver was bound, not something a @@ -36,6 +37,7 @@ //! [`types`]). //! - [`capabilities`]: the thirteen [`capabilities::Capability`] families and //! the [`capabilities::Capabilities`] set negotiated at bind time. +//! - [`health`]: [`health::MemoryHealth`], the liveness state a driver reports. //! - [`version`]: [`CONTRACT_VERSION`] and the [`is_compatible`] bind rule. //! - [`error`]: the typed [`error::MemoryError`] enum and its result alias. //! - [`traits`]: the [`traits::Memory`] storage-backend trait. @@ -50,6 +52,7 @@ pub mod capabilities; pub mod chunks; pub mod error; pub mod goals; +pub mod health; pub mod recall; pub mod tool_memory; pub mod traits; From 8d7241c00679bdf571aedb5a0bd288cc89f226c4 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 22:08:36 +0300 Subject: [PATCH 11/19] feat(api): define the MemoryProvider driver contract, its thirteen capability families, and the null reference driver --- api/src/lib.rs | 6 + api/src/null.rs | 434 ++++++++++++++++++++++++++++++++ api/src/null_tests.rs | 230 +++++++++++++++++ api/src/provider/audit.rs | 132 ++++++++++ api/src/provider/audit_tests.rs | 195 ++++++++++++++ api/src/provider/content.rs | 167 ++++++++++++ api/src/provider/driver.rs | 197 +++++++++++++++ api/src/provider/knowledge.rs | 176 +++++++++++++ api/src/provider/mandatory.rs | 184 ++++++++++++++ api/src/provider/mod.rs | 73 ++++++ api/src/provider/records.rs | 167 ++++++++++++ api/src/provider/types.rs | 393 +++++++++++++++++++++++++++++ api/src/provider/types_tests.rs | 133 ++++++++++ 13 files changed, 2487 insertions(+) create mode 100644 api/src/null.rs create mode 100644 api/src/null_tests.rs create mode 100644 api/src/provider/audit.rs create mode 100644 api/src/provider/audit_tests.rs create mode 100644 api/src/provider/content.rs create mode 100644 api/src/provider/driver.rs create mode 100644 api/src/provider/knowledge.rs create mode 100644 api/src/provider/mandatory.rs create mode 100644 api/src/provider/mod.rs create mode 100644 api/src/provider/records.rs create mode 100644 api/src/provider/types.rs create mode 100644 api/src/provider/types_tests.rs diff --git a/api/src/lib.rs b/api/src/lib.rs index d40d5ad..a0c3734 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -37,6 +37,10 @@ //! [`types`]). //! - [`capabilities`]: the thirteen [`capabilities::Capability`] families and //! the [`capabilities::Capabilities`] set negotiated at bind time. +//! - [`provider`]: the driver contract — [`provider::MemoryProvider`] plus the +//! thirteen capability family traits and the value types they need. +//! - [`null`]: [`null::NullMemoryProvider`], the reference driver a +//! compiled-out or unconfigured memory subsystem binds to. //! - [`health`]: [`health::MemoryHealth`], the liveness state a driver reports. //! - [`version`]: [`CONTRACT_VERSION`] and the [`is_compatible`] bind rule. //! - [`error`]: the typed [`error::MemoryError`] enum and its result alias. @@ -53,6 +57,8 @@ pub mod chunks; pub mod error; pub mod goals; pub mod health; +pub mod null; +pub mod provider; pub mod recall; pub mod tool_memory; pub mod traits; diff --git a/api/src/null.rs b/api/src/null.rs new file mode 100644 index 0000000..abd9b63 --- /dev/null +++ b/api/src/null.rs @@ -0,0 +1,434 @@ +//! [`NullMemoryProvider`] — the reference driver that stores nothing. +//! +//! ## What it is for +//! +//! A memory subsystem that is compiled out, disabled by configuration, or +//! explicitly bound to `driver = "null"` still has to bind *something*: the +//! kernel's registry holds exactly one driver per slot, and code that reaches +//! the slot must find a value rather than an `Option` it has to unwrap at every +//! call site. This is that value. It replaces the hand-written per-domain +//! `stub.rs` files with one generic answer. +//! +//! It is also the fixture the capability-degradation tests bind: with it in the +//! slot, the ten optional families are unadvertised, so their RPC methods are +//! unregistered and their agent tools are absent — and the core still boots. +//! +//! And it is the existence proof for the mandatory set: if +//! [`crate::provider::MemoryCore`], [`crate::provider::MemoryRecall`], and +//! [`crate::provider::MemoryPortability`] could not be implemented without a +//! storage engine, they would be the wrong three to have made mandatory. +//! +//! ## `/dev/null` semantics, and what that costs +//! +//! Writes are **accepted and discarded**; reads return empty. This mirrors the +//! Unix device the driver is named after, and it is the only behaviour that +//! lets the mandatory three be advertised honestly: a `store` that returned +//! [`crate::error::MemoryError::Unsupported`] would contradict advertising +//! [`crate::capabilities::Capability::Core`], and one that returned a hard +//! error would turn every optional auto-capture into a user-visible failure. +//! +//! The cost is real: content written here is gone. That is acceptable for a +//! subsystem the operator turned off, and unacceptable as a fallback for a +//! driver that failed to bind — **that** case falls back to the embedded +//! default, never to this. Do not wire it as a general-purpose failure mode. +//! +//! ## Why it implements all thirteen families but advertises three +//! +//! The ten optional families are implemented and every method returns +//! [`crate::error::MemoryError::Unsupported`] naming its family, but the +//! `as_*` accessors return `None` and +//! [`crate::provider::MemoryProvider::capabilities`] lists only the mandatory +//! three. So: +//! +//! - through `&dyn MemoryProvider` — the only way product code sees a driver — +//! an unadvertised family is simply **unreachable**, which is the intended +//! degradation; +//! - through the concrete type, a direct call yields a typed, *named* +//! `Unsupported` error, which is what makes the contract's error mapping +//! testable without writing a second mock. +//! +//! [`crate::provider::audit_provider`] confirms the two views agree. + +use async_trait::async_trait; + +use crate::capabilities::{Capabilities, Capability}; +use crate::error::MemoryError; +use crate::goals::GoalsDoc; +use crate::health::MemoryHealth; +use crate::provider::types::{ + DiffReport, EntityHit, ExportPage, ExportRecord, ImportOutcome, IngestItem, IngestOutcome, + MaintenanceReport, SnapshotRef, SourceItem, SourceScope, +}; +use crate::provider::{ + MemoryCore, MemoryDiff, MemoryDocuments, MemoryEntities, MemoryGoals, MemoryGraph, + MemoryIngest, MemoryMaintenance, MemoryPortability, MemoryProvider, MemoryRecall, + MemorySourceSink, MemoryToolMemory, MemoryTree, +}; +use crate::recall::OwnedRecallOpts; +use crate::tool_memory::ToolMemoryRule; +use crate::tree::{IngestRequest, QueryResult, TreeStatus}; +use crate::types::{ + GraphRelationRecord, MemoryCategory, MemoryEntry, MemoryKvRecord, MemoryTaint, + NamespaceDocumentInput, NamespaceRetrievalContext, NamespaceSummary, StoredMemoryDocument, +}; + +/// The [`driver_id`](MemoryProvider::driver_id) this driver reports. +pub const NULL_DRIVER_ID: &str = "null"; + +/// Shorthand for the `Unsupported` error every unadvertised family returns. +fn unsupported(capability: Capability) -> Result { + Err(MemoryError::unsupported(capability)) +} + +/// A driver that accepts every write, discards it, and returns nothing. +/// +/// See the module documentation for what it is for, why writes are silently +/// dropped, and why it implements ten families it does not advertise. +#[derive(Debug, Clone, Copy, Default)] +pub struct NullMemoryProvider; + +impl NullMemoryProvider { + /// Construct the null driver. It holds no state, so every instance is + /// interchangeable. + pub const fn new() -> Self { + Self + } +} + +#[async_trait] +impl MemoryProvider for NullMemoryProvider { + fn driver_id(&self) -> &str { + NULL_DRIVER_ID + } + + /// Exactly the mandatory three. The ten optional families are implemented + /// below but deliberately not advertised, so they stay unreachable through + /// the trait object. + fn capabilities(&self) -> Capabilities { + Capabilities::mandatory() + } + + /// Always [`MemoryHealth::Ready`]: a driver with no backing store has + /// nothing that can be unreachable, and reporting `Degraded` would make + /// every status view of a deliberately-disabled subsystem look broken. + async fn health(&self) -> MemoryHealth { + MemoryHealth::Ready + } + + // The `as_*` accessors are all left at their `None` defaults: nothing + // optional is reachable through the trait object. That absence is the whole + // point of this driver, so overriding any of them would be the bug. +} + +#[async_trait] +impl MemoryCore for NullMemoryProvider { + /// Accepts and discards. See the module docs on `/dev/null` semantics. + async fn store( + &self, + _namespace: &str, + _key: &str, + _content: &str, + _category: MemoryCategory, + _session_id: Option<&str>, + _taint: MemoryTaint, + ) -> Result<(), MemoryError> { + Ok(()) + } + + async fn get(&self, _namespace: &str, _key: &str) -> Result, MemoryError> { + Ok(None) + } + + /// Always `Ok(false)`: nothing was ever stored, so nothing existed to + /// forget. Consistent with the idempotence the family requires. + async fn forget(&self, _namespace: &str, _key: &str) -> Result { + Ok(false) + } + + async fn list( + &self, + _namespace: Option<&str>, + _category: Option<&MemoryCategory>, + _session_id: Option<&str>, + ) -> Result, MemoryError> { + Ok(Vec::new()) + } + + async fn namespaces(&self) -> Result, MemoryError> { + Ok(Vec::new()) + } +} + +#[async_trait] +impl MemoryRecall for NullMemoryProvider { + async fn recall( + &self, + _query: &str, + _limit: usize, + _opts: &OwnedRecallOpts, + _scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + Ok(Vec::new()) + } +} + +#[async_trait] +impl MemoryPortability for NullMemoryProvider { + /// One empty, terminal page: no records and no continuation cursor, so a + /// caller's export loop terminates on the first iteration. + async fn export_page( + &self, + _cursor: Option<&str>, + _limit: usize, + ) -> Result { + Ok(ExportPage::default()) + } + + /// Counts every record as skipped rather than imported. Reporting them as + /// imported would tell a migration its data landed somewhere it did not. + async fn import_records( + &self, + records: Vec, + ) -> Result { + Ok(ImportOutcome { + imported: 0, + skipped: u32::try_from(records.len()).unwrap_or(u32::MAX), + failed: 0, + errors: Vec::new(), + }) + } +} + +#[async_trait] +impl MemoryIngest for NullMemoryProvider { + async fn ingest_document(&self, _item: IngestItem) -> Result { + unsupported(Capability::Ingest) + } + + async fn ingest_chat(&self, _messages: Vec) -> Result { + unsupported(Capability::Ingest) + } +} + +#[async_trait] +impl MemoryDocuments for NullMemoryProvider { + async fn put_document(&self, _input: NamespaceDocumentInput) -> Result { + unsupported(Capability::Documents) + } + + async fn get_document( + &self, + _namespace: &str, + _key: &str, + ) -> Result, MemoryError> { + unsupported(Capability::Documents) + } + + async fn query_documents( + &self, + _namespace: &str, + _query: &str, + _limit: usize, + ) -> Result { + unsupported(Capability::Documents) + } +} + +#[async_trait] +impl MemoryTree for NullMemoryProvider { + async fn append(&self, _request: IngestRequest) -> Result<(), MemoryError> { + unsupported(Capability::Tree) + } + + async fn query_source( + &self, + _namespace: &str, + _source_id: &str, + _limit: usize, + _scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + unsupported(Capability::Tree) + } + + async fn drill_down( + &self, + _namespace: &str, + _node_id: &str, + ) -> Result { + unsupported(Capability::Tree) + } + + async fn seal(&self, _namespace: &str) -> Result { + unsupported(Capability::Tree) + } + + async fn cascade(&self, _namespace: &str) -> Result { + unsupported(Capability::Tree) + } +} + +#[async_trait] +impl MemoryEntities for NullMemoryProvider { + async fn entities( + &self, + _namespace: &str, + _query: Option<&str>, + _limit: usize, + ) -> Result, MemoryError> { + unsupported(Capability::Entities) + } + + async fn entity_edges( + &self, + _namespace: &str, + _entity_id: &str, + _limit: usize, + ) -> Result, MemoryError> { + unsupported(Capability::Entities) + } + + async fn touch_entities( + &self, + _namespace: &str, + _entity_ids: &[String], + ) -> Result<(), MemoryError> { + unsupported(Capability::Entities) + } +} + +#[async_trait] +impl MemoryGraph for NullMemoryProvider { + async fn kv_get( + &self, + _namespace: Option<&str>, + _key: &str, + ) -> Result, MemoryError> { + unsupported(Capability::Graph) + } + + async fn kv_put( + &self, + _namespace: Option<&str>, + _key: &str, + _value: serde_json::Value, + ) -> Result<(), MemoryError> { + unsupported(Capability::Graph) + } + + async fn kv_list( + &self, + _namespace: Option<&str>, + _prefix: Option<&str>, + _limit: usize, + ) -> Result, MemoryError> { + unsupported(Capability::Graph) + } + + async fn relations( + &self, + _namespace: Option<&str>, + _subject: Option<&str>, + _predicate: Option<&str>, + _limit: usize, + ) -> Result, MemoryError> { + unsupported(Capability::Graph) + } + + async fn put_relation(&self, _relation: GraphRelationRecord) -> Result<(), MemoryError> { + unsupported(Capability::Graph) + } +} + +#[async_trait] +impl MemoryDiff for NullMemoryProvider { + async fn capture_snapshot(&self, _source_id: &str) -> Result { + unsupported(Capability::Diff) + } + + async fn snapshots( + &self, + _source_id: &str, + _limit: usize, + ) -> Result, MemoryError> { + unsupported(Capability::Diff) + } + + async fn diff( + &self, + _source_id: &str, + _from: Option<&str>, + _to: &str, + ) -> Result { + unsupported(Capability::Diff) + } +} + +#[async_trait] +impl MemoryGoals for NullMemoryProvider { + async fn goals(&self) -> Result { + unsupported(Capability::Goals) + } + + async fn set_goals(&self, _goals: GoalsDoc) -> Result<(), MemoryError> { + unsupported(Capability::Goals) + } +} + +#[async_trait] +impl MemoryToolMemory for NullMemoryProvider { + async fn tool_rules(&self, _tool_name: &str) -> Result, MemoryError> { + unsupported(Capability::ToolMemory) + } + + async fn put_tool_rule(&self, _rule: ToolMemoryRule) -> Result<(), MemoryError> { + unsupported(Capability::ToolMemory) + } + + async fn delete_tool_rule( + &self, + _tool_name: &str, + _rule_id: &str, + ) -> Result { + unsupported(Capability::ToolMemory) + } +} + +#[async_trait] +impl MemorySourceSink for NullMemoryProvider { + async fn accept_source_items( + &self, + _source_id: &str, + _source_kind: &str, + _items: Vec, + _taint: MemoryTaint, + ) -> Result { + unsupported(Capability::Sources) + } + + async fn forget_source(&self, _source_id: &str) -> Result { + unsupported(Capability::Sources) + } +} + +#[async_trait] +impl MemoryMaintenance for NullMemoryProvider { + async fn reembed(&self) -> Result { + unsupported(Capability::Maintenance) + } + + async fn compact(&self) -> Result { + unsupported(Capability::Maintenance) + } + + async fn consolidate(&self) -> Result { + unsupported(Capability::Maintenance) + } + + async fn doctor(&self) -> Result { + unsupported(Capability::Maintenance) + } +} + +#[cfg(test)] +#[path = "null_tests.rs"] +mod tests; diff --git a/api/src/null_tests.rs b/api/src/null_tests.rs new file mode 100644 index 0000000..33f3f76 --- /dev/null +++ b/api/src/null_tests.rs @@ -0,0 +1,230 @@ +//! Tests for the reference null driver. +//! +//! These pin three separate contracts: +//! +//! 1. the mandatory-three set is genuinely implementable without a store; +//! 2. an unadvertised family is **unreachable** through the trait object, which +//! is the degradation behaviour the kernel relies on; +//! 3. a direct call to an unadvertised family yields a typed `Unsupported` +//! error that **names** the family, which is what the transport adapter's +//! `501` mapping is checked against. +//! +//! ## No async runtime here, on purpose +//! +//! `tinycortex-api` must not depend on tokio (or any executor) — that is the +//! whole point of the crate. Every future in this module completes on its first +//! poll, so a six-line std-only [`block_on`] is sufficient and adds no +//! dependency. + +use std::future::Future; +use std::pin::pin; +use std::task::{Context, Poll}; + +use super::*; +use crate::provider::audit_provider; +use crate::types::MemoryCategory; + +/// Drive a future that is ready on first poll to completion, without an +/// executor. Panics rather than spinning if a future ever returns `Pending`, +/// because in this module that would mean a supposedly-inert implementation +/// started doing real work. +fn block_on(future: F) -> F::Output { + let mut future = pin!(future); + let mut context = Context::from_waker(std::task::Waker::noop()); + match future.as_mut().poll(&mut context) { + Poll::Ready(value) => value, + Poll::Pending => panic!("null driver future must complete on first poll"), + } +} + +#[test] +fn null_driver_advertises_exactly_the_mandatory_families() { + let driver = NullMemoryProvider::new(); + let capabilities = driver.capabilities(); + + assert_eq!(driver.driver_id(), NULL_DRIVER_ID); + assert_eq!(capabilities.len(), 3); + for capability in Capability::MANDATORY { + assert!( + capabilities.contains(capability), + "{capability} must be advertised" + ); + } +} + +#[test] +fn null_driver_passes_capability_validation() { + // The mandatory-three set is the minimum bindable set, so the reference + // driver must be bindable. If this ever fails, either the mandatory list + // grew or the null driver stopped implementing it. + let driver = NullMemoryProvider::new(); + assert_eq!(driver.capabilities().validate(), Ok(())); +} + +#[test] +fn null_driver_is_self_consistent() { + assert_eq!(audit_provider(&NullMemoryProvider::new()), Ok(())); +} + +#[test] +fn null_driver_reports_ready() { + let health = block_on(NullMemoryProvider::new().health()); + assert_eq!(health, MemoryHealth::Ready); + assert!(health.is_usable()); +} + +#[test] +fn null_driver_shutdown_is_an_idempotent_no_op() { + let driver = NullMemoryProvider::new(); + assert!(block_on(driver.shutdown()).is_ok()); + assert!(block_on(driver.shutdown()).is_ok()); +} + +#[test] +fn mandatory_core_accepts_writes_and_reads_back_empty() { + let driver = NullMemoryProvider::new(); + + block_on(driver.store( + "global", + "k", + "v", + MemoryCategory::Core, + None, + MemoryTaint::ExternalSync, + )) + .expect("null store must accept the write"); + + assert!(block_on(driver.get("global", "k")) + .expect("get must succeed") + .is_none()); + assert!(!block_on(driver.forget("global", "k")).expect("forget must succeed")); + assert!(block_on(driver.list(None, None, None)) + .expect("list must succeed") + .is_empty()); + assert!(block_on(driver.namespaces()) + .expect("namespaces must succeed") + .is_empty()); +} + +#[test] +fn mandatory_recall_returns_no_hits() { + let driver = NullMemoryProvider::new(); + let hits = block_on(driver.recall("anything", 10, &OwnedRecallOpts::default(), None)) + .expect("recall must succeed"); + assert!(hits.is_empty()); +} + +#[test] +fn mandatory_portability_round_trips_as_an_empty_store() { + let driver = NullMemoryProvider::new(); + + let page = block_on(driver.export_page(None, 100)).expect("export must succeed"); + assert!(page.records.is_empty()); + assert!( + page.next_cursor.is_none(), + "the absent cursor is what terminates the caller's export loop" + ); + + let outcome = block_on(driver.import_records(vec![ExportRecord { + kind: "entry".to_string(), + id: "rec-1".to_string(), + namespace: None, + taint: MemoryTaint::Internal, + payload: serde_json::Value::Null, + }])) + .expect("import must succeed"); + + // Skipped, never imported: reporting an import would tell a migration its + // data landed somewhere it did not. + assert_eq!(outcome.imported, 0); + assert_eq!(outcome.skipped, 1); + assert_eq!(outcome.failed, 0); +} + +#[test] +fn every_unadvertised_family_is_unreachable_through_the_trait_object() { + let driver = NullMemoryProvider::new(); + let provider: &dyn MemoryProvider = &driver; + + assert!(provider.as_ingest().is_none()); + assert!(provider.as_documents().is_none()); + assert!(provider.as_tree().is_none()); + assert!(provider.as_entities().is_none()); + assert!(provider.as_graph().is_none()); + assert!(provider.as_diff().is_none()); + assert!(provider.as_goals().is_none()); + assert!(provider.as_tool_memory().is_none()); + assert!(provider.as_sources().is_none()); + assert!(provider.as_maintenance().is_none()); +} + +#[test] +fn advertised_and_reachable_agree_for_every_family() { + // The invariant that keeps the capability set honest, checked family by + // family rather than only through the aggregate audit. + let driver = NullMemoryProvider::new(); + let provider: &dyn MemoryProvider = &driver; + let advertised = provider.capabilities(); + + for capability in Capability::ALL { + assert_eq!( + advertised.contains(capability), + provider.provides(capability), + "{capability}: advertised and reachable must agree" + ); + } +} + +/// Assert a result is `Unsupported` and names the expected family. +fn assert_unsupported(result: Result, expected: Capability) { + match result { + Err(MemoryError::Unsupported { capability }) => { + assert_eq!(capability, expected.as_str()); + } + other => panic!("expected Unsupported({expected}), got {other:?}"), + } +} + +#[test] +fn unadvertised_families_return_unsupported_naming_their_capability() { + let driver = NullMemoryProvider::new(); + + assert_unsupported(block_on(driver.ingest_chat(Vec::new())), Capability::Ingest); + assert_unsupported( + block_on(driver.get_document("global", "k")), + Capability::Documents, + ); + assert_unsupported(block_on(driver.seal("global")), Capability::Tree); + assert_unsupported( + block_on(driver.entities("global", None, 10)), + Capability::Entities, + ); + assert_unsupported(block_on(driver.kv_get(None, "k")), Capability::Graph); + assert_unsupported( + block_on(driver.capture_snapshot("src-abc")), + Capability::Diff, + ); + assert_unsupported(block_on(driver.goals()), Capability::Goals); + assert_unsupported(block_on(driver.tool_rules("shell")), Capability::ToolMemory); + assert_unsupported( + block_on(driver.forget_source("src-abc")), + Capability::Sources, + ); + assert_unsupported(block_on(driver.doctor()), Capability::Maintenance); +} + +#[test] +fn provider_is_usable_as_a_shared_trait_object() { + // The registry binds `Arc`, so the trait object must be + // `Send + Sync` and every family trait must be object-safe. This test fails + // to *compile* rather than to run if that ever regresses. + fn assert_send_sync(_value: &T) {} + + let provider: std::sync::Arc = + std::sync::Arc::new(NullMemoryProvider::new()); + assert_send_sync(&provider); + assert_eq!(provider.driver_id(), NULL_DRIVER_ID); + assert!(block_on(provider.list(None, None, None)) + .expect("list through the trait object") + .is_empty()); +} diff --git a/api/src/provider/audit.rs b/api/src/provider/audit.rs new file mode 100644 index 0000000..b215c4e --- /dev/null +++ b/api/src/provider/audit.rs @@ -0,0 +1,132 @@ +//! The honesty check: does a driver's advertised capability set match the +//! surface it actually exposes? +//! +//! [`MemoryProvider::capabilities`] is a *claim*, and the kernel acts on it — +//! it registers RPC methods and assembles agent tools from the advertised set +//! and never re-checks. A driver that advertises a family it does not implement +//! therefore produces a surface that exists in `/schema`, appears in the agent's +//! tool list, and fails on first use. That is precisely the +//! "registered-but-failing" outcome the degradation design exists to avoid. +//! +//! [`audit_provider`] compares the claim against +//! [`MemoryProvider::provides`] — which is derived from the accessors, so it +//! cannot drift from reality — and reports both directions of mismatch. Run it +//! at bind time next to [`crate::capabilities::Capabilities::validate`], and in +//! every driver's own test suite. +//! +//! The two directions mean different things: +//! +//! - **Advertised but absent** is a bug that will surface as a failing call. It +//! should refuse the bind. +//! - **Present but unadvertised** is dead surface: the family works but the +//! kernel unregistered it, so nothing can reach it. Usually a forgotten +//! entry in the driver's `capabilities()` list. + +use std::fmt; + +use crate::capabilities::Capability; +use crate::error::MemoryError; +use crate::provider::driver::MemoryProvider; + +/// A disagreement between what a driver advertises and what it implements. +/// +/// Carries the families structurally rather than as a formatted string so a +/// caller can report them in a status payload or a bind-failure event as well +/// as in a log line. At least one of the two vectors is non-empty. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CapabilityAudit { + /// Families the driver advertises but does not expose. These will fail on + /// first call; refuse the bind. + pub advertised_but_absent: Vec, + /// Families the driver exposes but does not advertise. These are + /// unreachable, because the kernel filters from the advertised set. + pub present_but_unadvertised: Vec, +} + +impl fmt::Display for CapabilityAudit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut parts = Vec::new(); + if !self.advertised_but_absent.is_empty() { + parts.push(format!( + "advertised but not implemented: {}", + join(&self.advertised_but_absent) + )); + } + if !self.present_but_unadvertised.is_empty() { + parts.push(format!( + "implemented but not advertised: {}", + join(&self.present_but_unadvertised) + )); + } + write!(f, "memory driver capability mismatch; {}", parts.join("; ")) + } +} + +impl std::error::Error for CapabilityAudit {} + +impl From for MemoryError { + /// A mismatch is the driver saying something untrue about itself, which is + /// a configuration/implementation error rather than an unsupported call — + /// hence [`MemoryError::Invalid`] and not + /// [`MemoryError::Unsupported`]. Same reasoning as + /// [`crate::capabilities::MissingMandatoryCapabilities`]. + fn from(value: CapabilityAudit) -> Self { + MemoryError::Invalid(value.to_string()) + } +} + +fn join(families: &[Capability]) -> String { + families + .iter() + .map(|cap| cap.as_str()) + .collect::>() + .join(", ") +} + +/// Compare a driver's advertised capability set against its reachable surface. +/// +/// Walks every [`Capability`] in declaration order, so the returned vectors are +/// in that order too. +/// +/// # Errors +/// +/// Returns [`CapabilityAudit`] when the two disagree in either direction. A +/// driver that agrees with itself returns `Ok(())`. +/// +/// # Examples +/// +/// ``` +/// # use tinycortex_api::null::NullMemoryProvider; +/// # use tinycortex_api::provider::audit_provider; +/// // The reference null driver is self-consistent. +/// assert!(audit_provider(&NullMemoryProvider::new()).is_ok()); +/// ``` +pub fn audit_provider(provider: &dyn MemoryProvider) -> Result<(), CapabilityAudit> { + let advertised = provider.capabilities(); + let mut advertised_but_absent = Vec::new(); + let mut present_but_unadvertised = Vec::new(); + + for capability in Capability::ALL { + match ( + advertised.contains(capability), + provider.provides(capability), + ) { + (true, false) => advertised_but_absent.push(capability), + (false, true) => present_but_unadvertised.push(capability), + _ => {} + } + } + + if advertised_but_absent.is_empty() && present_but_unadvertised.is_empty() { + Ok(()) + } else { + Err(CapabilityAudit { + advertised_but_absent, + present_but_unadvertised, + }) + } +} + +#[cfg(test)] +#[path = "audit_tests.rs"] +mod tests; diff --git a/api/src/provider/audit_tests.rs b/api/src/provider/audit_tests.rs new file mode 100644 index 0000000..07ec80e --- /dev/null +++ b/api/src/provider/audit_tests.rs @@ -0,0 +1,195 @@ +//! Tests for the advertised-vs-implemented honesty check. +//! +//! Two deliberately dishonest fixtures sit here — one that over-claims and one +//! that under-claims — because the whole value of [`audit_provider`] is +//! catching drivers that disagree with themselves, and neither direction is +//! reachable from an honest driver. + +use async_trait::async_trait; + +use super::*; +use crate::capabilities::Capabilities; +use crate::health::MemoryHealth; +use crate::null::NullMemoryProvider; +use crate::provider::types::{ExportPage, ExportRecord, ImportOutcome, SourceScope}; +use crate::provider::{MemoryCore, MemoryPortability, MemoryRecall, MemoryTree}; +use crate::recall::OwnedRecallOpts; +use crate::types::{MemoryCategory, MemoryEntry, MemoryTaint, NamespaceSummary}; + +/// A provider that forwards the mandatory three to [`NullMemoryProvider`] so +/// each fixture below only has to describe the thing it is lying about. +struct Fixture { + inner: NullMemoryProvider, + advertised: Capabilities, + expose_tree: bool, +} + +impl Fixture { + fn new(advertised: Capabilities, expose_tree: bool) -> Self { + Self { + inner: NullMemoryProvider::new(), + advertised, + expose_tree, + } + } +} + +#[async_trait] +impl MemoryCore for Fixture { + async fn store( + &self, + namespace: &str, + key: &str, + content: &str, + category: MemoryCategory, + session_id: Option<&str>, + taint: MemoryTaint, + ) -> Result<(), MemoryError> { + self.inner + .store(namespace, key, content, category, session_id, taint) + .await + } + + async fn get(&self, namespace: &str, key: &str) -> Result, MemoryError> { + self.inner.get(namespace, key).await + } + + async fn forget(&self, namespace: &str, key: &str) -> Result { + self.inner.forget(namespace, key).await + } + + async fn list( + &self, + namespace: Option<&str>, + category: Option<&MemoryCategory>, + session_id: Option<&str>, + ) -> Result, MemoryError> { + self.inner.list(namespace, category, session_id).await + } + + async fn namespaces(&self) -> Result, MemoryError> { + self.inner.namespaces().await + } +} + +#[async_trait] +impl MemoryRecall for Fixture { + async fn recall( + &self, + query: &str, + limit: usize, + opts: &OwnedRecallOpts, + scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + self.inner.recall(query, limit, opts, scope).await + } +} + +#[async_trait] +impl MemoryPortability for Fixture { + async fn export_page( + &self, + cursor: Option<&str>, + limit: usize, + ) -> Result { + self.inner.export_page(cursor, limit).await + } + + async fn import_records( + &self, + records: Vec, + ) -> Result { + self.inner.import_records(records).await + } +} + +#[async_trait] +impl MemoryProvider for Fixture { + fn driver_id(&self) -> &str { + "fixture" + } + + fn capabilities(&self) -> Capabilities { + self.advertised + } + + async fn health(&self) -> MemoryHealth { + MemoryHealth::Ready + } + + fn as_tree(&self) -> Option<&dyn MemoryTree> { + if self.expose_tree { + Some(&self.inner) + } else { + None + } + } +} + +#[test] +fn honest_driver_passes_the_audit() { + let honest = Fixture::new(Capabilities::mandatory().with(Capability::Tree), true); + assert_eq!(audit_provider(&honest), Ok(())); +} + +#[test] +fn over_claiming_driver_is_reported_as_advertised_but_absent() { + // Advertises everything, exposes no optional accessor. Every one of the ten + // optional families would fail on first call — the exact + // registered-but-failing outcome the capability filter exists to prevent. + let liar = Fixture::new(Capabilities::all(), false); + + let audit = audit_provider(&liar).expect_err("over-claiming driver must fail the audit"); + assert_eq!(audit.present_but_unadvertised, Vec::new()); + assert_eq!(audit.advertised_but_absent.len(), 10); + assert!(audit.advertised_but_absent.contains(&Capability::Tree)); + // The mandatory three are supertraits, so they can never be missing. + assert!(!audit.advertised_but_absent.contains(&Capability::Core)); + assert!(!audit.advertised_but_absent.contains(&Capability::Recall)); + assert!(!audit + .advertised_but_absent + .contains(&Capability::Portability)); +} + +#[test] +fn under_claiming_driver_is_reported_as_present_but_unadvertised() { + // Implements the tree but forgot to list it: the family works and is + // completely unreachable, because the kernel filters from the advertised + // set. + let shy = Fixture::new(Capabilities::mandatory(), true); + + let audit = audit_provider(­).expect_err("under-claiming driver must fail the audit"); + assert_eq!(audit.advertised_but_absent, Vec::new()); + assert_eq!(audit.present_but_unadvertised, vec![Capability::Tree]); +} + +#[test] +fn audit_findings_are_reported_in_declaration_order() { + let liar = Fixture::new(Capabilities::all(), false); + let audit = audit_provider(&liar).expect_err("expected a mismatch"); + + let declaration_order: Vec = Capability::ALL + .into_iter() + .filter(|cap| audit.advertised_but_absent.contains(cap)) + .collect(); + assert_eq!(audit.advertised_but_absent, declaration_order); +} + +#[test] +fn audit_error_names_every_mismatched_family_and_maps_to_invalid() { + let liar = Fixture::new(Capabilities::all(), false); + let audit = audit_provider(&liar).expect_err("expected a mismatch"); + + let rendered = audit.to_string(); + for capability in &audit.advertised_but_absent { + assert!( + rendered.contains(capability.as_str()), + "audit message must name {capability}: {rendered}" + ); + } + + // A driver lying about itself is a config/implementation error, not an + // unsupported call. + let error: MemoryError = audit.into(); + assert!(matches!(error, MemoryError::Invalid(_))); +} diff --git a/api/src/provider/content.rs b/api/src/provider/content.rs new file mode 100644 index 0000000..9c16e86 --- /dev/null +++ b/api/src/provider/content.rs @@ -0,0 +1,167 @@ +//! Optional families that put content *into* memory and navigate it: +//! [`MemoryIngest`], [`MemoryDocuments`], and [`MemoryTree`]. +//! +//! All three are optional. A driver that advertises none of them is still a +//! memory backend — it just accepts entries only through +//! [`crate::provider::MemoryCore::store`] and has no document tier and no +//! summary tree. The kernel unregisters the matching RPC methods and omits the +//! matching agent tools rather than registering handlers that fail. +//! +//! ## No configuration crosses this boundary +//! +//! Chunk sizes, embedding models, summariser prompts, seal thresholds, and +//! cascade policy are all *driver* concerns. None of them appear in these +//! signatures: the embedded driver reads them from the `MemoryConfig` it +//! already holds, and an external driver has its own. This was the sharpest +//! test of whether the M0 crate carve-out drew the line in the right place — +//! the families that looked most config-dependent turned out not to need any. + +use async_trait::async_trait; + +use crate::chunks::Chunk; +use crate::error::MemoryError; +use crate::provider::types::{IngestItem, IngestOutcome, SourceScope}; +use crate::tree::{IngestRequest, QueryResult, TreeStatus}; +use crate::types::{NamespaceDocumentInput, NamespaceRetrievalContext, StoredMemoryDocument}; + +/// Bulk content ingestion — the driver owns chunking and embedding. +/// +/// The distinction from [`crate::provider::MemoryCore::store`] is ownership of +/// the pipeline: `store` persists exactly one entry the caller has already +/// shaped, whereas ingest hands over raw source material and lets the driver +/// decide how to split, embed, and index it. +#[async_trait] +pub trait MemoryIngest: Send + Sync { + /// Ingest one standalone document. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for content the driver refuses (empty body, + /// unsupported MIME), otherwise backend failures. + async fn ingest_document(&self, item: IngestItem) -> Result; + + /// Ingest a run of chat messages that share a conversation. + /// + /// Taken as a batch rather than one call per message because chat chunking + /// is inherently cross-message: a driver needs neighbouring turns to decide + /// where a chunk boundary belongs. Ordering within `messages` is + /// significant and must be preserved by the caller. + /// + /// # Errors + /// + /// As [`Self::ingest_document`]. Partial success is reported through the + /// counts in [`IngestOutcome`], not as an error. + async fn ingest_chat(&self, messages: Vec) -> Result; +} + +/// The namespace-document tier: whole documents addressed by `(namespace, key)`. +/// +/// Distinct from [`crate::provider::MemoryCore`] in granularity and in what is +/// stored: entries are short facts, documents are bodies with titles, tags, +/// source types, and structured metadata, and they carry their own ranked query +/// surface. +#[async_trait] +pub trait MemoryDocuments: Send + Sync { + /// Upsert a document, returning its driver-assigned id. + /// + /// Keyed by `(namespace, key)` from the input: reusing a key replaces the + /// existing document rather than creating a second one. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a rejected input, otherwise backend + /// failures. + async fn put_document(&self, input: NamespaceDocumentInput) -> Result; + + /// Fetch a document by `(namespace, key)`. + /// + /// # Errors + /// + /// A missing document is `Ok(None)`; `Err` is reserved for backend + /// failures. + async fn get_document( + &self, + namespace: &str, + key: &str, + ) -> Result, MemoryError>; + + /// Run a ranked query over one namespace's documents. + /// + /// Returns both the ranked hits and the driver's rendered context text, so + /// a caller that only wants something injectable does not have to + /// re-assemble it (and re-assemble it differently from every other caller). + /// + /// # Errors + /// + /// Backend failures only; a query that matches nothing returns an empty + /// hit list. + async fn query_documents( + &self, + namespace: &str, + query: &str, + limit: usize, + ) -> Result; +} + +/// The time-ordered summary tree: buffered leaves rolled up into hour → day → +/// month → year → root summaries. +/// +/// Sealing and cascading are exposed as explicit calls rather than happening +/// implicitly on ingest because the **host** owns scheduling. A driver runs one +/// step when asked; it does not get to install its own background loop. This is +/// the same rule as the engine's `queue::run_once`. +#[async_trait] +pub trait MemoryTree: Send + Sync { + /// Append raw content to the ingestion buffer for later sealing. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a rejected request, otherwise backend + /// failures. + async fn append(&self, request: IngestRequest) -> Result<(), MemoryError>; + + /// Retrieve the chunks a single logical source contributed, newest first. + /// + /// `scope` is the per-turn allowlist and must be applied **inside** the + /// driver's query, for the reasons in [`SourceScope`]. `None` means + /// unrestricted. + /// + /// # Errors + /// + /// Backend failures only; an unknown `source_id` yields an empty vector. + async fn query_source( + &self, + namespace: &str, + source_id: &str, + limit: usize, + scope: Option<&SourceScope>, + ) -> Result, MemoryError>; + + /// Fetch one node together with its direct children, for navigation. + /// + /// # Errors + /// + /// [`MemoryError::NotFound`] when `node_id` does not exist in `namespace`. + async fn drill_down(&self, namespace: &str, node_id: &str) -> Result; + + /// Convert buffered content into leaf nodes, returning the resulting tree + /// state. + /// + /// Idempotent when the buffer is empty: sealing nothing is a successful + /// no-op, not an error, so a scheduler may call it unconditionally. + /// + /// # Errors + /// + /// Backend failures only. + async fn seal(&self, namespace: &str) -> Result; + + /// Roll sealed leaves up through the parent levels, returning the resulting + /// tree state. + /// + /// Idempotent for the same reason as [`Self::seal`]. + /// + /// # Errors + /// + /// Backend failures only. + async fn cascade(&self, namespace: &str) -> Result; +} diff --git a/api/src/provider/driver.rs b/api/src/provider/driver.rs new file mode 100644 index 0000000..842ce38 --- /dev/null +++ b/api/src/provider/driver.rs @@ -0,0 +1,197 @@ +//! [`MemoryProvider`] — the single trait a memory driver implements, and the +//! object the kernel binds. +//! +//! ## Self-contained on purpose +//! +//! `MemoryProvider` does **not** extend a host `Driver` trait and names no host +//! type. `tinycortex-api` is what a third-party driver compiles against, so it +//! must not drag in the OpenHuman host; and the generic subsystem vocabulary +//! (`Driver`, `DriverClass`, `SubsystemRegistry`, the policy `Guard`) belongs +//! kernel-side, where inference and channels can share it without importing a +//! *memory* crate. +//! +//! The bridge is the host's memory adapter, which implements the host `Driver` +//! for an `Arc` and converts [`MemoryHealth`] into the +//! kernel's `DriverHealth`. That conversion is trivial by construction — see +//! [`crate::health`]. +//! +//! Driver **class** (embedded / external / null) is deliberately absent from +//! this trait. Class is a fact about how the host bound a driver, recorded in +//! host configuration; a driver self-reporting it would let a misconfigured +//! external backend claim to be embedded and skip the egress and trust checks +//! that class gates. +//! +//! ## The accessor form, and why not `Any` +//! +//! The kernel binds `Arc` and needs per-family access. Two +//! designs were available: downcast through [`std::any::Any`], or one +//! `Option`-returning accessor per optional family. The accessors win: +//! +//! - **No unchecked downcast.** `Any` would require the caller to name a +//! concrete driver type, which defeats the point of binding behind a trait +//! object, or to register type ids, which is the same table with worse +//! ergonomics. +//! - **The capability set and the reachable surface stay provably in sync.** +//! [`crate::provider::audit_provider`] compares [`MemoryProvider::capabilities`] +//! against what the accessors actually return, so "advertised but not +//! implemented" is a detectable, testable mistake instead of a runtime +//! surprise on the first call. +//! - **[`MemoryProvider::provides`] is an exhaustive `match`** over +//! [`Capability`], so adding a family without wiring an accessor fails to +//! compile. +//! +//! The three mandatory families are supertraits rather than accessors, so they +//! are callable directly on the trait object and cannot be absent. +//! +//! ## Object safety +//! +//! Every method here and in every family trait is object-safe: no generic +//! parameters, no `Self` in return position, no associated constants. The +//! `#[async_trait]` attribute rewrites the `async fn`s into boxed futures, +//! which is what makes them dyn-compatible at all. + +use async_trait::async_trait; + +use crate::capabilities::{Capabilities, Capability}; +use crate::error::MemoryError; +use crate::health::MemoryHealth; +use crate::provider::content::{MemoryDocuments, MemoryIngest, MemoryTree}; +use crate::provider::knowledge::{MemoryDiff, MemoryEntities, MemoryGraph}; +use crate::provider::mandatory::{MemoryCore, MemoryPortability, MemoryRecall}; +use crate::provider::records::{ + MemoryGoals, MemoryMaintenance, MemorySourceSink, MemoryToolMemory, +}; + +/// A bound memory driver. +/// +/// Implementors must also implement the three mandatory families +/// ([`MemoryCore`], [`MemoryRecall`], [`MemoryPortability`]) — they are +/// supertraits, so a driver missing any of them cannot be constructed as a +/// provider at all. +/// +/// The ten optional families are reached through the `as_*` accessors below. +/// Each defaults to `None`, so a minimal driver implements only what it +/// supports and inherits correct absence for everything else. +#[async_trait] +pub trait MemoryProvider: MemoryCore + MemoryRecall + MemoryPortability + 'static { + /// Stable identifier for this driver (`tinycortex`, `supermemory`, `null`). + /// + /// Appears in status output, log lines, tracing spans, and audit events, so + /// it must be stable across restarts and must not embed a URL, a token, or + /// anything else user- or deployment-specific. + fn driver_id(&self) -> &str; + + /// The families this driver implements. + /// + /// Asked **once** at bind time and cached: the kernel filters RPC + /// registration and agent-tool assembly from the cached answer, so a set + /// that changes after binding will not be noticed. A driver whose surface + /// genuinely varies must report the union and answer + /// [`MemoryError::Unsupported`] for the gaps. + /// + /// Must be honest: every advertised family must be reachable through its + /// accessor. [`crate::provider::audit_provider`] checks exactly that. + fn capabilities(&self) -> Capabilities; + + /// Current liveness, as the driver reports it. + /// + /// Called on bind and on demand for status output. Implementations should + /// be cheap and must not block indefinitely — a health probe that hangs is + /// indistinguishable from a subsystem that is down, but takes a timeout to + /// find out. + async fn health(&self) -> MemoryHealth; + + /// Release resources ahead of process exit or a rebind. + /// + /// Defaults to a successful no-op, because most drivers have nothing to + /// release; a driver holding a connection pool or a background task should + /// override it. The host's adapter forwards its `Driver::shutdown` here. + /// + /// Must be idempotent: a rebind followed by process exit calls it twice. + /// + /// # Errors + /// + /// Backend failures during teardown. The caller logs and continues — + /// shutdown failure never blocks exit. + async fn shutdown(&self) -> Result<(), MemoryError> { + Ok(()) + } + + /// Bulk ingestion, when advertised. + fn as_ingest(&self) -> Option<&dyn MemoryIngest> { + None + } + + /// The namespace-document tier, when advertised. + fn as_documents(&self) -> Option<&dyn MemoryDocuments> { + None + } + + /// The summary tree, when advertised. + fn as_tree(&self) -> Option<&dyn MemoryTree> { + None + } + + /// The entity index, when advertised. + fn as_entities(&self) -> Option<&dyn MemoryEntities> { + None + } + + /// The key/value and relation graph, when advertised. + fn as_graph(&self) -> Option<&dyn MemoryGraph> { + None + } + + /// Snapshot and change tracking, when advertised. + fn as_diff(&self) -> Option<&dyn MemoryDiff> { + None + } + + /// The long-term goals document, when advertised. + fn as_goals(&self) -> Option<&dyn MemoryGoals> { + None + } + + /// Per-tool learned rules, when advertised. + fn as_tool_memory(&self) -> Option<&dyn MemoryToolMemory> { + None + } + + /// The host-sync write seam, when advertised. + fn as_sources(&self) -> Option<&dyn MemorySourceSink> { + None + } + + /// Scheduler-driven upkeep, when advertised. + fn as_maintenance(&self) -> Option<&dyn MemoryMaintenance> { + None + } + + /// Whether `capability` is actually **reachable** on this driver. + /// + /// This is the implementation-side truth, as opposed to + /// [`Self::capabilities`], which is the advertised claim. The two should + /// agree; [`crate::provider::audit_provider`] is where they are compared. + /// + /// The mandatory three are always `true` because they are supertraits. The + /// remaining ten delegate to their accessor. + /// + /// The `match` is deliberately exhaustive: [`Capability`] is not + /// `#[non_exhaustive]`, so adding a family without adding an accessor and + /// an arm here is a compile error rather than a silent `false`. + fn provides(&self, capability: Capability) -> bool { + match capability { + Capability::Core | Capability::Recall | Capability::Portability => true, + Capability::Ingest => self.as_ingest().is_some(), + Capability::Documents => self.as_documents().is_some(), + Capability::Tree => self.as_tree().is_some(), + Capability::Entities => self.as_entities().is_some(), + Capability::Graph => self.as_graph().is_some(), + Capability::Diff => self.as_diff().is_some(), + Capability::Goals => self.as_goals().is_some(), + Capability::ToolMemory => self.as_tool_memory().is_some(), + Capability::Sources => self.as_sources().is_some(), + Capability::Maintenance => self.as_maintenance().is_some(), + } + } +} diff --git a/api/src/provider/knowledge.rs b/api/src/provider/knowledge.rs new file mode 100644 index 0000000..45140e5 --- /dev/null +++ b/api/src/provider/knowledge.rs @@ -0,0 +1,176 @@ +//! Optional families that expose *derived structure* over stored memory: +//! [`MemoryEntities`], [`MemoryGraph`], and [`MemoryDiff`]. +//! +//! Each is independently optional. A driver may have a key/value graph but no +//! entity index, or track source snapshots without either. The kernel filters +//! RPC registration and agent-tool assembly per family, so an absent family is +//! invisible rather than present-and-failing. +//! +//! As in [`crate::provider::content`], no configuration crosses this boundary: +//! extraction models, hotness decay curves, and snapshot retention are driver +//! concerns and appear in none of these signatures. + +use async_trait::async_trait; + +use crate::error::MemoryError; +use crate::provider::types::{DiffReport, EntityHit, SnapshotRef}; +use crate::types::{GraphRelationRecord, MemoryKvRecord}; + +/// The entity index: who and what the stored memory is about. +#[async_trait] +pub trait MemoryEntities: Send + Sync { + /// List entities in a namespace, ranked by hotness when `query` is `None` + /// and by match quality otherwise. + /// + /// # Errors + /// + /// Backend failures only; an unknown namespace yields an empty vector. + async fn entities( + &self, + namespace: &str, + query: Option<&str>, + limit: usize, + ) -> Result, MemoryError>; + + /// Edges incident to one entity, most relevant first. + /// + /// Returns [`GraphRelationRecord`] — the same shape [`MemoryGraph`] uses — + /// so a caller that has both families does not have to reconcile two edge + /// representations. + /// + /// # Errors + /// + /// Backend failures only; an unknown `entity_id` yields an empty vector + /// rather than [`MemoryError::NotFound`], because "no edges" and "no such + /// entity" are the same answer to this question. + async fn entity_edges( + &self, + namespace: &str, + entity_id: &str, + limit: usize, + ) -> Result, MemoryError>; + + /// Record that these entities were just observed, updating hotness. + /// + /// Separate from the read path because hotness is a *write* the host + /// triggers at known moments (a turn referenced these entities), not + /// something a driver should infer from being queried — otherwise merely + /// browsing the index would reshape ranking. + /// + /// # Errors + /// + /// Backend failures only. Unknown ids are ignored, not rejected. + async fn touch_entities( + &self, + namespace: &str, + entity_ids: &[String], + ) -> Result<(), MemoryError>; +} + +/// The key/value and relation graph tier. +/// +/// `namespace` is `Option<&str>` throughout: `None` addresses the global, +/// namespace-less slice, matching the storage shape of +/// [`MemoryKvRecord::namespace`] and [`GraphRelationRecord::namespace`]. +#[async_trait] +pub trait MemoryGraph: Send + Sync { + /// Read one key/value record. + /// + /// # Errors + /// + /// A missing key is `Ok(None)`; `Err` is reserved for backend failures. + async fn kv_get( + &self, + namespace: Option<&str>, + key: &str, + ) -> Result, MemoryError>; + + /// Upsert one key/value record. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a rejected key, otherwise backend failures. + async fn kv_put( + &self, + namespace: Option<&str>, + key: &str, + value: serde_json::Value, + ) -> Result<(), MemoryError>; + + /// List key/value records, optionally restricted to a key prefix. + /// + /// # Errors + /// + /// Backend failures only. + async fn kv_list( + &self, + namespace: Option<&str>, + prefix: Option<&str>, + limit: usize, + ) -> Result, MemoryError>; + + /// Query relations, narrowing by subject and/or predicate. + /// + /// Both filters are `None`-able so one method covers "everything about this + /// subject", "every edge of this type", and "the whole slice", instead of + /// three near-identical methods. + /// + /// # Errors + /// + /// Backend failures only. + async fn relations( + &self, + namespace: Option<&str>, + subject: Option<&str>, + predicate: Option<&str>, + limit: usize, + ) -> Result, MemoryError>; + + /// Upsert one relation, keyed by `(namespace, subject, predicate, object)`. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a malformed edge, otherwise backend + /// failures. + async fn put_relation(&self, relation: GraphRelationRecord) -> Result<(), MemoryError>; +} + +/// Snapshot capture and change computation over synced sources. +#[async_trait] +pub trait MemoryDiff: Send + Sync { + /// Capture a snapshot of one source's current items. + /// + /// # Errors + /// + /// [`MemoryError::NotFound`] for an unknown `source_id`, otherwise backend + /// failures. + async fn capture_snapshot(&self, source_id: &str) -> Result; + + /// List snapshots for one source, newest first. + /// + /// # Errors + /// + /// Backend failures only; an unknown `source_id` yields an empty vector. + async fn snapshots( + &self, + source_id: &str, + limit: usize, + ) -> Result, MemoryError>; + + /// Compute the change set between two snapshots of one source. + /// + /// `from` is `Option<&str>` so the first-ever diff — where there is no + /// baseline and every item is an addition — is expressible without a + /// separate method or a sentinel id. + /// + /// # Errors + /// + /// [`MemoryError::NotFound`] when either snapshot id is unknown, otherwise + /// backend failures. + async fn diff( + &self, + source_id: &str, + from: Option<&str>, + to: &str, + ) -> Result; +} diff --git a/api/src/provider/mandatory.rs b/api/src/provider/mandatory.rs new file mode 100644 index 0000000..4827848 --- /dev/null +++ b/api/src/provider/mandatory.rs @@ -0,0 +1,184 @@ +//! The three mandatory capability families: [`MemoryCore`], [`MemoryRecall`], +//! and [`MemoryPortability`]. +//! +//! These are supertraits of [`crate::provider::MemoryProvider`], which is what +//! makes "mandatory" a *compile-time* fact rather than a runtime check: a type +//! that does not implement all three cannot be a provider at all, so there is +//! no way to bind a driver that is missing them. +//! +//! The other ten families are reached through `Option`-returning accessors on +//! the provider, so their absence is representable and their presence is not +//! assumed. See [`crate::provider::MemoryProvider`] for that half. +//! +//! ## Why every method returns [`MemoryError`] and not `anyhow::Error` +//! +//! The transport adapter must be able to turn a `501` from an out-of-process +//! driver into [`MemoryError::Unsupported`], and the kernel must be able to +//! tell "this driver cannot do that" apart from "this driver failed". An +//! `anyhow::Error` erases exactly that distinction. The engine's own +//! [`crate::traits::Memory`] trait keeps `anyhow::Result` — it is an internal +//! storage abstraction with existing implementors, not the driver contract. + +use async_trait::async_trait; + +use crate::error::MemoryError; +use crate::provider::types::{ExportPage, ExportRecord, ImportOutcome, SourceScope}; +use crate::recall::OwnedRecallOpts; +use crate::types::{MemoryCategory, MemoryEntry, MemoryTaint, NamespaceSummary}; + +/// Store, read, and delete individual memory entries. **Mandatory.** +/// +/// This is the smallest surface that still makes something a memory backend: +/// without it there is nothing to recall from and nothing to export. +#[async_trait] +pub trait MemoryCore: Send + Sync { + /// Upsert an entry, keyed by `(namespace, key)`. + /// + /// ## Taint is an argument, never a decision + /// + /// Unlike the engine's [`crate::traits::Memory`], which has a `store` and a + /// separate `store_with_taint` whose default implementation silently drops + /// the taint, the contract has **one** store and it always takes a + /// [`MemoryTaint`]. Provenance is stamped by the host policy guard before + /// the call; a driver that could default it would be able to launder + /// externally-sourced content into internal-trust content, which is the + /// single failure mode the guard exists to prevent. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for caller input the driver rejects, + /// [`MemoryError::Io`] or [`MemoryError::Other`] for backend failures. + async fn store( + &self, + namespace: &str, + key: &str, + content: &str, + category: MemoryCategory, + session_id: Option<&str>, + taint: MemoryTaint, + ) -> Result<(), MemoryError>; + + /// Fetch the entry for an exact `(namespace, key)`. + /// + /// # Errors + /// + /// A missing entry is `Ok(None)`, never an error; `Err` is reserved for + /// backend failures. + async fn get(&self, namespace: &str, key: &str) -> Result, MemoryError>; + + /// Delete the entry for `(namespace, key)`, reporting whether it existed. + /// + /// Idempotent: forgetting an absent key is `Ok(false)`, so callers may call + /// it unconditionally. + /// + /// # Errors + /// + /// Backend failures only. + async fn forget(&self, namespace: &str, key: &str) -> Result; + + /// List entries, narrowing by namespace, category, and session. + /// + /// Each `Some` filter narrows the result; all `None` lists everything the + /// driver holds. An empty result is `Ok(vec![])`. + /// + /// # Errors + /// + /// Backend failures only. + async fn list( + &self, + namespace: Option<&str>, + category: Option<&MemoryCategory>, + session_id: Option<&str>, + ) -> Result, MemoryError>; + + /// Enumerate namespaces with their aggregate counts, for discovery. + /// + /// # Errors + /// + /// Backend failures only. + async fn namespaces(&self) -> Result, MemoryError>; +} + +/// Ranked retrieval. **Mandatory.** +#[async_trait] +pub trait MemoryRecall: Send + Sync { + /// Return up to `limit` entries relevant to `query`, most relevant first. + /// + /// `opts` is the **owned** [`OwnedRecallOpts`], never the borrowed + /// `RecallOpts<'a>`: a lifetime parameter cannot travel through an + /// object-safe `#[async_trait]` method, and the borrowed form derives no + /// serde impls so it could never be a request body. An embedded driver + /// converts to the borrowed form at its own boundary, which is zero-copy. + /// + /// `scope` is the per-turn source allowlist and is a **query predicate the + /// driver must apply internally** — see [`SourceScope`] for why applying it + /// after the fact is wrong. `None` means unrestricted. + /// + /// An empty or non-matching `query` yields `Ok(vec![])`, not an error. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a malformed filter, otherwise backend + /// failures. + async fn recall( + &self, + query: &str, + limit: usize, + opts: &OwnedRecallOpts, + scope: Option<&SourceScope>, + ) -> Result, MemoryError>; +} + +/// Export and import the whole store. **Mandatory.** +/// +/// Mandatory because binding a memory backend without it is a one-way door: a +/// user who cannot export cannot leave. It is the capability that makes every +/// other binding reversible, which is also why the `mirror` migration driver is +/// expressible at all. +#[async_trait] +pub trait MemoryPortability: Send + Sync { + /// Read one page of the export, continuing from `cursor`. + /// + /// Pass `None` to start. The export is complete when the returned + /// [`ExportPage::next_cursor`] is `None` — an empty `records` vector is + /// **not** a terminator, because a driver may legitimately return an empty + /// page while skipping a range. + /// + /// `limit` is a request, not a guarantee; a driver may return fewer. + /// + /// ## Why pages and not a stream + /// + /// A `Stream` return type would either make the trait non-object-safe or + /// drag an async runtime into a crate that deliberately has none. Paging + /// keeps both properties and still bounds memory, with the caller choosing + /// the bound. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a cursor this driver did not issue, + /// otherwise backend failures. + async fn export_page( + &self, + cursor: Option<&str>, + limit: usize, + ) -> Result; + + /// Write a batch of previously-exported records. + /// + /// Records carry their own [`crate::types::MemoryTaint`]; an importing + /// driver must persist what it is given and must not re-stamp provenance. + /// + /// Partial success is normal and is reported in [`ImportOutcome`] rather + /// than as an error: a migration should not abort a million-record restore + /// because one record was malformed. + /// + /// # Errors + /// + /// Reserved for failures that make the whole batch meaningless (backend + /// unavailable, transaction aborted). Per-record rejection belongs in + /// [`ImportOutcome::failed`]. + async fn import_records( + &self, + records: Vec, + ) -> Result; +} diff --git a/api/src/provider/mod.rs b/api/src/provider/mod.rs new file mode 100644 index 0000000..5fe65c9 --- /dev/null +++ b/api/src/provider/mod.rs @@ -0,0 +1,73 @@ +//! The memory driver contract: [`MemoryProvider`] plus the thirteen capability +//! family traits a driver may implement. +//! +//! ## Shape +//! +//! ```text +//! MemoryProvider ── identity, capabilities, health, shutdown +//! : MemoryCore (mandatory — supertrait, always callable) +//! : MemoryRecall (mandatory — supertrait, always callable) +//! : MemoryPortability (mandatory — supertrait, always callable) +//! ├─ as_ingest() -> Option<&dyn MemoryIngest> +//! ├─ as_documents() -> Option<&dyn MemoryDocuments> +//! ├─ as_tree() -> Option<&dyn MemoryTree> +//! ├─ as_entities() -> Option<&dyn MemoryEntities> +//! ├─ as_graph() -> Option<&dyn MemoryGraph> +//! ├─ as_diff() -> Option<&dyn MemoryDiff> +//! ├─ as_goals() -> Option<&dyn MemoryGoals> +//! ├─ as_tool_memory() -> Option<&dyn MemoryToolMemory> +//! ├─ as_sources() -> Option<&dyn MemorySourceSink> +//! └─ as_maintenance() -> Option<&dyn MemoryMaintenance> +//! ``` +//! +//! The mandatory three are supertraits, so "mandatory" is enforced by the type +//! system rather than by a runtime check. The optional ten are accessors that +//! default to `None`, so absence is the default and presence is opt-in. +//! +//! ## Rules that bind every family +//! +//! 1. **Typed errors, always.** Every method returns +//! `Result<_, MemoryError>`. The transport adapter maps an out-of-process +//! `501` onto [`crate::error::MemoryError::Unsupported`], and the kernel +//! distinguishes "cannot" from "failed". `anyhow::Error` would erase that. +//! 2. **No configuration crosses the boundary.** Not one signature names a +//! config type. A driver holds its own configuration; the contract passes +//! domain arguments only. +//! 3. **No host types.** Nothing here names an OpenHuman type, so a +//! third-party driver depends on this crate alone. +//! 4. **The driver never assigns provenance.** [`crate::types::MemoryTaint`] is +//! an argument on every write path and a preserved field on every import. +//! 5. **The host owns the loop.** Sealing, cascading, maintenance, and source +//! sync are all "run one step when asked"; no driver installs a background +//! task or hooks the agent turn. +//! 6. **Object safety throughout.** No generics, no `Self` returns, no +//! associated constants — every family is usable as `&dyn`. +//! +//! ## Reference implementation +//! +//! [`crate::null::NullMemoryProvider`] implements all thirteen families: +//! `/dev/null` semantics for the mandatory three, and +//! [`crate::error::MemoryError::Unsupported`] for the other ten, which it does +//! not advertise. It is what a compiled-out or unconfigured memory subsystem +//! binds to, and it doubles as the proof that the mandatory set is +//! implementable without a storage engine. + +pub mod audit; +pub mod content; +pub mod driver; +pub mod knowledge; +pub mod mandatory; +pub mod records; +pub mod types; + +pub use audit::{audit_provider, CapabilityAudit}; +pub use content::{MemoryDocuments, MemoryIngest, MemoryTree}; +pub use driver::MemoryProvider; +pub use knowledge::{MemoryDiff, MemoryEntities, MemoryGraph}; +pub use mandatory::{MemoryCore, MemoryPortability, MemoryRecall}; +pub use records::{MemoryGoals, MemoryMaintenance, MemorySourceSink, MemoryToolMemory}; +pub use types::{ + ChangeKind, DiffReport, EntityHit, EntityRef, ExportPage, ExportRecord, ImportOutcome, + IngestItem, IngestOutcome, MaintenanceReport, SnapshotRef, SourceChange, SourceItem, + SourceScope, +}; diff --git a/api/src/provider/records.rs b/api/src/provider/records.rs new file mode 100644 index 0000000..064697f --- /dev/null +++ b/api/src/provider/records.rs @@ -0,0 +1,167 @@ +//! The remaining optional families: [`MemoryGoals`], [`MemoryToolMemory`], +//! [`MemorySourceSink`], and [`MemoryMaintenance`]. +//! +//! Goals and tool memory are small curated record sets the agent reads on +//! nearly every turn. The source sink is the seam the host's sync machinery +//! writes through. Maintenance is the seam the host's scheduler drives. +//! +//! ## The host keeps the loop; the driver runs one step +//! +//! [`MemorySourceSink`] receives already-fetched items — the host owns +//! credentials, OAuth, rate limits, and the schedule. [`MemoryMaintenance`] +//! exposes four operations the host's existing scheduler calls; no driver +//! installs a background task of its own. Both follow the same rule as the +//! engine's `queue::run_once`, and both are why a driver never needs to see +//! configuration or a keychain. + +use async_trait::async_trait; + +use crate::error::MemoryError; +use crate::goals::GoalsDoc; +use crate::provider::types::{IngestOutcome, MaintenanceReport, SourceItem}; +use crate::tool_memory::ToolMemoryRule; +use crate::types::MemoryTaint; + +/// The agent's long-term goals document. +#[async_trait] +pub trait MemoryGoals: Send + Sync { + /// Read the current goals document. + /// + /// A driver with no goals yet returns an empty [`GoalsDoc`], not + /// [`MemoryError::NotFound`] — "no goals" is a valid state, not a missing + /// record. + /// + /// # Errors + /// + /// Backend failures only. + async fn goals(&self) -> Result; + + /// Replace the goals document wholesale. + /// + /// Whole-document replacement rather than per-item add/edit/delete because + /// the validating mutation surface (PII and secret predicates) is **host** + /// policy: the host parses, validates, mutates, and hands back the result. + /// Exposing per-item mutation here would put that policy behind a trait a + /// third-party driver implements, where it could be skipped. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a document the driver refuses (e.g. over + /// its own item cap), otherwise backend failures. + async fn set_goals(&self, goals: GoalsDoc) -> Result<(), MemoryError>; +} + +/// Per-tool learned rules — durable guidance attached to a specific tool. +#[async_trait] +pub trait MemoryToolMemory: Send + Sync { + /// Rules for one tool, highest priority first. + /// + /// # Errors + /// + /// Backend failures only; a tool with no rules yields an empty vector. + async fn tool_rules(&self, tool_name: &str) -> Result, MemoryError>; + + /// Upsert one rule, keyed by [`ToolMemoryRule::id`]. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a malformed rule, otherwise backend + /// failures. + async fn put_tool_rule(&self, rule: ToolMemoryRule) -> Result<(), MemoryError>; + + /// Delete one rule, reporting whether it existed. + /// + /// Idempotent, like [`crate::provider::MemoryCore::forget`]. + /// + /// # Errors + /// + /// Backend failures only. + async fn delete_tool_rule(&self, tool_name: &str, rule_id: &str) -> Result; +} + +/// The write seam for host-driven source sync. +#[async_trait] +pub trait MemorySourceSink: Send + Sync { + /// Accept a batch of items the host fetched from one logical source. + /// + /// `taint` applies to the whole batch and is stamped by the host. Sync + /// paths ingesting third-party content pass + /// [`MemoryTaint::ExternalSync`]; the driver persists what it is given and + /// never assigns provenance itself. + /// + /// `source_kind` is a wire string (`folder`, `composio`, …) rather than an + /// enum because the set of source kinds is owned by the host's sync + /// machinery and grows without a contract change. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a rejected batch, otherwise backend + /// failures. Per-item outcomes are counted in [`IngestOutcome`]. + async fn accept_source_items( + &self, + source_id: &str, + source_kind: &str, + items: Vec, + taint: MemoryTaint, + ) -> Result; + + /// Drop everything the driver holds for one logical source, returning how + /// many units were removed. + /// + /// This is the disconnect path: when a user removes a source, its content + /// must leave memory. Idempotent — an unknown `source_id` returns `Ok(0)`. + /// + /// # Errors + /// + /// Backend failures only. + async fn forget_source(&self, source_id: &str) -> Result; +} + +/// Periodic upkeep the host's scheduler drives. +/// +/// All four operations must be safe to call repeatedly and safe to interrupt: +/// the scheduler may invoke them on a timer, and a desktop process can exit at +/// any point. A driver that cannot bound the work should do a slice per call +/// and report progress in [`MaintenanceReport`]. +#[async_trait] +pub trait MemoryMaintenance: Send + Sync { + /// Recompute embeddings for content whose embedding is missing or stale. + /// + /// # Errors + /// + /// Backend failures, or [`MemoryError::BudgetExceeded`] when an embedding + /// budget is exhausted mid-run. + async fn reembed(&self) -> Result; + + /// Reclaim space: vacuum indexes, drop tombstones, prune dead references. + /// + /// # Errors + /// + /// Backend failures only. + async fn compact(&self) -> Result; + + /// Merge and summarise accumulated memory — the "dream" pass. + /// + /// The embedded driver maps this onto its seal/cascade/reembed cycle; an + /// external driver maps it onto whatever it calls the same idea. The + /// contract deliberately does not specify the mechanism, only that it is + /// the operation a scheduler runs when the system is idle. + /// + /// # Errors + /// + /// Backend failures only. + async fn consolidate(&self) -> Result; + + /// Read-only integrity check. + /// + /// Reports findings in [`MaintenanceReport::findings`] and must change + /// nothing — [`MaintenanceReport::changed`] is always `0`. A driver that + /// repairs as it inspects should expose that as [`Self::compact`] instead, + /// so an operator can diagnose without mutating. + /// + /// # Errors + /// + /// Backend failures only. A *finding* is not an error: a store with + /// problems still returns `Ok` with the problems listed. + async fn doctor(&self) -> Result; +} diff --git a/api/src/provider/types.rs b/api/src/provider/types.rs new file mode 100644 index 0000000..1c9c3d2 --- /dev/null +++ b/api/src/provider/types.rs @@ -0,0 +1,393 @@ +//! Value types that exist only because the *driver contract* needs them. +//! +//! Everything here is inert data: serde-derived, dependency-light, and free of +//! any engine or host type. They are separated from [`crate::types`] because +//! that module carries the historical engine value types (which the engine +//! crate aliases back into `tinycortex::memory::types`), whereas these are new +//! shapes introduced by the provider contract itself. +//! +//! ## Why these types and not the engine's +//! +//! Several families the contract exposes (diff, entities, sources, +//! maintenance) have richer types inside the `tinycortex` engine — for example +//! `memory::diff::types::DiffResult`. Those types are *implementation* shapes: +//! they carry git commit SHAs, ledger paths, and engine-specific enums. A +//! third-party driver cannot produce them and must not be required to. +//! +//! So the contract defines the narrower shape a *caller* actually needs, with +//! wire strings deliberately identical to the engine's where they overlap +//! (`added`/`removed`/`modified`), so the embedded driver's conversion is a +//! field-for-field map rather than a translation. +//! +//! ## What is deliberately absent +//! +//! No type here names a configuration struct. `MemoryConfig` stayed engine-side +//! in the M0 carve-out and stays there: a driver holds its own configuration +//! and the contract passes only domain arguments. If a future method cannot be +//! expressed without configuration, that is a signal the family was designed +//! wrong, not that the contract should widen. + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +use crate::chunks::{DataSource, SourceRef}; +use crate::types::MemoryTaint; + +/// A per-turn allowlist of memory sources, passed **into** the driver as a +/// query predicate. +/// +/// ## Why this is a parameter and not a post-filter +/// +/// The host computes a per-turn source allowlist from product policy. If that +/// allowlist were applied after the driver returned rows, a `limit` would be +/// consumed by rows the caller is not allowed to see — so a scoped query could +/// return fewer results than it should, or none at all, purely as an artefact +/// of filtering order. Worse, an out-of-process driver would have already been +/// handed a query it should never have answered in full. +/// +/// The predicate therefore travels with the call. `None` means unrestricted; +/// `Some(scope)` means the driver must apply it *inside* its query. +/// +/// ## Matching rule (fail-closed) +/// +/// [`SourceScope::allows_source_id`] encodes the embedded engine's SQL +/// semantics verbatim: a source-attributed id is in scope when it either equals +/// an allowed id outright, or begins with `mem_src:{allowed}:`. An **empty** +/// allow list therefore matches nothing — a scope that lists no sources denies +/// all source-attributed content rather than waving it through. +/// +/// Content that is not attributed to a memory source at all (no +/// `memory_sources` provenance) is outside this predicate's remit; the driver +/// decides that, exactly as the engine's SQL does today. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct SourceScope { + /// Allowed memory-source identifiers. Empty denies all source-attributed + /// content. + pub allow: Vec, +} + +impl SourceScope { + /// Builds a scope from any iterator of source identifiers. + pub fn new(allow: impl IntoIterator>) -> Self { + Self { + allow: allow.into_iter().map(Into::into).collect(), + } + } + + /// Whether this scope lists no sources — in which case it denies all + /// source-attributed content. See the type docs for why that is the + /// fail-closed reading and not "unrestricted". + pub fn is_empty(&self) -> bool { + self.allow.is_empty() + } + + /// Whether `source_id` is in scope, using the engine's equality-or-prefix + /// rule. + /// + /// ``` + /// use tinycortex_api::provider::types::SourceScope; + /// + /// let scope = SourceScope::new(["src-abc"]); + /// assert!(scope.allows_source_id("src-abc")); + /// assert!(scope.allows_source_id("mem_src:src-abc:item-1")); + /// assert!(!scope.allows_source_id("src-xyz")); + /// + /// // An empty scope denies everything. + /// assert!(!SourceScope::default().allows_source_id("src-abc")); + /// ``` + pub fn allows_source_id(&self, source_id: &str) -> bool { + self.allow.iter().any(|allowed| { + source_id == allowed || source_id.starts_with(&format!("mem_src:{allowed}:")) + }) + } +} + +/// One unit of content handed to [`crate::provider::MemoryIngest`]. +/// +/// The driver owns chunking, embedding, and persistence — this type carries +/// only what the driver cannot know: where the content came from, when, who it +/// belongs to, and how far it may be trusted. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct IngestItem { + /// Target namespace; `None` means the driver's default namespace. + #[serde(default)] + pub namespace: Option, + /// Concrete upstream provider the content came from. + pub source: DataSource, + /// Stable logical id for the ingestion group (channel id, thread id, doc + /// id). This is the dedupe key, not a display value. + pub source_id: String, + /// Account or user the content belongs to; empty for anonymous/system + /// sources. + #[serde(default)] + pub owner: String, + /// Opaque pointer back to the raw source record, for citation and + /// drill-down. + #[serde(default)] + pub source_ref: Option, + /// The content itself, already decoded to text. + pub content: String, + /// MIME type of [`Self::content`] when the caller knows it. + #[serde(default)] + pub mime: Option, + /// Event time used for ordering and tree placement; the driver substitutes + /// ingest time when absent. + #[serde(default)] + pub timestamp: Option>, + /// Labels carried through from the source. Ingest does not interpret them. + #[serde(default)] + pub tags: Vec, + /// Provenance taint. The **host** stamps this; a driver must persist what it + /// is given and must never assign or upgrade it. + #[serde(default)] + pub taint: MemoryTaint, + /// Overrides `source_id` for on-disk path grouping only; `source_id` + /// remains the dedupe key. + #[serde(default)] + pub path_scope: Option, +} + +/// What an ingest call actually persisted. +/// +/// Counts rather than content, so the caller can report progress and detect a +/// silently-dropping driver without holding the written material in memory. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct IngestOutcome { + /// Units the driver newly persisted. + pub written: u32, + /// Units the driver recognised as already present and skipped. + pub skipped: u32, + /// Driver-assigned ids for the written units, when the driver exposes them. + /// May be empty even when [`Self::written`] is non-zero — an external + /// backend is not obliged to surface its internal ids. + #[serde(default)] + pub ids: Vec, +} + +/// One line of the portability stream. +/// +/// Export and import are defined over records rather than bytes so the contract +/// stays free of an async runtime and of any streaming abstraction: the host +/// adapter turns a page of records into NDJSON (and back) at the transport +/// boundary. +/// +/// [`Self::kind`] is a driver-defined string rather than an enum. A backend has +/// record kinds this crate has never heard of, and a migration between two +/// backends must round-trip them untouched rather than drop what it cannot +/// classify. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct ExportRecord { + /// Driver-defined record kind (e.g. `entry`, `document`, `chunk`). + pub kind: String, + /// Driver-assigned id, unique within [`Self::kind`]. + pub id: String, + /// Owning namespace, when the record has one. + #[serde(default)] + pub namespace: Option, + /// Provenance taint of the record's content. Preserved across + /// export → import; an importing driver must not re-stamp it. + #[serde(default)] + pub taint: MemoryTaint, + /// The record body, in the exporting driver's own shape. + pub payload: serde_json::Value, +} + +/// One page of an export, plus the cursor that continues it. +/// +/// Paging (rather than a stream) keeps [`crate::provider::MemoryPortability`] +/// object-safe and runtime-agnostic while still bounding memory: the caller +/// decides the page size and drives the loop. +#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] +pub struct ExportPage { + /// Records in this page. May be empty on the final page. + pub records: Vec, + /// Opaque cursor to pass to the next call. `None` means the export is + /// complete — this, not an empty [`Self::records`], is the terminator. + #[serde(default)] + pub next_cursor: Option, +} + +/// What an import call actually accepted. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct ImportOutcome { + /// Records written. + pub imported: u32, + /// Records recognised as already present and skipped. + pub skipped: u32, + /// Records rejected. A non-zero value with an empty [`Self::errors`] is a + /// driver bug: a rejection the operator cannot diagnose. + pub failed: u32, + /// Operator-facing reasons for the failures, bounded by the driver. Must + /// not contain record content or credentials — this is logged. + #[serde(default)] + pub errors: Vec, +} + +/// Identity of an entity in the driver's index. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct EntityRef { + /// Canonical, driver-stable entity id. + pub id: String, + /// Entity kind as a wire string (`person`, `organization`, `topic`, …). + /// A string rather than an enum because the taxonomy is the driver's, and a + /// kind this build does not recognise must still round-trip. + pub kind: String, + /// Display name. + pub name: String, +} + +/// An entity together with its recency/frequency signals. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct EntityHit { + /// The entity itself. + pub entity: EntityRef, + /// Driver-computed hotness, higher is hotter. Not normalised across + /// drivers — compare within one driver's results only. + pub hotness: f64, + /// Number of times the entity was observed. + pub mentions: u32, +} + +/// Identity of a captured snapshot. +/// +/// The engine's own snapshot type additionally carries the git commit SHA and +/// ledger trailers that back it; those are implementation, so the contract +/// exposes only the identity and the counts a caller can act on. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct SnapshotRef { + /// Driver-stable snapshot id. + pub id: String, + /// Logical source this snapshot covers. + pub source_id: String, + /// Human-readable source label at capture time. + #[serde(default)] + pub label: String, + /// Number of items materialised into the snapshot. + pub item_count: u32, + /// Capture time in milliseconds since the Unix epoch. + pub taken_at_ms: i64, +} + +/// What happened to one item between two snapshots. +/// +/// Wire strings are identical to the engine's `memory::diff::types::ChangeKind` +/// so the embedded adapter maps rather than translates. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ChangeKind { + /// Present in the later snapshot only. + Added, + /// Present in the earlier snapshot only. + Removed, + /// Present in both, with differing content. + Modified, +} + +impl ChangeKind { + /// Stable wire string. + pub fn as_str(self) -> &'static str { + match self { + Self::Added => "added", + Self::Removed => "removed", + Self::Modified => "modified", + } + } +} + +/// A single item-level change inside a [`DiffReport`]. +/// +/// Item identity is the item id, never the title, so a rename reports as a +/// removal plus an addition rather than a modification. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct SourceChange { + /// Stable item id. + pub item_id: String, + /// Display title, or the id when the driver has no better label. + #[serde(default)] + pub title: String, + /// What kind of change occurred. + pub kind: ChangeKind, + /// Content hash on the earlier side; absent for an addition. + #[serde(default)] + pub old_content_hash: Option, + /// Content hash on the later side; absent for a removal. + #[serde(default)] + pub new_content_hash: Option, +} + +/// The result of diffing one source between two snapshots. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct DiffReport { + /// Source this diff covers. + pub source_id: String, + /// Baseline snapshot id; `None` for a first-ever diff, where everything is + /// an addition. + #[serde(default)] + pub from_snapshot_id: Option, + /// Target snapshot id. + pub to_snapshot_id: String, + /// Items added. + pub added: u32, + /// Items removed. + pub removed: u32, + /// Items modified. + pub modified: u32, + /// Items present and unchanged. + pub unchanged: u32, + /// Per-item changes. May be truncated by the driver; the counts above are + /// authoritative. + #[serde(default)] + pub changes: Vec, +} + +/// One item handed to [`crate::provider::MemorySourceSink`] by the host's sync +/// machinery. +/// +/// The host owns credentials, scheduling, and fetching; the driver owns storage +/// and indexing. This type is the whole of what crosses that line. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct SourceItem { + /// Stable per-source item id. Dedupe key; not a display value. + pub item_id: String, + /// Display title. + #[serde(default)] + pub title: String, + /// Item body, already decoded to text. + pub content: String, + /// MIME type of [`Self::content`] when known. + #[serde(default)] + pub mime: Option, + /// Canonical URL back to the item, when it has one. + #[serde(default)] + pub url: Option, + /// Upstream last-modified time in milliseconds since the Unix epoch. + #[serde(default)] + pub updated_at_ms: Option, + /// Labels carried through from the source. + #[serde(default)] + pub tags: Vec, +} + +/// Outcome of one maintenance operation. +/// +/// A single shape covers reembed, compact, consolidate, and doctor because the +/// caller does the same thing with all four: report progress and surface +/// findings. A per-operation result type would multiply the contract surface +/// without giving any caller more to act on. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct MaintenanceReport { + /// Which operation ran (`reembed`, `compact`, `consolidate`, `doctor`). + pub operation: String, + /// Units the driver examined. + pub examined: u64, + /// Units the driver changed. Always `0` for `doctor`, which is read-only. + pub changed: u64, + /// Operator-facing findings and notes. Must not contain memory content or + /// credentials — this is logged and shown in status output. + #[serde(default)] + pub findings: Vec, +} + +#[cfg(test)] +#[path = "types_tests.rs"] +mod tests; diff --git a/api/src/provider/types_tests.rs b/api/src/provider/types_tests.rs new file mode 100644 index 0000000..390e59a --- /dev/null +++ b/api/src/provider/types_tests.rs @@ -0,0 +1,133 @@ +//! Tests for the contract-only value types. +//! +//! The focus is the two things a later slice can silently break: the +//! fail-closed reading of an empty [`SourceScope`], and the wire strings / +//! serde defaults that an out-of-process driver depends on. + +use super::*; + +#[test] +fn empty_source_scope_denies_every_source() { + let scope = SourceScope::default(); + assert!(scope.is_empty()); + assert!(!scope.allows_source_id("src-abc")); + assert!(!scope.allows_source_id("mem_src:src-abc:item")); +} + +#[test] +fn source_scope_matches_exact_id_and_mem_src_prefix() { + let scope = SourceScope::new(["src-abc", "src-def"]); + + assert!(scope.allows_source_id("src-abc")); + assert!(scope.allows_source_id("src-def")); + assert!(scope.allows_source_id("mem_src:src-abc:item-1")); + assert!(scope.allows_source_id("mem_src:src-def:nested:item")); + + assert!(!scope.allows_source_id("src-xyz")); + assert!(!scope.allows_source_id("mem_src:src-xyz:item-1")); +} + +#[test] +fn source_scope_prefix_requires_the_trailing_separator() { + // `src-abc` must not smear onto `src-abcdef`: the engine's SQL binds + // `mem_src:{id}:` including the trailing colon, so a longer id that merely + // starts with an allowed one is out of scope. + let scope = SourceScope::new(["src-abc"]); + assert!(!scope.allows_source_id("mem_src:src-abcdef:item")); + assert!(!scope.allows_source_id("src-abcdef")); +} + +#[test] +fn change_kind_wire_strings_match_the_engine() { + // These strings are shared with `memory::diff::types::ChangeKind`, so the + // embedded adapter maps rather than translates. Changing one is a contract + // major bump. + for (kind, expected) in [ + (ChangeKind::Added, "added"), + (ChangeKind::Removed, "removed"), + (ChangeKind::Modified, "modified"), + ] { + assert_eq!(kind.as_str(), expected); + assert_eq!( + serde_json::to_value(kind).expect("serialize change kind"), + serde_json::Value::String(expected.to_string()), + ); + } +} + +#[test] +fn export_page_terminates_on_absent_cursor_not_empty_records() { + let page = ExportPage::default(); + assert!(page.records.is_empty()); + assert!(page.next_cursor.is_none()); + + // An empty page with a cursor is a legitimate mid-export state, so callers + // must not treat "no records" as the terminator. + let midway = ExportPage { + records: Vec::new(), + next_cursor: Some("cursor-2".to_string()), + }; + assert!(midway.next_cursor.is_some()); +} + +#[test] +fn export_record_round_trips_taint_and_opaque_payload() { + let record = ExportRecord { + kind: "vendor_specific_kind".to_string(), + id: "rec-1".to_string(), + namespace: Some("global".to_string()), + taint: MemoryTaint::ExternalSync, + payload: serde_json::json!({ "anything": [1, 2, 3] }), + }; + + let json = serde_json::to_string(&record).expect("serialize record"); + let back: ExportRecord = serde_json::from_str(&json).expect("deserialize record"); + + assert_eq!(back, record); + assert_eq!(back.taint, MemoryTaint::ExternalSync); +} + +#[test] +fn ingest_item_deserializes_from_the_minimal_body() { + // Every optional field carries `#[serde(default)]`, so a caller that knows + // only source, id, and content can still build a valid request. + let item: IngestItem = serde_json::from_value(serde_json::json!({ + "source": "notion", + "source_id": "page-1", + "content": "hello", + })) + .expect("deserialize minimal ingest item"); + + assert_eq!(item.source, DataSource::Notion); + assert_eq!(item.namespace, None); + assert_eq!(item.owner, ""); + assert!(item.tags.is_empty()); + // Provenance defaults to the conservative-for-writes `Internal`; the host + // guard overrides it explicitly on every sync path. + assert_eq!(item.taint, MemoryTaint::Internal); +} + +#[test] +fn maintenance_report_defaults_to_a_clean_read_only_run() { + let report = MaintenanceReport { + operation: "doctor".to_string(), + ..MaintenanceReport::default() + }; + assert_eq!(report.changed, 0); + assert!(report.findings.is_empty()); +} + +#[test] +fn diff_report_expresses_a_first_ever_diff_without_a_sentinel() { + let report = DiffReport { + source_id: "src-abc".to_string(), + from_snapshot_id: None, + to_snapshot_id: "snap-1".to_string(), + added: 3, + ..DiffReport::default() + }; + + let json = serde_json::to_value(&report).expect("serialize diff report"); + assert_eq!(json["from_snapshot_id"], serde_json::Value::Null); + assert_eq!(json["added"], 3); +} From 5a2e38cad6652ae338b9c6af0805cbbeb1a98b40 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 28 Jul 2026 22:15:04 +0300 Subject: [PATCH 12/19] feat(api): omit absent recall filters from the wire form --- api/src/recall.rs | 8 ++++---- api/src/recall_tests.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/api/src/recall.rs b/api/src/recall.rs index 593194a..4d82619 100644 --- a/api/src/recall.rs +++ b/api/src/recall.rs @@ -86,16 +86,16 @@ pub struct RecallOpts<'a> { #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] pub struct OwnedRecallOpts { /// Restrict recall to this namespace; `None` falls back to [`crate::types::GLOBAL_NAMESPACE`]. - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub namespace: Option, /// Restrict recall to entries of this category. - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub category: Option, /// Restrict recall to entries scoped to this session. - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub session_id: Option, /// Drop hits scoring below this threshold (typically 0.0–1.0). - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub min_score: Option, /// When `true`, include conversational hits from other sessions in the same /// workspace alongside the namespace recall. diff --git a/api/src/recall_tests.rs b/api/src/recall_tests.rs index c008482..5adf317 100644 --- a/api/src/recall_tests.rs +++ b/api/src/recall_tests.rs @@ -116,3 +116,43 @@ fn partial_recall_body_leaves_unmentioned_fields_at_default() { assert!(decoded.min_score.is_none()); assert!(!decoded.cross_session); } + +/// The wire form omits absent filters rather than emitting explicit nulls. +/// +/// `OwnedRecallOpts` is the body of `POST /v1/memory/recall`, which the spec +/// describes as an optional-filters bag. Emitting `"namespace": null` for every +/// unset filter is valid JSON but forces a backend to distinguish "absent" from +/// "explicitly null" for no gain. Pinned here because changing the emitted shape +/// after a driver has shipped is observable to any backend that draws that +/// distinction. +#[test] +fn absent_recall_filters_are_omitted_from_the_wire_form() { + let json = serde_json::to_value(OwnedRecallOpts::default()).expect("serialize"); + assert_eq!( + json, + serde_json::json!({ "cross_session": false }), + "unset optional filters must be omitted, not serialized as null" + ); + + let populated = OwnedRecallOpts { + namespace: Some("work".into()), + ..Default::default() + }; + let json = serde_json::to_value(&populated).expect("serialize"); + assert_eq!( + json, + serde_json::json!({ "namespace": "work", "cross_session": false }) + ); +} + +/// Omitting a filter and sending it as `null` must both decode to `None`, so a +/// backend built against either spelling keeps working. +#[test] +fn omitted_and_explicit_null_recall_filters_both_decode_to_none() { + let omitted: OwnedRecallOpts = serde_json::from_str("{}").expect("decode {}"); + let explicit: OwnedRecallOpts = + serde_json::from_str(r#"{"namespace":null,"category":null,"session_id":null,"min_score":null,"cross_session":false}"#) + .expect("decode explicit nulls"); + assert_eq!(omitted, explicit); + assert_eq!(omitted, OwnedRecallOpts::default()); +} From c30b76fc73591810f615a544e7bcf5ff6c3ccc42 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 4 Aug 2026 20:19:24 +0300 Subject: [PATCH 13/19] fix(api): skip unknown capabilities during deserialize, widen bitset Vec::deserialize failed the whole handshake decode on one unrecognised family string, contradicting the documented minor-version rule that a newer driver's unknown families should be skipped, not fail the bind. Decode as Vec and filter through Capability::parse instead. Also widens Capabilities' internal bitset from u16 to u64: at 13 of 16 addressable bits, a 17th family would have overflowed the shift. Co-authored-by: Medulla --- api/src/capabilities.rs | 25 +++++++++++++++++++------ api/src/capabilities_tests.rs | 33 +++++++++++++++++++++++++-------- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/api/src/capabilities.rs b/api/src/capabilities.rs index 1d67575..7c1e7d3 100644 --- a/api/src/capabilities.rs +++ b/api/src/capabilities.rs @@ -191,8 +191,8 @@ impl Capability { } /// Single-bit mask for this family within a [`Capabilities`] set. - fn bit(self) -> u16 { - 1 << self.index() + fn bit(self) -> u64 { + 1u64 << self.index() } } @@ -219,11 +219,13 @@ impl std::str::FromStr for Capability { /// set has neither. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] pub struct Capabilities { - bits: u16, + bits: u64, } impl Capabilities { - /// The empty set. Advertised by the `null` driver. + /// The empty default capability set. The `null` driver advertises + /// [`Self::mandatory`] via its [`MemoryProvider::capabilities`](crate::provider::MemoryProvider::capabilities) + /// implementation, not this. pub const fn empty() -> Self { Self { bits: 0 } } @@ -344,12 +346,23 @@ impl Serialize for Capabilities { } impl<'de> Deserialize<'de> for Capabilities { + /// Skips any family string this build does not recognise, rather than + /// failing the whole deserialize. + /// + /// A remote driver speaking a newer minor contract version may advertise a + /// family this build has never heard of — see [`Capability::parse`] and the + /// module-level "wire stability" docs. Rejecting the whole handshake on one + /// unknown string would refuse an otherwise-compatible driver; the correct + /// behaviour is to drop the family this kernel could never call anyway. fn deserialize(deserializer: D) -> Result where D: serde::Deserializer<'de>, { - let families = Vec::::deserialize(deserializer)?; - Ok(families.into_iter().collect()) + let raw = Vec::::deserialize(deserializer)?; + let families = raw + .into_iter() + .filter_map(|family| Capability::parse(&family).ok()); + Ok(families.collect()) } } diff --git a/api/src/capabilities_tests.rs b/api/src/capabilities_tests.rs index 4f4002e..1e5e550 100644 --- a/api/src/capabilities_tests.rs +++ b/api/src/capabilities_tests.rs @@ -135,10 +135,20 @@ fn capabilities_empty_contains_nothing() { for capability in Capability::ALL { assert!(!none.contains(capability)); } - // The `null` driver's set is the default. + // The default capability set is empty (the null driver itself advertises + // `Capabilities::mandatory()`, not the default). assert_eq!(Capabilities::default(), none); } +#[test] +fn capabilities_bit_width_has_room_well_beyond_the_current_thirteen_families() { + // A `u16` bitset (the original representation) has exactly 16 bit + // positions, leaving room for only 3 more families before a family's + // `1 << index` bit-shift overflows. Pin the wider `u64` representation so + // a future family addition doesn't have to rediscover that ceiling. + assert!(std::mem::size_of::() * 8 >= 64); +} + #[test] fn capabilities_insert_and_remove_are_idempotent() { let mut set = Capabilities::empty(); @@ -229,13 +239,20 @@ fn capabilities_deserialization_collapses_duplicates_and_ignores_order() { } #[test] -fn capabilities_deserialization_rejects_an_unknown_family() { - // The set type itself is strict; a handshake parser that wants to *skip* - // unknown families must decode the array element-wise via - // `Capability::parse`, which is why that function returns a `Result`. - let err = serde_json::from_value::(json!(["core", "warp_drive"])) - .expect_err("unknown family must not silently decode"); - assert!(err.to_string().contains("warp_drive"), "{err}"); +fn capabilities_deserialization_skips_an_unknown_family() { + // A remote driver speaking a newer minor contract version may advertise a + // family this build has never heard of (see the module docs' "wire + // stability" section and `Capability::parse`). The handshake must still + // decode — with the unknown family dropped — rather than failing the bind + // outright. + let decoded: Capabilities = + serde_json::from_value(json!(["core", "warp_drive", "recall"])).unwrap(); + assert_eq!( + decoded, + Capabilities::empty() + .with(Capability::Core) + .with(Capability::Recall) + ); } #[test] From 1b4657d399c777054d31784fbfb5cd63535a3219 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 4 Aug 2026 20:19:27 +0300 Subject: [PATCH 14/19] fix(api): add registry version to the api path dependency Cargo rejects a path-only dependency when packaging the root crate for publish (`all dependencies must have a version requirement specified when packaging`). Add the matching version alongside path. Co-authored-by: Medulla --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 7e0d5b6..216d105 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,7 +86,7 @@ tinyagents = "2.1" # The stable contract crate (value types, error enum, `Memory` trait). Declared # as a path dependency, not a registry requirement, so embedding hosts resolve # it through this checkout without needing their own `[patch.crates-io]` entry. -tinycortex-api = { path = "api" } +tinycortex-api = { path = "api", version = "0.1.1" } toml = "0.8" uuid = { version = "1", features = ["serde", "v4"] } walkdir = "2" From 90d2f7d0d8ee2a8e5a88d13fadf95c4ba6e0337a Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 4 Aug 2026 20:19:31 +0300 Subject: [PATCH 15/19] fix(api): align health wire-format doc with the actual reason field MemoryHealth and its wire tests serialize the degraded/down explanation as "reason", but the module doc described the GET /v1/health response as { status, detail }. Co-authored-by: Medulla --- api/src/health.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/health.rs b/api/src/health.rs index 248238a..5a68444 100644 --- a/api/src/health.rs +++ b/api/src/health.rs @@ -21,7 +21,7 @@ //! //! Serializes as an internally-tagged object with a stable snake_case `status` //! discriminant, which is also the shape of the transport adapter's -//! `GET /v1/health` → `{ status, detail }` response: +//! `GET /v1/health` → `{ status, reason }` response: //! //! ```json //! { "status": "ready" } From 62e41cb2cf36d6ecbf20acc654ef8ecf4356b61e Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 4 Aug 2026 20:19:44 +0300 Subject: [PATCH 16/19] fix(api): reject an export cursor the null driver never issued MemoryPortability::export_page accepted every Some(cursor) as a valid terminal page. The null driver never issues a cursor, so any Some value is necessarily one it did not hand out; a corrupted or wrong-driver cursor should fail loudly rather than silently terminate an export. Adds a regression test. Co-authored-by: Medulla --- api/src/null.rs | 13 ++++++++++++- api/src/null_tests.rs | 12 ++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/api/src/null.rs b/api/src/null.rs index abd9b63..482bc5c 100644 --- a/api/src/null.rs +++ b/api/src/null.rs @@ -176,11 +176,22 @@ impl MemoryRecall for NullMemoryProvider { impl MemoryPortability for NullMemoryProvider { /// One empty, terminal page: no records and no continuation cursor, so a /// caller's export loop terminates on the first iteration. + /// + /// This driver never issues a cursor (every page is the first and only + /// page), so any `Some(_)` cursor a caller passes back is necessarily one + /// this driver did not hand out — reject it rather than silently treating + /// it as a valid terminal page. async fn export_page( &self, - _cursor: Option<&str>, + cursor: Option<&str>, _limit: usize, ) -> Result { + if cursor.is_some() { + return Err(MemoryError::Invalid( + "null provider does not issue export cursors".into(), + )); + } + Ok(ExportPage::default()) } diff --git a/api/src/null_tests.rs b/api/src/null_tests.rs index 33f3f76..573ee90 100644 --- a/api/src/null_tests.rs +++ b/api/src/null_tests.rs @@ -141,6 +141,18 @@ fn mandatory_portability_round_trips_as_an_empty_store() { assert_eq!(outcome.failed, 0); } +#[test] +fn export_page_rejects_a_cursor_it_never_issued() { + let driver = NullMemoryProvider::new(); + + let err = block_on(driver.export_page(Some("unexpected"), 100)) + .expect_err("a cursor this driver never issued must be rejected, not silently accepted"); + assert!( + matches!(err, MemoryError::Invalid(_)), + "expected MemoryError::Invalid, got {err:?}" + ); +} + #[test] fn every_unadvertised_family_is_unreachable_through_the_trait_object() { let driver = NullMemoryProvider::new(); From 7af7b8d9f0217bf790a79dd8b8dcec5a353397fc Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 4 Aug 2026 20:20:49 +0300 Subject: [PATCH 17/19] fix(memory): validate goal text in save, fix duplicate-id delete, doc links GoalsDoc.items and GoalItem.text are public, so a caller could bypass GoalsDocMutations::add/edit's secret/PII validation entirely by constructing a document by hand and calling save directly. save is now the choke point that re-validates every item's text. GoalsDocMutations::delete used retain(), which removes every item sharing an id. GoalsDoc::parse tolerates duplicate ids in a hand-edited/corrupt file, so a delete could silently drop more than the one goal edit() would have addressed. Switched to removing only the first matching occurrence, matching edit's semantics. Also fixes two rustdoc intra-doc links in store.rs that pointed at private items (has_goal_secret/has_goal_pii), which broke under -D warnings. Co-authored-by: Medulla --- src/memory/goals/mutations_tests.rs | 13 ++++++++++++ src/memory/goals/store.rs | 33 +++++++++++++++++++++++------ src/memory/goals/store_tests.rs | 21 ++++++++++++++++++ 3 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/memory/goals/mutations_tests.rs b/src/memory/goals/mutations_tests.rs index 849ebdc..16fe261 100644 --- a/src/memory/goals/mutations_tests.rs +++ b/src/memory/goals/mutations_tests.rs @@ -73,6 +73,19 @@ fn delete_removes_known_id_and_rejects_unknown() { assert!(doc.delete("nope").is_err()); } +#[test] +fn delete_removes_only_the_first_occurrence_of_a_duplicate_id() { + // A well-formed document never has duplicate ids, but `GoalsDoc::parse` + // accepts a hand-edited or corrupt file that does. `delete` must not + // bulk-remove every item sharing the id. + let mut doc = GoalsDoc { + items: vec![GoalItem::new("g1", "first"), GoalItem::new("g1", "second")], + }; + doc.delete("g1").unwrap(); + assert_eq!(doc.items.len(), 1); + assert_eq!(doc.items[0].text, "second"); +} + #[test] fn next_id_avoids_collision_with_custom_ids() { let mut doc = GoalsDoc { diff --git a/src/memory/goals/store.rs b/src/memory/goals/store.rs index 647b0dc..1cab3c7 100644 --- a/src/memory/goals/store.rs +++ b/src/memory/goals/store.rs @@ -82,7 +82,7 @@ fn validate_goal_text(text: &str) -> Result<&str, MemoryError> { /// /// These were inherent methods on `GoalsDoc` until the value types moved into /// the dependency-light `tinycortex-api` crate. They live here, next to the -/// `regex`-backed [`has_goal_secret`] / [`has_goal_pii`] guards they call, so +/// `regex`-backed `has_goal_secret` / `has_goal_pii` guards they call, so /// the contract crate stays free of that machinery. The trait is implemented /// for `GoalsDoc` and re-exported from [`super`], so call sites keep using /// method syntax (`doc.add(text)`) with identical semantics. @@ -123,11 +123,17 @@ impl GoalsDocMutations for GoalsDoc { } fn delete(&mut self, id: &str) -> Result<(), MemoryError> { - let before = self.items.len(); - self.items.retain(|i| i.id != id); - if self.items.len() == before { - return Err(MemoryError::NotFound(format!("no goal with id '{id}'"))); - } + // `retain` would remove every item sharing `id`. A well-formed + // document never has duplicate ids, but `GoalsDoc::parse` accepts a + // hand-edited or corrupt file that does — match `edit`'s "first + // occurrence" semantics so a duplicate-id document loses at most one + // goal per delete, not all of them. + let index = self + .items + .iter() + .position(|item| item.id == id) + .ok_or_else(|| MemoryError::NotFound(format!("no goal with id '{id}'")))?; + self.items.remove(index); Ok(()) } } @@ -230,8 +236,23 @@ fn enforce_caps(doc: &mut GoalsDoc) -> Vec { /// NOTE: this write is a plain truncate-then-write, not atomic /// temp-file+rename; a crash mid-write can leave the file empty or partial /// (see the module-level caveat above). +/// +/// `GoalsDoc.items` and `GoalItem.text` are public, so a caller can build a +/// document directly and hand it to `save` without going through +/// [`GoalsDocMutations::add`]/[`GoalsDocMutations::edit`] — bypassing their +/// secret/PII validation. Re-validate every item's text here so `save` is the +/// one choke point that guarantees `MEMORY_GOALS.md` never holds +/// secret/PII-bearing text, regardless of how the caller built `doc`. pub fn save(workspace: &Path, doc: &mut GoalsDoc) -> MemoryEngineResult<()> { let path = validate_within_workspace(workspace)?; + for item in &doc.items { + if has_goal_secret(&item.text) || has_goal_pii(&item.text) { + return Err(MemoryError::Invalid(format!( + "goal '{}' must not contain secrets or PII", + item.id + ))); + } + } let _dropped = enforce_caps(doc); if let Some(parent) = path.parent() { diff --git a/src/memory/goals/store_tests.rs b/src/memory/goals/store_tests.rs index f628711..5bce1bd 100644 --- a/src/memory/goals/store_tests.rs +++ b/src/memory/goals/store_tests.rs @@ -59,6 +59,27 @@ fn save_enforces_byte_cap() { assert!(doc.render().len() <= GOALS_FILE_MAX_CHARS + big.len()); } +#[test] +fn save_rejects_secret_or_pii_text_even_when_items_are_built_directly() { + // `GoalsDoc.items` and `GoalItem.text` are public: a caller can bypass + // `GoalsDocMutations::add`/`edit`'s validation entirely by constructing a + // document by hand. `save` must be the choke point that still refuses to + // persist secret/PII-bearing text. + let tmp = tempfile::tempdir().unwrap(); + let mut doc = GoalsDoc { + items: vec![crate::memory::goals::types::GoalItem::new( + "g1", + "rotate api_key=sk-abcdefghijklmnopqrstuvwxyz123456", + )], + }; + let err = save(tmp.path(), &mut doc).unwrap_err(); + assert!(matches!(err, MemoryError::Invalid(_))); + + // Nothing was written. + let reloaded = load(tmp.path()).unwrap(); + assert!(reloaded.is_empty()); +} + #[test] fn config_rooted_wrappers_round_trip() { use crate::memory::config::MemoryConfig; From d7fb44a93992277546b2a70ce29cf41805430122 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 4 Aug 2026 20:27:27 +0300 Subject: [PATCH 18/19] fix(api): reserve a method added to an existing family for a major bump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit is_compatible() only gated on the major half, on the premise that a minor bump is safe because capability negotiation covers the delta. That premise breaks for one case the docs themselves called out as minor-safe: a new method added to a family a driver may already advertise. Negotiation has family granularity, not method granularity — there is no way to advertise 'Core, but without the new method' — so an older driver still advertising Core could be called into a method it never implemented. Reserve that case for a major bump instead of building method-level negotiation or per-endpoint minor gating, keeping the contract's negotiation surface at family granularity throughout. is_compatible's logic and CONTRACT_VERSION are unchanged; only the classification of what counts as minor-safe moves. Adds a test pinning the rule in prose so it can't be re-derived from the code alone. Co-authored-by: Medulla --- api/src/version.rs | 38 +++++++++++++++++++++++++++----------- api/src/version_tests.rs | 20 ++++++++++++++++++++ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/api/src/version.rs b/api/src/version.rs index 281482d..2ccf329 100644 --- a/api/src/version.rs +++ b/api/src/version.rs @@ -7,13 +7,25 @@ //! //! `CONTRACT_VERSION` is `(major, minor)`: //! -//! - **Minor bump — a capability was added.** A new [`crate::capabilities::Capability`] -//! family, or a new method inside an existing family that older drivers may -//! simply not implement. Nothing that already compiled stops compiling. -//! - **Major bump — an existing signature changed.** A method's parameters or -//! return type moved, a mandatory family was added or removed, or a wire -//! string changed. Anything an existing driver already implements now means -//! something different. +//! - **Minor bump — an addition that capability negotiation alone makes safe.** +//! A new [`crate::capabilities::Capability`] family is the canonical case: an +//! older driver simply never advertises it, the corresponding RPC methods are +//! unregistered, and the kernel never calls in. A new optional field on an +//! existing wire type, or a new error variant an older kernel can treat as +//! opaque, are the same shape — nothing that already compiled stops +//! compiling, and there is no way for an old driver to be asked for +//! something it never claimed to support. +//! - **Major bump — an existing signature changed, OR a method was added to an +//! already-advertised family.** A method's parameters or return type moved, a +//! mandatory family was added or removed, a wire string changed — or a driver +//! advertising an existing family (say [`crate::capabilities::Capability::Core`]) +//! now has to implement one more method on it. That last case looks additive +//! but is not: capability negotiation has **family granularity only** — there +//! is no way to advertise "`Core`, but without the new method" — so an older +//! driver that still advertises `Core` can be called into a method it does +//! not implement. Bump the major half instead, which forces every driver +//! claiming that family to actually implement the new surface before it can +//! bind again. //! //! ## Why only the major half gates the bind //! @@ -40,10 +52,14 @@ /// Version of the memory contract this crate defines, as `(major, minor)`. /// -/// See the module docs for the bump rule. Bump the **minor** half when a -/// capability family or an additive method lands; bump the **major** half — and -/// reset the minor to `0` — when an existing signature, mandatory family, or -/// wire string changes. +/// See the module docs for the bump rule. Bump the **minor** half only for an +/// addition capability negotiation alone makes safe — a new capability family, +/// a new optional wire field, a new opaque-to-old-kernels error variant. Bump +/// the **major** half — and reset the minor to `0` — for an existing signature +/// change, a mandatory family change, a wire string change, **or a new method +/// added to a family a driver may already advertise** (negotiation is +/// family-granular, not method-granular, so that case cannot be made minor-safe +/// by negotiation alone). pub const CONTRACT_VERSION: (u16, u16) = (1, 0); /// Whether a driver speaking `remote` can be bound against this build. diff --git a/api/src/version_tests.rs b/api/src/version_tests.rs index b9819fb..1b3a4c0 100644 --- a/api/src/version_tests.rs +++ b/api/src/version_tests.rs @@ -47,6 +47,26 @@ fn a_major_mismatch_refuses_the_bind() { assert!(!is_compatible((0, 0))); } +#[test] +fn adding_a_method_to_an_already_advertised_family_requires_a_major_bump() { + // Capability negotiation has family granularity, not method granularity: + // there is no way to advertise "Core, but without the new method". So a + // method added to a family a driver may already advertise (e.g. Core, + // Recall) cannot be made minor-safe by negotiation the way a brand-new + // capability family can — an older driver still advertising that family + // would be called into a method it never implemented. This is why the + // module docs classify that addition as a MAJOR bump, not minor, even + // though it looks additive. This test exists so the rule cannot be + // re-derived from `is_compatible`'s code alone, which only encodes "major + // halves must match" and says nothing about *why* a same-family method + // addition belongs on the major side of that line. + assert!( + !is_compatible((CONTRACT_VERSION.0 + 1, 0)), + "a method added to an existing family must ship as a major bump, \ + which this asserts refuses the bind against an old build" + ); +} + #[test] fn compatibility_depends_only_on_the_major_half() { let (major, _) = CONTRACT_VERSION; From 545fefb8ddfc99e71da005ac71634b11ce892ae6 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 4 Aug 2026 20:32:37 +0300 Subject: [PATCH 19/19] fix(memory): validate all goal-text invariants in save, not just secret/PII The prior fix to save() only re-ran the secret/PII guard, leaving the same public-field bypass open for the other two GoalsDocMutations invariants: non-empty and single-line text. Switched to validate_goal_text, which checks all three, so a directly-constructed GoalsDoc can no longer persist empty or multi-line goal text either. Adds regression tests for both cases. Co-authored-by: Medulla --- src/memory/goals/store.rs | 20 +++++++++++--------- src/memory/goals/store_tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/memory/goals/store.rs b/src/memory/goals/store.rs index 1cab3c7..a96bd04 100644 --- a/src/memory/goals/store.rs +++ b/src/memory/goals/store.rs @@ -239,19 +239,21 @@ fn enforce_caps(doc: &mut GoalsDoc) -> Vec { /// /// `GoalsDoc.items` and `GoalItem.text` are public, so a caller can build a /// document directly and hand it to `save` without going through -/// [`GoalsDocMutations::add`]/[`GoalsDocMutations::edit`] — bypassing their -/// secret/PII validation. Re-validate every item's text here so `save` is the -/// one choke point that guarantees `MEMORY_GOALS.md` never holds -/// secret/PII-bearing text, regardless of how the caller built `doc`. +/// [`GoalsDocMutations::add`]/[`GoalsDocMutations::edit`] — bypassing all of +/// their validation, not just the secret/PII guard. Re-run +/// `validate_goal_text` over every item's text here so `save` is the one +/// choke point that guarantees `MEMORY_GOALS.md` never holds empty, +/// multi-line, or secret/PII-bearing text, regardless of how the caller built +/// `doc`. pub fn save(workspace: &Path, doc: &mut GoalsDoc) -> MemoryEngineResult<()> { let path = validate_within_workspace(workspace)?; for item in &doc.items { - if has_goal_secret(&item.text) || has_goal_pii(&item.text) { - return Err(MemoryError::Invalid(format!( - "goal '{}' must not contain secrets or PII", + validate_goal_text(&item.text).map_err(|_| { + MemoryError::Invalid(format!( + "goal '{}' text must be a non-empty, single-line, secret/PII-free string", item.id - ))); - } + )) + })?; } let _dropped = enforce_caps(doc); diff --git a/src/memory/goals/store_tests.rs b/src/memory/goals/store_tests.rs index 5bce1bd..67c1d26 100644 --- a/src/memory/goals/store_tests.rs +++ b/src/memory/goals/store_tests.rs @@ -80,6 +80,38 @@ fn save_rejects_secret_or_pii_text_even_when_items_are_built_directly() { assert!(reloaded.is_empty()); } +#[test] +fn save_rejects_empty_or_multiline_text_even_when_items_are_built_directly() { + // Same bypass as the secret/PII case above, but for the other two + // `validate_goal_text` invariants: non-empty and single-line. `GoalItem`'s + // fields are public, so a caller can set `text` to anything, including + // values `GoalsDocMutations::add`/`edit` would never have accepted. + use crate::memory::goals::types::GoalItem; + + let tmp = tempfile::tempdir().unwrap(); + let mut empty_doc = GoalsDoc { + items: vec![GoalItem { + id: "g1".to_string(), + text: " ".to_string(), + }], + }; + let err = save(tmp.path(), &mut empty_doc).unwrap_err(); + assert!(matches!(err, MemoryError::Invalid(_))); + + let mut multiline_doc = GoalsDoc { + items: vec![GoalItem { + id: "g1".to_string(), + text: "line one\n- [x] injected".to_string(), + }], + }; + let err = save(tmp.path(), &mut multiline_doc).unwrap_err(); + assert!(matches!(err, MemoryError::Invalid(_))); + + // Neither call wrote anything to disk. + let reloaded = load(tmp.path()).unwrap(); + assert!(reloaded.is_empty()); +} + #[test] fn config_rooted_wrappers_round_trip() { use crate::memory::config::MemoryConfig;