diff --git a/Cargo.lock b/Cargo.lock index 0c5ca9c..f3cec9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1788,15 +1788,7 @@ dependencies = [ name = "tinycortex-api" version = "0.1.1" dependencies = [ - "anyhow", - "async-trait", - "chrono", - "serde", - "serde_json", - "sha2 0.10.9", - "thiserror 2.0.20", "tinymemory-api", - "uuid", ] [[package]] @@ -1868,6 +1860,7 @@ dependencies = [ "tinycortex-api", "tinymemory", "tinymemory-api", + "tinymemory-conformance", "tinymemory-sync", "tokio", "tracing", diff --git a/core/Cargo.toml b/core/Cargo.toml index d876807..c667795 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -70,6 +70,10 @@ walkdir = "2" # `TestHostConfig` — the concrete `MemoryHostConfig` the extracted test suites # build, since `Config` is a trait object and cannot be `Default`ed. tinymemory-api = { path = "../api", features = ["test-support"] } +# The driver conformance suite (#18 §E1). A dev-dependency only: it exists to +# hold this crate's own store to the same contract the adapters are held to. +# No cycle — `tinymemory-conformance` depends on `tinymemory-api` alone. +tinymemory-conformance = { path = "../conformance" } tempfile = "3" tokio = { version = "1", features = ["test-util"] } diff --git a/core/src/store/factories.rs b/core/src/store/factories.rs index e230136..0e22dee 100644 --- a/core/src/store/factories.rs +++ b/core/src/store/factories.rs @@ -353,6 +353,71 @@ pub fn create_memory( create_memory_full(config, &[], None, None, "", workspace_dir) } +/// Bind this crate's own store as a [`MemoryProvider`], the way every other +/// engine in the workspace is bound. +/// +/// Issue #18 §A3/§A5. Until this existed, `tinymemory-core`'s +/// [`UnifiedMemory`] was the one storage +/// implementation in the workspace with no route through the contract: the +/// TinyCortex adapter binds the bundled engine, `adapters/remote` binds the +/// three hosted services, and core's own SQLite store was reachable only by +/// naming its concrete type. A host could therefore not treat "the store +/// tinymemory ships with" as a driver, which is the whole premise of the +/// registry — and `create_memory` returning `Box` is exactly the +/// bypass §A3 names. +/// +/// Nothing structural was missing, which is worth recording because it was +/// mis-diagnosed at one point as needing the crate split: `UnifiedMemory` +/// already implements [`Memory`], and +/// [`MemoryTraitProvider`] already wraps any `Memory` into a provider. This is +/// the one-line composition the adapters have been doing all along. +/// +/// # Capabilities +/// +/// The returned provider advertises the mandatory three — Core, Recall, +/// Portability — and nothing else, because that is what [`Memory`] can express. +/// `UnifiedMemory` implements more than that internally (trees, chunks, +/// entities), but those reach the caller through their own concrete APIs rather +/// than through an optional family accessor, so advertising them here would be +/// a claim `audit_provider` correctly rejects. Widening that is §C3's shape of +/// work, not this function's. +/// +/// # Errors +/// +/// Propagates whatever [`create_memory`] fails with — a store that cannot open +/// cannot be bound. +/// +/// [`MemoryProvider`]: tinymemory_api::provider::MemoryProvider +/// [`Memory`]: tinymemory_api::traits::Memory +/// [`MemoryTraitProvider`]: tinymemory::mandatory::MemoryTraitProvider +pub fn create_memory_provider( + config: &MemoryConfig, + workspace_dir: &Path, +) -> anyhow::Result> { + let memory = create_memory(config, workspace_dir)?; + Ok(bind_as_provider(memory)) +} + +/// Wraps an already-built store as a driver under [`NAMESPACE_DRIVER_ID`]. +/// +/// Split out from [`create_memory_provider`] so a caller that already holds a +/// store — the migration path, tests, a host that built one through +/// [`create_memory_with_local_ai`] — can bind it without constructing a second +/// one. Constructing twice against one directory is the hazard the host-side +/// bypass allowlists exist to refuse, so the seam that avoids it belongs here +/// rather than at each call site. +/// +/// [`NAMESPACE_DRIVER_ID`]: tinymemory::registry::NAMESPACE_DRIVER_ID +#[must_use] +pub fn bind_as_provider( + memory: Box, +) -> Arc { + Arc::new(tinymemory::mandatory::MemoryTraitProvider::new( + Arc::from(memory), + tinymemory::registry::NAMESPACE_DRIVER_ID, + )) +} + /// Create a memory instance honouring the unified per-workload embedding /// provider. /// diff --git a/core/src/store/factories_provider_test.rs b/core/src/store/factories_provider_test.rs new file mode 100644 index 0000000..04c84be --- /dev/null +++ b/core/src/store/factories_provider_test.rs @@ -0,0 +1,85 @@ +//! `tinymemory-core`'s own store, held to the driver contract (#18 §A3/§E1). +//! +//! The TinyCortex adapter and the three hosted adapters each have a +//! `conformance_test.rs` asserting they uphold `MemoryProvider`. This crate's +//! own store had no such file, because until `create_memory_provider` there was +//! no way to express it as a driver at all — which is precisely the gap §A3 +//! describes. These are the missing equivalents. + +// `expect` is the assertion mechanism here, and its message is the failure +// diagnostic — `expect_used` is `warn` workspace-wide (Cargo.toml) and CI runs +// clippy with `-D warnings`. Scoped to that one lint: unlike the sibling +// conformance tests this file has no explicit `panic!`, so it does not need +// `clippy::panic` too. +#![allow(clippy::expect_used)] + +use std::sync::Arc; + +use tinymemory_api::host::MemoryConfig; +use tinymemory_api::provider::{audit_provider, MemoryProvider}; + +use super::factories::create_memory_provider; + +/// Builds a provider over a real store in a throwaway directory. +/// +/// A temp dir rather than an in-memory backend on purpose: `UnifiedMemory` is a +/// SQLite store, and a driver that only ever answered from memory would not be +/// the thing hosts actually bind. +fn provider(dir: &std::path::Path) -> Arc { + // The store resolves its embedder through the process-global `EmbeddingHost` + // and refuses to open without one. `init` is the crate's idempotent stub + // installer, so this is the same seam every other core test uses rather + // than a second setup path. + crate::test_seams::init(); + create_memory_provider(&MemoryConfig::default(), dir).expect("the bundled store opens") +} + +#[tokio::test] +async fn the_core_store_upholds_the_contract() { + let dir = tempfile::tempdir().expect("temp dir"); + tinymemory_conformance::assert_provider(provider(dir.path())).await; +} + +#[tokio::test] +async fn the_core_store_actually_retains() { + // The conformance suite tolerates a driver that refuses a write; without + // this probe a store that silently retained nothing could pass it + // vacuously. That is not hypothetical — it is how a broken double slipped + // through review once already. + let dir = tempfile::tempdir().expect("temp dir"); + assert!( + tinymemory_conformance::retains_writes(provider(dir.path()).as_ref()).await, + "the bundled store reported success and kept nothing" + ); +} + +#[tokio::test] +async fn it_binds_under_the_reserved_namespace_id() { + let dir = tempfile::tempdir().expect("temp dir"); + assert_eq!( + provider(dir.path()).driver_id(), + tinymemory::registry::NAMESPACE_DRIVER_ID, + "the bundled store must not bind under another engine's id" + ); +} + +#[tokio::test] +async fn its_advertised_capabilities_match_what_it_exposes() { + // `audit_provider` is the honesty check: advertised families must equal + // reachable accessors. Wrapping through `MemoryTraitProvider` derives the + // advertisement from the accessors, so this should hold by construction — + // it runs because that construction lives in another crate. + let dir = tempfile::tempdir().expect("temp dir"); + audit_provider(provider(dir.path()).as_ref()).expect("the bundled store is honest"); +} + +#[tokio::test] +async fn the_registry_admits_it_as_an_embedded_driver() { + use tinymemory::registry::{DriverClass, DriverRegistry, NAMESPACE_DRIVER_ID}; + + // A reserved id with no admission path would be a driver nothing can bind. + let admitted = DriverRegistry::builtin() + .admit(NAMESPACE_DRIVER_ID, None, Default::default()) + .expect("the bundled store is admissible"); + assert_eq!(admitted.class, DriverClass::Embedded); +} diff --git a/core/src/store/memory_trait.rs b/core/src/store/memory_trait.rs index 4f4b3c5..0ad0d60 100644 --- a/core/src/store/memory_trait.rs +++ b/core/src/store/memory_trait.rs @@ -370,9 +370,14 @@ impl Memory for UnifiedMemory { let ns = UnifiedMemory::sanitize_namespace(namespace); let key = crate::store::safety::canonical_document_key(key); let conn = self.conn.lock(); - let row: Option<(String, String, String, f64, String, String)> = conn + // `session_id` is selected here for the same reason `list` selects it: + // it is a column on this row, and a `get` that dropped it made the two + // readers disagree about one record. The contract's round-trip + // assertion catches exactly that (`tinymemory_conformance`), and it was + // invisible until #18 §A3 let this store be bound as a driver at all. + let row: Option<(String, String, String, f64, String, String, Option)> = conn .query_row( - "SELECT document_id, key, content, updated_at, category, taint + "SELECT document_id, key, content, updated_at, category, taint, session_id FROM memory_docs WHERE namespace = ?1 AND key = ?2 LIMIT 1", params![ns, key], |row| { @@ -383,19 +388,20 @@ impl Memory for UnifiedMemory { row.get(3)?, row.get(4)?, row.get(5)?, + row.get(6)?, )) }, ) .optional()?; Ok(row.map( - |(id, key, content, updated_at, category, taint_str)| MemoryEntry { + |(id, key, content, updated_at, category, taint_str, session_id)| MemoryEntry { id, key, content, namespace: Some(ns.clone()), category: memory_category_from_stored(&category), timestamp: timestamp_to_rfc3339(updated_at), - session_id: None, + session_id, score: None, taint: crate::MemoryTaint::from_db_str(&taint_str), }, diff --git a/core/src/store/mod.rs b/core/src/store/mod.rs index bf78ba3..50580c5 100644 --- a/core/src/store/mod.rs +++ b/core/src/store/mod.rs @@ -39,6 +39,8 @@ pub mod types; mod client; pub mod factories; +#[cfg(test)] +mod factories_provider_test; /// Golden-workspace fixture seeding / read-back / schema-manifest capture. /// /// Public only so `tests/memory_golden_fixture_e2e.rs` can drive it; it needs diff --git a/src/registry/mod.rs b/src/registry/mod.rs index b5de045..c31cad2 100644 --- a/src/registry/mod.rs +++ b/src/registry/mod.rs @@ -61,6 +61,23 @@ pub use tinymemory_api::null::NULL_DRIVER_ID; /// still refuse to bind something *else* under this name. pub const TINYCORTEX_DRIVER_ID: &str = "tinycortex"; +/// The driver id of `tinymemory-core`'s own in-process store. +/// +/// Distinct from [`TINYCORTEX_DRIVER_ID`], and the distinction is the point: +/// that one names the bundled TinyCortex engine, this one names +/// `tinymemory_core::store::UnifiedMemory` — a separate SQLite store this +/// workspace implements itself and which, until now, no host could reach +/// through [`MemoryProvider`]. Both are `Embedded`; they are not the same +/// engine, and a host that binds one has not bound the other. +/// +/// Named for what `create_memory` has always called this backend +/// (`effective_memory_backend_name` returns `"namespace"`), so the id an +/// operator sees in status matches the name already in the logs rather than +/// introducing a third vocabulary for one store. +/// +/// [`MemoryProvider`]: tinymemory_api::provider::MemoryProvider +pub const NAMESPACE_DRIVER_ID: &str = "namespace"; + /// Driver id of the native Supermemory HTTP adapter. pub const SUPERMEMORY_DRIVER_ID: &str = "supermemory"; @@ -169,13 +186,15 @@ impl Default for DriverRegistry { } impl DriverRegistry { - /// The registry every host starts from: the null placeholder, TinyCortex, - /// and the three supported native HTTP engines. + /// The registry every host starts from: the null placeholder, the two + /// embedded engines — TinyCortex and this workspace's own `namespace` + /// store — and the three supported native HTTP engines. #[must_use] pub fn builtin() -> Self { let mut reserved = BTreeMap::new(); reserved.insert(NULL_DRIVER_ID.to_string(), DriverClass::Null); reserved.insert(TINYCORTEX_DRIVER_ID.to_string(), DriverClass::Embedded); + reserved.insert(NAMESPACE_DRIVER_ID.to_string(), DriverClass::Embedded); reserved.insert(SUPERMEMORY_DRIVER_ID.to_string(), DriverClass::External); reserved.insert(MEM0_DRIVER_ID.to_string(), DriverClass::External); reserved.insert(COGNEE_DRIVER_ID.to_string(), DriverClass::External);