huskies: merge 1203 bug Gateway status gives no response — 1187 made it proxy-only with no local resolution or error surfacing
This commit is contained in:
@@ -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.
|
/// right project before falling back to the generic per-command proxy.
|
||||||
///
|
///
|
||||||
/// - Empty arg: proxies bare `status` to the active project (pipeline status).
|
/// - Empty arg: proxies bare `status` to the active project (pipeline status).
|
||||||
/// - All-digit arg: proxies `status <arg>` to the active project — stories
|
/// - All-digit arg: resolves locally across *every* registered project's
|
||||||
/// live within whichever project is currently active, not by name.
|
/// 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 <n>` reports for an existing story.
|
||||||
/// - Arg matching a known project name: proxies bare `status` to *that*
|
/// - Arg matching a known project name: proxies bare `status` to *that*
|
||||||
/// project rather than whichever one is active.
|
/// project rather than whichever one is active.
|
||||||
/// - Anything else: a clear error listing the valid project names.
|
/// - Anything else: a clear error listing the valid project names.
|
||||||
@@ -86,7 +89,7 @@ async fn eval_gateway_status_command(
|
|||||||
) -> String {
|
) -> String {
|
||||||
const NO_ACTIVE_PROJECT: &str = "No active project selected or project URL not configured.";
|
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 name = active_project.read().await.clone();
|
||||||
let url = store.read().await.get(&name).and_then(|e| e.url.clone());
|
let url = store.read().await.get(&name).and_then(|e| e.url.clone());
|
||||||
return match url {
|
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<String> = store.read().await.keys().cloned().collect();
|
let known: Vec<String> = store.read().await.keys().cloned().collect();
|
||||||
if known.iter().any(|p| p == arg) {
|
if known.iter().any(|p| p == arg) {
|
||||||
let url = store.read().await.get(arg).and_then(|e| e.url.clone());
|
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}")
|
format!("Unknown project or story number `{arg}`. Available projects: {available}")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Resolve a numeric `status <n>` 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
|
||||||
|
/// <n>` 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, crate::service::gateway::config::ProjectEntry>,
|
||||||
|
>,
|
||||||
|
) -> String {
|
||||||
|
let project_urls: std::collections::BTreeMap<String, String> = 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).
|
/// Evaluate a gateway-local `overview` command (story 1193).
|
||||||
///
|
///
|
||||||
/// - Non-empty `args`: unchanged legacy behavior — proxies `overview <args>`
|
/// - Non-empty `args`: unchanged legacy behavior — proxies `overview <args>`
|
||||||
@@ -1460,9 +1520,11 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn status_numeric_arg_targets_active_project_not_by_name() {
|
async fn status_numeric_arg_with_no_project_urls_gives_clear_message() {
|
||||||
// A project literally named "42" must not shadow story-number routing:
|
// A project literally named "42" must not shadow story-number routing
|
||||||
// digits always mean "look up this story on the active project".
|
// 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 active = RwLock::new("huskies".to_string());
|
||||||
let store: RwLock<BTreeMap<String, ProjectEntry>> = RwLock::new(BTreeMap::from([
|
let store: RwLock<BTreeMap<String, ProjectEntry>> = RwLock::new(BTreeMap::from([
|
||||||
("huskies".to_string(), project_without_url()),
|
("huskies".to_string(), project_without_url()),
|
||||||
@@ -1471,8 +1533,125 @@ mod tests {
|
|||||||
|
|
||||||
let resp = eval_gateway_status_command("42", &active, &store).await;
|
let resp = eval_gateway_status_command("42", &active, &store).await;
|
||||||
assert!(
|
assert!(
|
||||||
resp.contains("No active project selected"),
|
resp.contains("No registered projects have a URL configured"),
|
||||||
"numeric arg should resolve via the active project, not the '42' project: {resp}"
|
"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<serde_json::Value> = 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<BTreeMap<String, ProjectEntry>> = 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<BTreeMap<String, ProjectEntry>> = 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}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -125,6 +125,51 @@ pub fn format_overview_compact(items_by_project: &BTreeMap<String, Value>) -> St
|
|||||||
format!("**Overview: Active Work**\n\n{}", sections.join("\n\n"))
|
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 <n>` 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<String, Value>,
|
||||||
|
num_str: &str,
|
||||||
|
) -> Option<String> {
|
||||||
|
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 ────────────────────────────────────────────────────────────────────
|
// ── Tests ────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
@@ -267,4 +312,84 @@ mod tests {
|
|||||||
// alpha's section should come before beta's (BTreeMap sorted order).
|
// alpha's section should come before beta's (BTreeMap sorted order).
|
||||||
assert!(output.find("alpha").unwrap() < output.find("beta").unwrap());
|
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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -19,7 +19,9 @@ pub mod polling;
|
|||||||
/// Pure signed release-manifest verification (signature, sha256, rollback) — no I/O.
|
/// Pure signed release-manifest verification (signature, sha256, rollback) — no I/O.
|
||||||
pub mod release_manifest;
|
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 config::{GatewayConfig, ProjectEntry};
|
||||||
pub use identity::{IdentityCheck, check_identity};
|
pub use identity::{IdentityCheck, check_identity};
|
||||||
pub use io::{
|
pub use io::{
|
||||||
|
|||||||
Reference in New Issue
Block a user