refactor(bot_turns): address PR openabdev#487 review comments

- Fix slack.rs MultibotMentions gating: require !mentions_bot so the
  logic matches Discord parity and survives changes to the earlier
  mention-dedup path (comment 4290258138).
- Extract shared TurnAction/TurnSeverity helper in bot_turns.rs so
  Discord and Slack stop duplicating the ~30-line TurnResult match
  (comment 4290267743). Added byte-for-byte and per-thread independence
  tests for classify_bot_message.

Author: dogzzdogzz

src/discord.rs

@@ -1,7 +1,7 @@
 use crate::acp::ContentBlock;
 use crate::acp::protocol::ConfigOption;
 use crate::adapter::{AdapterRouter, ChatAdapter, ChannelRef, MessageRef, SenderContext};
-use crate::bot_turns::{BotTurnTracker, TurnResult, HARD_BOT_TURN_LIMIT};
+use crate::bot_turns::{BotTurnTracker, TurnAction, TurnSeverity};
 use crate::config::{AllowBots, AllowUsers, SttConfig};
 use crate::format;
 use crate::media;
@@ -10,7 +10,7 @@ use std::sync::LazyLock;
 use serenity::builder::{CreateActionRow, CreateCommand, CreateInteractionResponse, CreateInteractionResponseMessage, CreateSelectMenu, CreateSelectMenuKind, CreateSelectMenuOption, CreateThread, EditMessage};
 use serenity::http::Http;
 use serenity::model::application::{ComponentInteractionDataKind, Interaction};
-use serenity::model::channel::{AutoArchiveDuration, Message, MessageType, ReactionType};
+use serenity::model::channel::{AutoArchiveDuration, ChannelType, Message, MessageType, ReactionType};
 use serenity::model::gateway::Ready;
 use serenity::model::id::{ChannelId, MessageId, UserId};
 use serenity::prelude::*;
@@ -259,30 +259,26 @@ impl EventHandler for Handler {
             let thread_key = msg.channel_id.to_string();
             let mut tracker = self.bot_turns.lock().await;
             if msg.author.bot {
-                match tracker.on_bot_message(&thread_key) {
-                    TurnResult::HardLimit => {
-                        tracing::warn!(channel_id = %msg.channel_id, "hard bot turn limit reached");
-                        if msg.author.id != bot_id {
-                            let _ = msg.channel_id.say(
-                                &ctx.http,
-                                format!("🛑 Hard bot turn limit reached ({HARD_BOT_TURN_LIMIT}). A human must reply to continue."),
-                            ).await;
+                match tracker.classify_bot_message(&thread_key) {
+                    TurnAction::Continue => {}
+                    TurnAction::SilentStop => return,
+                    TurnAction::WarnAndStop { severity, user_message } => {
+                        match severity {
+                            TurnSeverity::Hard => tracing::warn!(
+                                channel_id = %msg.channel_id,
+                                "hard bot turn limit reached",
+                            ),
+                            TurnSeverity::Soft => tracing::info!(
+                                channel_id = %msg.channel_id,
+                                max = self.max_bot_turns,
+                                "soft bot turn limit reached",
+                            ),
                         }
-                        return;
-                    }
-                    TurnResult::Stopped => return,
-                    TurnResult::SoftLimit(n) => {
-                        tracing::info!(channel_id = %msg.channel_id, turns = n, max = self.max_bot_turns, "soft bot turn limit reached");
                         if msg.author.id != bot_id {
-                            let _ = msg.channel_id.say(
-                                &ctx.http,
-                                format!("⚠️ Bot turn limit reached ({n}/{}). A human must reply in this thread to continue bot-to-bot conversation.", self.max_bot_turns),
-                            ).await;
+                            let _ = msg.channel_id.say(&ctx.http, &user_message).await;
                         }
                         return;
                     }
-                    TurnResult::Throttled => return,
-                    TurnResult::Ok => {}
                 }
             } else if matches!(msg.kind, MessageType::Regular | MessageType::InlineReply)
                 && !msg.content.is_empty()
@@ -359,9 +355,12 @@ impl EventHandler for Handler {
 
         // Thread detection: single to_channel() call for both allowed and
         // non-allowed channels. A message is "in a thread" when the channel
-        // has a parent_id AND the parent is in the allowlist (or allow_all).
+        // type is a thread variant AND the parent is in the allowlist (or allow_all).
         let (in_thread, bot_owns_thread) = match msg.channel_id.to_channel(&ctx.http).await {
-            Ok(serenity::model::channel::Channel::Guild(gc)) if gc.parent_id.is_some() => {
+            Ok(serenity::model::channel::Channel::Guild(gc))
+                if is_thread_channel(gc.kind) =>
+            {
+                // parent_id here points from thread → parent channel (not channel → category)
                 let parent_allowed = in_allowed_channel
                     || self.allow_all_channels
                     || gc.parent_id.is_some_and(|pid| self.allowed_channels.contains(&pid.get()));
@@ -864,6 +863,12 @@ fn resolve_mentions(content: &str, bot_id: UserId) -> String {
     out.trim().to_string()
 }
 
+/// Returns `true` if the given `ChannelType` is a Discord thread.
+/// Extracted for testability and to centralise thread detection logic.
+fn is_thread_channel(kind: ChannelType) -> bool {
+    matches!(kind, ChannelType::PublicThread | ChannelType::PrivateThread | ChannelType::NewsThread)
+}
+
 /// Pure decision function: should this message be processed or ignored?
 /// Returns `true` if the message should be processed (bot responds).
 /// Extracted from the EventHandler::message gating logic for testability.
@@ -1069,4 +1074,46 @@ mod tests {
             false,          // other_bot_present
         ));
     }
+
+    // --- is_thread_channel tests (regression for #518) ---
+    // PR #506 used parent_id.is_some() to detect threads, but category text
+    // channels also have parent_id (pointing to the category). This caused
+    // the bot to skip thread creation for normal channels inside categories.
+
+    /// Regression test for #518: a text channel inside a category has parent_id
+    /// set but is NOT a thread — is_thread_channel must return false.
+    #[test]
+    fn category_text_channel_is_not_thread() {
+        assert!(!is_thread_channel(ChannelType::Text));
+    }
+
+    /// Category channel itself is not a thread.
+    #[test]
+    fn category_channel_is_not_thread() {
+        assert!(!is_thread_channel(ChannelType::Category));
+    }
+
+    /// Voice channel is not a thread.
+    #[test]
+    fn voice_channel_is_not_thread() {
+        assert!(!is_thread_channel(ChannelType::Voice));
+    }
+
+    /// PublicThread is correctly detected as a thread.
+    #[test]
+    fn public_thread_is_thread() {
+        assert!(is_thread_channel(ChannelType::PublicThread));
+    }
+
+    /// PrivateThread is correctly detected as a thread.
+    #[test]
+    fn private_thread_is_thread() {
+        assert!(is_thread_channel(ChannelType::PrivateThread));
+    }
+
+    /// NewsThread is correctly detected as a thread.
+    #[test]
+    fn news_thread_is_thread() {
+        assert!(is_thread_channel(ChannelType::NewsThread));
+    }
 }