diff --git a/server/src/chat/transport/matrix/bot/messages.rs b/server/src/chat/transport/matrix/bot/messages.rs index fde9805d..cc788c87 100644 --- a/server/src/chat/transport/matrix/bot/messages.rs +++ b/server/src/chat/transport/matrix/bot/messages.rs @@ -1,4 +1,4 @@ -use crate::chat::util::drain_complete_paragraphs; +use crate::chat::util::{drain_complete_paragraphs, is_permission_approval}; use crate::http::context::PermissionDecision; use crate::llm::providers::claude_code::{ClaudeCodeProvider, ClaudeCodeResult}; use crate::slog; @@ -22,24 +22,6 @@ use super::history::{ConversationEntry, ConversationRole, save_history}; use super::mentions::{is_reply_to_bot, mentions_bot}; use super::verification::check_sender_verified; -/// Returns `true` if the message body is an affirmative permission response. -/// -/// Recognised affirmative tokens (case-insensitive): `yes`, `y`, `approve`, -/// `allow`, `ok`. Anything else — including ambiguous text — is treated as -/// denial (fail-closed). -pub(super) fn is_permission_approval(body: &str) -> bool { - // Strip a leading @mention (e.g. "@timmy yes") so the bot name doesn't - // interfere with the check. - let trimmed = body - .trim() - .trim_start_matches('@') - .split_whitespace() - .last() - .unwrap_or("") - .to_ascii_lowercase(); - matches!(trimmed.as_str(), "yes" | "y" | "approve" | "allow" | "ok") -} - /// Build the user-facing prompt for a single turn. In multi-user rooms the /// sender is included so the LLM can distinguish participants. pub(super) fn format_user_prompt(sender: &str, message: &str) -> String { @@ -704,45 +686,6 @@ mod tests { assert_eq!(prompt, "@bob:example.com: What's up?"); } - // -- is_permission_approval ----------------------------------------------- - - #[test] - fn is_permission_approval_accepts_yes_variants() { - assert!(is_permission_approval("yes")); - assert!(is_permission_approval("Yes")); - assert!(is_permission_approval("YES")); - assert!(is_permission_approval("y")); - assert!(is_permission_approval("Y")); - assert!(is_permission_approval("approve")); - assert!(is_permission_approval("allow")); - assert!(is_permission_approval("ok")); - assert!(is_permission_approval("OK")); - } - - #[test] - fn is_permission_approval_denies_no_and_other() { - assert!(!is_permission_approval("no")); - assert!(!is_permission_approval("No")); - assert!(!is_permission_approval("n")); - assert!(!is_permission_approval("deny")); - assert!(!is_permission_approval("reject")); - assert!(!is_permission_approval("maybe")); - assert!(!is_permission_approval("")); - assert!(!is_permission_approval("yes please do it")); - } - - #[test] - fn is_permission_approval_strips_at_mention_prefix() { - assert!(is_permission_approval("@timmy yes")); - assert!(!is_permission_approval("@timmy no")); - } - - #[test] - fn is_permission_approval_handles_whitespace() { - assert!(is_permission_approval(" yes ")); - assert!(is_permission_approval("\tyes\n")); - } - // -- bot_name / system prompt ------------------------------------------- #[test] diff --git a/server/src/chat/transport/slack/commands.rs b/server/src/chat/transport/slack/commands.rs index 1bae6ebb..33a0ef8f 100644 --- a/server/src/chat/transport/slack/commands.rs +++ b/server/src/chat/transport/slack/commands.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; use crate::agents::AgentPool; use crate::chat::transport::matrix::{ConversationEntry, ConversationRole, RoomConversation}; +use crate::chat::util::is_permission_approval; use crate::slog; use crate::chat::ChatTransport; use crate::http::context::{PermissionDecision, PermissionForward}; @@ -86,17 +87,6 @@ pub struct SlackWebhookContext { pub permission_timeout_secs: u64, } -// ── Permission approval detection ────────────────────────────────────── - -/// Returns `true` if the message body should be interpreted as permission approval. -fn is_permission_approval(body: &str) -> bool { - let trimmed = body.trim().to_ascii_lowercase(); - matches!( - trimmed.as_str(), - "yes" | "y" | "approve" | "allow" | "ok" - ) -} - // ── Incoming message dispatch ─────────────────────────────────────────── pub(super) async fn handle_incoming_message( diff --git a/server/src/chat/transport/whatsapp/commands.rs b/server/src/chat/transport/whatsapp/commands.rs index d2e419f0..e0246649 100644 --- a/server/src/chat/transport/whatsapp/commands.rs +++ b/server/src/chat/transport/whatsapp/commands.rs @@ -1,21 +1,13 @@ use std::sync::Arc; use crate::chat::transport::matrix::{ConversationEntry, ConversationRole, RoomConversation}; +use crate::chat::util::is_permission_approval; use crate::http::context::{PermissionDecision}; use crate::slog; use super::WhatsAppWebhookContext; use super::format::{chunk_for_whatsapp, markdown_to_whatsapp}; use super::history::save_whatsapp_history; -/// Returns `true` if the message body should be interpreted as permission approval. -fn is_permission_approval(body: &str) -> bool { - let trimmed = body.trim().to_ascii_lowercase(); - matches!( - trimmed.as_str(), - "yes" | "y" | "approve" | "allow" | "ok" - ) -} - /// Dispatch an incoming WhatsApp message to bot commands. pub(super) async fn handle_incoming_message(ctx: &WhatsAppWebhookContext, sender: &str, message: &str) { use crate::chat::commands::{CommandDispatch, try_handle_command}; diff --git a/server/src/chat/util.rs b/server/src/chat/util.rs index 2462ffd6..f0e944d5 100644 --- a/server/src/chat/util.rs +++ b/server/src/chat/util.rs @@ -3,6 +3,27 @@ //! These functions are transport-agnostic helpers for processing chat messages: //! prefix stripping, bot-mention handling, and paragraph buffering. +/// Returns `true` if the message body is an affirmative permission response. +/// +/// Recognised affirmative tokens (case-insensitive): `yes`, `y`, `approve`, +/// `allow`, `ok`. Anything else — including ambiguous text — is treated as +/// denial (fail-closed). +/// +/// A leading `@mention` (e.g. `"@timmy yes"`) is stripped before checking, so +/// the bot name does not interfere with the result. +pub fn is_permission_approval(body: &str) -> bool { + // Strip a leading @mention (e.g. "@timmy yes") so the bot name doesn't + // interfere with the check. + let trimmed = body + .trim() + .trim_start_matches('@') + .split_whitespace() + .last() + .unwrap_or("") + .to_ascii_lowercase(); + matches!(trimmed.as_str(), "yes" | "y" | "approve" | "allow" | "ok") +} + /// Case-insensitive prefix strip that also requires the match to end at a /// word boundary (whitespace, punctuation, or end-of-string). pub fn strip_prefix_ci<'a>(text: &'a str, prefix: &str) -> Option<&'a str> { @@ -190,6 +211,45 @@ pub fn normalize_line_breaks(text: &str) -> String { mod tests { use super::*; + // -- is_permission_approval --------------------------------------------- + + #[test] + fn is_permission_approval_accepts_yes_variants() { + assert!(is_permission_approval("yes")); + assert!(is_permission_approval("Yes")); + assert!(is_permission_approval("YES")); + assert!(is_permission_approval("y")); + assert!(is_permission_approval("Y")); + assert!(is_permission_approval("approve")); + assert!(is_permission_approval("allow")); + assert!(is_permission_approval("ok")); + assert!(is_permission_approval("OK")); + } + + #[test] + fn is_permission_approval_denies_no_and_other() { + assert!(!is_permission_approval("no")); + assert!(!is_permission_approval("No")); + assert!(!is_permission_approval("n")); + assert!(!is_permission_approval("deny")); + assert!(!is_permission_approval("reject")); + assert!(!is_permission_approval("maybe")); + assert!(!is_permission_approval("")); + assert!(!is_permission_approval("yes please do it")); + } + + #[test] + fn is_permission_approval_strips_at_mention_prefix() { + assert!(is_permission_approval("@timmy yes")); + assert!(!is_permission_approval("@timmy no")); + } + + #[test] + fn is_permission_approval_handles_whitespace() { + assert!(is_permission_approval(" yes ")); + assert!(is_permission_approval("\tyes\n")); + } + // -- strip_prefix_ci ---------------------------------------------------- #[test]