From 62bfaf20f44d3123152a85a60178caec1b9b1331 Mon Sep 17 00:00:00 2001 From: dave Date: Fri, 24 Apr 2026 17:07:44 +0000 Subject: [PATCH] huskies: merge 611_story_extract_settings_service --- server/src/http/mcp/agent_tools.rs | 4 +- server/src/http/settings.rs | 229 +++----------- server/src/service/mod.rs | 1 + server/src/service/settings/io.rs | 52 ++++ server/src/service/settings/mod.rs | 262 ++++++++++++++++ server/src/service/settings/project.rs | 396 ++++++++++++++++++++++++ server/src/service/settings/validate.rs | 149 +++++++++ 7 files changed, 897 insertions(+), 196 deletions(-) create mode 100644 server/src/service/settings/io.rs create mode 100644 server/src/service/settings/mod.rs create mode 100644 server/src/service/settings/project.rs create mode 100644 server/src/service/settings/validate.rs diff --git a/server/src/http/mcp/agent_tools.rs b/server/src/http/mcp/agent_tools.rs index 9315a9ae..8f6d0024 100644 --- a/server/src/http/mcp/agent_tools.rs +++ b/server/src/http/mcp/agent_tools.rs @@ -2,7 +2,7 @@ use crate::agents::PipelineStage; use crate::config::ProjectConfig; use crate::http::context::AppContext; -use crate::http::settings::get_editor_command_from_store; +use crate::service::settings::get_editor_command; use crate::slog_warn; use crate::worktree; use serde_json::{Value, json}; @@ -414,7 +414,7 @@ pub(super) fn tool_get_editor_command(args: &Value, ctx: &AppContext) -> Result< .and_then(|v| v.as_str()) .ok_or("Missing required argument: worktree_path")?; - let editor = get_editor_command_from_store(ctx) + let editor = get_editor_command(&*ctx.store) .ok_or_else(|| "No editor configured. Set one via PUT /api/settings/editor.".to_string())?; Ok(format!("{editor} {worktree_path}")) diff --git a/server/src/http/settings.rs b/server/src/http/settings.rs index 30c0d2cf..9c960f91 100644 --- a/server/src/http/settings.rs +++ b/server/src/http/settings.rs @@ -1,179 +1,38 @@ //! HTTP settings endpoints — REST API for user preferences and editor configuration. -use crate::config::ProjectConfig; use crate::http::context::{AppContext, OpenApiResult, bad_request}; +use crate::service::settings as svc; use crate::store::StoreOps; use poem_openapi::{Object, OpenApi, Tags, param::Query, payload::Json}; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use serde_json::json; +#[cfg(test)] use std::path::Path; use std::sync::Arc; -const EDITOR_COMMAND_KEY: &str = "editor_command"; +// Re-export service types so the test module (which does `use super::*`) can +// access them without modification. +pub use svc::EDITOR_COMMAND_KEY; +pub use svc::ProjectSettings; +#[cfg(test)] +pub use svc::settings_from_config; -/// Project-level settings exposed via `GET /api/settings` and `PUT /api/settings`. -/// -/// Only contains the scalar fields of `ProjectConfig` — array sections -/// (`[[component]]`, `[[agent]]`, `[watcher]`) are preserved in the TOML file -/// and are not editable through this API. -#[derive(Debug, Object, Serialize, Deserialize)] -struct ProjectSettings { - /// Project-wide default QA mode: "server", "agent", or "human". Default: "server". - default_qa: String, - /// Default model for coder-stage agents (e.g. "sonnet"). When set, only agents whose - /// model matches this value are used for auto-assignment. - default_coder_model: Option, - /// Maximum number of concurrent coder-stage agents. When set, stories wait in - /// 2_current/ until a slot is free. - max_coders: Option, - /// Maximum retries per story per pipeline stage before marking as blocked. Default: 2. - max_retries: u32, - /// Optional base branch name (e.g. "main", "master"). Overrides auto-detection. - base_branch: Option, - /// Whether to send RateLimitWarning chat notifications. Default: true. - rate_limit_notifications: bool, - /// IANA timezone name (e.g. "Europe/London"). Timer inputs are interpreted in this tz. - timezone: Option, - /// WebSocket URL of a remote huskies node to sync CRDT state with. - rendezvous: Option, - /// How often (seconds) to check 5_done/ for items to archive. Default: 60. - watcher_sweep_interval_secs: u64, - /// How long (seconds) an item must remain in 5_done/ before archiving. Default: 14400. - watcher_done_retention_secs: u64, -} - -/// Load `ProjectSettings` from `ProjectConfig`. -fn settings_from_config(cfg: &ProjectConfig) -> ProjectSettings { - ProjectSettings { - default_qa: cfg.default_qa.clone(), - default_coder_model: cfg.default_coder_model.clone(), - max_coders: cfg.max_coders.map(|v| v as u32), - max_retries: cfg.max_retries, - base_branch: cfg.base_branch.clone(), - rate_limit_notifications: cfg.rate_limit_notifications, - timezone: cfg.timezone.clone(), - rendezvous: cfg.rendezvous.clone(), - watcher_sweep_interval_secs: cfg.watcher.sweep_interval_secs, - watcher_done_retention_secs: cfg.watcher.done_retention_secs, - } -} - -/// Validate the incoming `ProjectSettings` before writing. +/// Thin wrapper — delegates to [`svc::validate_project_settings`] and maps +/// the typed error to `String` so existing tests calling `.unwrap_err()` can +/// call `.contains()` directly. fn validate_project_settings(s: &ProjectSettings) -> Result<(), String> { - match s.default_qa.as_str() { - "server" | "agent" | "human" => {} - other => { - return Err(format!( - "Invalid default_qa value '{other}'. Must be one of: server, agent, human" - )); - } - } - Ok(()) + svc::validate_project_settings(s).map_err(|e| e.to_string()) } -/// Write only the scalar settings from `s` into the project.toml at the given root. -/// Array sections (`[[component]]`, `[[agent]]`) are preserved unchanged. +/// Thin wrapper — delegates to [`svc::write_project_settings`] and maps the +/// typed error to `String` so existing tests can call `.unwrap()` unchanged. +#[cfg(test)] fn write_project_settings(project_root: &Path, s: &ProjectSettings) -> Result<(), String> { - let config_path = project_root.join(".huskies/project.toml"); + svc::write_project_settings(project_root, s).map_err(|e| e.to_string()) +} - let content = if config_path.exists() { - std::fs::read_to_string(&config_path).map_err(|e| format!("Read config: {e}"))? - } else { - String::new() - }; - - let mut val: toml::Value = if content.trim().is_empty() { - toml::Value::Table(toml::map::Map::new()) - } else { - toml::from_str(&content).map_err(|e| format!("Parse config: {e}"))? - }; - - let table = val - .as_table_mut() - .ok_or_else(|| "Config is not a TOML table".to_string())?; - - // Scalar root fields - table.insert( - "default_qa".to_string(), - toml::Value::String(s.default_qa.clone()), - ); - table.insert( - "max_retries".to_string(), - toml::Value::Integer(s.max_retries as i64), - ); - table.insert( - "rate_limit_notifications".to_string(), - toml::Value::Boolean(s.rate_limit_notifications), - ); - - // Optional scalar fields - match &s.default_coder_model { - Some(v) => { - table.insert( - "default_coder_model".to_string(), - toml::Value::String(v.clone()), - ); - } - None => { - table.remove("default_coder_model"); - } - } - match s.max_coders { - Some(v) => { - table.insert("max_coders".to_string(), toml::Value::Integer(v as i64)); - } - None => { - table.remove("max_coders"); - } - } - match &s.base_branch { - Some(v) => { - table.insert("base_branch".to_string(), toml::Value::String(v.clone())); - } - None => { - table.remove("base_branch"); - } - } - match &s.timezone { - Some(v) => { - table.insert("timezone".to_string(), toml::Value::String(v.clone())); - } - None => { - table.remove("timezone"); - } - } - match &s.rendezvous { - Some(v) => { - table.insert("rendezvous".to_string(), toml::Value::String(v.clone())); - } - None => { - table.remove("rendezvous"); - } - } - - // [watcher] sub-table - let watcher_entry = table - .entry("watcher".to_string()) - .or_insert_with(|| toml::Value::Table(toml::map::Map::new())); - if let toml::Value::Table(wt) = watcher_entry { - wt.insert( - "sweep_interval_secs".to_string(), - toml::Value::Integer(s.watcher_sweep_interval_secs as i64), - ); - wt.insert( - "done_retention_secs".to_string(), - toml::Value::Integer(s.watcher_done_retention_secs as i64), - ); - } - - // Ensure .huskies/ directory exists - if let Some(parent) = config_path.parent() { - std::fs::create_dir_all(parent).map_err(|e| format!("Create .huskies dir: {e}"))?; - } - - let new_content = toml::to_string_pretty(&val).map_err(|e| format!("Serialize config: {e}"))?; - std::fs::write(&config_path, new_content).map_err(|e| format!("Write config: {e}"))?; - - Ok(()) +/// Return the configured editor command from the store, or `None` if not set. +pub fn get_editor_command_from_store(ctx: &AppContext) -> Option { + svc::get_editor_command(&*ctx.store) } #[derive(Tags)] @@ -205,11 +64,7 @@ impl SettingsApi { /// Get the configured editor command (e.g. "zed", "code", "cursor"), or null if not set. #[oai(path = "/settings/editor", method = "get")] async fn get_editor(&self) -> OpenApiResult> { - let editor_command = self - .ctx - .store - .get(EDITOR_COMMAND_KEY) - .and_then(|v| v.as_str().map(|s| s.to_string())); + let editor_command = get_editor_command_from_store(&self.ctx); Ok(Json(EditorCommandResponse { editor_command })) } @@ -223,19 +78,8 @@ impl SettingsApi { path: Query, line: Query>, ) -> OpenApiResult> { - let editor_command = get_editor_command_from_store(&self.ctx) - .ok_or_else(|| bad_request("No editor configured".to_string()))?; - - let file_ref = match line.0 { - Some(l) => format!("{}:{}", path.0, l), - None => path.0.clone(), - }; - - std::process::Command::new(&editor_command) - .arg(&file_ref) - .spawn() - .map_err(|e| bad_request(format!("Failed to open editor: {e}")))?; - + svc::open_file_in_editor(&*self.ctx.store, &path.0, line.0) + .map_err(|e| bad_request(e.to_string()))?; Ok(Json(OpenFileResponse { success: true })) } @@ -243,8 +87,9 @@ impl SettingsApi { #[oai(path = "/settings", method = "get")] async fn get_settings(&self) -> OpenApiResult> { let project_root = self.ctx.state.get_project_root().map_err(bad_request)?; - let config = ProjectConfig::load(&project_root).map_err(bad_request)?; - Ok(Json(settings_from_config(&config))) + let s = + svc::load_project_settings(&project_root).map_err(|e| bad_request(e.to_string()))?; + Ok(Json(s)) } /// Update project.toml scalar settings. Array sections (component, agent) are preserved. @@ -257,10 +102,12 @@ impl SettingsApi { ) -> OpenApiResult> { validate_project_settings(&payload.0).map_err(bad_request)?; let project_root = self.ctx.state.get_project_root().map_err(bad_request)?; - write_project_settings(&project_root, &payload.0).map_err(bad_request)?; + svc::write_project_settings(&project_root, &payload.0) + .map_err(|e| bad_request(e.to_string()))?; // Re-read to confirm what was written - let config = ProjectConfig::load(&project_root).map_err(bad_request)?; - Ok(Json(settings_from_config(&config))) + let s = + svc::load_project_settings(&project_root).map_err(|e| bad_request(e.to_string()))?; + Ok(Json(s)) } /// Set the preferred editor command (e.g. "zed", "code", "cursor"). @@ -294,12 +141,6 @@ impl SettingsApi { } } -pub fn get_editor_command_from_store(ctx: &AppContext) -> Option { - ctx.store - .get(EDITOR_COMMAND_KEY) - .and_then(|v| v.as_str().map(|s| s.to_string())) -} - #[cfg(test)] impl From> for SettingsApi { fn from(ctx: std::sync::Arc) -> Self { @@ -556,7 +397,7 @@ mod tests { // ── /api/settings GET/PUT ────────────────────────────────────────────── fn default_project_settings() -> ProjectSettings { - let cfg = ProjectConfig::default(); + let cfg = crate::config::ProjectConfig::default(); settings_from_config(&cfg) } @@ -709,7 +550,7 @@ path = "." write_project_settings(dir.path(), &s).unwrap(); - let config = ProjectConfig::load(dir.path()).unwrap(); + let config = crate::config::ProjectConfig::load(dir.path()).unwrap(); let loaded = settings_from_config(&config); assert_eq!(loaded.default_qa, "agent"); @@ -763,7 +604,7 @@ path = "." }; write_project_settings(dir.path(), &s_clear).unwrap(); - let config = ProjectConfig::load(dir.path()).unwrap(); + let config = crate::config::ProjectConfig::load(dir.path()).unwrap(); let loaded = settings_from_config(&config); assert!(loaded.default_coder_model.is_none()); assert!(loaded.max_coders.is_none()); diff --git a/server/src/service/mod.rs b/server/src/service/mod.rs index 978559a6..79f80f48 100644 --- a/server/src/service/mod.rs +++ b/server/src/service/mod.rs @@ -13,5 +13,6 @@ pub mod file_io; pub mod health; pub mod oauth; pub mod project; +pub mod settings; pub mod wizard; pub mod ws; diff --git a/server/src/service/settings/io.rs b/server/src/service/settings/io.rs new file mode 100644 index 00000000..3b5b3e98 --- /dev/null +++ b/server/src/service/settings/io.rs @@ -0,0 +1,52 @@ +//! Settings I/O — the ONLY place in `service/settings/` that may perform side effects. +//! +//! Side effects here include: reading/writing `.huskies/project.toml` and +//! spawning the editor process via `std::process::Command`. +//! All business logic, branching, and type definitions belong in `mod.rs`, +//! `project.rs`, or `validate.rs`. + +use super::Error; +use std::path::Path; + +/// Read the raw TOML content from `config_path`. +/// +/// Returns an empty string if the file does not exist yet, so callers can +/// treat a missing config the same as an empty one. +/// +/// # Errors +/// - [`Error::Io`] if the file exists but cannot be read. +pub(super) fn read_config_toml(config_path: &Path) -> Result { + if config_path.exists() { + std::fs::read_to_string(config_path).map_err(|e| Error::Io(format!("Read config: {e}"))) + } else { + Ok(String::new()) + } +} + +/// Write `content` to `config_path`, creating parent directories as needed. +/// +/// # Errors +/// - [`Error::Io`] if the directory cannot be created or the file write fails. +pub(super) fn write_config_toml(config_path: &Path, content: &str) -> Result<(), Error> { + if let Some(parent) = config_path.parent() { + std::fs::create_dir_all(parent) + .map_err(|e| Error::Io(format!("Create .huskies dir: {e}")))?; + } + std::fs::write(config_path, content).map_err(|e| Error::Io(format!("Write config: {e}"))) +} + +/// Spawn the editor process with `file_ref` as the sole argument. +/// +/// Does not wait for the editor to exit — fire-and-forget so the UI remains +/// responsive. +/// +/// # Errors +/// - [`Error::SpawnError`] if the operating system cannot start the process +/// (e.g. the editor binary is not on `$PATH`). +pub(super) fn spawn_editor(editor_command: &str, file_ref: &str) -> Result<(), Error> { + std::process::Command::new(editor_command) + .arg(file_ref) + .spawn() + .map(|_| ()) + .map_err(|e| Error::Spawn(format!("Failed to open editor: {e}"))) +} diff --git a/server/src/service/settings/mod.rs b/server/src/service/settings/mod.rs new file mode 100644 index 00000000..123c86b3 --- /dev/null +++ b/server/src/service/settings/mod.rs @@ -0,0 +1,262 @@ +//! Settings service — domain logic for project settings and editor configuration. +//! +//! Extracts business logic from `http/settings.rs` following the conventions in +//! `docs/architecture/service-modules.md`: +//! - `mod.rs` (this file) — public API, typed [`Error`], orchestration +//! - `io.rs` — the ONLY place that performs side effects (filesystem I/O, process spawn) +//! - `project.rs` — pure types: [`ProjectSettings`], [`settings_from_config`], +//! [`merge_settings_into_toml`] +//! - `validate.rs` — pure validation: [`validate_project_settings`] + +pub(super) mod io; +pub mod project; +pub mod validate; + +pub use project::{ProjectSettings, merge_settings_into_toml, settings_from_config}; +pub use validate::validate_project_settings; + +use crate::config::ProjectConfig; +use crate::store::StoreOps; +use std::path::Path; + +/// The store key for the configured editor command. +pub const EDITOR_COMMAND_KEY: &str = "editor_command"; + +// ── Error type ──────────────────────────────────────────────────────────────── + +/// Typed errors returned by `service::settings` functions. +/// +/// HTTP handlers map these to status codes: +/// - [`Error::Validation`] → 400 Bad Request +/// - [`Error::NotConfigured`] → 400 Bad Request +/// - [`Error::Io`] → 500 Internal Server Error +/// - [`Error::Spawn`] → 500 Internal Server Error +#[derive(Debug)] +pub enum Error { + /// A field value failed validation (e.g. unknown QA mode). + Validation(String), + /// No editor is configured in the store. + NotConfigured, + /// A filesystem read or write operation failed. + Io(String), + /// The editor process failed to spawn. + Spawn(String), +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Validation(msg) => write!(f, "Validation error: {msg}"), + Self::NotConfigured => write!(f, "No editor configured"), + Self::Io(msg) => write!(f, "I/O error: {msg}"), + Self::Spawn(msg) => write!(f, "Spawn error: {msg}"), + } + } +} + +// ── Public API ──────────────────────────────────────────────────────────────── + +/// Load the current project settings from disk. +/// +/// # Errors +/// - [`Error::IoError`] if the config file cannot be read or parsed. +pub fn load_project_settings(project_root: &Path) -> Result { + let config = + ProjectConfig::load(project_root).map_err(|e| Error::Io(format!("Load config: {e}")))?; + Ok(settings_from_config(&config)) +} + +/// Write the given settings to disk, preserving array sections. +/// +/// Reads the existing project.toml, merges only the scalar fields from `s`, +/// and rewrites the file. Array sections (`[[component]]`, `[[agent]]`) are +/// untouched. +/// +/// # Errors +/// - [`Error::IoError`] if the config file cannot be read, parsed, or written. +pub fn write_project_settings(project_root: &Path, s: &ProjectSettings) -> Result<(), Error> { + let config_path = project_root.join(".huskies/project.toml"); + + let content = io::read_config_toml(&config_path)?; + + let mut val: toml::Value = if content.trim().is_empty() { + toml::Value::Table(toml::map::Map::new()) + } else { + toml::from_str(&content).map_err(|e| Error::Io(format!("Parse config: {e}")))? + }; + + merge_settings_into_toml(&mut val, s)?; + + let new_content = + toml::to_string_pretty(&val).map_err(|e| Error::Io(format!("Serialize config: {e}")))?; + io::write_config_toml(&config_path, &new_content)?; + + Ok(()) +} + +/// Return the configured editor command from the store, or `None` if not set. +/// +/// Pure: reads from in-memory store only — no filesystem or network I/O. +pub fn get_editor_command(store: &dyn StoreOps) -> Option { + store + .get(EDITOR_COMMAND_KEY) + .and_then(|v| v.as_str().map(|s| s.to_string())) +} + +/// Open a file in the configured editor at the optional line number. +/// +/// # Errors +/// - [`Error::NotConfigured`] if no editor has been set in the store. +/// - [`Error::SpawnError`] if the editor process fails to start. +pub fn open_file_in_editor( + store: &dyn StoreOps, + path: &str, + line: Option, +) -> Result<(), Error> { + let editor_command = get_editor_command(store).ok_or(Error::NotConfigured)?; + + let file_ref = match line { + Some(l) => format!("{path}:{l}"), + None => path.to_string(), + }; + + io::spawn_editor(&editor_command, &file_ref) +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + // ── Error Display ───────────────────────────────────────────────────────── + + #[test] + fn error_display_validation() { + let e = Error::Validation("bad value".to_string()); + assert!(e.to_string().contains("Validation error")); + assert!(e.to_string().contains("bad value")); + } + + #[test] + fn error_display_not_configured() { + let e = Error::NotConfigured; + assert!(e.to_string().contains("No editor configured")); + } + + #[test] + fn error_display_io() { + let e = Error::Io("disk full".to_string()); + assert!(e.to_string().contains("I/O error")); + assert!(e.to_string().contains("disk full")); + } + + #[test] + fn error_display_spawn() { + let e = Error::Spawn("not found".to_string()); + assert!(e.to_string().contains("Spawn error")); + assert!(e.to_string().contains("not found")); + } + + // ── load_project_settings ───────────────────────────────────────────────── + + #[test] + fn load_project_settings_returns_defaults_when_no_toml() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join(".huskies")).unwrap(); + let s = load_project_settings(dir.path()).unwrap(); + assert_eq!(s.default_qa, "server"); + assert_eq!(s.max_retries, 2); + assert!(s.rate_limit_notifications); + } + + // ── write_project_settings ──────────────────────────────────────────────── + + #[test] + fn write_project_settings_creates_file() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join(".huskies")).unwrap(); + let s = load_project_settings(dir.path()).unwrap(); + write_project_settings(dir.path(), &s).unwrap(); + assert!(dir.path().join(".huskies/project.toml").exists()); + } + + #[test] + fn write_project_settings_roundtrips() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join(".huskies")).unwrap(); + let mut s = load_project_settings(dir.path()).unwrap(); + s.default_qa = "agent".to_string(); + s.max_retries = 7; + write_project_settings(dir.path(), &s).unwrap(); + + let loaded = load_project_settings(dir.path()).unwrap(); + assert_eq!(loaded.default_qa, "agent"); + assert_eq!(loaded.max_retries, 7); + } + + // ── get_editor_command ──────────────────────────────────────────────────── + + #[test] + fn get_editor_command_returns_none_when_unset() { + use crate::store::JsonFileStore; + let dir = TempDir::new().unwrap(); + let store = JsonFileStore::new(dir.path().join("store.json")).unwrap(); + assert!(get_editor_command(&store).is_none()); + } + + #[test] + fn get_editor_command_returns_value_when_set() { + use crate::store::JsonFileStore; + use serde_json::json; + let dir = TempDir::new().unwrap(); + let store = JsonFileStore::new(dir.path().join("store.json")).unwrap(); + store.set(EDITOR_COMMAND_KEY, json!("zed")); + assert_eq!(get_editor_command(&store), Some("zed".to_string())); + } + + // ── open_file_in_editor ─────────────────────────────────────────────────── + + #[test] + fn open_file_in_editor_returns_not_configured_when_no_editor() { + use crate::store::JsonFileStore; + let dir = TempDir::new().unwrap(); + let store = JsonFileStore::new(dir.path().join("store.json")).unwrap(); + let result = open_file_in_editor(&store, "src/main.rs", Some(42)); + assert!(matches!(result, Err(Error::NotConfigured))); + } + + #[test] + fn open_file_in_editor_returns_spawn_error_for_nonexistent_editor() { + use crate::store::JsonFileStore; + use serde_json::json; + let dir = TempDir::new().unwrap(); + let store = JsonFileStore::new(dir.path().join("store.json")).unwrap(); + store.set(EDITOR_COMMAND_KEY, json!("this_editor_xyz_does_not_exist")); + let result = open_file_in_editor(&store, "src/main.rs", Some(1)); + assert!(matches!(result, Err(Error::Spawn(_)))); + } + + #[test] + fn open_file_in_editor_succeeds_with_echo() { + use crate::store::JsonFileStore; + use serde_json::json; + let dir = TempDir::new().unwrap(); + let store = JsonFileStore::new(dir.path().join("store.json")).unwrap(); + store.set(EDITOR_COMMAND_KEY, json!("echo")); + let result = open_file_in_editor(&store, "src/main.rs", Some(10)); + assert!(result.is_ok()); + } + + #[test] + fn open_file_in_editor_formats_path_without_line() { + use crate::store::JsonFileStore; + use serde_json::json; + let dir = TempDir::new().unwrap(); + let store = JsonFileStore::new(dir.path().join("store.json")).unwrap(); + store.set(EDITOR_COMMAND_KEY, json!("echo")); + let result = open_file_in_editor(&store, "src/lib.rs", None); + assert!(result.is_ok()); + } +} diff --git a/server/src/service/settings/project.rs b/server/src/service/settings/project.rs new file mode 100644 index 00000000..d19ada7b --- /dev/null +++ b/server/src/service/settings/project.rs @@ -0,0 +1,396 @@ +//! Pure settings types and TOML merge logic — no side effects. +//! +//! Owns [`ProjectSettings`] (the API-facing settings payload), +//! [`settings_from_config`] (conversion from `ProjectConfig`), and +//! [`merge_settings_into_toml`] (the pure TOML key-updating logic used by the +//! write path in `mod.rs` + `io.rs`). + +use crate::config::ProjectConfig; +use poem_openapi::Object; +use serde::{Deserialize, Serialize}; + +/// Project-level settings exposed via `GET /api/settings` and `PUT /api/settings`. +/// +/// Only contains the scalar fields of `ProjectConfig` — array sections +/// (`[[component]]`, `[[agent]]`, `[watcher]`) are preserved in the TOML file +/// and are not editable through this API. +#[derive(Debug, Object, Serialize, Deserialize)] +pub struct ProjectSettings { + /// Project-wide default QA mode: "server", "agent", or "human". Default: "server". + pub default_qa: String, + /// Default model for coder-stage agents (e.g. "sonnet"). + pub default_coder_model: Option, + /// Maximum number of concurrent coder-stage agents. + pub max_coders: Option, + /// Maximum retries per story per pipeline stage before marking as blocked. Default: 2. + pub max_retries: u32, + /// Optional base branch name (e.g. "main", "master"). + pub base_branch: Option, + /// Whether to send RateLimitWarning chat notifications. Default: true. + pub rate_limit_notifications: bool, + /// IANA timezone name (e.g. "Europe/London"). + pub timezone: Option, + /// WebSocket URL of a remote huskies node to sync CRDT state with. + pub rendezvous: Option, + /// How often (seconds) to check 5_done/ for items to archive. Default: 60. + pub watcher_sweep_interval_secs: u64, + /// How long (seconds) an item must remain in 5_done/ before archiving. Default: 14400. + pub watcher_done_retention_secs: u64, +} + +/// Convert a [`ProjectConfig`] into a [`ProjectSettings`] payload. +/// +/// Pure: performs no I/O. +pub fn settings_from_config(cfg: &ProjectConfig) -> ProjectSettings { + ProjectSettings { + default_qa: cfg.default_qa.clone(), + default_coder_model: cfg.default_coder_model.clone(), + max_coders: cfg.max_coders.map(|v| v as u32), + max_retries: cfg.max_retries, + base_branch: cfg.base_branch.clone(), + rate_limit_notifications: cfg.rate_limit_notifications, + timezone: cfg.timezone.clone(), + rendezvous: cfg.rendezvous.clone(), + watcher_sweep_interval_secs: cfg.watcher.sweep_interval_secs, + watcher_done_retention_secs: cfg.watcher.done_retention_secs, + } +} + +/// Merge the scalar settings from `s` into an existing TOML value in-place. +/// +/// Array sections (`[[component]]`, `[[agent]]`) and unknown keys are preserved. +/// Pure: performs no I/O. +/// +/// # Errors +/// - [`super::Error::IoError`] if `val` is not a TOML table. +pub fn merge_settings_into_toml( + val: &mut toml::Value, + s: &ProjectSettings, +) -> Result<(), super::Error> { + let table = val + .as_table_mut() + .ok_or_else(|| super::Error::Io("Config is not a TOML table".to_string()))?; + + // Scalar root fields — always written + table.insert( + "default_qa".to_string(), + toml::Value::String(s.default_qa.clone()), + ); + table.insert( + "max_retries".to_string(), + toml::Value::Integer(s.max_retries as i64), + ); + table.insert( + "rate_limit_notifications".to_string(), + toml::Value::Boolean(s.rate_limit_notifications), + ); + + // Optional scalar fields — insert when Some, remove when None + match &s.default_coder_model { + Some(v) => { + table.insert( + "default_coder_model".to_string(), + toml::Value::String(v.clone()), + ); + } + None => { + table.remove("default_coder_model"); + } + } + match s.max_coders { + Some(v) => { + table.insert("max_coders".to_string(), toml::Value::Integer(v as i64)); + } + None => { + table.remove("max_coders"); + } + } + match &s.base_branch { + Some(v) => { + table.insert("base_branch".to_string(), toml::Value::String(v.clone())); + } + None => { + table.remove("base_branch"); + } + } + match &s.timezone { + Some(v) => { + table.insert("timezone".to_string(), toml::Value::String(v.clone())); + } + None => { + table.remove("timezone"); + } + } + match &s.rendezvous { + Some(v) => { + table.insert("rendezvous".to_string(), toml::Value::String(v.clone())); + } + None => { + table.remove("rendezvous"); + } + } + + // [watcher] sub-table + let watcher_entry = table + .entry("watcher".to_string()) + .or_insert_with(|| toml::Value::Table(toml::map::Map::new())); + if let toml::Value::Table(wt) = watcher_entry { + wt.insert( + "sweep_interval_secs".to_string(), + toml::Value::Integer(s.watcher_sweep_interval_secs as i64), + ); + wt.insert( + "done_retention_secs".to_string(), + toml::Value::Integer(s.watcher_done_retention_secs as i64), + ); + } + + Ok(()) +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + fn default_settings() -> ProjectSettings { + settings_from_config(&ProjectConfig::default()) + } + + fn empty_toml() -> toml::Value { + toml::Value::Table(toml::map::Map::new()) + } + + // ── settings_from_config ────────────────────────────────────────────────── + + #[test] + fn settings_from_config_reflects_defaults() { + let s = default_settings(); + assert_eq!(s.default_qa, "server"); + assert_eq!(s.max_retries, 2); + assert!(s.rate_limit_notifications); + assert!(s.default_coder_model.is_none()); + assert!(s.max_coders.is_none()); + assert!(s.base_branch.is_none()); + assert!(s.timezone.is_none()); + assert!(s.rendezvous.is_none()); + } + + #[test] + fn settings_from_config_copies_all_scalar_fields() { + let cfg = ProjectConfig { + default_qa: "human".to_string(), + default_coder_model: Some("opus".to_string()), + max_coders: Some(4), + max_retries: 5, + base_branch: Some("main".to_string()), + rate_limit_notifications: false, + timezone: Some("UTC".to_string()), + rendezvous: Some("ws://host:3001/crdt-sync".to_string()), + watcher: crate::config::WatcherConfig { + sweep_interval_secs: 30, + done_retention_secs: 7200, + }, + ..Default::default() + }; + + let s = settings_from_config(&cfg); + assert_eq!(s.default_qa, "human"); + assert_eq!(s.default_coder_model, Some("opus".to_string())); + assert_eq!(s.max_coders, Some(4)); + assert_eq!(s.max_retries, 5); + assert_eq!(s.base_branch, Some("main".to_string())); + assert!(!s.rate_limit_notifications); + assert_eq!(s.timezone, Some("UTC".to_string())); + assert_eq!(s.rendezvous, Some("ws://host:3001/crdt-sync".to_string())); + assert_eq!(s.watcher_sweep_interval_secs, 30); + assert_eq!(s.watcher_done_retention_secs, 7200); + } + + #[test] + fn settings_from_config_max_coders_usize_to_u32() { + let cfg = ProjectConfig { + max_coders: Some(3usize), + ..Default::default() + }; + let s = settings_from_config(&cfg); + assert_eq!(s.max_coders, Some(3u32)); + } + + // ── merge_settings_into_toml ────────────────────────────────────────────── + + #[test] + fn merge_writes_scalar_root_fields() { + let mut val = empty_toml(); + let s = ProjectSettings { + default_qa: "agent".to_string(), + default_coder_model: None, + max_coders: None, + max_retries: 3, + base_branch: None, + rate_limit_notifications: false, + timezone: None, + rendezvous: None, + watcher_sweep_interval_secs: 60, + watcher_done_retention_secs: 14400, + }; + merge_settings_into_toml(&mut val, &s).unwrap(); + + let t = val.as_table().unwrap(); + assert_eq!(t["default_qa"].as_str(), Some("agent")); + assert_eq!(t["max_retries"].as_integer(), Some(3)); + assert_eq!(t["rate_limit_notifications"].as_bool(), Some(false)); + } + + #[test] + fn merge_inserts_optional_fields_when_some() { + let mut val = empty_toml(); + let s = ProjectSettings { + default_qa: "server".to_string(), + default_coder_model: Some("sonnet".to_string()), + max_coders: Some(2), + max_retries: 2, + base_branch: Some("main".to_string()), + rate_limit_notifications: true, + timezone: Some("America/New_York".to_string()), + rendezvous: Some("ws://host/crdt-sync".to_string()), + watcher_sweep_interval_secs: 60, + watcher_done_retention_secs: 14400, + }; + merge_settings_into_toml(&mut val, &s).unwrap(); + + let t = val.as_table().unwrap(); + assert_eq!(t["default_coder_model"].as_str(), Some("sonnet")); + assert_eq!(t["max_coders"].as_integer(), Some(2)); + assert_eq!(t["base_branch"].as_str(), Some("main")); + assert_eq!(t["timezone"].as_str(), Some("America/New_York")); + assert_eq!(t["rendezvous"].as_str(), Some("ws://host/crdt-sync")); + } + + #[test] + fn merge_removes_optional_fields_when_none() { + let mut val = empty_toml(); + // First set them + let s_with = ProjectSettings { + default_qa: "server".to_string(), + default_coder_model: Some("sonnet".to_string()), + max_coders: Some(3), + max_retries: 2, + base_branch: Some("master".to_string()), + rate_limit_notifications: true, + timezone: Some("UTC".to_string()), + rendezvous: None, + watcher_sweep_interval_secs: 60, + watcher_done_retention_secs: 14400, + }; + merge_settings_into_toml(&mut val, &s_with).unwrap(); + + // Then clear them + let s_clear = ProjectSettings { + default_qa: "server".to_string(), + default_coder_model: None, + max_coders: None, + max_retries: 2, + base_branch: None, + rate_limit_notifications: true, + timezone: None, + rendezvous: None, + watcher_sweep_interval_secs: 60, + watcher_done_retention_secs: 14400, + }; + merge_settings_into_toml(&mut val, &s_clear).unwrap(); + + let t = val.as_table().unwrap(); + assert!(!t.contains_key("default_coder_model")); + assert!(!t.contains_key("max_coders")); + assert!(!t.contains_key("base_branch")); + assert!(!t.contains_key("timezone")); + } + + #[test] + fn merge_writes_watcher_sub_table() { + let mut val = empty_toml(); + let s = ProjectSettings { + default_qa: "server".to_string(), + default_coder_model: None, + max_coders: None, + max_retries: 2, + base_branch: None, + rate_limit_notifications: true, + timezone: None, + rendezvous: None, + watcher_sweep_interval_secs: 45, + watcher_done_retention_secs: 3600, + }; + merge_settings_into_toml(&mut val, &s).unwrap(); + + let t = val.as_table().unwrap(); + let wt = t["watcher"].as_table().unwrap(); + assert_eq!(wt["sweep_interval_secs"].as_integer(), Some(45)); + assert_eq!(wt["done_retention_secs"].as_integer(), Some(3600)); + } + + #[test] + fn merge_preserves_unknown_toml_keys() { + let existing_toml = r#" +[[agent]] +name = "coder-1" +model = "sonnet" +stage = "coder" + +[[component]] +name = "server" +path = "." +"#; + let mut val: toml::Value = toml::from_str(existing_toml).unwrap(); + let s = default_settings(); + merge_settings_into_toml(&mut val, &s).unwrap(); + + // Re-serialize and verify agent/component sections are preserved + let output = toml::to_string_pretty(&val).unwrap(); + assert!( + output.contains("coder-1"), + "agent section should be preserved" + ); + assert!( + output.contains("component"), + "component section should be preserved" + ); + } + + #[test] + fn merge_returns_error_for_non_table_toml() { + let mut val = toml::Value::String("not a table".to_string()); + let s = default_settings(); + let result = merge_settings_into_toml(&mut val, &s); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), super::super::Error::Io(_))); + } + + #[test] + fn merge_formatting_produces_valid_toml() { + let mut val = empty_toml(); + let s = ProjectSettings { + default_qa: "human".to_string(), + default_coder_model: Some("opus".to_string()), + max_coders: Some(2), + max_retries: 4, + base_branch: Some("develop".to_string()), + rate_limit_notifications: false, + timezone: Some("Europe/London".to_string()), + rendezvous: Some("ws://remote:3001/crdt-sync".to_string()), + watcher_sweep_interval_secs: 120, + watcher_done_retention_secs: 28800, + }; + merge_settings_into_toml(&mut val, &s).unwrap(); + + let output = toml::to_string_pretty(&val).unwrap(); + // Verify round-trip: the output must be valid TOML + let reparsed: toml::Value = toml::from_str(&output).unwrap(); + let t = reparsed.as_table().unwrap(); + assert_eq!(t["default_qa"].as_str(), Some("human")); + assert_eq!(t["default_coder_model"].as_str(), Some("opus")); + assert_eq!(t["max_coders"].as_integer(), Some(2)); + } +} diff --git a/server/src/service/settings/validate.rs b/server/src/service/settings/validate.rs new file mode 100644 index 00000000..4620c7c9 --- /dev/null +++ b/server/src/service/settings/validate.rs @@ -0,0 +1,149 @@ +//! Pure validation logic for project settings — no side effects. +//! +//! All functions in this module are pure: given the same input, they always +//! return the same output, and they never perform any I/O. + +use super::{Error, project::ProjectSettings}; + +/// Validate the incoming [`ProjectSettings`] before writing to disk. +/// +/// # Errors +/// - [`Error::Validation`] if any field value is invalid. +pub fn validate_project_settings(s: &ProjectSettings) -> Result<(), Error> { + match s.default_qa.as_str() { + "server" | "agent" | "human" => {} + other => { + return Err(Error::Validation(format!( + "Invalid default_qa value '{other}'. Must be one of: server, agent, human" + ))); + } + } + Ok(()) +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::ProjectConfig; + use crate::service::settings::project::settings_from_config; + + fn make_settings(default_qa: &str) -> ProjectSettings { + let cfg = ProjectConfig { + default_qa: default_qa.to_string(), + ..Default::default() + }; + settings_from_config(&cfg) + } + + // ── Valid cases ─────────────────────────────────────────────────────────── + + #[test] + fn accepts_server_qa_mode() { + assert!(validate_project_settings(&make_settings("server")).is_ok()); + } + + #[test] + fn accepts_agent_qa_mode() { + assert!(validate_project_settings(&make_settings("agent")).is_ok()); + } + + #[test] + fn accepts_human_qa_mode() { + assert!(validate_project_settings(&make_settings("human")).is_ok()); + } + + #[test] + fn accepts_all_qa_modes() { + for mode in &["server", "agent", "human"] { + let result = validate_project_settings(&make_settings(mode)); + assert!(result.is_ok(), "qa mode '{mode}' should be valid"); + } + } + + // ── Invalid cases ───────────────────────────────────────────────────────── + + #[test] + fn rejects_empty_qa_mode() { + let err = validate_project_settings(&make_settings("")).unwrap_err(); + assert!(matches!(err, Error::Validation(_))); + } + + #[test] + fn rejects_unknown_qa_mode() { + let err = validate_project_settings(&make_settings("robot")).unwrap_err(); + assert!(matches!(err, Error::Validation(ref msg) if msg.contains("robot"))); + } + + #[test] + fn rejects_uppercase_qa_mode() { + let err = validate_project_settings(&make_settings("Server")).unwrap_err(); + assert!(matches!(err, Error::Validation(_))); + } + + #[test] + fn rejects_partial_qa_mode() { + let err = validate_project_settings(&make_settings("serv")).unwrap_err(); + assert!(matches!(err, Error::Validation(_))); + } + + #[test] + fn rejects_qa_mode_with_trailing_space() { + let err = validate_project_settings(&make_settings("server ")).unwrap_err(); + assert!(matches!(err, Error::Validation(_))); + } + + #[test] + fn error_message_contains_invalid_value() { + let err = validate_project_settings(&make_settings("bad_mode")).unwrap_err(); + if let Error::Validation(msg) = err { + assert!( + msg.contains("bad_mode"), + "error message should include the bad value" + ); + assert!( + msg.contains("server") && msg.contains("agent") && msg.contains("human"), + "error message should list valid values" + ); + } else { + panic!("expected ValidationError"); + } + } + + // ── Settings with other fields set ─────────────────────────────────────── + + #[test] + fn valid_settings_with_all_optional_fields_set() { + let s = ProjectSettings { + default_qa: "agent".to_string(), + default_coder_model: Some("opus".to_string()), + max_coders: Some(4), + max_retries: 5, + base_branch: Some("main".to_string()), + rate_limit_notifications: false, + timezone: Some("UTC".to_string()), + rendezvous: Some("ws://host:3001/crdt-sync".to_string()), + watcher_sweep_interval_secs: 30, + watcher_done_retention_secs: 3600, + }; + assert!(validate_project_settings(&s).is_ok()); + } + + #[test] + fn valid_settings_with_no_optional_fields() { + let s = ProjectSettings { + default_qa: "human".to_string(), + default_coder_model: None, + max_coders: None, + max_retries: 2, + base_branch: None, + rate_limit_notifications: true, + timezone: None, + rendezvous: None, + watcher_sweep_interval_secs: 60, + watcher_done_retention_secs: 14400, + }; + assert!(validate_project_settings(&s).is_ok()); + } +}