diff --git a/docs/implementation/PHASE-1-STEP-16f-NOTES.md b/docs/implementation/PHASE-1-STEP-16f-NOTES.md new file mode 100644 index 0000000..d667711 --- /dev/null +++ b/docs/implementation/PHASE-1-STEP-16f-NOTES.md @@ -0,0 +1,303 @@ +# Phase 1 Step 16f — Implementation Notes + +## Status + +Done. Phase 1 Step 16 (Config → AppConfig migration) complete. + +## Plan reference + +- Parent plan: `docs/implementation/PHASE-1-STEP-16-NOTES.md` +- Predecessor: `docs/implementation/PHASE-1-STEP-16e-NOTES.md` +- Sub-phase goal: "Delete all #[allow(dead_code)] scaffolding from + Config and bridge.rs, delete runtime fields from Config, delete + bridge.rs entirely." + +## Summary + +`Config` is now a pure serde POJO. `bridge.rs` is gone. Every +runtime field, every `Config::init*` flavor, and every Config method +that was scaffolding for the old god-init has been deleted. The +project compiles clean, clippy clean, and all 122 tests pass. + +## What was changed + +### Deleted: `src/config/bridge.rs` + +Whole file removed. `mod bridge;` declaration in `config/mod.rs` +removed. The two methods (`Config::to_app_config` and +`Config::to_request_context`) had no remaining callers after 16e. + +### `src/config/mod.rs` — Config slimmed to a POJO + +**Deleted runtime (`#[serde(skip)]`) fields from `Config`:** +- `vault`, `macro_flag`, `info_flag`, `agent_variables` +- `model`, `functions`, `mcp_registry`, `working_mode`, + `last_message` +- `role`, `session`, `rag`, `agent`, `tool_call_tracker` +- `supervisor`, `parent_supervisor`, `self_agent_id`, + `current_depth`, `inbox`, `root_escalation_queue` + +**Deleted methods on `Config`:** +- `init`, `init_bare` (god-init replaced by + `load_with_interpolation` + `AppConfig::from_config` + + `AppState::init` + `RequestContext::bootstrap`) +- `sessions_dir`, `list_sessions` (replaced by + `config::default_sessions_dir` / `config::list_sessions` free + functions for use without a Config; per-context paths live on + `RequestContext::sessions_dir` / `RequestContext::list_sessions`) +- `role_like_mut` (lives on `RequestContext` post-migration) +- `set_wrap`, `setup_document_loaders`, `setup_user_agent`, + `load_envs` (lives on `AppConfig` post-migration) +- `set_model`, `setup_model` (model resolution now in + `AppConfig::resolve_model`; per-scope model selection lives on + `RequestContext`) +- `load_functions`, `load_mcp_servers` (absorbed by + `AppState::init`) + +**Default impl entries** for the deleted runtime fields removed. + +**Imports cleaned up:** removed unused `ToolCallTracker`, +`McpRegistry`, `Supervisor`, `EscalationQueue`, `Inbox`, `RwLock`, +`ColorScheme`, `QueryOptions`, `color_scheme`, `Handle`. Kept +`Model`, `ModelType`, `GlobalVault` because sibling modules +(`role.rs`, `input.rs`, `agent.rs`, `session.rs`) use +`use super::*;` and depend on those re-exports. + +**Removed assertions** for the deleted runtime fields from +`config_defaults_match_expected` test. + +### `src/config/mod.rs` — `load_with_interpolation` no longer touches AppConfig::to_app_config + +Previously called `config.to_app_config()` to build a Vault for +secret interpolation. Now constructs a minimal `AppConfig` inline +with only `vault_password_file` populated, since that's all +`Vault::init` reads. Also removed the `config.vault = Arc::new(vault)` +assignment that was the last write to the deleted runtime field. + +### `src/config/mod.rs` — `vault_password_file` made `pub(super)` + +Previously private. Now `pub(super)` so `AppConfig::from_config` (a +sibling module under `config/`) can read it during the field-copy. + +### `src/config/app_config.rs` — `AppConfig::from_config` self-contained + +Previously delegated to `Config::to_app_config()` (lived on bridge) +for the field-copy. Now inlines the field-copy directly in +`from_config`, then runs `load_envs`, `set_wrap`, +`setup_document_loaders`, `setup_user_agent`, and `resolve_model` +as before. + +**Removed `#[allow(dead_code)]` from `AppConfig.model_id`** — it's +read from `app.config.model_id` in `RequestContext::bootstrap` so +the lint exemption was stale. + +**Test refactor:** the three `to_app_config_*` tests rewritten as +`from_config_*` tests using `AppConfig::from_config(cfg).unwrap()`. +A `ClientConfig::default()` and non-empty `model_id: "test-model"` +were added so `resolve_model()` doesn't bail with "No available +model" during the runtime initialization. + +### `src/config/session.rs` — Test helper rewired + +`session_new_from_ctx_captures_save_session` rewritten to build the +test `AppState` directly with `AppConfig::default()`, +`Vault::default()`, `Functions::default()` instead of going through +`cfg.to_app_config()` / `cfg.vault` / `cfg.functions`. Then uses +`RequestContext::new(app_state, WorkingMode::Cmd)` instead of the +deleted `cfg.to_request_context(app_state)`. + +### `src/config/request_context.rs` — Test helpers rewired + +The `app_state_from_config(&Config)` helper rewritten as +`default_app_state()` — no longer takes a Config, builds AppState +from `AppConfig::default()` + `Vault::default()` + `Functions::default()` +directly. The two callers (`create_test_ctx`, +`update_app_config_persists_changes`) updated. + +The `to_request_context_creates_clean_state` test renamed to +`new_creates_clean_state` and rewritten to use `RequestContext::new` +directly. + +### Doc comment refresh + +Three module docstrings rewritten to reflect the post-16f world: + +- `app_config.rs` — was "Phase 1 Step 0 ... not yet wired into the + runtime." Now describes `AppConfig` as the runtime-resolved + view of YAML, built via `AppConfig::from_config`. +- `app_state.rs` — was "Step 6.5 added mcp_factory and rag_cache + ... neither wired in yet ... Step 8+ will connect." Now + describes `AppState::init` as the wiring point. +- `request_context.rs` — was an extensive description of the + bridge window with flat fields vs sub-struct fields, citing + `Config::to_request_context`. Now describes the type's actual + ownership/lifecycle without referring to deleted entry points. +- `tool_scope.rs` — was "Step 6.5 scope ... unused parallel + structure ... Step 8 will rewrite." Now describes `ToolScope` + as the live per-scope tool runtime. + +(Other phase-era comments in `paths.rs`, `mcp_factory.rs`, +`rag_cache.rs` not touched. They reference Step 2 / Step 6.5 / +Step 8 but the affected types still exist and the descriptions +aren't actively misleading — those files weren't part of 16f +scope. Future cleanup if desired.) + +## What Config looks like now + +```rust +#[derive(Debug, Clone, Deserialize)] +#[serde(default)] +pub struct Config { + pub model_id: String, + pub temperature: Option, + pub top_p: Option, + pub dry_run: bool, + pub stream: bool, + pub save: bool, + pub keybindings: String, + pub editor: Option, + pub wrap: Option, + pub wrap_code: bool, + pub(super) vault_password_file: Option, + pub function_calling_support: bool, + pub mapping_tools: IndexMap, + pub enabled_tools: Option, + pub visible_tools: Option>, + pub mcp_server_support: bool, + pub mapping_mcp_servers: IndexMap, + pub enabled_mcp_servers: Option, + pub repl_prelude: Option, + pub cmd_prelude: Option, + pub agent_session: Option, + pub save_session: Option, + pub compression_threshold: usize, + pub summarization_prompt: Option, + pub summary_context_prompt: Option, + pub rag_embedding_model: Option, + pub rag_reranker_model: Option, + pub rag_top_k: usize, + pub rag_chunk_size: Option, + pub rag_chunk_overlap: Option, + pub rag_template: Option, + pub document_loaders: HashMap, + pub highlight: bool, + pub theme: Option, + pub left_prompt: Option, + pub right_prompt: Option, + pub user_agent: Option, + pub save_shell_history: bool, + pub sync_models_url: Option, + pub clients: Vec, +} + +impl Config { + pub async fn search_rag(...) -> Result { ... } + pub fn load_macro(name: &str) -> Result { ... } + pub async fn sync_models(url: &str, ...) -> Result<()> { ... } + pub async fn load_with_interpolation(info_flag: bool) -> Result { ... } + pub fn load_from_file(config_path: &Path) -> Result<(Self, String)> { ... } + pub fn load_from_str(content: &str) -> Result { ... } + pub fn load_dynamic(model_id: &str) -> Result { ... } +} +``` + +Just shape + loaders + a few helpers that don't touch runtime +state. `search_rag`/`load_macro`/`sync_models` are associated +functions (no `&self`) that could be free functions someday, but +that's cosmetic and out of scope for 16f. + +## Assumptions made + +1. **Doc cleanup scope**: The user asked to "delete the dead-code + scaffolding from Config and bridge.rs." Doc comments in + `paths.rs`, `mcp_factory.rs`, `rag_cache.rs` still reference + "Phase 1 Step 6.5 / Step 8" but the types they describe are + still real and the descriptions aren't actively wrong (just + historically dated). Left them alone. Updated only the docs in + `app_config.rs`, `app_state.rs`, `request_context.rs`, and + `tool_scope.rs` because those were either pointing at deleted + types (`Config::to_request_context`) or making explicitly + false claims ("not wired into the runtime yet"). + +2. **`set_*_default` helpers on AppConfig**: Lines 485–528 of + `app_config.rs` define nine `#[allow(dead_code)]` + `set_*_default` methods. These were added in earlier sub-phases + as planned setters for runtime overrides. They're still unused. + The 16-NOTES plan flagged them ("set_*_default ... become + reachable") but reachability never happened. Since the user's + directive was specifically "Config and bridge.rs scaffolding," + I left these untouched. Removing them is independent cleanup + that doesn't block 16f. + +3. **`reload_current_model` on RequestContext**: Same situation — + one `#[allow(dead_code)]` left on a RequestContext method. + Belongs to a different cleanup task; not Config or bridge + scaffolding. + +4. **`vault_password_file` visibility**: `Config.vault_password_file` + was a private field. Made it `pub(super)` so + `AppConfig::from_config` (sibling under `config/`) can read it + for the field-copy. This is the minimum viable visibility — + no code outside `config/` can touch it, matching the previous + intent. + +5. **Bootstrap vault construction in `load_with_interpolation`**: + Used `AppConfig { vault_password_file: ..., ..AppConfig::default() }` + instead of e.g. a dedicated helper. The vault only reads + `vault_password_file` so this is sufficient. A comment explains + the dual-vault pattern (bootstrap for secret interpolation vs + canonical from `AppState::init`). + +## Verification + +- `cargo check` — clean, zero warnings +- `cargo clippy --all-targets` — clean, zero warnings +- `cargo test` — 122 passing, zero failures (same count as 16e) +- Grep confirmation: + - `to_app_config` — zero hits in `src/` + - `to_request_context` — zero hits in `src/` + - `Config::init` / `Config::init_bare` — zero hits in `src/` + - `bridge::` / `config::bridge` / `mod bridge` — zero hits in `src/` + - `src/config/bridge.rs` — file deleted +- Config now contains only serde fields and load/helper + functions; no runtime state. + +## Phase 1 Step 16 — overall outcome + +The full migration is complete: + +| Sub-phase | Outcome | +|-----------|---------| +| 16a | `AppConfig::from_config` built | +| 16b | `install_builtins()` extracted | +| 16c | Vault on AppState (already-existing field, `Vault::init` rewired to `&AppConfig`) | +| 16d | `AppState::init` built | +| 16e | `main.rs` + completers + `RequestContext::bootstrap` switched to new flow | +| 16f | Bridge + Config runtime fields + dead methods deleted | + +`Config` is a serde POJO. `AppConfig` is the runtime-resolved +process-wide settings. `AppState` owns process-wide services +(vault, MCP registry, base functions, MCP factory, RAG cache). +`RequestContext` owns per-request mutable state. Each struct +owns its initialization. The REST API surface is now trivial: +parse YAML → `AppConfig::from_config` → `AppState::init` → +per-request `RequestContext`. + +## Files modified (16f) + +- `src/config/mod.rs` — runtime fields/methods/Default entries + deleted, imports cleaned up, `vault_password_file` made + `pub(super)`, `load_with_interpolation` decoupled from + `to_app_config`, default-test simplified +- `src/config/app_config.rs` — `from_config` inlines field-copy, + `#[allow(dead_code)]` on `model_id` removed, three tests + rewritten, module docstring refreshed +- `src/config/session.rs` — test helper rewired, imports updated +- `src/config/request_context.rs` — test helpers rewired, + imports updated, module docstring refreshed +- `src/config/app_state.rs` — module docstring refreshed +- `src/config/tool_scope.rs` — module docstring refreshed + +## Files deleted (16f) + +- `src/config/bridge.rs` diff --git a/src/config/app_config.rs b/src/config/app_config.rs index 1157578..fdd1da8 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -1,23 +1,19 @@ -//! Immutable, server-wide application configuration. +//! 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. +//! global to the Loki process: LLM provider configs, UI preferences, +//! tool and MCP settings, RAG defaults, etc. //! -//! This is Phase 1, Step 0 of the REST API refactor: the struct is -//! introduced alongside the existing [`Config`](super::Config) and is not -//! yet wired into the runtime. See `docs/PHASE-1-IMPLEMENTATION-PLAN.md` -//! for the full migration plan. -//! -//! # Relationship to `Config` -//! -//! `AppConfig` mirrors the **serialized** fields of [`Config`] — that is, -//! every field that is NOT marked `#[serde(skip)]`. The deserialization -//! shape is identical so an existing `config.yaml` can be loaded into -//! either type without modification. +//! `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}; @@ -38,7 +34,6 @@ use terminal_colorsaurus::{ColorScheme, QueryOptions, color_scheme}; pub struct AppConfig { #[serde(rename(serialize = "model", deserialize = "model"))] #[serde(default)] - #[allow(dead_code)] pub model_id: String, pub temperature: Option, pub top_p: Option, @@ -151,7 +146,58 @@ impl Default for AppConfig { impl AppConfig { pub fn from_config(config: super::Config) -> Result { - let mut app_config = config.to_app_config(); + let mut app_config = Self { + model_id: config.model_id, + temperature: config.temperature, + top_p: config.top_p, + + dry_run: config.dry_run, + stream: config.stream, + save: config.save, + keybindings: config.keybindings, + editor: config.editor, + wrap: config.wrap, + wrap_code: config.wrap_code, + vault_password_file: config.vault_password_file, + + function_calling_support: config.function_calling_support, + mapping_tools: config.mapping_tools, + enabled_tools: config.enabled_tools, + visible_tools: config.visible_tools, + + mcp_server_support: config.mcp_server_support, + mapping_mcp_servers: config.mapping_mcp_servers, + enabled_mcp_servers: config.enabled_mcp_servers, + + repl_prelude: config.repl_prelude, + cmd_prelude: config.cmd_prelude, + agent_session: config.agent_session, + + save_session: config.save_session, + compression_threshold: config.compression_threshold, + summarization_prompt: config.summarization_prompt, + summary_context_prompt: config.summary_context_prompt, + + rag_embedding_model: config.rag_embedding_model, + rag_reranker_model: config.rag_reranker_model, + rag_top_k: config.rag_top_k, + rag_chunk_size: config.rag_chunk_size, + rag_chunk_overlap: config.rag_chunk_overlap, + rag_template: config.rag_template, + + document_loaders: config.document_loaders, + + highlight: config.highlight, + theme: config.theme, + left_prompt: config.left_prompt, + right_prompt: config.right_prompt, + + user_agent: config.user_agent, + save_shell_history: config.save_shell_history, + sync_models_url: config.sync_models_url, + + clients: config.clients, + }; app_config.load_envs(); if let Some(wrap) = app_config.wrap.clone() { app_config.set_wrap(&wrap)?; @@ -491,7 +537,7 @@ mod tests { } #[test] - fn to_app_config_copies_serialized_fields() { + fn from_config_copies_serialized_fields() { let cfg = Config { model_id: "test-model".to_string(), temperature: Some(0.7), @@ -502,10 +548,11 @@ mod tests { highlight: false, compression_threshold: 2000, rag_top_k: 10, + clients: vec![ClientConfig::default()], ..Config::default() }; - let app = cfg.to_app_config(); + let app = AppConfig::from_config(cfg).unwrap(); assert_eq!(app.model_id, "test-model"); assert_eq!(app.temperature, Some(0.7)); @@ -519,22 +566,30 @@ mod tests { } #[test] - fn to_app_config_copies_clients() { - let cfg = Config::default(); - let app = cfg.to_app_config(); + fn from_config_copies_clients() { + let cfg = Config { + model_id: "test-model".to_string(), + clients: vec![ClientConfig::default()], + ..Config::default() + }; + let app = AppConfig::from_config(cfg).unwrap(); - assert!(app.clients.is_empty()); + assert_eq!(app.clients.len(), 1); } #[test] - fn to_app_config_copies_mapping_fields() { - let mut cfg = Config::default(); + fn from_config_copies_mapping_fields() { + let mut cfg = Config { + model_id: "test-model".to_string(), + clients: vec![ClientConfig::default()], + ..Config::default() + }; cfg.mapping_tools .insert("alias".to_string(), "real_tool".to_string()); cfg.mapping_mcp_servers .insert("gh".to_string(), "github-mcp".to_string()); - let app = cfg.to_app_config(); + let app = AppConfig::from_config(cfg).unwrap(); assert_eq!( app.mapping_tools.get("alias"), diff --git a/src/config/app_state.rs b/src/config/app_state.rs index dfc4b50..fdc18b2 100644 --- a/src/config/app_state.rs +++ b/src/config/app_state.rs @@ -3,30 +3,16 @@ //! `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, and the [`RagCache`](super::rag_cache::RagCache) -//! for shared RAG instances. It is intended to be wrapped in `Arc` -//! and shared across every [`RequestContext`] that a frontend (CLI, -//! REPL, API) creates. +//! 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. //! -//! This struct deliberately does **not** hold a live `McpRegistry`. -//! MCP server processes are scoped to whichever `RoleLike` -//! (role/session/agent) is currently active, because each scope may -//! demand a different enabled server set. Live MCP processes are -//! owned by per-scope -//! [`ToolScope`](super::tool_scope::ToolScope)s on the -//! [`RequestContext`] and acquired through `McpFactory`. -//! -//! # Phase 1 scope -//! -//! This is Phase 1 of the REST API refactor: -//! -//! * **Step 0** introduced this struct alongside the existing -//! [`Config`](super::Config) -//! * **Step 6.5** added the `mcp_factory` and `rag_cache` fields -//! -//! Neither field is wired into the runtime yet — they exist as -//! additive scaffolding that Step 8+ will connect when the entry -//! points migrate. See `docs/PHASE-1-IMPLEMENTATION-PLAN.md`. +//! 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; diff --git a/src/config/bridge.rs b/src/config/bridge.rs deleted file mode 100644 index b589cbb..0000000 --- a/src/config/bridge.rs +++ /dev/null @@ -1,104 +0,0 @@ -//! Transitional conversions between the legacy [`Config`] struct and the -//! new [`AppConfig`] + [`RequestContext`] split. - -use crate::config::todo::TodoList; - -use super::{AppConfig, AppState, Config, RequestContext}; - -use std::sync::Arc; - -impl Config { - pub fn to_app_config(&self) -> AppConfig { - AppConfig { - model_id: self.model_id.clone(), - temperature: self.temperature, - top_p: self.top_p, - - dry_run: self.dry_run, - stream: self.stream, - save: self.save, - keybindings: self.keybindings.clone(), - editor: self.editor.clone(), - wrap: self.wrap.clone(), - wrap_code: self.wrap_code, - vault_password_file: self.vault_password_file.clone(), - - function_calling_support: self.function_calling_support, - mapping_tools: self.mapping_tools.clone(), - enabled_tools: self.enabled_tools.clone(), - visible_tools: self.visible_tools.clone(), - - mcp_server_support: self.mcp_server_support, - mapping_mcp_servers: self.mapping_mcp_servers.clone(), - enabled_mcp_servers: self.enabled_mcp_servers.clone(), - - repl_prelude: self.repl_prelude.clone(), - cmd_prelude: self.cmd_prelude.clone(), - agent_session: self.agent_session.clone(), - - save_session: self.save_session, - compression_threshold: self.compression_threshold, - summarization_prompt: self.summarization_prompt.clone(), - summary_context_prompt: self.summary_context_prompt.clone(), - - rag_embedding_model: self.rag_embedding_model.clone(), - rag_reranker_model: self.rag_reranker_model.clone(), - rag_top_k: self.rag_top_k, - rag_chunk_size: self.rag_chunk_size, - rag_chunk_overlap: self.rag_chunk_overlap, - rag_template: self.rag_template.clone(), - - document_loaders: self.document_loaders.clone(), - - highlight: self.highlight, - theme: self.theme.clone(), - left_prompt: self.left_prompt.clone(), - right_prompt: self.right_prompt.clone(), - - user_agent: self.user_agent.clone(), - save_shell_history: self.save_shell_history, - sync_models_url: self.sync_models_url.clone(), - - clients: self.clients.clone(), - } - } - - #[allow(dead_code)] - pub fn to_request_context(&self, app: Arc) -> RequestContext { - let mut mcp_runtime = super::tool_scope::McpRuntime::default(); - if let Some(registry) = &self.mcp_registry { - mcp_runtime.sync_from_registry(registry); - } - let tool_tracker = self - .tool_call_tracker - .clone() - .unwrap_or_else(crate::function::ToolCallTracker::default); - RequestContext { - app, - macro_flag: self.macro_flag, - info_flag: self.info_flag, - working_mode: self.working_mode, - model: self.model.clone(), - agent_variables: self.agent_variables.clone(), - role: self.role.clone(), - session: self.session.clone(), - rag: self.rag.clone(), - agent: self.agent.clone(), - last_message: self.last_message.clone(), - tool_scope: super::tool_scope::ToolScope { - functions: self.functions.clone(), - mcp_runtime, - tool_tracker, - }, - supervisor: self.supervisor.clone(), - parent_supervisor: self.parent_supervisor.clone(), - self_agent_id: self.self_agent_id.clone(), - inbox: self.inbox.clone(), - escalation_queue: self.root_escalation_queue.clone(), - current_depth: self.current_depth, - auto_continue_count: 0, - todo_list: TodoList::default(), - last_continuation_response: None, - } - } -} diff --git a/src/config/mod.rs b/src/config/mod.rs index 0ba5efb..b2b6114 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,7 +1,6 @@ mod agent; mod app_config; mod app_state; -mod bridge; mod input; mod macros; mod mcp_factory; @@ -28,25 +27,20 @@ pub use self::role::{ use self::session::Session; use crate::client::{ ClientConfig, MessageContentToolCalls, Model, ModelType, OPENAI_COMPATIBLE_PROVIDERS, - ProviderModels, create_client_config, list_client_types, list_models, + ProviderModels, create_client_config, list_client_types, }; -use crate::function::{FunctionDeclaration, Functions, ToolCallTracker}; +use crate::function::{FunctionDeclaration, Functions}; use crate::rag::Rag; use crate::utils::*; pub use macros::macro_execute; use crate::config::macros::Macro; -use crate::mcp::McpRegistry; -use crate::supervisor::Supervisor; -use crate::supervisor::escalation::EscalationQueue; -use crate::supervisor::mailbox::Inbox; use crate::vault::{GlobalVault, Vault, create_vault_password_file, interpolate_secrets}; use anyhow::{Context, Result, anyhow, bail}; use fancy_regex::Regex; use indexmap::IndexMap; use indoc::formatdoc; use inquire::{Confirm, Select}; -use parking_lot::RwLock; use serde::{Deserialize, Serialize}; use serde_json::json; use std::collections::HashMap; @@ -59,8 +53,6 @@ use std::{ process, sync::{Arc, OnceLock}, }; -use terminal_colorsaurus::{ColorScheme, QueryOptions, color_scheme}; -use tokio::runtime::Handle; pub const TEMP_ROLE_NAME: &str = "temp"; pub const TEMP_RAG_NAME: &str = "temp"; @@ -142,7 +134,7 @@ pub struct Config { pub editor: Option, pub wrap: Option, pub wrap_code: bool, - vault_password_file: Option, + pub(super) vault_password_file: Option, pub function_calling_support: bool, pub mapping_tools: IndexMap, @@ -182,50 +174,6 @@ pub struct Config { pub sync_models_url: Option, pub clients: Vec, - - #[serde(skip)] - pub vault: GlobalVault, - - #[serde(skip)] - pub macro_flag: bool, - #[serde(skip)] - pub info_flag: bool, - #[serde(skip)] - pub agent_variables: Option, - - #[serde(skip)] - pub model: Model, - #[serde(skip)] - pub functions: Functions, - #[serde(skip)] - pub mcp_registry: Option, - #[serde(skip)] - pub working_mode: WorkingMode, - #[serde(skip)] - pub last_message: Option, - - #[serde(skip)] - pub role: Option, - #[serde(skip)] - pub session: Option, - #[serde(skip)] - pub rag: Option>, - #[serde(skip)] - pub agent: Option, - #[serde(skip)] - pub(crate) tool_call_tracker: Option, - #[serde(skip)] - pub supervisor: Option>>, - #[serde(skip)] - pub parent_supervisor: Option>>, - #[serde(skip)] - pub self_agent_id: Option, - #[serde(skip)] - pub current_depth: usize, - #[serde(skip)] - pub inbox: Option>, - #[serde(skip)] - pub root_escalation_queue: Option>, } impl Default for Config { @@ -281,30 +229,6 @@ impl Default for Config { sync_models_url: None, clients: vec![], - - vault: Default::default(), - - macro_flag: false, - info_flag: false, - agent_variables: None, - - model: Default::default(), - functions: Default::default(), - mcp_registry: Default::default(), - working_mode: WorkingMode::Cmd, - last_message: None, - - role: None, - session: None, - rag: None, - agent: None, - tool_call_tracker: Some(ToolCallTracker::default()), - supervisor: None, - parent_supervisor: None, - self_agent_id: None, - current_depth: 0, - inbox: None, - root_escalation_queue: None, } } } @@ -328,154 +252,6 @@ pub fn list_sessions() -> Vec { } impl Config { - #[allow(dead_code)] - pub fn init_bare() -> Result { - let h = Handle::current(); - tokio::task::block_in_place(|| { - h.block_on(Self::init( - WorkingMode::Cmd, - true, - false, - None, - create_abort_signal(), - )) - }) - } - - pub async fn init( - working_mode: WorkingMode, - info_flag: bool, - start_mcp_servers: bool, - log_path: Option, - abort_signal: AbortSignal, - ) -> Result { - let config_path = paths::config_file(); - let (mut config, content) = if !config_path.exists() { - match env::var(get_env_name("provider")) - .ok() - .or_else(|| env::var(get_env_name("platform")).ok()) - { - Some(v) => (Self::load_dynamic(&v)?, String::new()), - None => { - if *IS_STDOUT_TERMINAL { - create_config_file(&config_path).await?; - } - Self::load_from_file(&config_path)? - } - } - } else { - Self::load_from_file(&config_path)? - }; - - let setup = async |config: &mut Self| -> Result<()> { - let vault = Vault::init(&config.to_app_config()); - - let (parsed_config, missing_secrets) = interpolate_secrets(&content, &vault); - if !missing_secrets.is_empty() && !info_flag { - debug!( - "Global config references secrets that are missing from the vault: {missing_secrets:?}" - ); - return Err(anyhow!(formatdoc!( - " - Global config file references secrets that are missing from the vault: {:?} - Please add these secrets to the vault and try again.", - missing_secrets - ))); - } - - if !parsed_config.is_empty() && !info_flag { - debug!("Global config is invalid once secrets are injected: {parsed_config}"); - let new_config = Self::load_from_str(&parsed_config).with_context(|| { - formatdoc!( - " - Global config is invalid once secrets are injected. - Double check the secret values and file syntax, then try again. - " - ) - })?; - *config = new_config.clone(); - } - - config.working_mode = working_mode; - config.info_flag = info_flag; - config.vault = Arc::new(vault); - - config.load_envs(); - - if let Some(wrap) = config.wrap.clone() { - config.set_wrap(&wrap)?; - } - - config.load_functions()?; - config - .load_mcp_servers(log_path, start_mcp_servers, abort_signal) - .await?; - - config.setup_model()?; - config.setup_document_loaders(); - config.setup_user_agent(); - Ok(()) - }; - let ret = setup(&mut config).await; - if !info_flag { - ret?; - } - Ok(config) - } - - #[allow(dead_code)] - pub fn sessions_dir(&self) -> PathBuf { - match &self.agent { - None => match env::var(get_env_name("sessions_dir")) { - Ok(value) => PathBuf::from(value), - Err(_) => paths::local_path(SESSIONS_DIR_NAME), - }, - Some(agent) => paths::agent_data_dir(agent.name()).join(SESSIONS_DIR_NAME), - } - } - - pub fn role_like_mut(&mut self) -> Option<&mut dyn RoleLike> { - if let Some(session) = self.session.as_mut() { - Some(session) - } else if let Some(agent) = self.agent.as_mut() { - Some(agent) - } else if let Some(role) = self.role.as_mut() { - Some(role) - } else { - None - } - } - - pub fn set_wrap(&mut self, value: &str) -> Result<()> { - if value == "no" { - self.wrap = None; - } else if value == "auto" { - self.wrap = Some(value.into()); - } else { - value - .parse::() - .map_err(|_| anyhow!("Invalid wrap value"))?; - self.wrap = Some(value.into()) - } - Ok(()) - } - - pub fn set_model(&mut self, model_id: &str) -> Result<()> { - let model = Model::retrieve_model(&self.to_app_config(), model_id, ModelType::Chat)?; - match self.role_like_mut() { - Some(role_like) => role_like.set_model(model), - None => { - self.model = model; - } - } - Ok(()) - } - - #[allow(dead_code)] - pub fn list_sessions(&self) -> Vec { - list_file_names(self.sessions_dir(), ".yaml") - } - pub async fn search_rag( app: &AppConfig, rag: &Rag, @@ -548,7 +324,11 @@ impl Config { Self::load_from_file(&config_path)? }; - let vault = Vault::init(&config.to_app_config()); + let bootstrap_app = AppConfig { + vault_password_file: config.vault_password_file.clone(), + ..AppConfig::default() + }; + let vault = Vault::init(&bootstrap_app); let (parsed_config, missing_secrets) = interpolate_secrets(&content, &vault); if !missing_secrets.is_empty() && !info_flag { debug!( @@ -573,7 +353,6 @@ impl Config { })?; config = new_config; } - config.vault = Arc::new(vault); Ok(config) } @@ -633,227 +412,6 @@ impl Config { serde_json::from_value(config).with_context(|| "Failed to load config from env")?; Ok(config) } - - fn load_envs(&mut self) { - if let Ok(v) = env::var(get_env_name("model")) { - self.model_id = v; - } - if let Some(v) = read_env_value::(&get_env_name("temperature")) { - self.temperature = v; - } - if let Some(v) = read_env_value::(&get_env_name("top_p")) { - self.top_p = v; - } - - if let Some(Some(v)) = read_env_bool(&get_env_name("dry_run")) { - self.dry_run = v; - } - if let Some(Some(v)) = read_env_bool(&get_env_name("stream")) { - self.stream = v; - } - if let Some(Some(v)) = read_env_bool(&get_env_name("save")) { - self.save = v; - } - if let Ok(v) = env::var(get_env_name("keybindings")) - && v == "vi" - { - self.keybindings = v; - } - if let Some(v) = read_env_value::(&get_env_name("editor")) { - self.editor = v; - } - if let Some(v) = read_env_value::(&get_env_name("wrap")) { - self.wrap = v; - } - if let Some(Some(v)) = read_env_bool(&get_env_name("wrap_code")) { - self.wrap_code = v; - } - - if let Some(Some(v)) = read_env_bool(&get_env_name("function_calling_support")) { - self.function_calling_support = v; - } - if let Ok(v) = env::var(get_env_name("mapping_tools")) - && let Ok(v) = serde_json::from_str(&v) - { - self.mapping_tools = v; - } - if let Some(v) = read_env_value::(&get_env_name("enabled_tools")) { - self.enabled_tools = v; - } - - if let Some(Some(v)) = read_env_bool(&get_env_name("mcp_server_support")) { - self.mcp_server_support = v; - } - if let Ok(v) = env::var(get_env_name("mapping_mcp_servers")) - && let Ok(v) = serde_json::from_str(&v) - { - self.mapping_mcp_servers = v; - } - if let Some(v) = read_env_value::(&get_env_name("enabled_mcp_servers")) { - self.enabled_mcp_servers = v; - } - - if let Some(v) = read_env_value::(&get_env_name("repl_prelude")) { - self.repl_prelude = v; - } - if let Some(v) = read_env_value::(&get_env_name("cmd_prelude")) { - self.cmd_prelude = v; - } - if let Some(v) = read_env_value::(&get_env_name("agent_session")) { - self.agent_session = v; - } - - if let Some(v) = read_env_bool(&get_env_name("save_session")) { - self.save_session = v; - } - if let Some(Some(v)) = read_env_value::(&get_env_name("compression_threshold")) { - self.compression_threshold = v; - } - if let Some(v) = read_env_value::(&get_env_name("summarization_prompt")) { - self.summarization_prompt = v; - } - if let Some(v) = read_env_value::(&get_env_name("summary_context_prompt")) { - self.summary_context_prompt = v; - } - - if let Some(v) = read_env_value::(&get_env_name("rag_embedding_model")) { - self.rag_embedding_model = v; - } - if let Some(v) = read_env_value::(&get_env_name("rag_reranker_model")) { - self.rag_reranker_model = v; - } - if let Some(Some(v)) = read_env_value::(&get_env_name("rag_top_k")) { - self.rag_top_k = v; - } - if let Some(v) = read_env_value::(&get_env_name("rag_chunk_size")) { - self.rag_chunk_size = v; - } - if let Some(v) = read_env_value::(&get_env_name("rag_chunk_overlap")) { - self.rag_chunk_overlap = v; - } - if let Some(v) = read_env_value::(&get_env_name("rag_template")) { - self.rag_template = v; - } - - if let Ok(v) = env::var(get_env_name("document_loaders")) - && let Ok(v) = serde_json::from_str(&v) - { - self.document_loaders = v; - } - - if let Some(Some(v)) = read_env_bool(&get_env_name("highlight")) { - self.highlight = v; - } - if *NO_COLOR { - self.highlight = false; - } - if self.highlight && self.theme.is_none() { - if let Some(v) = read_env_value::(&get_env_name("theme")) { - self.theme = v; - } else if *IS_STDOUT_TERMINAL - && let Ok(color_scheme) = color_scheme(QueryOptions::default()) - { - let theme = match color_scheme { - ColorScheme::Dark => "dark", - ColorScheme::Light => "light", - }; - self.theme = Some(theme.into()); - } - } - if let Some(v) = read_env_value::(&get_env_name("left_prompt")) { - self.left_prompt = v; - } - if let Some(v) = read_env_value::(&get_env_name("right_prompt")) { - self.right_prompt = v; - } - if let Some(v) = read_env_value::(&get_env_name("user_agent")) { - self.user_agent = v; - } - if let Some(Some(v)) = read_env_bool(&get_env_name("save_shell_history")) { - self.save_shell_history = v; - } - if let Some(v) = read_env_value::(&get_env_name("sync_models_url")) { - self.sync_models_url = v; - } - } - - fn load_functions(&mut self) -> Result<()> { - self.functions = Functions::init(self.visible_tools.as_ref().unwrap_or(&Vec::new()))?; - if self.working_mode.is_repl() { - self.functions.append_user_interaction_functions(); - } - Ok(()) - } - - async fn load_mcp_servers( - &mut self, - log_path: Option, - start_mcp_servers: bool, - abort_signal: AbortSignal, - ) -> Result<()> { - let app_config = self.to_app_config(); - let mcp_registry = McpRegistry::init( - log_path, - start_mcp_servers, - self.enabled_mcp_servers.clone(), - abort_signal.clone(), - &app_config, - &self.vault, - ) - .await?; - match mcp_registry.is_empty() { - false => { - if self.mcp_server_support { - self.functions - .append_mcp_meta_functions(mcp_registry.list_started_servers()); - } else { - debug!( - "Skipping global MCP functions registration since 'mcp_server_support' was 'false'" - ); - } - } - _ => debug!( - "Skipping global MCP functions registration since 'start_mcp_servers' was 'false'" - ), - } - self.mcp_registry = Some(mcp_registry); - - Ok(()) - } - - fn setup_model(&mut self) -> Result<()> { - let mut model_id = self.model_id.clone(); - if model_id.is_empty() { - let models = list_models(&self.to_app_config(), ModelType::Chat); - if models.is_empty() { - bail!("No available model"); - } - model_id = models[0].id() - } - self.set_model(&model_id)?; - self.model_id = model_id; - - Ok(()) - } - - fn setup_document_loaders(&mut self) { - [("pdf", "pdftotext $1 -"), ("docx", "pandoc --to plain $1")] - .into_iter() - .for_each(|(k, v)| { - let (k, v) = (k.to_string(), v.to_string()); - self.document_loaders.entry(k).or_insert(v); - }); - } - - fn setup_user_agent(&mut self) { - if let Some("auto") = self.user_agent.as_deref() { - self.user_agent = Some(format!( - "{}/{}", - env!("CARGO_CRATE_NAME"), - env!("CARGO_PKG_VERSION") - )); - } - } } pub fn load_env_file() -> Result<()> { @@ -1089,10 +647,6 @@ mod tests { assert!(cfg.save_shell_history); assert_eq!(cfg.keybindings, "emacs"); assert!(cfg.clients.is_empty()); - assert!(cfg.role.is_none()); - assert!(cfg.session.is_none()); - assert!(cfg.agent.is_none()); - assert!(cfg.rag.is_none()); assert!(cfg.save_session.is_none()); assert!(cfg.enabled_tools.is_none()); assert!(cfg.enabled_mcp_servers.is_none()); diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 130971b..f4dfce3 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -1,9 +1,11 @@ //! Per-request mutable state for a single Loki interaction. //! -//! `RequestContext` holds everything that was formerly the runtime -//! (`#[serde(skip)]`) half of [`Config`](super::Config): the active role, -//! session, agent, RAG, supervisor state, inbox/escalation queues, and -//! the conversation's "last message" cursor. +//! `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`: //! @@ -12,43 +14,11 @@ //! * **API** — one `RequestContext` per HTTP request, hydrated from a //! persisted session and written back at the end. //! -//! # Tool scope and agent runtime (planned) -//! -//! # Flat fields and sub-struct fields coexist during the bridge -//! -//! The flat fields (`functions`, `tool_call_tracker`, `supervisor`, -//! `inbox`, `root_escalation_queue`, `self_agent_id`, `current_depth`, -//! `parent_supervisor`) mirror the runtime half of today's `Config` -//! and are populated by -//! [`Config::to_request_context`](super::Config::to_request_context) -//! during the bridge. -//! -//! Step 6.5 added two **sub-struct fields** alongside the flat ones: -//! -//! * [`tool_scope: ToolScope`](super::tool_scope::ToolScope) — the -//! planned home for `functions`, `mcp_runtime`, and `tool_tracker` -//! * [`agent_runtime: Option`](super::agent_runtime::AgentRuntime) — -//! the planned home for `supervisor`, `inbox`, `escalation_queue`, -//! `todo_list`, `self_agent_id`, `parent_supervisor`, and `depth` -//! -//! During the bridge window the sub-struct fields are **additive -//! scaffolding**: they're initialized to defaults in -//! [`RequestContext::new`] and stay empty until Step 8 rewrites the -//! entry points to build them explicitly. The flat fields continue -//! to carry the actual state from `Config` during the bridge. -//! -//! # Phase 1 refactor history -//! -//! * **Step 0** — struct introduced alongside `Config` -//! * **Step 5** — added 13 request-read methods -//! * **Step 6** — added 12 request-write methods -//! * **Step 6.5** — added `tool_scope` and `agent_runtime` sub-struct -//! fields as additive scaffolding -//! * **Step 7** — added 14 mixed-method splits that take `&AppConfig` -//! as a parameter for the serialized half -//! -//! See `docs/PHASE-1-IMPLEMENTATION-PLAN.md` for the full migration -//! plan. +//! `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}; @@ -2396,7 +2366,7 @@ impl RequestContext { #[cfg(test)] mod tests { use super::*; - use crate::config::{AppState, Config}; + use crate::config::AppState; use crate::utils::get_env_name; use std::env; use std::fs::{create_dir_all, remove_dir_all, write}; @@ -2446,31 +2416,26 @@ mod tests { } } - fn app_state_from_config(cfg: &Config) -> Arc { + fn default_app_state() -> Arc { Arc::new(AppState { - config: Arc::new(cfg.to_app_config()), - vault: cfg.vault.clone(), + config: Arc::new(AppConfig::default()), + vault: Arc::new(crate::vault::Vault::default()), mcp_factory: Arc::new(super::super::mcp_factory::McpFactory::default()), rag_cache: Arc::new(RagCache::default()), mcp_config: None, mcp_log_path: None, mcp_registry: None, - functions: cfg.functions.clone(), + functions: Functions::default(), }) } fn create_test_ctx() -> RequestContext { - let cfg = Config::default(); - let app_state = app_state_from_config(&cfg); - cfg.to_request_context(app_state) + RequestContext::new(default_app_state(), WorkingMode::Cmd) } #[test] - fn to_request_context_creates_clean_state() { - let cfg = Config::default(); - let app_state = app_state_from_config(&cfg); - - let ctx = cfg.to_request_context(app_state); + fn new_creates_clean_state() { + let ctx = RequestContext::new(default_app_state(), WorkingMode::Cmd); assert!(ctx.role.is_none()); assert!(ctx.session.is_none()); @@ -2483,9 +2448,7 @@ mod tests { #[test] fn update_app_config_persists_changes() { - let cfg = Config::default(); - let app_state = app_state_from_config(&cfg); - let mut ctx = RequestContext::new(app_state, WorkingMode::Cmd); + let mut ctx = RequestContext::new(default_app_state(), WorkingMode::Cmd); let previous = Arc::clone(&ctx.app.config); ctx.update_app_config(|app| { diff --git a/src/config/session.rs b/src/config/session.rs index edd3e8e..42f19c8 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -674,7 +674,8 @@ impl AutoName { mod tests { use super::*; use crate::client::{Message, MessageContent, MessageRole, Model}; - use crate::config::{AppState, Config}; + use crate::config::{AppConfig, AppState, RequestContext, WorkingMode}; + use crate::function::Functions; use std::sync::Arc; #[test] @@ -688,19 +689,18 @@ mod tests { #[test] fn session_new_from_ctx_captures_save_session() { - let cfg = Config::default(); - let app_config = Arc::new(cfg.to_app_config()); + let app_config = Arc::new(AppConfig::default()); let app_state = Arc::new(AppState { config: app_config.clone(), - vault: cfg.vault.clone(), + vault: Arc::new(crate::vault::Vault::default()), mcp_factory: Arc::new(mcp_factory::McpFactory::default()), rag_cache: Arc::new(rag_cache::RagCache::default()), mcp_config: None, mcp_log_path: None, mcp_registry: None, - functions: cfg.functions.clone(), + functions: Functions::default(), }); - let ctx = cfg.to_request_context(app_state); + let ctx = RequestContext::new(app_state, WorkingMode::Cmd); let session = Session::new_from_ctx(&ctx, &app_config, "test-session"); assert_eq!(session.name(), "test-session"); diff --git a/src/config/tool_scope.rs b/src/config/tool_scope.rs index 9a9b06f..b499b23 100644 --- a/src/config/tool_scope.rs +++ b/src/config/tool_scope.rs @@ -12,19 +12,11 @@ //! * `tool_tracker` — per-scope tool call history for auto-continuation //! and looping detection //! -//! # Phase 1 Step 6.5 scope -//! -//! This file introduces the type scaffolding. Scope transitions -//! (`use_role`, `use_session`, `use_agent`, `exit_*`) that actually -//! build and swap `ToolScope` instances are deferred to Step 8 when -//! the entry points (`main.rs`, `repl/mod.rs`) get rewritten to thread -//! `RequestContext` through the pipeline. During the bridge window, -//! `Config.functions` / `Config.mcp_registry` keep serving today's -//! callers and `ToolScope` sits alongside them on `RequestContext` as -//! an unused (but compiling and tested) parallel structure. -//! -//! The fields mirror the plan in `docs/REST-API-ARCHITECTURE.md` -//! section 5 and `docs/PHASE-1-IMPLEMENTATION-PLAN.md` Step 6.5. +//! `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};