From 70797753df287b31baae5a3259a2216daa939cce Mon Sep 17 00:00:00 2001 From: dave Date: Sun, 17 May 2026 23:57:44 +0000 Subject: [PATCH] huskies: merge 1132 story Chat-bot proxy reads stale `gateway_project_urls` BTreeMap instead of live store (1122 missed this seam) --- .../src/chat/transport/matrix/bot/context.rs | 96 +++++++++++++------ .../matrix/bot/messages/on_room_message.rs | 13 ++- server/src/chat/transport/matrix/bot/run.rs | 14 ++- server/src/chat/transport/matrix/mod.rs | 2 - server/src/gateway/mod.rs | 8 -- server/src/main.rs | 1 - server/src/service/gateway/io.rs | 3 - server/src/service/gateway/mod.rs | 9 -- 8 files changed, 88 insertions(+), 58 deletions(-) diff --git a/server/src/chat/transport/matrix/bot/context.rs b/server/src/chat/transport/matrix/bot/context.rs index 29822293..28a6d496 100644 --- a/server/src/chat/transport/matrix/bot/context.rs +++ b/server/src/chat/transport/matrix/bot/context.rs @@ -88,10 +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: 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. - pub gateway_project_urls: BTreeMap, /// In gateway mode: shared live projects map from [`GatewayState`]. /// /// The `new project` command writes here so HTTP handlers see the new entry @@ -130,7 +126,12 @@ impl BotContext { pub async fn active_project_url(&self) -> Option { let ap = self.gateway_active_project.as_ref()?; let name = ap.read().await.clone(); - self.gateway_project_urls.get(&name).cloned() + let store = self.gateway_projects_store.as_ref()?; + store + .read() + .await + .get(&name) + .and_then(|entry| entry.url.clone()) } /// Proxy a bot command to the active project over a WebSocket RPC call. @@ -266,7 +267,9 @@ mod tests { fn test_bot_context( services: Arc, gateway_active_project: Option>>, - gateway_project_urls: BTreeMap, + gateway_projects_store: Option< + Arc>>, + >, ) -> BotContext { BotContext { services, @@ -286,8 +289,7 @@ mod tests { std::path::PathBuf::from("/tmp/timers.json"), )), gateway_active_project, - gateway_project_urls, - gateway_projects_store: None, + gateway_projects_store, handled_incoming_event_ids: Arc::new(TokioMutex::new(SeenEventIds::new( SEEN_EVENT_IDS_CAP, ))), @@ -304,7 +306,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, BTreeMap::new()); + let ctx = test_bot_context(services, None, None); assert_eq!( ctx.effective_project_root().await, PathBuf::from("/projects/myapp") @@ -315,14 +317,7 @@ mod tests { async fn effective_project_root_gateway_uses_active_project_subdir() { let services = test_services(PathBuf::from("/gateway")); let active = Arc::new(RwLock::new("huskies".to_string())); - let ctx = test_bot_context( - services, - Some(Arc::clone(&active)), - BTreeMap::from([ - ("huskies".into(), "http://localhost:3001".into()), - ("robot-studio".into(), "http://localhost:3002".into()), - ]), - ); + let ctx = test_bot_context(services, Some(Arc::clone(&active)), None); assert_eq!( ctx.effective_project_root().await, PathBuf::from("/gateway/huskies") @@ -333,14 +328,7 @@ mod tests { async fn effective_project_root_gateway_reflects_project_switch() { let services = test_services(PathBuf::from("/gateway")); let active = Arc::new(RwLock::new("huskies".to_string())); - let ctx = test_bot_context( - services, - Some(Arc::clone(&active)), - BTreeMap::from([ - ("huskies".into(), "http://localhost:3001".into()), - ("robot-studio".into(), "http://localhost:3002".into()), - ]), - ); + let ctx = test_bot_context(services, Some(Arc::clone(&active)), None); assert_eq!( ctx.effective_project_root().await, @@ -416,7 +404,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, BTreeMap::new()); + let ctx = test_bot_context(services, None, None); let _cloned = ctx.clone(); } @@ -463,11 +451,16 @@ mod tests { let base_url = format!("http://127.0.0.1:{port}"); let services = test_services(PathBuf::from("/gateway")); let active = Arc::new(RwLock::new("huskies".to_string())); - let ctx = test_bot_context( - services, - Some(Arc::clone(&active)), - BTreeMap::from([("huskies".into(), base_url)]), - ); + let store = Arc::new(RwLock::new(BTreeMap::from([( + "huskies".to_string(), + crate::service::gateway::config::ProjectEntry { + url: Some(base_url), + auth_token: None, + ssh_port: None, + host_path: None, + }, + )]))); + let ctx = test_bot_context(services, Some(Arc::clone(&active)), Some(store)); let result = ctx.proxy_bot_command("status", "").await; assert_eq!( @@ -478,4 +471,45 @@ mod tests { server.await.unwrap(); } + + /// Regression test for story 1132: `active_project_url` must read from the + /// live `gateway_projects_store`, not a stale snapshot frozen at bot startup. + /// Adding a project to the store after `BotContext` is created must be + /// visible immediately — no restart required. + #[tokio::test] + async fn active_project_url_reflects_runtime_added_project() { + let store: Arc>> = + Arc::new(RwLock::new(BTreeMap::new())); + let active = Arc::new(RwLock::new("new-project".to_string())); + let services = test_services(PathBuf::from("/gateway")); + let ctx = test_bot_context( + services, + Some(Arc::clone(&active)), + Some(Arc::clone(&store)), + ); + + // Store is empty — must return None. + assert!( + ctx.active_project_url().await.is_none(), + "URL must be None when store is empty" + ); + + // Insert the entry at runtime (simulates `new project` command). + store.write().await.insert( + "new-project".to_string(), + crate::service::gateway::config::ProjectEntry { + url: Some("http://localhost:3099".to_string()), + auth_token: None, + ssh_port: None, + host_path: None, + }, + ); + + // Now the live store has the entry — active_project_url must see it. + assert_eq!( + ctx.active_project_url().await.as_deref(), + Some("http://localhost:3099"), + "URL must be visible after runtime insertion without bot restart" + ); + } } 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 721023a7..724e980c 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 @@ -265,7 +265,18 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message( // `all_status` — aggregate pipeline status across all projects (gateway-only). if cmd == "all_status" { - let project_urls = ctx.gateway_project_urls.clone(); + let project_urls: std::collections::BTreeMap = if let Some(ref store) = + ctx.gateway_projects_store + { + store + .read() + .await + .iter() + .filter_map(|(name, entry)| entry.url.clone().map(|url| (name.clone(), url))) + .collect() + } else { + std::collections::BTreeMap::new() + }; let client = reqwest::Client::new(); let statuses = crate::gateway::fetch_all_project_pipeline_statuses(&project_urls, &client).await; diff --git a/server/src/chat/transport/matrix/bot/run.rs b/server/src/chat/transport/matrix/bot/run.rs index 32c5253c..653f898d 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_project_urls: std::collections::BTreeMap, gateway_projects_store: Option< Arc< RwLock< @@ -182,7 +181,17 @@ pub async fn run_bot( let announce_room_ids = target_room_ids.clone(); // Clone values needed by the gateway notification poller (only used in gateway mode). let poller_room_ids: Vec = target_room_ids.iter().map(|r| r.to_string()).collect(); - let poller_project_urls = gateway_project_urls.clone(); + let poller_project_urls: std::collections::BTreeMap = + if let Some(ref store) = gateway_projects_store { + store + .read() + .await + .iter() + .filter_map(|(name, entry)| entry.url.clone().map(|url| (name.clone(), url))) + .collect() + } else { + std::collections::BTreeMap::new() + }; let poller_poll_interval = config.aggregated_notifications_poll_interval_secs; let poller_enabled = config.aggregated_notifications_enabled; @@ -321,7 +330,6 @@ pub async fn run_bot( transport: Arc::clone(&transport), timer_store, gateway_active_project, - gateway_project_urls, gateway_projects_store, handled_incoming_event_ids: Arc::new(TokioMutex::new(super::context::SeenEventIds::new( super::context::SEEN_EVENT_IDS_CAP, diff --git a/server/src/chat/transport/matrix/mod.rs b/server/src/chat/transport/matrix/mod.rs index d9f7429a..9dc93c9b 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_project_urls: std::collections::BTreeMap, gateway_projects_store: Option< Arc< RwLock< @@ -128,7 +127,6 @@ pub fn spawn_bot( watcher_tx, shutdown_rx, gateway_active_project, - gateway_project_urls, gateway_projects_store, timer_store, gateway_event_rx, diff --git a/server/src/gateway/mod.rs b/server/src/gateway/mod.rs index a75b6eb2..3bdc403e 100644 --- a/server/src/gateway/mod.rs +++ b/server/src/gateway/mod.rs @@ -106,17 +106,9 @@ 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_project_urls: std::collections::BTreeMap = state_arc - .projects - .read() - .await - .iter() - .filter_map(|(name, entry)| entry.url.as_ref().map(|u| (name.clone(), u.clone()))) - .collect(); let (bot_abort, bot_shutdown_tx) = gateway::io::spawn_gateway_bot( &config_dir, Arc::clone(&state_arc.active_project), - gateway_project_urls, Arc::clone(&state_arc.projects), port, Some(state_arc.event_tx.clone()), diff --git a/server/src/main.rs b/server/src/main.rs index 7cc442d9..a95ee792 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -368,7 +368,6 @@ async fn main() -> Result<(), std::io::Error> { Arc::clone(&services), matrix_shutdown_rx, None, - std::collections::BTreeMap::new(), None, timer_store_for_bot, None, diff --git a/server/src/service/gateway/io.rs b/server/src/service/gateway/io.rs index 0b9b02c8..09d90114 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_project_urls: BTreeMap, gateway_projects_store: std::sync::Arc>>, port: u16, gateway_event_tx: Option>, @@ -577,7 +576,6 @@ pub fn spawn_gateway_bot( services, shutdown_rx, Some(active_project), - gateway_project_urls, Some(gateway_projects_store), timer_store, gateway_event_rx, @@ -608,7 +606,6 @@ mod tests { let (handle, shutdown_tx) = spawn_gateway_bot( tmp.path(), active, - std::collections::BTreeMap::new(), projects_store, 3001, Some(event_tx), diff --git a/server/src/service/gateway/mod.rs b/server/src/service/gateway/mod.rs index fe17c65c..df0497f9 100644 --- a/server/src/service/gateway/mod.rs +++ b/server/src/service/gateway/mod.rs @@ -673,18 +673,9 @@ pub async fn save_bot_config_and_restart(state: &GatewayState, content: &str) -> if let Some(h) = handle.take() { h.abort(); } - let gateway_project_urls: BTreeMap = state - .projects - .read() - .await - .iter() - .filter_map(|(name, entry)| entry.url.as_ref().map(|u| (name.clone(), u.clone()))) - .collect(); - let (new_handle, new_shutdown_tx) = io::spawn_gateway_bot( &state.config_dir, Arc::clone(&state.active_project), - gateway_project_urls, Arc::clone(&state.projects), state.port, Some(state.event_tx.clone()),