diff --git a/server/src/chat/transport/matrix/notifications.rs b/server/src/chat/transport/matrix/notifications.rs index 064b1920..85e20dbb 100644 --- a/server/src/chat/transport/matrix/notifications.rs +++ b/server/src/chat/transport/matrix/notifications.rs @@ -3,6 +3,7 @@ //! Subscribes to [`WatcherEvent`] broadcasts and posts a notification to all //! configured Matrix rooms whenever a work item moves between pipeline stages. +use crate::config::ProjectConfig; use crate::io::story_metadata::parse_front_matter; use crate::io::watcher::WatcherEvent; use crate::slog; @@ -202,6 +203,8 @@ pub fn spawn_notification_listener( ) { tokio::spawn(async move { let mut rx = watcher_rx; + // Load initial config; re-loaded on ConfigChanged events. + let mut config = ProjectConfig::load(&project_root).unwrap_or_default(); // 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(); @@ -314,6 +317,13 @@ pub fn spawn_notification_listener( ref story_id, ref agent_name, }) => { + if !config.rate_limit_notifications { + slog!( + "[bot] RateLimitWarning suppressed by config for \ + {story_id}:{agent_name}" + ); + continue; + } // Debounce: skip if we sent a notification for this agent // within the last RATE_LIMIT_DEBOUNCE seconds. let debounce_key = format!("{story_id}:{agent_name}"); @@ -407,7 +417,13 @@ pub fn spawn_notification_listener( } } } - Ok(_) => {} // Ignore non-work-item events + Ok(WatcherEvent::ConfigChanged) => { + // Hot-reload: pick up any changes to rate_limit_notifications. + if let Ok(new_cfg) = ProjectConfig::load(&project_root) { + config = new_cfg; + } + } + Ok(_) => {} // Ignore other events Err(broadcast::error::RecvError::Lagged(n)) => { slog!( "[bot] Notification listener lagged, skipped {n} events" @@ -1009,4 +1025,159 @@ mod tests { "#abc_story_thing Some Story \u{2014} QA \u{2192} Merge" ); } + + // ── rate_limit_notifications config flag ───────────────────────────────── + + /// AC1+AC2: when rate_limit_notifications = false in project.toml, + /// RateLimitWarning events are suppressed (no send_message call). + #[tokio::test] + async fn rate_limit_warning_suppressed_when_config_false() { + let tmp = tempfile::tempdir().unwrap(); + let sk_dir = tmp.path().join(".storkit"); + std::fs::create_dir_all(&sk_dir).unwrap(); + std::fs::write( + sk_dir.join("project.toml"), + "rate_limit_notifications = false\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::RateLimitWarning { + story_id: "42_story_suppress".to_string(), + agent_name: "coder-1".to_string(), + }).unwrap(); + + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 0, "RateLimitWarning should be suppressed when rate_limit_notifications = false"); + } + + /// AC3: RateLimitHardBlock is always sent regardless of rate_limit_notifications. + #[tokio::test] + async fn rate_limit_hard_block_always_sent_when_config_false() { + let tmp = tempfile::tempdir().unwrap(); + let sk_dir = tmp.path().join(".storkit"); + std::fs::create_dir_all(&sk_dir).unwrap(); + std::fs::write( + sk_dir.join("project.toml"), + "rate_limit_notifications = false\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(), + ); + + let reset_at = chrono::Utc::now() + chrono::Duration::hours(1); + watcher_tx.send(WatcherEvent::RateLimitHardBlock { + story_id: "42_story_hard_block".to_string(), + agent_name: "coder-1".to_string(), + reset_at, + }).unwrap(); + + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 1, "RateLimitHardBlock should always be sent"); + } + + /// AC3: StoryBlocked is always sent regardless of rate_limit_notifications. + #[tokio::test] + async fn story_blocked_always_sent_when_config_false() { + let tmp = tempfile::tempdir().unwrap(); + let sk_dir = tmp.path().join(".storkit"); + std::fs::create_dir_all(&sk_dir).unwrap(); + std::fs::write( + sk_dir.join("project.toml"), + "rate_limit_notifications = false\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::StoryBlocked { + story_id: "42_story_blocked".to_string(), + reason: "retry limit exceeded".to_string(), + }).unwrap(); + + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 1, "StoryBlocked should always be sent"); + } + + /// AC5: Config is hot-reloaded — disabling rate_limit_notifications after + /// startup suppresses subsequent RateLimitWarning events. + #[tokio::test] + async fn rate_limit_warning_suppressed_after_hot_reload() { + let tmp = tempfile::tempdir().unwrap(); + let sk_dir = tmp.path().join(".storkit"); + std::fs::create_dir_all(&sk_dir).unwrap(); + // Start with notifications enabled. + std::fs::write( + sk_dir.join("project.toml"), + "rate_limit_notifications = true\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(), + ); + + // First warning is sent. + watcher_tx.send(WatcherEvent::RateLimitWarning { + story_id: "42_story_reload".to_string(), + agent_name: "coder-1".to_string(), + }).unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + + // Disable notifications and trigger hot-reload. + std::fs::write( + sk_dir.join("project.toml"), + "rate_limit_notifications = false\n", + ) + .unwrap(); + watcher_tx.send(WatcherEvent::ConfigChanged).unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + + // Second warning (different agent to bypass debounce) should be suppressed. + watcher_tx.send(WatcherEvent::RateLimitWarning { + story_id: "42_story_reload".to_string(), + agent_name: "coder-2".to_string(), + }).unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 1, "Only the first warning should be sent; second should be suppressed after hot-reload"); + } } diff --git a/server/src/chat/util.rs b/server/src/chat/util.rs index cfe1bdf5..1f9dff5b 100644 --- a/server/src/chat/util.rs +++ b/server/src/chat/util.rs @@ -55,19 +55,18 @@ pub fn strip_bot_mention<'a>(message: &'a str, bot_name: &str, bot_user_id: &str // Try Element Markdown mention pill format: // "[DisplayName](https://matrix.to/#/@user:server) rest" - if trimmed.starts_with('[') { - if let Some(after_label) = trimmed.find("](https://matrix.to/#/") { - let url_start = after_label + 2; // skip "](" - let url_content = &trimmed[url_start..]; // "https://matrix.to/#/@user:server) rest" - if let Some(close_paren) = url_content.find(')') { - let url = &url_content[..close_paren]; // "https://matrix.to/#/@user:server" - let matrix_prefix = "https://matrix.to/#/"; - if url.starts_with(matrix_prefix) { - let mentioned_id = &url[matrix_prefix.len()..]; - if mentioned_id.eq_ignore_ascii_case(bot_user_id) { - let rest = &url_content[close_paren + 1..]; - return strip_mention_separator(rest); - } + if trimmed.starts_with('[') + && let Some(after_label) = trimmed.find("](https://matrix.to/#/") + { + let url_start = after_label + 2; // skip "](" + let url_content = &trimmed[url_start..]; // "https://matrix.to/#/@user:server) rest" + if let Some(close_paren) = url_content.find(')') { + let url = &url_content[..close_paren]; // "https://matrix.to/#/@user:server" + let matrix_prefix = "https://matrix.to/#/"; + if let Some(mentioned_id) = url.strip_prefix(matrix_prefix) { + if mentioned_id.eq_ignore_ascii_case(bot_user_id) { + let rest = &url_content[close_paren + 1..]; + return strip_mention_separator(rest); } } } diff --git a/server/src/config.rs b/server/src/config.rs index ace5eeca..c6d73801 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -36,6 +36,12 @@ pub struct ProjectConfig { /// When not set, the system falls back to `detect_base_branch` (reads current HEAD). #[serde(default)] pub base_branch: Option, + /// Whether to send `RateLimitWarning` chat notifications. + /// Set to `false` to suppress noisy soft rate-limit warnings while still + /// receiving `RateLimitHardBlock` and `StoryBlocked` notifications. + /// Default: `true`. + #[serde(default = "default_rate_limit_notifications")] + pub rate_limit_notifications: bool, } /// Configuration for the filesystem watcher's sweep behaviour. @@ -79,6 +85,10 @@ fn default_max_retries() -> u32 { 2 } +fn default_rate_limit_notifications() -> bool { + true +} + #[derive(Debug, Clone, Deserialize)] #[allow(dead_code)] pub struct ComponentConfig { @@ -172,6 +182,8 @@ struct LegacyProjectConfig { max_retries: u32, #[serde(default)] base_branch: Option, + #[serde(default = "default_rate_limit_notifications")] + rate_limit_notifications: bool, } impl Default for ProjectConfig { @@ -199,6 +211,7 @@ impl Default for ProjectConfig { max_coders: None, max_retries: default_max_retries(), base_branch: None, + rate_limit_notifications: default_rate_limit_notifications(), } } } @@ -245,6 +258,7 @@ impl ProjectConfig { max_coders: legacy.max_coders, max_retries: legacy.max_retries, base_branch: legacy.base_branch, + rate_limit_notifications: legacy.rate_limit_notifications, }; validate_agents(&config.agent)?; return Ok(config); @@ -270,6 +284,7 @@ impl ProjectConfig { max_coders: legacy.max_coders, max_retries: legacy.max_retries, base_branch: legacy.base_branch, + rate_limit_notifications: legacy.rate_limit_notifications, }; validate_agents(&config.agent)?; Ok(config) @@ -283,6 +298,7 @@ impl ProjectConfig { max_coders: legacy.max_coders, max_retries: legacy.max_retries, base_branch: legacy.base_branch, + rate_limit_notifications: legacy.rate_limit_notifications, }) } } @@ -946,6 +962,28 @@ prompt = "git difftool {{base_branch}}...HEAD" ); } + #[test] + fn rate_limit_notifications_defaults_to_true() { + let toml_str = r#" +[[agent]] +name = "coder" +"#; + let config = ProjectConfig::parse(toml_str).unwrap(); + assert!(config.rate_limit_notifications, "rate_limit_notifications should default to true"); + } + + #[test] + fn rate_limit_notifications_can_be_disabled() { + let toml_str = r#" +rate_limit_notifications = false + +[[agent]] +name = "coder" +"#; + let config = ProjectConfig::parse(toml_str).unwrap(); + assert!(!config.rate_limit_notifications); + } + #[test] fn project_toml_has_three_sonnet_coders() { let manifest_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); diff --git a/server/src/worktree.rs b/server/src/worktree.rs index cea3d1a7..d1399d0d 100644 --- a/server/src/worktree.rs +++ b/server/src/worktree.rs @@ -526,6 +526,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; // Should complete without panic run_setup_commands(tmp.path(), &config).await; @@ -548,6 +549,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; // Should complete without panic run_setup_commands(tmp.path(), &config).await; @@ -570,6 +572,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; // Setup command failures are non-fatal — should not panic or propagate run_setup_commands(tmp.path(), &config).await; @@ -592,6 +595,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; // Teardown failures are best-effort — should not propagate assert!(run_teardown_commands(tmp.path(), &config).await.is_ok()); @@ -613,6 +617,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; let info = create_worktree(&project_root, "42_fresh_test", &config, 3001) .await @@ -641,6 +646,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; // First creation let _info1 = create_worktree(&project_root, "43_reuse_test", &config, 3001) @@ -710,6 +716,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; let result = remove_worktree_by_story_id(tmp.path(), "99_nonexistent", &config).await; @@ -737,6 +744,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; create_worktree(&project_root, "88_remove_by_id", &config, 3001) .await @@ -811,6 +819,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; // Even though setup commands fail, create_worktree must succeed // so the agent can start and fix the problem itself. @@ -841,6 +850,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; // First creation — no setup commands, should succeed create_worktree(&project_root, "173_reuse_fail", &empty_config, 3001) @@ -861,6 +871,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; // Second call — worktree exists, setup commands fail, must still succeed let result = create_worktree(&project_root, "173_reuse_fail", &failing_config, 3002).await; @@ -887,6 +898,7 @@ mod tests { max_coders: None, max_retries: 2, base_branch: None, + rate_limit_notifications: true, }; let info = create_worktree(&project_root, "77_remove_async", &config, 3001) .await