diff --git a/crates/travsr-mcp/src/lib.rs b/crates/travsr-mcp/src/lib.rs index d3d222a2..5a45a416 100644 --- a/crates/travsr-mcp/src/lib.rs +++ b/crates/travsr-mcp/src/lib.rs @@ -132,7 +132,7 @@ fn inject_embed_hook(store: &mut SqliteStore, db_path: &Path) { use travsr_error::StoreError; use travsr_plugin_host::{ - active_backend_id, embed_backends, lookup_embed_backend, EmbedQueryHook, EmbedSupervisor, + embed_backends, lookup_embed_backend, EmbedQueryHook, EmbedSupervisor, }; use travsr_store::{EmbedKnnHook, EmbedReadiness, EmbedScoreHook}; @@ -143,7 +143,13 @@ fn inject_embed_hook(store: &mut SqliteStore, db_path: &Path) { } let Some(home) = dirs::home_dir() else { return }; - let backend = active_backend_id() + // Prefer the repo's own `.travsr/embed.toml` override, then the user's + // machine-wide active backend from ~/.travsr/embed.toml, then the catalog + // default so a fresh install without `travsr embed switch` still works. + // Mirrors `resolve_backend`'s resolution order (travsr-plugin-host) — this + // is a per-repo embedding decision, not an install/list/hint path, so it + // must not resolve on `active_backend_id()` alone (#547). + let backend = hook_backend_id(db_path) .as_deref() .and_then(lookup_embed_backend) .or_else(|| embed_backends().first()) @@ -286,3 +292,117 @@ fn inject_embed_hook(store: &mut SqliteStore, db_path: &Path) { store.set_embed_score_hook(meta_score); tracing::info!("embed plugin hook installed (lazy, sidecar starting in background)"); } + +/// Resolve the embed backend id to use for hook injection at `db_path`, +/// preferring the repo's own `.travsr/embed.toml` override over the +/// machine-wide `~/.travsr/embed.toml` default. Mirrors `resolve_backend`'s +/// resolution order in travsr-plugin-host — hook injection is a per-repo +/// embedding decision, so it must not resolve on `active_backend_id()` alone +/// (#547: a repo-level `travsr embed switch` was silently ignored, causing +/// `travsr mcp --stdio` to spawn the wrong sidecar model and disable Step 4). +/// +/// Same helper as `travsr-daemon`'s `hook_backend_id` (#526), which fixed the +/// identical resolution order in the daemon's own hook injection. +/// +/// `parent().parent()` assumes `db_path` is `/.travsr/graph.db`, which +/// holds for the default path. An explicit `travsr mcp --stdio --db ` +/// outside a repo yields no `embed.toml` there and falls through to the +/// machine default, which is the pre-existing behaviour and not a regression. +/// `resolve_backend_paths` on the spawn side makes the same assumption from +/// the same input, so hook and spawn still agree in that case, which is the +/// invariant that matters (#770 review). +fn hook_backend_id(db_path: &Path) -> Option { + use travsr_plugin_host::{active_backend_id, repo_backend_id}; + + let repo_root = db_path.parent().and_then(|p| p.parent()); + repo_root + .and_then(repo_backend_id) + .or_else(active_backend_id) +} + +#[cfg(test)] +mod tests { + use super::*; + + // HOME is process-global and Rust tests run in parallel, so every test + // that mutates it must serialize on ONE lock. Reuses the existing + // `seed::DOCS_ENV_LOCK` rather than declaring a second: these tests and + // the docs-lane tests share the `travsr_mcp` lib test binary, so two + // disjoint mutexes would serialize each group against itself while still + // racing the other. That is not hypothetical here; DOCS_ENV_LOCK's own doc + // records it happening once already, and #770's review demonstrated this + // pair interleaving (#770 review). + use crate::seed::DOCS_ENV_LOCK as ENV_LOCK; + + /// #547: hook injection must prefer a repo's own `.travsr/embed.toml` + /// override over the machine-wide `~/.travsr/embed.toml` default, the + /// same resolution order `resolve_backend` (travsr-plugin-host) uses. + #[test] + fn hook_backend_id_prefers_repo_config_over_global() { + let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let home = tempfile::tempdir().unwrap(); + let repo = tempfile::tempdir().unwrap(); + let old_home = std::env::var_os("HOME"); + std::env::set_var("HOME", home.path()); + + std::fs::create_dir_all(home.path().join(".travsr")).unwrap(); + std::fs::write( + home.path().join(".travsr").join("embed.toml"), + "active = \"global-backend\"\n", + ) + .unwrap(); + + let repo_travsr = repo.path().join(".travsr"); + std::fs::create_dir_all(&repo_travsr).unwrap(); + std::fs::write( + repo_travsr.join("embed.toml"), + "active = \"repo-backend\"\n", + ) + .unwrap(); + + let resolved = hook_backend_id(&repo_travsr.join("graph.db")); + + match old_home { + Some(h) => std::env::set_var("HOME", h), + None => std::env::remove_var("HOME"), + } + + assert_eq!( + resolved.as_deref(), + Some("repo-backend"), + "repo's .travsr/embed.toml must win over the machine-wide default" + ); + } + + /// #547: with no repo-level override the machine-wide default still applies, + /// so the fix narrows nothing that used to work. + #[test] + #[cfg_attr( + windows, + ignore = "dirs::home_dir() on Windows ignores HOME/USERPROFILE entirely (SHGetKnownFolderPath) - this test's isolation cannot work there, see crates/travsr-cli/tests/embed_switch.rs's module doc comment" + )] + fn hook_backend_id_falls_back_to_global_when_repo_unconfigured() { + let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let home = tempfile::tempdir().unwrap(); + let repo = tempfile::tempdir().unwrap(); + let old_home = std::env::var_os("HOME"); + std::env::set_var("HOME", home.path()); + + std::fs::create_dir_all(home.path().join(".travsr")).unwrap(); + std::fs::write( + home.path().join(".travsr").join("embed.toml"), + "active = \"global-backend\"\n", + ) + .unwrap(); + + // No repo .travsr/embed.toml written at all. + let resolved = hook_backend_id(&repo.path().join(".travsr").join("graph.db")); + + match old_home { + Some(h) => std::env::set_var("HOME", h), + None => std::env::remove_var("HOME"), + } + + assert_eq!(resolved.as_deref(), Some("global-backend")); + } +}