huskies: merge 607_story_extract_bot_command_service

2026-04-24 15:23:54 +00:00
parent 1910365321
commit aba3120388
5 changed files with 507 additions and 248 deletions
@@ -0,0 +1,158 @@
+//! Bot command I/O — the ONLY place in `service/bot_command/` that may call
+//! transport handlers, load stores, spawn tasks, or interact with the agent
+//! pool.
+//!
+//! Every function here is a thin adapter over the underlying matrix/timer/htop
+//! handlers. No argument parsing or business logic lives here — that belongs in
+//! `parse.rs` or `mod.rs`.
+
+use crate::agents::AgentPool;
+use std::path::Path;
+use std::sync::Arc;
+
+use super::parse::{AssignArgs, StartArgs};
+
+/// Call the Matrix `assign` handler with pre-validated arguments.
+pub(super) async fn call_assign(
+    args: &AssignArgs,
+    project_root: &Path,
+    agents: &Arc<AgentPool>,
+) -> String {
+    crate::chat::transport::matrix::assign::handle_assign(
+        "web-ui",
+        &args.number,
+        &args.model,
+        project_root,
+        agents,
+    )
+    .await
+}
+
+/// Call the Matrix `start` handler with pre-validated arguments.
+pub(super) async fn call_start(
+    args: &StartArgs,
+    project_root: &Path,
+    agents: &Arc<AgentPool>,
+) -> String {
+    crate::chat::transport::matrix::start::handle_start(
+        "web-ui",
+        &args.number,
+        args.hint.as_deref(),
+        project_root,
+        agents,
+    )
+    .await
+}
+
+/// Call the Matrix `delete` handler with a pre-validated story number.
+pub(super) async fn call_delete(
+    number: &str,
+    project_root: &Path,
+    agents: &Arc<AgentPool>,
+) -> String {
+    crate::chat::transport::matrix::delete::handle_delete("web-ui", number, project_root, agents)
+        .await
+}
+
+/// Call the Matrix `rmtree` handler with a pre-validated story number.
+pub(super) async fn call_rmtree(
+    number: &str,
+    project_root: &Path,
+    agents: &Arc<AgentPool>,
+) -> String {
+    crate::chat::transport::matrix::rmtree::handle_rmtree("web-ui", number, project_root, agents)
+        .await
+}
+
+/// Call the Matrix `rebuild` handler.
+pub(super) async fn call_rebuild(project_root: &Path, agents: &Arc<AgentPool>) -> String {
+    crate::chat::transport::matrix::rebuild::handle_rebuild("web-ui", project_root, agents).await
+}
+
+/// Parse and execute a `timer` command.
+///
+/// Returns `Err` with a usage string if the timer arguments cannot be parsed.
+pub(super) async fn call_timer(args: &str, project_root: &Path) -> Result<String, String> {
+    let synthetic = format!("__web_ui__ timer {args}");
+    let timer_cmd = match crate::chat::timer::extract_timer_command(
+        &synthetic,
+        "__web_ui__",
+        "@__web_ui__:localhost",
+    ) {
+        Some(cmd) => cmd,
+        None => {
+            return Err(
+                "Usage: `/timer list`, `/timer <number> <HH:MM>`, or `/timer cancel <number>`"
+                    .to_string(),
+            );
+        }
+    };
+    let store =
+        crate::chat::timer::TimerStore::load(project_root.join(".huskies").join("timers.json"));
+    Ok(crate::chat::timer::handle_timer_command(timer_cmd, &store, project_root).await)
+}
+
+/// Build an `htop` snapshot for the web UI.
+///
+/// The web UI uses one-shot HTTP requests, so live-updating sessions are not
+/// supported. `htop stop` returns a helpful explanation instead of an error.
+pub(super) fn call_htop(args: &str, agents: &Arc<AgentPool>) -> String {
+    use crate::chat::transport::matrix::htop::{HtopCommand, build_htop_message};
+
+    let synthetic = if args.is_empty() {
+        "__web_ui__ htop".to_string()
+    } else {
+        format!("__web_ui__ htop {args}")
+    };
+
+    match crate::chat::transport::matrix::htop::extract_htop_command(
+        &synthetic,
+        "__web_ui__",
+        "@__web_ui__:localhost",
+    ) {
+        Some(HtopCommand::Stop) => "No active htop session in the web UI. \
+             Live sessions are only supported in chat transports (Matrix, Slack, Discord)."
+            .to_string(),
+        Some(HtopCommand::Start { duration_secs }) => build_htop_message(agents, 0, duration_secs),
+        None => build_htop_message(agents, 0, 300),
+    }
+}
+
+/// Dispatch through the synchronous command registry.
+///
+/// Returns `Some(response)` if the command keyword is registered, or `None`
+/// if the keyword is unknown.
+pub(super) fn call_sync(
+    cmd: &str,
+    args: &str,
+    project_root: &Path,
+    agents: &Arc<AgentPool>,
+) -> Option<String> {
+    use crate::chat::commands::CommandDispatch;
+    use std::collections::HashSet;
+    use std::sync::Mutex;
+
+    let ambient_rooms: Arc<Mutex<HashSet<String>>> = Arc::new(Mutex::new(HashSet::new()));
+    let bot_name = "__web_ui__";
+    let bot_user_id = "@__web_ui__:localhost";
+    let room_id = "__web_ui__";
+
+    let dispatch = CommandDispatch {
+        bot_name,
+        bot_user_id,
+        project_root,
+        agents,
+        ambient_rooms: &ambient_rooms,
+        room_id,
+    };
+
+    // Build a synthetic bot-addressed message so the registry parses it
+    // identically to messages from chat transports.
+    let synthetic = if args.is_empty() {
+        format!("{bot_name} {cmd}")
+    } else {
+        format!("{bot_name} {cmd} {args}")
+    };
+
+    crate::chat::commands::try_handle_command(&dispatch, &synthetic)
+}
@@ -0,0 +1,97 @@
+//! Bot command service — domain logic for dispatching slash commands.
+//!
+//! Extracted from `http/bot_command.rs` so that argument parsing and dispatch
+//! are independently testable without an HTTP layer.
+//!
+//! Conventions: `docs/architecture/service-modules.md`
+//!
+//! # Structure
+//! - `mod.rs` (this file) — public API and typed `Error` type
+//! - `parse.rs` — pure argument parsing, no I/O
+//! - `io.rs` — all side-effectful calls (transport handlers, stores, agent pool)
+
+pub(super) mod io;
+pub mod parse;
+
+use crate::agents::AgentPool;
+use std::path::Path;
+use std::sync::Arc;
+
+// ── Error type ────────────────────────────────────────────────────────────────
+
+/// Typed errors returned by `service::bot_command::execute`.
+///
+/// HTTP handlers map these to specific status codes:
+/// - [`Error::UnknownCommand`] → 404 Not Found
+/// - [`Error::BadArgs`] → 400 Bad Request
+/// - [`Error::CommandFailed`] → 500 Internal Server Error
+#[derive(Debug)]
+#[allow(dead_code)] // CommandFailed is part of the public API contract; not yet reachable
+pub enum Error {
+    /// The command keyword does not match any registered command.
+    UnknownCommand(String),
+    /// The command exists but the provided arguments are invalid.
+    BadArgs(String),
+    /// The command ran but failed with an internal error.
+    CommandFailed(String),
+}
+
+impl std::fmt::Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::UnknownCommand(msg) | Self::BadArgs(msg) | Self::CommandFailed(msg) => {
+                write!(f, "{msg}")
+            }
+        }
+    }
+}
+
+// ── Public API ────────────────────────────────────────────────────────────────
+
+/// Execute a bot command and return the markdown response.
+///
+/// Dispatches to the same handlers used by the Matrix and Slack bots. The
+/// `cmd` argument is the lower-cased command keyword (e.g. `"status"`,
+/// `"start"`). The `args` argument is any text after the keyword, already
+/// trimmed.
+///
+/// # Errors
+/// - [`Error::UnknownCommand`] if the command keyword is not registered.
+/// - [`Error::BadArgs`] if the arguments fail validation.
+/// - [`Error::CommandFailed`] if command execution raises an internal error.
+pub async fn execute(
+    cmd: &str,
+    args: &str,
+    project_root: &Path,
+    agents: &Arc<AgentPool>,
+) -> Result<String, Error> {
+    match cmd {
+        "assign" => {
+            let parsed = parse::parse_assign(args).map_err(Error::BadArgs)?;
+            Ok(io::call_assign(&parsed, project_root, agents).await)
+        }
+        "start" => {
+            let parsed = parse::parse_start(args).map_err(Error::BadArgs)?;
+            Ok(io::call_start(&parsed, project_root, agents).await)
+        }
+        "delete" => {
+            let number = parse::parse_number("delete", args).map_err(Error::BadArgs)?;
+            Ok(io::call_delete(&number, project_root, agents).await)
+        }
+        "rmtree" => {
+            let number = parse::parse_number("rmtree", args).map_err(Error::BadArgs)?;
+            Ok(io::call_rmtree(&number, project_root, agents).await)
+        }
+        "rebuild" => Ok(io::call_rebuild(project_root, agents).await),
+        "timer" => io::call_timer(args, project_root)
+            .await
+            .map_err(Error::BadArgs),
+        "htop" => Ok(io::call_htop(args, agents)),
+        _ => match io::call_sync(cmd, args, project_root, agents) {
+            Some(response) => Ok(response),
+            None => Err(Error::UnknownCommand(format!(
+                "Unknown command: `/{cmd}`. Type `/help` to see available commands."
+            ))),
+        },
+    }
+}
@@ -0,0 +1,216 @@
+//! Pure argument parsing for bot commands.
+//!
+//! Every function in this module is synchronous and free of I/O. All
+//! filesystem, network, and agent-pool access belongs in `io.rs`.
+
+// ── Parsed argument types ─────────────────────────────────────────────────────
+
+/// Parsed arguments for the `assign` command.
+#[derive(Debug)]
+pub struct AssignArgs {
+    /// The numeric story identifier (as a string, e.g. `"42"`).
+    pub number: String,
+    /// The model / agent name (e.g. `"opus"`, `"coder-sonnet"`).
+    pub model: String,
+}
+
+/// Parsed arguments for the `start` command.
+#[derive(Debug)]
+pub struct StartArgs {
+    /// The numeric story identifier.
+    pub number: String,
+    /// Optional model hint (e.g. `"opus"` → resolved to `"coder-opus"`).
+    pub hint: Option<String>,
+}
+
+// ── Parsing functions ─────────────────────────────────────────────────────────
+
+/// Parse `assign` arguments: `<number> <model>`.
+///
+/// Returns `Err` with a user-visible usage string if the arguments are missing
+/// or invalid (non-numeric number, empty model).
+pub fn parse_assign(args: &str) -> Result<AssignArgs, String> {
+    let mut parts = args.splitn(2, char::is_whitespace);
+    let number = parts.next().unwrap_or("").trim().to_string();
+    let model = parts.next().unwrap_or("").trim().to_string();
+
+    if number.is_empty() || !number.chars().all(|c| c.is_ascii_digit()) || model.is_empty() {
+        return Err("Usage: `/assign <number> <model>` (e.g. `/assign 42 opus`)".to_string());
+    }
+
+    Ok(AssignArgs { number, model })
+}
+
+/// Parse `start` arguments: `<number>` or `<number> <model_hint>`.
+///
+/// Returns `Err` with a user-visible usage string if the number is missing
+/// or non-numeric.
+pub fn parse_start(args: &str) -> Result<StartArgs, String> {
+    let mut parts = args.splitn(2, char::is_whitespace);
+    let number = parts.next().unwrap_or("").trim().to_string();
+    let hint_str = parts.next().unwrap_or("").trim();
+
+    if number.is_empty() || !number.chars().all(|c| c.is_ascii_digit()) {
+        return Err(
+            "Usage: `/start <number>` or `/start <number> <model>` (e.g. `/start 42 opus`)"
+                .to_string(),
+        );
+    }
+
+    let hint = if hint_str.is_empty() {
+        None
+    } else {
+        Some(hint_str.to_string())
+    };
+
+    Ok(StartArgs { number, hint })
+}
+
+/// Parse a single numeric argument for commands like `delete` and `rmtree`.
+///
+/// `cmd_name` is used only in the error message (e.g. `"delete"` or `"rmtree"`).
+/// Returns `Err` with a user-visible usage string if the argument is missing
+/// or non-numeric.
+pub fn parse_number(cmd_name: &str, args: &str) -> Result<String, String> {
+    let number = args.trim().to_string();
+    if number.is_empty() || !number.chars().all(|c| c.is_ascii_digit()) {
+        return Err(format!(
+            "Usage: `/{cmd_name} <number>` (e.g. `/{cmd_name} 42`)"
+        ));
+    }
+    Ok(number)
+}
+
+// ── Tests ─────────────────────────────────────────────────────────────────────
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // -- parse_assign ----------------------------------------------------------
+
+    #[test]
+    fn assign_valid() {
+        let r = parse_assign("42 opus").unwrap();
+        assert_eq!(r.number, "42");
+        assert_eq!(r.model, "opus");
+    }
+
+    #[test]
+    fn assign_valid_model_with_spaces() {
+        // splitn(2): everything after first whitespace goes into `model`.
+        let r = parse_assign("42 claude-opus-4").unwrap();
+        assert_eq!(r.number, "42");
+        assert_eq!(r.model, "claude-opus-4");
+    }
+
+    #[test]
+    fn assign_missing_all_args() {
+        assert!(parse_assign("").is_err());
+    }
+
+    #[test]
+    fn assign_missing_model() {
+        let err = parse_assign("42").unwrap_err();
+        assert!(
+            err.contains("Usage"),
+            "error should contain usage hint: {err}"
+        );
+    }
+
+    #[test]
+    fn assign_non_numeric_number() {
+        let err = parse_assign("foo opus").unwrap_err();
+        assert!(
+            err.contains("Usage"),
+            "error should contain usage hint: {err}"
+        );
+    }
+
+    #[test]
+    fn assign_number_with_letters_is_invalid() {
+        assert!(parse_assign("42x opus").is_err());
+    }
+
+    // -- parse_start -----------------------------------------------------------
+
+    #[test]
+    fn start_valid_number_only() {
+        let r = parse_start("42").unwrap();
+        assert_eq!(r.number, "42");
+        assert!(r.hint.is_none());
+    }
+
+    #[test]
+    fn start_valid_with_hint() {
+        let r = parse_start("42 opus").unwrap();
+        assert_eq!(r.number, "42");
+        assert_eq!(r.hint.as_deref(), Some("opus"));
+    }
+
+    #[test]
+    fn start_missing_number() {
+        let err = parse_start("").unwrap_err();
+        assert!(
+            err.contains("Usage"),
+            "error should contain usage hint: {err}"
+        );
+    }
+
+    #[test]
+    fn start_non_numeric_number() {
+        let err = parse_start("foo").unwrap_err();
+        assert!(
+            err.contains("Usage"),
+            "error should contain usage hint: {err}"
+        );
+    }
+
+    #[test]
+    fn start_non_numeric_with_hint() {
+        assert!(parse_start("foo opus").is_err());
+    }
+
+    // -- parse_number ----------------------------------------------------------
+
+    #[test]
+    fn number_valid() {
+        assert_eq!(parse_number("delete", "99").unwrap(), "99");
+    }
+
+    #[test]
+    fn number_missing() {
+        let err = parse_number("delete", "").unwrap_err();
+        assert!(
+            err.contains("Usage"),
+            "error should contain usage hint: {err}"
+        );
+        assert!(
+            err.contains("delete"),
+            "error should mention the command: {err}"
+        );
+    }
+
+    #[test]
+    fn number_non_numeric() {
+        let err = parse_number("delete", "abc").unwrap_err();
+        assert!(
+            err.contains("Usage"),
+            "error should contain usage hint: {err}"
+        );
+    }
+
+    #[test]
+    fn number_usage_contains_cmd_name() {
+        let err = parse_number("rmtree", "").unwrap_err();
+        assert!(
+            err.contains("rmtree"),
+            "usage should mention the command: {err}"
+        );
+    }
+
+    #[test]
+    fn number_whitespace_only_is_invalid() {
+        assert!(parse_number("delete", "   ").is_err());
+    }
+}