storkit: merge 463_story_configurable_rate_limit_notification_suppression
This commit is contained in:
@@ -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<String, Instant> = 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::<WatcherEvent>(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::<WatcherEvent>(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::<WatcherEvent>(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::<WatcherEvent>(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");
|
||||
}
|
||||
}
|
||||
|
||||
+12
-13
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user