From e2efebcf05f93da98fabf012eacf57f05dddfb40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B6=85=E6=B8=A1=E6=B3=95=E5=B8=AB?= Date: Mon, 1 Jun 2026 14:30:56 +0000 Subject: [PATCH 1/3] feat(discord): trusted bot @mention bypasses involvement gate Allow trusted bots to @mention other bots into threads, treating the mention the same as a human @mention. Previously, all bot messages were blocked by the allow_bot_messages mode check (defaults to Off), even from trusted bots. Now, if a bot is in trusted_bot_ids AND explicitly @mentions this bot, the mode check is skipped entirely. This enables bot-to-bot coordination (e.g. a coordinator bot pulling reviewer bots into threads) without requiring human intervention for every thread. --- src/discord.rs | 167 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 111 insertions(+), 56 deletions(-) diff --git a/src/discord.rs b/src/discord.rs index af8364966..df66fe952 100644 --- a/src/discord.rs +++ b/src/discord.rs @@ -457,68 +457,75 @@ impl EventHandler for Handler { // Bot message gating (from upstream #321) if msg.author.bot { - match self.allow_bot_messages { - AllowBots::Off => return, - AllowBots::Mentions => { - if !is_mentioned { - return; + // Trusted bot @mention bypass: treat as human mention, skip mode check. + let trusted_mention = is_mentioned + && !self.trusted_bot_ids.is_empty() + && self.trusted_bot_ids.contains(&msg.author.id.get()); + + if !trusted_mention { + match self.allow_bot_messages { + AllowBots::Off => return, + AllowBots::Mentions => { + if !is_mentioned { + return; + } } - } - AllowBots::All => { - let cap = MAX_CONSECUTIVE_BOT_TURNS as usize; - let limit = std::cmp::min(MAX_CONSECUTIVE_BOT_TURNS, 100) as u8; - let history = ctx - .cache - .channel_messages(msg.channel_id) - .map(|msgs| { - let mut recent: Vec<_> = msgs - .iter() - .filter(|(mid, _)| **mid < msg.id) - .map(|(_, m)| m.clone()) - .collect(); - recent.sort_unstable_by_key(|m| std::cmp::Reverse(m.id)); - recent.truncate(cap); - recent - }) - .filter(|msgs| !msgs.is_empty()); - - let recent = if let Some(cached) = history { - cached - } else { - match msg - .channel_id - .messages( - &ctx.http, - serenity::builder::GetMessages::new() - .before(msg.id) - .limit(limit), - ) - .await - { - Ok(msgs) => msgs, - Err(e) => { - tracing::warn!(channel_id = %msg.channel_id, error = %e, "failed to fetch history for bot turn cap, rejecting (fail-closed)"); - return; + AllowBots::All => { + let cap = MAX_CONSECUTIVE_BOT_TURNS as usize; + let limit = std::cmp::min(MAX_CONSECUTIVE_BOT_TURNS, 100) as u8; + let history = ctx + .cache + .channel_messages(msg.channel_id) + .map(|msgs| { + let mut recent: Vec<_> = msgs + .iter() + .filter(|(mid, _)| **mid < msg.id) + .map(|(_, m)| m.clone()) + .collect(); + recent.sort_unstable_by_key(|m| std::cmp::Reverse(m.id)); + recent.truncate(cap); + recent + }) + .filter(|msgs| !msgs.is_empty()); + + let recent = if let Some(cached) = history { + cached + } else { + match msg + .channel_id + .messages( + &ctx.http, + serenity::builder::GetMessages::new() + .before(msg.id) + .limit(limit), + ) + .await + { + Ok(msgs) => msgs, + Err(e) => { + tracing::warn!(channel_id = %msg.channel_id, error = %e, "failed to fetch history for bot turn cap, rejecting (fail-closed)"); + return; + } } + }; + + let consecutive_bot = recent + .iter() + .take_while(|m| m.author.bot && m.author.id != bot_id) + .count(); + if consecutive_bot >= cap { + tracing::warn!(channel_id = %msg.channel_id, cap, "bot turn cap reached, ignoring"); + return; } - }; - - let consecutive_bot = recent - .iter() - .take_while(|m| m.author.bot && m.author.id != bot_id) - .count(); - if consecutive_bot >= cap { - tracing::warn!(channel_id = %msg.channel_id, cap, "bot turn cap reached, ignoring"); - return; } } - } - if !self.trusted_bot_ids.is_empty() - && !self.trusted_bot_ids.contains(&msg.author.id.get()) - { - tracing::debug!(bot_id = %msg.author.id, "bot not in trusted_bot_ids, ignoring"); - return; + if !self.trusted_bot_ids.is_empty() + && !self.trusted_bot_ids.contains(&msg.author.id.get()) + { + tracing::debug!(bot_id = %msg.author.id, "bot not in trusted_bot_ids, ignoring"); + return; + } } } @@ -2170,6 +2177,18 @@ fn is_denied_user( !is_bot && !allow_all_users && !allowed_users.contains(&user_id) } +/// Returns `true` if a bot message should bypass the `allow_bot_messages` mode check. +/// A trusted bot that @mentions this bot is treated the same as a human @mention — +/// it can pull the bot into a thread regardless of the `allow_bot_messages` setting. +#[cfg(test)] +fn is_trusted_bot_mention( + is_mentioned: bool, + trusted_bot_ids: &HashSet, + author_id: u64, +) -> bool { + is_mentioned && !trusted_bot_ids.is_empty() && trusted_bot_ids.contains(&author_id) +} + /// Pure decision function: should a DM be processed? /// Returns `true` if the DM should be processed (bot responds). /// Mirrors the DM gating logic in EventHandler::message: @@ -2939,6 +2958,42 @@ mod tests { assert!(!is_denied_user(true, false, &allowed, 999)); } + // --- Trusted bot mention bypass tests --- + // A trusted bot @mentioning this bot bypasses allow_bot_messages mode, + // treating the mention the same as a human @mention. + + /// GIVEN: trusted bot @mentions this bot + /// THEN: bypass is granted (treated as human mention) + #[test] + fn trusted_bot_mention_bypasses_gate() { + let trusted = HashSet::from([42]); + assert!(is_trusted_bot_mention(true, &trusted, 42)); + } + + /// GIVEN: untrusted bot @mentions this bot + /// THEN: no bypass (normal bot gating applies) + #[test] + fn untrusted_bot_mention_no_bypass() { + let trusted = HashSet::from([42]); + assert!(!is_trusted_bot_mention(true, &trusted, 99)); + } + + /// GIVEN: trusted bot sends message WITHOUT @mention + /// THEN: no bypass (must explicitly @mention) + #[test] + fn trusted_bot_no_mention_no_bypass() { + let trusted = HashSet::from([42]); + assert!(!is_trusted_bot_mention(false, &trusted, 42)); + } + + /// GIVEN: empty trusted_bot_ids (feature not configured) + /// THEN: no bypass regardless of mention + #[test] + fn empty_trusted_ids_no_bypass() { + let trusted: HashSet = HashSet::new(); + assert!(!is_trusted_bot_mention(true, &trusted, 42)); + } + // --- DM gating tests (#656) --- // DMs are gated by `allow_dm` config. When allowed, DMs bypass // `allowed_channels` and treat the message as implicit @mention. From f1b1a047910302d6916c7be92c48e9e17aed4a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B6=85=E6=B8=A1=E6=B3=95=E5=B8=AB?= Date: Mon, 1 Jun 2026 15:33:03 +0000 Subject: [PATCH 2/3] address review: clarify trusted_bot_ids semantic + add integration tests - Expanded code comment to explain this is a 'bot admission override' not just an involvement gate bypass - Updated config doc for trusted_bot_ids to document the @mention override behavior - Added should_admit_bot_message() integration test helper that mirrors the actual gating logic in EventHandler::message - Added 6 integration tests covering: Off+trusted mention (pass), Off+untrusted (block), Off+trusted no mention (block), Off+empty trusted_ids (block), Mentions+trusted (pass), All+untrusted (block) --- src/config.rs | 4 +++ src/discord.rs | 98 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 934a622aa..6fe25a807 100644 --- a/src/config.rs +++ b/src/config.rs @@ -203,6 +203,10 @@ pub struct DiscordConfig { /// the allowlist filters further. Empty = allow any bot (mode permitting). /// Only relevant when `allow_bot_messages` is `"mentions"` or `"all"`; /// ignored when `"off"` since all bot messages are rejected before this check. + /// + /// **Admission override**: a trusted bot that explicitly @mentions this bot + /// bypasses the `allow_bot_messages` mode entirely (treated as human @mention). + /// This allows trusted bots to pull this bot into threads regardless of mode. #[serde(default)] pub trusted_bot_ids: Vec, #[serde(default)] diff --git a/src/discord.rs b/src/discord.rs index df66fe952..055228de8 100644 --- a/src/discord.rs +++ b/src/discord.rs @@ -457,7 +457,20 @@ impl EventHandler for Handler { // Bot message gating (from upstream #321) if msg.author.bot { - // Trusted bot @mention bypass: treat as human mention, skip mode check. + // Trusted bot admission override: when a bot listed in `trusted_bot_ids` + // explicitly @mentions this bot, bypass the entire `allow_bot_messages` + // mode check. This treats the trusted bot's @mention identically to a + // human @mention — the bot becomes involved in the thread and the message + // is dispatched regardless of the `allow_bot_messages` setting. + // + // Rationale: `trusted_bot_ids` expresses admin-level trust. A trusted bot + // that @mentions this bot is performing a deliberate handoff/coordination + // action, equivalent to a human pulling the bot into a conversation. + // + // Safety: requires both (1) explicit @mention AND (2) sender in + // trusted_bot_ids. Messages from trusted bots without @mention still + // follow normal gating. Empty trusted_bot_ids (default) disables this + // entirely — no behavioral change for existing deployments. let trusted_mention = is_mentioned && !self.trusted_bot_ids.is_empty() && self.trusted_bot_ids.contains(&msg.author.id.get()); @@ -2994,6 +3007,89 @@ mod tests { assert!(!is_trusted_bot_mention(true, &trusted, 42)); } + // --- Trusted bot admission integration tests --- + // These test the full bot gating decision path: allow_bot_messages mode + + // trusted_bot_ids + trusted mention bypass, mirroring the actual logic in + // EventHandler::message. + + /// Simulates the bot admission decision from EventHandler::message. + /// Returns `true` if the bot message would be processed (not dropped). + fn should_admit_bot_message( + allow_bot_messages: AllowBots, + is_mentioned: bool, + trusted_bot_ids: &HashSet, + author_id: u64, + ) -> bool { + let trusted_mention = is_mentioned + && !trusted_bot_ids.is_empty() + && trusted_bot_ids.contains(&author_id); + + if !trusted_mention { + match allow_bot_messages { + AllowBots::Off => return false, + AllowBots::Mentions => { + if !is_mentioned { + return false; + } + } + AllowBots::All => {} // would check consecutive cap, skip for unit test + } + + if !trusted_bot_ids.is_empty() && !trusted_bot_ids.contains(&author_id) { + return false; + } + } + true + } + + /// GIVEN: allow_bot_messages=Off, trusted bot @mentions this bot + /// THEN: admitted (trusted mention overrides Off mode) + #[test] + fn bot_admission_trusted_mention_overrides_off() { + let trusted = HashSet::from([42]); + assert!(should_admit_bot_message(AllowBots::Off, true, &trusted, 42)); + } + + /// GIVEN: allow_bot_messages=Off, untrusted bot @mentions this bot + /// THEN: rejected (Off mode blocks) + #[test] + fn bot_admission_untrusted_mention_blocked_by_off() { + let trusted = HashSet::from([42]); + assert!(!should_admit_bot_message(AllowBots::Off, true, &trusted, 99)); + } + + /// GIVEN: allow_bot_messages=Off, trusted bot without @mention + /// THEN: rejected (no mention = no bypass) + #[test] + fn bot_admission_trusted_no_mention_blocked_by_off() { + let trusted = HashSet::from([42]); + assert!(!should_admit_bot_message(AllowBots::Off, false, &trusted, 42)); + } + + /// GIVEN: allow_bot_messages=Off, empty trusted_bot_ids, bot @mentions + /// THEN: rejected (feature not configured) + #[test] + fn bot_admission_empty_trusted_ids_off_mode() { + let trusted: HashSet = HashSet::new(); + assert!(!should_admit_bot_message(AllowBots::Off, true, &trusted, 42)); + } + + /// GIVEN: allow_bot_messages=Mentions, trusted bot @mentions + /// THEN: admitted (would pass anyway, but bypass also works) + #[test] + fn bot_admission_mentions_mode_trusted_mention() { + let trusted = HashSet::from([42]); + assert!(should_admit_bot_message(AllowBots::Mentions, true, &trusted, 42)); + } + + /// GIVEN: allow_bot_messages=All, untrusted bot (not in trusted_bot_ids) + /// THEN: rejected by trusted_bot_ids filter + #[test] + fn bot_admission_all_mode_untrusted_bot_rejected() { + let trusted = HashSet::from([42]); + assert!(!should_admit_bot_message(AllowBots::All, false, &trusted, 99)); + } + // --- DM gating tests (#656) --- // DMs are gated by `allow_dm` config. When allowed, DMs bypass // `allowed_channels` and treat the message as implicit @mention. From 675a5e2a38f192e2a94f2b2825b669a3f2f38943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B6=85=E6=B8=A1=E6=B3=95=E5=B8=AB?= Date: Mon, 1 Jun 2026 15:36:38 +0000 Subject: [PATCH 3/3] docs: update involvement gate docs for trusted bot admission override - messaging.md: update design principle, gate flow diagram, practical impact table, config table, and add dedicated 'Trusted bot admission override' section - discord.md: update trusted_bot_ids section and involvement gate examples to show trusted bot path - config-reference.md: update trusted_bot_ids description --- docs/config-reference.md | 2 +- docs/discord.md | 16 +++++++++++----- docs/messaging.md | 30 ++++++++++++++++++++++++++---- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/docs/config-reference.md b/docs/config-reference.md index 84037914a..b25228a7e 100644 --- a/docs/config-reference.md +++ b/docs/config-reference.md @@ -37,7 +37,7 @@ Discord adapter. Requires a Discord bot token. | `allow_all_users` | bool \| omit | auto-detect | `true` = any user; `false` = only `allowed_users`. Omitted = inferred from list. | | `allowed_users` | string[] | `[]` | User IDs to allow. Only checked when `allow_all_users` resolves to false. | | `allow_bot_messages` | string | `"off"` | `"off"` — ignore all bot messages. `"mentions"` — only process bot messages that @mention this bot. `"all"` — process all bot messages (capped by `max_bot_turns`). | -| `trusted_bot_ids` | string[] | `[]` | When non-empty, only these bot IDs pass the bot gate. Empty = any bot (mode permitting). Ignored when `allow_bot_messages = "off"`. | +| `trusted_bot_ids` | string[] | `[]` | When non-empty, only these bot IDs pass the bot gate. Empty = any bot (mode permitting). **Admission override:** a trusted bot that @mentions this bot bypasses `allow_bot_messages` mode entirely (treated as human @mention, can pull bot into threads). | | `allow_user_messages` | string | `"involved"` | `"involved"` — reply in threads bot has participated in without @mention; channel messages require @mention; DMs always process. `"mentions"` — always require @mention. `"multibot-mentions"` — like `"involved"`, but require @mention once another bot has posted in the thread. | | `allow_dm` | bool | `false` | `true` = respond to Discord DMs; `false` = ignore DMs. `allowed_users` still applies in DMs. Each DM user consumes one session slot. | | `max_bot_turns` | u32 | `100` | Max consecutive bot turns per thread before throttling (soft limit). Human message resets the counter. A compiled-in hard cap of 1000 consecutive bot messages is always enforced. | diff --git a/docs/discord.md b/docs/discord.md index 4ef210ba0..fd185f149 100644 --- a/docs/discord.md +++ b/docs/discord.md @@ -134,6 +134,8 @@ trusted_bot_ids = ["123456789012345678"] # only this bot's messages pass throug Empty (default) = any bot can pass through (subject to the mode check). +**Admission override:** A trusted bot that explicitly @mentions this bot bypasses the `allow_bot_messages` mode entirely — the mention is treated the same as a human @mention. This allows trusted bots to pull this bot into threads even when `allow_bot_messages = "off"`. Messages from trusted bots *without* @mention still follow normal gating. + ### `allowed_role_ids` Role IDs that trigger the bot, same as a direct @mention. This enables users to invoke multiple bots simultaneously with a single role mention (e.g. `@AllBots review this`). @@ -282,19 +284,23 @@ In a multi-bot setup, every bot enforces an **involvement gate** before processi **Rule:** A bot must be **involved** (thread owner or has previously replied) before it will process any message in that thread. -**Key constraint:** Only a human @mention can pull a bot into a thread for the first time. A bot @mentioning another bot that is not yet involved will be **silently dropped**. +**Key constraint:** Only a human @mention — or a @mention from a bot in `trusted_bot_ids` — can pull a bot into a thread for the first time. A @mention from an untrusted bot will be **silently dropped**. ``` -Bot A's thread (Bot B not yet involved): +Bot A's thread (Bot B not yet involved, Bot A NOT in Bot B's trusted_bot_ids): - Bot A: "@Bot_B please review this" → ❌ dropped (Bot B not involved) + Bot A: "@Bot_B please review this" → ❌ dropped (Bot B not involved, Bot A untrusted) Human: "@Bot_B please review this" → ✅ Bot B replies, now involved Bot A: "@Bot_B any updates?" → ✅ processed (Bot B is involved) + +Bot A's thread (Bot B not yet involved, Bot A IS in Bot B's trusted_bot_ids): + + Bot A: "@Bot_B please review this" → ✅ treated as human @mention, Bot B becomes involved ``` -**Why:** This prevents bots from pulling other bots into arbitrary threads without human consent, protects session pool resources, and eliminates cross-thread chain reactions. +**Why:** This prevents untrusted bots from pulling other bots into arbitrary threads without human consent, protects session pool resources, and eliminates cross-thread chain reactions. Trusted bots are explicitly authorized by the admin. -**Workaround:** Pre-involve all needed bots at thread creation by @mentioning them (or using a shared role via `allowed_role_ids`). +**Workaround (without trusted_bot_ids):** Pre-involve all needed bots at thread creation by @mentioning them (or using a shared role via `allowed_role_ids`). > 📖 Full design details: [docs/messaging.md — Involvement Gate](messaging.md#involvement-gate) diff --git a/docs/messaging.md b/docs/messaging.md index 8cd81a049..d13a9e5a1 100644 --- a/docs/messaging.md +++ b/docs/messaging.md @@ -187,7 +187,7 @@ BotA in thread: here's my analysis | Key | Type | Default | Description | |-----|------|---------|-------------| | `allow_bot_messages` | string | `"off"` | `"off"` — ignore bot messages. `"mentions"` — only process bot messages that @mention this bot. `"all"` — process all bot messages (capped by `max_bot_turns`). | -| `trusted_bot_ids` | string[] | `[]` | Whitelist of bot IDs. For Slack, entries may be Bot User IDs (`U...`) or Bot IDs (`B...`); `U...` matching requires `users:read` so OpenAB can call `bots.info`. Empty = any bot (mode permitting). Ignored when `allow_bot_messages = "off"`. | +| `trusted_bot_ids` | string[] | `[]` | Whitelist of bot IDs. For Slack, entries may be Bot User IDs (`U...`) or Bot IDs (`B...`); `U...` matching requires `users:read` so OpenAB can call `bots.info`. Empty = any bot (mode permitting). **Admission override:** a trusted bot that @mentions this bot bypasses `allow_bot_messages` mode entirely (treated as human @mention). | | `max_bot_turns` | u32 | `20` | Max consecutive bot turns per thread before throttling. A human message resets the counter. | > **Safety:** When `allow_bot_messages = "all"`, a separate hardcoded cap of 10 consecutive bot turns applies regardless of `max_bot_turns`. @@ -267,7 +267,7 @@ All message routing in OpenAB is guarded by the **involvement gate** — a pre-d ### Design principle -**Humans are the gatekeepers.** A bot cannot participate in a thread until a human explicitly pulls it in via @mention. Bots cannot pull other bots into threads — only humans can. +**Humans are the gatekeepers.** A bot cannot participate in a thread until a human explicitly pulls it in via @mention. Bots cannot pull other bots into threads — only humans can, **unless** the sending bot is in the target bot's `trusted_bot_ids` (see [Trusted bot admission override](#trusted-bot-admission-override) below). ### How a bot becomes involved @@ -297,7 +297,11 @@ Inbound message in thread │ │ │ ├─ From a human → pass (bot will reply and become involved) │ │ - │ └─ From another bot → ❌ DROP (bot-to-bot cannot break the gate) + │ └─ From another bot: + │ │ + │ ├─ Sender in trusted_bot_ids → pass (same as human @mention) + │ │ + │ └─ Otherwise → ❌ DROP (bot-to-bot cannot break the gate) │ └─ Message dropped — never reaches Dispatcher or SessionPool ``` @@ -318,8 +322,26 @@ This is an intentional safety constraint: | Bot A @mentions Bot B (Bot B not yet involved) | ❌ Silently dropped | | Bot A @mentions Bot B (Bot B already involved) | ✅ Processed per `allow_bot_messages` mode | | Human @mentions Bot B, then Bot A @mentions Bot B | ✅ Works — Bot B is already involved | +| **Trusted** Bot A @mentions Bot B (Bot A in Bot B's `trusted_bot_ids`) | ✅ Treated as human @mention — Bot B becomes involved | -### Workaround for bot-to-bot handoff +### Trusted bot admission override + +When a bot is listed in another bot's `trusted_bot_ids` and explicitly @mentions that bot, the mention is treated identically to a human @mention: + +- The target bot becomes **involved** in the thread +- The `allow_bot_messages` mode check is **bypassed** entirely +- The message is dispatched to the session + +This enables bot-to-bot coordination (e.g. a coordinator bot pulling reviewer bots into threads) without requiring human intervention for every thread. + +**Requirements:** +- The sending bot must be in the target bot's `trusted_bot_ids` config +- The sending bot must explicitly @mention the target bot +- Messages from trusted bots **without** @mention still follow normal `allow_bot_messages` gating + +**Safety:** `trusted_bot_ids` defaults to empty — this feature is entirely opt-in. The `max_bot_turns` cap still applies after involvement to prevent runaway loops. + +### Workaround for bot-to-bot handoff (without trusted_bot_ids) If you need Bot A to hand off to Bot B in a thread where Bot B is not yet involved: