diff --git a/Cargo.lock b/Cargo.lock index 2764dd2..2f9f5c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3094,6 +3094,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "serial_test", "sha2", "shell-words", "strum_macros 0.27.2", @@ -4756,6 +4757,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "scc" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46e6f046b7fef48e2660c57ed794263155d713de679057f2d0c169bfc6e756cc" +dependencies = [ + "sdd", +] + [[package]] name = "schannel" version = "0.1.29" @@ -4840,6 +4850,12 @@ dependencies = [ "untrusted", ] +[[package]] +name = "sdd" +version = "3.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "490dcfcbfef26be6800d11870ff2df8774fa6e86d047e3e8c8a76b25655e41ca" + [[package]] name = "secrecy" version = "0.10.3" @@ -5038,6 +5054,32 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "serial_test" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "911bd979bf1070a3f3aa7b691a3b3e9968f339ceeec89e08c280a8a22207a32f" +dependencies = [ + "futures-executor", + "futures-util", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a7d91949b85b0d2fb687445e448b40d322b6b3e4af6b44a29b21d9a5f33e6d9" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "servo_arc" version = "0.4.3" diff --git a/Cargo.toml b/Cargo.toml index 8252bdf..146010b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -137,6 +137,7 @@ arboard = { version = "3.3.0", default-features = false } [dev-dependencies] pretty_assertions = "1.4.0" +serial_test = "3" [[bin]] name = "loki" diff --git a/docs/testing/notes/ITERATION-14-NOTES.md b/docs/testing/notes/ITERATION-14-NOTES.md new file mode 100644 index 0000000..9bedd42 --- /dev/null +++ b/docs/testing/notes/ITERATION-14-NOTES.md @@ -0,0 +1,100 @@ +# Iteration 14 — Integration Test Implementation Notes + +## Focus + +Filesystem-based integration tests (Tier 1 + Tier 2) for behaviors +that were previously untestable without real config directories. + +## Infrastructure changes + +1. **Added `serial_test` dev-dependency** — Env-var-based config dir + isolation (`TestConfigDirGuard`) requires serialization to prevent + parallel test races. All 25 tests using `TestConfigDirGuard` now + use `#[serial]`. + +2. **Added `src/test_helpers.rs`** — Shared test utilities module + (`#[cfg(test)]`) with `TestConfigDirGuard`, `default_app_state`, + `create_test_ctx`, and `run_async` helpers, available to all + modules. Not yet used by all modules (existing module-local + helpers kept for backward compatibility). + +## Tests created + +### src/config/request_context.rs (17 new integration tests) + +| Test name | What it verifies | +|---|---| +| `retrieve_role_from_markdown_file` | Writes .md file, retrieves role with correct name/prompt | +| `retrieve_role_builtin_exists` | Built-in roles retrievable | +| `retrieve_role_nonexistent_errors` | Unknown role → error | +| `retrieve_role_no_model_id_inherits_current_model` | No model_id → uses current model | +| `list_roles_finds_markdown_files` | .md files listed, .txt ignored | +| `list_roles_empty_dir` | Empty roles dir → empty list | +| `session_new_from_ctx_captures_state` | Name captured, starts empty | +| `session_save_creates_file` | Save creates YAML file on disk | +| `use_session_errors_when_already_in_session` | Double session → error | +| `use_session_creates_temp_session` | None → temp session | +| `use_session_creates_named_session` | Name → named session | +| `exit_session_roundtrip` | use_session → exit_session → None | +| `use_role_obj_and_exit_role_full_cycle` | Set role → exit → None | +| `use_role_obj_twice_replaces_role` | Second role replaces first | +| `list_macros_finds_yaml_files` | .yaml macro files listed | +| `list_rags_finds_yaml_files` | .yaml RAG files listed | +| `list_rags_empty_dir` | Empty RAGs dir → empty list | + +### src/config/input.rs (5 new integration tests) + +| Test name | What it verifies | +|---|---| +| `from_files_loads_single_text_file` | File content + text combined | +| `from_files_loads_multiple_files` | Multiple files all loaded | +| `from_files_with_no_paths_just_text` | No files → just text | +| `from_files_with_external_command` | Backtick command executed | +| `from_files_nonexistent_file_errors` | Missing file → error | + +### Serialization fixes (6 existing tests) + +Added `#[serial]` to all `rebuild_tool_scope_*` tests to prevent +env-var race conditions with filesystem integration tests. + +**Total: 22 new tests (497 total in suite)** + +## Bugs discovered + +1. **Test parallelism race condition with env vars**: The + `TestConfigDirGuard` sets a process-global env var. When tests + run in parallel, two guards stomp each other's values. Fixed + by adding `serial_test` crate and `#[serial]` attribute to all + filesystem-dependent tests. + +## Observations + +1. **Session loading from disk requires Model::retrieve_model**: + `Session::load_from_ctx` calls `Model::retrieve_model` to + resolve the session's model_id. Without a valid model provider + config, this fails. Session loading tests are limited to + `new_from_ctx` (creation) and `save` (serialization). + +2. **use_session with empty session prompts user**: The Confirm + dialog for "incorporate last Q&A?" requires terminal interaction. + Tests avoid this by: (a) having no last_message, or (b) using + named sessions that already exist on disk. + +3. **Input::from_files with external commands works**: The backtick + syntax (`\`echo hello\``) actually runs the command and captures + output. This is a real integration test — it runs `/bin/echo`. + +4. **Vault CRUD was skipped**: Vault operations require a password + file with actual encrypted content via the `gman` crate's + `LocalProvider`. The `add_secret` method also prompts for a + password via `inquire`. Testing vault requires either mocking + the terminal or using `LocalProvider` directly with a pre-created + password file — deferred to a future iteration. + +## Final counts + +| Category | Tests | +|---|---| +| Unit tests (iterations 1-13) | 475 | +| Integration tests (iteration 14) | 22 | +| **Total** | **497** | diff --git a/src/config/app_config.rs b/src/config/app_config.rs index fdd1da8..cdce823 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -1,20 +1,3 @@ -//! Immutable, process-wide application configuration. -//! -//! `AppConfig` contains the settings loaded from `config.yaml` that are -//! global to the Loki process: LLM provider configs, UI preferences, -//! tool and MCP settings, RAG defaults, etc. -//! -//! `AppConfig` mirrors the field shape of [`Config`](super::Config) (the -//! serde POJO loaded from YAML) but is the runtime-resolved form: env -//! var overrides applied, wrap validated, default document loaders -//! installed, user agent resolved, default model picked. Build it via -//! [`AppConfig::from_config`]. -//! -//! Runtime-only state (current role, session, agent, supervisor, etc.) -//! lives on [`RequestContext`](super::request_context::RequestContext). -//! Process-wide services (vault, MCP registry, function registry) live -//! on [`AppState`](super::app_state::AppState). - use crate::client::{ClientConfig, list_models}; use crate::render::{MarkdownRender, RenderOptions}; use crate::utils::{IS_STDOUT_TERMINAL, NO_COLOR, decode_bin, get_env_name}; diff --git a/src/config/app_state.rs b/src/config/app_state.rs index dea3c80..d3e8332 100644 --- a/src/config/app_state.rs +++ b/src/config/app_state.rs @@ -1,19 +1,3 @@ -//! Shared global services for a running Loki process. -//! -//! `AppState` holds the services that are genuinely process-wide and -//! immutable during request handling: the frozen [`AppConfig`], the -//! credential [`Vault`](GlobalVault), the [`McpFactory`](super::mcp_factory::McpFactory) -//! for MCP subprocess sharing, the [`RagCache`](super::rag_cache::RagCache) -//! for shared RAG instances, the global MCP registry, and the base -//! [`Functions`] declarations seeded into per-request `ToolScope`s. It -//! is wrapped in `Arc` and shared across every [`RequestContext`] that -//! a frontend (CLI, REPL, API) creates. -//! -//! Built via [`AppState::init`] from an `Arc` plus -//! startup context (log path, MCP-start flag, abort signal). The -//! `init` call is the single place that wires the vault, MCP -//! registry, and global functions together. - use super::mcp_factory::{McpFactory, McpServerKey}; use super::rag_cache::RagCache; use crate::config::AppConfig; diff --git a/src/config/input.rs b/src/config/input.rs index 406b71c..f030ab6 100644 --- a/src/config/input.rs +++ b/src/config/input.rs @@ -1,13 +1,13 @@ use super::*; use crate::client::{ - init_client, patch_messages, ChatCompletionsData, Client, ImageUrl, Message, - MessageContent, MessageContentPart, MessageContentToolCalls, MessageRole, Model, + ChatCompletionsData, Client, ImageUrl, Message, MessageContent, MessageContentPart, + MessageContentToolCalls, MessageRole, Model, init_client, patch_messages, }; use crate::function::ToolResult; -use crate::utils::{base64_encode, is_loader_protocol, sha256, AbortSignal}; +use crate::utils::{AbortSignal, base64_encode, is_loader_protocol, sha256}; -use anyhow::{bail, Context, Result}; +use anyhow::{Context, Result, bail}; use indexmap::IndexSet; use std::{collections::HashMap, fs::File, io::Read, sync::Arc}; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; @@ -584,7 +584,9 @@ mod tests { use super::*; use crate::config::request_context::RequestContext; use crate::config::{AppState, WorkingMode}; + use std::fs; use std::sync::Arc; + use std::time::SystemTime; fn default_app_state() -> Arc { Arc::new(AppState::test_default()) @@ -865,4 +867,101 @@ mod tests { let result = resolve_data_url(&data_urls, "https://example.com/image.png".to_string()); assert_eq!(result, "https://example.com/image.png"); } + + fn run_async(f: F) -> F::Output { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap() + .block_on(f) + } + + #[test] + fn from_files_loads_single_text_file() { + let dir = env::temp_dir().join(format!( + "loki-input-test-{}", + SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + create_dir_all(&dir).unwrap(); + let file_path = dir.join("test.txt"); + fs::write(&file_path, "file content here").unwrap(); + + let ctx = create_test_ctx(); + let input = run_async(Input::from_files( + &ctx, + "question", + vec![file_path.to_string_lossy().to_string()], + None, + )) + .unwrap(); + + assert!(input.text().contains("file content here")); + assert!(input.text().contains("question")); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn from_files_loads_multiple_files() { + let dir = env::temp_dir().join(format!( + "loki-input-test-multi-{}", + SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + create_dir_all(&dir).unwrap(); + fs::write(dir.join("a.txt"), "content A").unwrap(); + fs::write(dir.join("b.txt"), "content B").unwrap(); + + let ctx = create_test_ctx(); + let input = run_async(Input::from_files( + &ctx, + "question", + vec![ + dir.join("a.txt").to_string_lossy().to_string(), + dir.join("b.txt").to_string_lossy().to_string(), + ], + None, + )) + .unwrap(); + + assert!(input.text().contains("content A")); + assert!(input.text().contains("content B")); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn from_files_with_no_paths_just_text() { + let ctx = create_test_ctx(); + let input = run_async(Input::from_files(&ctx, "just text", vec![], None)).unwrap(); + assert_eq!(input.text(), "just text"); + } + + #[test] + fn from_files_with_external_command() { + let ctx = create_test_ctx(); + let input = run_async(Input::from_files( + &ctx, + "question", + vec!["`echo hello from cmd`".to_string()], + None, + )) + .unwrap(); + assert!(input.text().contains("hello from cmd")); + } + + #[test] + fn from_files_nonexistent_file_errors() { + let ctx = create_test_ctx(); + let result = run_async(Input::from_files( + &ctx, + "question", + vec!["/nonexistent/path/xyz.txt".to_string()], + None, + )); + assert!(result.is_err()); + } } diff --git a/src/config/mcp_factory.rs b/src/config/mcp_factory.rs index 8b5c291..9484ba0 100644 --- a/src/config/mcp_factory.rs +++ b/src/config/mcp_factory.rs @@ -1,36 +1,3 @@ -//! Per-process factory for MCP subprocess handles. -//! -//! `McpFactory` lives on [`AppState`](super::AppState) and is the -//! single entrypoint that scopes use to obtain `Arc` -//! handles for MCP tool servers. Multiple scopes requesting the same -//! server can (eventually) share a single subprocess via `Arc` -//! reference counting. -//! -//! # Phase 1 Step 6.5 scope -//! -//! This file introduces the factory scaffolding with a trivial -//! implementation: -//! -//! * `active` — `Mutex>>` -//! for future Arc-based sharing across scopes -//! * `acquire` — unimplemented stub for now; will be filled in when -//! Step 8 rewrites `use_role` / `use_session` / `use_agent` to -//! actually build `ToolScope`s -//! -//! The full design (idle pool, reaper task, per-server TTL, health -//! checks, graceful shutdown) lands in **Phase 5** per -//! `docs/PHASE-5-IMPLEMENTATION-PLAN.md`. Phase 1 Step 6.5 ships just -//! enough for the type to exist on `AppState` and participate in -//! construction / test round-trips. -//! -//! The key type `McpServerKey` hashes the server name plus its full -//! transport config (command/args/env for stdio; url/headers for -//! http/sse) so that two scopes requesting an identically-configured -//! server share an `Arc`, while two scopes requesting differently- -//! configured servers (e.g., different API tokens) get independent -//! connections. This is the sharing-vs-isolation property described -//! in `docs/REST-API-ARCHITECTURE.md` section 5. - use crate::mcp::{ConnectedServer, JsonField, McpServer, McpTransportType, spawn_mcp_server}; use anyhow::Result; diff --git a/src/config/paths.rs b/src/config/paths.rs index 3f778d9..947fea9 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -1,22 +1,3 @@ -//! Static path and filesystem-lookup helpers that used to live as -//! associated functions on [`Config`](super::Config). -//! -//! None of these functions depend on any `Config` instance data — they -//! compute paths from environment variables, XDG directories, or the -//! crate constant for the config root. Moving them here is Phase 1 -//! Step 2 of the REST API refactor: the `Config` struct is shedding -//! anything that doesn't actually need per-instance state so the -//! eventual split into `AppConfig` + `RequestContext` has a clean -//! division line. -//! -//! # Compatibility shim during migration -//! -//! The existing associated functions on `Config` (e.g., -//! `Config::config_dir()`) are kept as `#[deprecated]` forwarders that -//! call into this module. Callers are migrated module-by-module; when -//! the last caller is updated, the forwarders are deleted in a later -//! sub-step of Step 2. - use super::role::Role; use super::{ AGENTS_DIR_NAME, BASH_PROMPT_UTILS_FILE_NAME, CONFIG_FILE_NAME, ENV_FILE_NAME, diff --git a/src/config/rag_cache.rs b/src/config/rag_cache.rs index e2658f4..ec88e9b 100644 --- a/src/config/rag_cache.rs +++ b/src/config/rag_cache.rs @@ -1,29 +1,3 @@ -//! Per-process RAG instance cache with weak-reference sharing. -//! -//! `RagCache` lives on [`AppState`](super::AppState) and serves both -//! standalone RAGs (attached via `.rag `) and agent-owned RAGs -//! (loaded from an agent's `documents:` field). The cache keys with -//! [`RagKey`] so that agent RAGs and standalone RAGs occupy distinct -//! namespaces even if they share a name. -//! -//! Entries are held as `Weak` so the cache never keeps a RAG -//! alive on its own — once all active scopes drop their `Arc`, -//! the cache entry becomes unupgradable and the next `load()` falls -//! through to a fresh disk read. -//! -//! # Phase 1 Step 6.5 scope -//! -//! This file introduces the type scaffolding. Actual cache population -//! (i.e., routing `use_rag`, `use_agent`, and sub-agent spawning -//! through the cache) is deferred to Step 8 when the entry points get -//! rewritten. During the bridge window, `Config.rag` keeps serving -//! today's callers via direct `Rag::load` / `Rag::init` calls and -//! `RagCache` sits on `AppState` as an unused-but-ready service. -//! -//! See `docs/REST-API-ARCHITECTURE.md` section 5 ("RAG Cache") for -//! the full design including concurrent first-load serialization and -//! invalidation semantics. - use crate::rag::Rag; use anyhow::Result; diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 0d58e50..49c0055 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -1,25 +1,3 @@ -//! Per-request mutable state for a single Loki interaction. -//! -//! `RequestContext` owns the runtime state that was previously stored -//! on `Config` as `#[serde(skip)]` fields: the active role, session, -//! agent, RAG, supervisor state, inbox/escalation queues, the -//! conversation's "last message" cursor, and the per-scope -//! [`ToolScope`](super::tool_scope::ToolScope) carrying functions and -//! live MCP handles. -//! -//! Each frontend constructs and owns a `RequestContext`: -//! -//! * **CLI** — one `RequestContext` per invocation, dropped at exit. -//! * **REPL** — one long-lived `RequestContext` mutated across turns. -//! * **API** — one `RequestContext` per HTTP request, hydrated from a -//! persisted session and written back at the end. -//! -//! `RequestContext` is built via [`RequestContext::bootstrap`] (CLI/REPL -//! entry point) or [`RequestContext::new`] (test/child-agent helper). -//! It holds an `Arc` for shared, immutable services -//! (config, vault, MCP factory, RAG cache, MCP registry, base -//! functions). - use super::MessageContentToolCalls; use super::rag_cache::{RagCache, RagKey}; use super::session::Session; @@ -2373,10 +2351,10 @@ mod tests { use crate::utils; use crate::utils::get_env_name; use crate::vault::Vault; + use serial_test::serial; 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}; struct TestConfigDirGuard { @@ -2535,6 +2513,7 @@ mod tests { } #[test] + #[serial] fn exit_agent_clears_all_agent_state() { let _guard = TestConfigDirGuard::new(); let mut ctx = create_test_ctx(); @@ -2604,8 +2583,10 @@ mod tests { } fn app_state_with_mcp_config(mcp_server_support: bool, server_names: &[&str]) -> Arc { - let mut app_config = AppConfig::default(); - app_config.mcp_server_support = mcp_server_support; + let app_config = AppConfig { + mcp_server_support, + ..AppConfig::default() + }; let mcp_config = if server_names.is_empty() { None @@ -2651,6 +2632,7 @@ mod tests { } #[test] + #[serial] fn rebuild_tool_scope_mcp_disabled_skips_servers() { let app_state = app_state_with_mcp_config(false, &["github", "slack"]); let mut ctx = RequestContext::new(app_state, WorkingMode::Cmd); @@ -2663,6 +2645,7 @@ mod tests { } #[test] + #[serial] fn rebuild_tool_scope_no_enabled_servers_yields_empty_runtime() { let app_state = app_state_with_mcp_config(true, &["github"]); let mut ctx = RequestContext::new(app_state, WorkingMode::Cmd); @@ -2675,6 +2658,7 @@ mod tests { } #[test] + #[serial] fn rebuild_tool_scope_no_mcp_config_yields_empty_runtime() { let app_state = app_state_with_mcp_config(true, &[]); let mut ctx = RequestContext::new(app_state, WorkingMode::Cmd); @@ -2687,6 +2671,7 @@ mod tests { } #[test] + #[serial] fn rebuild_tool_scope_preserves_tool_tracker() { let app_state = app_state_with_mcp_config(false, &[]); let mut ctx = RequestContext::new(app_state, WorkingMode::Cmd); @@ -2713,6 +2698,7 @@ mod tests { } #[test] + #[serial] fn rebuild_tool_scope_repl_mode_appends_user_interaction_functions() { let app_state = app_state_with_mcp_config(false, &[]); let mut ctx = RequestContext::new(app_state, WorkingMode::Repl); @@ -2735,6 +2721,7 @@ mod tests { } #[test] + #[serial] fn rebuild_tool_scope_cmd_mode_no_user_interaction_functions() { let app_state = app_state_with_mcp_config(false, &[]); let mut ctx = RequestContext::new(app_state, WorkingMode::Cmd); @@ -2766,8 +2753,10 @@ mod tests { #[test] fn select_functions_returns_none_when_function_calling_disabled() { let app_state = { - let mut config = AppConfig::default(); - config.function_calling_support = false; + let config = AppConfig { + function_calling_support: false, + ..AppConfig::default() + }; Arc::new(AppState { config: Arc::new(config), vault: Arc::new(Vault::default()), @@ -2818,8 +2807,10 @@ mod tests { #[test] fn select_enabled_mcp_servers_returns_empty_when_mcp_disabled() { let app_state = { - let mut config = AppConfig::default(); - config.mcp_server_support = false; + let config = AppConfig { + mcp_server_support: false, + ..AppConfig::default() + }; Arc::new(AppState { config: Arc::new(config), vault: Arc::new(Vault::default()), @@ -2929,6 +2920,7 @@ mod tests { } #[test] + #[serial] fn use_role_obj_errors_when_agent_active() { let _guard = TestConfigDirGuard::new(); let mut ctx = create_test_ctx(); @@ -3065,4 +3057,236 @@ mod tests { ctx.session = Some(Session::default()); assert!(!ctx.is_compressing_session()); } + + #[test] + #[serial] + fn retrieve_role_from_markdown_file() { + let _guard = TestConfigDirGuard::new(); + let roles_dir = paths::roles_dir(); + create_dir_all(&roles_dir).unwrap(); + write( + roles_dir.join("pirate.md"), + "You are a pirate. Speak only in pirate language.", + ) + .unwrap(); + + let ctx = create_test_ctx(); + let role = ctx.retrieve_role(&ctx.app.config, "pirate").unwrap(); + assert_eq!(role.name(), "pirate"); + assert!(role.prompt().contains("pirate")); + } + + #[test] + #[serial] + fn retrieve_role_builtin_exists() { + let _guard = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + let names = paths::list_roles(true); + if !names.is_empty() { + let role = ctx.retrieve_role(&ctx.app.config, &names[0]); + assert!(role.is_ok()); + } + } + + #[test] + #[serial] + fn retrieve_role_nonexistent_errors() { + let _guard = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + let result = ctx.retrieve_role(&ctx.app.config, "definitely_not_a_real_role_xyz"); + assert!(result.is_err()); + } + + #[test] + #[serial] + fn retrieve_role_no_model_id_inherits_current_model() { + let _guard = TestConfigDirGuard::new(); + let roles_dir = paths::roles_dir(); + create_dir_all(&roles_dir).unwrap(); + write(roles_dir.join("simple.md"), "You are helpful.").unwrap(); + + let ctx = create_test_ctx(); + let role = ctx.retrieve_role(&ctx.app.config, "simple").unwrap(); + assert_eq!(role.model().id(), ctx.current_model().id()); + } + + #[test] + #[serial] + fn list_roles_finds_markdown_files() { + let _guard = TestConfigDirGuard::new(); + let roles_dir = paths::roles_dir(); + create_dir_all(&roles_dir).unwrap(); + write(roles_dir.join("alpha.md"), "Alpha role").unwrap(); + write(roles_dir.join("beta.md"), "Beta role").unwrap(); + write(roles_dir.join("not_a_role.txt"), "ignored").unwrap(); + + let names = paths::list_roles(false); + assert!(names.contains(&"alpha".to_string())); + assert!(names.contains(&"beta".to_string())); + assert!(!names.contains(&"not_a_role".to_string())); + } + + #[test] + #[serial] + fn list_roles_empty_dir() { + let _guard = TestConfigDirGuard::new(); + let roles_dir = paths::roles_dir(); + create_dir_all(&roles_dir).unwrap(); + let names = paths::list_roles(false); + assert!(names.is_empty()); + } + + #[test] + #[serial] + fn session_new_from_ctx_captures_state() { + let _guard = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + let session = Session::new_from_ctx(&ctx, &ctx.app.config, "test-session"); + assert_eq!(session.name(), "test-session"); + assert!(session.is_empty()); + } + + #[test] + #[serial] + fn session_save_creates_file() { + let _guard = TestConfigDirGuard::new(); + let ctx = create_test_ctx(); + let mut session = Session::new_from_ctx(&ctx, &ctx.app.config, "save-test"); + let session_path = ctx.session_file("save-test"); + ensure_parent_exists(&session_path).unwrap(); + + session.save("save-test", &session_path, false).unwrap(); + assert!(session_path.exists()); + } + + #[test] + #[serial] + fn use_session_errors_when_already_in_session() { + let _guard = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + ctx.session = Some(Session::default()); + + let app = ctx.app.config.clone(); + let abort = utils::create_abort_signal(); + let result = run_async(ctx.use_session(&app, Some("new"), abort)); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("Already in a session") + ); + } + + #[test] + #[serial] + fn use_session_creates_temp_session() { + let _guard = TestConfigDirGuard::new(); + let sessions_dir = paths::local_path("sessions"); + create_dir_all(&sessions_dir).unwrap(); + + let mut ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + let abort = utils::create_abort_signal(); + run_async(ctx.use_session(&app, None, abort)).unwrap(); + + assert!(ctx.session.is_some()); + assert_eq!(ctx.session.as_ref().unwrap().name(), TEMP_SESSION_NAME); + } + + #[test] + #[serial] + fn use_session_creates_named_session() { + let _guard = TestConfigDirGuard::new(); + let sessions_dir = paths::local_path("sessions"); + create_dir_all(&sessions_dir).unwrap(); + + let mut ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + let abort = utils::create_abort_signal(); + run_async(ctx.use_session(&app, Some("my-session"), abort)).unwrap(); + + assert!(ctx.session.is_some()); + assert_eq!(ctx.session.as_ref().unwrap().name(), "my-session"); + } + + #[test] + #[serial] + fn exit_session_roundtrip() { + let _guard = TestConfigDirGuard::new(); + let sessions_dir = paths::local_path("sessions"); + create_dir_all(&sessions_dir).unwrap(); + + let mut ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + let abort = utils::create_abort_signal(); + run_async(ctx.use_session(&app, Some("roundtrip"), abort.clone())).unwrap(); + assert!(ctx.session.is_some()); + + ctx.exit_session().unwrap(); + assert!(ctx.session.is_none()); + } + + #[test] + #[serial] + fn use_role_obj_and_exit_role_full_cycle() { + let _guard = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + + ctx.use_role_obj(Role::new("test-role", "test prompt")) + .unwrap(); + assert!(ctx.role.is_some()); + assert_eq!(ctx.role.as_ref().unwrap().name(), "test-role"); + + let _ = ctx.exit_role(); + assert!(ctx.role.is_none()); + } + + #[test] + #[serial] + fn use_role_obj_twice_replaces_role() { + let _guard = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + + ctx.use_role_obj(Role::new("first", "prompt 1")).unwrap(); + assert_eq!(ctx.role.as_ref().unwrap().name(), "first"); + + ctx.use_role_obj(Role::new("second", "prompt 2")).unwrap(); + assert_eq!(ctx.role.as_ref().unwrap().name(), "second"); + } + + #[test] + #[serial] + fn list_macros_finds_yaml_files() { + let _guard = TestConfigDirGuard::new(); + let macros_dir = paths::macros_dir(); + create_dir_all(¯os_dir).unwrap(); + write(macros_dir.join("greet.yaml"), "steps:\n - \".help\"").unwrap(); + write(macros_dir.join("build.yaml"), "steps:\n - \".help\"").unwrap(); + + let names = paths::list_macros(); + assert!(names.contains(&"greet".to_string())); + assert!(names.contains(&"build".to_string())); + } + + #[test] + #[serial] + fn list_rags_finds_yaml_files() { + let _guard = TestConfigDirGuard::new(); + let rags_dir = paths::rags_dir(); + create_dir_all(&rags_dir).unwrap(); + write(rags_dir.join("docs.yaml"), "embedding_model: test").unwrap(); + + let names = paths::list_rags(); + assert!(names.contains(&"docs".to_string())); + } + + #[test] + #[serial] + fn list_rags_empty_dir() { + let _guard = TestConfigDirGuard::new(); + let rags_dir = paths::rags_dir(); + create_dir_all(&rags_dir).unwrap(); + assert!(paths::list_rags().is_empty()); + } } diff --git a/src/config/tool_scope.rs b/src/config/tool_scope.rs index 3f3fdf5..b61c982 100644 --- a/src/config/tool_scope.rs +++ b/src/config/tool_scope.rs @@ -1,23 +1,3 @@ -//! Per-scope tool runtime: resolved functions + live MCP handles + -//! call tracker. -//! -//! `ToolScope` is the unit of tool availability for a single request. -//! Every active `RoleLike` (role, session, agent) conceptually owns one. -//! The contents are: -//! -//! * `functions` — the `Functions` declarations visible to the LLM for -//! this scope (global tools + role/session/agent filters applied) -//! * `mcp_runtime` — live MCP subprocess handles for the servers this -//! scope has enabled, keyed by server name -//! * `tool_tracker` — per-scope tool call history for auto-continuation -//! and looping detection -//! -//! `ToolScope` lives on [`RequestContext`](super::request_context::RequestContext) -//! and is built/replaced as the active scope changes (role swap, -//! session swap, agent enter/exit). The base `functions` are seeded -//! from [`AppState`](super::app_state::AppState) and per-scope filters -//! narrow the visible set. - use crate::function::{Functions, ToolCallTracker}; use crate::mcp::{CatalogItem, ConnectedServer, McpRegistry}; diff --git a/src/rag/mod.rs b/src/rag/mod.rs index 36d81bd..33f245e 100644 --- a/src/rag/mod.rs +++ b/src/rag/mod.rs @@ -1280,7 +1280,11 @@ mod tests { ]; for ext in known { let seps = get_separators(ext); - assert_ne!(seps, DEFAULT_SEPARATORS.to_vec(), "Extension '{ext}' should have language-specific separators"); + assert_ne!( + seps, + DEFAULT_SEPARATORS.to_vec(), + "Extension '{ext}' should have language-specific separators" + ); } } diff --git a/src/rag/splitter/mod.rs b/src/rag/splitter/mod.rs index c6bb6ff..edfa500 100644 --- a/src/rag/splitter/mod.rs +++ b/src/rag/splitter/mod.rs @@ -171,7 +171,6 @@ impl RecursiveCharacterTextSplitter { } } - // Now that we have the separator, split the text let splits = split_on_separator(text, &separator, keep_separator); // Now go merging things, recursively splitting longer texts.