-
Notifications
You must be signed in to change notification settings - Fork 324
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 1 commit
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 |
|---|---|---|
|
|
@@ -1759,7 +1759,13 @@ impl Channel { | |
|
|
||
| // Listen-first guardrail: | ||
| // ingest all messages, but only reply when explicitly invoked. | ||
| if self.listen_only_mode && message.source != "system" && !self.is_dm() { | ||
| // Cron-originated messages carry a platform source (e.g., "slack") for adapter | ||
| // routing but use sender_id="system" — exempt them from suppression. | ||
| if self.listen_only_mode | ||
| && message.source != "system" | ||
| && message.sender_id != "system" | ||
| && !self.is_dm() | ||
| { | ||
| (invoked_by_command, invoked_by_mention, invoked_by_reply) = | ||
| self.compute_listen_mode_invocation(&message, &raw_text); | ||
|
|
||
|
|
@@ -3684,6 +3690,36 @@ 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 listen_only_guard_allows_cron_messages_through() { | ||
| // Cron messages have source="slack" (for adapter routing) but sender_id="system". | ||
| // The listen-only 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 listen_only = true; | ||
| 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 = | ||
| listen_only && !source_is_system && !sender_is_system && !is_dm; | ||
| assert!( | ||
| !would_suppress, | ||
| "cron messages (sender_id=system) must bypass listen-only suppression" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn discord_quiet_mode_ping_ack_requires_directed_ping() { | ||
| let directed_message = inbound_message( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.