From c1b7e12b0b1eb6670af7655cbc6de32739fcb594 Mon Sep 17 00:00:00 2001 From: dave Date: Sun, 17 May 2026 18:44:56 +0000 Subject: [PATCH] huskies: merge 1122 story Chat-bot switch command reads stale `gateway_projects` Vec instead of live `gateway_projects_store` --- .../src/chat/transport/matrix/bot/context.rs | 12 +- .../matrix/bot/messages/on_room_message.rs | 114 ++++++++++++++++-- server/src/chat/transport/matrix/bot/run.rs | 2 - server/src/chat/transport/matrix/mod.rs | 2 - server/src/gateway/mod.rs | 2 - server/src/main.rs | 1 - server/src/service/gateway/io.rs | 3 - server/src/service/gateway/mod.rs | 2 - 8 files changed, 107 insertions(+), 31 deletions(-) diff --git a/server/src/chat/transport/matrix/bot/context.rs b/server/src/chat/transport/matrix/bot/context.rs index 8da73421..f26ee918 100644 --- a/server/src/chat/transport/matrix/bot/context.rs +++ b/server/src/chat/transport/matrix/bot/context.rs @@ -88,9 +88,6 @@ pub struct BotContext { /// In gateway mode: the currently active project (shared with the gateway HTTP handler). /// `None` in standalone single-project mode. pub gateway_active_project: Option>>, - /// In gateway mode: valid project names accepted by the `switch` command. - /// Empty in standalone mode. - pub gateway_projects: Vec, /// In gateway mode: mapping of project name → base URL (e.g. `"http://localhost:3001"`). /// Used to proxy bot commands to the active project over WebSocket (`/ws`). /// Empty in standalone mode. @@ -283,7 +280,6 @@ mod tests { fn test_bot_context( services: Arc, gateway_active_project: Option>>, - gateway_projects: Vec, gateway_project_urls: BTreeMap, ) -> BotContext { BotContext { @@ -304,7 +300,6 @@ mod tests { std::path::PathBuf::from("/tmp/timers.json"), )), gateway_active_project, - gateway_projects, gateway_project_urls, gateway_projects_store: None, pending_pipeline_events: Arc::new(TokioMutex::new(Vec::new())), @@ -325,7 +320,7 @@ mod tests { #[tokio::test] async fn effective_project_root_standalone_returns_project_root() { let services = test_services(PathBuf::from("/projects/myapp")); - let ctx = test_bot_context(services, None, vec![], BTreeMap::new()); + let ctx = test_bot_context(services, None, BTreeMap::new()); assert_eq!( ctx.effective_project_root().await, PathBuf::from("/projects/myapp") @@ -339,7 +334,6 @@ mod tests { let ctx = test_bot_context( services, Some(Arc::clone(&active)), - vec!["huskies".into(), "robot-studio".into()], BTreeMap::from([ ("huskies".into(), "http://localhost:3001".into()), ("robot-studio".into(), "http://localhost:3002".into()), @@ -358,7 +352,6 @@ mod tests { let ctx = test_bot_context( services, Some(Arc::clone(&active)), - vec!["huskies".into(), "robot-studio".into()], BTreeMap::from([ ("huskies".into(), "http://localhost:3001".into()), ("robot-studio".into(), "http://localhost:3002".into()), @@ -439,7 +432,7 @@ mod tests { #[test] fn bot_context_has_no_require_verified_devices_field() { let services = test_services(PathBuf::from("/tmp")); - let ctx = test_bot_context(services, None, vec![], BTreeMap::new()); + let ctx = test_bot_context(services, None, BTreeMap::new()); let _cloned = ctx.clone(); } @@ -489,7 +482,6 @@ mod tests { let ctx = test_bot_context( services, Some(Arc::clone(&active)), - vec!["huskies".into()], BTreeMap::from([("huskies".into(), base_url)]), ); diff --git a/server/src/chat/transport/matrix/bot/messages/on_room_message.rs b/server/src/chat/transport/matrix/bot/messages/on_room_message.rs index 91eab920..721023a7 100644 --- a/server/src/chat/transport/matrix/bot/messages/on_room_message.rs +++ b/server/src/chat/transport/matrix/bot/messages/on_room_message.rs @@ -19,6 +19,31 @@ use super::super::verification::check_sender_verified; use super::handle_message; +/// Evaluate a `switch ` command against the live project store. +/// +/// Reads valid project names from the store at call time so newly added +/// projects are visible without a bot restart. Returns the reply text. +pub(super) async fn eval_switch_command( + arg: &str, + active_project: &tokio::sync::RwLock, + store: &tokio::sync::RwLock< + std::collections::BTreeMap, + >, +) -> String { + let projects: Vec = store.read().await.keys().cloned().collect(); + if arg.is_empty() { + let available = projects.join(", "); + format!("Usage: `switch `. Available projects: {available}") + } else if projects.iter().any(|p| p == arg) { + *active_project.write().await = arg.to_string(); + crate::crdt_state::write_gateway_active_project(arg); + format!("Switched to project **{arg}**.") + } else { + let available = projects.join(", "); + format!("Unknown project `{arg}`. Available: {available}") + } +} + pub(in crate::chat::transport::matrix::bot) async fn on_room_message( ev: OriginalSyncRoomMessageEvent, room: Room, @@ -572,16 +597,10 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message( }; if cmd.eq_ignore_ascii_case("switch") { - let response = if arg.is_empty() { - let available = ctx.gateway_projects.join(", "); - format!("Usage: `switch `. Available projects: {available}") - } else if ctx.gateway_projects.iter().any(|p| p == &arg) { - *active_project.write().await = arg.clone(); - crate::crdt_state::write_gateway_active_project(&arg); - format!("Switched to project **{arg}**.") + let response = if let Some(ref store) = ctx.gateway_projects_store { + eval_switch_command(&arg, active_project, store).await } else { - let available = ctx.gateway_projects.join(", "); - format!("Unknown project `{arg}`. Available: {available}") + "Switch is unavailable: project store not initialised.".to_string() }; let html = markdown_to_html(&response); if let Ok(msg_id) = ctx @@ -704,3 +723,80 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message( .chat_dispatcher .submit(room_id_str, user_message, factory); } + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::eval_switch_command; + use crate::service::gateway::config::ProjectEntry; + use std::collections::BTreeMap; + use tokio::sync::RwLock; + + /// Regression test: `switch` reads from the live store, not a snapshot Vec. + /// + /// Seeds an empty store, inserts a project at runtime, then asserts the + /// command finds it — covering the bug where a stale `gateway_projects` Vec + /// caused newly added projects to be invisible until the bot restarted. + #[tokio::test] + async fn switch_reads_live_store_after_runtime_insert() { + let active = RwLock::new("huskies".to_string()); + let store: RwLock> = RwLock::new(BTreeMap::new()); + + // Empty store: unknown project. + let resp = eval_switch_command("robot-studio", &active, &store).await; + assert!( + resp.contains("Unknown project"), + "empty store should not find robot-studio: {resp}" + ); + + // Insert the project at runtime — no restart. + store.write().await.insert( + "robot-studio".to_string(), + ProjectEntry { + url: Some("http://localhost:3002".to_string()), + auth_token: None, + ssh_port: None, + host_path: None, + }, + ); + + // Now the live store has the project; switch must succeed. + let resp = eval_switch_command("robot-studio", &active, &store).await; + assert_eq!( + resp, "Switched to project **robot-studio**.", + "live store insert must be visible without restart: {resp}" + ); + assert_eq!( + *active.read().await, + "robot-studio", + "active project must be updated after switch" + ); + } + + #[tokio::test] + async fn switch_empty_arg_lists_available_projects() { + let active = RwLock::new("huskies".to_string()); + let store: RwLock> = RwLock::new(BTreeMap::from([( + "huskies".to_string(), + ProjectEntry { + url: None, + auth_token: None, + ssh_port: None, + host_path: None, + }, + )])); + + let resp = eval_switch_command("", &active, &store).await; + assert!( + resp.contains("Usage:"), + "empty arg should show usage: {resp}" + ); + assert!( + resp.contains("huskies"), + "usage should list available projects: {resp}" + ); + } +} diff --git a/server/src/chat/transport/matrix/bot/run.rs b/server/src/chat/transport/matrix/bot/run.rs index a9da7217..8dc3a58a 100644 --- a/server/src/chat/transport/matrix/bot/run.rs +++ b/server/src/chat/transport/matrix/bot/run.rs @@ -28,7 +28,6 @@ pub async fn run_bot( watcher_tx: tokio::sync::broadcast::Sender, shutdown_rx: watch::Receiver>, gateway_active_project: Option>>, - gateway_projects: Vec, gateway_project_urls: std::collections::BTreeMap, gateway_projects_store: Option< Arc< @@ -404,7 +403,6 @@ pub async fn run_bot( transport: Arc::clone(&transport), timer_store, gateway_active_project, - gateway_projects, gateway_project_urls, gateway_projects_store, pending_pipeline_events, diff --git a/server/src/chat/transport/matrix/mod.rs b/server/src/chat/transport/matrix/mod.rs index 5c8e4f44..d9f7429a 100644 --- a/server/src/chat/transport/matrix/mod.rs +++ b/server/src/chat/transport/matrix/mod.rs @@ -81,7 +81,6 @@ pub fn spawn_bot( services: Arc, shutdown_rx: watch::Receiver>, gateway_active_project: Option>>, - gateway_projects: Vec, gateway_project_urls: std::collections::BTreeMap, gateway_projects_store: Option< Arc< @@ -129,7 +128,6 @@ pub fn spawn_bot( watcher_tx, shutdown_rx, gateway_active_project, - gateway_projects, gateway_project_urls, gateway_projects_store, timer_store, diff --git a/server/src/gateway/mod.rs b/server/src/gateway/mod.rs index cd0178d6..a75b6eb2 100644 --- a/server/src/gateway/mod.rs +++ b/server/src/gateway/mod.rs @@ -106,7 +106,6 @@ pub async fn run(config_path: &Path, port: u16) -> Result<(), std::io::Error> { } // Spawn the Matrix bot if `.huskies/bot.toml` exists in the config directory. - let gateway_projects: Vec = state_arc.projects.read().await.keys().cloned().collect(); let gateway_project_urls: std::collections::BTreeMap = state_arc .projects .read() @@ -117,7 +116,6 @@ pub async fn run(config_path: &Path, port: u16) -> Result<(), std::io::Error> { let (bot_abort, bot_shutdown_tx) = gateway::io::spawn_gateway_bot( &config_dir, Arc::clone(&state_arc.active_project), - gateway_projects, gateway_project_urls, Arc::clone(&state_arc.projects), port, diff --git a/server/src/main.rs b/server/src/main.rs index e138e954..5ef3ccf6 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -364,7 +364,6 @@ async fn main() -> Result<(), std::io::Error> { Arc::clone(&services), matrix_shutdown_rx, None, - vec![], std::collections::BTreeMap::new(), None, timer_store_for_bot, diff --git a/server/src/service/gateway/io.rs b/server/src/service/gateway/io.rs index 102a784a..0b9b02c8 100644 --- a/server/src/service/gateway/io.rs +++ b/server/src/service/gateway/io.rs @@ -504,7 +504,6 @@ pub type ActiveProject = std::sync::Arc>; pub fn spawn_gateway_bot( config_dir: &Path, active_project: ActiveProject, - gateway_projects: Vec, gateway_project_urls: BTreeMap, gateway_projects_store: std::sync::Arc>>, port: u16, @@ -578,7 +577,6 @@ pub fn spawn_gateway_bot( services, shutdown_rx, Some(active_project), - gateway_projects, gateway_project_urls, Some(gateway_projects_store), timer_store, @@ -610,7 +608,6 @@ mod tests { let (handle, shutdown_tx) = spawn_gateway_bot( tmp.path(), active, - vec!["proj".to_string()], std::collections::BTreeMap::new(), projects_store, 3001, diff --git a/server/src/service/gateway/mod.rs b/server/src/service/gateway/mod.rs index 4bdb5dda..38900ac4 100644 --- a/server/src/service/gateway/mod.rs +++ b/server/src/service/gateway/mod.rs @@ -647,7 +647,6 @@ pub async fn save_bot_config_and_restart(state: &GatewayState, content: &str) -> if let Some(h) = handle.take() { h.abort(); } - let gateway_projects: Vec = state.projects.read().await.keys().cloned().collect(); let gateway_project_urls: BTreeMap = state .projects .read() @@ -659,7 +658,6 @@ pub async fn save_bot_config_and_restart(state: &GatewayState, content: &str) -> let (new_handle, new_shutdown_tx) = io::spawn_gateway_bot( &state.config_dir, Arc::clone(&state.active_project), - gateway_projects, gateway_project_urls, Arc::clone(&state.projects), state.port,