feat(discord): support role mention as trigger (allowed_role_ids)#759
feat(discord): support role mention as trigger (allowed_role_ids)#759
Conversation
This comment has been minimized.
This comment has been minimized.
6d15d9c to
864e92b
Compare
OpenAB PR ScreeningThis is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Screening report## IntentPR #759 adds Discord role mentions as a valid bot trigger through a new FeatFeature: Discord trigger expansion. The change allows configured Discord role IDs to behave like direct bot mentions. When a message mentions one of those roles, OpenAB treats the message as addressed to the bot and removes the triggering role mention from the prompt before passing text downstream. Default behavior remains unchanged because Who It ServesPrimary beneficiaries are Discord end users and OpenAB deployers. End users get a simpler invocation path for multi-bot workflows. Deployers get an explicit allowlist so only selected Discord roles can trigger bots, avoiding broad role-mention activation by default. Rewritten PromptImplement support for Discord role mentions as bot triggers. Add a Discord config field Update config examples and Helm chart values/templates to expose the setting as Merge PitchThis is a small, practical Discord feature with a clear operator opt-in and low default-risk profile. It improves multi-agent Discord workflows without changing behavior for existing deployments. Likely reviewer concern: role mention handling must not accidentally allow every mentioned role to trigger the bot or strip non-triggering role mentions from user prompts. The key review points are config parsing, exact ID matching, and preserving existing mention-resolution behavior. Best-Practice ComparisonOpenClaw principles relevant here:
Hermes Agent principles relevant here:
Overall, the proposed direction fits the “explicit routing” principle well: role mentions should be opt-in, configured by stable IDs, and handled before prompt construction. Implementation OptionsConservative option: merge the current narrow feature if tests confirm exact allowlist matching and prompt stripping. Keep Balanced option: merge role-trigger support but refactor mention-trigger checks into a small helper that returns both trigger status and matched trigger role IDs. This reduces drift between Ambitious option: introduce a generalized Discord trigger policy model covering bot mentions, role mentions, channel allowlists, user allowlists, and future trigger types. This could unify message admission and prompt cleanup but is larger than this PR needs. Comparison Table
RecommendationAdvance this PR using the balanced path if the current code has duplicated trigger/cleanup logic. The feature is worth moving forward, but the safest merge shape is to ensure trigger detection and prompt stripping share the same source of truth for matched allowed role IDs. If reviewers want to keep the PR smaller, the conservative path is acceptable as long as tests cover: empty config no-op, allowed role triggers, non-allowed role does not trigger, triggering role is stripped, and non-triggering role mentions remain represented in the prompt. Broader trigger-policy work should be split into a later design issue rather than blocking this feature. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
LGTM ✅ — Clean, minimal implementation of role-mention-as-trigger. Backward compatible, well-tested, follows existing patterns. 四問框架 (Four Questions)1. What problem does it solve?Users with multiple bots (e.g. 4 法師) must @mention each bot individually. Discord role mentions ( 2. How does it solve it?
3. What was considered?
4. Is this the best approach?Yes. The implementation is minimal (+66/-10), follows established patterns exactly (same Traffic Light🟢 INFO — Reuses 🟢 INFO — 🟢 INFO — Two new unit tests cover the key behaviors (allowed role stripped, non-allowed role becomes placeholder). 🟢 INFO — Helm chart validation block is identical in structure to |
Add allowed_role_ids config field to DiscordConfig. When a message mentions a role in this list, it is treated as equivalent to a direct @mention for trigger purposes. - src/config.rs: add allowed_role_ids field (default empty) - src/discord.rs: extend is_mentioned to check msg.mention_roles against allowed_role_ids; update resolve_mentions to strip triggering role mentions from prompt - src/main.rs: parse allowed_role_ids via parse_id_set, pass to Handler - charts/openab: add allowedRoleIds with snowflake validation - config.toml.example: document new field Closes #758 Discord Discussion URL: https://discord.com/channels/1488041051187974246/1501546581105705012
864e92b to
abe1ab7
Compare
|
<@1490365068863606784> Re-review feedback from 擺渡法師, cc <@1493128125402320996>.\n\nBlocking: PR #759 is now behind current |
|
<@1490365068863606784> Re-review after rebase from 擺渡法師, cc <@1493128125402320996> <@1496553369442189472>.\n\nLGTM on the rebase. I refreshed PR head/merge refs and confirmed the merge ref is now based on latest |
cffd3f3 to
d08c526
Compare
Update docs/discord.md: - Add allowed_role_ids config reference section with setup steps - Update @Mention Behavior to include role mention trigger - Update Helm Values example with allowedRoleIds - Update troubleshooting to reflect role mention support
d08c526 to
e77bcb3
Compare
Summary
Closes #758
Add
allowed_role_idsconfig field so that role mentions (e.g.@法師團) trigger the bot, same as a direct@mention. This enables users to invoke multiple bots simultaneously with a single role mention.Changes
src/config.rsallowed_role_ids: Vec<String>toDiscordConfigsrc/discord.rsis_mentionedto checkmsg.mention_rolesagainstallowed_role_ids; updateresolve_mentions()to strip triggering role mentions from promptsrc/main.rsallowed_role_idsviaparse_id_set, pass to Handlercharts/openab/templates/configmap.yamlallowed_role_idswith snowflake ID validationcharts/openab/values.yamlallowedRoleIdsfield with documentationconfig.toml.exampleConfig
helm install openab openab/openab \ --set-string 'agents.kiro.discord.allowedRoleIds[0]=1234567890123456789'Behavior
allowed_role_ids(default) = no change, backward compatible@mention@(role)placeholderallow_user_messagesmodes (Involved, Mentions, MultibotMentions)Tests
cargo check✅cargo test— 240/240 passed ✅ (including 2 new role mention tests)helm lint— not run (no helm binary in CI env)Discord Discussion URL
https://discord.com/channels/1488041051187974246/1501546581105705012