-
Notifications
You must be signed in to change notification settings - Fork 299
fix: cron jobs bypassed by ResponseMode/listen-only mode #519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
bbf02f6
9929498
df87ece
ad7856c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1822,8 +1822,11 @@ impl Channel { | |
|
|
||
| // Response mode guardrail: | ||
| // In Quiet/MentionOnly modes, ingest messages but only reply when explicitly invoked. | ||
| // Cron-originated messages carry a platform source (e.g., "slack") for adapter | ||
| // routing but use sender_id="system" — exempt them from suppression. | ||
| if !matches!(self.resolved_settings.response_mode, ResponseMode::Active) | ||
| && message.source != "system" | ||
| && message.sender_id != "system" | ||
| && !self.is_dm() | ||
| { | ||
| (invoked_by_command, invoked_by_mention, invoked_by_reply) = | ||
|
|
@@ -3809,6 +3812,35 @@ mod tests { | |
| assert!(!invoked_by_reply); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new test recomputes a local boolean instead of driving the actual channel suppression logic, so it can still pass even if |
||
| } | ||
|
|
||
| #[test] | ||
| fn response_mode_guard_allows_cron_messages_through() { | ||
| // Cron messages have source="slack" (for adapter routing) but sender_id="system". | ||
| // The response-mode guard must not suppress them. | ||
| let mut message = inbound_message("slack", &[], "Check the weather"); | ||
| message.sender_id = "system".into(); | ||
| message.conversation_id = "cron:daily-weather".into(); | ||
|
|
||
| // compute_listen_mode_invocation returns all false — no command/mention/reply | ||
| let (cmd, mention, reply) = compute_listen_mode_invocation(&message, "Check the weather"); | ||
| assert!(!cmd); | ||
| assert!(!mention); | ||
| assert!(!reply); | ||
|
|
||
| // The guard condition should NOT enter suppression because sender_id == "system" | ||
| let response_mode_not_active = true; // Quiet or MentionOnly | ||
| let source_is_system = message.source == "system"; // false | ||
| let sender_is_system = message.sender_id == "system"; // true | ||
| let is_dm = is_dm_conversation_id(&message.conversation_id); // false | ||
|
|
||
| // Full guard: all four conditions must be true to suppress | ||
| let would_suppress = | ||
| response_mode_not_active && !source_is_system && !sender_is_system && !is_dm; | ||
| assert!( | ||
| !would_suppress, | ||
| "cron messages (sender_id=system) must bypass response-mode suppression" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn discord_quiet_mode_ping_ack_requires_directed_ping() { | ||
| let directed_message = inbound_message( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the single-message response-mode guard, but the coalesced batch path still suppresses unsolicited traffic whenever
response_modeis notActiveand!batch_has_invoke. Cron/system-originated work can flow throughhandle_message_batchtoo, so scheduled output can still be dropped in Quiet/MentionOnly mode when messages are buffered together. Please mirror this exemption in the batch suppression branch as well.