huskies: merge 1132 story Chat-bot proxy reads stale gateway_project_urls BTreeMap instead of live store (1122 missed this seam)
This commit is contained in:
@@ -88,10 +88,6 @@ pub struct BotContext {
|
|||||||
/// In gateway mode: the currently active project (shared with the gateway HTTP handler).
|
/// In gateway mode: the currently active project (shared with the gateway HTTP handler).
|
||||||
/// `None` in standalone single-project mode.
|
/// `None` in standalone single-project mode.
|
||||||
pub gateway_active_project: Option<Arc<RwLock<String>>>,
|
pub gateway_active_project: Option<Arc<RwLock<String>>>,
|
||||||
/// 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<String, String>,
|
|
||||||
/// In gateway mode: shared live projects map from [`GatewayState`].
|
/// In gateway mode: shared live projects map from [`GatewayState`].
|
||||||
///
|
///
|
||||||
/// The `new project` command writes here so HTTP handlers see the new entry
|
/// 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<String> {
|
pub async fn active_project_url(&self) -> Option<String> {
|
||||||
let ap = self.gateway_active_project.as_ref()?;
|
let ap = self.gateway_active_project.as_ref()?;
|
||||||
let name = ap.read().await.clone();
|
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.
|
/// Proxy a bot command to the active project over a WebSocket RPC call.
|
||||||
@@ -266,7 +267,9 @@ mod tests {
|
|||||||
fn test_bot_context(
|
fn test_bot_context(
|
||||||
services: Arc<Services>,
|
services: Arc<Services>,
|
||||||
gateway_active_project: Option<Arc<RwLock<String>>>,
|
gateway_active_project: Option<Arc<RwLock<String>>>,
|
||||||
gateway_project_urls: BTreeMap<String, String>,
|
gateway_projects_store: Option<
|
||||||
|
Arc<RwLock<BTreeMap<String, crate::service::gateway::config::ProjectEntry>>>,
|
||||||
|
>,
|
||||||
) -> BotContext {
|
) -> BotContext {
|
||||||
BotContext {
|
BotContext {
|
||||||
services,
|
services,
|
||||||
@@ -286,8 +289,7 @@ mod tests {
|
|||||||
std::path::PathBuf::from("/tmp/timers.json"),
|
std::path::PathBuf::from("/tmp/timers.json"),
|
||||||
)),
|
)),
|
||||||
gateway_active_project,
|
gateway_active_project,
|
||||||
gateway_project_urls,
|
gateway_projects_store,
|
||||||
gateway_projects_store: None,
|
|
||||||
handled_incoming_event_ids: Arc::new(TokioMutex::new(SeenEventIds::new(
|
handled_incoming_event_ids: Arc::new(TokioMutex::new(SeenEventIds::new(
|
||||||
SEEN_EVENT_IDS_CAP,
|
SEEN_EVENT_IDS_CAP,
|
||||||
))),
|
))),
|
||||||
@@ -304,7 +306,7 @@ mod tests {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn effective_project_root_standalone_returns_project_root() {
|
async fn effective_project_root_standalone_returns_project_root() {
|
||||||
let services = test_services(PathBuf::from("/projects/myapp"));
|
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!(
|
assert_eq!(
|
||||||
ctx.effective_project_root().await,
|
ctx.effective_project_root().await,
|
||||||
PathBuf::from("/projects/myapp")
|
PathBuf::from("/projects/myapp")
|
||||||
@@ -315,14 +317,7 @@ mod tests {
|
|||||||
async fn effective_project_root_gateway_uses_active_project_subdir() {
|
async fn effective_project_root_gateway_uses_active_project_subdir() {
|
||||||
let services = test_services(PathBuf::from("/gateway"));
|
let services = test_services(PathBuf::from("/gateway"));
|
||||||
let active = Arc::new(RwLock::new("huskies".to_string()));
|
let active = Arc::new(RwLock::new("huskies".to_string()));
|
||||||
let ctx = test_bot_context(
|
let ctx = test_bot_context(services, Some(Arc::clone(&active)), None);
|
||||||
services,
|
|
||||||
Some(Arc::clone(&active)),
|
|
||||||
BTreeMap::from([
|
|
||||||
("huskies".into(), "http://localhost:3001".into()),
|
|
||||||
("robot-studio".into(), "http://localhost:3002".into()),
|
|
||||||
]),
|
|
||||||
);
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
ctx.effective_project_root().await,
|
ctx.effective_project_root().await,
|
||||||
PathBuf::from("/gateway/huskies")
|
PathBuf::from("/gateway/huskies")
|
||||||
@@ -333,14 +328,7 @@ mod tests {
|
|||||||
async fn effective_project_root_gateway_reflects_project_switch() {
|
async fn effective_project_root_gateway_reflects_project_switch() {
|
||||||
let services = test_services(PathBuf::from("/gateway"));
|
let services = test_services(PathBuf::from("/gateway"));
|
||||||
let active = Arc::new(RwLock::new("huskies".to_string()));
|
let active = Arc::new(RwLock::new("huskies".to_string()));
|
||||||
let ctx = test_bot_context(
|
let ctx = test_bot_context(services, Some(Arc::clone(&active)), None);
|
||||||
services,
|
|
||||||
Some(Arc::clone(&active)),
|
|
||||||
BTreeMap::from([
|
|
||||||
("huskies".into(), "http://localhost:3001".into()),
|
|
||||||
("robot-studio".into(), "http://localhost:3002".into()),
|
|
||||||
]),
|
|
||||||
);
|
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
ctx.effective_project_root().await,
|
ctx.effective_project_root().await,
|
||||||
@@ -416,7 +404,7 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn bot_context_has_no_require_verified_devices_field() {
|
fn bot_context_has_no_require_verified_devices_field() {
|
||||||
let services = test_services(PathBuf::from("/tmp"));
|
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();
|
let _cloned = ctx.clone();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -463,11 +451,16 @@ mod tests {
|
|||||||
let base_url = format!("http://127.0.0.1:{port}");
|
let base_url = format!("http://127.0.0.1:{port}");
|
||||||
let services = test_services(PathBuf::from("/gateway"));
|
let services = test_services(PathBuf::from("/gateway"));
|
||||||
let active = Arc::new(RwLock::new("huskies".to_string()));
|
let active = Arc::new(RwLock::new("huskies".to_string()));
|
||||||
let ctx = test_bot_context(
|
let store = Arc::new(RwLock::new(BTreeMap::from([(
|
||||||
services,
|
"huskies".to_string(),
|
||||||
Some(Arc::clone(&active)),
|
crate::service::gateway::config::ProjectEntry {
|
||||||
BTreeMap::from([("huskies".into(), base_url)]),
|
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;
|
let result = ctx.proxy_bot_command("status", "").await;
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
@@ -478,4 +471,45 @@ mod tests {
|
|||||||
|
|
||||||
server.await.unwrap();
|
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<RwLock<BTreeMap<String, crate::service::gateway::config::ProjectEntry>>> =
|
||||||
|
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"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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).
|
// `all_status` — aggregate pipeline status across all projects (gateway-only).
|
||||||
if cmd == "all_status" {
|
if cmd == "all_status" {
|
||||||
let project_urls = ctx.gateway_project_urls.clone();
|
let project_urls: std::collections::BTreeMap<String, String> = 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 client = reqwest::Client::new();
|
||||||
let statuses =
|
let statuses =
|
||||||
crate::gateway::fetch_all_project_pipeline_statuses(&project_urls, &client).await;
|
crate::gateway::fetch_all_project_pipeline_statuses(&project_urls, &client).await;
|
||||||
|
|||||||
@@ -28,7 +28,6 @@ pub async fn run_bot(
|
|||||||
watcher_tx: tokio::sync::broadcast::Sender<crate::io::watcher::WatcherEvent>,
|
watcher_tx: tokio::sync::broadcast::Sender<crate::io::watcher::WatcherEvent>,
|
||||||
shutdown_rx: watch::Receiver<Option<crate::rebuild::ShutdownReason>>,
|
shutdown_rx: watch::Receiver<Option<crate::rebuild::ShutdownReason>>,
|
||||||
gateway_active_project: Option<Arc<RwLock<String>>>,
|
gateway_active_project: Option<Arc<RwLock<String>>>,
|
||||||
gateway_project_urls: std::collections::BTreeMap<String, String>,
|
|
||||||
gateway_projects_store: Option<
|
gateway_projects_store: Option<
|
||||||
Arc<
|
Arc<
|
||||||
RwLock<
|
RwLock<
|
||||||
@@ -182,7 +181,17 @@ pub async fn run_bot(
|
|||||||
let announce_room_ids = target_room_ids.clone();
|
let announce_room_ids = target_room_ids.clone();
|
||||||
// Clone values needed by the gateway notification poller (only used in gateway mode).
|
// Clone values needed by the gateway notification poller (only used in gateway mode).
|
||||||
let poller_room_ids: Vec<String> = target_room_ids.iter().map(|r| r.to_string()).collect();
|
let poller_room_ids: Vec<String> = 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<String, String> =
|
||||||
|
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_poll_interval = config.aggregated_notifications_poll_interval_secs;
|
||||||
let poller_enabled = config.aggregated_notifications_enabled;
|
let poller_enabled = config.aggregated_notifications_enabled;
|
||||||
|
|
||||||
@@ -321,7 +330,6 @@ pub async fn run_bot(
|
|||||||
transport: Arc::clone(&transport),
|
transport: Arc::clone(&transport),
|
||||||
timer_store,
|
timer_store,
|
||||||
gateway_active_project,
|
gateway_active_project,
|
||||||
gateway_project_urls,
|
|
||||||
gateway_projects_store,
|
gateway_projects_store,
|
||||||
handled_incoming_event_ids: Arc::new(TokioMutex::new(super::context::SeenEventIds::new(
|
handled_incoming_event_ids: Arc::new(TokioMutex::new(super::context::SeenEventIds::new(
|
||||||
super::context::SEEN_EVENT_IDS_CAP,
|
super::context::SEEN_EVENT_IDS_CAP,
|
||||||
|
|||||||
@@ -81,7 +81,6 @@ pub fn spawn_bot(
|
|||||||
services: Arc<Services>,
|
services: Arc<Services>,
|
||||||
shutdown_rx: watch::Receiver<Option<ShutdownReason>>,
|
shutdown_rx: watch::Receiver<Option<ShutdownReason>>,
|
||||||
gateway_active_project: Option<Arc<RwLock<String>>>,
|
gateway_active_project: Option<Arc<RwLock<String>>>,
|
||||||
gateway_project_urls: std::collections::BTreeMap<String, String>,
|
|
||||||
gateway_projects_store: Option<
|
gateway_projects_store: Option<
|
||||||
Arc<
|
Arc<
|
||||||
RwLock<
|
RwLock<
|
||||||
@@ -128,7 +127,6 @@ pub fn spawn_bot(
|
|||||||
watcher_tx,
|
watcher_tx,
|
||||||
shutdown_rx,
|
shutdown_rx,
|
||||||
gateway_active_project,
|
gateway_active_project,
|
||||||
gateway_project_urls,
|
|
||||||
gateway_projects_store,
|
gateway_projects_store,
|
||||||
timer_store,
|
timer_store,
|
||||||
gateway_event_rx,
|
gateway_event_rx,
|
||||||
|
|||||||
@@ -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.
|
// Spawn the Matrix bot if `.huskies/bot.toml` exists in the config directory.
|
||||||
let gateway_project_urls: std::collections::BTreeMap<String, String> = 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(
|
let (bot_abort, bot_shutdown_tx) = gateway::io::spawn_gateway_bot(
|
||||||
&config_dir,
|
&config_dir,
|
||||||
Arc::clone(&state_arc.active_project),
|
Arc::clone(&state_arc.active_project),
|
||||||
gateway_project_urls,
|
|
||||||
Arc::clone(&state_arc.projects),
|
Arc::clone(&state_arc.projects),
|
||||||
port,
|
port,
|
||||||
Some(state_arc.event_tx.clone()),
|
Some(state_arc.event_tx.clone()),
|
||||||
|
|||||||
@@ -368,7 +368,6 @@ async fn main() -> Result<(), std::io::Error> {
|
|||||||
Arc::clone(&services),
|
Arc::clone(&services),
|
||||||
matrix_shutdown_rx,
|
matrix_shutdown_rx,
|
||||||
None,
|
None,
|
||||||
std::collections::BTreeMap::new(),
|
|
||||||
None,
|
None,
|
||||||
timer_store_for_bot,
|
timer_store_for_bot,
|
||||||
None,
|
None,
|
||||||
|
|||||||
@@ -504,7 +504,6 @@ pub type ActiveProject = std::sync::Arc<tokio::sync::RwLock<String>>;
|
|||||||
pub fn spawn_gateway_bot(
|
pub fn spawn_gateway_bot(
|
||||||
config_dir: &Path,
|
config_dir: &Path,
|
||||||
active_project: ActiveProject,
|
active_project: ActiveProject,
|
||||||
gateway_project_urls: BTreeMap<String, String>,
|
|
||||||
gateway_projects_store: std::sync::Arc<tokio::sync::RwLock<BTreeMap<String, ProjectEntry>>>,
|
gateway_projects_store: std::sync::Arc<tokio::sync::RwLock<BTreeMap<String, ProjectEntry>>>,
|
||||||
port: u16,
|
port: u16,
|
||||||
gateway_event_tx: Option<tokio::sync::broadcast::Sender<super::GatewayStatusEvent>>,
|
gateway_event_tx: Option<tokio::sync::broadcast::Sender<super::GatewayStatusEvent>>,
|
||||||
@@ -577,7 +576,6 @@ pub fn spawn_gateway_bot(
|
|||||||
services,
|
services,
|
||||||
shutdown_rx,
|
shutdown_rx,
|
||||||
Some(active_project),
|
Some(active_project),
|
||||||
gateway_project_urls,
|
|
||||||
Some(gateway_projects_store),
|
Some(gateway_projects_store),
|
||||||
timer_store,
|
timer_store,
|
||||||
gateway_event_rx,
|
gateway_event_rx,
|
||||||
@@ -608,7 +606,6 @@ mod tests {
|
|||||||
let (handle, shutdown_tx) = spawn_gateway_bot(
|
let (handle, shutdown_tx) = spawn_gateway_bot(
|
||||||
tmp.path(),
|
tmp.path(),
|
||||||
active,
|
active,
|
||||||
std::collections::BTreeMap::new(),
|
|
||||||
projects_store,
|
projects_store,
|
||||||
3001,
|
3001,
|
||||||
Some(event_tx),
|
Some(event_tx),
|
||||||
|
|||||||
@@ -673,18 +673,9 @@ pub async fn save_bot_config_and_restart(state: &GatewayState, content: &str) ->
|
|||||||
if let Some(h) = handle.take() {
|
if let Some(h) = handle.take() {
|
||||||
h.abort();
|
h.abort();
|
||||||
}
|
}
|
||||||
let gateway_project_urls: BTreeMap<String, String> = 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(
|
let (new_handle, new_shutdown_tx) = io::spawn_gateway_bot(
|
||||||
&state.config_dir,
|
&state.config_dir,
|
||||||
Arc::clone(&state.active_project),
|
Arc::clone(&state.active_project),
|
||||||
gateway_project_urls,
|
|
||||||
Arc::clone(&state.projects),
|
Arc::clone(&state.projects),
|
||||||
state.port,
|
state.port,
|
||||||
Some(state.event_tx.clone()),
|
Some(state.event_tx.clone()),
|
||||||
|
|||||||
Reference in New Issue
Block a user