From 80efb7fcfcff4709479dc2e5d33b939df75068a8 Mon Sep 17 00:00:00 2001 From: Huskies Agent Date: Fri, 17 Jul 2026 19:36:17 +0000 Subject: [PATCH] huskies: merge 1201 story Chat notification when a new work item is filed --- server/src/http/mcp/story_tools/bug.rs | 3 +- server/src/http/mcp/story_tools/epic.rs | 11 +- server/src/http/mcp/story_tools/mod.rs | 16 +- server/src/http/mcp/story_tools/refactor.rs | 3 +- server/src/http/mcp/story_tools/spike.rs | 3 +- .../src/http/mcp/story_tools/story/create.rs | 26 +- server/src/io/watcher/events.rs | 5 +- server/src/service/notifications/events.rs | 1 + server/src/service/notifications/filter.rs | 6 + server/src/service/notifications/format.rs | 913 ------------------ .../src/service/notifications/format/mod.rs | 432 +++++++++ .../src/service/notifications/format/tests.rs | 674 +++++++++++++ .../src/service/notifications/io/listener.rs | 99 +- .../notifications/io/tests_notifications.rs | 139 ++- 14 files changed, 1390 insertions(+), 941 deletions(-) delete mode 100644 server/src/service/notifications/format.rs create mode 100644 server/src/service/notifications/format/mod.rs create mode 100644 server/src/service/notifications/format/tests.rs diff --git a/server/src/http/mcp/story_tools/bug.rs b/server/src/http/mcp/story_tools/bug.rs index 6cd18874..c06e7e01 100644 --- a/server/src/http/mcp/story_tools/bug.rs +++ b/server/src/http/mcp/story_tools/bug.rs @@ -28,7 +28,7 @@ pub(crate) fn tool_create_bug(args: &Value, ctx: &AppContext) -> Result Result Result Result Result { +/// Returns `(origin_json, origin_label)` on success: the canonical origin +/// JSON string (for `crdt_state::set_origin`) alongside a human-readable +/// `"{kind} {id}"` label (for creation-notification display, story 1201). +/// Returns `Err` with a human-readable explanation when the caller failed to +/// identify itself; the caller (`tool_create_*` handlers) must propagate the +/// error without creating the work item, so a missing-attribution call +/// leaves no half-state behind. +pub(super) fn build_origin(args: &serde_json::Value) -> Result<(String, String), String> { let ts = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() @@ -57,7 +60,8 @@ pub(super) fn build_origin(args: &serde_json::Value) -> Result { .unwrap_or("user"); let ts_val = origin_obj.get("ts").and_then(|v| v.as_f64()).unwrap_or(ts); - Ok(serde_json::json!({"kind": kind, "id": id, "ts": ts_val}).to_string()) + let origin_json = serde_json::json!({"kind": kind, "id": id, "ts": ts_val}).to_string(); + Ok((origin_json, format!("{kind} {id}"))) } pub(crate) use bug::{tool_close_bug, tool_create_bug, tool_list_bugs}; diff --git a/server/src/http/mcp/story_tools/refactor.rs b/server/src/http/mcp/story_tools/refactor.rs index c51163fd..df7475b2 100644 --- a/server/src/http/mcp/story_tools/refactor.rs +++ b/server/src/http/mcp/story_tools/refactor.rs @@ -29,7 +29,7 @@ pub(crate) fn tool_create_refactor(args: &Value, ctx: &AppContext) -> Result Result Result Result Result Result &'static str { - match stage { - Stage::Upcoming => "Upcoming", - Stage::Backlog => "Backlog", - Stage::Coding { .. } => "Current", - Stage::Blocked { .. } => "Blocked", - Stage::Qa => "QA", - Stage::Merge { .. } => "Merge", - Stage::Done { .. } => "Done", - Stage::Archived { .. } => "Archived", - Stage::MergeFailure { .. } => "MergeFailure", - Stage::MergeFailureFinal { .. } => "MergeFailureFinal", - Stage::Frozen { .. } => "Frozen", - Stage::ReviewHold { .. } => "ReviewHold", - Stage::Abandoned { .. } => "Abandoned", - Stage::Superseded { .. } => "Superseded", - Stage::Rejected { .. } => "Rejected", - } -} - -/// Format a stage transition notification message. -/// -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_stage_notification( - item_id: &str, - story_name: &str, - from_stage: &Stage, - to_stage: &Stage, -) -> (String, String) { - let number = extract_item_number(item_id).unwrap_or(item_id); - let effective_name = if story_name.is_empty() { - None - } else { - Some(story_name) - }; - let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); - let name_html = effective_name - .map(|n| format!("{n} ")) - .unwrap_or_default(); - - let from_display = stage_display_name(from_stage); - let to_display = stage_display_name(to_stage); - let prefix = if matches!(to_stage, Stage::Done { .. }) { - "\u{1f389} " - } else { - "" - }; - let plain = - format!("{prefix}#{number} {name_plain}\u{2014} {from_display} \u{2192} {to_display}"); - let html = format!( - "{prefix}#{number} {name_html}\u{2014} {from_display} \u{2192} {to_display}" - ); - (plain, html) -} - -/// Format an error notification message for a story merge failure. -/// -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_error_notification( - item_id: &str, - story_name: &str, - reason: &str, -) -> (String, String) { - let number = extract_item_number(item_id).unwrap_or(item_id); - let effective_name = if story_name.is_empty() { - None - } else { - Some(story_name) - }; - let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); - let name_html = effective_name - .map(|n| format!("{n} ")) - .unwrap_or_default(); - - let plain = format!("\u{274c} #{number} {name_plain}\u{2014} {reason}"); - let html = format!("\u{274c} #{number} {name_html}\u{2014} {reason}"); - (plain, html) -} - -/// Format a blocked-story notification message. -/// -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_blocked_notification( - item_id: &str, - story_name: &str, - reason: &str, -) -> (String, String) { - let number = extract_item_number(item_id).unwrap_or(item_id); - let effective_name = if story_name.is_empty() { - None - } else { - Some(story_name) - }; - let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); - let name_html = effective_name - .map(|n| format!("{n} ")) - .unwrap_or_default(); - - let plain = format!("\u{1f6ab} #{number} {name_plain}\u{2014} BLOCKED: {reason}"); - let html = - format!("\u{1f6ab} #{number} {name_html}\u{2014} BLOCKED: {reason}"); - (plain, html) -} - -/// Format a rate limit warning notification message. -/// -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_rate_limit_notification( - item_id: &str, - story_name: &str, - agent_name: &str, -) -> (String, String) { - let number = extract_item_number(item_id).unwrap_or(item_id); - let effective_name = if story_name.is_empty() { - None - } else { - Some(story_name) - }; - let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); - let name_html = effective_name - .map(|n| format!("{n} ")) - .unwrap_or_default(); - - let plain = format!( - "\u{26a0}\u{fe0f} #{number} {name_plain}\u{2014} {agent_name} hit an API rate limit" - ); - let html = format!( - "\u{26a0}\u{fe0f} #{number} {name_html}\u{2014} \ - {agent_name} hit an API rate limit" - ); - (plain, html) -} - -/// Format an OAuth account-swap notification message. -/// -/// Sent when the pool successfully rotates to a new account after a rate-limit. -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_oauth_account_swapped(new_email: &str) -> (String, String) { - let plain = format!("\u{1f504} OAuth account rotated \u{2014} now using {new_email}"); - let html = - format!("\u{1f504} OAuth account rotated \u{2014} now using {new_email}"); - (plain, html) -} - -/// Format an OAuth accounts-exhausted notification message. -/// -/// Sent when all pool accounts are rate-limited and no swap was possible. -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_oauth_accounts_exhausted(earliest_reset_msg: &str) -> (String, String) { - let plain = format!("\u{26d4} {earliest_reset_msg}"); - let html = format!("\u{26d4} {earliest_reset_msg}"); - (plain, html) -} - -/// Format an agent-started notification message. -/// -/// Sent when an agent transitions to the Running state. -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_agent_started_notification( - item_id: &str, - story_name: &str, - agent_name: &str, -) -> (String, String) { - let number = extract_item_number(item_id).unwrap_or(item_id); - let effective_name = if story_name.is_empty() { - None - } else { - Some(story_name) - }; - let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); - let name_html = effective_name - .map(|n| format!("{n} ")) - .unwrap_or_default(); - - let plain = format!("\u{1F916} #{number} {name_plain}\u{2014} {agent_name} started"); - let html = - format!("\u{1F916} #{number} {name_html}\u{2014} {agent_name} started"); - (plain, html) -} - -/// Format an agent-completed notification message. -/// -/// Sent when an agent finishes processing a story (gates passed or failed). -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_agent_completed_notification( - item_id: &str, - story_name: &str, - agent_name: &str, - success: bool, -) -> (String, String) { - let number = extract_item_number(item_id).unwrap_or(item_id); - let effective_name = if story_name.is_empty() { - None - } else { - Some(story_name) - }; - let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); - let name_html = effective_name - .map(|n| format!("{n} ")) - .unwrap_or_default(); - - let (emoji, result) = if success { - ("\u{2705}", "completed") // ✅ - } else { - ("\u{274C}", "failed") // ❌ - }; - let plain = format!("{emoji} #{number} {name_plain}\u{2014} {agent_name} {result}"); - let html = - format!("{emoji} #{number} {name_html}\u{2014} {agent_name} {result}"); - (plain, html) -} - -/// Format a new-work-item creation notification. -/// -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_new_item_notification( - item_id: &str, - item_type: &str, - name: &str, -) -> (String, String) { - let number = extract_item_number(item_id).unwrap_or(item_id); - let emoji = match item_type { - "bug" => "\u{1f41b}", // 🐛 - "refactor" => "\u{1f4dd}", // 📝 - "spike" => "\u{1f52c}", // 🔬 - _ => "\u{1f4d6}", // 📖 (story and unknown) - }; - let plain = format!("{emoji} New {item_type} #{number} \u{2014} {name}"); - let html = format!("{emoji} New {item_type} #{number} \u{2014} {name}"); - (plain, html) -} - -/// Format a merge-auto-retry notification message. -/// -/// Sent when a `GatesFailed` merge failure is automatically retried after a -/// delay (story 1185). Returns `(plain_text, html)` suitable for -/// `ChatTransport::send_message`. -pub fn format_merge_auto_retry_notification( - item_id: &str, - story_name: &str, - attempt: u32, - budget: u32, -) -> (String, String) { - let number = extract_item_number(item_id).unwrap_or(item_id); - let effective_name = if story_name.is_empty() { - None - } else { - Some(story_name) - }; - let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); - let name_html = effective_name - .map(|n| format!("{n} ")) - .unwrap_or_default(); - - let plain = format!( - "\u{1f504} #{number} {name_plain}\u{2014} auto-retrying merge (attempt {attempt}/{budget})" - ); - let html = format!( - "\u{1f504} #{number} {name_html}\u{2014} auto-retrying merge \ - (attempt {attempt}/{budget})" - ); - (plain, html) -} - -/// Format a low-disk-space warning notification message (story 1200 AC3). -/// -/// Includes free space, `target/` and `.huskies/worktrees/` directory sizes, -/// and names the `gc` tool as the first remediation step. -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_disk_warning_notification( - level: &str, - host_id: &str, - free_bytes: u64, - target_bytes: u64, - worktrees_bytes: u64, -) -> (String, String) { - let emoji = if level == "critical" { - "\u{1f6a8}" // 🚨 - } else { - "\u{26a0}\u{fe0f}" // ⚠️ - }; - let free_gb = bytes_to_gb(free_bytes); - let target_gb = bytes_to_gb(target_bytes); - let worktrees_gb = bytes_to_gb(worktrees_bytes); - let plain = format!( - "{emoji} Low disk space on {host_id} ({level}): {free_gb:.1}GB free \ - (target/ {target_gb:.1}GB, worktrees/ {worktrees_gb:.1}GB) \ - \u{2014} first response: run the `gc` tool to reclaim space" - ); - let html = format!( - "{emoji} Low disk space on {host_id} ({level}): {free_gb:.1}GB free \ - (target/ {target_gb:.1}GB, worktrees/ {worktrees_gb:.1}GB) \ - \u{2014} first response: run the gc tool to reclaim space" - ); - (plain, html) -} - -/// Format a disk-space-recovered notification message (story 1200 AC2). -/// -/// Sent once when free space climbs back above the configured recovery -/// margin after a warn/critical warning. -/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. -pub fn format_disk_recovery_notification(host_id: &str, free_bytes: u64) -> (String, String) { - let free_gb = bytes_to_gb(free_bytes); - let plain = format!("\u{2705} Disk space recovered on {host_id}: {free_gb:.1}GB free"); - let html = - format!("\u{2705} Disk space recovered on {host_id}: {free_gb:.1}GB free"); - (plain, html) -} - -/// Convert a byte count to gigabytes for display (story 1200). -fn bytes_to_gb(bytes: u64) -> f64 { - bytes as f64 / 1_000_000_000.0 -} - -/// Maximum number of trailing gate-output lines included in a merge-failure -/// chat notification. -/// -/// Gate output can be hundreds of lines; only the tail (where errors appear) -/// is useful at a glance. Full output remains available via `get_merge_status` -/// or the web UI — this limit is chat-display-only. -pub const MERGE_FAILURE_TAIL_LINES: usize = 30; - -/// Truncate `gate_output` to its last `max_lines` lines for chat notifications. -/// -/// If the output contains more than `max_lines` non-empty lines, a leading -/// marker line `[...output truncated, last N lines shown...]` is prepended to -/// the tail so readers know output was cut. If the output fits within the -/// limit it is returned unchanged (no marker added). -pub fn truncate_gate_output(gate_output: &str, max_lines: usize) -> String { - let lines: Vec<&str> = gate_output.lines().collect(); - if lines.len() <= max_lines { - return gate_output.to_string(); - } - let tail = &lines[lines.len() - max_lines..]; - let marker = format!("[...output truncated, last {max_lines} lines shown...]"); - format!("{marker}\n{}", tail.join("\n")) -} - -#[cfg(test)] -mod tests { - use super::*; - - // ── stage_display_name ──────────────────────────────────────────────────── - - fn done_stage() -> Stage { - Stage::from_dir("done").unwrap() - } - fn merge_stage() -> Stage { - Stage::from_dir("merge").unwrap() - } - - #[test] - fn stage_display_name_maps_all_known_stages() { - assert_eq!(stage_display_name(&Stage::Backlog), "Backlog"); - assert_eq!( - stage_display_name(&Stage::Coding { - claim: None, - plan: Default::default(), - retries: 0, - }), - "Current" - ); - assert_eq!(stage_display_name(&Stage::Qa), "QA"); - assert_eq!(stage_display_name(&merge_stage()), "Merge"); - assert_eq!(stage_display_name(&done_stage()), "Done"); - assert_eq!( - stage_display_name(&Stage::from_dir("archived").unwrap()), - "Archived" - ); - assert_eq!(stage_display_name(&Stage::Upcoming), "Upcoming"); - } - - // ── format_stage_notification ───────────────────────────────────────────── - - #[test] - fn format_notification_done_stage_includes_party_emoji() { - let (plain, html) = format_stage_notification( - "353_story_done", - "Done Story", - &merge_stage(), - &done_stage(), - ); - assert_eq!( - plain, - "\u{1f389} #353 Done Story \u{2014} Merge \u{2192} Done" - ); - assert_eq!( - html, - "\u{1f389} #353 Done Story \u{2014} Merge \u{2192} Done" - ); - } - - #[test] - fn format_notification_non_done_stage_has_no_emoji() { - let (plain, _html) = format_stage_notification( - "42_story_thing", - "Some Story", - &Stage::Backlog, - &Stage::Coding { - claim: None, - plan: Default::default(), - retries: 0, - }, - ); - assert!(!plain.contains("\u{1f389}")); - } - - #[test] - fn format_notification_with_story_name() { - let (plain, html) = format_stage_notification( - "261_story_bot_notifications", - "Bot notifications", - &Stage::Upcoming, - &Stage::Coding { - claim: None, - plan: Default::default(), - retries: 0, - }, - ); - assert_eq!( - plain, - "#261 Bot notifications \u{2014} Upcoming \u{2192} Current" - ); - assert_eq!( - html, - "#261 Bot notifications \u{2014} Upcoming \u{2192} Current" - ); - } - - #[test] - fn format_stage_notification_without_story_name_falls_back_to_number() { - let (plain, html) = format_stage_notification( - "42_bug_fix_thing", - "", - &Stage::Coding { - claim: None, - plan: Default::default(), - retries: 0, - }, - &Stage::Qa, - ); - assert_eq!(plain, "#42 \u{2014} Current \u{2192} QA"); - assert_eq!(html, "#42 \u{2014} Current \u{2192} QA"); - } - - #[test] - fn format_notification_non_numeric_id_uses_full_id() { - let (plain, _html) = - format_stage_notification("abc_story_thing", "Some Story", &Stage::Qa, &merge_stage()); - assert_eq!( - plain, - "#abc_story_thing Some Story \u{2014} QA \u{2192} Merge" - ); - } - - #[test] - fn format_stage_notification_long_name_is_preserved() { - let long_name = "A".repeat(300); - let (plain, _html) = format_stage_notification( - "1_story_long", - &long_name, - &Stage::Coding { - claim: None, - plan: Default::default(), - retries: 0, - }, - &Stage::Qa, - ); - assert!(plain.contains(&long_name)); - } - - #[test] - fn format_stage_notification_empty_story_name_falls_back_to_number() { - let (plain, html) = format_stage_notification( - "42_story_empty", - "", - &Stage::Coding { - claim: None, - plan: Default::default(), - retries: 0, - }, - &Stage::Qa, - ); - assert_eq!(plain, "#42 \u{2014} Current \u{2192} QA"); - assert_eq!(html, "#42 \u{2014} Current \u{2192} QA"); - } - - #[test] - fn format_stage_notification_unicode_name() { - let (plain, html) = format_stage_notification( - "7_story_i18n", - "Ünïcödé Ñämé 🎉", - &Stage::Qa, - &merge_stage(), - ); - assert!(plain.contains("Ünïcödé Ñämé 🎉")); - assert!(html.contains("Ünïcödé Ñämé 🎉")); - } - - // ── format_error_notification ───────────────────────────────────────────── - - #[test] - fn format_error_notification_with_story_name() { - let (plain, html) = format_error_notification( - "262_story_bot_errors", - "Bot error notifications", - "merge conflict in src/main.rs", - ); - assert_eq!( - plain, - "\u{274c} #262 Bot error notifications \u{2014} merge conflict in src/main.rs" - ); - assert_eq!( - html, - "\u{274c} #262 Bot error notifications \u{2014} merge conflict in src/main.rs" - ); - } - - #[test] - fn format_error_notification_without_story_name_falls_back_to_number() { - let (plain, html) = format_error_notification("42_bug_fix_thing", "", "tests failed"); - assert_eq!(plain, "\u{274c} #42 \u{2014} tests failed"); - assert_eq!(html, "\u{274c} #42 \u{2014} tests failed"); - } - - #[test] - fn format_error_notification_non_numeric_id_uses_full_id() { - let (plain, _html) = - format_error_notification("abc_story_thing", "Some Story", "clippy errors"); - assert_eq!( - plain, - "\u{274c} #abc_story_thing Some Story \u{2014} clippy errors" - ); - } - - #[test] - fn format_error_notification_long_reason_preserved() { - let long_reason = "x".repeat(500); - let (plain, _html) = format_error_notification("1_story_foo", "", &long_reason); - assert!(plain.contains(&long_reason)); - } - - #[test] - fn format_error_notification_unicode_reason() { - let (plain, _html) = format_error_notification("5_story_foo", "Foo", "错误：合并冲突"); - assert!(plain.contains("错误：合并冲突")); - } - - #[test] - fn format_error_notification_empty_story_name_falls_back_to_number() { - let (plain, _html) = format_error_notification("42_bug_fix_thing", "", "tests failed"); - assert_eq!(plain, "\u{274c} #42 \u{2014} tests failed"); - } - - // ── format_blocked_notification ─────────────────────────────────────────── - - #[test] - fn format_blocked_notification_with_story_name() { - let (plain, html) = format_blocked_notification( - "425_story_blocking_reason", - "Blocking Reason Story", - "Retry limit exceeded (3/3) at coder stage", - ); - assert_eq!( - plain, - "\u{1f6ab} #425 Blocking Reason Story \u{2014} BLOCKED: Retry limit exceeded (3/3) at coder stage" - ); - assert_eq!( - html, - "\u{1f6ab} #425 Blocking Reason Story \u{2014} BLOCKED: Retry limit exceeded (3/3) at coder stage" - ); - } - - #[test] - fn format_blocked_notification_falls_back_to_number() { - let (plain, html) = format_blocked_notification("42_story_thing", "", "empty diff"); - assert_eq!(plain, "\u{1f6ab} #42 \u{2014} BLOCKED: empty diff"); - assert_eq!( - html, - "\u{1f6ab} #42 \u{2014} BLOCKED: empty diff" - ); - } - - #[test] - fn format_blocked_notification_empty_story_name_falls_back_to_number() { - let (plain, _html) = format_blocked_notification("42_story_thing", "", "empty diff"); - assert_eq!(plain, "\u{1f6ab} #42 \u{2014} BLOCKED: empty diff"); - } - - #[test] - fn format_blocked_notification_unicode_reason() { - let (plain, _html) = format_blocked_notification("3_story_x", "X", "理由：空の差分"); - assert!(plain.contains("BLOCKED: 理由：空の差分")); - } - - // ── format_rate_limit_notification ──────────────────────────────────────── - - #[test] - fn format_rate_limit_notification_includes_agent_and_story() { - let (plain, html) = - format_rate_limit_notification("365_story_my_feature", "My Feature", "coder-2"); - assert_eq!( - plain, - "\u{26a0}\u{fe0f} #365 My Feature \u{2014} coder-2 hit an API rate limit" - ); - assert_eq!( - html, - "\u{26a0}\u{fe0f} #365 My Feature \u{2014} coder-2 hit an API rate limit" - ); - } - - #[test] - fn format_rate_limit_notification_falls_back_to_number() { - let (plain, html) = format_rate_limit_notification("42_story_thing", "", "coder-1"); - assert_eq!( - plain, - "\u{26a0}\u{fe0f} #42 \u{2014} coder-1 hit an API rate limit" - ); - assert_eq!( - html, - "\u{26a0}\u{fe0f} #42 \u{2014} coder-1 hit an API rate limit" - ); - } - - #[test] - fn format_rate_limit_notification_empty_story_name_falls_back_to_number() { - let (plain, _html) = format_rate_limit_notification("42_story_thing", "", "coder-1"); - assert_eq!( - plain, - "\u{26a0}\u{fe0f} #42 \u{2014} coder-1 hit an API rate limit" - ); - } - - #[test] - fn format_rate_limit_notification_unicode_agent_name() { - let (plain, _html) = format_rate_limit_notification("9_story_foo", "Foo", "агент-1"); - assert!(plain.contains("агент-1")); - assert!(plain.contains("hit an API rate limit")); - } - - // ── format_agent_started_notification ───────────────────────────────────── - - #[test] - fn format_agent_started_notification_with_story_name() { - let (plain, html) = - format_agent_started_notification("42_story_foo", "My Feature", "coder-1"); - assert_eq!(plain, "\u{1F916} #42 My Feature \u{2014} coder-1 started"); - assert_eq!( - html, - "\u{1F916} #42 My Feature \u{2014} coder-1 started" - ); - } - - #[test] - fn format_agent_started_notification_falls_back_to_number() { - let (plain, html) = format_agent_started_notification("42_story_foo", "", "coder-1"); - assert_eq!(plain, "\u{1F916} #42 \u{2014} coder-1 started"); - assert_eq!( - html, - "\u{1F916} #42 \u{2014} coder-1 started" - ); - } - - #[test] - fn format_agent_started_notification_empty_name_falls_back_to_number() { - let (plain, _html) = format_agent_started_notification("42_story_foo", "", "coder-1"); - assert_eq!(plain, "\u{1F916} #42 \u{2014} coder-1 started"); - } - - // ── format_merge_auto_retry_notification ────────────────────────────────── - - #[test] - fn format_merge_auto_retry_notification_with_story_name() { - let (plain, html) = - format_merge_auto_retry_notification("42_story_foo", "My Feature", 1, 3); - assert_eq!( - plain, - "\u{1f504} #42 My Feature \u{2014} auto-retrying merge (attempt 1/3)" - ); - assert_eq!( - html, - "\u{1f504} #42 My Feature \u{2014} auto-retrying merge \ - (attempt 1/3)" - ); - } - - #[test] - fn format_merge_auto_retry_notification_falls_back_to_number() { - let (plain, html) = format_merge_auto_retry_notification("42_story_foo", "", 2, 3); - assert_eq!( - plain, - "\u{1f504} #42 \u{2014} auto-retrying merge (attempt 2/3)" - ); - assert_eq!( - html, - "\u{1f504} #42 \u{2014} auto-retrying merge (attempt 2/3)" - ); - } - - // ── truncate_gate_output ────────────────────────────────────────────────── - - #[test] - fn truncate_gate_output_short_output_returned_unchanged() { - let output = "line1\nline2\nline3"; - assert_eq!(truncate_gate_output(output, 30), output); - } - - #[test] - fn truncate_gate_output_exact_limit_returned_unchanged() { - let lines: Vec = (1..=30).map(|i| format!("line{i}")).collect(); - let output = lines.join("\n"); - assert_eq!(truncate_gate_output(&output, 30), output); - } - - #[test] - fn truncate_gate_output_over_limit_prepends_marker() { - let lines: Vec = (1..=35).map(|i| format!("line{i}")).collect(); - let output = lines.join("\n"); - let result = truncate_gate_output(&output, 30); - assert!( - result.starts_with("[...output truncated, last 30 lines shown...]"), - "must start with truncation marker; got: {result}" - ); - } - - #[test] - fn truncate_gate_output_over_limit_contains_tail_lines() { - let lines: Vec = (1..=35).map(|i| format!("line{i}")).collect(); - let output = lines.join("\n"); - let result = truncate_gate_output(&output, 30); - // Last 30 lines are line6..line35. - assert!(result.contains("line35"), "must contain last line"); - assert!(result.contains("line6"), "must contain first tail line"); - assert!(!result.contains("line5"), "must not contain dropped line"); - } - - #[test] - fn truncate_gate_output_empty_input_returned_unchanged() { - assert_eq!(truncate_gate_output("", 30), ""); - } - - #[test] - fn truncate_gate_output_single_line_returned_unchanged() { - assert_eq!(truncate_gate_output("only one line", 30), "only one line"); - } - - #[test] - fn truncate_gate_output_marker_contains_configured_limit() { - let lines: Vec = (1..=10).map(|i| format!("x{i}")).collect(); - let output = lines.join("\n"); - let result = truncate_gate_output(&output, 5); - assert!( - result.contains("last 5 lines shown"), - "marker must state configured limit; got: {result}" - ); - } - - // ── format_agent_completed_notification ─────────────────────────────────── - - #[test] - fn format_agent_completed_notification_success_with_story_name() { - let (plain, html) = - format_agent_completed_notification("42_story_foo", "My Feature", "coder-1", true); - assert_eq!(plain, "\u{2705} #42 My Feature \u{2014} coder-1 completed"); - assert_eq!( - html, - "\u{2705} #42 My Feature \u{2014} coder-1 completed" - ); - } - - #[test] - fn format_agent_completed_notification_failure_with_story_name() { - let (plain, _html) = - format_agent_completed_notification("42_story_foo", "My Feature", "coder-1", false); - assert_eq!(plain, "\u{274C} #42 My Feature \u{2014} coder-1 failed"); - } - - #[test] - fn format_agent_completed_notification_falls_back_to_number() { - let (plain, html) = - format_agent_completed_notification("42_story_foo", "", "coder-1", true); - assert_eq!(plain, "\u{2705} #42 \u{2014} coder-1 completed"); - assert_eq!( - html, - "\u{2705} #42 \u{2014} coder-1 completed" - ); - } - - // ── format_new_item_notification ────────────────────────────────────────── - - #[test] - fn format_new_item_notification_story() { - let (plain, html) = - format_new_item_notification("42_story_my_feature", "story", "My Feature"); - assert_eq!(plain, "\u{1f4d6} New story #42 \u{2014} My Feature"); - assert_eq!( - html, - "\u{1f4d6} New story #42 \u{2014} My Feature" - ); - } - - #[test] - fn format_new_item_notification_bug() { - let (plain, html) = - format_new_item_notification("99_bug_login_crash", "bug", "Login Crash"); - assert_eq!(plain, "\u{1f41b} New bug #99 \u{2014} Login Crash"); - assert_eq!( - html, - "\u{1f41b} New bug #99 \u{2014} Login Crash" - ); - } - - #[test] - fn format_new_item_notification_refactor() { - let (plain, html) = format_new_item_notification( - "1075_refactor_split_stage", - "refactor", - "Split Stage enum into Pipeline + Status", - ); - assert_eq!( - plain, - "\u{1f4dd} New refactor #1075 \u{2014} Split Stage enum into Pipeline + Status" - ); - assert_eq!( - html, - "\u{1f4dd} New refactor #1075 \u{2014} Split Stage enum into Pipeline + Status" - ); - } - - #[test] - fn format_new_item_notification_spike() { - let (plain, html) = - format_new_item_notification("7_spike_encoder_comparison", "spike", "Compare Encoders"); - assert_eq!(plain, "\u{1f52c} New spike #7 \u{2014} Compare Encoders"); - assert_eq!( - html, - "\u{1f52c} New spike #7 \u{2014} Compare Encoders" - ); - } - - #[test] - fn format_new_item_notification_non_numeric_id_uses_full_id() { - let (plain, _html) = format_new_item_notification("abc_story_thing", "story", "Some Story"); - assert_eq!( - plain, - "\u{1f4d6} New story #abc_story_thing \u{2014} Some Story" - ); - } - - #[test] - fn format_agent_completed_notification_empty_name_falls_back_to_number() { - let (plain, _html) = - format_agent_completed_notification("42_story_foo", "", "coder-1", false); - assert_eq!(plain, "\u{274C} #42 \u{2014} coder-1 failed"); - } - - // ── format_disk_warning_notification ────────────────────────────────────── - - #[test] - fn format_disk_warning_notification_warn_level_includes_content() { - let (plain, html) = format_disk_warning_notification( - "warn", - "sled-a", - 45_000_000_000, - 30_000_000_000, - 10_000_000_000, - ); - assert!(plain.contains("sled-a")); - assert!(plain.contains("45.0GB free")); - assert!(plain.contains("target/ 30.0GB")); - assert!(plain.contains("worktrees/ 10.0GB")); - assert!(plain.contains("`gc` tool")); - assert!(html.contains("gc tool")); - } - - #[test] - fn format_disk_warning_notification_critical_uses_distinct_emoji() { - let (plain, _html) = - format_disk_warning_notification("critical", "sled-b", 5_000_000_000, 0, 0); - assert!(plain.starts_with("\u{1f6a8}")); - } - - #[test] - fn format_disk_warning_notification_warn_uses_warning_emoji() { - let (plain, _html) = - format_disk_warning_notification("warn", "sled-b", 45_000_000_000, 0, 0); - assert!(plain.starts_with("\u{26a0}\u{fe0f}")); - } - - // ── format_disk_recovery_notification ───────────────────────────────────── - - #[test] - fn format_disk_recovery_notification_includes_host_and_free_space() { - let (plain, html) = format_disk_recovery_notification("sled-a", 60_000_000_000); - assert_eq!( - plain, - "\u{2705} Disk space recovered on sled-a: 60.0GB free" - ); - assert!(html.contains("sled-a")); - assert!(html.contains("60.0GB free")); - } -} diff --git a/server/src/service/notifications/format/mod.rs b/server/src/service/notifications/format/mod.rs new file mode 100644 index 00000000..5e63718c --- /dev/null +++ b/server/src/service/notifications/format/mod.rs @@ -0,0 +1,432 @@ +//! Pure message-formatting functions for pipeline-event notifications. +//! +//! All functions are pure (no I/O, no side effects) and accept only owned +//! or borrowed string data. They return `(plain_text, html)` pairs suitable +//! for `ChatTransport::send_message`. + +use crate::pipeline_state::Stage; +use crate::service::common::item_id::extract_item_number; +use std::path::Path; + +/// Human-readable display name for a typed pipeline [`Stage`]. +pub fn stage_display_name(stage: &Stage) -> &'static str { + match stage { + Stage::Upcoming => "Upcoming", + Stage::Backlog => "Backlog", + Stage::Coding { .. } => "Current", + Stage::Blocked { .. } => "Blocked", + Stage::Qa => "QA", + Stage::Merge { .. } => "Merge", + Stage::Done { .. } => "Done", + Stage::Archived { .. } => "Archived", + Stage::MergeFailure { .. } => "MergeFailure", + Stage::MergeFailureFinal { .. } => "MergeFailureFinal", + Stage::Frozen { .. } => "Frozen", + Stage::ReviewHold { .. } => "ReviewHold", + Stage::Abandoned { .. } => "Abandoned", + Stage::Superseded { .. } => "Superseded", + Stage::Rejected { .. } => "Rejected", + } +} + +/// Format a stage transition notification message. +/// +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_stage_notification( + item_id: &str, + story_name: &str, + from_stage: &Stage, + to_stage: &Stage, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let effective_name = if story_name.is_empty() { + None + } else { + Some(story_name) + }; + let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); + let name_html = effective_name + .map(|n| format!("{n} ")) + .unwrap_or_default(); + + let from_display = stage_display_name(from_stage); + let to_display = stage_display_name(to_stage); + let prefix = if matches!(to_stage, Stage::Done { .. }) { + "\u{1f389} " + } else { + "" + }; + let plain = + format!("{prefix}#{number} {name_plain}\u{2014} {from_display} \u{2192} {to_display}"); + let html = format!( + "{prefix}#{number} {name_html}\u{2014} {from_display} \u{2192} {to_display}" + ); + (plain, html) +} + +/// Format an error notification message for a story merge failure. +/// +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_error_notification( + item_id: &str, + story_name: &str, + reason: &str, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let effective_name = if story_name.is_empty() { + None + } else { + Some(story_name) + }; + let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); + let name_html = effective_name + .map(|n| format!("{n} ")) + .unwrap_or_default(); + + let plain = format!("\u{274c} #{number} {name_plain}\u{2014} {reason}"); + let html = format!("\u{274c} #{number} {name_html}\u{2014} {reason}"); + (plain, html) +} + +/// Format a blocked-story notification message. +/// +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_blocked_notification( + item_id: &str, + story_name: &str, + reason: &str, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let effective_name = if story_name.is_empty() { + None + } else { + Some(story_name) + }; + let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); + let name_html = effective_name + .map(|n| format!("{n} ")) + .unwrap_or_default(); + + let plain = format!("\u{1f6ab} #{number} {name_plain}\u{2014} BLOCKED: {reason}"); + let html = + format!("\u{1f6ab} #{number} {name_html}\u{2014} BLOCKED: {reason}"); + (plain, html) +} + +/// Format a rate limit warning notification message. +/// +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_rate_limit_notification( + item_id: &str, + story_name: &str, + agent_name: &str, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let effective_name = if story_name.is_empty() { + None + } else { + Some(story_name) + }; + let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); + let name_html = effective_name + .map(|n| format!("{n} ")) + .unwrap_or_default(); + + let plain = format!( + "\u{26a0}\u{fe0f} #{number} {name_plain}\u{2014} {agent_name} hit an API rate limit" + ); + let html = format!( + "\u{26a0}\u{fe0f} #{number} {name_html}\u{2014} \ + {agent_name} hit an API rate limit" + ); + (plain, html) +} + +/// Format an OAuth account-swap notification message. +/// +/// Sent when the pool successfully rotates to a new account after a rate-limit. +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_oauth_account_swapped(new_email: &str) -> (String, String) { + let plain = format!("\u{1f504} OAuth account rotated \u{2014} now using {new_email}"); + let html = + format!("\u{1f504} OAuth account rotated \u{2014} now using {new_email}"); + (plain, html) +} + +/// Format an OAuth accounts-exhausted notification message. +/// +/// Sent when all pool accounts are rate-limited and no swap was possible. +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_oauth_accounts_exhausted(earliest_reset_msg: &str) -> (String, String) { + let plain = format!("\u{26d4} {earliest_reset_msg}"); + let html = format!("\u{26d4} {earliest_reset_msg}"); + (plain, html) +} + +/// Format an agent-started notification message. +/// +/// Sent when an agent transitions to the Running state. +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_agent_started_notification( + item_id: &str, + story_name: &str, + agent_name: &str, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let effective_name = if story_name.is_empty() { + None + } else { + Some(story_name) + }; + let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); + let name_html = effective_name + .map(|n| format!("{n} ")) + .unwrap_or_default(); + + let plain = format!("\u{1F916} #{number} {name_plain}\u{2014} {agent_name} started"); + let html = + format!("\u{1F916} #{number} {name_html}\u{2014} {agent_name} started"); + (plain, html) +} + +/// Format an agent-completed notification message. +/// +/// Sent when an agent finishes processing a story (gates passed or failed). +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_agent_completed_notification( + item_id: &str, + story_name: &str, + agent_name: &str, + success: bool, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let effective_name = if story_name.is_empty() { + None + } else { + Some(story_name) + }; + let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); + let name_html = effective_name + .map(|n| format!("{n} ")) + .unwrap_or_default(); + + let (emoji, result) = if success { + ("\u{2705}", "completed") // ✅ + } else { + ("\u{274C}", "failed") // ❌ + }; + let plain = format!("{emoji} #{number} {name_plain}\u{2014} {agent_name} {result}"); + let html = + format!("{emoji} #{number} {name_html}\u{2014} {agent_name} {result}"); + (plain, html) +} + +/// Emoji used to badge a work item's kind in creation notifications. +fn item_type_emoji(item_type: &str) -> &'static str { + match item_type { + "bug" => "\u{1f41b}", // 🐛 + "refactor" => "\u{1f4dd}", // 📝 + "spike" => "\u{1f52c}", // 🔬 + _ => "\u{1f4d6}", // 📖 (story, epic, and unknown) + } +} + +/// A single newly-created work item, as reported in a creation notification. +/// +/// `origin_label` is the human-readable `"{kind} {id}"` string produced by +/// `build_origin` (story 1201) — e.g. `"user alice"`, `"agent coder-1@story=42"`. +pub struct NewItemInfo { + /// Work item ID (e.g. `"1075_refactor_split_stage"`). + pub item_id: String, + /// Human-readable item type (`"story"`, `"bug"`, `"refactor"`, `"spike"`, `"epic"`). + pub item_type: String, + /// Human-readable item name. + pub name: String, + /// Readable label for who/what created the item. + pub origin_label: String, +} + +/// Format a new-work-item creation notification. +/// +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_new_item_notification( + item_id: &str, + item_type: &str, + name: &str, + origin_label: &str, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let emoji = item_type_emoji(item_type); + let plain = format!("{emoji} New {item_type} #{number} \u{2014} {name} (via {origin_label})"); + let html = format!( + "{emoji} New {item_type} #{number} \u{2014} {name} \ + (via {origin_label})" + ); + (plain, html) +} + +/// Format a creation notification for a burst of items created within the +/// same coalescing window (story 1201, AC3). +/// +/// A single pending item still produces exactly one line, identical to +/// [`format_new_item_notification`] (AC4). Two or more items are combined +/// into one message listing every item. +/// +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_new_items_notification(items: &[NewItemInfo]) -> (String, String) { + if let [only] = items { + return format_new_item_notification( + &only.item_id, + &only.item_type, + &only.name, + &only.origin_label, + ); + } + + let mut plain = format!("\u{1f4da} {} new items created:\n", items.len()); + let mut html = format!("\u{1f4da} {} new items created:
", items.len()); + for item in items { + let number = extract_item_number(&item.item_id).unwrap_or(&item.item_id); + let emoji = item_type_emoji(&item.item_type); + plain.push_str(&format!( + "\u{2022} {emoji} {} #{number} \u{2014} {} (via {})\n", + item.item_type, item.name, item.origin_label + )); + html.push_str(&format!( + "\u{2022} {emoji} {} #{number} \u{2014} {} (via {})
", + item.item_type, item.name, item.origin_label + )); + } + ( + plain.trim_end().to_string(), + html.trim_end_matches("
").to_string(), + ) +} + +/// Format a merge-auto-retry notification message. +/// +/// Sent when a `GatesFailed` merge failure is automatically retried after a +/// delay (story 1185). Returns `(plain_text, html)` suitable for +/// `ChatTransport::send_message`. +pub fn format_merge_auto_retry_notification( + item_id: &str, + story_name: &str, + attempt: u32, + budget: u32, +) -> (String, String) { + let number = extract_item_number(item_id).unwrap_or(item_id); + let effective_name = if story_name.is_empty() { + None + } else { + Some(story_name) + }; + let name_plain = effective_name.map(|n| format!("{n} ")).unwrap_or_default(); + let name_html = effective_name + .map(|n| format!("{n} ")) + .unwrap_or_default(); + + let plain = format!( + "\u{1f504} #{number} {name_plain}\u{2014} auto-retrying merge (attempt {attempt}/{budget})" + ); + let html = format!( + "\u{1f504} #{number} {name_html}\u{2014} auto-retrying merge \ + (attempt {attempt}/{budget})" + ); + (plain, html) +} + +/// Format a low-disk-space warning notification message (story 1200 AC3). +/// +/// Includes free space, `target/` and `.huskies/worktrees/` directory sizes, +/// and names the `gc` tool as the first remediation step. +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_disk_warning_notification( + level: &str, + host_id: &str, + free_bytes: u64, + target_bytes: u64, + worktrees_bytes: u64, +) -> (String, String) { + let emoji = if level == "critical" { + "\u{1f6a8}" // 🚨 + } else { + "\u{26a0}\u{fe0f}" // ⚠️ + }; + let free_gb = bytes_to_gb(free_bytes); + let target_gb = bytes_to_gb(target_bytes); + let worktrees_gb = bytes_to_gb(worktrees_bytes); + let plain = format!( + "{emoji} Low disk space on {host_id} ({level}): {free_gb:.1}GB free \ + (target/ {target_gb:.1}GB, worktrees/ {worktrees_gb:.1}GB) \ + \u{2014} first response: run the `gc` tool to reclaim space" + ); + let html = format!( + "{emoji} Low disk space on {host_id} ({level}): {free_gb:.1}GB free \ + (target/ {target_gb:.1}GB, worktrees/ {worktrees_gb:.1}GB) \ + \u{2014} first response: run the gc tool to reclaim space" + ); + (plain, html) +} + +/// Format a disk-space-recovered notification message (story 1200 AC2). +/// +/// Sent once when free space climbs back above the configured recovery +/// margin after a warn/critical warning. +/// Returns `(plain_text, html)` suitable for `ChatTransport::send_message`. +pub fn format_disk_recovery_notification(host_id: &str, free_bytes: u64) -> (String, String) { + let free_gb = bytes_to_gb(free_bytes); + let plain = format!("\u{2705} Disk space recovered on {host_id}: {free_gb:.1}GB free"); + let html = + format!("\u{2705} Disk space recovered on {host_id}: {free_gb:.1}GB free"); + (plain, html) +} + +/// Convert a byte count to gigabytes for display (story 1200). +fn bytes_to_gb(bytes: u64) -> f64 { + bytes as f64 / 1_000_000_000.0 +} + +/// Derive the human-readable project display name used to prefix creation +/// notifications (story 1201, AC2). +/// +/// Prefers the explicit `gateway_project` config value (the field the +/// codebase already treats as "this project's name" for gateway relay); +/// falls back to the project root directory's basename, then to the literal +/// `"project"` if neither is available. +pub fn project_display_name(gateway_project: Option<&str>, project_root: &Path) -> String { + gateway_project + .map(str::to_string) + .or_else(|| { + project_root + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + }) + .unwrap_or_else(|| "project".to_string()) +} + +/// Maximum number of trailing gate-output lines included in a merge-failure +/// chat notification. +/// +/// Gate output can be hundreds of lines; only the tail (where errors appear) +/// is useful at a glance. Full output remains available via `get_merge_status` +/// or the web UI — this limit is chat-display-only. +pub const MERGE_FAILURE_TAIL_LINES: usize = 30; + +/// Truncate `gate_output` to its last `max_lines` lines for chat notifications. +/// +/// If the output contains more than `max_lines` non-empty lines, a leading +/// marker line `[...output truncated, last N lines shown...]` is prepended to +/// the tail so readers know output was cut. If the output fits within the +/// limit it is returned unchanged (no marker added). +pub fn truncate_gate_output(gate_output: &str, max_lines: usize) -> String { + let lines: Vec<&str> = gate_output.lines().collect(); + if lines.len() <= max_lines { + return gate_output.to_string(); + } + let tail = &lines[lines.len() - max_lines..]; + let marker = format!("[...output truncated, last {max_lines} lines shown...]"); + format!("{marker}\n{}", tail.join("\n")) +} + +#[cfg(test)] +mod tests; diff --git a/server/src/service/notifications/format/tests.rs b/server/src/service/notifications/format/tests.rs new file mode 100644 index 00000000..09c2f4bc --- /dev/null +++ b/server/src/service/notifications/format/tests.rs @@ -0,0 +1,674 @@ +//! Tests for notification message formatting. +use super::*; + +// ── stage_display_name ──────────────────────────────────────────────────── + +fn done_stage() -> Stage { + Stage::from_dir("done").unwrap() +} +fn merge_stage() -> Stage { + Stage::from_dir("merge").unwrap() +} + +#[test] +fn stage_display_name_maps_all_known_stages() { + assert_eq!(stage_display_name(&Stage::Backlog), "Backlog"); + assert_eq!( + stage_display_name(&Stage::Coding { + claim: None, + plan: Default::default(), + retries: 0, + }), + "Current" + ); + assert_eq!(stage_display_name(&Stage::Qa), "QA"); + assert_eq!(stage_display_name(&merge_stage()), "Merge"); + assert_eq!(stage_display_name(&done_stage()), "Done"); + assert_eq!( + stage_display_name(&Stage::from_dir("archived").unwrap()), + "Archived" + ); + assert_eq!(stage_display_name(&Stage::Upcoming), "Upcoming"); +} + +// ── format_stage_notification ───────────────────────────────────────────── + +#[test] +fn format_notification_done_stage_includes_party_emoji() { + let (plain, html) = format_stage_notification( + "353_story_done", + "Done Story", + &merge_stage(), + &done_stage(), + ); + assert_eq!( + plain, + "\u{1f389} #353 Done Story \u{2014} Merge \u{2192} Done" + ); + assert_eq!( + html, + "\u{1f389} #353 Done Story \u{2014} Merge \u{2192} Done" + ); +} + +#[test] +fn format_notification_non_done_stage_has_no_emoji() { + let (plain, _html) = format_stage_notification( + "42_story_thing", + "Some Story", + &Stage::Backlog, + &Stage::Coding { + claim: None, + plan: Default::default(), + retries: 0, + }, + ); + assert!(!plain.contains("\u{1f389}")); +} + +#[test] +fn format_notification_with_story_name() { + let (plain, html) = format_stage_notification( + "261_story_bot_notifications", + "Bot notifications", + &Stage::Upcoming, + &Stage::Coding { + claim: None, + plan: Default::default(), + retries: 0, + }, + ); + assert_eq!( + plain, + "#261 Bot notifications \u{2014} Upcoming \u{2192} Current" + ); + assert_eq!( + html, + "#261 Bot notifications \u{2014} Upcoming \u{2192} Current" + ); +} + +#[test] +fn format_stage_notification_without_story_name_falls_back_to_number() { + let (plain, html) = format_stage_notification( + "42_bug_fix_thing", + "", + &Stage::Coding { + claim: None, + plan: Default::default(), + retries: 0, + }, + &Stage::Qa, + ); + assert_eq!(plain, "#42 \u{2014} Current \u{2192} QA"); + assert_eq!(html, "#42 \u{2014} Current \u{2192} QA"); +} + +#[test] +fn format_notification_non_numeric_id_uses_full_id() { + let (plain, _html) = + format_stage_notification("abc_story_thing", "Some Story", &Stage::Qa, &merge_stage()); + assert_eq!( + plain, + "#abc_story_thing Some Story \u{2014} QA \u{2192} Merge" + ); +} + +#[test] +fn format_stage_notification_long_name_is_preserved() { + let long_name = "A".repeat(300); + let (plain, _html) = format_stage_notification( + "1_story_long", + &long_name, + &Stage::Coding { + claim: None, + plan: Default::default(), + retries: 0, + }, + &Stage::Qa, + ); + assert!(plain.contains(&long_name)); +} + +#[test] +fn format_stage_notification_empty_story_name_falls_back_to_number() { + let (plain, html) = format_stage_notification( + "42_story_empty", + "", + &Stage::Coding { + claim: None, + plan: Default::default(), + retries: 0, + }, + &Stage::Qa, + ); + assert_eq!(plain, "#42 \u{2014} Current \u{2192} QA"); + assert_eq!(html, "#42 \u{2014} Current \u{2192} QA"); +} + +#[test] +fn format_stage_notification_unicode_name() { + let (plain, html) = format_stage_notification( + "7_story_i18n", + "Ünïcödé Ñämé 🎉", + &Stage::Qa, + &merge_stage(), + ); + assert!(plain.contains("Ünïcödé Ñämé 🎉")); + assert!(html.contains("Ünïcödé Ñämé 🎉")); +} + +// ── format_error_notification ───────────────────────────────────────────── + +#[test] +fn format_error_notification_with_story_name() { + let (plain, html) = format_error_notification( + "262_story_bot_errors", + "Bot error notifications", + "merge conflict in src/main.rs", + ); + assert_eq!( + plain, + "\u{274c} #262 Bot error notifications \u{2014} merge conflict in src/main.rs" + ); + assert_eq!( + html, + "\u{274c} #262 Bot error notifications \u{2014} merge conflict in src/main.rs" + ); +} + +#[test] +fn format_error_notification_without_story_name_falls_back_to_number() { + let (plain, html) = format_error_notification("42_bug_fix_thing", "", "tests failed"); + assert_eq!(plain, "\u{274c} #42 \u{2014} tests failed"); + assert_eq!(html, "\u{274c} #42 \u{2014} tests failed"); +} + +#[test] +fn format_error_notification_non_numeric_id_uses_full_id() { + let (plain, _html) = + format_error_notification("abc_story_thing", "Some Story", "clippy errors"); + assert_eq!( + plain, + "\u{274c} #abc_story_thing Some Story \u{2014} clippy errors" + ); +} + +#[test] +fn format_error_notification_long_reason_preserved() { + let long_reason = "x".repeat(500); + let (plain, _html) = format_error_notification("1_story_foo", "", &long_reason); + assert!(plain.contains(&long_reason)); +} + +#[test] +fn format_error_notification_unicode_reason() { + let (plain, _html) = format_error_notification("5_story_foo", "Foo", "错误：合并冲突"); + assert!(plain.contains("错误：合并冲突")); +} + +#[test] +fn format_error_notification_empty_story_name_falls_back_to_number() { + let (plain, _html) = format_error_notification("42_bug_fix_thing", "", "tests failed"); + assert_eq!(plain, "\u{274c} #42 \u{2014} tests failed"); +} + +// ── format_blocked_notification ─────────────────────────────────────────── + +#[test] +fn format_blocked_notification_with_story_name() { + let (plain, html) = format_blocked_notification( + "425_story_blocking_reason", + "Blocking Reason Story", + "Retry limit exceeded (3/3) at coder stage", + ); + assert_eq!( + plain, + "\u{1f6ab} #425 Blocking Reason Story \u{2014} BLOCKED: Retry limit exceeded (3/3) at coder stage" + ); + assert_eq!( + html, + "\u{1f6ab} #425 Blocking Reason Story \u{2014} BLOCKED: Retry limit exceeded (3/3) at coder stage" + ); +} + +#[test] +fn format_blocked_notification_falls_back_to_number() { + let (plain, html) = format_blocked_notification("42_story_thing", "", "empty diff"); + assert_eq!(plain, "\u{1f6ab} #42 \u{2014} BLOCKED: empty diff"); + assert_eq!( + html, + "\u{1f6ab} #42 \u{2014} BLOCKED: empty diff" + ); +} + +#[test] +fn format_blocked_notification_empty_story_name_falls_back_to_number() { + let (plain, _html) = format_blocked_notification("42_story_thing", "", "empty diff"); + assert_eq!(plain, "\u{1f6ab} #42 \u{2014} BLOCKED: empty diff"); +} + +#[test] +fn format_blocked_notification_unicode_reason() { + let (plain, _html) = format_blocked_notification("3_story_x", "X", "理由：空の差分"); + assert!(plain.contains("BLOCKED: 理由：空の差分")); +} + +// ── format_rate_limit_notification ──────────────────────────────────────── + +#[test] +fn format_rate_limit_notification_includes_agent_and_story() { + let (plain, html) = + format_rate_limit_notification("365_story_my_feature", "My Feature", "coder-2"); + assert_eq!( + plain, + "\u{26a0}\u{fe0f} #365 My Feature \u{2014} coder-2 hit an API rate limit" + ); + assert_eq!( + html, + "\u{26a0}\u{fe0f} #365 My Feature \u{2014} coder-2 hit an API rate limit" + ); +} + +#[test] +fn format_rate_limit_notification_falls_back_to_number() { + let (plain, html) = format_rate_limit_notification("42_story_thing", "", "coder-1"); + assert_eq!( + plain, + "\u{26a0}\u{fe0f} #42 \u{2014} coder-1 hit an API rate limit" + ); + assert_eq!( + html, + "\u{26a0}\u{fe0f} #42 \u{2014} coder-1 hit an API rate limit" + ); +} + +#[test] +fn format_rate_limit_notification_empty_story_name_falls_back_to_number() { + let (plain, _html) = format_rate_limit_notification("42_story_thing", "", "coder-1"); + assert_eq!( + plain, + "\u{26a0}\u{fe0f} #42 \u{2014} coder-1 hit an API rate limit" + ); +} + +#[test] +fn format_rate_limit_notification_unicode_agent_name() { + let (plain, _html) = format_rate_limit_notification("9_story_foo", "Foo", "агент-1"); + assert!(plain.contains("агент-1")); + assert!(plain.contains("hit an API rate limit")); +} + +// ── format_agent_started_notification ───────────────────────────────────── + +#[test] +fn format_agent_started_notification_with_story_name() { + let (plain, html) = format_agent_started_notification("42_story_foo", "My Feature", "coder-1"); + assert_eq!(plain, "\u{1F916} #42 My Feature \u{2014} coder-1 started"); + assert_eq!( + html, + "\u{1F916} #42 My Feature \u{2014} coder-1 started" + ); +} + +#[test] +fn format_agent_started_notification_falls_back_to_number() { + let (plain, html) = format_agent_started_notification("42_story_foo", "", "coder-1"); + assert_eq!(plain, "\u{1F916} #42 \u{2014} coder-1 started"); + assert_eq!( + html, + "\u{1F916} #42 \u{2014} coder-1 started" + ); +} + +#[test] +fn format_agent_started_notification_empty_name_falls_back_to_number() { + let (plain, _html) = format_agent_started_notification("42_story_foo", "", "coder-1"); + assert_eq!(plain, "\u{1F916} #42 \u{2014} coder-1 started"); +} + +// ── format_merge_auto_retry_notification ────────────────────────────────── + +#[test] +fn format_merge_auto_retry_notification_with_story_name() { + let (plain, html) = format_merge_auto_retry_notification("42_story_foo", "My Feature", 1, 3); + assert_eq!( + plain, + "\u{1f504} #42 My Feature \u{2014} auto-retrying merge (attempt 1/3)" + ); + assert_eq!( + html, + "\u{1f504} #42 My Feature \u{2014} auto-retrying merge \ + (attempt 1/3)" + ); +} + +#[test] +fn format_merge_auto_retry_notification_falls_back_to_number() { + let (plain, html) = format_merge_auto_retry_notification("42_story_foo", "", 2, 3); + assert_eq!( + plain, + "\u{1f504} #42 \u{2014} auto-retrying merge (attempt 2/3)" + ); + assert_eq!( + html, + "\u{1f504} #42 \u{2014} auto-retrying merge (attempt 2/3)" + ); +} + +// ── project_display_name ────────────────────────────────────────────────── + +#[test] +fn project_display_name_prefers_gateway_project() { + let root = std::path::Path::new("/tmp/whatever-dir-name"); + assert_eq!( + project_display_name(Some("my-configured-name"), root), + "my-configured-name" + ); +} + +#[test] +fn project_display_name_falls_back_to_dir_basename() { + let root = std::path::Path::new("/tmp/my-project-dir"); + assert_eq!(project_display_name(None, root), "my-project-dir"); +} + +#[test] +fn project_display_name_falls_back_to_project_literal() { + let root = std::path::Path::new("/"); + assert_eq!(project_display_name(None, root), "project"); +} + +// ── truncate_gate_output ────────────────────────────────────────────────── + +#[test] +fn truncate_gate_output_short_output_returned_unchanged() { + let output = "line1\nline2\nline3"; + assert_eq!(truncate_gate_output(output, 30), output); +} + +#[test] +fn truncate_gate_output_exact_limit_returned_unchanged() { + let lines: Vec = (1..=30).map(|i| format!("line{i}")).collect(); + let output = lines.join("\n"); + assert_eq!(truncate_gate_output(&output, 30), output); +} + +#[test] +fn truncate_gate_output_over_limit_prepends_marker() { + let lines: Vec = (1..=35).map(|i| format!("line{i}")).collect(); + let output = lines.join("\n"); + let result = truncate_gate_output(&output, 30); + assert!( + result.starts_with("[...output truncated, last 30 lines shown...]"), + "must start with truncation marker; got: {result}" + ); +} + +#[test] +fn truncate_gate_output_over_limit_contains_tail_lines() { + let lines: Vec = (1..=35).map(|i| format!("line{i}")).collect(); + let output = lines.join("\n"); + let result = truncate_gate_output(&output, 30); + // Last 30 lines are line6..line35. + assert!(result.contains("line35"), "must contain last line"); + assert!(result.contains("line6"), "must contain first tail line"); + assert!(!result.contains("line5"), "must not contain dropped line"); +} + +#[test] +fn truncate_gate_output_empty_input_returned_unchanged() { + assert_eq!(truncate_gate_output("", 30), ""); +} + +#[test] +fn truncate_gate_output_single_line_returned_unchanged() { + assert_eq!(truncate_gate_output("only one line", 30), "only one line"); +} + +#[test] +fn truncate_gate_output_marker_contains_configured_limit() { + let lines: Vec = (1..=10).map(|i| format!("x{i}")).collect(); + let output = lines.join("\n"); + let result = truncate_gate_output(&output, 5); + assert!( + result.contains("last 5 lines shown"), + "marker must state configured limit; got: {result}" + ); +} + +// ── format_agent_completed_notification ─────────────────────────────────── + +#[test] +fn format_agent_completed_notification_success_with_story_name() { + let (plain, html) = + format_agent_completed_notification("42_story_foo", "My Feature", "coder-1", true); + assert_eq!(plain, "\u{2705} #42 My Feature \u{2014} coder-1 completed"); + assert_eq!( + html, + "\u{2705} #42 My Feature \u{2014} coder-1 completed" + ); +} + +#[test] +fn format_agent_completed_notification_failure_with_story_name() { + let (plain, _html) = + format_agent_completed_notification("42_story_foo", "My Feature", "coder-1", false); + assert_eq!(plain, "\u{274C} #42 My Feature \u{2014} coder-1 failed"); +} + +#[test] +fn format_agent_completed_notification_falls_back_to_number() { + let (plain, html) = format_agent_completed_notification("42_story_foo", "", "coder-1", true); + assert_eq!(plain, "\u{2705} #42 \u{2014} coder-1 completed"); + assert_eq!( + html, + "\u{2705} #42 \u{2014} coder-1 completed" + ); +} + +// ── format_new_item_notification ────────────────────────────────────────── + +#[test] +fn format_new_item_notification_story() { + let (plain, html) = + format_new_item_notification("42_story_my_feature", "story", "My Feature", "user alice"); + assert_eq!( + plain, + "\u{1f4d6} New story #42 \u{2014} My Feature (via user alice)" + ); + assert_eq!( + html, + "\u{1f4d6} New story #42 \u{2014} My Feature (via user alice)" + ); +} + +#[test] +fn format_new_item_notification_bug() { + let (plain, html) = format_new_item_notification( + "99_bug_login_crash", + "bug", + "Login Crash", + "agent coder-1@story=42", + ); + assert_eq!( + plain, + "\u{1f41b} New bug #99 \u{2014} Login Crash (via agent coder-1@story=42)" + ); + assert_eq!( + html, + "\u{1f41b} New bug #99 \u{2014} Login Crash \ + (via agent coder-1@story=42)" + ); +} + +#[test] +fn format_new_item_notification_refactor() { + let (plain, html) = format_new_item_notification( + "1075_refactor_split_stage", + "refactor", + "Split Stage enum into Pipeline + Status", + "chat-bot Timmy@!room:home", + ); + assert_eq!( + plain, + "\u{1f4dd} New refactor #1075 \u{2014} Split Stage enum into Pipeline + Status \ + (via chat-bot Timmy@!room:home)" + ); + assert_eq!( + html, + "\u{1f4dd} New refactor #1075 \u{2014} Split Stage enum into \ + Pipeline + Status (via chat-bot Timmy@!room:home)" + ); +} + +#[test] +fn format_new_item_notification_spike() { + let (plain, html) = format_new_item_notification( + "7_spike_encoder_comparison", + "spike", + "Compare Encoders", + "user alice", + ); + assert_eq!( + plain, + "\u{1f52c} New spike #7 \u{2014} Compare Encoders (via user alice)" + ); + assert_eq!( + html, + "\u{1f52c} New spike #7 \u{2014} Compare Encoders \ + (via user alice)" + ); +} + +#[test] +fn format_new_item_notification_epic() { + let (plain, _html) = + format_new_item_notification("50_epic_big_thing", "epic", "Big Thing", "user alice"); + assert!(plain.contains("New epic #50")); + assert!(plain.contains("Big Thing")); +} + +#[test] +fn format_new_item_notification_non_numeric_id_uses_full_id() { + let (plain, _html) = + format_new_item_notification("abc_story_thing", "story", "Some Story", "user alice"); + assert_eq!( + plain, + "\u{1f4d6} New story #abc_story_thing \u{2014} Some Story (via user alice)" + ); +} + +// ── origin rendering (AC2, AC5) ─────────────────────────────────────────── + +#[test] +fn format_new_item_notification_renders_user_origin() { + let (plain, _html) = format_new_item_notification("1_story_a", "story", "A", "user alice"); + assert!(plain.contains("(via user alice)")); +} + +#[test] +fn format_new_item_notification_renders_chat_bot_origin() { + let (plain, _html) = + format_new_item_notification("1_story_a", "story", "A", "chat-bot Timmy@!room:home"); + assert!(plain.contains("(via chat-bot Timmy@!room:home)")); +} + +#[test] +fn format_new_item_notification_renders_agent_origin() { + let (plain, _html) = + format_new_item_notification("1_story_a", "story", "A", "agent coder-1@story=42"); + assert!(plain.contains("(via agent coder-1@story=42)")); +} + +// ── format_new_items_notification ───────────────────────────────────────── + +fn item(item_id: &str, item_type: &str, name: &str, origin_label: &str) -> NewItemInfo { + NewItemInfo { + item_id: item_id.to_string(), + item_type: item_type.to_string(), + name: name.to_string(), + origin_label: origin_label.to_string(), + } +} + +#[test] +fn format_new_items_notification_single_item_matches_single_formatter() { + let items = vec![item("42_story_a", "story", "A", "user alice")]; + let (plain, html) = format_new_items_notification(&items); + let (expected_plain, expected_html) = + format_new_item_notification("42_story_a", "story", "A", "user alice"); + assert_eq!(plain, expected_plain); + assert_eq!(html, expected_html); +} + +#[test] +fn format_new_items_notification_multiple_items_lists_all() { + let items = vec![ + item("42_story_a", "story", "A Feature", "user alice"), + item("43_bug_b", "bug", "B Crash", "agent coder-1@story=42"), + item("44_refactor_c", "refactor", "C Cleanup", "user alice"), + ]; + let (plain, html) = format_new_items_notification(&items); + assert!(plain.starts_with("\u{1f4da} 3 new items created:")); + assert!(plain.contains("#42"), "missing item 1: {plain}"); + assert!(plain.contains("A Feature"), "missing item 1 name: {plain}"); + assert!(plain.contains("#43"), "missing item 2: {plain}"); + assert!(plain.contains("B Crash"), "missing item 2 name: {plain}"); + assert!(plain.contains("#44"), "missing item 3: {plain}"); + assert!(plain.contains("C Cleanup"), "missing item 3 name: {plain}"); + assert!(html.contains("A Feature")); + assert!(html.contains("B Crash")); + assert!(html.contains("C Cleanup")); +} + +#[test] +fn format_agent_completed_notification_empty_name_falls_back_to_number() { + let (plain, _html) = format_agent_completed_notification("42_story_foo", "", "coder-1", false); + assert_eq!(plain, "\u{274C} #42 \u{2014} coder-1 failed"); +} + +// ── format_disk_warning_notification ────────────────────────────────────── + +#[test] +fn format_disk_warning_notification_warn_level_includes_content() { + let (plain, html) = format_disk_warning_notification( + "warn", + "sled-a", + 45_000_000_000, + 30_000_000_000, + 10_000_000_000, + ); + assert!(plain.contains("sled-a")); + assert!(plain.contains("45.0GB free")); + assert!(plain.contains("target/ 30.0GB")); + assert!(plain.contains("worktrees/ 10.0GB")); + assert!(plain.contains("`gc` tool")); + assert!(html.contains("gc tool")); +} + +#[test] +fn format_disk_warning_notification_critical_uses_distinct_emoji() { + let (plain, _html) = + format_disk_warning_notification("critical", "sled-b", 5_000_000_000, 0, 0); + assert!(plain.starts_with("\u{1f6a8}")); +} + +#[test] +fn format_disk_warning_notification_warn_uses_warning_emoji() { + let (plain, _html) = format_disk_warning_notification("warn", "sled-b", 45_000_000_000, 0, 0); + assert!(plain.starts_with("\u{26a0}\u{fe0f}")); +} + +// ── format_disk_recovery_notification ───────────────────────────────────── + +#[test] +fn format_disk_recovery_notification_includes_host_and_free_space() { + let (plain, html) = format_disk_recovery_notification("sled-a", 60_000_000_000); + assert_eq!( + plain, + "\u{2705} Disk space recovered on sled-a: 60.0GB free" + ); + assert!(html.contains("sled-a")); + assert!(html.contains("60.0GB free")); +} diff --git a/server/src/service/notifications/io/listener.rs b/server/src/service/notifications/io/listener.rs index dcc87a4e..f24e4015 100644 --- a/server/src/service/notifications/io/listener.rs +++ b/server/src/service/notifications/io/listener.rs @@ -12,18 +12,46 @@ use std::time::Instant; use tokio::sync::broadcast; use super::super::events::classify; -use super::super::filter::{AGENT_EVENT_DEBOUNCE, should_send_rate_limit}; +use super::super::filter::{ + AGENT_EVENT_DEBOUNCE, NEW_ITEM_COALESCE_WINDOW, should_send_rate_limit, +}; use super::super::format::{ - MERGE_FAILURE_TAIL_LINES, format_agent_completed_notification, + MERGE_FAILURE_TAIL_LINES, NewItemInfo, format_agent_completed_notification, format_agent_started_notification, format_blocked_notification, format_disk_recovery_notification, format_disk_warning_notification, format_error_notification, - format_merge_auto_retry_notification, format_new_item_notification, + format_merge_auto_retry_notification, format_new_items_notification, format_oauth_account_swapped, format_oauth_accounts_exhausted, format_rate_limit_notification, - truncate_gate_output, + project_display_name, truncate_gate_output, }; use super::super::route::rooms_for_notification; use super::{find_story_name_any_stage, read_story_name}; +/// Format and send any pending new-item-creation notifications as a single +/// combined message, then clear the pending buffer. +/// +/// A no-op if `pending` is empty. The message is prefixed with +/// `[{project_name}] ` (story 1201, AC2). +async fn send_new_item_notifications( + pending: &mut Vec, + project_name: &str, + transport: &Arc, + get_room_ids: &impl Fn() -> Vec, +) { + if pending.is_empty() { + return; + } + let items = std::mem::take(pending); + let (plain, html) = format_new_items_notification(&items); + let plain = format!("[{project_name}] {plain}"); + let html = format!("[{project_name}] {html}"); + slog!("[bot] Sending new-item notification: {plain}"); + for room_id in &rooms_for_notification(get_room_ids) { + if let Err(e) = transport.send_message(room_id, &plain, &html).await { + slog!("[bot] Failed to send new-item notification to {room_id}: {e}"); + } + } +} + /// Spawn a background task that listens for watcher events and posts /// stage-transition notifications to all configured rooms via the /// [`ChatTransport`] abstraction. @@ -34,7 +62,7 @@ use super::{find_story_name_any_stage, read_story_name}; /// for WhatsApp ambient senders. pub fn spawn_notification_listener( transport: Arc, - get_room_ids: impl Fn() -> Vec + Send + 'static, + get_room_ids: impl Fn() -> Vec + Send + Sync + 'static, watcher_rx: broadcast::Receiver, project_root: PathBuf, ) { @@ -42,6 +70,8 @@ pub fn spawn_notification_listener( let mut rx = watcher_rx; // Load initial config; re-loaded on ConfigChanged events. let mut config = ProjectConfig::load(&project_root).unwrap_or_default(); + let mut project_name = + project_display_name(config.gateway_project.as_deref(), &project_root); // Tracks when a rate-limit notification was last sent for each // "story_id:agent_name" key, to debounce repeated warnings. let mut rate_limit_last_notified: HashMap = HashMap::new(); @@ -53,18 +83,30 @@ pub fn spawn_notification_listener( let mut pending_agent_events: HashMap = HashMap::new(); let mut agent_flush_deadline: Option = None; + // Pending new-item-creation notifications. Unlike agent events, every + // item created within the window is kept (not latest-wins) so a burst + // of creations produces one combined message listing all of them + // (story 1201, AC3). + let mut pending_new_items: Vec = Vec::new(); + let mut new_item_flush_deadline: Option = None; + loop { - // Wait for the next event, or flush pending agent notifications when - // the debounce window expires. - let recv_result = if let Some(deadline) = agent_flush_deadline { + // Wait for the next event, or flush pending notifications when the + // earliest debounce window expires. + let next_deadline = [agent_flush_deadline, new_item_flush_deadline] + .into_iter() + .flatten() + .min(); + let recv_result = if let Some(deadline) = next_deadline { tokio::time::timeout_at(deadline, rx.recv()).await.ok() } else { Some(rx.recv().await) }; if recv_result.is_none() { + let now = tokio::time::Instant::now(); // Flush agent events if their deadline has passed. - if agent_flush_deadline.is_some_and(|d| d <= tokio::time::Instant::now()) { + if agent_flush_deadline.is_some_and(|d| d <= now) { for (_key, (plain, html)) in pending_agent_events.drain() { slog!("[bot] Sending agent notification: {plain}"); if config.status_push_enabled { @@ -80,6 +122,17 @@ pub fn spawn_notification_listener( } agent_flush_deadline = None; } + // Flush new-item events if their deadline has passed. + if new_item_flush_deadline.is_some_and(|d| d <= now) { + send_new_item_notifications( + &mut pending_new_items, + &project_name, + &transport, + &get_room_ids, + ) + .await; + new_item_flush_deadline = None; + } continue; } @@ -104,6 +157,13 @@ pub fn spawn_notification_listener( } } } + send_new_item_notifications( + &mut pending_new_items, + &project_name, + &transport, + &get_room_ids, + ) + .await; break; } }; @@ -285,16 +345,23 @@ pub fn spawn_notification_listener( ref item_id, ref item_type, ref name, + ref origin, } = event else { continue; }; - let (plain, html) = format_new_item_notification(item_id, item_type, name); - slog!("[bot] Sending new-item notification: {plain}"); - for room_id in &rooms_for_notification(&get_room_ids) { - if let Err(e) = transport.send_message(room_id, &plain, &html).await { - slog!("[bot] Failed to send new-item notification to {room_id}: {e}"); - } + pending_new_items.push(NewItemInfo { + item_id: item_id.clone(), + item_type: item_type.clone(), + name: name.clone(), + origin_label: origin.clone(), + }); + // Set the deadline once from the first arriving event so + // concurrent bursts don't keep pushing the window out and + // starving the flush. + if new_item_flush_deadline.is_none() { + new_item_flush_deadline = + Some(tokio::time::Instant::now() + NEW_ITEM_COALESCE_WINDOW); } } EventAction::MergeAutoRetry => { @@ -398,6 +465,8 @@ pub fn spawn_notification_listener( EventAction::ReloadConfig => { if let Ok(new_cfg) = ProjectConfig::load(&project_root) { config = new_cfg; + project_name = + project_display_name(config.gateway_project.as_deref(), &project_root); } } EventAction::Skip => {} diff --git a/server/src/service/notifications/io/tests_notifications.rs b/server/src/service/notifications/io/tests_notifications.rs index 296ebbbc..2a75bdee 100644 --- a/server/src/service/notifications/io/tests_notifications.rs +++ b/server/src/service/notifications/io/tests_notifications.rs @@ -1,10 +1,147 @@ -//! Tests for rate-limit, story-blocked, OAuth, and config-reload notifications. +//! Tests for rate-limit, story-blocked, OAuth, config-reload, and +//! new-item-creation notifications. use super::mock_transport::MockTransport; use super::spawn_notification_listener; use crate::io::watcher::WatcherEvent; use tokio::sync::broadcast; +// ── spawn_notification_listener: NewItemCreated ────────────────────────────── + +/// AC1+AC2 (story 1201): a single creation produces exactly one chat line, +/// prefixed with the project name and naming the readable origin. +#[tokio::test] +async fn new_item_created_sends_single_notification_with_project_prefix_and_origin() { + let tmp = tempfile::tempdir().unwrap(); + let sk_dir = tmp.path().join(".huskies"); + std::fs::create_dir_all(&sk_dir).unwrap(); + std::fs::write( + sk_dir.join("project.toml"), + "gateway_project = \"TestProj\"\n", + ) + .unwrap(); + + let (watcher_tx, watcher_rx) = broadcast::channel::(16); + let (transport, calls) = MockTransport::new(); + + spawn_notification_listener( + transport, + || vec!["!room1:example.org".to_string()], + watcher_rx, + tmp.path().to_path_buf(), + ); + + watcher_tx + .send(WatcherEvent::NewItemCreated { + item_id: "42_story_my_feature".to_string(), + item_type: "story".to_string(), + name: "My Feature".to_string(), + origin: "user alice".to_string(), + }) + .unwrap(); + + tokio::time::sleep(std::time::Duration::from_millis(400)).await; + + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 1, "Expected exactly one notification"); + let (_, plain, _) = &calls[0]; + assert!( + plain.starts_with("[TestProj] "), + "must be prefixed with the project name; got: {plain}" + ); + assert!(plain.contains("42"), "must contain the item number"); + assert!(plain.contains("My Feature"), "must contain the item name"); + assert!( + plain.contains("user alice"), + "must render the origin readably; got: {plain}" + ); +} + +/// AC3 (story 1201): multiple creations within the coalescing window produce +/// one combined chat message listing all items. +#[tokio::test] +async fn new_item_created_burst_coalesces_into_one_combined_message() { + let tmp = tempfile::tempdir().unwrap(); + + let (watcher_tx, watcher_rx) = broadcast::channel::(16); + let (transport, calls) = MockTransport::new(); + + spawn_notification_listener( + transport, + || vec!["!room1:example.org".to_string()], + watcher_rx, + tmp.path().to_path_buf(), + ); + + // Three creations in rapid succession (no sleep between) — all within + // the coalescing window. + watcher_tx + .send(WatcherEvent::NewItemCreated { + item_id: "1_story_a".to_string(), + item_type: "story".to_string(), + name: "Story A".to_string(), + origin: "user alice".to_string(), + }) + .unwrap(); + watcher_tx + .send(WatcherEvent::NewItemCreated { + item_id: "2_bug_b".to_string(), + item_type: "bug".to_string(), + name: "Bug B".to_string(), + origin: "agent coder-1@story=42".to_string(), + }) + .unwrap(); + watcher_tx + .send(WatcherEvent::NewItemCreated { + item_id: "3_refactor_c".to_string(), + item_type: "refactor".to_string(), + name: "Refactor C".to_string(), + origin: "chat-bot Timmy@!room:home".to_string(), + }) + .unwrap(); + + // Wait past the coalescing window for the combined flush. + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + + let calls = calls.lock().unwrap(); + assert_eq!( + calls.len(), + 1, + "A burst of creations must coalesce into one combined message; got: {calls:?}" + ); + let (_, plain, _) = &calls[0]; + assert!(plain.contains("Story A"), "missing item 1; got: {plain}"); + assert!(plain.contains("Bug B"), "missing item 2; got: {plain}"); + assert!(plain.contains("Refactor C"), "missing item 3; got: {plain}"); +} + +/// AC4 (story 1201): when no chat rooms are registered, new-item creation +/// notifications are silently dropped rather than causing a panic or error — +/// mirroring the failure-isolation guarantee at the create-call boundary. +#[tokio::test] +async fn new_item_created_with_no_rooms_is_silent() { + let tmp = tempfile::tempdir().unwrap(); + + let (watcher_tx, watcher_rx) = broadcast::channel::(16); + let (transport, calls) = MockTransport::new(); + + spawn_notification_listener(transport, Vec::new, watcher_rx, tmp.path().to_path_buf()); + + watcher_tx + .send(WatcherEvent::NewItemCreated { + item_id: "42_story_no_rooms".to_string(), + item_type: "story".to_string(), + name: "No Rooms".to_string(), + origin: "user alice".to_string(), + }) + .unwrap(); + + tokio::time::sleep(std::time::Duration::from_millis(400)).await; + + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 0, "No rooms means no notifications"); +} + // ── spawn_notification_listener: MergeFailure ──────────────────────────────── /// Long gate output is truncated to the tail and includes the marker line.