From 75c4a8de33da3b626bd577aad676bffee935c095 Mon Sep 17 00:00:00 2001 From: Huskies Agent Date: Fri, 17 Jul 2026 23:16:40 +0000 Subject: [PATCH] =?UTF-8?q?huskies:=20merge=201203=20bug=20Gateway=20`stat?= =?UTF-8?q?us=20`=20gives=20no=20response=20=E2=80=94=201187=20made=20it?= =?UTF-8?q?=20proxy-only=20with=20no=20local=20resolution=20or=20error=20s?= =?UTF-8?q?urfacing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../matrix/bot/messages/on_room_message.rs | 195 +++++++++++++++++- server/src/service/gateway/aggregation.rs | 125 +++++++++++ server/src/service/gateway/mod.rs | 4 +- 3 files changed, 315 insertions(+), 9 deletions(-) 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 a1ba2424..36431baa 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 @@ -72,8 +72,11 @@ fn extract_rebuild_gateway_command(message: &str, bot_name: &str, bot_user_id: & /// right project before falling back to the generic per-command proxy. /// /// - Empty arg: proxies bare `status` to the active project (pipeline status). -/// - All-digit arg: proxies `status ` to the active project — stories -/// live within whichever project is currently active, not by name. +/// - All-digit arg: resolves locally across *every* registered project's +/// pipeline (story 1203) — a story number is looked up wherever it +/// actually lives, not forwarded blindly to whichever project happens to +/// be active. This avoids "active-project drift": switching projects must +/// not change what `status ` reports for an existing story. /// - Arg matching a known project name: proxies bare `status` to *that* /// project rather than whichever one is active. /// - Anything else: a clear error listing the valid project names. @@ -86,7 +89,7 @@ async fn eval_gateway_status_command( ) -> String { const NO_ACTIVE_PROJECT: &str = "No active project selected or project URL not configured."; - if arg.is_empty() || arg.chars().all(|c| c.is_ascii_digit()) { + if arg.is_empty() { let name = active_project.read().await.clone(); let url = store.read().await.get(&name).and_then(|e| e.url.clone()); return match url { @@ -97,6 +100,10 @@ async fn eval_gateway_status_command( }; } + if arg.chars().all(|c| c.is_ascii_digit()) { + return eval_gateway_numeric_status_lookup(arg, store).await; + } + let known: Vec = store.read().await.keys().cloned().collect(); if known.iter().any(|p| p == arg) { let url = store.read().await.get(arg).and_then(|e| e.url.clone()); @@ -112,6 +119,59 @@ async fn eval_gateway_status_command( format!("Unknown project or story number `{arg}`. Available projects: {available}") } +/// Resolve a numeric `status ` lookup locally across every registered +/// project (story 1203), rather than trusting whichever one is "active". +/// +/// Fetches pipeline items from every project with a configured URL, finds +/// the one whose pipeline actually contains story `n`, and proxies `status +/// ` to that project so the caller gets the full triage card (ACs, +/// worktree, diff, commits) — not just a thin pipeline-item summary. +/// Returns a clear message when no project has a URL configured or no +/// registered project contains the story. +async fn eval_gateway_numeric_status_lookup( + num_str: &str, + store: &tokio::sync::RwLock< + std::collections::BTreeMap, + >, +) -> String { + let project_urls: std::collections::BTreeMap = store + .read() + .await + .iter() + .filter_map(|(name, entry)| entry.url.clone().map(|url| (name.clone(), url))) + .collect(); + + if project_urls.is_empty() { + return "No registered projects have a URL configured — cannot resolve story lookups." + .to_string(); + } + + let client = reqwest::Client::new(); + let items = crate::gateway::fetch_all_project_pipeline_items(&project_urls, &client).await; + + match crate::service::gateway::find_project_containing_story(&items, num_str) { + Some(project_name) => { + let url = project_urls.get(&project_name).cloned().unwrap_or_default(); + super::super::context::BotContext::run_proxy_bot_command(&url, "status", num_str).await + } + None => { + let unreachable: Vec<&str> = items + .iter() + .filter(|(_, v)| v.get("error").is_some()) + .map(|(name, _)| name.as_str()) + .collect(); + if unreachable.is_empty() { + format!("Story **{num_str}** not found in any registered project.") + } else { + format!( + "Story **{num_str}** not found in any registered project. Unreachable: {}.", + unreachable.join(", ") + ) + } + } + } +} + /// Evaluate a gateway-local `overview` command (story 1193). /// /// - Non-empty `args`: unchanged legacy behavior — proxies `overview ` @@ -1460,9 +1520,11 @@ mod tests { } #[tokio::test] - async fn status_numeric_arg_targets_active_project_not_by_name() { - // A project literally named "42" must not shadow story-number routing: - // digits always mean "look up this story on the active project". + async fn status_numeric_arg_with_no_project_urls_gives_clear_message() { + // A project literally named "42" must not shadow story-number routing + // by being matched as a project name — and with no project URLs + // configured at all, the response must say so plainly rather than + // silently failing (AC3). let active = RwLock::new("huskies".to_string()); let store: RwLock> = RwLock::new(BTreeMap::from([ ("huskies".to_string(), project_without_url()), @@ -1471,8 +1533,125 @@ mod tests { let resp = eval_gateway_status_command("42", &active, &store).await; assert!( - resp.contains("No active project selected"), - "numeric arg should resolve via the active project, not the '42' project: {resp}" + resp.contains("No registered projects have a URL configured"), + "numeric arg with no reachable projects should give a clear message: {resp}" + ); + } + + /// Spawn a mock server on one port that answers both the HTTP `/mcp` + /// `get_pipeline_status` call (used to resolve which project owns a + /// story) and the WebSocket `/ws` `bot.command` proxy call (used to fetch + /// the actual status card) — mirroring how a real project container + /// serves both endpoints from the same base URL. + fn spawn_project_mock_server( + pipeline_active_story_ids: &'static [&'static str], + status_card: &'static str, + ) -> String { + use futures::{SinkExt, StreamExt}; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio_tungstenite::tungstenite::Message as WsMsg; + + let std_listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + std_listener.set_nonblocking(true).unwrap(); + let port = std_listener.local_addr().unwrap().port(); + let listener = tokio::net::TcpListener::from_std(std_listener).unwrap(); + + tokio::spawn(async move { + loop { + let Ok((stream, _)) = listener.accept().await else { + break; + }; + tokio::spawn(async move { + let mut peek_buf = [0u8; 8]; + let n = stream.peek(&mut peek_buf).await.unwrap_or(0); + let is_ws_upgrade = peek_buf[..n].starts_with(b"GET"); + + if is_ws_upgrade { + if let Ok(mut ws) = tokio_tungstenite::accept_async(stream).await + && let Some(Ok(WsMsg::Text(text))) = ws.next().await + { + let req: serde_json::Value = serde_json::from_str(&text).unwrap(); + let resp = serde_json::json!({ + "kind": "rpc_response", + "correlation_id": req["correlation_id"], + "ok": true, + "result": { "response": status_card }, + }); + let _ = ws.send(WsMsg::Text(resp.to_string().into())).await; + } + return; + } + + let mut stream = stream; + let mut buf = vec![0u8; 8192]; + let _ = stream.read(&mut buf).await; + + let active: Vec = pipeline_active_story_ids + .iter() + .map(|id| serde_json::json!({ "story_id": id, "name": id, "stage": "current" })) + .collect(); + let pipeline = serde_json::json!({ + "active": active, "backlog": [], "backlog_count": 0, "archived": [] + }); + let body = serde_json::json!({ + "jsonrpc": "2.0", + "id": 1, + "result": { + "content": [{ "type": "text", "text": pipeline.to_string() }] + } + }); + let body_bytes = serde_json::to_vec(&body).unwrap(); + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n", + body_bytes.len() + ); + let _ = stream.write_all(response.as_bytes()).await; + let _ = stream.write_all(&body_bytes).await; + }); + } + }); + + format!("http://127.0.0.1:{port}") + } + + /// AC1 + AC2: a story hosted in a project that is *not* currently active + /// is still found and its status card returned — proving resolution + /// happens locally across all registered projects rather than being + /// proxied blindly to whichever one is active. + #[tokio::test] + async fn status_numeric_arg_resolves_across_non_active_projects() { + let target_url = + spawn_project_mock_server(&["777_story_target"], "## Story 777 — Target\n"); + let active_url = spawn_project_mock_server(&[], "should not be used"); + + let active = RwLock::new("other".to_string()); + let store: RwLock> = RwLock::new(BTreeMap::from([ + ("other".to_string(), ProjectEntry::with_url(active_url)), + ("target".to_string(), ProjectEntry::with_url(target_url)), + ])); + + let resp = eval_gateway_status_command("777", &active, &store).await; + assert_eq!( + resp, "## Story 777 — Target\n", + "should return the status card from the project that actually owns the story: {resp}" + ); + } + + /// AC3: a numeric lookup that matches no registered project's pipeline + /// gives a clear "not found" message rather than failing silently. + #[tokio::test] + async fn status_numeric_arg_not_found_anywhere_gives_clear_message() { + let url = spawn_project_mock_server(&["1_story_other"], "unused"); + let active = RwLock::new("huskies".to_string()); + let store: RwLock> = RwLock::new(BTreeMap::from([( + "huskies".to_string(), + ProjectEntry::with_url(url), + )])); + + let resp = eval_gateway_status_command("999", &active, &store).await; + assert!( + resp.contains("not found in any registered project"), + "unmatched numeric lookup should say so plainly: {resp}" ); } diff --git a/server/src/service/gateway/aggregation.rs b/server/src/service/gateway/aggregation.rs index 0dcc7e04..f609e769 100644 --- a/server/src/service/gateway/aggregation.rs +++ b/server/src/service/gateway/aggregation.rs @@ -125,6 +125,51 @@ pub fn format_overview_compact(items_by_project: &BTreeMap) -> St format!("**Overview: Active Work**\n\n{}", sections.join("\n\n")) } +/// Find the registered project whose pipeline contains a story with the +/// given numeric ID prefix, searching `active`, `backlog`, and `archived` +/// alike so gateway `status ` resolves regardless of which project is +/// currently "active" (story 1203). +/// +/// `items_by_project` is the output of `fetch_all_project_pipeline_items`. +/// Projects whose entry carries an `error` key (unreachable, no URL) are +/// skipped rather than matched. Returns the owning project's name, or `None` +/// if no registered, reachable project has a matching story. +pub fn find_project_containing_story( + items_by_project: &BTreeMap, + num_str: &str, +) -> Option { + for (name, status) in items_by_project { + if status.get("error").is_some() { + continue; + } + for key in ["active", "backlog", "archived"] { + let Some(items) = status.get(key).and_then(|a| a.as_array()) else { + continue; + }; + let found = items.iter().any(|item| { + item.get("story_id") + .and_then(|s| s.as_str()) + .map(|story_id| story_number_prefix(story_id) == num_str) + .unwrap_or(false) + }); + if found { + return Some(name.clone()); + } + } + } + None +} + +/// Extract the leading numeric prefix from a `story_id` like `"42_story_x"`, +/// or `""` if the ID has no purely-digit prefix before the first `_`. +fn story_number_prefix(story_id: &str) -> &str { + story_id + .split('_') + .next() + .filter(|s| !s.is_empty() && s.chars().all(|c| c.is_ascii_digit())) + .unwrap_or("") +} + // ── Tests ──────────────────────────────────────────────────────────────────── #[cfg(test)] @@ -267,4 +312,84 @@ mod tests { // alpha's section should come before beta's (BTreeMap sorted order). assert!(output.find("alpha").unwrap() < output.find("beta").unwrap()); } + + // ── find_project_containing_story ────────────────────────────────────── + + #[test] + fn find_story_matches_active_array() { + let mut items = BTreeMap::new(); + items.insert( + "huskies".to_string(), + json!({ "active": [{ "story_id": "42_story_x", "name": "X", "stage": "current" }], "backlog": [], "archived": [] }), + ); + assert_eq!( + find_project_containing_story(&items, "42"), + Some("huskies".to_string()) + ); + } + + #[test] + fn find_story_matches_backlog_array() { + let mut items = BTreeMap::new(); + items.insert( + "huskies".to_string(), + json!({ "active": [], "backlog": [{ "story_id": "77_story_y", "name": "Y" }], "archived": [] }), + ); + assert_eq!( + find_project_containing_story(&items, "77"), + Some("huskies".to_string()) + ); + } + + #[test] + fn find_story_matches_archived_array() { + let mut items = BTreeMap::new(); + items.insert( + "huskies".to_string(), + json!({ "active": [], "backlog": [], "archived": [{ "story_id": "13_story_z", "name": "Z" }] }), + ); + assert_eq!( + find_project_containing_story(&items, "13"), + Some("huskies".to_string()) + ); + } + + #[test] + fn find_story_returns_none_when_not_present_anywhere() { + let mut items = BTreeMap::new(); + items.insert( + "huskies".to_string(), + json!({ "active": [{ "story_id": "1_story_a", "name": "A" }], "backlog": [], "archived": [] }), + ); + assert_eq!(find_project_containing_story(&items, "999"), None); + } + + #[test] + fn find_story_skips_unreachable_projects() { + let mut items = BTreeMap::new(); + items.insert( + "broken".to_string(), + json!({ "error": "connection refused" }), + ); + items.insert( + "huskies".to_string(), + json!({ "active": [{ "story_id": "5_story_b", "name": "B" }], "backlog": [], "archived": [] }), + ); + assert_eq!( + find_project_containing_story(&items, "5"), + Some("huskies".to_string()) + ); + } + + /// A project literally named after the digits (e.g. "42") must not + /// spuriously match — only `story_id` prefixes are compared. + #[test] + fn find_story_does_not_match_on_project_name() { + let mut items = BTreeMap::new(); + items.insert( + "42".to_string(), + json!({ "active": [{ "story_id": "1_story_a", "name": "A" }], "backlog": [], "archived": [] }), + ); + assert_eq!(find_project_containing_story(&items, "42"), None); + } } diff --git a/server/src/service/gateway/mod.rs b/server/src/service/gateway/mod.rs index 8bc93283..d855d3c1 100644 --- a/server/src/service/gateway/mod.rs +++ b/server/src/service/gateway/mod.rs @@ -19,7 +19,9 @@ pub mod polling; /// Pure signed release-manifest verification (signature, sha256, rollback) — no I/O. pub mod release_manifest; -pub use aggregation::{format_aggregate_status_compact, format_overview_compact}; +pub use aggregation::{ + find_project_containing_story, format_aggregate_status_compact, format_overview_compact, +}; pub use config::{GatewayConfig, ProjectEntry}; pub use identity::{IdentityCheck, check_identity}; pub use io::{