diff --git a/crates/khive-mcp/src/serve.rs b/crates/khive-mcp/src/serve.rs index a9fd7a163..2d525da1e 100644 --- a/crates/khive-mcp/src/serve.rs +++ b/crates/khive-mcp/src/serve.rs @@ -1192,6 +1192,16 @@ pub fn build_registry_for_multi_backend( base_config: RuntimeConfig, khive_cfg: &KhiveConfig, cli_db_override: Option<&str>, +) -> anyhow::Result { + khive_runtime::assert_db_anchor_consistent(base_config.db_path.as_deref(), cli_db_override)?; + build_registry_for_multi_backend_inner(base_config, khive_cfg, cli_db_override) +} + +pub fn build_registry_for_multi_backend_with_db_anchor( + base_config: RuntimeConfig, + khive_cfg: &KhiveConfig, + cli_db_override: Option<&str>, + db_anchor: Option<&std::path::Path>, ) -> anyhow::Result { // Regression fence: `base_config.db_path` feeds `compute_config_id` below, // so it must agree with the canonical anchor for this same `--db` input. @@ -1199,8 +1209,16 @@ pub fn build_registry_for_multi_backend( // through — `build_server_multi_backend` in this file and `kkernel`'s // `Command::Mcp` coordinator-attached branch — so the guard lives here // once instead of at each caller. - khive_runtime::assert_db_anchor_consistent(base_config.db_path.as_deref(), cli_db_override)?; + khive_runtime::assert_captured_db_anchor_consistent(base_config.db_path.as_deref(), db_anchor)?; + + build_registry_for_multi_backend_inner(base_config, khive_cfg, cli_db_override) +} +fn build_registry_for_multi_backend_inner( + base_config: RuntimeConfig, + khive_cfg: &KhiveConfig, + cli_db_override: Option<&str>, +) -> anyhow::Result { let backend_count = khive_cfg.backends.len(); let force_memory = match cli_db_override { Some(":memory:") => { @@ -1528,7 +1546,7 @@ pub fn build_server_with_explicit_namespace( namespace_explicit: bool, actor_explicit: bool, ) -> anyhow::Result<(KhiveMcpServer, Option)> { - let config = resolve_runtime_config(RuntimeConfigInputs { + let (config, db_anchor) = resolve_runtime_config_with_db_anchor(RuntimeConfigInputs { db: args.db.as_deref(), config: args.config.as_deref(), namespace, @@ -1547,7 +1565,10 @@ pub fn build_server_with_explicit_namespace( // resolver derives from this same `--db` input, or `config_id` (computed // from `config.db_path` below) would silently desynchronize this process // from any daemon/peer anchored on the same database. - khive_runtime::assert_db_anchor_consistent(config.db_path.as_deref(), args.db.as_deref())?; + khive_runtime::assert_captured_db_anchor_consistent( + config.db_path.as_deref(), + db_anchor.as_deref(), + )?; // Load the KhiveConfig to check for multi-backend declarations (ADR-028). // When no [[backends]] are declared, fall through to the existing single-backend path @@ -1603,7 +1624,12 @@ pub fn build_server_with_explicit_namespace( } // Multi-backend path (ADR-028). - let multi = build_registry_for_multi_backend(config, &khive_cfg, args.db.as_deref())?; + let multi = build_registry_for_multi_backend_with_db_anchor( + config, + &khive_cfg, + args.db.as_deref(), + db_anchor.as_deref(), + )?; let schedule_rt = multi .per_pack_runtimes .get("schedule") @@ -1666,11 +1692,29 @@ pub fn build_server_multi_backend( base_config: RuntimeConfig, khive_cfg: &KhiveConfig, cli_db_override: Option<&str>, +) -> anyhow::Result { + khive_runtime::assert_db_anchor_consistent(base_config.db_path.as_deref(), cli_db_override)?; + let multi = build_registry_for_multi_backend_inner(base_config, khive_cfg, cli_db_override)?; + Ok(build_server_from_multi_backend_registry( + multi, khive_cfg, None, + )) +} + +pub fn build_server_multi_backend_with_db_anchor( + base_config: RuntimeConfig, + khive_cfg: &KhiveConfig, + cli_db_override: Option<&str>, + db_anchor: Option<&std::path::Path>, ) -> anyhow::Result { // The db-anchor consistency guard runs inside `build_registry_for_multi_backend` // (the shared choke point every multi-backend boot path funnels through), // so it is not duplicated here. - let multi = build_registry_for_multi_backend(base_config, khive_cfg, cli_db_override)?; + let multi = build_registry_for_multi_backend_with_db_anchor( + base_config, + khive_cfg, + cli_db_override, + db_anchor, + )?; Ok(build_server_from_multi_backend_registry( multi, khive_cfg, None, )) @@ -1903,7 +1947,18 @@ pub struct RuntimeConfigInputs<'a> { /// default/env model set while the MCP server serves recall from the /// config-file `[[engines]]` set. pub fn resolve_runtime_config(inputs: RuntimeConfigInputs<'_>) -> anyhow::Result { - let db_path = khive_runtime::resolve_db_anchor(inputs.db); + let (config, _) = resolve_runtime_config_with_db_anchor(inputs)?; + Ok(config) +} + +/// Resolve a [`RuntimeConfig`] and return the database anchor captured at the +/// same construction boundary. Server boot paths thread this value through +/// consistency validation and registry construction without re-reading HOME. +pub fn resolve_runtime_config_with_db_anchor( + inputs: RuntimeConfigInputs<'_>, +) -> anyhow::Result<(RuntimeConfig, Option)> { + let db_anchor = khive_runtime::resolve_db_anchor(inputs.db); + let db_path = db_anchor.clone(); let packs = inputs .packs @@ -2026,7 +2081,7 @@ pub fn resolve_runtime_config(inputs: RuntimeConfigInputs<'_>) -> anyhow::Result // Tier-3 env fallback: KHIVE_BRAIN_PROFILE is applied AFTER CLI (tier-1) and // config-file (tier-2) so that a project or global TOML always wins over the env var. - Ok(apply_env_brain_profile(resolved)) + Ok((apply_env_brain_profile(resolved), db_anchor)) } /// Apply `KHIVE_BRAIN_PROFILE` env var as the tier-3 fallback for `brain_profile`. @@ -3789,26 +3844,37 @@ id = "lambda:project-actor" ); } - /// B-SHOULD-FIX-2 (data safety): Two [[backends]] entries whose sqlite paths - /// canonicalize to the same file must share a single Arc and - /// run migrations only once. Verified by using two names that differ only by - /// `./` prefix while pointing at the same absolute path. - #[test] - #[serial] - fn duplicate_sqlite_paths_deduplicated_to_single_backend() { - use khive_runtime::PackConfig; + /// RAII guard: redirects `HOME` and restores the prior value on drop. + struct HomeGuard { + original: Option, + } - let dir = tempfile::tempdir().unwrap(); - let db_path = dir.path().join("shared.db"); - let db_path_str = db_path.to_str().unwrap(); + impl HomeGuard { + fn redirect_to(dir: &std::path::Path) -> Self { + let original = std::env::var_os("HOME"); + std::env::set_var("HOME", dir); + Self { original } + } + } - // Two backend names pointing to the same file (one with ./ prefix). - let khive_cfg = KhiveConfig { + impl Drop for HomeGuard { + fn drop(&mut self) { + match &self.original { + Some(h) => std::env::set_var("HOME", h), + None => std::env::remove_var("HOME"), + } + } + } + + fn duplicate_sqlite_path_config(db_path: &std::path::Path) -> KhiveConfig { + use khive_runtime::PackConfig; + + KhiveConfig { backends: vec![ BackendConfig { name: "main".to_string(), kind: BackendKind::Sqlite, - path: Some(db_path.clone()), + path: Some(db_path.to_path_buf()), cache_mb: None, journal_mode: None, read_only: false, @@ -3816,25 +3882,120 @@ id = "lambda:project-actor" BackendConfig { name: "alias".to_string(), kind: BackendKind::Sqlite, - path: Some(db_path.clone()), + path: Some(db_path.to_path_buf()), cache_mb: None, journal_mode: None, read_only: false, }, ], packs: { - let mut m = std::collections::HashMap::new(); - m.insert( + let mut packs = std::collections::HashMap::new(); + packs.insert( "comm".to_string(), PackConfig { backend: "alias".to_string(), }, ); - m + packs }, ..KhiveConfig::default() + } + } + + fn memory_main_backend_config() -> KhiveConfig { + KhiveConfig { + backends: vec![BackendConfig { + name: "main".to_string(), + kind: BackendKind::Memory, + path: None, + cache_mb: None, + journal_mode: None, + read_only: false, + }], + ..KhiveConfig::default() + } + } + + fn assert_db_anchor_drift(result: anyhow::Result) { + match result { + Err(error) => assert!( + error.to_string().contains("db-path resolution drift"), + "legacy builder must reject raw db input that disagrees with the resolved config: {error}" + ), + Ok(_) => panic!("legacy builder accepted raw db input that disagrees with the resolved config"), + } + } + + #[test] + fn legacy_registry_rejects_mismatched_explicit_db_override() { + let base_cfg = RuntimeConfig { + db_path: Some(PathBuf::from("/tmp/khive-resolved.db")), + ..base_runtime_config_for_multi_backend() + }; + + assert_db_anchor_drift(build_registry_for_multi_backend( + base_cfg, + &memory_main_backend_config(), + Some("/tmp/khive-raw.db"), + )); + } + + #[test] + fn legacy_server_rejects_mismatched_explicit_db_override() { + let base_cfg = RuntimeConfig { + db_path: Some(PathBuf::from("/tmp/khive-resolved.db")), + ..base_runtime_config_for_multi_backend() }; - let _ = db_path_str; // used above to show intent + + assert_db_anchor_drift(build_server_multi_backend( + base_cfg, + &memory_main_backend_config(), + Some("/tmp/khive-raw.db"), + )); + } + + #[test] + #[serial] + fn legacy_registry_rejects_unset_db_after_home_changes() { + let first_home = tempfile::tempdir().unwrap(); + let _home_guard = HomeGuard::redirect_to(first_home.path()); + let base_cfg = base_runtime_config_for_multi_backend(); + let second_home = tempfile::tempdir().unwrap(); + std::env::set_var("HOME", second_home.path()); + + assert_db_anchor_drift(build_registry_for_multi_backend( + base_cfg, + &memory_main_backend_config(), + None, + )); + } + + #[test] + #[serial] + fn legacy_server_rejects_unset_db_after_home_changes() { + let first_home = tempfile::tempdir().unwrap(); + let _home_guard = HomeGuard::redirect_to(first_home.path()); + let base_cfg = base_runtime_config_for_multi_backend(); + let second_home = tempfile::tempdir().unwrap(); + std::env::set_var("HOME", second_home.path()); + + assert_db_anchor_drift(build_server_multi_backend( + base_cfg, + &memory_main_backend_config(), + None, + )); + } + + /// B-SHOULD-FIX-2 (data safety): Two [[backends]] entries whose sqlite paths + /// canonicalize to the same file must share a single Arc and + /// run migrations only once. Verified by using two names that differ only by + /// `./` prefix while pointing at the same absolute path. + #[test] + #[serial] + fn duplicate_sqlite_paths_deduplicated_to_single_backend() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("shared.db"); + let khive_cfg = duplicate_sqlite_path_config(&db_path); let base_cfg = base_runtime_config_for_multi_backend(); @@ -3847,6 +4008,48 @@ id = "lambda:project-actor" } } + /// Regression for #720: changing `HOME` after runtime-config resolution but + /// before multi-backend registry construction must not change the database + /// anchor used by the consistency guard. + #[test] + #[serial] + fn multi_backend_boot_uses_anchor_captured_by_runtime_config() { + let first_home = tempfile::tempdir().unwrap(); + let _home_guard = HomeGuard::redirect_to(first_home.path()); + let config_path = first_home.path().join("config.toml"); + std::fs::write(&config_path, "").expect("write empty config"); + let (base_cfg, db_anchor) = resolve_runtime_config_with_db_anchor(RuntimeConfigInputs { + db: None, + config: Some(&config_path), + namespace: Namespace::parse("local").expect("namespace"), + namespace_explicit: false, + actor_explicit: false, + no_embed: true, + packs: Some(vec!["kg".to_string()]), + brain_profile: None, + }) + .expect("resolve runtime config before HOME changes"); + + let db_dir = tempfile::tempdir().unwrap(); + let db_path = db_dir.path().join("shared.db"); + let khive_cfg = duplicate_sqlite_path_config(&db_path); + + let second_home = tempfile::tempdir().unwrap(); + std::env::set_var("HOME", second_home.path()); + let result = build_server_multi_backend_with_db_anchor( + base_cfg, + &khive_cfg, + None, + db_anchor.as_deref(), + ); + if let Err(error) = result { + panic!( + "multi-backend construction must retain the anchor captured by \ + resolve_runtime_config instead of re-reading HOME: {error}" + ); + } + } + /// Issue #553 sibling gap: `build_server_multi_backend` is reachable from /// `build_server` -> `main.rs` whenever `[[backends]]` is non-empty (e.g. /// exactly one declared backend, which still routes through `build_server`'s diff --git a/crates/khive-runtime/src/config.rs b/crates/khive-runtime/src/config.rs index 339c1d918..349a2d20b 100644 --- a/crates/khive-runtime/src/config.rs +++ b/crates/khive-runtime/src/config.rs @@ -381,22 +381,38 @@ pub fn resolve_db_anchor(db: Option<&str>) -> Option { /// Assert that a resolved `db_path`: which `compute_config_id` folds into a /// process's `config_id`: agrees with what [`resolve_db_anchor`] derives from -/// the same `--db`/`KHIVE_DB` input. Call this right after `db_path` is -/// resolved at each construction boundary. +/// the same raw `--db`/`KHIVE_DB` input. /// -/// Guards against a construction path recomputing `db_path` independently of -/// `resolve_db_anchor`: left unchecked, that would silently desync -/// `config_id` from a daemon or peer sharing the same database instead of -/// failing loud. Inert (`Ok(())`) when the anchor itself is `None` (the -/// `:memory:` sentinel) since there is nothing to compare against. +/// This compatibility entry point preserves the raw-string API. Construction +/// paths that already captured the anchor should call +/// [`assert_captured_db_anchor_consistent`] so they do not re-read mutable +/// process environment. pub fn assert_db_anchor_consistent( resolved_db_path: Option<&std::path::Path>, args_db: Option<&str>, ) -> anyhow::Result<()> { - let Some(anchor) = resolve_db_anchor(args_db) else { + let db_anchor = resolve_db_anchor(args_db); + assert_captured_db_anchor_consistent(resolved_db_path, db_anchor.as_deref()) +} + +/// Assert that a resolved `db_path`: which `compute_config_id` folds into a +/// process's `config_id`: agrees with the database anchor captured from the +/// same `--db`/`KHIVE_DB` input at the construction boundary. +/// +/// Guards against a construction path recomputing `db_path` independently of +/// `resolve_db_anchor`: left unchecked, that would silently desync `config_id` +/// from a daemon or peer sharing the same database instead of failing loud. +/// The caller passes the captured anchor so validation never re-reads mutable +/// process environment. Inert (`Ok(())`) when the anchor itself is `None` (the +/// `:memory:` sentinel) since there is nothing to compare against. +pub fn assert_captured_db_anchor_consistent( + resolved_db_path: Option<&std::path::Path>, + db_anchor: Option<&std::path::Path>, +) -> anyhow::Result<()> { + let Some(anchor) = db_anchor else { return Ok(()); }; - if resolved_db_path != Some(anchor.as_path()) { + if resolved_db_path != Some(anchor) { anyhow::bail!( "db-path resolution drift at server construction: resolved db_path {:?} \ does not match the canonical anchor {:?} computed by resolve_db_anchor \ @@ -690,7 +706,8 @@ mod resolve_db_anchor_tests { #[cfg(test)] mod assert_db_anchor_consistent_tests { - use super::{assert_db_anchor_consistent, resolve_db_anchor}; + use super::{assert_captured_db_anchor_consistent, resolve_db_anchor}; + use crate::assert_db_anchor_consistent; #[test] fn diverging_db_path_is_rejected_naming_both_paths() { @@ -698,8 +715,9 @@ mod assert_db_anchor_consistent_tests { let anchor = resolve_db_anchor(Some(args_db)).expect("explicit path always anchors"); let wrong = std::path::PathBuf::from("/tmp/khive-anchor-guard-wrong.db"); - let err = assert_db_anchor_consistent(Some(wrong.as_path()), Some(args_db)) - .expect_err("a resolved db_path diverging from the anchor must be rejected"); + let err = + assert_captured_db_anchor_consistent(Some(wrong.as_path()), Some(anchor.as_path())) + .expect_err("a resolved db_path diverging from the anchor must be rejected"); let msg = err.to_string(); assert!( @@ -716,7 +734,11 @@ mod assert_db_anchor_consistent_tests { fn matching_explicit_db_path_passes() { let args_db = "/tmp/khive-anchor-guard-consistent.db"; let anchor = resolve_db_anchor(Some(args_db)).expect("explicit path always anchors"); - assert!(assert_db_anchor_consistent(Some(anchor.as_path()), Some(args_db)).is_ok()); + assert!(assert_captured_db_anchor_consistent( + Some(anchor.as_path()), + Some(anchor.as_path()) + ) + .is_ok()); } #[test] @@ -725,8 +747,8 @@ mod assert_db_anchor_consistent_tests { // path to assert against, so the guard passes regardless of what // `resolved_db_path` happens to carry. let bogus = std::path::PathBuf::from("/tmp/should-not-matter.db"); - assert!(assert_db_anchor_consistent(Some(bogus.as_path()), Some(":memory:")).is_ok()); - assert!(assert_db_anchor_consistent(None, Some(":memory:")).is_ok()); + assert!(assert_captured_db_anchor_consistent(Some(bogus.as_path()), None).is_ok()); + assert!(assert_captured_db_anchor_consistent(None, None).is_ok()); } #[test] @@ -736,7 +758,17 @@ mod assert_db_anchor_consistent_tests { // concrete anchor), so a runtime whose resolved `db_path` matches // passes silently. let anchor = resolve_db_anchor(None); - assert!(assert_db_anchor_consistent(anchor.as_deref(), None).is_ok()); + assert!(assert_captured_db_anchor_consistent(anchor.as_deref(), anchor.as_deref()).is_ok()); + } + + #[test] + fn public_compatibility_wrapper_accepts_path_and_memory_sentinel() { + let args_db = "/tmp/khive-anchor-guard-public-api.db"; + let anchor = resolve_db_anchor(Some(args_db)).expect("explicit path always anchors"); + assert!(assert_db_anchor_consistent(Some(anchor.as_path()), Some(args_db)).is_ok()); + + let unrelated = std::path::Path::new("/tmp/khive-anchor-guard-unrelated.db"); + assert!(assert_db_anchor_consistent(Some(unrelated), Some(":memory:")).is_ok()); } } diff --git a/crates/khive-runtime/src/lib.rs b/crates/khive-runtime/src/lib.rs index 60d216c6c..836bc5f5e 100644 --- a/crates/khive-runtime/src/lib.rs +++ b/crates/khive-runtime/src/lib.rs @@ -102,9 +102,9 @@ pub use registry::{ObjectiveRegistry, RegisteredObjective}; pub use resource::{cpu_delta_us, process_resource_usage, ProcessResourceUsage}; pub use retrieval::{SearchHit, SearchSource}; pub use runtime::{ - assert_db_anchor_consistent, parse_pack_list, resolve_db_anchor, resolve_project_actor_id, - runtime_config_from_khive_config, BackendId, EntityTypeValidatorFn, KhiveRuntime, - NamespaceToken, NoteMutationHookFn, RuntimeConfig, + assert_captured_db_anchor_consistent, assert_db_anchor_consistent, parse_pack_list, + resolve_db_anchor, resolve_project_actor_id, runtime_config_from_khive_config, BackendId, + EntityTypeValidatorFn, KhiveRuntime, NamespaceToken, NoteMutationHookFn, RuntimeConfig, }; pub use secret_gate::SecretMatch; pub use validation::{ diff --git a/crates/khive-runtime/src/runtime.rs b/crates/khive-runtime/src/runtime.rs index 06a160076..5be81fe88 100644 --- a/crates/khive-runtime/src/runtime.rs +++ b/crates/khive-runtime/src/runtime.rs @@ -44,8 +44,9 @@ pub type NoteMutationHookFn = Arc< >; pub use crate::config::{ - assert_db_anchor_consistent, parse_pack_list, resolve_db_anchor, resolve_project_actor_id, - runtime_config_from_khive_config, BackendId, NamespaceToken, RuntimeConfig, + assert_captured_db_anchor_consistent, assert_db_anchor_consistent, parse_pack_list, + resolve_db_anchor, resolve_project_actor_id, runtime_config_from_khive_config, BackendId, + NamespaceToken, RuntimeConfig, }; // ---- KhiveRuntime ---- diff --git a/crates/kkernel/src/exec.rs b/crates/kkernel/src/exec.rs index 52481799a..802872607 100644 --- a/crates/kkernel/src/exec.rs +++ b/crates/kkernel/src/exec.rs @@ -31,9 +31,11 @@ use std::path::PathBuf; use anyhow::{Context, Result}; use clap::Parser; +#[cfg(test)] +use khive_mcp::serve::resolve_runtime_config; use khive_mcp::serve::{ - apply_env_output_format, build_server_multi_backend, config_discovery_db_anchor, - enforce_strict_actor_mode, resolve_runtime_config, RuntimeConfigInputs, + apply_env_output_format, build_server_multi_backend_with_db_anchor, config_discovery_db_anchor, + enforce_strict_actor_mode, RuntimeConfigInputs, }; #[cfg(unix)] use khive_mcp::server::compute_config_id; @@ -434,37 +436,45 @@ pub async fn run_exec(args: ExecArgs) -> Result<()> { // forwarded frame as a `ConfigMismatch` and `exec` silently fell back to an // in-process, TOML-blind, effectively-anonymous dispatch (issue #581). let namespace = Namespace::parse(&args.namespace).map_err(|e| anyhow::anyhow!("{e}"))?; - let cfg = resolve_runtime_config(RuntimeConfigInputs { - db: args.db.as_deref(), - config: None, // `kkernel exec` has no `--config` flag today - namespace, - // `--namespace` has a clap `default_value = "local"`, so it is always - // present — there is no way to distinguish "operator typed --namespace - // local" from "operator didn't pass --namespace at all". `true` is the - // conservative, behavior-preserving choice: it keeps exec's pre-existing - // semantics (the CLI/default value always becomes `default_namespace`, - // matching what `resolve_runtime_config`'s embed path already did - // unconditionally). It is also empirically inert for config_id parity: - // in the embed path (`no_embed: false`, exec's only mode), this flag - // gates only the actor_id fill-when-None guard in `resolve_runtime_config` - // — and `compute_config_id` never reads identity fields (`actor_id` or - // `visible_namespaces`; namespace is carried separately per its own doc - // comment). See the - // `namespace_explicit_changes_actor_id_fill_but_not_config_id` and - // `exec_config_id_matches_serve_config_id_for_project_toml_actor` tests - // below, which construct both arms and assert this directly rather than - // assuming it. - namespace_explicit: true, - actor_explicit: false, - no_embed: false, - packs: None, - brain_profile: None, - })?; + let (cfg, db_anchor) = + khive_mcp::serve::resolve_runtime_config_with_db_anchor(RuntimeConfigInputs { + db: args.db.as_deref(), + config: None, // `kkernel exec` has no `--config` flag today + namespace, + // `--namespace` has a clap `default_value = "local"`, so it is always + // present — there is no way to distinguish "operator typed --namespace + // local" from "operator didn't pass --namespace at all". `true` is the + // conservative, behavior-preserving choice: it keeps exec's pre-existing + // semantics (the CLI/default value always becomes `default_namespace`, + // matching what `resolve_runtime_config`'s embed path already did + // unconditionally). It is also empirically inert for config_id parity: + // in the embed path (`no_embed: false`, exec's only mode), this flag + // gates only the actor_id fill-when-None guard in `resolve_runtime_config` + // — and `compute_config_id` never reads identity fields (`actor_id` or + // `visible_namespaces`; namespace is carried separately per its own doc + // comment). See the + // `namespace_explicit_changes_actor_id_fill_but_not_config_id` and + // `exec_config_id_matches_serve_config_id_for_project_toml_actor` tests + // below, which construct both arms and assert this directly rather than + // assuming it. + namespace_explicit: true, + actor_explicit: false, + no_embed: false, + packs: None, + brain_profile: None, + })?; // Regression fence: `cfg.db_path` must agree with the canonical anchor for // this same `--db`/`KHIVE_DB` input, or `compute_config_id` would silently // desynchronize `kkernel exec` from the daemon it is trying to reach. - khive_runtime::assert_db_anchor_consistent(cfg.db_path.as_deref(), args.db.as_deref())?; + khive_runtime::assert_captured_db_anchor_consistent( + cfg.db_path.as_deref(), + db_anchor.as_deref(), + )?; + let db_context = ExecDbContext { + raw: args.db, + anchor: db_anchor, + }; match mode { ExecMode::Inline(ops) => { @@ -474,7 +484,7 @@ pub async fn run_exec(args: ExecArgs) -> Result<()> { args.presentation, args.output_format, args.save_file, - args.db, + db_context, ) .await } @@ -484,7 +494,7 @@ pub async fn run_exec(args: ExecArgs) -> Result<()> { cfg, args.presentation, args.dry_run, - args.db, + db_context, args.atomic, args.atomic_max_ops, ) @@ -498,13 +508,19 @@ enum ExecMode { OpsFile(PathBuf), } +#[derive(Default)] +struct ExecDbContext { + raw: Option, + anchor: Option, +} + async fn run_exec_inline( ops: String, cfg: RuntimeConfig, presentation: Option, output_format: Option, save_file: Option, - db: Option, + db_context: ExecDbContext, ) -> Result<()> { #[cfg(unix)] return run_exec_inline_with_forward( @@ -513,13 +529,20 @@ async fn run_exec_inline( presentation, output_format, save_file, - db, + db_context, forward_or_spawn_boxed, ) .await; #[cfg(not(unix))] - return run_exec_inline_with_forward(ops, cfg, presentation, output_format, save_file, db) - .await; + return run_exec_inline_with_forward( + ops, + cfg, + presentation, + output_format, + save_file, + db_context, + ) + .await; } /// Inner implementation of `run_exec_inline`, parameterised over the daemon @@ -541,7 +564,7 @@ async fn run_exec_inline_with_forward( presentation: Option, output_format: Option, save_file: Option, - db: Option, + db_context: ExecDbContext, #[cfg(unix)] forward_fn: ForwardFnPtr, ) -> Result<()> { // ── strict-actor gate (before any forwarding) ───────────────────────────── @@ -574,7 +597,7 @@ async fn run_exec_inline_with_forward( // diverged, so a correctly-configured client was rejected as a // `ConfigMismatch` and silently fell back to the cold in-process path on // every call. - let db_path_for_config = config_discovery_db_anchor(db.as_deref()); + let db_path_for_config = config_discovery_db_anchor(db_context.raw.as_deref()); let khive_cfg = KhiveConfig::load_with_home_fallback(None, db_path_for_config.as_deref()) .map_err(|e| anyhow::anyhow!("config error: {e}"))? .unwrap_or_default(); @@ -627,7 +650,12 @@ async fn run_exec_inline_with_forward( // precedence chain (env var over TOML `[runtime] default_output_format` // over builtin json) AND honors `[[backends]]` multi-backend topology — // see its doc comment. - let server = build_local_fallback_server(cfg, &khive_cfg, db.as_deref())?; + let server = build_local_fallback_server( + cfg, + &khive_cfg, + db_context.raw.as_deref(), + db_context.anchor.as_deref(), + )?; let params = RequestParams { ops, @@ -668,17 +696,20 @@ async fn run_exec_inline_with_forward( /// `khive_cfg.backends` is empty, build the plain single-backend server /// exactly as before (byte-identical `config_id`, since `compute_config_id` /// skips the topology fold for an empty backends list); otherwise delegate to -/// `build_server_multi_backend`, the same constructor `kkernel mcp` uses. +/// `build_server_multi_backend_with_db_anchor`, the captured-anchor constructor +/// used by the production MCP boot path. /// /// `cli_db_override` is the raw, pre-resolution `--db`/`KHIVE_DB` value — -/// required by `build_server_multi_backend`'s db-anchor consistency guard and -/// its `--db :memory:` multi-backend override handling (ADR-028 §8); passing -/// the wrong value here would either falsely reject a legitimate `--db` or -/// silently ignore an operator's `:memory:` isolation request. +/// required for `--db :memory:` multi-backend override handling (ADR-028 §8). +/// Passing the wrong value here would silently ignore an operator's in-memory +/// isolation request. +/// `db_anchor` is the canonical anchor captured alongside `cfg`; passing it +/// through prevents fallback construction from re-reading a changed `HOME`. fn build_local_fallback_server( cfg: RuntimeConfig, khive_cfg: &KhiveConfig, cli_db_override: Option<&str>, + db_anchor: Option<&std::path::Path>, ) -> Result { // Held across construction below (`KhiveRuntime::new` / `KhiveMcpServer::new` // / `build_server_multi_backend`, both of which run migrations and apply @@ -691,17 +722,16 @@ fn build_local_fallback_server( .map_err(|e| anyhow::anyhow!("{e}"))? .with_default_output_format(env_fmt)) } else { - build_server_multi_backend(cfg, khive_cfg, cli_db_override) + build_server_multi_backend_with_db_anchor(cfg, khive_cfg, cli_db_override, db_anchor) } } -#[allow(clippy::too_many_arguments)] async fn run_exec_ops_file( path: PathBuf, cfg: RuntimeConfig, presentation: Option, dry_run: bool, - db: Option, + db_context: ExecDbContext, atomic: bool, atomic_max_ops: Option, ) -> Result<()> { @@ -736,7 +766,7 @@ async fn run_exec_ops_file( // `[[backends]]` multi-backend topology exactly like the daemon-fallback // path — see `build_local_fallback_server`. enforce_strict_actor_mode(cfg.actor_id.as_deref(), &cfg.packs)?; - let db_path_for_config = config_discovery_db_anchor(db.as_deref()); + let db_path_for_config = config_discovery_db_anchor(db_context.raw.as_deref()); let khive_cfg = KhiveConfig::load_with_home_fallback(None, db_path_for_config.as_deref()) .map_err(|e| anyhow::anyhow!("config error: {e}"))? .unwrap_or_default(); @@ -753,7 +783,12 @@ async fn run_exec_ops_file( return Ok(()); } - let server = build_local_fallback_server(cfg, &khive_cfg, db.as_deref())?; + let server = build_local_fallback_server( + cfg, + &khive_cfg, + db_context.raw.as_deref(), + db_context.anchor.as_deref(), + )?; apply_ops_file(&server, ops, presentation).await } @@ -1241,7 +1276,7 @@ default = true // `db_path` here is NOT the actual storage location when `[[backends]]` // is declared — `build_server_multi_backend` opens each backend's own // declared path (the tempfiles above) independently. It is only the - // identity/fingerprint value `assert_db_anchor_consistent` checks + // identity/fingerprint value `assert_captured_db_anchor_consistent` checks // against `resolve_db_anchor(cli_db_override)`, exactly mirroring what // a real `kkernel exec` invocation with NO explicit `--db` flag would // resolve to (the realistic shape when `[[backends]]` fully governs @@ -1260,7 +1295,8 @@ default = true // matching the `cfg.db_path` shape above. An explicit override here // would be rejected as ambiguous by `build_registry_for_multi_backend` // (ADR-028 §8) since 2 backends are already declared. - let server = build_local_fallback_server(cfg, &khive_cfg, None) + let db_anchor = cfg.db_path.clone(); + let server = build_local_fallback_server(cfg, &khive_cfg, None, db_anchor.as_deref()) .expect("multi-backend local fallback must build"); let send = server @@ -1333,6 +1369,42 @@ default = true ); } + #[test] + #[serial] + fn build_local_fallback_server_uses_captured_anchor_after_home_changes() { + let (previous_home, _first_home) = isolate_home_for_test(); + let cfg = RuntimeConfig { + db_path: khive_runtime::resolve_db_anchor(None), + embedding_model: None, + additional_embedding_models: vec![], + packs: vec!["kg".to_string()], + ..RuntimeConfig::default() + }; + let db_anchor = cfg.db_path.clone(); + let khive_cfg = KhiveConfig { + backends: vec![khive_runtime::BackendConfig { + name: "main".to_string(), + kind: khive_runtime::BackendKind::Memory, + path: None, + cache_mb: None, + journal_mode: None, + read_only: false, + }], + ..KhiveConfig::default() + }; + let second_home = tempfile::tempdir().expect("second HOME"); + std::env::set_var("HOME", second_home.path()); + + let result = build_local_fallback_server(cfg, &khive_cfg, None, db_anchor.as_deref()); + restore_home(previous_home); + + assert!( + result.is_ok(), + "exec fallback must use the anchor captured with RuntimeConfig after HOME changes: {}", + result.err().unwrap() + ); + } + // ── guarded local construction races a guarded boot (#667/#645) ────────── // // Mirrors `khive-runtime/tests/cold_boot_fts_race.rs`'s deterministic @@ -1408,7 +1480,7 @@ default = true // built `KhiveRuntime`/`KhiveMcpServer` without acquiring any // guard, so it could run migrations/FTS DDL concurrently with // the guarded boot above against the same file. - let server = build_local_fallback_server(cfg, &khive_cfg, None) + let server = build_local_fallback_server(cfg, &khive_cfg, None, None) .expect("guarded local-exec construction must succeed"); for i in 0..count { @@ -1485,8 +1557,8 @@ default = true // The exact call under test: every non-atomic local-exec path // (daemon-unreachable fallback, --save-file, KHIVE_NO_DAEMON=1, // non-atomic --ops-file) funnels through this one function. - let result = - rt_handle.block_on(async { build_local_fallback_server(cfg, &khive_cfg, None) }); + let result = rt_handle + .block_on(async { build_local_fallback_server(cfg, &khive_cfg, None, None) }); // Sent only AFTER construction returns — the test observes // whether this arrives before or after the lock is released. let _ = tx.send(()); @@ -1817,9 +1889,17 @@ default = true }; // dry_run=true → no writes. - run_exec_ops_file(path.clone(), cfg.clone(), None, true, None, false, None) - .await - .unwrap(); + run_exec_ops_file( + path.clone(), + cfg.clone(), + None, + true, + ExecDbContext::default(), + false, + None, + ) + .await + .unwrap(); // Verify nothing was written by checking with a fresh server. let server = isolated_server(&db_path); @@ -1877,7 +1957,15 @@ default = true ..RuntimeConfig::default() }; - let result = run_exec_inline("stats()".to_string(), cfg, None, None, None, None).await; + let result = run_exec_inline( + "stats()".to_string(), + cfg, + None, + None, + None, + ExecDbContext::default(), + ) + .await; // Restore env. match prev_strict { @@ -1924,7 +2012,15 @@ default = true }; // The strict gate must pass; the actual dispatch will succeed (stats() is safe). - let result = run_exec_inline("stats()".to_string(), cfg, None, None, None, None).await; + let result = run_exec_inline( + "stats()".to_string(), + cfg, + None, + None, + None, + ExecDbContext::default(), + ) + .await; match prev_strict { Some(v) => std::env::set_var("KHIVE_REQUIRE_ATTRIBUTED_ACTOR", v), @@ -1961,7 +2057,15 @@ default = true ..RuntimeConfig::default() }; - let result = run_exec_inline("stats()".to_string(), cfg, None, None, None, None).await; + let result = run_exec_inline( + "stats()".to_string(), + cfg, + None, + None, + None, + ExecDbContext::default(), + ) + .await; match prev_strict { Some(v) => std::env::set_var("KHIVE_REQUIRE_ATTRIBUTED_ACTOR", v), @@ -2039,7 +2143,7 @@ default = true None, None, // output_format None, - None, // db + ExecDbContext::default(), spy_forward_records_call, ) .await; @@ -2095,7 +2199,7 @@ default = true None, None, // output_format None, - None, // db + ExecDbContext::default(), spy_forward_records_call, ) .await; @@ -2213,7 +2317,7 @@ backend = "sessions" None, None, None, - None, // db: no explicit --db, matching default discovery + ExecDbContext::default(), spy_capture_config_id, ) .await; diff --git a/crates/kkernel/src/main.rs b/crates/kkernel/src/main.rs index 277215355..c6f54fbe7 100644 --- a/crates/kkernel/src/main.rs +++ b/crates/kkernel/src/main.rs @@ -299,22 +299,23 @@ async fn main() -> Result<()> { let (cli_ns_explicit, cli_ns) = khive_mcp::args::resolve_cli_namespace(&a) .map_err(|e| anyhow::anyhow!("{e}"))?; - let base_cfg = khive_mcp::serve::resolve_runtime_config( - khive_mcp::serve::RuntimeConfigInputs { - db: a.db.as_deref(), - config: a.config.as_deref(), - namespace: cli_ns, - namespace_explicit: cli_ns_explicit, - actor_explicit: cli_ns_explicit, - no_embed: a.no_embed, - packs: if a.pack.is_empty() { - None - } else { - Some(a.pack.clone()) + let (base_cfg, db_anchor) = + khive_mcp::serve::resolve_runtime_config_with_db_anchor( + khive_mcp::serve::RuntimeConfigInputs { + db: a.db.as_deref(), + config: a.config.as_deref(), + namespace: cli_ns, + namespace_explicit: cli_ns_explicit, + actor_explicit: cli_ns_explicit, + no_embed: a.no_embed, + packs: if a.pack.is_empty() { + None + } else { + Some(a.pack.clone()) + }, + brain_profile: a.brain_profile.clone(), }, - brain_profile: a.brain_profile.clone(), - }, - )?; + )?; // #667: acquire the boot/recovery lock before building the // coordinator server — that construction runs migrations and @@ -334,11 +335,13 @@ async fn main() -> Result<()> { #[cfg(not(unix))] let boot_guard: Option = None; - let (server, schedule_rt) = build_multi_backend_server_with_coordinator( - base_cfg, - &khive_cfg, - a.db.as_deref(), - )?; + let (server, schedule_rt) = + build_multi_backend_server_with_coordinator_and_db_anchor( + base_cfg, + &khive_cfg, + a.db.as_deref(), + db_anchor.as_deref(), + )?; khive_mcp::serve::serve_server( server, @@ -439,13 +442,37 @@ fn resolve_command(exec: Option, command: Option) -> Command { /// (`spawn_schedule_tick_loop_if_daemon`) drains the exact backend this /// coordinator-attached boot resolved, never a re-derived config (PR /// #782). +#[cfg(test)] fn build_multi_backend_server_with_coordinator( base_cfg: RuntimeConfig, khive_cfg: &KhiveConfig, cli_db_override: Option<&str>, ) -> Result<(khive_mcp::server::KhiveMcpServer, Option)> { - let multi = - khive_mcp::serve::build_registry_for_multi_backend(base_cfg, khive_cfg, cli_db_override)?; + let db_anchor = if cli_db_override == Some(":memory:") { + None + } else { + base_cfg.db_path.clone() + }; + build_multi_backend_server_with_coordinator_and_db_anchor( + base_cfg, + khive_cfg, + cli_db_override, + db_anchor.as_deref(), + ) +} + +fn build_multi_backend_server_with_coordinator_and_db_anchor( + base_cfg: RuntimeConfig, + khive_cfg: &KhiveConfig, + cli_db_override: Option<&str>, + db_anchor: Option<&std::path::Path>, +) -> Result<(khive_mcp::server::KhiveMcpServer, Option)> { + let multi = khive_mcp::serve::build_registry_for_multi_backend_with_db_anchor( + base_cfg, + khive_cfg, + cli_db_override, + db_anchor, + )?; let schedule_rt = multi .per_pack_runtimes @@ -1150,8 +1177,13 @@ mod tests { }; let khive_cfg = KhiveConfig::default(); - let result = - build_multi_backend_server_with_coordinator(base_cfg, &khive_cfg, Some(args_db)); + let db_anchor = khive_runtime::resolve_db_anchor(Some(args_db)); + let result = build_multi_backend_server_with_coordinator_and_db_anchor( + base_cfg, + &khive_cfg, + Some(args_db), + db_anchor.as_deref(), + ); let err = match result { Ok(_) => panic!( @@ -1173,6 +1205,70 @@ mod tests { ); } + /// Regression for #720: the coordinator-attached `kkernel mcp` path must + /// retain the HOME-derived anchor captured during runtime-config resolution + /// when HOME changes before registry construction. + #[test] + #[serial] + fn coordinator_boot_uses_anchor_captured_by_runtime_config() { + struct HomeGuard(Option); + + impl Drop for HomeGuard { + fn drop(&mut self) { + match &self.0 { + Some(home) => std::env::set_var("HOME", home), + None => std::env::remove_var("HOME"), + } + } + } + + let original_home = std::env::var_os("HOME"); + let _home_guard = HomeGuard(original_home); + let first_home = TempDir::new().expect("first HOME"); + std::env::set_var("HOME", first_home.path()); + let config_path = first_home.path().join("config.toml"); + std::fs::write(&config_path, "").expect("write empty config"); + + let (base_cfg, db_anchor) = khive_mcp::serve::resolve_runtime_config_with_db_anchor( + khive_mcp::serve::RuntimeConfigInputs { + db: None, + config: Some(&config_path), + namespace: khive_runtime::Namespace::parse("local").expect("namespace"), + namespace_explicit: false, + actor_explicit: false, + no_embed: true, + packs: Some(vec!["kg".to_string()]), + brain_profile: None, + }, + ) + .expect("resolve runtime config before HOME changes"); + + let mut khive_cfg = single_main_backend_config(khive_runtime::BackendKind::Memory, None); + khive_cfg.backends.push(khive_runtime::BackendConfig { + name: "secondary".to_string(), + kind: khive_runtime::BackendKind::Memory, + path: None, + cache_mb: None, + journal_mode: None, + read_only: false, + }); + + let second_home = TempDir::new().expect("second HOME"); + std::env::set_var("HOME", second_home.path()); + let result = build_multi_backend_server_with_coordinator_and_db_anchor( + base_cfg, + &khive_cfg, + None, + db_anchor.as_deref(), + ); + if let Err(error) = result { + panic!( + "coordinator-attached construction must retain the anchor captured by \ + resolve_runtime_config instead of re-reading HOME: {error}" + ); + } + } + // --- #674: coordinator link-target resolution parity with `get` --- /// Regression for #674: a full-UUID `link(..., relation="annotates")` whose