huskies: merge 1122 story Chat-bot switch command reads stale gateway_projects Vec instead of live gateway_projects_store
This commit is contained in:
@@ -88,9 +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: valid project names accepted by the `switch` command.
|
|
||||||
/// Empty in standalone mode.
|
|
||||||
pub gateway_projects: Vec<String>,
|
|
||||||
/// In gateway mode: mapping of project name → base URL (e.g. `"http://localhost:3001"`).
|
/// 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`).
|
/// Used to proxy bot commands to the active project over WebSocket (`/ws`).
|
||||||
/// Empty in standalone mode.
|
/// Empty in standalone mode.
|
||||||
@@ -283,7 +280,6 @@ 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_projects: Vec<String>,
|
|
||||||
gateway_project_urls: BTreeMap<String, String>,
|
gateway_project_urls: BTreeMap<String, String>,
|
||||||
) -> BotContext {
|
) -> BotContext {
|
||||||
BotContext {
|
BotContext {
|
||||||
@@ -304,7 +300,6 @@ mod tests {
|
|||||||
std::path::PathBuf::from("/tmp/timers.json"),
|
std::path::PathBuf::from("/tmp/timers.json"),
|
||||||
)),
|
)),
|
||||||
gateway_active_project,
|
gateway_active_project,
|
||||||
gateway_projects,
|
|
||||||
gateway_project_urls,
|
gateway_project_urls,
|
||||||
gateway_projects_store: None,
|
gateway_projects_store: None,
|
||||||
pending_pipeline_events: Arc::new(TokioMutex::new(Vec::new())),
|
pending_pipeline_events: Arc::new(TokioMutex::new(Vec::new())),
|
||||||
@@ -325,7 +320,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, vec![], BTreeMap::new());
|
let ctx = test_bot_context(services, None, BTreeMap::new());
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
ctx.effective_project_root().await,
|
ctx.effective_project_root().await,
|
||||||
PathBuf::from("/projects/myapp")
|
PathBuf::from("/projects/myapp")
|
||||||
@@ -339,7 +334,6 @@ mod tests {
|
|||||||
let ctx = test_bot_context(
|
let ctx = test_bot_context(
|
||||||
services,
|
services,
|
||||||
Some(Arc::clone(&active)),
|
Some(Arc::clone(&active)),
|
||||||
vec!["huskies".into(), "robot-studio".into()],
|
|
||||||
BTreeMap::from([
|
BTreeMap::from([
|
||||||
("huskies".into(), "http://localhost:3001".into()),
|
("huskies".into(), "http://localhost:3001".into()),
|
||||||
("robot-studio".into(), "http://localhost:3002".into()),
|
("robot-studio".into(), "http://localhost:3002".into()),
|
||||||
@@ -358,7 +352,6 @@ mod tests {
|
|||||||
let ctx = test_bot_context(
|
let ctx = test_bot_context(
|
||||||
services,
|
services,
|
||||||
Some(Arc::clone(&active)),
|
Some(Arc::clone(&active)),
|
||||||
vec!["huskies".into(), "robot-studio".into()],
|
|
||||||
BTreeMap::from([
|
BTreeMap::from([
|
||||||
("huskies".into(), "http://localhost:3001".into()),
|
("huskies".into(), "http://localhost:3001".into()),
|
||||||
("robot-studio".into(), "http://localhost:3002".into()),
|
("robot-studio".into(), "http://localhost:3002".into()),
|
||||||
@@ -439,7 +432,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, vec![], BTreeMap::new());
|
let ctx = test_bot_context(services, None, BTreeMap::new());
|
||||||
let _cloned = ctx.clone();
|
let _cloned = ctx.clone();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -489,7 +482,6 @@ mod tests {
|
|||||||
let ctx = test_bot_context(
|
let ctx = test_bot_context(
|
||||||
services,
|
services,
|
||||||
Some(Arc::clone(&active)),
|
Some(Arc::clone(&active)),
|
||||||
vec!["huskies".into()],
|
|
||||||
BTreeMap::from([("huskies".into(), base_url)]),
|
BTreeMap::from([("huskies".into(), base_url)]),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -19,6 +19,31 @@ use super::super::verification::check_sender_verified;
|
|||||||
|
|
||||||
use super::handle_message;
|
use super::handle_message;
|
||||||
|
|
||||||
|
/// Evaluate a `switch <arg>` 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<String>,
|
||||||
|
store: &tokio::sync::RwLock<
|
||||||
|
std::collections::BTreeMap<String, crate::service::gateway::config::ProjectEntry>,
|
||||||
|
>,
|
||||||
|
) -> String {
|
||||||
|
let projects: Vec<String> = store.read().await.keys().cloned().collect();
|
||||||
|
if arg.is_empty() {
|
||||||
|
let available = projects.join(", ");
|
||||||
|
format!("Usage: `switch <project>`. 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(
|
pub(in crate::chat::transport::matrix::bot) async fn on_room_message(
|
||||||
ev: OriginalSyncRoomMessageEvent,
|
ev: OriginalSyncRoomMessageEvent,
|
||||||
room: Room,
|
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") {
|
if cmd.eq_ignore_ascii_case("switch") {
|
||||||
let response = if arg.is_empty() {
|
let response = if let Some(ref store) = ctx.gateway_projects_store {
|
||||||
let available = ctx.gateway_projects.join(", ");
|
eval_switch_command(&arg, active_project, store).await
|
||||||
format!("Usage: `switch <project>`. 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}**.")
|
|
||||||
} else {
|
} else {
|
||||||
let available = ctx.gateway_projects.join(", ");
|
"Switch is unavailable: project store not initialised.".to_string()
|
||||||
format!("Unknown project `{arg}`. Available: {available}")
|
|
||||||
};
|
};
|
||||||
let html = markdown_to_html(&response);
|
let html = markdown_to_html(&response);
|
||||||
if let Ok(msg_id) = ctx
|
if let Ok(msg_id) = ctx
|
||||||
@@ -704,3 +723,80 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message(
|
|||||||
.chat_dispatcher
|
.chat_dispatcher
|
||||||
.submit(room_id_str, user_message, factory);
|
.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<BTreeMap<String, ProjectEntry>> = 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<BTreeMap<String, ProjectEntry>> = 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}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -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_projects: Vec<String>,
|
|
||||||
gateway_project_urls: std::collections::BTreeMap<String, String>,
|
gateway_project_urls: std::collections::BTreeMap<String, String>,
|
||||||
gateway_projects_store: Option<
|
gateway_projects_store: Option<
|
||||||
Arc<
|
Arc<
|
||||||
@@ -404,7 +403,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_projects,
|
|
||||||
gateway_project_urls,
|
gateway_project_urls,
|
||||||
gateway_projects_store,
|
gateway_projects_store,
|
||||||
pending_pipeline_events,
|
pending_pipeline_events,
|
||||||
|
|||||||
@@ -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_projects: Vec<String>,
|
|
||||||
gateway_project_urls: std::collections::BTreeMap<String, String>,
|
gateway_project_urls: std::collections::BTreeMap<String, String>,
|
||||||
gateway_projects_store: Option<
|
gateway_projects_store: Option<
|
||||||
Arc<
|
Arc<
|
||||||
@@ -129,7 +128,6 @@ pub fn spawn_bot(
|
|||||||
watcher_tx,
|
watcher_tx,
|
||||||
shutdown_rx,
|
shutdown_rx,
|
||||||
gateway_active_project,
|
gateway_active_project,
|
||||||
gateway_projects,
|
|
||||||
gateway_project_urls,
|
gateway_project_urls,
|
||||||
gateway_projects_store,
|
gateway_projects_store,
|
||||||
timer_store,
|
timer_store,
|
||||||
|
|||||||
@@ -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.
|
// Spawn the Matrix bot if `.huskies/bot.toml` exists in the config directory.
|
||||||
let gateway_projects: Vec<String> = state_arc.projects.read().await.keys().cloned().collect();
|
|
||||||
let gateway_project_urls: std::collections::BTreeMap<String, String> = state_arc
|
let gateway_project_urls: std::collections::BTreeMap<String, String> = state_arc
|
||||||
.projects
|
.projects
|
||||||
.read()
|
.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(
|
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_projects,
|
|
||||||
gateway_project_urls,
|
gateway_project_urls,
|
||||||
Arc::clone(&state_arc.projects),
|
Arc::clone(&state_arc.projects),
|
||||||
port,
|
port,
|
||||||
|
|||||||
@@ -364,7 +364,6 @@ async fn main() -> Result<(), std::io::Error> {
|
|||||||
Arc::clone(&services),
|
Arc::clone(&services),
|
||||||
matrix_shutdown_rx,
|
matrix_shutdown_rx,
|
||||||
None,
|
None,
|
||||||
vec![],
|
|
||||||
std::collections::BTreeMap::new(),
|
std::collections::BTreeMap::new(),
|
||||||
None,
|
None,
|
||||||
timer_store_for_bot,
|
timer_store_for_bot,
|
||||||
|
|||||||
@@ -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_projects: Vec<String>,
|
|
||||||
gateway_project_urls: BTreeMap<String, String>,
|
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,
|
||||||
@@ -578,7 +577,6 @@ pub fn spawn_gateway_bot(
|
|||||||
services,
|
services,
|
||||||
shutdown_rx,
|
shutdown_rx,
|
||||||
Some(active_project),
|
Some(active_project),
|
||||||
gateway_projects,
|
|
||||||
gateway_project_urls,
|
gateway_project_urls,
|
||||||
Some(gateway_projects_store),
|
Some(gateway_projects_store),
|
||||||
timer_store,
|
timer_store,
|
||||||
@@ -610,7 +608,6 @@ mod tests {
|
|||||||
let (handle, shutdown_tx) = spawn_gateway_bot(
|
let (handle, shutdown_tx) = spawn_gateway_bot(
|
||||||
tmp.path(),
|
tmp.path(),
|
||||||
active,
|
active,
|
||||||
vec!["proj".to_string()],
|
|
||||||
std::collections::BTreeMap::new(),
|
std::collections::BTreeMap::new(),
|
||||||
projects_store,
|
projects_store,
|
||||||
3001,
|
3001,
|
||||||
|
|||||||
@@ -647,7 +647,6 @@ 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_projects: Vec<String> = state.projects.read().await.keys().cloned().collect();
|
|
||||||
let gateway_project_urls: BTreeMap<String, String> = state
|
let gateway_project_urls: BTreeMap<String, String> = state
|
||||||
.projects
|
.projects
|
||||||
.read()
|
.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(
|
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_projects,
|
|
||||||
gateway_project_urls,
|
gateway_project_urls,
|
||||||
Arc::clone(&state.projects),
|
Arc::clone(&state.projects),
|
||||||
state.port,
|
state.port,
|
||||||
|
|||||||
Reference in New Issue
Block a user