From c0224e20cbe3dd5bb45ff1f3d383aff554ffec34 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 27 Aug 2026 15:12:22 -0600 Subject: [PATCH] test(mcp): drive session re-attach and RAG attach filter guards through their real entry points - use_session_applies_persisted_mcp_tools_immediately now loads a session file from disk through the real use_session, proving the post-assignment filter refresh applies a re-attached session's persisted allowlist. - New use_rag_does_not_drop_role_filters loads a yaml-driver RAG through the real use_rag and proves the tool-scope rebuild recomputes the role's filter layer instead of dropping it. - Seed the process-wide client/model registries in a pre-main ctor (new ctor dev-dependency) so model resolution is deterministic across test orderings; the seed exposes only an embedding model so tests that assert 'no chat model available' keep their premise. --- Cargo.lock | 23 ++++++++ Cargo.toml | 3 +- src/config/request_context.rs | 102 ++++++++++++++++++++++++++++------ 3 files changed, 111 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d856e4a..0208f01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1685,6 +1685,7 @@ dependencies = [ "colored", "comfy-table", "crossterm 0.29.0", + "ctor", "dirs", "duckdb", "duct", @@ -1907,6 +1908,16 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "ctor" +version = "1.0.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "914a755b7c2d4af2bdcff7ce1739e2db9a1b81a9b07123d8015786ae03c0980d" +dependencies = [ + "link-section", + "linktime-proc-macro", +] + [[package]] name = "ctutils" version = "0.4.2" @@ -3833,6 +3844,18 @@ dependencies = [ "libc", ] +[[package]] +name = "link-section" +version = "0.19.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39c29a617ce3df32c08497bdc1ab6e2376e0b17948ac166a2fbe5977c5954cd9" + +[[package]] +name = "linktime-proc-macro" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e57c38c1e860fd37c604281cdfb1dd2216977fd76a50f85ba2f388ef3219616" + [[package]] name = "linux-raw-sys" version = "0.4.15" diff --git a/Cargo.toml b/Cargo.toml index fbfa196..784f512 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -143,6 +143,7 @@ arboard = { version = "3.3.0", default-features = false } libc = "0.2" [dev-dependencies] +ctor = "1.0.13" pretty_assertions = "1.4.0" rmcp = { version = "3.1.2", features = ["server"] } serial_test = "3" @@ -154,4 +155,4 @@ path = "src/main.rs" [profile.release] lto = true strip = true -opt-level = "z" \ No newline at end of file +opt-level = "z" diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 8018bab..bbf711d 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -4901,6 +4901,39 @@ mod tests { use std::time::{Instant, SystemTime, UNIX_EPOCH}; use std::{env, mem}; + // `list_client_names` / `list_all_models` cache the first AppConfig they + // see in process-wide OnceLocks. Several tests reach them through configs + // with an empty client list, which would permanently pin every later + // model lookup in this process to "unknown model" and make any test that + // needs a resolvable model dependent on test ordering. Seed the caches + // before any test runs with one client, "test-seeded", exposing a single + // embedding model. Deliberately NO chat model: some tests assert that no + // chat model is available, and chat lookups don't need one — a + // "test-seeded:" chat id resolves through the create-from-name + // fallback because the client name is registered. + // + // `unsafe` is ctor's required acknowledgment that this runs before main; + // the body only allocates and initializes OnceLocks, both of which are + // sound pre-main. + #[ctor::ctor(unsafe)] + fn seed_model_registries() { + use crate::client::{ClientConfig, ModelData, list_all_models, list_client_names}; + + let mut client = ClientConfig::default(); + if let ClientConfig::OpenAIConfig(config) = &mut client { + config.name = Some("test-seeded".to_string()); + let mut embedder = ModelData::new("test-embedder"); + embedder.model_type = "embedding".to_string(); + config.models = vec![embedder]; + } + let config = AppConfig { + clients: vec![client], + ..AppConfig::default() + }; + let _ = list_client_names(&config); + let _ = list_all_models(&config); + } + struct TestConfigDirGuard { key: String, previous: Option, @@ -8199,25 +8232,23 @@ mod tests { #[test] #[serial] fn use_session_applies_persisted_mcp_tools_immediately() { - // Mirrors use_session's ordering hazard: the tool-scope rebuild runs - // before `self.session` is assigned, so a re-attached session's - // persisted map is only enforced by the post-assignment refresh. - // (Session::load_from_ctx needs live model resolution, so the - // persisted session is round-tripped through serde directly.) + // use_session rebuilds the tool scope BEFORE `self.session` is + // assigned, so a re-attached session's persisted allowlist is + // enforced only by the filter refresh that runs after the + // assignment. Drive the real use_session against a session file on + // disk to prove that refresh happens. let _guard = TestConfigDirGuard::new(); let mut ctx = RequestContext::new(mcp_app_state(&["gh"]), WorkingMode::Cmd); - let mut persisted = Session::default(); - persisted.set_mcp_tools(Some(gh_get_only_map())); - let yaml = serde_yaml::to_string(&persisted).unwrap(); - let reloaded: Session = serde_yaml::from_str(&yaml).unwrap(); + let session_path = ctx.session_file("persisted"); + ensure_parent_exists(&session_path).unwrap(); + write( + &session_path, + "model: test-seeded:test-chat\nmessages: []\nmcp_tools:\n gh:\n - get_*\n", + ) + .unwrap(); + let app = ctx.app.config.clone(); - run_async(ctx.refresh_tool_scope(utils::create_abort_signal())).unwrap(); - assert!( - !ctx.tool_scope.mcp_runtime.tool_filters.contains_key("gh"), - "the pre-assignment rebuild cannot see the session layer" - ); - ctx.session = Some(reloaded); - ctx.refresh_mcp_tool_filters(); + run_async(ctx.use_session(&app, Some("persisted"), utils::create_abort_signal())).unwrap(); let filter = ctx .tool_scope @@ -8229,6 +8260,45 @@ mod tests { assert!(!filter.allows("delete_repo")); } + #[test] + #[serial] + fn use_rag_does_not_drop_role_filters() { + // Attaching a RAG rebuilds the tool scope from scratch, and the + // rebuilt McpRuntime starts with no filters — so the rebuild must + // recompute the declarative filter layers or the active role's + // allowlist silently disappears. Uses the yaml driver so the load + // stays on the local filesystem; the externally-backed attach path + // needs a live vector store and cannot run here, but it funnels + // through the same tool-scope refresh. + let _guard = TestConfigDirGuard::new(); + let mut ctx = RequestContext::new(mcp_app_state(&["gh"]), WorkingMode::Cmd); + let mut role = Role::new("dev", "prompt"); + role.set_mcp_tools(Some(gh_get_only_map())); + ctx.role = Some(role); + ctx.refresh_mcp_tool_filters(); + assert!(ctx.tool_scope.mcp_runtime.tool_filters.contains_key("gh")); + + let rag_path = ctx.rag_file("kb"); + ensure_parent_exists(&rag_path).unwrap(); + write( + &rag_path, + "driver: yaml\nembedding_model: test-seeded:test-embedder\nchunk_size: 512\nchunk_overlap: 64\ntop_k: 5\n", + ) + .unwrap(); + + run_async(ctx.use_rag(Some("kb"), utils::create_abort_signal())).unwrap(); + + assert!(ctx.rag.is_some(), "the RAG must actually load"); + let filter = ctx + .tool_scope + .mcp_runtime + .tool_filters + .get("gh") + .expect("attaching a RAG must not drop the role's filter layer"); + assert!(filter.allows("get_issue")); + assert!(!filter.allows("delete_repo")); + } + #[test] #[serial] fn use_agent_applies_agent_filters_without_a_session() {