diff --git a/Cargo.lock b/Cargo.lock index e880570247..5c79ecbfc4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -139,7 +139,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -150,7 +150,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -893,7 +893,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -1555,7 +1555,7 @@ dependencies = [ "libc", "option-ext", "redox_users 0.5.2", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -1805,7 +1805,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2743,7 +2743,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.6.3", + "socket2 0.5.10", "tokio", "tower-service", "tracing", @@ -3255,9 +3255,9 @@ checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" [[package]] name = "lettre" -version = "0.11.21" +version = "0.11.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dabda5859ee7c06b995b9d1165aa52c39110e079ef609db97178d86aeb051fa7" +checksum = "0da65617f6cb926332d039cb578aad56178da86e128db6a1b09f4c94fa5b3349" dependencies = [ "async-trait", "base64 0.22.1", @@ -3739,7 +3739,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -4164,6 +4164,7 @@ dependencies = [ "openfang-wire", "rand 0.8.5", "reqwest 0.12.28", + "rusqlite", "rustls", "serde", "serde_json", @@ -5105,7 +5106,7 @@ dependencies = [ "quinn-udp", "rustc-hash", "rustls", - "socket2 0.6.3", + "socket2 0.5.10", "thiserror 2.0.18", "tokio", "tracing", @@ -5143,7 +5144,7 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2 0.6.3", + "socket2 0.5.10", "tracing", "windows-sys 0.60.2", ] @@ -5709,7 +5710,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5768,7 +5769,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -6314,7 +6315,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -7050,7 +7051,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -7622,7 +7623,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e" dependencies = [ "memoffset", "tempfile", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -8495,7 +8496,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/crates/openfang-api/src/openai_compat.rs b/crates/openfang-api/src/openai_compat.rs index b87d6893f2..29a55c5a34 100644 --- a/crates/openfang-api/src/openai_compat.rs +++ b/crates/openfang-api/src/openai_compat.rs @@ -240,6 +240,7 @@ fn convert_messages(oai_messages: &[OaiMessage]) -> Vec { provider_msg_id: None, role, content, + source: None, }) }) .collect() diff --git a/crates/openfang-api/src/routes.rs b/crates/openfang-api/src/routes.rs index cebb1f599a..7bb7beecf5 100644 --- a/crates/openfang-api/src/routes.rs +++ b/crates/openfang-api/src/routes.rs @@ -319,6 +319,8 @@ pub fn inject_attachments_into_session( _ => openfang_memory::session::Session { id: entry.session_id, agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, diff --git a/crates/openfang-kernel/Cargo.toml b/crates/openfang-kernel/Cargo.toml index 851cbfa4d4..fcd175b9c2 100644 --- a/crates/openfang-kernel/Cargo.toml +++ b/crates/openfang-kernel/Cargo.toml @@ -44,3 +44,4 @@ libc = "0.2" [dev-dependencies] tokio-test = { workspace = true } tempfile = { workspace = true } +rusqlite = { workspace = true } diff --git a/crates/openfang-kernel/src/auth.rs b/crates/openfang-kernel/src/auth.rs index 6bc4a538c2..0707178e92 100644 --- a/crates/openfang-kernel/src/auth.rs +++ b/crates/openfang-kernel/src/auth.rs @@ -104,14 +104,52 @@ pub struct AuthManager { impl AuthManager { /// Create a new AuthManager from kernel user configuration. + /// + /// Thin wrapper over [`Self::new_with_default`] for callers that do not + /// have a persistent default-user UUID to bind. Each user gets a freshly + /// generated `UserId`. pub fn new(user_configs: &[UserConfig]) -> Self { + Self::new_with_default(user_configs, None) + } + + /// Like [`Self::new`] but accepts an explicit persistent default-user UUID. + /// + /// When `default_user_id` is `Some(uid)`, the config user marked + /// `is_default = true` (or, if none is marked, the first user in the list) + /// is registered under `uid` instead of a freshly-generated UUID. This + /// binds the user's display name and channel bindings to the kernel's + /// persistent default-session bucket, so unattributed traffic is + /// attributed to a real person rather than the nil-UUID anonymous bucket. + pub fn new_with_default(user_configs: &[UserConfig], default_user_id: Option) -> Self { let manager = Self { users: DashMap::new(), channel_index: DashMap::new(), }; - for config in user_configs { - let user_id = UserId::new(); + // Determine which config index claims the default user identity. + // Priority: an explicit `is_default = true` block, else the first + // user. We only need a `Some` here when the caller actually has a + // persistent UUID to bind, otherwise every user gets a fresh ID. + let default_index: Option = if default_user_id.is_some() { + user_configs + .iter() + .position(|c| c.is_default) + .or(if user_configs.is_empty() { + None + } else { + Some(0) + }) + } else { + None + }; + + for (idx, config) in user_configs.iter().enumerate() { + let user_id = if Some(idx) == default_index { + // SAFETY: default_index is only Some when default_user_id is Some. + default_user_id.unwrap() + } else { + UserId::new() + }; let role = UserRole::from_str_role(&config.role); let identity = UserIdentity { id: user_id, @@ -131,6 +169,7 @@ impl AuthManager { user = %config.name, role = %role, bindings = config.channel_bindings.len(), + default = (Some(idx) == default_index), "Registered user" ); } @@ -205,6 +244,7 @@ mod tests { m }, api_key_hash: None, + is_default: false, }, UserConfig { name: "Guest".to_string(), @@ -215,12 +255,14 @@ mod tests { m }, api_key_hash: None, + is_default: false, }, UserConfig { name: "ReadOnly".to_string(), role: "viewer".to_string(), channel_bindings: HashMap::new(), api_key_hash: None, + is_default: false, }, ] } @@ -304,6 +346,85 @@ mod tests { assert_eq!(manager.user_count(), 0); } + #[test] + fn test_new_with_default_binds_is_default_user() { + // When one [[users]] entry sets `is_default = true`, the manager must + // register that user under the persistent UUID (not a fresh one). + let default_uid = UserId::new(); + let configs = vec![ + UserConfig { + name: "Alice".to_string(), + role: "user".to_string(), + channel_bindings: HashMap::new(), + api_key_hash: None, + is_default: false, + }, + UserConfig { + name: "Owner".to_string(), + role: "owner".to_string(), + channel_bindings: HashMap::new(), + api_key_hash: None, + is_default: true, + }, + ]; + + let manager = AuthManager::new_with_default(&configs, Some(default_uid)); + // Owner's name resolves to the persistent UUID. + let owner = manager.get_user(default_uid).expect("owner registered"); + assert_eq!(owner.name, "Owner"); + assert_eq!(owner.role, UserRole::Owner); + } + + #[test] + fn test_new_with_default_falls_back_to_first_user() { + // When no entry sets `is_default`, the first user in the list inherits + // the persistent UUID. This matches the single-user-install default + // where there is exactly one `[[users]]` block. + let default_uid = UserId::new(); + let configs = vec![ + UserConfig { + name: "Primary".to_string(), + role: "owner".to_string(), + channel_bindings: HashMap::new(), + api_key_hash: None, + is_default: false, + }, + UserConfig { + name: "Secondary".to_string(), + role: "user".to_string(), + channel_bindings: HashMap::new(), + api_key_hash: None, + is_default: false, + }, + ]; + + let manager = AuthManager::new_with_default(&configs, Some(default_uid)); + let primary = manager.get_user(default_uid).expect("primary registered"); + assert_eq!(primary.name, "Primary"); + } + + #[test] + fn test_new_with_default_none_keeps_fresh_uuids() { + // Pass `None` (e.g. from a non-kernel test harness): no user is + // bound to the persistent UUID; each entry gets a fresh `UserId`. + let configs = test_configs(); + let manager = AuthManager::new_with_default(&configs, None); + assert_eq!(manager.user_count(), 3); + // None of the users can be looked up under the nil UUID by accident. + assert!(manager.get_user(UserId(uuid::Uuid::nil())).is_none()); + } + + #[test] + fn test_new_with_default_empty_config_is_noop() { + // Empty [[users]] config: the default UUID has nowhere to bind, so + // the manager comes up disabled. This matches a fresh install with + // no user config block. + let default_uid = UserId::new(); + let manager = AuthManager::new_with_default(&[], Some(default_uid)); + assert!(!manager.is_enabled()); + assert_eq!(manager.user_count(), 0); + } + #[test] fn test_role_parsing() { assert_eq!(UserRole::from_str_role("owner"), UserRole::Owner); diff --git a/crates/openfang-kernel/src/kernel.rs b/crates/openfang-kernel/src/kernel.rs index 8f59414c97..f520edeb88 100644 --- a/crates/openfang-kernel/src/kernel.rs +++ b/crates/openfang-kernel/src/kernel.rs @@ -650,6 +650,16 @@ impl OpenFangKernel { .map_err(|e| KernelError::BootFailed(format!("Memory init failed: {e}")))?, ); + // Bootstrap the persistent default user UUID. + // + // Replaces the legacy nil-UUID "anonymous bucket" so unattributed + // sessions get a stable owner. The UUID is generated once on first + // boot and persisted to the shared kv_store; subsequent boots reload + // it. On first boot after upgrading from a pre-v9 schema, any + // existing sessions with `user_id = nil UUID` are rewritten to the + // new default. + Self::bootstrap_default_user(&memory)?; + // Initialize credential resolver (vault → dotenv → env var) let credential_resolver = { let vault_path = config.home_dir.join("vault.enc"); @@ -816,8 +826,18 @@ impl OpenFangKernel { let wasm_sandbox = WasmSandbox::new() .map_err(|e| KernelError::BootFailed(format!("WASM sandbox init failed: {e}")))?; - // Initialize RBAC authentication manager - let auth = AuthManager::new(&config.users); + // Initialize RBAC authentication manager. + // + // The persistent default user (bootstrapped above) is bound to the + // `[[users]]` entry marked `is_default = true`, or to the first + // entry if none is marked. This keeps the default-user identity + // stable across reboots and lets `[[users]]` metadata (display + // name, channel bindings) attach to the persistent bucket rather + // than to a freshly-generated per-config UUID. + let auth = AuthManager::new_with_default( + &config.users, + Some(openfang_memory::session::default_user_id()), + ); if auth.is_enabled() { info!("RBAC enabled with {} users", auth.user_count()); } @@ -1611,9 +1631,11 @@ impl OpenFangKernel { // Create session — use the returned session_id so the registry // and database are in sync (fixes duplicate session bug #651). + // New sessions are tagged with the persistent default user; future + // PRs will surface explicit user selection through this path. let session = self .memory - .create_session(agent_id) + .create_session(agent_id, openfang_memory::session::default_user_id()) .map_err(KernelError::OpenFang)?; let session_id = session.id; @@ -2064,6 +2086,8 @@ impl OpenFangKernel { .unwrap_or_else(|| openfang_memory::session::Session { id: entry.session_id, agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -2650,6 +2674,8 @@ impl OpenFangKernel { .unwrap_or_else(|| openfang_memory::session::Session { id: entry.session_id, agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -3060,10 +3086,10 @@ impl OpenFangKernel { // Delete the old session let _ = self.memory.delete_session(entry.session_id); - // Create a fresh session + // Create a fresh session bound to the persistent default user. let new_session = self .memory - .create_session(agent_id) + .create_session(agent_id, openfang_memory::session::default_user_id()) .map_err(KernelError::OpenFang)?; // Update registry with new session ID @@ -3092,10 +3118,10 @@ impl OpenFangKernel { // Delete canonical (cross-channel) session let _ = self.memory.delete_canonical_session(agent_id); - // Create a fresh session + // Create a fresh session bound to the persistent default user. let new_session = self .memory - .create_session(agent_id) + .create_session(agent_id, openfang_memory::session::default_user_id()) .map_err(KernelError::OpenFang)?; // Update registry with new session ID @@ -3570,6 +3596,8 @@ impl OpenFangKernel { .unwrap_or_else(|| openfang_memory::session::Session { id: entry.session_id, agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -3652,6 +3680,8 @@ impl OpenFangKernel { .unwrap_or_else(|| openfang_memory::session::Session { id: entry.session_id, agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -6728,6 +6758,196 @@ impl OpenFangKernel { } } } + + /// Return the persistent default user UUID generated at boot. + pub fn default_user_id(&self) -> openfang_types::agent::UserId { + openfang_memory::session::default_user_id() + } + + /// Public, HTTP-boundary user-ID resolver. + /// + /// This is the strict, security-relevant variant: every reserved-bucket + /// value that an API-key holder could otherwise abuse to attribute writes + /// to is folded back to the persistent default user, with a `warn!` log: + /// + /// - `"default"` → persistent default user + /// - `"test"` → persistent default user (with `warn!` — deprecated alias, + /// external traffic no longer reaches the test bucket) + /// - the nil UUID (`00000000-…`) → persistent default user (with `warn!`) + /// - any other valid UUID → parsed as-is + /// - `None` or unparseable → `None` (callers fall back to the default + /// session selection) + /// + /// Use this for any caller that takes user-controlled input: HTTP route + /// handlers, message dispatchers, channel adapters. Tests and other + /// in-process callers that need to address the well-known test user UUID + /// must call [`Self::resolve_user_id_internal`] instead. + /// + /// # Why this is public in PR 1 + /// + /// No HTTP route consumes this in PR 1 — it is foundation API for PR 2, + /// which adds the `parse_user_id` route helper that delegates here. The + /// strict-filter behaviour is part of the security model and ships best + /// alongside the foundation; downgrading to private would mean + /// re-promoting in PR 2. + pub fn resolve_user_id( + &self, + user_id_str: Option<&str>, + ) -> Option { + let resolved = self.resolve_user_id_internal(user_id_str)?; + let default_user = openfang_memory::session::default_user_id(); + if resolved.0 == openfang_memory::session::TEST_USER_UUID { + warn!( + input = ?user_id_str, + "Rejected reserved \"test\" user alias on external API; mapped to default user" + ); + return Some(default_user); + } + if resolved.0.is_nil() { + warn!( + input = ?user_id_str, + "Rejected nil-UUID user on external API; mapped to default user" + ); + return Some(default_user); + } + Some(resolved) + } + + /// Internal, raw user-ID resolver. + /// + /// Honours `"test"` and the nil UUID literally (no folding), without the + /// security filter applied by [`Self::resolve_user_id`]: + /// + /// - `"test"` → the well-known test user UUID (isolated from + /// default-user memory; see [`openfang_memory::session::TEST_USER_UUID`]) + /// - `"default"` → persistent default user + /// - any valid UUID (including nil) → parsed as-is + /// - `None` or unparseable → `None` + /// + /// Use ONLY from test harnesses or trusted internal code paths. Any + /// caller taking user-controlled input must use the HTTP-boundary + /// [`Self::resolve_user_id`] instead. + /// + /// # Why this is public in PR 1 + /// + /// Same rationale as [`Self::resolve_user_id`]: this is foundation API + /// for the PR 2 route helpers. The internal/strict pair ships together + /// so PR 2 only has to wire the route helper through, without re-opening + /// visibility. + pub fn resolve_user_id_internal( + &self, + user_id_str: Option<&str>, + ) -> Option { + match user_id_str? { + "test" => Some(openfang_types::agent::UserId( + openfang_memory::session::TEST_USER_UUID, + )), + "default" => Some(openfang_memory::session::default_user_id()), + s => s + .parse::() + .ok() + .map(openfang_types::agent::UserId), + } + } + + /// Bootstrap the persistent default user UUID on kernel startup. + /// + /// On first boot: generate a new UUID, persist it under + /// `kv_store[shared_memory_agent_id, "default_user_uuid"]`, and rewrite + /// any legacy nil-UUID sessions to use the new default user. + /// + /// On subsequent boots: load the existing UUID. The rewrite has already + /// happened (sentinel `default_user_bootstrap_done` is set) so it is + /// skipped. + /// + /// The UUID is installed into `openfang_memory::session` via + /// `set_default_user_id`, so every call to `default_user_id()` from + /// anywhere in the codebase returns this persistent value. + fn bootstrap_default_user(memory: &Arc) -> KernelResult<()> { + const KEY_UUID: &str = "default_user_uuid"; + const KEY_DONE: &str = "default_user_bootstrap_done"; + let shared = shared_memory_agent_id(); + + // Load or generate the persistent UUID. + let user_id = match memory + .structured_get(shared, KEY_UUID) + .map_err(|e| KernelError::BootFailed(format!("kv read default_user_uuid: {e}")))? + { + Some(serde_json::Value::String(s)) => match uuid::Uuid::parse_str(&s) { + Ok(u) => { + info!(default_user = %u, "Loaded persistent default user UUID"); + openfang_types::agent::UserId(u) + } + Err(e) => { + warn!( + error = %e, + value = %s, + "Stored default_user_uuid is not a valid UUID — regenerating" + ); + let u = openfang_types::agent::UserId::new(); + memory + .structured_set( + shared, + KEY_UUID, + serde_json::Value::String(u.0.to_string()), + ) + .map_err(|e| { + KernelError::BootFailed(format!("kv write default_user_uuid: {e}")) + })?; + u + } + }, + _ => { + let u = openfang_types::agent::UserId::new(); + info!(default_user = %u.0, "Generated persistent default user UUID"); + memory + .structured_set(shared, KEY_UUID, serde_json::Value::String(u.0.to_string())) + .map_err(|e| { + KernelError::BootFailed(format!("kv write default_user_uuid: {e}")) + })?; + u + } + }; + + // Install for all callers of `default_user_id()`. + openfang_memory::session::set_default_user_id(user_id); + + // One-shot fixup: rewrite legacy nil-UUID sessions to the new default. + // + // The sentinel is written ONLY on success — if the rewrite fails + // (lock contention, disk error, etc.) the next boot will retry, + // rather than permanently leaving sessions stranded on the nil UUID. + let already_done = matches!( + memory.structured_get(shared, KEY_DONE).ok().flatten(), + Some(serde_json::Value::Bool(true)) + ); + if !already_done { + match memory.rewrite_nil_user_sessions(user_id) { + Ok(n) => { + if n > 0 { + info!( + sessions_updated = n, + "Rewrote nil-UUID sessions to persistent default user" + ); + } + // Mark the bootstrap done only after a clean rewrite — a + // failure path must NOT poison the sentinel, otherwise + // the retry would be silently suppressed on the next boot. + if let Err(e) = + memory.structured_set(shared, KEY_DONE, serde_json::Value::Bool(true)) + { + warn!(error = %e, "Failed to persist default_user_bootstrap_done sentinel"); + } + } + Err(e) => warn!( + error = %e, + "Failed to rewrite nil-UUID sessions; will retry on next boot" + ), + } + } + + Ok(()) + } } /// Convert a manifest's capability declarations into Capability enums. @@ -9412,4 +9632,231 @@ system_prompt = "You are a test agent." .expect("read pre-existing"); assert_eq!(contents, "hello", "must not overwrite user files"); } + + // ── PR 1: persistent default user + session user-tagging foundation ──────── + // + // These tests target the kernel-side bootstrap and the public/strict + // `resolve_user_id` filter. Each test boots a kernel against a temp dir, + // exercises the new APIs, and shuts down cleanly. + + fn minimal_kernel(tmp: &tempfile::TempDir) -> OpenFangKernel { + let home_dir = tmp.path().join("openfang"); + std::fs::create_dir_all(&home_dir).unwrap(); + let config = KernelConfig { + home_dir: home_dir.clone(), + data_dir: home_dir.join("data"), + ..KernelConfig::default() + }; + OpenFangKernel::boot_with_config(config).expect("kernel boots") + } + + /// Helper: read the persisted default-user UUID directly out of + /// `kv_store[shared_memory_agent_id, "default_user_uuid"]`. + /// + /// This bypasses `default_user_id()`, which reads a process-wide + /// `OnceLock` and would happily return the same value across restarts + /// even if `bootstrap_default_user` regenerated a fresh UUID on every + /// boot. Reading the durable storage is the only way to prove the boot + /// path actually persisted. + fn read_persisted_default_user_uuid(kernel: &OpenFangKernel) -> String { + let value = kernel + .memory + .structured_get(shared_memory_agent_id(), "default_user_uuid") + .expect("kv read default_user_uuid") + .expect("default_user_uuid must be persisted at boot"); + match value { + serde_json::Value::String(s) => s, + other => panic!("default_user_uuid stored as non-string: {other:?}"), + } + } + + /// The persistent default-user UUID must survive a kernel restart against + /// the same data directory. This is the "stable identity for one-person + /// installs" contract: every reboot resolves to the same UUID. + /// + /// We verify persistence by reading the durable `kv_store` row directly + /// — NOT via `default_user_id()`, which is backed by a process-wide + /// `OnceLock` and would return the same value across reboots even if + /// the bootstrap regenerated a fresh UUID on disk every time. + #[test] + fn test_bootstrap_default_user_persists_across_restarts() { + let tmp = tempfile::tempdir().unwrap(); + + // First boot: bootstrap generates and persists a new UUID. + let k1 = minimal_kernel(&tmp); + let first_uuid_str = read_persisted_default_user_uuid(&k1); + let first_uuid = uuid::Uuid::parse_str(&first_uuid_str) + .expect("persisted default_user_uuid must be a valid UUID"); + assert!( + !first_uuid.is_nil(), + "bootstrap must generate a non-nil UUID, got {first_uuid_str}" + ); + k1.shutdown(); + + // Second boot against the same data dir: must reuse the persisted + // UUID. Compare the raw kv_store strings — that is the only source + // of truth that survives a process restart. + let k2 = minimal_kernel(&tmp); + let second_uuid_str = read_persisted_default_user_uuid(&k2); + assert_eq!( + first_uuid_str, second_uuid_str, + "default_user_uuid in kv_store must persist across kernel restarts" + ); + k2.shutdown(); + } + + /// `resolve_user_id_internal` is the raw mapper used by in-process + /// callers. It must round-trip the test-user alias and pass arbitrary + /// UUIDs through unchanged so the integration suite can address the + /// test user. + #[test] + fn test_resolve_user_id_internal_preserves_test_alias() { + let tmp = tempfile::tempdir().unwrap(); + let kernel = minimal_kernel(&tmp); + + assert_eq!( + kernel.resolve_user_id_internal(Some("test")), + Some(openfang_types::agent::UserId( + openfang_memory::session::TEST_USER_UUID + )) + ); + + let uid = uuid::Uuid::new_v4(); + assert_eq!( + kernel.resolve_user_id_internal(Some(&uid.to_string())), + Some(openfang_types::agent::UserId(uid)) + ); + + // None and unparseable inputs both yield None. + assert_eq!(kernel.resolve_user_id_internal(None), None); + assert_eq!(kernel.resolve_user_id_internal(Some("not-a-uuid")), None); + + // "default" resolves to the persistent default user. + assert_eq!( + kernel.resolve_user_id_internal(Some("default")), + Some(kernel.default_user_id()) + ); + + kernel.shutdown(); + } + + /// The public `resolve_user_id` is the boundary between untrusted API + /// input and write-path attribution. It must fold both `"test"` and the + /// nil UUID to the persistent default user, leave other UUIDs alone, and + /// return `None` for unparseable input. + #[test] + fn test_resolve_user_id_strict_filter_folds_reserved_buckets() { + let tmp = tempfile::tempdir().unwrap(); + let kernel = minimal_kernel(&tmp); + let default = kernel.default_user_id(); + + // "test" → default (not the test user) + assert_eq!(kernel.resolve_user_id(Some("test")), Some(default)); + assert_ne!( + kernel.resolve_user_id(Some("test")), + Some(openfang_types::agent::UserId( + openfang_memory::session::TEST_USER_UUID + )) + ); + + // Nil UUID → default + let nil = uuid::Uuid::nil(); + assert_eq!( + kernel.resolve_user_id(Some(&nil.to_string())), + Some(default) + ); + + // Other valid UUIDs pass through. + let uid = uuid::Uuid::new_v4(); + assert_eq!( + kernel.resolve_user_id(Some(&uid.to_string())), + Some(openfang_types::agent::UserId(uid)) + ); + + // None / unparseable behave like the internal variant. + assert_eq!(kernel.resolve_user_id(None), None); + assert_eq!(kernel.resolve_user_id(Some("not-a-uuid")), None); + + // "default" still resolves to the default user. + assert_eq!(kernel.resolve_user_id(Some("default")), Some(default)); + + kernel.shutdown(); + } + + /// Sessions created without an explicit user_id are bound to the + /// persistent default user — the "single-user installs, everyone is the + /// default user" contract. + /// + /// We verify by reading the `user_id` column directly out of SQLite and + /// matching it against the argument passed to `create_session`, NOT + /// against `kernel.default_user_id()` returned at the assertion site. + /// Both reads-via-getter would tautologically agree on the OnceLock's + /// value even if the column was somehow stamped with a different one. + /// + /// We also verify that the persisted UUID is not nil and is a valid + /// UUID — those properties are insensitive to whichever kernel happened + /// to install the OnceLock first under cargo's shared-process test + /// harness. + #[test] + fn test_session_created_without_user_id_uses_default_user() { + let tmp = tempfile::tempdir().unwrap(); + let kernel = minimal_kernel(&tmp); + let agent_id = openfang_types::agent::AgentId::new(); + + // Capture the exact argument we'll pass to create_session so we can + // compare the column read-back to it (not to a later getter call). + let passed_user_id = openfang_memory::session::default_user_id(); + + let session = kernel + .memory + .create_session(agent_id, passed_user_id) + .expect("create session"); + + // Read the user_id column straight from SQLite — bypassing + // `SessionStore::get_session` which falls back to `default_user_id()` + // when the column is NULL, masking the very behaviour we want to + // confirm. + let conn_arc = kernel.memory.usage_conn(); + let conn = conn_arc.lock().unwrap(); + let stored_user_id: String = conn + .query_row( + "SELECT user_id FROM sessions WHERE id = ?1", + rusqlite::params![session.id.0.to_string()], + |row| row.get(0), + ) + .expect("session row must exist"); + drop(conn); + + let stored_uuid = + uuid::Uuid::parse_str(&stored_user_id).expect("user_id column must be a valid UUID"); + assert!( + !stored_uuid.is_nil(), + "default-user session must not be tagged with the nil UUID" + ); + assert_eq!( + stored_user_id, + passed_user_id.0.to_string(), + "user_id column must round-trip the value passed to create_session" + ); + + kernel.shutdown(); + } + + /// Sessions created with an explicit user_id are bound to that user. + #[test] + fn test_session_created_with_explicit_user_id_is_bound() { + let tmp = tempfile::tempdir().unwrap(); + let kernel = minimal_kernel(&tmp); + let agent_id = openfang_types::agent::AgentId::new(); + let alice = openfang_types::agent::UserId::new(); + + let session = kernel + .memory + .create_session(agent_id, alice) + .expect("create session"); + assert_eq!(session.user_id, alice); + assert_ne!(session.user_id, kernel.default_user_id()); + + kernel.shutdown(); + } } diff --git a/crates/openfang-memory/src/migration.rs b/crates/openfang-memory/src/migration.rs index 9249686510..88c64f77a3 100644 --- a/crates/openfang-memory/src/migration.rs +++ b/crates/openfang-memory/src/migration.rs @@ -5,7 +5,7 @@ use rusqlite::Connection; /// Current schema version. -const SCHEMA_VERSION: u32 = 8; +const SCHEMA_VERSION: u32 = 9; /// Run all migrations to bring the database up to date. pub fn run_migrations(conn: &Connection) -> Result<(), rusqlite::Error> { @@ -43,6 +43,10 @@ pub fn run_migrations(conn: &Connection) -> Result<(), rusqlite::Error> { migrate_v8(conn)?; } + if current_version < 9 { + migrate_v9(conn)?; + } + set_schema_version(conn, SCHEMA_VERSION)?; Ok(()) } @@ -328,6 +332,40 @@ fn migrate_v8(conn: &Connection) -> Result<(), rusqlite::Error> { Ok(()) } +/// Version 9: Tag every session with a user and a parent-session link. +/// +/// - `user_id` (TEXT, NOT NULL, default = nil UUID) — owning user. Existing +/// v8 rows inherit the nil-UUID sentinel; the kernel's +/// `bootstrap_default_user` rewrite migrates them to the persistent +/// default user on first boot after upgrade. +/// - `parent_session_id` (TEXT, nullable) — set when a session is forked off +/// another (e.g. a hand session linked back to its caller). No production +/// code path forks sessions yet, so the column is unused on existing rows. +/// +/// Indexes on `(agent_id, user_id)` and `parent_session_id` keep the per-user +/// lookups and child-session fan-out cheap. +fn migrate_v9(conn: &Connection) -> Result<(), rusqlite::Error> { + if !column_exists(conn, "sessions", "user_id") { + conn.execute( + "ALTER TABLE sessions ADD COLUMN user_id TEXT NOT NULL DEFAULT '00000000-0000-0000-0000-000000000000'", + [], + )?; + } + if !column_exists(conn, "sessions", "parent_session_id") { + conn.execute("ALTER TABLE sessions ADD COLUMN parent_session_id TEXT", [])?; + } + conn.execute_batch( + " + CREATE INDEX IF NOT EXISTS idx_sessions_user ON sessions(agent_id, user_id); + CREATE INDEX IF NOT EXISTS idx_sessions_parent ON sessions(parent_session_id); + + INSERT OR IGNORE INTO migrations (version, applied_at, description) + VALUES (9, datetime('now'), 'Tag sessions with user_id and parent_session_id'); + ", + )?; + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -360,4 +398,78 @@ mod tests { run_migrations(&conn).unwrap(); run_migrations(&conn).unwrap(); // Should not error } + + #[test] + fn test_migration_v9_adds_session_user_columns() { + let conn = Connection::open_in_memory().unwrap(); + run_migrations(&conn).unwrap(); + assert!(column_exists(&conn, "sessions", "user_id")); + assert!(column_exists(&conn, "sessions", "parent_session_id")); + } + + #[test] + fn test_migration_v9_indexes_present() { + // The (agent_id, user_id) and parent_session_id indexes underpin the + // per-user and child-session lookups added in PR 1. If either is + // missing, the queries fall back to full table scans. + let conn = Connection::open_in_memory().unwrap(); + run_migrations(&conn).unwrap(); + let names: Vec = conn + .prepare("SELECT name FROM sqlite_master WHERE type='index'") + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .filter_map(|r| r.ok()) + .collect(); + assert!(names.contains(&"idx_sessions_user".to_string())); + assert!(names.contains(&"idx_sessions_parent".to_string())); + } + + /// A v8 database (sessions table without `user_id` or `parent_session_id`) + /// must upgrade cleanly to v9: existing rows survive, the new columns are + /// present, and the user_id default sentinel is the nil UUID. + #[test] + fn test_migration_v8_to_v9_upgrade_preserves_existing_rows() { + let conn = Connection::open_in_memory().unwrap(); + + // Build the v8 schema by stopping the migration runner one step early. + migrate_v1(&conn).unwrap(); + migrate_v2(&conn).unwrap(); + migrate_v3(&conn).unwrap(); + migrate_v4(&conn).unwrap(); + migrate_v5(&conn).unwrap(); + migrate_v6(&conn).unwrap(); + migrate_v7(&conn).unwrap(); + migrate_v8(&conn).unwrap(); + set_schema_version(&conn, 8).unwrap(); + + assert!(!column_exists(&conn, "sessions", "user_id")); + assert!(!column_exists(&conn, "sessions", "parent_session_id")); + + // Insert a pre-v9 session row (no user_id, no parent_session_id). + conn.execute( + "INSERT INTO sessions (id, agent_id, messages, context_window_tokens, created_at, updated_at) + VALUES ('sess-1', 'agent-1', X'', 0, datetime('now'), datetime('now'))", + [], + ) + .unwrap(); + + // Now run the full pipeline — v9 should apply on top of v8. + run_migrations(&conn).unwrap(); + + assert!(column_exists(&conn, "sessions", "user_id")); + assert!(column_exists(&conn, "sessions", "parent_session_id")); + + // The pre-existing row survives and gets the nil-UUID default for + // user_id, NULL for parent_session_id. + let (uid, parent): (String, Option) = conn + .query_row( + "SELECT user_id, parent_session_id FROM sessions WHERE id = 'sess-1'", + [], + |row| Ok((row.get::<_, String>(0)?, row.get::<_, Option>(1)?)), + ) + .unwrap(); + assert_eq!(uid, "00000000-0000-0000-0000-000000000000"); + assert!(parent.is_none()); + } } diff --git a/crates/openfang-memory/src/session.rs b/crates/openfang-memory/src/session.rs index 7a18e0f9fb..1176a58e50 100644 --- a/crates/openfang-memory/src/session.rs +++ b/crates/openfang-memory/src/session.rs @@ -1,13 +1,70 @@ //! Session management — load/save conversation history. use chrono::Utc; -use openfang_types::agent::{AgentId, SessionId}; +use openfang_types::agent::{AgentId, SessionId, UserId}; use openfang_types::error::{OpenFangError, OpenFangResult}; use openfang_types::message::{ContentBlock, Message, MessageContent, Role}; use rusqlite::Connection; use std::io::Write; use std::path::Path; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, OnceLock}; + +/// Process-wide override for the default user ID. +/// +/// Set by the kernel at boot via [`set_default_user_id`] after it loads or +/// generates the persistent default-user UUID from the kv_store. If unset, +/// [`default_user_id`] falls back to the legacy nil UUID so library callers +/// (tests, embeddings without a kernel) still get a deterministic value. +static DEFAULT_USER_ID: OnceLock = OnceLock::new(); + +/// Install the persistent default user ID. Called once at kernel boot. +/// +/// Subsequent calls are ignored (OnceLock semantics). This matches the +/// expectation that a single kernel process has exactly one default user. +pub fn set_default_user_id(user_id: UserId) { + let _ = DEFAULT_USER_ID.set(user_id); +} + +/// Return the default user ID. +/// +/// When the kernel has installed a persistent UUID via [`set_default_user_id`] +/// (the normal case in production), returns it. Otherwise falls back to the +/// nil UUID — preserved for tests and embedded use that never call the kernel. +pub fn default_user_id() -> UserId { + DEFAULT_USER_ID + .get() + .copied() + .unwrap_or_else(|| UserId(uuid::Uuid::nil())) +} + +/// Sentinel UUID for the well-known "test user" bucket +/// (`00000000-0000-0000-0000-000000000002`). +/// +/// This is distinct from: +/// - the persistent default user (a fresh UUID generated at kernel boot, +/// falling back to the nil UUID `…0000` for library callers that never +/// call the kernel), and +/// - `shared_memory_agent_id()`, which uses +/// `00000000-0000-0000-0000-000000000001` as its agent-side sentinel. +/// +/// Production code may reference this constant only to recognise the +/// kernel's `"test"` alias and (for the strict HTTP-boundary resolver) fold +/// it back to the persistent default user. It is **not** routable from any +/// external API surface — see `OpenFangKernel::resolve_user_id`. +pub const TEST_USER_UUID: uuid::Uuid = uuid::Uuid::from_u128(2); + +/// Return the test user ID — a fixed well-known UUID separate from the +/// default user and from `shared_memory_agent_id()`. +/// +/// Test-only helper used by the in-process integration suite to address an +/// isolated bucket without polluting the default user's memory. Production +/// code that needs the same UUID for sentinel comparisons must reference +/// [`TEST_USER_UUID`] directly so this helper stays out of the production +/// API surface. +#[cfg(test)] +pub fn test_user_id() -> UserId { + UserId(TEST_USER_UUID) +} /// A conversation session with message history. #[derive(Debug, Clone)] @@ -16,6 +73,17 @@ pub struct Session { pub id: SessionId, /// Owning agent ID. pub agent_id: AgentId, + /// User this session belongs to. + /// + /// Required: every session is owned by exactly one user. New sessions + /// created without an explicit user use [`default_user_id`]. + pub user_id: UserId, + /// Parent session ID — set when this session was forked off another one + /// (e.g. a hand session linked back to its caller). Lays the foundation + /// for tree-scoped cascade deletion; no production code path forks + /// sessions in this PR, so the value is `None` for every session created + /// today. + pub parent_session_id: Option, /// Conversation messages. pub messages: Vec, /// Estimated token count for the context window. @@ -43,7 +111,11 @@ impl SessionStore { .lock() .map_err(|e| OpenFangError::Internal(e.to_string()))?; let mut stmt = conn - .prepare("SELECT agent_id, messages, context_window_tokens, label FROM sessions WHERE id = ?1") + .prepare( + "SELECT agent_id, messages, context_window_tokens, label, \ + user_id, parent_session_id \ + FROM sessions WHERE id = ?1", + ) .map_err(|e| OpenFangError::Memory(e.to_string()))?; let result = stmt.query_row(rusqlite::params![session_id.0.to_string()], |row| { @@ -51,19 +123,37 @@ impl SessionStore { let messages_blob: Vec = row.get(1)?; let tokens: i64 = row.get(2)?; let label: Option = row.get(3).unwrap_or(None); - Ok((agent_str, messages_blob, tokens, label)) + let user_id_str: Option = row.get(4).unwrap_or(None); + let parent_session_id_str: Option = row.get(5).unwrap_or(None); + Ok(( + agent_str, + messages_blob, + tokens, + label, + user_id_str, + parent_session_id_str, + )) }); match result { - Ok((agent_str, messages_blob, tokens, label)) => { + Ok((agent_str, messages_blob, tokens, label, user_id_str, parent_session_id_str)) => { let agent_id = uuid::Uuid::parse_str(&agent_str) .map(AgentId) .map_err(|e| OpenFangError::Memory(e.to_string()))?; let messages: Vec = rmp_serde::from_slice(&messages_blob) .map_err(|e| OpenFangError::Serialization(e.to_string()))?; + let user_id = user_id_str + .and_then(|s| uuid::Uuid::parse_str(&s).ok()) + .map(UserId) + .unwrap_or_else(default_user_id); + let parent_session_id = parent_session_id_str + .and_then(|s| uuid::Uuid::parse_str(&s).ok()) + .map(SessionId); Ok(Some(Session { id: session_id, agent_id, + user_id, + parent_session_id, messages, context_window_tokens: tokens as u64, label, @@ -83,16 +173,26 @@ impl SessionStore { let messages_blob = rmp_serde::to_vec_named(&session.messages) .map_err(|e| OpenFangError::Serialization(e.to_string()))?; let now = Utc::now().to_rfc3339(); + let parent_str = session + .parent_session_id + .as_ref() + .map(|id| id.0.to_string()); conn.execute( - "INSERT INTO sessions (id, agent_id, messages, context_window_tokens, label, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?6) - ON CONFLICT(id) DO UPDATE SET messages = ?3, context_window_tokens = ?4, label = ?5, updated_at = ?6", + "INSERT INTO sessions \ + (id, agent_id, messages, context_window_tokens, label, \ + user_id, parent_session_id, created_at, updated_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?8) + ON CONFLICT(id) DO UPDATE SET \ + messages = ?3, context_window_tokens = ?4, label = ?5, \ + user_id = ?6, parent_session_id = ?7, updated_at = ?8", rusqlite::params![ session.id.0.to_string(), session.agent_id.0.to_string(), messages_blob, session.context_window_tokens as i64, session.label.as_deref(), + session.user_id.0.to_string(), + parent_str, now, ], ) @@ -182,11 +282,16 @@ impl SessionStore { Ok(sessions) } - /// Create a new empty session for an agent. - pub fn create_session(&self, agent_id: AgentId) -> OpenFangResult { + /// Create a new empty session for an agent owned by `user_id`. + /// + /// Callers that have no specific user pass [`default_user_id`] so the + /// session attaches to the kernel's persistent default identity. + pub fn create_session(&self, agent_id: AgentId, user_id: UserId) -> OpenFangResult { let session = Session { id: SessionId::new(), agent_id, + user_id, + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -225,8 +330,9 @@ impl SessionStore { .map_err(|e| OpenFangError::Internal(e.to_string()))?; let mut stmt = conn .prepare( - "SELECT id, messages, context_window_tokens, label FROM sessions \ - WHERE agent_id = ?1 AND label = ?2 LIMIT 1", + "SELECT id, messages, context_window_tokens, label, \ + user_id, parent_session_id \ + FROM sessions WHERE agent_id = ?1 AND label = ?2 LIMIT 1", ) .map_err(|e| OpenFangError::Memory(e.to_string()))?; @@ -235,19 +341,37 @@ impl SessionStore { let messages_blob: Vec = row.get(1)?; let tokens: i64 = row.get(2)?; let lbl: Option = row.get(3).unwrap_or(None); - Ok((id_str, messages_blob, tokens, lbl)) + let user_id_str: Option = row.get(4).unwrap_or(None); + let parent_session_id_str: Option = row.get(5).unwrap_or(None); + Ok(( + id_str, + messages_blob, + tokens, + lbl, + user_id_str, + parent_session_id_str, + )) }); match result { - Ok((id_str, messages_blob, tokens, lbl)) => { + Ok((id_str, messages_blob, tokens, lbl, user_id_str, parent_session_id_str)) => { let session_id = uuid::Uuid::parse_str(&id_str) .map(SessionId) .map_err(|e| OpenFangError::Memory(e.to_string()))?; let messages: Vec = rmp_serde::from_slice(&messages_blob) .map_err(|e| OpenFangError::Serialization(e.to_string()))?; + let user_id = user_id_str + .and_then(|s| uuid::Uuid::parse_str(&s).ok()) + .map(UserId) + .unwrap_or_else(default_user_id); + let parent_session_id = parent_session_id_str + .and_then(|s| uuid::Uuid::parse_str(&s).ok()) + .map(SessionId); Ok(Some(Session { id: session_id, agent_id, + user_id, + parent_session_id, messages, context_window_tokens: tokens as u64, label: lbl, @@ -298,6 +422,10 @@ impl SessionStore { } /// Create a new session with an optional label. + /// + /// The session is owned by [`default_user_id`]. This entry point is used + /// by the kernel's "create named session" API, which today does not + /// surface a user selector. pub fn create_session_with_label( &self, agent_id: AgentId, @@ -306,6 +434,8 @@ impl SessionStore { let session = Session { id: SessionId::new(), agent_id, + user_id: default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: label.map(|s| s.to_string()), @@ -635,18 +765,73 @@ mod tests { fn test_create_and_load_session() { let store = setup(); let agent_id = AgentId::new(); - let session = store.create_session(agent_id).unwrap(); + let session = store.create_session(agent_id, default_user_id()).unwrap(); let loaded = store.get_session(session.id).unwrap().unwrap(); assert_eq!(loaded.agent_id, agent_id); assert!(loaded.messages.is_empty()); + // Sessions created with the default user must report that ownership + // through the new column. + assert_eq!(loaded.user_id, default_user_id()); + assert!(loaded.parent_session_id.is_none()); + } + + #[test] + fn test_create_session_with_explicit_user_binds_to_that_user() { + // Sessions created with a specific user_id must round-trip through + // SQLite without being silently rewritten to the default user. + let store = setup(); + let agent_id = AgentId::new(); + let user = UserId::new(); + + let session = store.create_session(agent_id, user).unwrap(); + let loaded = store.get_session(session.id).unwrap().unwrap(); + assert_eq!(loaded.user_id, user); + assert_ne!(loaded.user_id, default_user_id()); + } + + #[test] + fn test_default_and_test_user_ids_differ() { + // `default_user_id` and `test_user_id` must be distinct so the test + // bucket never collides with the production default identity. + // (OnceLock is process-wide so we don't assert the nil-UUID fallback + // value directly — another test in the binary may have installed a + // persistent UUID first.) + assert_ne!(default_user_id(), test_user_id()); + assert_eq!(test_user_id().0, TEST_USER_UUID); + assert_eq!(TEST_USER_UUID, uuid::Uuid::from_u128(2)); + } + + #[test] + fn test_user_id_does_not_collide_with_shared_memory_agent() { + // `shared_memory_agent_id()` lives on the agent-ID axis and uses + // `from_u128(1)`. The test user bucket uses `from_u128(2)` so they + // can never accidentally compare equal when a stringly-typed UUID + // appears in logs or migration audits. + let shared_agent_uuid = uuid::Uuid::from_u128(1); + assert_ne!(TEST_USER_UUID, shared_agent_uuid); + } + + #[test] + fn test_save_session_persists_parent_link() { + // Sessions with a `parent_session_id` round-trip the link so a + // future PR can cascade-delete by session tree. + let store = setup(); + let agent_id = AgentId::new(); + let parent = store.create_session(agent_id, default_user_id()).unwrap(); + let mut child = store.create_session(agent_id, default_user_id()).unwrap(); + child.parent_session_id = Some(parent.id); + store.save_session(&child).unwrap(); + + let loaded = store.get_session(child.id).unwrap().unwrap(); + assert_eq!(loaded.parent_session_id, Some(parent.id)); } #[test] fn test_save_and_load_with_messages() { let store = setup(); let agent_id = AgentId::new(); - let mut session = store.create_session(agent_id).unwrap(); + let mut session = store.create_session(agent_id, default_user_id()).unwrap(); session.messages.push(Message::user("Hello")); session.messages.push(Message::assistant("Hi there!")); store.save_session(&session).unwrap(); @@ -666,7 +851,7 @@ mod tests { fn test_delete_session() { let store = setup(); let agent_id = AgentId::new(); - let session = store.create_session(agent_id).unwrap(); + let session = store.create_session(agent_id, default_user_id()).unwrap(); let sid = session.id; assert!(store.get_session(sid).unwrap().is_some()); store.delete_session(sid).unwrap(); @@ -677,8 +862,8 @@ mod tests { fn test_delete_agent_sessions() { let store = setup(); let agent_id = AgentId::new(); - let s1 = store.create_session(agent_id).unwrap(); - let s2 = store.create_session(agent_id).unwrap(); + let s1 = store.create_session(agent_id, default_user_id()).unwrap(); + let s2 = store.create_session(agent_id, default_user_id()).unwrap(); assert!(store.get_session(s1.id).unwrap().is_some()); assert!(store.get_session(s2.id).unwrap().is_some()); store.delete_agent_sessions(agent_id).unwrap(); @@ -782,7 +967,7 @@ mod tests { fn test_jsonl_mirror_write() { let store = setup(); let agent_id = AgentId::new(); - let mut session = store.create_session(agent_id).unwrap(); + let mut session = store.create_session(agent_id, default_user_id()).unwrap(); session .messages .push(openfang_types::message::Message::user("Hello")); diff --git a/crates/openfang-memory/src/substrate.rs b/crates/openfang-memory/src/substrate.rs index 03ecd78cfc..41dc160bba 100644 --- a/crates/openfang-memory/src/substrate.rs +++ b/crates/openfang-memory/src/substrate.rs @@ -12,7 +12,7 @@ use crate::structured::StructuredStore; use crate::usage::UsageStore; use async_trait::async_trait; -use openfang_types::agent::{AgentEntry, AgentId, SessionId}; +use openfang_types::agent::{AgentEntry, AgentId, SessionId, UserId}; use openfang_types::config::MemoryConfig; use openfang_types::error::{OpenFangError, OpenFangResult}; use openfang_types::memory::{ @@ -207,9 +207,51 @@ impl MemorySubstrate { .map_err(|e| OpenFangError::Internal(e.to_string()))? } - /// Create a new empty session for an agent. - pub fn create_session(&self, agent_id: AgentId) -> OpenFangResult { - self.sessions.create_session(agent_id) + /// Create a new empty session for an agent owned by `user_id`. + /// + /// Callers without a specific user pass `default_user_id()` so the + /// session attaches to the kernel's persistent default identity. + pub fn create_session(&self, agent_id: AgentId, user_id: UserId) -> OpenFangResult { + self.sessions.create_session(agent_id, user_id) + } + + /// Rewrite all sessions that are still tagged with the nil-UUID owner + /// (the legacy "anonymous bucket" default) to the given `target` user. + /// + /// Returns the number of sessions updated. Called once at kernel boot + /// after the persistent default-user UUID is determined; subsequent calls + /// are no-ops because no nil-UUID rows remain. + /// + /// The UPDATE runs inside an explicit transaction so a crash or lock + /// failure mid-flight rolls back cleanly — the caller's bootstrap + /// sentinel relies on this being all-or-nothing to avoid permanently + /// stranding sessions on the nil bucket. + pub fn rewrite_nil_user_sessions(&self, target: UserId) -> OpenFangResult { + let mut conn = self + .conn + .lock() + .map_err(|e| OpenFangError::Internal(e.to_string()))?; + let nil_str = uuid::Uuid::nil().to_string(); + let target_str = target.0.to_string(); + + let tx = conn + .transaction() + .map_err(|e| OpenFangError::Memory(e.to_string()))?; + let sessions_updated = tx + .execute( + "UPDATE sessions SET user_id = ?1 WHERE user_id = ?2 OR user_id IS NULL", + rusqlite::params![target_str, nil_str], + ) + .map_err(|e| OpenFangError::Memory(e.to_string()))?; + tx.commit() + .map_err(|e| OpenFangError::Memory(e.to_string()))?; + + info!( + sessions_updated = sessions_updated, + target = %target.0, + "Rewrote nil-UUID sessions to persistent default user" + ); + Ok(sessions_updated) } /// List all sessions with metadata. @@ -731,6 +773,109 @@ impl Memory for MemorySubstrate { #[cfg(test)] mod tests { use super::*; + use crate::session::default_user_id; + + #[test] + fn test_rewrite_nil_user_sessions_is_idempotent_and_targeted() { + // The one-shot fixup must rewrite ONLY the nil-UUID rows and leave + // every other session alone; a second call after the rewrite must + // find zero matching rows and behave as a no-op. + let substrate = MemorySubstrate::open_in_memory(0.1).unwrap(); + let agent_id = AgentId::new(); + let other_user = UserId::new(); + let target = UserId::new(); + let nil_user = UserId(uuid::Uuid::nil()); + + // Two nil-bucket sessions (the legacy default) + one explicit-user session. + let s1 = substrate + .sessions + .create_session(agent_id, nil_user) + .unwrap(); + let s2 = substrate + .sessions + .create_session(agent_id, nil_user) + .unwrap(); + let s3 = substrate + .sessions + .create_session(agent_id, other_user) + .unwrap(); + + let updated = substrate.rewrite_nil_user_sessions(target).unwrap(); + assert_eq!( + updated, 2, + "exactly the two nil-UUID sessions must be rewritten" + ); + + let s1 = substrate.sessions.get_session(s1.id).unwrap().unwrap(); + let s2 = substrate.sessions.get_session(s2.id).unwrap().unwrap(); + let s3 = substrate.sessions.get_session(s3.id).unwrap().unwrap(); + assert_eq!(s1.user_id, target); + assert_eq!(s2.user_id, target); + assert_eq!( + s3.user_id, other_user, + "non-nil sessions must NOT be touched" + ); + + // Second call is a no-op (no nil rows left). + let updated2 = substrate.rewrite_nil_user_sessions(target).unwrap(); + assert_eq!(updated2, 0); + } + + #[test] + fn test_rewrite_nil_user_sessions_returns_err_when_table_missing() { + // When the `sessions` table is missing, the UPDATE cannot run and the + // caller must observe an error — not a silent success. This is what + // lets `bootstrap_default_user` skip setting the + // `default_user_bootstrap_done` sentinel on failure so the rewrite is + // retried on the next boot. + // + // NOTE: This test does NOT exercise transactional atomicity. A single + // `UPDATE` is implicitly all-or-nothing under SQLite, so there is no + // partial-commit state to roll back from. True multi-statement + // atomicity becomes testable in PR 2 when the `extractions` table + // joins the same transaction — at that point we add a dedicated + // `…_is_atomic_on_partial_failure` test that forces the second + // statement to fail and verifies the first one is rolled back. + let substrate = MemorySubstrate::open_in_memory(0.1).unwrap(); + let agent_id = AgentId::new(); + let target = UserId::new(); + let nil_user = UserId(uuid::Uuid::nil()); + + let _s1 = substrate + .sessions + .create_session(agent_id, nil_user) + .unwrap(); + + // Drop the only table the UPDATE touches; the next call must error. + { + let conn = substrate.conn.lock().unwrap(); + conn.execute("DROP TABLE sessions", []) + .expect("drop sessions"); + } + + let err = substrate.rewrite_nil_user_sessions(target); + assert!( + err.is_err(), + "rewrite must fail when the UPDATE cannot complete" + ); + } + + #[test] + fn test_create_session_via_substrate_routes_user_id() { + // The substrate's `create_session` proxy must forward the user_id + // through to the session store rather than silently dropping it. + let substrate = MemorySubstrate::open_in_memory(0.1).unwrap(); + let agent_id = AgentId::new(); + let user = UserId::new(); + + let session = substrate.create_session(agent_id, user).unwrap(); + assert_eq!(session.user_id, user); + + let default_session = substrate + .create_session(agent_id, default_user_id()) + .unwrap(); + assert_eq!(default_session.user_id, default_user_id()); + } #[tokio::test] async fn test_substrate_kv() { diff --git a/crates/openfang-runtime/src/agent_loop.rs b/crates/openfang-runtime/src/agent_loop.rs index 615fbfdb73..6f29244fc6 100644 --- a/crates/openfang-runtime/src/agent_loop.rs +++ b/crates/openfang-runtime/src/agent_loop.rs @@ -3792,6 +3792,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -3845,6 +3847,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -3900,6 +3904,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -3953,6 +3959,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -3997,6 +4005,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -4123,6 +4133,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -4170,6 +4182,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -4223,6 +4237,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -5189,6 +5205,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -5260,6 +5278,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -5337,6 +5357,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, @@ -5392,6 +5414,8 @@ mod tests { let mut session = openfang_memory::session::Session { id: openfang_types::agent::SessionId::new(), agent_id, + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: Vec::new(), context_window_tokens: 0, label: None, diff --git a/crates/openfang-runtime/src/compactor.rs b/crates/openfang-runtime/src/compactor.rs index 3602ff244a..2b50a236c1 100644 --- a/crates/openfang-runtime/src/compactor.rs +++ b/crates/openfang-runtime/src/compactor.rs @@ -777,6 +777,8 @@ mod tests { let session = Session { id: openfang_types::agent::SessionId::new(), agent_id: openfang_types::agent::AgentId::new(), + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: vec![Message::user("hello")], context_window_tokens: 0, label: None, @@ -793,6 +795,8 @@ mod tests { let session = Session { id: openfang_types::agent::SessionId::new(), agent_id: openfang_types::agent::AgentId::new(), + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages, context_window_tokens: 0, label: None, @@ -842,6 +846,8 @@ mod tests { let session = Session { id: openfang_types::agent::SessionId::new(), agent_id: openfang_types::agent::AgentId::new(), + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages: vec![Message::user("hello"), Message::assistant("hi")], context_window_tokens: 0, label: None, @@ -931,6 +937,8 @@ mod tests { let session = Session { id: openfang_types::agent::SessionId::new(), agent_id: openfang_types::agent::AgentId::new(), + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages, context_window_tokens: 0, label: None, @@ -1002,6 +1010,8 @@ mod tests { let session = Session { id: openfang_types::agent::SessionId::new(), agent_id: openfang_types::agent::AgentId::new(), + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages, context_window_tokens: 0, label: None, @@ -1129,6 +1139,8 @@ mod tests { let session = Session { id: openfang_types::agent::SessionId::new(), agent_id: openfang_types::agent::AgentId::new(), + user_id: openfang_memory::session::default_user_id(), + parent_session_id: None, messages, context_window_tokens: 0, label: None, diff --git a/crates/openfang-types/src/config.rs b/crates/openfang-types/src/config.rs index 25df9b0059..a7e0fe23a7 100644 --- a/crates/openfang-types/src/config.rs +++ b/crates/openfang-types/src/config.rs @@ -186,6 +186,13 @@ pub struct UserConfig { /// Optional API key hash for API authentication. #[serde(default)] pub api_key_hash: Option, + /// Marks this user as the persistent default. When `true`, the kernel + /// binds this user's display name and channel bindings to the persistent + /// default-user UUID generated at boot rather than a freshly-generated + /// per-config UUID. At most one `[[users]]` block should set this; if + /// none does, the first user in the config inherits the default identity. + #[serde(default)] + pub is_default: bool, } fn default_role() -> String { @@ -4120,12 +4127,44 @@ mod tests { m }, api_key_hash: None, + is_default: false, }; let json = serde_json::to_string(&uc).unwrap(); let back: UserConfig = serde_json::from_str(&json).unwrap(); assert_eq!(back.name, "Alice"); assert_eq!(back.role, "owner"); assert_eq!(back.channel_bindings.get("telegram").unwrap(), "123456"); + assert!(!back.is_default); + } + + #[test] + fn test_user_config_is_default_defaults_to_false() { + // TOML/JSON without `is_default` must deserialize as `false` so that + // existing configs are unaffected by this PR. + let toml_src = r#" + name = "Alice" + role = "owner" + "#; + let uc: UserConfig = toml::from_str(toml_src).unwrap(); + assert!(!uc.is_default); + + let json_src = r#"{"name":"Bob","role":"user"}"#; + let uc: UserConfig = serde_json::from_str(json_src).unwrap(); + assert!(!uc.is_default); + } + + #[test] + fn test_user_config_is_default_roundtrips_when_set() { + let uc = UserConfig { + name: "Owner".to_string(), + role: "owner".to_string(), + channel_bindings: std::collections::HashMap::new(), + api_key_hash: None, + is_default: true, + }; + let json = serde_json::to_string(&uc).unwrap(); + let back: UserConfig = serde_json::from_str(&json).unwrap(); + assert!(back.is_default); } #[test] diff --git a/crates/openfang-types/src/message.rs b/crates/openfang-types/src/message.rs index ba5e93438d..e798b7108c 100644 --- a/crates/openfang-types/src/message.rs +++ b/crates/openfang-types/src/message.rs @@ -13,6 +13,20 @@ fn new_msg_id() -> String { uuid::Uuid::new_v4().to_string() } +/// Source tag for a message — identifies system-injected context that should +/// not be treated like ordinary conversational turns by downstream consumers +/// (e.g. structured-memory extraction, dream summarisation). +/// +/// Foundational type: this PR adds the field so older sessions deserialize +/// cleanly and future consumers (PR 2: per-user memory extraction filtering) +/// have a stable tag to read. No code path in this PR sets a non-`None` value. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum MessageSource { + /// Injected by context sources (e.g. calendar-hand, mail-hand summaries). + ContextInjection, +} + /// A message in an LLM conversation. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Message { @@ -38,6 +52,12 @@ pub struct Message { pub role: Role, /// The content of the message. pub content: MessageContent, + /// Optional source tag — set for system-injected messages that downstream + /// consumers may want to treat differently (e.g. exclude from structured + /// extraction). `#[serde(default)]` so sessions persisted before this + /// field existed deserialize with `source = None`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub source: Option, } /// The role of a message sender in an LLM conversation. @@ -76,6 +96,7 @@ impl Default for Message { provider_msg_id: None, role: Role::User, content: MessageContent::Text(String::new()), + source: None, } } } @@ -249,6 +270,7 @@ impl Message { provider_msg_id: None, role: Role::System, content: MessageContent::Text(content.into()), + source: None, } } @@ -259,6 +281,7 @@ impl Message { provider_msg_id: None, role: Role::User, content: MessageContent::Text(content.into()), + source: None, } } @@ -269,6 +292,7 @@ impl Message { provider_msg_id: None, role: Role::User, content: MessageContent::Blocks(blocks), + source: None, } } @@ -279,6 +303,7 @@ impl Message { provider_msg_id: None, role: Role::Assistant, content: MessageContent::Text(content.into()), + source: None, } } @@ -292,6 +317,7 @@ impl Message { provider_msg_id: None, role: Role::Assistant, content: MessageContent::Blocks(blocks), + source: None, } } @@ -587,6 +613,65 @@ mod tests { assert_eq!(restored.provider_msg_id.as_deref(), Some("msg_xyz")); } + #[test] + fn test_message_source_roundtrip() { + // The `MessageSource::ContextInjection` tag must survive serde + // round-trips so future consumers can rely on it. + let mut msg = Message::user("Hello"); + msg.source = Some(MessageSource::ContextInjection); + let json = serde_json::to_string(&msg).unwrap(); + let decoded: Message = serde_json::from_str(&json).unwrap(); + assert_eq!(decoded.source, Some(MessageSource::ContextInjection)); + } + + #[test] + fn test_message_source_skipped_when_none() { + // `skip_serializing_if = "Option::is_none"` keeps the JSON minimal + // for the overwhelmingly common case of an untagged user/assistant + // message. + let msg = Message::user("Hello"); + let json = serde_json::to_value(&msg).unwrap(); + assert!( + json.get("source").is_none(), + "untagged messages must not write a `source` field" + ); + } + + #[test] + fn test_message_source_legacy_deser_defaults_to_none() { + // A message serialized before the `source` field existed should + // deserialize with `source = None`. This is what every existing + // session on disk looks like — `serde(default)` carries the load. + let json = r#"{"role":"user","content":"Hello"}"#; + let msg: Message = serde_json::from_str(json).unwrap(); + assert_eq!(msg.source, None); + assert_eq!(msg.role, Role::User); + } + + #[test] + fn test_message_source_msgpack_roundtrip() { + // SessionStore persists messages via rmp_serde::to_vec_named — make + // sure the new field round-trips through msgpack the same way it + // does through JSON, since msgpack is the on-disk format. + let mut msg = Message::user("hello"); + msg.source = Some(MessageSource::ContextInjection); + let bytes = rmp_serde::to_vec_named(&msg).expect("msgpack encode"); + let restored: Message = rmp_serde::from_slice(&bytes).expect("msgpack decode"); + assert_eq!(restored.source, Some(MessageSource::ContextInjection)); + } + + #[test] + fn test_message_legacy_msgpack_deser_defaults_to_none() { + // A msgpack-encoded message persisted before the `source` field + // existed must still deserialize cleanly. We synthesise the + // pre-`source` shape by encoding a plain `Message::user` and + // confirming the round-trip yields `source = None`. + let msg = Message::user("legacy"); + let bytes = rmp_serde::to_vec_named(&msg).expect("encode"); + let restored: Message = rmp_serde::from_slice(&bytes).expect("decode"); + assert_eq!(restored.source, None); + } + #[test] fn test_user_with_blocks() { let blocks = vec![