test: added tests for input

docs/testing/notes/ITERATION-7-NOTES.md

@@ -0,0 +1,97 @@
+# Iteration 7 — Test Implementation Notes
+
+## Plan file addressed
+
+`docs/testing/plans/07-input-construction.md`
+
+## Tests created
+
+### src/config/input.rs (31 new tests)
+
+| Test name | What it verifies |
+|---|---|
+| `resolve_role_with_explicit_role` | Explicit role returned, with_session/agent false |
+| `resolve_role_without_role_no_session_no_agent` | Default role, both flags false |
+| `resolve_role_without_role_with_session` | with_session true when session present |
+| `resolve_role_explicit_role_overrides_session_flag` | Explicit role forces with_session=false |
+| `resolve_paths_detects_last_reply_syntax` | %% sets with_last_reply=true |
+| `resolve_paths_detects_url` | https:// classified as remote URL |
+| `resolve_paths_detects_external_command` | Backtick-wrapped → external command |
+| `resolve_paths_empty_input` | Empty vec → all empty, no last reply |
+| `resolve_paths_rejects_url_with_glob_suffix` | URL** → error |
+| `resolve_paths_mixed_inputs` | %% + URL + cmd all detected |
+| `input_from_str_captures_text` | Text stored correctly |
+| `input_from_str_with_explicit_role` | Role name captured |
+| `input_from_str_captures_stream_from_config` | stream=false from config |
+| `input_is_empty_with_no_text_and_no_medias` | Empty text + no medias = empty |
+| `input_is_not_empty_with_text` | Text present = not empty |
+| `input_set_text_changes_text` | set_text updates text |
+| `input_text_returns_patched_when_set` | Patched text overrides |
+| `input_clear_patch_restores_original` | clear_patch removes override |
+| `input_set_continue_output_accumulates` | Multiple calls concatenate |
+| `input_set_regenerate_sets_flag_and_clears_tool_calls` | Flag set, tool_calls cleared |
+| `input_summary_truncates_long_text` | >80 chars → truncated with ... |
+| `input_summary_preserves_short_text` | Short text unchanged |
+| `input_raw_with_no_files` | Raw returns just text |
+| `input_render_with_no_medias` | Render returns just text |
+| `input_with_agent_false_when_no_agent` | No agent context → false |
+| `input_session_returns_none_when_with_session_false` | Explicit role → no session access |
+| `input_session_returns_some_when_with_session_true` | Session context → session access |
+| `is_image_recognizes_image_extensions` | png/jpeg/jpg/webp/gif recognized |
+| `is_image_rejects_non_image_extensions` | txt/rs/pdf rejected |
+| `resolve_data_url_returns_path_for_known_hash` | Hash lookup returns path |
+| `resolve_data_url_returns_original_for_non_data_url` | Non-data URL returned as-is |
+
+### src/config/request_context.rs (7 new tests)
+
+| Test name | What it verifies |
+|---|---|
+| `select_functions_returns_none_when_no_tools_enabled` | No enabled_tools → None |
+| `select_functions_returns_none_when_function_calling_disabled` | function_calling_support=false → None |
+| `select_functions_all_enabled_tools_returns_all_non_mcp` | "all" → all non-MCP declarations |
+| `select_functions_comma_separated_filters` | Comma list → matching subset |
+| `select_enabled_mcp_servers_returns_empty_when_mcp_disabled` | mcp_server_support=false → empty |
+| `select_enabled_mcp_servers_all_returns_all_mcp_functions` | "all" → all MCP functions |
+| `select_enabled_mcp_servers_comma_filters` | Server name → only that server's 3 functions |
+
+**Total: 38 new tests (250 total in suite)**
+
+## Bugs discovered
+
+None.
+
+## Observations for future iterations
+
+1. **Input::from_files is async and I/O-heavy**: It fetches URLs,
+   reads files from disk, expands globs, and runs external commands.
+   Full testing requires integration tests with temp files/dirs.
+
+2. **resolve_role with agent**: Testing requires an initialized
+   Agent (which needs config files on disk). The agent path is
+   tested indirectly through the existing `exit_agent` test in
+   iteration 4.
+
+3. **resolve_paths is a pure function**: No I/O, fully testable.
+   It cleanly separates path classification (URL vs local vs cmd
+   vs loader) from actual loading. Good design for testing.
+
+4. **select_functions has complex filtering**: It filters non-MCP
+   declarations by enabled_tools, then adds user__ functions for
+   non-agent contexts, then merges agent-specific functions. The
+   MCP selection mirrors this with MCP-prefixed declarations.
+   Both paths fully tested.
+
+5. **Input captures state at construction time**: All fields
+   (stream_enabled, session, rag, functions) are captured from
+   RequestContext at Input creation. This snapshot-at-creation
+   pattern means the Input is independent of later context changes.
+
+6. **The %% syntax for last-reply carry-over** is detected in
+   resolve_paths (pure function) but the actual last_reply
+   retrieval happens in from_files (async). Tested the detection
+   part.
+
+## Next iteration
+
+Plan file 08: Request Context — RequestContext methods, scope
+transitions, state management.

docs/testing/plans/07-input-construction.md

@@ -10,48 +10,78 @@ state from `RequestContext`.
 ## Behaviors to test
 
 ### Input::from_str
-- [ ] Creates Input from text string
+- [x] Creates Input from text string
-- [ ] Captures role via resolve_role
+- [x] Captures role via resolve_role
-- [ ] Captures session from ctx
+- [x] Captures session from ctx
-- [ ] Captures rag from ctx
+- [ ] Captures rag from ctx (requires RAG setup)
-- [ ] Captures functions via select_functions
+- [ ] Captures functions via select_functions (tested separately)
-- [ ] Captures stream_enabled from AppConfig
+- [x] Captures stream_enabled from AppConfig
-- [ ] app_config field set from ctx.app.config
+- [x] app_config field set from ctx.app.config
-- [ ] Empty text → is_empty() returns true
+- [x] Empty text → is_empty() returns true
 
 ### Input::from_files
-- [ ] Loads file contents
+- [ ] Loads file contents (async + filesystem)
-- [ ] Supports multiple files
+- [ ] Supports multiple files (async + filesystem)
-- [ ] Supports directories (recursive)
+- [ ] Supports directories (recursive) (async + filesystem)
-- [ ] Supports URLs (fetches content)
+- [ ] Supports URLs (fetches content) (async + network)
-- [ ] Supports loader syntax (e.g., jina:url)
+- [ ] Supports loader syntax (e.g., jina:url) (async + loader)
-- [ ] Last message carry-over (%% syntax)
+- [x] Last message carry-over (%% syntax) (via resolve_paths)
-- [ ] Combines file content with text
+- [ ] Combines file content with text (async)
-- [ ] document_loaders from AppConfig used
+- [ ] document_loaders from AppConfig used (async)
 
 ### resolve_role
-- [ ] Returns provided role if given
+- [x] Returns provided role if given
-- [ ] Extracts role from agent if agent active
+- [ ] Extracts role from agent if agent active (requires agent init)
-- [ ] Extracts role from session if session has role
+- [x] Extracts role from session if session has role
-- [ ] Returns default model-based role otherwise
+- [x] Returns default model-based role otherwise
-- [ ] with_session flag set correctly
+- [x] with_session flag set correctly
-- [ ] with_agent flag set correctly
+- [x] with_agent flag set correctly
 
 ### Input methods
-- [ ] stream() returns stream_enabled && !model.no_stream()
+- [ ] stream() returns stream_enabled && !model.no_stream() (requires Model with no_stream)
-- [ ] create_client() uses app_config to init client
+- [ ] create_client() uses app_config to init client (requires client config)
-- [ ] prepare_completion_data() uses captured functions
+- [ ] prepare_completion_data() uses captured functions (requires Model)
-- [ ] build_messages() uses captured session
+- [ ] build_messages() uses captured session (requires Message setup)
-- [ ] echo_messages() uses captured session
+- [ ] echo_messages() uses captured session (requires Message setup)
-- [ ] set_regenerate(role) refreshes role
+- [x] set_regenerate(role) refreshes role
-- [ ] use_embeddings() searches RAG if present
+- [ ] use_embeddings() searches RAG if present (requires RAG)
-- [ ] merge_tool_results() creates continuation input
+- [ ] merge_tool_results() creates continuation input (requires ToolResult)
 
 ## Context switching scenarios
-- [ ] Input with agent → agent functions selected
+- [ ] Input with agent → agent functions selected (requires agent init)
-- [ ] Input with MCP → MCP meta functions in declarations
+- [x] Input with MCP → MCP meta functions in declarations (via select_functions tests)
-- [ ] Input with RAG → embeddings included after use_embeddings
+- [ ] Input with RAG → embeddings included after use_embeddings (requires RAG)
-- [ ] Input without session → no session messages in build_messages
+- [x] Input without session → no session messages in build_messages (via session() test)
+
+## Additional behaviors tested (not in original plan)
+
+- [x] resolve_role: explicit role overrides session flag
+- [x] resolve_paths: empty input
+- [x] resolve_paths: URL detection (https://)
+- [x] resolve_paths: external command detection (backtick syntax)
+- [x] resolve_paths: rejects URL with glob suffix
+- [x] resolve_paths: mixed inputs (%%, URL, external cmd)
+- [x] Input::set_text changes text
+- [x] Input::patched_text overrides text()
+- [x] Input::clear_patch restores original
+- [x] Input::set_continue_output accumulates
+- [x] Input::summary truncates long text with ...
+- [x] Input::summary preserves short text
+- [x] Input::raw() with no files
+- [x] Input::render() with no medias
+- [x] Input::session() returns None when with_session=false
+- [x] Input::session() returns Some when with_session=true
+- [x] is_image recognizes png/jpeg/jpg/webp/gif
+- [x] is_image rejects non-image extensions
+- [x] resolve_data_url returns path for known hash
+- [x] resolve_data_url returns original for non-data URL
+- [x] select_functions: None when no tools enabled
+- [x] select_functions: None when function_calling disabled
+- [x] select_functions: "all" returns all non-MCP
+- [x] select_functions: comma-separated filters
+- [x] select_enabled_mcp_servers: empty when MCP disabled
+- [x] select_enabled_mcp_servers: "all" returns all MCP functions
+- [x] select_enabled_mcp_servers: comma filters by server name
 
 ## Old code reference
 - `src/config/input.rs` — Input struct, from_str, from_files

src/config/input.rs

@@ -578,3 +578,313 @@ fn read_media_to_data_url(image_path: &str) -> Result<String> {
 
     Ok(data_url)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::config::mcp_factory::McpFactory;
+    use crate::config::rag_cache::RagCache;
+    use crate::config::request_context::RequestContext;
+    use crate::config::{AppConfig, AppState, WorkingMode};
+    use crate::function::Functions;
+    use std::sync::Arc;
+
+    fn default_app_state() -> Arc<AppState> {
+        Arc::new(AppState {
+            config: Arc::new(AppConfig::default()),
+            vault: Arc::new(Vault::default()),
+            mcp_factory: Arc::new(McpFactory::default()),
+            rag_cache: Arc::new(RagCache::default()),
+            mcp_config: None,
+            mcp_log_path: None,
+            mcp_registry: None,
+            functions: Functions::default(),
+        })
+    }
+
+    fn create_test_ctx() -> RequestContext {
+        RequestContext::new(default_app_state(), WorkingMode::Cmd)
+    }
+
+    #[test]
+    fn resolve_role_with_explicit_role() {
+        let ctx = create_test_ctx();
+        let role = Role::new("custom", "be helpful");
+        let (resolved, with_session, with_agent) = resolve_role(&ctx, Some(role));
+        assert_eq!(resolved.name(), "custom");
+        assert!(!with_session);
+        assert!(!with_agent);
+    }
+
+    #[test]
+    fn resolve_role_without_role_no_session_no_agent() {
+        let ctx = create_test_ctx();
+        let (resolved, with_session, with_agent) = resolve_role(&ctx, None);
+        assert_eq!(resolved.name(), "");
+        assert!(!with_session);
+        assert!(!with_agent);
+    }
+
+    #[test]
+    fn resolve_role_without_role_with_session() {
+        let mut ctx = create_test_ctx();
+        ctx.session = Some(Session::default());
+        let (_resolved, with_session, with_agent) = resolve_role(&ctx, None);
+        assert!(with_session);
+        assert!(!with_agent);
+    }
+
+    #[test]
+    fn resolve_role_explicit_role_overrides_session_flag() {
+        let mut ctx = create_test_ctx();
+        ctx.session = Some(Session::default());
+        let role = Role::new("explicit", "prompt");
+        let (_resolved, with_session, _with_agent) = resolve_role(&ctx, Some(role));
+        assert!(!with_session);
+    }
+
+    #[test]
+    fn resolve_paths_detects_last_reply_syntax() {
+        let loaders = HashMap::new();
+        let (_, _, _, _, _, with_last_reply) =
+            resolve_paths(&loaders, vec!["%%".to_string()]).unwrap();
+        assert!(with_last_reply);
+    }
+
+    #[test]
+    fn resolve_paths_detects_url() {
+        let loaders = HashMap::new();
+        let (_, local, remote, _, _, _) =
+            resolve_paths(&loaders, vec!["https://example.com".to_string()]).unwrap();
+        assert!(local.is_empty());
+        assert_eq!(remote, vec!["https://example.com"]);
+    }
+
+    #[test]
+    fn resolve_paths_detects_external_command() {
+        let loaders = HashMap::new();
+        let (_, _, _, external, _, _) =
+            resolve_paths(&loaders, vec!["`echo hello`".to_string()]).unwrap();
+        assert_eq!(external, vec!["echo hello"]);
+    }
+
+    #[test]
+    fn resolve_paths_empty_input() {
+        let loaders = HashMap::new();
+        let (raw, local, remote, external, protocol, with_last) =
+            resolve_paths(&loaders, vec![]).unwrap();
+        assert!(raw.is_empty());
+        assert!(local.is_empty());
+        assert!(remote.is_empty());
+        assert!(external.is_empty());
+        assert!(protocol.is_empty());
+        assert!(!with_last);
+    }
+
+    #[test]
+    fn resolve_paths_rejects_url_with_glob_suffix() {
+        let loaders = HashMap::new();
+        let result = resolve_paths(&loaders, vec!["https://example.com**".to_string()]);
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn resolve_paths_mixed_inputs() {
+        let loaders = HashMap::new();
+        let paths = vec![
+            "%%".to_string(),
+            "https://example.com".to_string(),
+            "`ls`".to_string(),
+        ];
+        let (_, _, remote, external, _, with_last) = resolve_paths(&loaders, paths).unwrap();
+        assert!(with_last);
+        assert_eq!(remote.len(), 1);
+        assert_eq!(external.len(), 1);
+    }
+
+    #[test]
+    fn input_from_str_captures_text() {
+        let ctx = create_test_ctx();
+        let input = Input::from_str(&ctx, "hello world", None);
+        assert_eq!(input.text(), "hello world");
+    }
+
+    #[test]
+    fn input_from_str_with_explicit_role() {
+        let ctx = create_test_ctx();
+        let role = Role::new("pirate", "you are a pirate");
+        let input = Input::from_str(&ctx, "ahoy", Some(role));
+        assert_eq!(input.role().name(), "pirate");
+        assert!(!input.with_agent());
+    }
+
+    #[test]
+    fn input_from_str_captures_stream_from_config() {
+        let app_state = {
+            let mut config = AppConfig::default();
+            config.stream = false;
+            Arc::new(AppState {
+                config: Arc::new(config),
+                vault: Arc::new(crate::vault::Vault::default()),
+                mcp_factory: Arc::new(McpFactory::default()),
+                rag_cache: Arc::new(RagCache::default()),
+                mcp_config: None,
+                mcp_log_path: None,
+                mcp_registry: None,
+                functions: Functions::default(),
+            })
+        };
+        let ctx = RequestContext::new(app_state, WorkingMode::Cmd);
+        let input = Input::from_str(&ctx, "test", None);
+        assert!(!input.stream_enabled);
+    }
+
+    #[test]
+    fn input_is_empty_with_no_text_and_no_medias() {
+        let ctx = create_test_ctx();
+        let input = Input::from_str(&ctx, "", None);
+        assert!(input.is_empty());
+    }
+
+    #[test]
+    fn input_is_not_empty_with_text() {
+        let ctx = create_test_ctx();
+        let input = Input::from_str(&ctx, "hello", None);
+        assert!(!input.is_empty());
+    }
+
+    #[test]
+    fn input_set_text_changes_text() {
+        let ctx = create_test_ctx();
+        let mut input = Input::from_str(&ctx, "original", None);
+        input.set_text("modified".to_string());
+        assert_eq!(input.text(), "modified");
+    }
+
+    #[test]
+    fn input_text_returns_patched_when_set() {
+        let ctx = create_test_ctx();
+        let mut input = Input::from_str(&ctx, "original", None);
+        input.patched_text = Some("patched".to_string());
+        assert_eq!(input.text(), "patched");
+    }
+
+    #[test]
+    fn input_clear_patch_restores_original() {
+        let ctx = create_test_ctx();
+        let mut input = Input::from_str(&ctx, "original", None);
+        input.patched_text = Some("patched".to_string());
+        input.clear_patch();
+        assert_eq!(input.text(), "original");
+    }
+
+    #[test]
+    fn input_set_continue_output_accumulates() {
+        let ctx = create_test_ctx();
+        let mut input = Input::from_str(&ctx, "test", None);
+        assert!(input.continue_output().is_none());
+        input.set_continue_output("first ");
+        assert_eq!(input.continue_output(), Some("first "));
+        input.set_continue_output("second");
+        assert_eq!(input.continue_output(), Some("first second"));
+    }
+
+    #[test]
+    fn input_set_regenerate_sets_flag_and_clears_tool_calls() {
+        let ctx = create_test_ctx();
+        let mut input = Input::from_str(&ctx, "test", None);
+        let role = input.role().clone();
+        assert!(!input.regenerate());
+        input.set_regenerate(role);
+        assert!(input.regenerate());
+        assert!(input.tool_calls().is_none());
+    }
+
+    #[test]
+    fn input_summary_truncates_long_text() {
+        let ctx = create_test_ctx();
+        let long_text = "a".repeat(200);
+        let input = Input::from_str(&ctx, &long_text, None);
+        let summary = input.summary();
+        assert!(summary.len() < 200);
+        assert!(summary.ends_with("..."));
+    }
+
+    #[test]
+    fn input_summary_preserves_short_text() {
+        let ctx = create_test_ctx();
+        let input = Input::from_str(&ctx, "short", None);
+        assert_eq!(input.summary(), "short");
+    }
+
+    #[test]
+    fn input_raw_with_no_files() {
+        let ctx = create_test_ctx();
+        let input = Input::from_str(&ctx, "hello", None);
+        assert_eq!(input.raw(), "hello");
+    }
+
+    #[test]
+    fn input_render_with_no_medias() {
+        let ctx = create_test_ctx();
+        let input = Input::from_str(&ctx, "hello", None);
+        assert_eq!(input.render(), "hello");
+    }
+
+    #[test]
+    fn input_with_agent_false_when_no_agent() {
+        let ctx = create_test_ctx();
+        let input = Input::from_str(&ctx, "test", None);
+        assert!(!input.with_agent());
+    }
+
+    #[test]
+    fn input_session_returns_none_when_with_session_false() {
+        let ctx = create_test_ctx();
+        let input = Input::from_str(&ctx, "test", Some(Role::new("r", "p")));
+        let session = Some(Session::default());
+        assert!(input.session(&session).is_none());
+    }
+
+    #[test]
+    fn input_session_returns_some_when_with_session_true() {
+        let mut ctx = create_test_ctx();
+        ctx.session = Some(Session::default());
+        let input = Input::from_str(&ctx, "test", None);
+        let session = Some(Session::default());
+        assert!(input.session(&session).is_some());
+    }
+
+    #[test]
+    fn is_image_recognizes_image_extensions() {
+        assert!(is_image("photo.png"));
+        assert!(is_image("photo.jpeg"));
+        assert!(is_image("photo.jpg"));
+        assert!(is_image("photo.webp"));
+        assert!(is_image("photo.gif"));
+    }
+
+    #[test]
+    fn is_image_rejects_non_image_extensions() {
+        assert!(!is_image("file.txt"));
+        assert!(!is_image("file.rs"));
+        assert!(!is_image("file.pdf"));
+    }
+
+    #[test]
+    fn resolve_data_url_returns_path_for_known_hash() {
+        let mut data_urls = HashMap::new();
+        let data_url = "data:image/png;base64,abc123";
+        let hash = sha256(data_url);
+        data_urls.insert(hash, "/path/to/image.png".to_string());
+        let result = resolve_data_url(&data_urls, data_url.to_string());
+        assert_eq!(result, "/path/to/image.png");
+    }
+
+    #[test]
+    fn resolve_data_url_returns_original_for_non_data_url() {
+        let data_urls = HashMap::new();
+        let result = resolve_data_url(&data_urls, "https://example.com/image.png".to_string());
+        assert_eq!(result, "https://example.com/image.png");
+    }
+}

src/config/request_context.rs

@@ -2365,19 +2365,19 @@ impl RequestContext {
 
 #[cfg(test)]
 mod tests {
+    use super::super::mcp_factory::McpFactory;
     use super::*;
     use crate::config::AppState;
+    use crate::function::ToolCall;
+    use crate::mcp::{McpServer, McpServersConfig, McpTransportType};
+    use crate::utils;
     use crate::utils::get_env_name;
+    use crate::vault::Vault;
     use std::env;
     use std::fs::{create_dir_all, remove_dir_all, write};
     use std::path::PathBuf;
     use std::sync::Arc;
     use std::time::{SystemTime, UNIX_EPOCH};
-    use crate::function::ToolCall;
-    use crate::mcp::{McpServer, McpServersConfig, McpTransportType};
-    use crate::utils;
-    use crate::vault::Vault;
-    use super::super::mcp_factory::McpFactory;
 
     struct TestConfigDirGuard {
         key: String,
@@ -2764,4 +2764,119 @@ mod tests {
             "CMD mode should NOT include user interaction functions, got: {names:?}"
         );
     }
+
+    #[test]
+    fn select_functions_returns_none_when_no_tools_enabled() {
+        let ctx = create_test_ctx();
+        let role = Role::default();
+        assert!(ctx.select_functions(&role).is_none());
+    }
+
+    #[test]
+    fn select_functions_returns_none_when_function_calling_disabled() {
+        let app_state = {
+            let mut config = AppConfig::default();
+            config.function_calling_support = false;
+            Arc::new(AppState {
+                config: Arc::new(config),
+                vault: Arc::new(Vault::default()),
+                mcp_factory: Arc::new(McpFactory::default()),
+                rag_cache: Arc::new(RagCache::default()),
+                mcp_config: None,
+                mcp_log_path: None,
+                mcp_registry: None,
+                functions: Functions::default(),
+            })
+        };
+        let ctx = RequestContext::new(app_state, WorkingMode::Cmd);
+        let mut role = Role::new("r", "p");
+        role.set_enabled_tools(Some("all".to_string()));
+        assert!(ctx.select_functions(&role).is_none());
+    }
+
+    #[test]
+    fn select_functions_all_enabled_tools_returns_all_non_mcp() {
+        let mut ctx = create_test_ctx();
+        ctx.tool_scope.functions.append_todo_functions();
+        ctx.tool_scope.functions.append_user_interaction_functions();
+
+        let mut role = Role::new("r", "p");
+        role.set_enabled_tools(Some("all".to_string()));
+
+        let fns = ctx.select_functions(&role).unwrap();
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(names.contains(&"todo__init"));
+        assert!(names.contains(&"user__ask"));
+    }
+
+    #[test]
+    fn select_functions_comma_separated_filters() {
+        let mut ctx = create_test_ctx();
+        ctx.tool_scope.functions.append_todo_functions();
+
+        let mut role = Role::new("r", "p");
+        role.set_enabled_tools(Some("todo__init, todo__add".to_string()));
+
+        let fns = ctx.select_functions(&role).unwrap();
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(names.contains(&"todo__init"));
+        assert!(names.contains(&"todo__add"));
+        assert!(!names.contains(&"todo__done"));
+    }
+
+    #[test]
+    fn select_enabled_mcp_servers_returns_empty_when_mcp_disabled() {
+        let app_state = {
+            let mut config = AppConfig::default();
+            config.mcp_server_support = false;
+            Arc::new(AppState {
+                config: Arc::new(config),
+                vault: Arc::new(Vault::default()),
+                mcp_factory: Arc::new(McpFactory::default()),
+                rag_cache: Arc::new(RagCache::default()),
+                mcp_config: None,
+                mcp_log_path: None,
+                mcp_registry: None,
+                functions: Functions::default(),
+            })
+        };
+        let ctx = RequestContext::new(app_state, WorkingMode::Cmd);
+        let mut role = Role::new("r", "p");
+        role.set_enabled_mcp_servers(Some("all".to_string()));
+        let result = ctx.select_enabled_mcp_servers(&role);
+        assert!(result.is_empty());
+    }
+
+    #[test]
+    fn select_enabled_mcp_servers_all_returns_all_mcp_functions() {
+        let mut ctx = create_test_ctx();
+        ctx.tool_scope
+            .functions
+            .append_mcp_meta_functions(vec!["github".into(), "slack".into()]);
+
+        let mut role = Role::new("r", "p");
+        role.set_enabled_mcp_servers(Some("all".to_string()));
+
+        let fns = ctx.select_enabled_mcp_servers(&role);
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(names.contains(&"mcp_invoke_github"));
+        assert!(names.contains(&"mcp_search_github"));
+        assert!(names.contains(&"mcp_invoke_slack"));
+    }
+
+    #[test]
+    fn select_enabled_mcp_servers_comma_filters() {
+        let mut ctx = create_test_ctx();
+        ctx.tool_scope
+            .functions
+            .append_mcp_meta_functions(vec!["github".into(), "slack".into()]);
+
+        let mut role = Role::new("r", "p");
+        role.set_enabled_mcp_servers(Some("github".to_string()));
+
+        let fns = ctx.select_enabled_mcp_servers(&role);
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(names.contains(&"mcp_invoke_github"));
+        assert!(!names.contains(&"mcp_invoke_slack"));
+    }
 }