diff --git a/crates/tinymemory-core/src/sources/registry.rs b/crates/tinymemory-core/src/sources/registry.rs index 4d0151e..93f8f0a 100644 --- a/crates/tinymemory-core/src/sources/registry.rs +++ b/crates/tinymemory-core/src/sources/registry.rs @@ -77,6 +77,41 @@ pub fn get_source_in( .map_err(|error| error.to_string()) } +/// [`list_sources`] against an **explicit** config — the same reasoning as +/// [`get_source_in`]: a driver bound to one workspace must read that +/// workspace's registry file, not whatever the process environment names. +/// +/// Synchronous for the same reason; this is what lets a host-config view +/// answer `memory_sources_json` from the file the host writes rather than +/// from a load-time snapshot (openhuman#5820). +/// +/// # Errors +/// +/// Returns `Err` with the registry's error message, stringified, when the +/// file cannot be read or parsed as `[[memory_sources]]`. +pub fn list_sources_in(config: &crate::Config) -> Result, String> { + registry_in(config) + .list() + .map_err(|error| error.to_string()) +} + +/// Replace the registry file an **explicit** config names with `entries` — +/// the write half of [`list_sources_in`], so a host-config view that reads +/// live can also write through (openhuman#5820). +/// +/// # Errors +/// +/// Returns `Err` with the registry's error message, stringified, when an +/// entry fails validation or the file cannot be written atomically. +pub fn replace_sources_in( + config: &crate::Config, + entries: &[MemorySourceEntry], +) -> Result<(), String> { + registry_in(config) + .replace_all(entries) + .map_err(|error| error.to_string()) +} + pub async fn add_source(entry: MemorySourceEntry) -> Result { let _guard = memory_sources_write_guard().await; log::debug!("[memory_sources] crate add kind={}", entry.kind.as_str()); diff --git a/crates/tinymemory-module/src/config.rs b/crates/tinymemory-module/src/config.rs index 0009836..25ae7e8 100644 --- a/crates/tinymemory-module/src/config.rs +++ b/crates/tinymemory-module/src/config.rs @@ -62,6 +62,20 @@ pub struct ModuleConfig { /// memory store somewhere nobody would look for it. pub workspace_dir: PathBuf, + /// Path of the host's `config.toml` — the file that holds the + /// `[[memory_sources]]` registry the host writes. + /// + /// The engine's source registry is a TOML file, and the host and this + /// module must read and write the *same one*: `get_source_in` looks a + /// source up by id in that file, so a module reading a different path + /// answers `NotFound` for every source the host registered + /// (openhuman#5820, the "no memory source registered as src_…" strand). + /// A host too old to send this field deserializes as `None`, and + /// `provider::host_config_path` then falls back to the host's documented + /// layout — `config.toml` beside the `workspace/` directory — before the + /// historical (and wrong) `workspace_dir/config.toml`. + pub config_path: Option, + /// The engine's own configuration, passed through unchanged. pub memory: MemoryConfig, @@ -214,6 +228,7 @@ impl Default for ModuleConfig { fn default() -> Self { Self { workspace_dir: PathBuf::new(), + config_path: None, memory: MemoryConfig::default(), memory_tree: MemoryTreeConfig::default(), scheduler_gate: SchedulerGateConfig::default(), diff --git a/crates/tinymemory-module/src/config_test.rs b/crates/tinymemory-module/src/config_test.rs index e74cbf0..249fa79 100644 --- a/crates/tinymemory-module/src/config_test.rs +++ b/crates/tinymemory-module/src/config_test.rs @@ -231,3 +231,23 @@ fn a_populated_config_round_trips() { assert_eq!(back.models_supporting_dimensions, vec!["m".to_string()]); assert_eq!(back.driver_id, "tinycortex"); } + +/// A host that predates `config_path` still deserializes, with the field +/// absent rather than invented; a host that sends it is honoured verbatim. +#[test] +fn config_path_is_optional_on_the_wire_and_honoured_when_sent() { + let old_host: ModuleConfig = + serde_json::from_value(serde_json::json!({ "workspace_dir": "/w/workspace" })) + .expect("an older host's payload still deserializes"); + assert!(old_host.config_path.is_none()); + + let new_host: ModuleConfig = serde_json::from_value(serde_json::json!({ + "workspace_dir": "/w/workspace", + "config_path": "/w/config.toml", + })) + .expect("a host that sends config_path deserializes"); + assert_eq!( + new_host.config_path.as_deref(), + Some(std::path::Path::new("/w/config.toml")) + ); +} diff --git a/crates/tinymemory-module/src/provider.rs b/crates/tinymemory-module/src/provider.rs index 4aeb437..c8fb3e0 100644 --- a/crates/tinymemory-module/src/provider.rs +++ b/crates/tinymemory-module/src/provider.rs @@ -16,7 +16,7 @@ impl From<&ModuleConfig> for EngineRuntimeConfig { fn from(config: &ModuleConfig) -> Self { Self { workspace_dir: config.workspace_dir.clone(), - config_path: config.workspace_dir.join("config.toml"), + config_path: host_config_path(config), memory: config.memory.clone(), memory_tree: config.memory_tree.clone(), scheduler_gate: config.scheduler_gate.clone(), @@ -39,6 +39,41 @@ impl From<&ModuleConfig> for EngineRuntimeConfig { } } +/// The `config.toml` the host's source registry lives in. +/// +/// Three answers, in order of trust: +/// +/// 1. What the host sent (`ModuleConfig::config_path`) — the file it writes. +/// 2. For a host too old to send it: `config.toml` beside the `workspace/` +/// directory, which is the layout every OpenHuman profile has +/// (`/config.toml` next to `/workspace`), taken only when that +/// file actually exists. +/// 3. The historical `workspace_dir/config.toml`, kept so a host with neither +/// behaves exactly as before rather than failing to build a config. +/// +/// The second and third are fallbacks for old hosts only; the first is the +/// contract. Reading any file other than the host's is what made every +/// host-registered source answer `NotFound` on sync (openhuman#5820). +pub(crate) fn host_config_path(config: &ModuleConfig) -> std::path::PathBuf { + if let Some(path) = &config.config_path { + return path.clone(); + } + if let Some(beside_workspace) = config + .workspace_dir + .parent() + .map(|root| root.join("config.toml")) + .filter(|candidate| candidate.is_file()) + { + log::warn!( + "[tinymemory:module] host sent no config_path; using the registry file beside \ + the workspace at {}", + beside_workspace.display() + ); + return beside_workspace; + } + config.workspace_dir.join("config.toml") +} + /// Builds the engine provider this module serves over the bus. pub(crate) fn provider(config: &ModuleConfig, client: Arc) -> TinycortexProvider { TinycortexProvider::new( @@ -47,3 +82,7 @@ pub(crate) fn provider(config: &ModuleConfig, client: Arc) -> Tiny client, ) } + +#[cfg(test)] +#[path = "provider_test.rs"] +mod test; diff --git a/crates/tinymemory-module/src/provider_test.rs b/crates/tinymemory-module/src/provider_test.rs new file mode 100644 index 0000000..7a7e255 --- /dev/null +++ b/crates/tinymemory-module/src/provider_test.rs @@ -0,0 +1,53 @@ +//! Tests for the surrounding module: the config-path resolution the engine +//! provider is built from (openhuman#5820). + +use super::host_config_path; +use crate::ModuleConfig; + +fn config_with( + workspace_dir: &std::path::Path, + config_path: Option, +) -> ModuleConfig { + let mut config: ModuleConfig = + serde_json::from_value(serde_json::json!({ "workspace_dir": workspace_dir })) + .expect("a workspace alone deserializes"); + config.config_path = config_path; + config +} + +/// What the host sends wins, whether or not the file exists yet — the host +/// is about to write it. +#[test] +fn an_explicit_host_path_is_taken_verbatim() { + let config = config_with( + std::path::Path::new("/w/workspace"), + Some("/elsewhere/config.toml".into()), + ); + assert_eq!( + host_config_path(&config), + std::path::PathBuf::from("/elsewhere/config.toml") + ); +} + +/// An older host sends nothing; the registry file beside `workspace/` is the +/// documented layout, so it is used when it exists. +#[test] +fn an_old_host_falls_back_to_the_file_beside_the_workspace() { + let root = tempfile::tempdir().expect("tempdir"); + let workspace = root.path().join("workspace"); + std::fs::create_dir_all(&workspace).unwrap(); + std::fs::write(root.path().join("config.toml"), "[[memory_sources]]\n").unwrap(); + + let config = config_with(&workspace, None); + assert_eq!(host_config_path(&config), root.path().join("config.toml")); +} + +/// With neither, the historical path stands — behaviour unchanged for a host +/// that never had a registry file at all. +#[test] +fn with_no_candidate_the_historical_path_is_kept() { + let root = tempfile::tempdir().expect("tempdir"); + let workspace = root.path().join("workspace"); + let config = config_with(&workspace, None); + assert_eq!(host_config_path(&config), workspace.join("config.toml")); +} diff --git a/crates/tinymemory-sources/src/registry.rs b/crates/tinymemory-sources/src/registry.rs index 88a6a7f..3e388d0 100644 --- a/crates/tinymemory-sources/src/registry.rs +++ b/crates/tinymemory-sources/src/registry.rs @@ -354,6 +354,28 @@ impl SourceRegistry { Ok(targets.len().min(u32::MAX as usize) as u32) } + /// Replace the whole registry with `entries`, validating each first. + /// + /// The write-through behind a host-config view whose `memory_sources_json` + /// reads this file: a setter that only updated an in-memory snapshot would + /// be invisible to the very next getter (openhuman#5820). Same atomic + /// load-modify-validate-save cycle as the other mutations, so other + /// top-level keys in the file are preserved. + /// + /// # Errors + /// + /// Returns an error when an entry fails validation, or when the file + /// cannot be read, parsed, serialized or atomically replaced. + pub fn replace_all(&self, entries: &[MemorySourceEntry]) -> Result<()> { + let _guard = mutation_guard(); + for entry in entries { + entry + .validate() + .map_err(|reason| anyhow!("invalid memory source `{}`: {reason}", entry.id))?; + } + self.write_all(entries) + } + /// Enable every source and clear all per-source caps ("All In" mode). pub fn apply_all_in(&self) -> Result> { let _guard = mutation_guard(); diff --git a/crates/tinymemory-tinycortex/src/engine/mod.rs b/crates/tinymemory-tinycortex/src/engine/mod.rs index 744bd45..7b916e4 100644 --- a/crates/tinymemory-tinycortex/src/engine/mod.rs +++ b/crates/tinymemory-tinycortex/src/engine/mod.rs @@ -268,9 +268,35 @@ impl MemoryHostConfig for EngineRuntimeConfig { } } fn memory_sources_json(&self) -> anyhow::Result { + // The host's registry file is the source of truth and it is live: a + // source the user adds after this module loaded is in that file and + // not in the load-time snapshot. Read the file when there is one; + // the snapshot stays the answer for a host that never wrote a + // registry, and for a read that fails (openhuman#5820). + if self.config_path.is_file() { + match tinymemory_core::sources::registry::list_sources_in(self) { + Ok(sources) => return Ok(serde_json::to_value(sources)?), + Err(error) => log::warn!( + "[tinycortex:engine] could not read the source registry at {}; \ + answering from the load-time snapshot: {error}", + self.config_path.display() + ), + } + } Ok(self.memory_sources.clone()) } fn set_memory_sources_json(&mut self, value: serde_json::Value) -> anyhow::Result<()> { + // Write through to the host's registry file when there is one, so the + // next `memory_sources_json` (a live read) sees this update and it + // survives the process; `save` on this config is a no-op, so this is + // the persistence step. The snapshot is kept in step for a config + // with no file (openhuman#5820). + if self.config_path.is_file() { + let entries: Vec = + serde_json::from_value(value.clone())?; + tinymemory_core::sources::registry::replace_sources_in(self, &entries) + .map_err(|error| anyhow::anyhow!("write memory sources: {error}"))?; + } self.memory_sources = value; Ok(()) } diff --git a/crates/tinymemory-tinycortex/src/engine/test.rs b/crates/tinymemory-tinycortex/src/engine/test.rs index 7d26441..e508bea 100644 --- a/crates/tinymemory-tinycortex/src/engine/test.rs +++ b/crates/tinymemory-tinycortex/src/engine/test.rs @@ -427,3 +427,97 @@ fn the_two_new_families_are_advertised_by_the_full_engine() { assert!(caps.contains(Capability::SourceSync)); assert!(caps.contains(Capability::CodingSessions)); } + +/// `memory_sources_json` answers from the host's registry file when it +/// exists — a source added after load is visible — and from the load-time +/// snapshot only when there is no file (openhuman#5820). +#[test] +fn memory_sources_json_reads_the_live_registry_file_when_present() { + use tinymemory_api::host::MemoryHostConfig; + + let root = tempfile::tempdir().expect("tempdir"); + let config_path = root.path().join("config.toml"); + let snapshot = serde_json::json!([ + { "id": "src_snapshot", "kind": "folder", "label": "old", "path": "." } + ]); + + let mut config = runtime_config(); + config.workspace_dir = root.path().join("workspace"); + config.config_path = config_path.clone(); + config.memory_sources = snapshot.clone(); + + // No file yet: the snapshot is the only answer. + assert_eq!( + config.memory_sources_json().expect("snapshot answer"), + snapshot + ); + + // The host writes a registry entry after load; the live read sees it. + std::fs::write( + &config_path, + "[[memory_sources]]\nid = \"src_live\"\nkind = \"folder\"\nlabel = \"new\"\npath = \".\"\n", + ) + .expect("write the registry file"); + let live = config.memory_sources_json().expect("live answer"); + let ids: Vec<&str> = live + .as_array() + .expect("a JSON array of sources") + .iter() + .filter_map(|entry| entry.get("id").and_then(|id| id.as_str())) + .collect(); + assert_eq!( + ids, + vec!["src_live"], + "the file, not the snapshot, is the registry" + ); +} + +/// With a registry file present, `set_memory_sources_json` writes through: +/// the next (live) getter returns the new entries and the file holds them +/// (openhuman#5820, review follow-up). Without a file, the snapshot is +/// updated and read back as before. +#[test] +fn set_memory_sources_json_writes_through_to_the_registry_file() { + use tinymemory_api::host::MemoryHostConfig; + + let root = tempfile::tempdir().expect("tempdir"); + let config_path = root.path().join("config.toml"); + std::fs::write(&config_path, "other_key = 1\n").expect("seed the config file"); + + let mut config = runtime_config(); + config.workspace_dir = root.path().join("workspace"); + config.config_path = config_path.clone(); + + let entries = serde_json::json!([ + { "id": "src_written", "kind": "folder", "label": "written", "path": "." } + ]); + config + .set_memory_sources_json(entries.clone()) + .expect("the setter writes through"); + + // The live getter sees the update... + let live = config.memory_sources_json().expect("live answer"); + let ids: Vec<&str> = live + .as_array() + .expect("a JSON array of sources") + .iter() + .filter_map(|entry| entry.get("id").and_then(|id| id.as_str())) + .collect(); + assert_eq!(ids, vec!["src_written"]); + + // ...it is on disk, and the file's other keys survived the write. + let on_disk = std::fs::read_to_string(&config_path).expect("read the config file"); + assert!(on_disk.contains("src_written"), "{on_disk}"); + assert!(on_disk.contains("other_key = 1"), "{on_disk}"); + + // No file: the snapshot is the store. + let mut snapshot_only = runtime_config(); + snapshot_only.config_path = root.path().join("missing").join("config.toml"); + snapshot_only + .set_memory_sources_json(entries.clone()) + .expect("snapshot-only setter"); + assert_eq!( + snapshot_only.memory_sources_json().expect("snapshot"), + entries + ); +}