huskies: merge 611_story_extract_settings_service

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}"))

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`.
+/// Thin wrapper — delegates to [`svc::validate_project_settings`] and maps
-///
+/// the typed error to `String` so existing tests calling `.unwrap_err()` can
-/// Only contains the scalar fields of `ProjectConfig` — array sections
+/// call `.contains()` directly.
-/// (`[[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<String>,
-    /// Maximum number of concurrent coder-stage agents. When set, stories wait in
-    /// 2_current/ until a slot is free.
-    max_coders: Option<u32>,
-    /// 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<String>,
-    /// 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<String>,
-    /// WebSocket URL of a remote huskies node to sync CRDT state with.
-    rendezvous: Option<String>,
-    /// 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.
 fn validate_project_settings(s: &ProjectSettings) -> Result<(), String> {
-    match s.default_qa.as_str() {
+    svc::validate_project_settings(s).map_err(|e| e.to_string())
-        "server" | "agent" | "human" => {}
-        other => {
-            return Err(format!(
-                "Invalid default_qa value '{other}'. Must be one of: server, agent, human"
-            ));
-        }
-    }
-    Ok(())
 }
 
-/// Write only the scalar settings from `s` into the project.toml at the given root.
+/// Thin wrapper — delegates to [`svc::write_project_settings`] and maps the
-/// Array sections (`[[component]]`, `[[agent]]`) are preserved unchanged.
+/// 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() {
+/// Return the configured editor command from the store, or `None` if not set.
-        std::fs::read_to_string(&config_path).map_err(|e| format!("Read config: {e}"))?
+pub fn get_editor_command_from_store(ctx: &AppContext) -> Option<String> {
-    } else {
+    svc::get_editor_command(&*ctx.store)
-        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(())
 }
 
 #[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<Json<EditorCommandResponse>> {
-        let editor_command = self
+        let editor_command = get_editor_command_from_store(&self.ctx);
-            .ctx
-            .store
-            .get(EDITOR_COMMAND_KEY)
-            .and_then(|v| v.as_str().map(|s| s.to_string()));
         Ok(Json(EditorCommandResponse { editor_command }))
     }
 
@@ -223,19 +78,8 @@ impl SettingsApi {
         path: Query<String>,
         line: Query<Option<u32>>,
     ) -> OpenApiResult<Json<OpenFileResponse>> {
-        let editor_command = get_editor_command_from_store(&self.ctx)
+        svc::open_file_in_editor(&*self.ctx.store, &path.0, line.0)
-            .ok_or_else(|| bad_request("No editor configured".to_string()))?;
+            .map_err(|e| bad_request(e.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}")))?;
-
         Ok(Json(OpenFileResponse { success: true }))
     }
 
@@ -243,8 +87,9 @@ impl SettingsApi {
     #[oai(path = "/settings", method = "get")]
     async fn get_settings(&self) -> OpenApiResult<Json<ProjectSettings>> {
         let project_root = self.ctx.state.get_project_root().map_err(bad_request)?;
-        let config = ProjectConfig::load(&project_root).map_err(bad_request)?;
+        let s =
-        Ok(Json(settings_from_config(&config)))
+            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<Json<ProjectSettings>> {
         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)?;
+        let s =
-        Ok(Json(settings_from_config(&config)))
+            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<String> {
-    ctx.store
-        .get(EDITOR_COMMAND_KEY)
-        .and_then(|v| v.as_str().map(|s| s.to_string()))
-}
-
 #[cfg(test)]
 impl From<std::sync::Arc<AppContext>> for SettingsApi {
     fn from(ctx: std::sync::Arc<AppContext>) -> 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());

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;

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<String, Error> {
+    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}")))
+}

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<ProjectSettings, Error> {
+    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<String> {
+    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<u32>,
+) -> 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());
+    }
+}

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<String>,
+    /// Maximum number of concurrent coder-stage agents.
+    pub max_coders: Option<u32>,
+    /// 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<String>,
+    /// Whether to send RateLimitWarning chat notifications. Default: true.
+    pub rate_limit_notifications: bool,
+    /// IANA timezone name (e.g. "Europe/London").
+    pub timezone: Option<String>,
+    /// WebSocket URL of a remote huskies node to sync CRDT state with.
+    pub rendezvous: Option<String>,
+    /// 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));
+    }
+}

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());
+    }
+}