fix: harden Discord poll handling and refresh docs#407
Merged
jamiepine merged 2 commits intospacedriveapp:mainfrom Mar 12, 2026
Merged
fix: harden Discord poll handling and refresh docs#407jamiepine merged 2 commits intospacedriveapp:mainfrom
jamiepine merged 2 commits intospacedriveapp:mainfrom
Conversation
Contributor
WalkthroughThis PR consolidates Docker image variants (slim/full) into a unified latest image model, updates related documentation, and refactors message handling code to support Discord quiet mode functionality and poll payload normalization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
Comment on lines
+224
to
+245
| let question = poll.question.trim().to_string(); | ||
| if question.is_empty() { | ||
| return None; | ||
| } | ||
|
|
||
| let answers: Vec<String> = poll | ||
| .answers | ||
| .into_iter() | ||
| .map(|answer| answer.trim().to_string()) | ||
| .filter(|answer| !answer.is_empty()) | ||
| .collect(); | ||
|
|
||
| if answers.len() < 2 { | ||
| return None; | ||
| } | ||
|
|
||
| Some(crate::Poll { | ||
| question, | ||
| answers, | ||
| allow_multiselect: poll.allow_multiselect, | ||
| duration_hours: poll.duration_hours, | ||
| }) |
Contributor
There was a problem hiding this comment.
Minor perf/readability nit: this allocates question/answer strings even for polls that get dropped. You can trim/check first and use filter_map to avoid allocating empty answers.
Suggested change
| let question = poll.question.trim().to_string(); | |
| if question.is_empty() { | |
| return None; | |
| } | |
| let answers: Vec<String> = poll | |
| .answers | |
| .into_iter() | |
| .map(|answer| answer.trim().to_string()) | |
| .filter(|answer| !answer.is_empty()) | |
| .collect(); | |
| if answers.len() < 2 { | |
| return None; | |
| } | |
| Some(crate::Poll { | |
| question, | |
| answers, | |
| allow_multiselect: poll.allow_multiselect, | |
| duration_hours: poll.duration_hours, | |
| }) | |
| let crate::Poll { | |
| question, | |
| answers, | |
| allow_multiselect, | |
| duration_hours, | |
| } = poll; | |
| let question = question.trim(); | |
| if question.is_empty() { | |
| return None; | |
| } | |
| let answers: Vec<String> = answers | |
| .into_iter() | |
| .filter_map(|answer| { | |
| let answer = answer.trim(); | |
| (!answer.is_empty()).then(|| answer.to_string()) | |
| }) | |
| .collect(); | |
| if answers.len() < 2 { | |
| return None; | |
| } | |
| Some(crate::Poll { | |
| question: question.to_string(), | |
| answers, | |
| allow_multiselect, | |
| duration_hours, | |
| }) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agent/channel.rs`:
- Around line 3125-3146: The helper looks_like_liveness_ping currently uses
broad substring checks and causes directed commands (e.g., "@bot ping deploy")
to be auto-acked; update looks_like_liveness_ping to operate on a normalized,
mention/reply-stripped token list (use compute_listen_mode_invocation or
replicate its mention-stripping) and only return true for exact or near-exact
liveness tokens/phrases (e.g., "ping", "are you there", "yo", "you here") after
tokenization — avoid contains() matches that match inside longer tokens (e.g.,
"shipping" or "deploy-ping"); then ensure
should_send_discord_quiet_mode_ping_ack continues to call the tightened
looks_like_liveness_ping and add a regression test that sends a substantive
directed message containing "ping" (like "@bot ping deploy-web") asserting it is
not treated as a liveness ping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f833fc5-6189-4557-a392-cc05fc53c93e
📒 Files selected for processing (10)
docs/content/docs/(deployment)/metrics.mdxdocs/content/docs/(getting-started)/docker.mdxdocs/content/docs/(getting-started)/quickstart.mdxdocs/content/docs/(messaging)/discord-setup.mdxdocs/docker.mddocs/metrics.mdsrc/agent/channel.rssrc/messaging/discord.rssrc/tools/reply.rssrc/tools/send_message_to_another_channel.rs
Comment on lines
+3125
to
+3146
| fn looks_like_liveness_ping(text: &str) -> bool { | ||
| let text = text.trim().to_lowercase(); | ||
| text.contains("you here") | ||
| || text.contains("ping") | ||
| || text.ends_with(" yo") | ||
| || text == "yo" | ||
| || text.contains("alive") | ||
| || text.contains("there?") | ||
| } | ||
|
|
||
| fn should_send_discord_quiet_mode_ping_ack( | ||
| message: &InboundMessage, | ||
| raw_text: &str, | ||
| listen_only_mode: bool, | ||
| ) -> bool { | ||
| if message.source != "discord" || !listen_only_mode { | ||
| return false; | ||
| } | ||
|
|
||
| let (_, invoked_by_mention, invoked_by_reply) = | ||
| compute_listen_mode_invocation(message, raw_text); | ||
| (invoked_by_mention || invoked_by_reply) && looks_like_liveness_ping(raw_text) |
Contributor
There was a problem hiding this comment.
Avoid auto-acking any directed message that contains ping.
looks_like_liveness_ping() uses raw substring checks (contains("ping"), contains("there?"), etc.), so messages like @bot ping deploy-web or @bot shipping ETA? will hit the fast-path and return "yeah i'm here" instead of reaching normal handling. Because both the Telegram shortcut and the Discord quiet-mode shortcut return early on this helper, this can swallow legitimate user requests. Please tighten this to exact/near-exact liveness phrases after tokenization/mention stripping, and add a negative regression for a substantive directed message that contains ping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agent/channel.rs` around lines 3125 - 3146, The helper
looks_like_liveness_ping currently uses broad substring checks and causes
directed commands (e.g., "@bot ping deploy") to be auto-acked; update
looks_like_liveness_ping to operate on a normalized, mention/reply-stripped
token list (use compute_listen_mode_invocation or replicate its
mention-stripping) and only return true for exact or near-exact liveness
tokens/phrases (e.g., "ping", "are you there", "yo", "you here") after
tokenization — avoid contains() matches that match inside longer tokens (e.g.,
"shipping" or "deploy-ping"); then ensure
should_send_discord_quiet_mode_ping_ack continues to call the tightened
looks_like_liveness_ping and add a regression test that sends a substantive
directed message containing "ping" (like "@bot ping deploy-web") asserting it is
not treated as a liveness ping.
jamiepine
approved these changes
Mar 12, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/agent/channel.rs (1)
3178-3200:⚠️ Potential issue | 🟠 MajorTighten liveness matching to exact phrases.
looks_like_liveness_ping()still relies on broad substring checks (contains("ping"),contains("there?"), etc.), so substantive directed messages like@bot ping deploy-weborshipping ETA?can still short-circuit into"yeah i'm here"via the Telegram path at Line 1583 and the Discord quiet-mode path at Lines 1592-1595. Please switch this to normalized exact/near-exact liveness phrases and add a negative regression for a directed message that merely containsping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 3178 - 3200, The liveness matcher looks_like_liveness_ping currently uses broad substring checks that misidentify directed messages (e.g., "@bot ping deploy-web"); change it to normalize the input (trim, lowercase) and match only exact or very near-exact phrases (e.g., "ping", "yo", "are you there", "you here", "alive?") rather than contains("ping") or contains("there?"); update should_send_discord_quiet_mode_ping_ack to rely on the tightened looks_like_liveness_ping and ensure compute_listen_mode_invocation behavior is preserved, and add a negative regression test asserting that messages like "@bot ping deploy-web" (and "shipping ETA?") do NOT trigger liveness responses.
🧹 Nitpick comments (1)
src/tools/reply.rs (1)
222-245: Keep reply-tool poll normalization aligned with the Discord validator.
normalize_poll_payload()now trims/filter-validates content, butsrc/messaging/discord.rs:1172-1208still applies extra rules (take(10)andduration_hours.clamp(1, 720)). That means the reply tool can return success for a poll payload that the Discord adapter silently changes on send. I’d rather share one normalizer/validator, or mirror those limits here and add a regression for>10answers / out-of-range duration so the tool and adapter stay in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/reply.rs` around lines 222 - 245, normalize_poll_payload currently trims and filters answers but does not enforce the same limits as the Discord adapter (it omits the answers cap and duration clamp), causing divergence; update normalize_poll_payload to apply answers.truncate(10) or reject when answers.len() > 10 (prefer rejecting to surface validation errors) and clamp duration_hours to 1..=720 (or reject if out of range) so the reply tool and src/messaging/discord.rs behavior match, and add regression tests that submit >10 answers and out-of-range duration to ensure normalization/validation stays in sync with the Discord adapter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/agent/channel.rs`:
- Around line 3178-3200: The liveness matcher looks_like_liveness_ping currently
uses broad substring checks that misidentify directed messages (e.g., "@bot ping
deploy-web"); change it to normalize the input (trim, lowercase) and match only
exact or very near-exact phrases (e.g., "ping", "yo", "are you there", "you
here", "alive?") rather than contains("ping") or contains("there?"); update
should_send_discord_quiet_mode_ping_ack to rely on the tightened
looks_like_liveness_ping and ensure compute_listen_mode_invocation behavior is
preserved, and add a negative regression test asserting that messages like "@bot
ping deploy-web" (and "shipping ETA?") do NOT trigger liveness responses.
---
Nitpick comments:
In `@src/tools/reply.rs`:
- Around line 222-245: normalize_poll_payload currently trims and filters
answers but does not enforce the same limits as the Discord adapter (it omits
the answers cap and duration clamp), causing divergence; update
normalize_poll_payload to apply answers.truncate(10) or reject when
answers.len() > 10 (prefer rejecting to surface validation errors) and clamp
duration_hours to 1..=720 (or reject if out of range) so the reply tool and
src/messaging/discord.rs behavior match, and add regression tests that submit
>10 answers and out-of-range duration to ensure normalization/validation stays
in sync with the Discord adapter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1231b545-b63a-4c45-bd84-dea99b99df01
📒 Files selected for processing (2)
src/agent/channel.rssrc/tools/reply.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
replyand validate again in the Discord adapter before sendclippyfindings insend_message_to_another_channeluncovered byjust gate-prTest Plan
just preflightjust gate-prcargo test test_prepare_rich_message_parts_drops_invalid_poll_but_keeps_text -- --nocapturecargo test quiet_mode_invocation_uses_discord_mention_and_reply_metadata -- --nocaptureP1/P2 Finding Closure
src/tools/reply.rs+src/messaging/discord.rs; verified bycargo test test_prepare_rich_message_parts_drops_invalid_poll_but_keeps_text -- --nocaptureandjust gate-pr(pass)src/agent/channel.rs; verified bycargo test quiet_mode_invocation_uses_discord_mention_and_reply_metadata -- --nocaptureandjust gate-pr(pass)docs/docker.md,docs/content/docs/(getting-started)/docker.mdx,docs/metrics.md,docs/content/docs/(deployment)/metrics.mdx; verified byrg -n \"slim|full image|image variants\" docs/docker.md docs/metrics.md docs/content/docs/\\(getting-started\\)/docker.mdx docs/content/docs/\\(getting-started\\)/quickstart.mdx docs/content/docs/\\(deployment\\)/metrics.mdxandjust gate-pr(pass; only legacy deprecation notes remain)Residual Risk
Note
Addresses Discord poll validation hardening, rich-message test coverage, and documentation updates for the unified image model. Includes sanitization of invalid poll payloads in both the reply tool and Discord adapter, new regression tests for quiet-mode routing, and updates to Docker/metrics documentation. All gates pass.
Written by Tembo for commit bfaf70f.