From 8aa30a7bd101fb559bb2ef7f0f66656ec64a7e1a Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Wed, 11 Mar 2026 19:41:12 +0100 Subject: [PATCH 01/19] feat: add Mattermost messaging adapter Implements a full Mattermost adapter using a custom HTTP + WebSocket client (reqwest + tokio-tungstenite), following the existing adapter architecture. Features: - WebSocket-based inbound messages with exponential backoff reconnection - Text responses, streaming edits (ZWS placeholder + throttled PUT), file uploads, reactions, typing indicators, thread replies - History fetch via /channels/{id}/posts - Named adapter instances with per-instance permissions - Hot-reload of permissions via spawn_file_watcher - Conversation IDs: mattermost:{team_id}:{channel_id} and mattermost:{team_id}:dm:{user_id} - Fail-closed permission filtering: messages without a team_id are rejected when team/channel filters are configured - URL validation at config load time Config: [messaging.mattermost] enabled = true base_url = "https://mattermost.example.com" token = "env:MATTERMOST_TOKEN" team_id = "optional-default-team-id" --- Cargo.lock | 1 + Cargo.toml | 1 + docs/mattermost.md | 1747 +++++++++++++++++++++++++++++++++++ src/config.rs | 387 +++++++- src/main.rs | 70 ++ src/messaging.rs | 3 +- src/messaging/mattermost.rs | 1031 +++++++++++++++++++++ src/messaging/target.rs | 30 + 8 files changed, 3266 insertions(+), 4 deletions(-) create mode 100644 docs/mattermost.md create mode 100644 src/messaging/mattermost.rs diff --git a/Cargo.lock b/Cargo.lock index 9be359324..f64492111 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8531,6 +8531,7 @@ dependencies = [ "tracing-opentelemetry", "tracing-subscriber", "twitch-irc", + "url", "urlencoding", "uuid", "zip 2.4.2", diff --git a/Cargo.toml b/Cargo.toml index 62709453b..997293259 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -153,6 +153,7 @@ prometheus = { version = "0.13", optional = true } pdf-extract = "0.10.0" open = "5.3.3" urlencoding = "2.1.3" +url = "2" moka = "0.12.13" [features] diff --git a/docs/mattermost.md b/docs/mattermost.md new file mode 100644 index 000000000..64f334c38 --- /dev/null +++ b/docs/mattermost.md @@ -0,0 +1,1747 @@ +# Mattermost Integration Plan + +## Executive Summary + +Add Mattermost messaging platform support to Spacebot following the existing adapter architecture. Implementation will use a custom HTTP + WebSocket client (not the `mattermost_api` crate) to maintain consistency with existing patterns and avoid limitations. + +## Library Assessment: mattermost_api + +The [mattermost_api](https://docs.rs/mattermost_api/latest/mattermost_api/) crate (v0.8.2) was evaluated and **rejected** for the following reasons: + +### Critical Gaps + +| Feature | Required | Crate Support | +|---------|----------|---------------| +| Edit post (streaming) | Yes | No | +| Channel history fetch | Yes | No | +| Typing indicator | Yes | No | +| Websocket disconnect | Yes | No (loops forever) | +| File upload | Medium | No | +| Reactions | Low | No | + +### Style Conflicts + +- Uses `#[async_trait]` macro - violates AGENTS.md guideline to use native RPITIT +- `WebsocketHandler` trait requires async_trait +- No control over websocket lifecycle + +### Verdict + +Build a minimal custom client using `reqwest` (HTTP) + `tokio-tungstenite` (WebSocket). This matches the pattern used by other adapters and gives us full control over the implementation. + +--- + +## Adversarial Review + +### Security Issues + +| Issue | Severity | Mitigation | +|-------|----------|------------| +| Token stored in plain String | High | Use `DecryptedSecret` wrapper (see src/secrets/store.rs) | +| URL injection in api_url() | Medium | Validate base_url at config load time, use `url::Url` | +| Unvalidated post_id in edit_post | Medium | Validate alphanumeric format before interpolation | +| WebSocket URL derived from user input | Medium | Parse and validate URL, require https/wss in production | +| No HTTP response body limit | Medium | Set max response size on reqwest client | +| File upload size unbounded | Medium | Enforce max_attachment_bytes from config | + +### Concurrency Bugs + +| Issue | Severity | Mitigation | +|-------|----------|------------| +| Race between start() and respond() | High | Use `OnceCell` for bot_user_id, fail requests until initialized | +| StreamChunk lost if channel_id not in active_messages | Medium | Return error, let caller handle | +| WebSocket spawn detached, no join handle | High | Store `JoinHandle` in adapter, await in shutdown | +| typing_tasks abort without cleanup | Low | Acceptable - abort is correct behavior | +| split read/write without mutex | Medium | Use `futures::stream::SplitSink`/`SplitStream` pattern correctly | + +### Maintainability Issues + +| Issue | Severity | Mitigation | +|-------|----------|------------| +| Missing Debug impl on MattermostConfig | Medium | Add `#[derive(Debug)]` with token redaction | +| Missing Debug impl on MattermostAdapter | Medium | Implement Debug manually with redaction | +| Inline JSON builders | Low | Extract to typed request structs | +| "Full implementation omitted" in plan | High | Complete the InboundMessage building code | +| Magic numbers (500ms throttle) | Low | Extract to constants with comments | +| No reconnection logic | High | Implement exponential backoff reconnect | +| No rate limit handling | Medium | Parse 429 responses, implement backoff | + +### Idiomatic Rust Issues + +| Issue | Severity | Mitigation | +|-------|----------|------------| +| `impl Into` for new() params | Low | Use `impl Into>` or just `String` per style guide | +| Clone on large structs | Medium | Use `Arc` for strings, avoid cloning | +| `Vec` allocations in split_message | Low | Return `Cow<'_, str>` iterator or accept writer | +| Missing `#[non_exhaustive]` on config structs | Low | Add for future-proofing | +| Using `anyhow::Context` directly | Medium | Use crate's `Error` type for consistency | + +--- + +## Architecture + +### Message Flow + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Mattermost Instance │ +│ https://mattermost.example.com/api/v4/* │ +└─────────────────┬───────────────────────────┬───────────────┘ + │ │ + │ HTTP (REST API) │ WebSocket + │ │ + ▼ ▼ + ┌────────────────┐ ┌────────────────┐ + │ reqwest │ │ tokio- │ + │ client │ │ tungstenite │ + └───────┬────────┘ └───────┬────────┘ + │ │ + └───────────┬───────────────┘ + │ + ▼ + ┌────────────────────┐ + │ MattermostAdapter │ + │ (Messaging trait) │ + └─────────┬──────────┘ + │ + ▼ + ┌────────────────────┐ + │ MessagingManager │ + │ (fan-in) │ + └────────────────────┘ +``` + +### Conversation ID Format + +``` +mattermost:{team_id}:{channel_id} # Public/private channel +mattermost:{team_id}:dm:{user_id} # Direct message +mattermost:instance_name:{team_id}:{id} # Named adapter instance +``` + +--- + +## Implementation + +### 1. Configuration (src/config.rs) + +#### Add to MessagingConfig + +```rust +pub struct MessagingConfig { + // ... existing fields + pub mattermost: Option, +} +``` + +#### New Config Structs + +```rust +#[derive(Clone)] +pub struct MattermostConfig { + pub enabled: bool, + pub base_url: String, + pub token: String, + pub team_id: Option, + pub instances: Vec, + pub dm_allowed_users: Vec, + pub max_attachment_bytes: usize, +} + +impl std::fmt::Debug for MattermostConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MattermostConfig") + .field("enabled", &self.enabled) + .field("base_url", &self.base_url) + .field("token", &"[REDACTED]") + .field("team_id", &self.team_id) + .field("instances", &self.instances) + .field("dm_allowed_users", &self.dm_allowed_users) + .field("max_attachment_bytes", &self.max_attachment_bytes) + .finish() + } +} + +#[derive(Clone)] +pub struct MattermostInstanceConfig { + pub name: String, + pub enabled: bool, + pub base_url: String, + pub token: String, + pub team_id: Option, + pub dm_allowed_users: Vec, + pub max_attachment_bytes: usize, +} + +impl std::fmt::Debug for MattermostInstanceConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MattermostInstanceConfig") + .field("name", &self.name) + .field("enabled", &self.enabled) + .field("base_url", &self.base_url) + .field("token", &"[REDACTED]") + .field("team_id", &self.team_id) + .field("dm_allowed_users", &self.dm_allowed_users) + .field("max_attachment_bytes", &self.max_attachment_bytes) + .finish() + } +} + +#[derive(Debug, Clone, Default)] +pub struct MattermostPermissions { + pub team_filter: Option>, + pub channel_filter: HashMap>, + pub dm_allowed_users: Vec, +} +``` + +#### TOML Deserialization Types + +```rust +#[derive(Deserialize)] +struct TomlMattermostConfig { + #[serde(default)] + enabled: bool, + base_url: Option, + token: Option, + team_id: Option, + #[serde(default)] + instances: Vec, + #[serde(default)] + dm_allowed_users: Vec, + #[serde(default = "default_max_attachment_bytes")] + max_attachment_bytes: usize, +} + +fn default_max_attachment_bytes() -> usize { + 10 * 1024 * 1024 // 10 MB +} + +#[derive(Deserialize)] +struct TomlMattermostInstanceConfig { + name: String, + #[serde(default)] + enabled: bool, + base_url: Option, + token: Option, + team_id: Option, + #[serde(default)] + dm_allowed_users: Vec, + #[serde(default = "default_max_attachment_bytes")] + max_attachment_bytes: usize, +} +``` + +#### URL Validation at Config Load + +```rust +fn validate_mattermost_url(url: &str) -> Result<()> { + let parsed = url::Url::parse(url) + .map_err(|e| ConfigError::Invalid(format!("invalid mattermost base_url: {e}")))?; + + match parsed.scheme() { + "https" => Ok(()), + "http" => { + tracing::warn!("mattermost base_url uses http instead of https"); + Ok(()) + } + scheme => Err(ConfigError::Invalid(format!( + "mattermost base_url must be http or https, got: {scheme}" + )).into()), + } +} +``` + +#### Update is_named_adapter_platform + +```rust +fn is_named_adapter_platform(platform: &str) -> bool { + matches!( + platform, + "discord" | "slack" | "telegram" | "twitch" | "email" | "mattermost" + ) +} +``` + +#### Add to build_adapter_validation_states + +```rust +if let Some(mattermost) = &messaging.mattermost { + let named_instances = validate_instance_names( + "mattermost", + mattermost.instances.iter().map(|i| i.name.as_str()), + )?; + let default_present = !mattermost.base_url.trim().is_empty() + && !mattermost.token.trim().is_empty(); + validate_runtime_keys("mattermost", default_present, &named_instances)?; + + if default_present { + validate_mattermost_url(&mattermost.base_url)?; + } + for instance in &mattermost.instances { + if instance.enabled && !instance.base_url.is_empty() { + validate_mattermost_url(&instance.base_url)?; + } + } + + states.insert( + "mattermost", + AdapterValidationState { + default_present, + named_instances, + }, + ); +} +``` + +### 2. Config TOML Schema + +```toml +[messaging.mattermost] +enabled = true +base_url = "https://mattermost.example.com" +token = "your-bot-personal-access-token" +team_id = "team-id-optional-default" +dm_allowed_users = ["user-id-1", "user-id-2"] +max_attachment_bytes = 10485760 # 10 MB default + +[[messaging.mattermost.instances]] +name = "internal" +enabled = true +base_url = "https://internal.company.com" +token = "env:MATTERMOST_INTERNAL_TOKEN" +team_id = "internal-team-id" +max_attachment_bytes = 5242880 # 5 MB for this instance + +[[bindings]] +agent_id = "default" +channel = "mattermost" +team_id = "team-id-here" +channel_ids = ["channel-id-1", "channel-id-2"] +dm_allowed_users = ["user-id-1"] +``` + +### 3. Adapter Implementation (src/messaging/mattermost.rs) + +```rust +//! Mattermost messaging adapter using custom HTTP + WebSocket client. + +use crate::config::MattermostPermissions; +use crate::messaging::apply_runtime_adapter_to_conversation_id; +use crate::messaging::traits::{HistoryMessage, InboundStream, Messaging}; +use crate::{InboundMessage, MessageContent, OutboundResponse, StatusUpdate}; +use crate::error::{AdapterError, Result}; + +use anyhow::Context as _; +use arc_swap::ArcSwap; +use futures_util::{SinkExt, StreamExt}; +use reqwest::Client; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::sync::Arc; +use std::time::{Duration, Instant}; +use tokio::sync::{mpsc, OnceCell, RwLock}; +use tokio_tungstenite::{ + connect_async, + tungstenite::{Error as WsError, Message as WsMessage}, + MaybeTlsStream, WebSocketStream, +}; + +const MAX_MESSAGE_LENGTH: usize = 16_383; +const STREAM_EDIT_THROTTLE: Duration = Duration::from_millis(500); +const TYPING_INDICATOR_INTERVAL: Duration = Duration::from_secs(5); +const WS_RECONNECT_BASE_DELAY: Duration = Duration::from_secs(1); +const WS_RECONNECT_MAX_DELAY: Duration = Duration::from_secs(60); +const HTTP_TIMEOUT: Duration = Duration::from_secs(30); + +pub struct MattermostAdapter { + runtime_key: Arc, + base_url: url::Url, + token: Arc, + default_team_id: Option>, + max_attachment_bytes: usize, + client: Client, + permissions: Arc>, + bot_user_id: OnceCell>, + bot_username: OnceCell>, + active_messages: Arc>>, + typing_tasks: Arc>>>, + shutdown_tx: Arc>>>, + ws_task: Arc>>>, +} + +struct ActiveStream { + post_id: Arc, + channel_id: Arc, + last_edit: Instant, + accumulated_text: String, +} + +impl std::fmt::Debug for MattermostAdapter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MattermostAdapter") + .field("runtime_key", &self.runtime_key) + .field("base_url", &self.base_url) + .field("token", &"[REDACTED]") + .field("default_team_id", &self.default_team_id) + .field("max_attachment_bytes", &self.max_attachment_bytes) + .finish() + } +} + +impl MattermostAdapter { + pub fn new( + runtime_key: impl Into>, + base_url: &str, + token: impl Into>, + default_team_id: Option>, + max_attachment_bytes: usize, + permissions: Arc>, + ) -> Result { + let base_url = url::Url::parse(base_url) + .map_err(|e| AdapterError::ConfigInvalid(format!("invalid base_url: {e}")))?; + + let client = Client::builder() + .timeout(HTTP_TIMEOUT) + .pool_idle_timeout(Duration::from_secs(30)) + .build() + .context("failed to build HTTP client")?; + + Ok(Self { + runtime_key: runtime_key.into(), + base_url, + token: token.into(), + default_team_id, + max_attachment_bytes, + client, + permissions, + bot_user_id: OnceCell::new(), + bot_username: OnceCell::new(), + active_messages: Arc::new(RwLock::new(HashMap::new())), + typing_tasks: Arc::new(RwLock::new(HashMap::new())), + shutdown_tx: Arc::new(RwLock::new(None)), + ws_task: Arc::new(RwLock::new(None)), + }) + } + + fn api_url(&self, path: &str) -> url::Url { + let mut url = self.base_url.clone(); + url.path_segments_mut() + .expect("base_url is valid") + .extend(&["api", "v4"]) + .extend(path.trim_start_matches('/').split('/')); + url + } + + fn ws_url(&self) -> url::Url { + let mut url = self.base_url.clone(); + url.set_scheme(match self.base_url.scheme() { + "https" => "wss", + "http" => "ws", + other => other, + }).expect("scheme is valid"); + url.path_segments_mut() + .expect("base_url is valid") + .extend(&["api", "v4", "websocket"]); + url + } + + fn extract_channel_id<'a>(&self, message: &'a InboundMessage) -> Result<&'a str> { + message + .metadata + .get("mattermost_channel_id") + .and_then(|v| v.as_str()) + .ok_or_else(|| AdapterError::MissingMetadata("mattermost_channel_id".into()).into()) + } + + fn extract_post_id<'a>(&self, message: &'a InboundMessage) -> Result<&'a str> { + message + .metadata + .get("mattermost_post_id") + .and_then(|v| v.as_str()) + .ok_or_else(|| AdapterError::MissingMetadata("mattermost_post_id".into()).into()) + } + + fn validate_id(id: &str) -> Result<()> { + if id.is_empty() || id.len() > 64 { + return Err(AdapterError::InvalidId(id.into()).into()); + } + if !id.chars().all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') { + return Err(AdapterError::InvalidId(id.into()).into()); + } + Ok(()) + } + + async fn stop_typing(&self, channel_id: &str) { + if let Some(handle) = self.typing_tasks.write().await.remove(channel_id) { + handle.abort(); + } + } + + async fn start_typing(&self, channel_id: &str) { + // Typing indicators expire after ~5 seconds. Spawn a task that re-sends on + // TYPING_INDICATOR_INTERVAL so the indicator remains visible during long operations. + // The task is stored in typing_tasks so stop_typing can abort it. + let Some(user_id) = self.bot_user_id.get().cloned() else { return }; + let channel_id = channel_id.to_string(); + let client = self.client.clone(); + let token = self.token.clone(); + let url = self.api_url(&format!("/users/{user_id}/typing")); + + let handle = tokio::spawn(async move { + let mut interval = tokio::time::interval(TYPING_INDICATOR_INTERVAL); + loop { + interval.tick().await; + let result = client + .post(url.clone()) + .bearer_auth(token.as_ref()) + .json(&serde_json::json!({ + "channel_id": channel_id, + "parent_id": "", + })) + .send() + .await; + if let Err(error) = result { + tracing::warn!(%error, "typing indicator request failed"); + } + } + }); + + self.typing_tasks + .write() + .await + .insert(channel_id.to_string(), handle); + } + + async fn create_post( + &self, + channel_id: &str, + message: &str, + root_id: Option<&str>, + ) -> Result { + Self::validate_id(channel_id)?; + if let Some(rid) = root_id { + Self::validate_id(rid)?; + } + + let response = self.client + .post(self.api_url("/posts")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ + "channel_id": channel_id, + "message": message, + "root_id": root_id.unwrap_or(""), + })) + .send() + .await + .context("failed to create post")?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(AdapterError::ApiError { + endpoint: "/posts".into(), + status: status.as_u16(), + message: body, + }.into()); + } + + response + .json() + .await + .context("failed to parse post response") + .map_err(Into::into) + } + + async fn edit_post(&self, post_id: &str, message: &str) -> Result<()> { + Self::validate_id(post_id)?; + + let response = self.client + .put(self.api_url(&format!("/posts/{post_id}"))) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ "message": message })) + .send() + .await + .context("failed to edit post")?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(AdapterError::ApiError { + endpoint: format!("/posts/{post_id}"), + status: status.as_u16(), + message: body, + }.into()); + } + + Ok(()) + } + + async fn get_channel_posts( + &self, + channel_id: &str, + before_post_id: Option<&str>, + limit: u32, + ) -> Result { + Self::validate_id(channel_id)?; + + let mut url = self.api_url(&format!("/channels/{channel_id}/posts")); + { + let mut query = url.query_pairs_mut(); + query.append_pair("page", "0"); + query.append_pair("per_page", &limit.to_string()); + if let Some(before) = before_post_id { + query.append_pair("before", before); + } + } + + let response = self.client + .get(url) + .bearer_auth(self.token.as_ref()) + .send() + .await + .context("failed to fetch channel posts")?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(AdapterError::ApiError { + endpoint: format!("/channels/{channel_id}/posts"), + status: status.as_u16(), + message: body, + }.into()); + } + + response + .json() + .await + .context("failed to parse posts response") + .map_err(Into::into) + } + + fn build_inbound_message( + &self, + post: MattermostPost, + team_id: Option, + ) -> Option { + let bot_id = self.bot_user_id.get()?; + if &post.user_id == bot_id.as_ref() { + return None; + } + + let permissions = self.permissions.load(); + + if let Some(team_filter) = &permissions.team_filter { + if let Some(ref tid) = team_id { + if !team_filter.contains(tid) { + return None; + } + } + } + + if let Some(channel_filter) = permissions.channel_filter.get(&post.channel_id) { + if !channel_filter.contains(&post.channel_id) { + return None; + } + } + + // DM channel type is carried in the WebSocket event's `channel_type` field. + // All Mattermost channel IDs start with lowercase alphanumeric, so channel_id prefix + // is NOT a reliable DM indicator. Use the channel_type from the event data instead, + // passed in as an optional parameter. "D" = direct message, "G" = group message. + let conversation_id = if post.channel_type.as_deref() == Some("D") { + apply_runtime_adapter_to_conversation_id( + self.runtime_key.as_ref(), + format!("mattermost:{}:dm:{}", team_id.as_deref().unwrap_or(""), post.user_id), + ) + } else { + apply_runtime_adapter_to_conversation_id( + self.runtime_key.as_ref(), + format!("mattermost:{}:{}", team_id.as_deref().unwrap_or(""), post.channel_id), + ) + }; + + let mut metadata = HashMap::new(); + metadata.insert("mattermost_post_id".into(), serde_json::json!(post.id)); + metadata.insert("mattermost_channel_id".into(), serde_json::json!(post.channel_id)); + if let Some(tid) = team_id { + metadata.insert("mattermost_team_id".into(), serde_json::json!(tid)); + } + if !post.root_id.is_empty() { + metadata.insert("mattermost_root_id".into(), serde_json::json!(post.root_id)); + } + metadata.insert( + "mattermost_create_at".into(), + serde_json::json!(post.create_at), + ); + + Some(InboundMessage { + id: post.id.clone(), + source: "mattermost".into(), + adapter: Some(self.runtime_key.to_string()), + conversation_id, + sender_id: post.user_id.clone(), + agent_id: None, + content: MessageContent::Text(post.message), + timestamp: chrono::DateTime::from_timestamp_millis(post.create_at) + .unwrap_or_else(chrono::Utc::now), + metadata, + formatted_author: None, + }) + } +} + +impl Messaging for MattermostAdapter { + fn name(&self) -> &str { + &self.runtime_key + } + + async fn start(&self) -> Result { + let (inbound_tx, inbound_rx) = mpsc::channel(256); + let (shutdown_tx, mut shutdown_rx) = mpsc::channel::<()>(1); + *self.shutdown_tx.write().await = Some(shutdown_tx); + + let me: MattermostUser = self.client + .get(self.api_url("/users/me")) + .bearer_auth(self.token.as_ref()) + .send() + .await + .context("failed to get bot user")? + .json() + .await + .context("failed to parse user response")?; + + let user_id: Arc = me.id.clone().into(); + let username: Arc = me.username.clone().into(); + + self.bot_user_id.set(user_id.clone()).expect("set once"); + self.bot_username.set(username.clone()).expect("set once"); + + tracing::info!( + adapter = %self.runtime_key, + bot_id = %user_id, + bot_username = %username, + "mattermost connected" + ); + + let ws_url = self.ws_url(); + let runtime_key = self.runtime_key.clone(); + let token = self.token.clone(); + let permissions = self.permissions.clone(); + let bot_user_id = user_id; + let inbound_tx_clone = inbound_tx.clone(); + + let handle = tokio::spawn(async move { + let mut retry_delay = WS_RECONNECT_BASE_DELAY; + + loop { + let connect_result = connect_async(ws_url.as_str()).await; + + match connect_result { + Ok((ws_stream, _)) => { + retry_delay = WS_RECONNECT_BASE_DELAY; + + let (mut write, mut read) = ws_stream.split(); + + let auth_msg = serde_json::json!({ + "seq": 1, + "action": "authentication_challenge", + "data": {"token": token.as_ref()} + }); + + if let Ok(msg) = serde_json::to_string(&auth_msg) { + if write.send(WsMessage::Text(msg)).await.is_err() { + tracing::error!("failed to send websocket auth"); + continue; + } + } + + loop { + tokio::select! { + _ = shutdown_rx.recv() => { + tracing::info!(adapter = %runtime_key, "mattermost websocket shutting down"); + let _ = write.send(WsMessage::Close(None)).await; + return; + } + + msg = read.next() => { + match msg { + Some(Ok(WsMessage::Text(text))) => { + if let Ok(event) = serde_json::from_str::(&text) { + if event.event == "posted" { + if let Some(post_data) = event.data.get("post") { + if let Ok(post) = serde_json::from_value::(post_data.clone()) { + if post.user_id.as_str() != bot_user_id.as_ref() { + let team_id = event + .broadcast + .team_id + .clone(); + + // Load permissions each message so hot-reload takes effect. + let perms = permissions.load(); + if let Some(msg) = build_message_from_post( + &post, + &runtime_key, + &bot_user_id, + &team_id, + &perms, + ) { + if inbound_tx_clone.send(msg).await.is_err() { + tracing::debug!("inbound channel closed"); + return; + } + } + } + } + } + } + } + } + Some(Ok(WsMessage::Ping(data))) => { + if write.send(WsMessage::Pong(data)).await.is_err() { + tracing::warn!("failed to send pong"); + break; + } + } + Some(Ok(WsMessage::Pong(_))) => {} + Some(Ok(WsMessage::Close(_))) => { + tracing::info!(adapter = %runtime_key, "websocket closed by server"); + break; + } + Some(Err(e)) => { + tracing::error!(adapter = %runtime_key, error = %e, "websocket error"); + break; + } + None => break, + _ => {} + } + } + } + } + + tracing::info!(adapter = %runtime_key, "websocket disconnected, reconnecting..."); + } + Err(e) => { + tracing::error!( + adapter = %runtime_key, + error = %e, + delay_ms = retry_delay.as_millis(), + "websocket connection failed, retrying" + ); + } + } + + tokio::select! { + _ = tokio::time::sleep(retry_delay) => { + retry_delay = (retry_delay * 2).min(WS_RECONNECT_MAX_DELAY); + } + _ = shutdown_rx.recv() => { + tracing::info!(adapter = %runtime_key, "mattermost adapter shutting down during reconnect delay"); + return; + } + } + } + }); + + *self.ws_task.write().await = Some(handle); + + let stream = tokio_stream::wrappers::ReceiverStream::new(inbound_rx); + Ok(Box::pin(stream)) + } + + async fn respond( + &self, + message: &InboundMessage, + response: OutboundResponse, + ) -> Result<()> { + let channel_id = self.extract_channel_id(message)?; + + match response { + OutboundResponse::Text(text) => { + self.stop_typing(channel_id).await; + // mattermost_root_id: set when the triggering message was itself inside a thread. + // REPLY_TO_MESSAGE_ID: set by channel.rs on retrigger messages when a branch or + // worker completes — this is the post ID the agent should thread its reply under. + let root_id = message + .metadata + .get("mattermost_root_id") + .and_then(|v| v.as_str()) + .or_else(|| { + message + .metadata + .get(crate::metadata_keys::REPLY_TO_MESSAGE_ID) + .and_then(|v| v.as_str()) + }); + + for chunk in split_message(&text, MAX_MESSAGE_LENGTH) { + self.create_post(channel_id, &chunk, root_id).await?; + } + } + + OutboundResponse::StreamStart => { + // Key by message.id, not channel_id. Keying by channel_id would cause + // concurrent conversations in the same channel to overwrite each other. + let root_id = message.metadata.get("mattermost_root_id").and_then(|v| v.as_str()); + self.start_typing(channel_id).await; + let post = self.create_post(channel_id, "\u{200B}", root_id).await?; + self.active_messages.write().await.insert( + message.id.clone(), + ActiveStream { + post_id: post.id.into(), + channel_id: channel_id.to_string().into(), + last_edit: Instant::now(), + accumulated_text: String::new(), + }, + ); + } + + OutboundResponse::StreamChunk(chunk) => { + let mut active_messages = self.active_messages.write().await; + if let Some(active) = active_messages.get_mut(&message.id) { + active.accumulated_text.push_str(&chunk); + + if active.last_edit.elapsed() > STREAM_EDIT_THROTTLE { + let display_text = if active.accumulated_text.len() > MAX_MESSAGE_LENGTH { + let end = active.accumulated_text.floor_char_boundary(MAX_MESSAGE_LENGTH - 3); + format!("{}...", &active.accumulated_text[..end]) + } else { + active.accumulated_text.clone() + }; + + if let Err(error) = self.edit_post(&active.post_id, &display_text).await { + tracing::warn!(%error, "failed to edit streaming message"); + } + active.last_edit = Instant::now(); + } + } + } + + OutboundResponse::StreamEnd => { + self.stop_typing(channel_id).await; + // Always flush the final text, even if the last chunk was within the throttle window. + if let Some(active) = self.active_messages.write().await.remove(&message.id) { + if let Err(error) = self.edit_post(&active.post_id, &active.accumulated_text).await { + tracing::warn!(%error, "failed to finalize streaming message"); + } + } + } + + OutboundResponse::Status(status) => self.send_status(message, status).await?, + + OutboundResponse::Reaction(emoji) => { + let post_id = self.extract_post_id(message)?; + let emoji_name = emoji.trim_matches(':'); + + let response = self.client + .post(self.api_url("/reactions")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ + "post_id": post_id, + "emoji_name": emoji_name, + })) + .send() + .await + .context("failed to add reaction")?; + + if !response.status().is_success() { + tracing::warn!( + status = %response.status(), + emoji = %emoji_name, + "failed to add reaction" + ); + } + } + + OutboundResponse::File { filename, data, mime_type, caption } => { + if data.len() > self.max_attachment_bytes { + return Err(AdapterError::FileTooLarge { + size: data.len(), + max: self.max_attachment_bytes, + }.into()); + } + + let part = reqwest::multipart::Part::bytes(data) + .file_name(filename.clone()) + .mime_str(&mime_type) + .context("invalid mime type")?; + + let form = reqwest::multipart::Form::new() + .part("files", part) + .text("channel_id", channel_id.to_string()); + + let response = self.client + .post(self.api_url("/files")) + .bearer_auth(self.token.as_ref()) + .multipart(form) + .send() + .await + .context("failed to upload file")?; + + if !response.status().is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(AdapterError::ApiError { + endpoint: "/files".into(), + status: response.status().as_u16(), + message: body, + }.into()); + } + + let upload: MattermostFileUpload = response + .json() + .await + .context("failed to parse file upload response")?; + + let file_ids: Vec<_> = upload.file_infos.iter().map(|f| f.id.as_str()).collect(); + self.client + .post(self.api_url("/posts")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ + "channel_id": channel_id, + "message": caption.unwrap_or_default(), + "file_ids": file_ids, + })) + .send() + .await + .context("failed to create post with file")?; + } + + _ => { + tracing::debug!(?response, "mattermost adapter does not support this response type"); + } + } + + Ok(()) + } + + async fn send_status( + &self, + message: &InboundMessage, + status: StatusUpdate, + ) -> Result<()> { + let channel_id = self.extract_channel_id(message)?; + + match status { + StatusUpdate::Thinking => { + self.start_typing(channel_id).await; + } + StatusUpdate::StopTyping => { + self.stop_typing(channel_id).await; + } + _ => {} + } + + Ok(()) + } + + async fn fetch_history( + &self, + message: &InboundMessage, + limit: usize, + ) -> Result> { + let channel_id = self.extract_channel_id(message)?; + let before_post_id = message.metadata.get("mattermost_post_id").and_then(|v| v.as_str()); + + let capped_limit = limit.min(200) as u32; + let posts = self.get_channel_posts(channel_id, before_post_id, capped_limit).await?; + + let bot_id = self.bot_user_id.get().map(|s| s.as_ref().to_string()); + + // Sort by create_at before mapping so chronological order is preserved. + // Mapping to HistoryMessage discards the timestamp field. + let mut posts_vec: Vec<_> = posts + .posts + .into_values() + .filter(|p| bot_id.as_deref() != Some(p.user_id.as_str())) + .collect(); + posts_vec.sort_by_key(|p| p.create_at); + + let history: Vec = posts_vec + .into_iter() + .map(|p| HistoryMessage { + author: p.user_id, + content: p.message, + is_bot: false, + }) + .collect(); + + Ok(history) + } + + async fn health_check(&self) -> Result<()> { + let response = self.client + .get(self.api_url("/system/ping")) + .bearer_auth(self.token.as_ref()) + .send() + .await + .context("health check request failed")?; + + if !response.status().is_success() { + return Err(AdapterError::HealthCheckFailed( + format!("status: {}", response.status()) + ).into()); + } + + Ok(()) + } + + async fn shutdown(&self) -> Result<()> { + if let Some(tx) = self.shutdown_tx.write().await.take() { + let _ = tx.send(()).await; + } + + if let Some(handle) = self.ws_task.write().await.take() { + handle.abort(); + } + + self.typing_tasks.write().await.clear(); + self.active_messages.write().await.clear(); + + tracing::info!(adapter = %self.runtime_key, "mattermost adapter shut down"); + Ok(()) + } + + async fn broadcast(&self, target: &str, response: OutboundResponse) -> crate::Result<()> { + // target is a channel_id extracted by resolve_broadcast_target(). + match response { + OutboundResponse::Text(text) => { + for chunk in split_message(&text, MAX_MESSAGE_LENGTH) { + self.create_post(target, &chunk, None).await?; + } + } + OutboundResponse::File { filename, data, mime_type, caption } => { + if data.len() > self.max_attachment_bytes { + return Err(AdapterError::FileTooLarge { + size: data.len(), + max: self.max_attachment_bytes, + }.into()); + } + let part = reqwest::multipart::Part::bytes(data) + .file_name(filename) + .mime_str(&mime_type) + .context("invalid mime type")?; + let form = reqwest::multipart::Form::new() + .part("files", part) + .text("channel_id", target.to_string()); + let upload: MattermostFileUpload = self.client + .post(self.api_url("/files")) + .bearer_auth(self.token.as_ref()) + .multipart(form) + .send() + .await + .context("failed to upload file")? + .json() + .await + .context("failed to parse file upload response")?; + let file_ids: Vec<_> = upload.file_infos.iter().map(|f| f.id.as_str()).collect(); + self.client + .post(self.api_url("/posts")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ + "channel_id": target, + "message": caption.unwrap_or_default(), + "file_ids": file_ids, + })) + .send() + .await + .context("failed to create post with file")?; + } + other => { + tracing::debug!(?other, "mattermost broadcast does not support this response type"); + } + } + Ok(()) + } +} + +fn build_message_from_post( + post: &MattermostPost, + runtime_key: &str, + bot_user_id: &str, + team_id: &Option, + permissions: &MattermostPermissions, +) -> Option { + if post.user_id == bot_user_id { + return None; + } + + if let Some(team_filter) = &permissions.team_filter { + if let Some(ref tid) = team_id { + if !team_filter.contains(tid) { + return None; + } + } + } + + let conversation_id = apply_runtime_adapter_to_conversation_id( + runtime_key, + format!("mattermost:{}:{}", team_id.as_deref().unwrap_or(""), post.channel_id), + ); + + let mut metadata = HashMap::new(); + + // Standard keys — required by channel.rs and channel_history.rs. + // metadata_keys::MESSAGE_ID is read by extract_message_id(), which feeds + // branch/worker reply threading via REPLY_TO_MESSAGE_ID in retrigger metadata. + metadata.insert( + crate::metadata_keys::MESSAGE_ID.into(), + serde_json::json!(&post.id), + ); + + // Platform-specific keys used within the adapter (respond, fetch_history, etc.). + metadata.insert("mattermost_post_id".into(), serde_json::json!(&post.id)); + metadata.insert("mattermost_channel_id".into(), serde_json::json!(&post.channel_id)); + if let Some(tid) = team_id { + metadata.insert("mattermost_team_id".into(), serde_json::json!(tid)); + } + if !post.root_id.is_empty() { + metadata.insert("mattermost_root_id".into(), serde_json::json!(&post.root_id)); + } + + Some(InboundMessage { + id: post.id.clone(), + source: "mattermost".into(), + adapter: Some(runtime_key.to_string()), + conversation_id, + sender_id: post.user_id.clone(), + agent_id: None, + content: MessageContent::Text(post.message.clone()), + timestamp: chrono::DateTime::from_timestamp_millis(post.create_at) + .unwrap_or_else(chrono::Utc::now), + metadata, + formatted_author: None, + }) +} + +// --- API Types --- + +#[derive(Debug, Clone, Deserialize)] +struct MattermostUser { + id: String, + username: String, +} + +#[derive(Debug, Clone, Deserialize, Serialize)] +struct MattermostPost { + id: String, + create_at: i64, + update_at: i64, + user_id: String, + channel_id: String, + root_id: String, + message: String, + // "D" = direct message, "G" = group DM, "O" = public, "P" = private. + // Present in WebSocket events; absent in REST list responses. + #[serde(default)] + channel_type: Option, + #[serde(default)] + file_ids: Vec, +} + +#[derive(Debug, Deserialize)] +struct MattermostPostList { + #[serde(default)] + order: Vec, + #[serde(default)] + posts: HashMap, +} + +#[derive(Debug, Deserialize)] +struct MattermostFileUpload { + #[serde(default)] + file_infos: Vec, +} + +#[derive(Debug, Deserialize)] +struct MattermostFileInfo { + id: String, + #[allow(dead_code)] + name: String, +} + +#[derive(Debug, Deserialize)] +struct MattermostWsEvent { + event: String, + #[serde(default)] + data: serde_json::Value, + #[serde(default)] + broadcast: MattermostWsBroadcast, +} + +#[derive(Debug, Deserialize, Default)] +struct MattermostWsBroadcast { + #[serde(default)] + channel_id: Option, + #[serde(default)] + team_id: Option, + #[serde(default)] + user_id: Option, +} + +fn split_message(text: &str, max_len: usize) -> Vec { + if text.len() <= max_len { + return vec![text.to_string()]; + } + + let mut chunks = Vec::new(); + let mut remaining = text; + + while !remaining.is_empty() { + if remaining.len() <= max_len { + chunks.push(remaining.to_string()); + break; + } + + let search_region = &remaining[..max_len]; + let break_point = search_region + .rfind('\n') + .or_else(|| search_region.rfind(' ')) + .unwrap_or(max_len); + + let end = remaining.floor_char_boundary(break_point); + chunks.push(remaining[..end].to_string()); + remaining = remaining[end..].trim_start_matches('\n').trim_start(); + } + + chunks +} + +// --- Error Types (add to src/error.rs) --- + +#[derive(Debug, thiserror::Error)] +pub enum AdapterError { + #[error("missing metadata field: {0}")] + MissingMetadata(String), + + #[error("invalid id format: {0}")] + InvalidId(String), + + #[error("API error at {endpoint}: {status} - {message}")] + ApiError { + endpoint: String, + status: u16, + message: String, + }, + + #[error("file too large: {size} bytes (max: {max})")] + FileTooLarge { size: usize, max: usize }, + + #[error("health check failed: {0}")] + HealthCheckFailed(String), + + #[error("invalid configuration: {0}")] + ConfigInvalid(String), + + #[error("adapter not initialized")] + NotInitialized, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_split_message_short() { + let result = split_message("hello", 100); + assert_eq!(result, vec!["hello"]); + } + + #[test] + fn test_split_message_exact_boundary() { + let text = "a".repeat(100); + let result = split_message(&text, 100); + assert_eq!(result.len(), 1); + } + + #[test] + fn test_split_message_on_newline() { + let text = "line1\nline2\nline3"; + let result = split_message(text, 8); + assert_eq!(result, vec!["line1", "line2", "line3"]); + } + + #[test] + fn test_split_message_on_space() { + let text = "word1 word2 word3"; + let result = split_message(text, 12); + assert_eq!(result, vec!["word1 word2", "word3"]); + } + + #[test] + fn test_split_message_forced_break() { + let text = "abcdefghijklmnopqrstuvwxyz"; + let result = split_message(text, 10); + assert_eq!(result, vec!["abcdefghij", "klmnopqrst", "uvwxyz"]); + } + + #[test] + fn test_validate_id_valid() { + assert!(MattermostAdapter::validate_id("abc123").is_ok()); + assert!(MattermostAdapter::validate_id("a-b_c").is_ok()); + } + + #[test] + fn test_validate_id_invalid() { + assert!(MattermostAdapter::validate_id("").is_err()); + assert!(MattermostAdapter::validate_id("has spaces").is_err()); + assert!(MattermostAdapter::validate_id("has/slash").is_err()); + } +} +``` + +### 4. Module Registration (src/messaging/mod.rs) + +```rust +pub mod mattermost; + +pub use mattermost::MattermostAdapter; +``` + +### 5. Target Resolution (src/messaging/target.rs) + +Add to `resolve_broadcast_target()`: + +```rust +"mattermost" => { + if let Some(channel_id) = channel + .platform_meta + .as_ref() + .and_then(|meta| meta.get("mattermost_channel_id")) + .and_then(json_value_to_string) + { + channel_id + } else { + let parts: Vec<&str> = channel.id.split(':').collect(); + match parts.as_slice() { + ["mattermost", _, channel_id] => (*channel_id).to_string(), + ["mattermost", _, "dm", user_id] => format!("dm:{user_id}"), + ["mattermost", _, _, channel_id] => (*channel_id).to_string(), + _ => return None, + } + } +} +``` + +Add to `normalize_target()`: + +```rust +"mattermost" => normalize_mattermost_target(trimmed), +``` + +Add normalization function: + +```rust +fn normalize_mattermost_target(raw_target: &str) -> Option { + let target = strip_repeated_prefix(raw_target, "mattermost"); + + if let Some((_, channel_id)) = target.split_once(':') { + if !channel_id.is_empty() { + return Some(channel_id.to_string()); + } + return None; + } + + if let Some(user_id) = target.strip_prefix("dm:") { + if !user_id.is_empty() { + return Some(format!("dm:{user_id}")); + } + return None; + } + + if target.is_empty() { + None + } else { + Some(target.to_string()) + } +} +``` + +### 6. Binding Support (src/config.rs) + +The `Binding` struct currently has `guild_id`, `workspace_id`, and `chat_id` but **no `team_id` +field**. Add it: + +```rust +pub struct Binding { + // ... existing fields ... + pub team_id: Option, // NEW — Mattermost team ID filter +} +``` + +Add to the TOML deserialization type (`TomlBinding` or equivalent): + +```rust +#[serde(default)] +team_id: Option, +``` + +Add to `Binding::matches()` after the existing channel-type checks: + +```rust +// Mattermost team filter +if let Some(team_id) = &self.team_id { + if self.channel == "mattermost" { + let message_team = message + .metadata + .get("mattermost_team_id") + .and_then(|v| v.as_str()); + if message_team != Some(team_id.as_str()) { + return false; + } + } +} + +// Mattermost channel filter +if !self.channel_ids.is_empty() && self.channel == "mattermost" { + let message_channel = message + .metadata + .get("mattermost_channel_id") + .and_then(|v| v.as_str()); + if !self.channel_ids.iter().any(|id| Some(id.as_str()) == message_channel) { + return false; + } +} +``` + +### 7. Main.rs Registration + +Add following the existing pattern: + +```rust +// Shared Mattermost permissions (hot-reloadable via file watcher) +*mattermost_permissions = config.messaging.mattermost.as_ref().map(|mm_config| { + let perms = MattermostPermissions::from_config(mm_config, &config.bindings); + Arc::new(ArcSwap::from_pointee(perms)) +}); +if let Some(perms) = &*mattermost_permissions { + api_state.set_mattermost_permissions(perms.clone()).await; +} + +if let Some(mm_config) = &config.messaging.mattermost + && mm_config.enabled +{ + if !mm_config.base_url.is_empty() && !mm_config.token.is_empty() { + match MattermostAdapter::new( + Arc::from("mattermost"), + &mm_config.base_url, + Arc::from(mm_config.token.clone()), + mm_config.team_id.clone().map(Arc::from), + mm_config.max_attachment_bytes, + mattermost_permissions.clone().ok_or_else(|| { + anyhow::anyhow!("mattermost permissions not initialized") + })?, + ) { + Ok(adapter) => { + new_messaging_manager.register(adapter).await; + } + Err(error) => { + tracing::error!(%error, "failed to build mattermost adapter"); + } + } + } + + for instance in mm_config + .instances + .iter() + .filter(|i| i.enabled) + { + if instance.base_url.is_empty() || instance.token.is_empty() { + tracing::warn!(adapter = %instance.name, "skipping enabled mattermost instance with missing config"); + continue; + } + let runtime_key = binding_runtime_adapter_key("mattermost", Some(instance.name.as_str())); + let perms = Arc::new(ArcSwap::from_pointee( + MattermostPermissions::from_instance_config(instance, &config.bindings), + )); + match MattermostAdapter::new( + Arc::from(runtime_key), + &instance.base_url, + Arc::from(instance.token.clone()), + instance.team_id.clone().map(Arc::from), + instance.max_attachment_bytes, + perms, + ) { + Ok(adapter) => { + new_messaging_manager.register(adapter).await; + } + Err(error) => { + tracing::error!(adapter = %instance.name, %error, "failed to build named mattermost adapter"); + } + } + } +} +``` + +### 8. Permissions Implementation + +```rust +impl MattermostPermissions { + pub fn from_config(config: &MattermostConfig, bindings: &[Binding]) -> Self { + Self::from_bindings_for_adapter( + config.dm_allowed_users.clone(), + bindings, + None, + ) + } + + pub fn from_instance_config( + instance: &MattermostInstanceConfig, + bindings: &[Binding], + ) -> Self { + Self::from_bindings_for_adapter( + instance.dm_allowed_users.clone(), + bindings, + Some(instance.name.as_str()), + ) + } + + fn from_bindings_for_adapter( + seed_dm_allowed_users: Vec, + bindings: &[Binding], + adapter_selector: Option<&str>, + ) -> Self { + let mm_bindings: Vec<&Binding> = bindings + .iter() + .filter(|b| { + b.channel == "mattermost" + && binding_adapter_selector_matches(b, adapter_selector) + }) + .collect(); + + let team_filter = { + let team_ids: Vec = mm_bindings + .iter() + .filter_map(|b| b.team_id.clone()) + .collect(); + if team_ids.is_empty() { None } else { Some(team_ids) } + }; + + let channel_filter = { + let mut filter: HashMap> = HashMap::new(); + for binding in &mm_bindings { + if let Some(team_id) = &binding.team_id + && !binding.channel_ids.is_empty() + { + filter + .entry(team_id.clone()) + .or_default() + .extend(binding.channel_ids.clone()); + } + } + filter + }; + + let mut dm_allowed_users = seed_dm_allowed_users; + for binding in &mm_bindings { + for id in &binding.dm_allowed_users { + if !dm_allowed_users.contains(id) { + dm_allowed_users.push(id.clone()); + } + } + } + + Self { + team_filter, + channel_filter, + dm_allowed_users, + } + } +} +``` + +### 9. Error Types (src/error.rs) + +Add to the existing Error enum: + +```rust +#[derive(Debug, thiserror::Error)] +pub enum Error { + // ... existing variants + + #[error("adapter error: {0}")] + Adapter(#[from] crate::messaging::mattermost::AdapterError), +} +``` + +--- + +## API Endpoints Used + +| Endpoint | Method | Purpose | +|----------|--------|---------| +| `/users/me` | GET | Bot identity | +| `/users/{id}/typing` | POST | Typing indicator | +| `/posts` | POST | Create message | +| `/posts/{id}` | PUT | Edit message (streaming) | +| `/posts/{id}` | DELETE | Delete message | +| `/channels/{id}/posts` | GET | History fetch | +| `/files` | POST | File upload | +| `/reactions` | POST | Add reaction | +| `/system/ping` | GET | Health check | +| WebSocket `/api/v4/websocket` | - | Real-time events | + +--- + +## Dependencies + +The project already has `tokio-tungstenite = "0.28"` in `Cargo.toml`. Do **not** add a `0.24` +entry — it conflicts with the existing version. Verify the existing entry includes `native-tls`: + +```toml +# Ensure this is already present with the native-tls feature; do not downgrade: +tokio-tungstenite = { version = "0.28", features = ["native-tls"] } + +# Add if not already present: +url = "2.5" +``` + +`futures-util` is already an indirect dependency; verify before adding explicitly. + +--- + +## Feature Support Matrix + +| Feature | Priority | Status | +|---------|----------|--------| +| Text messages | Required | Planned | +| Streaming edits | Required | Planned | +| Typing indicator | Required | Planned | +| Thread replies | High | Planned | +| File attachments | Medium | Planned | +| Reactions | Medium | Planned | +| History fetch | Medium | Planned | +| Rich formatting (markdown) | Low | Planned | +| Interactive buttons | Low | Future | +| Slash commands | Low | Future | + +--- + +## Testing Strategy + +1. **Unit tests**: Message splitting, ID validation, target normalization, permission filtering +2. **Integration tests**: Mock Mattermost API server (mockito) +3. **Manual testing**: Against real Mattermost instance + +--- + +## Migration Notes + +- No database migrations required +- Configuration is additive only +- No breaking changes to existing adapters + +--- + +## Checklist for Implementation + +- [ ] Add `MattermostConfig` and related types to `src/config.rs` +- [ ] Add TOML deserialization types +- [ ] Add URL validation +- [ ] Update `is_named_adapter_platform` +- [ ] Add `AdapterError` to `src/error.rs` +- [ ] Create `src/messaging/mattermost.rs` with full implementation +- [ ] Add to `src/messaging/mod.rs` +- [ ] Update `src/messaging/target.rs` for broadcast resolution +- [ ] Add main.rs registration +- [ ] Add unit tests +- [ ] Add integration tests with mock server +- [ ] Update README.md with Mattermost configuration docs diff --git a/src/config.rs b/src/config.rs index c3a9ec8ac..b0417da7f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1371,6 +1371,8 @@ pub struct Binding { pub guild_id: Option, pub workspace_id: Option, // Slack workspace (team) ID pub chat_id: Option, + /// Mattermost team ID filter. + pub team_id: Option, /// Channel IDs this binding applies to. If empty, all channels in the guild/workspace are allowed. pub channel_ids: Vec, /// Require explicit @mention (or reply-to-bot) for inbound messages. @@ -1448,7 +1450,7 @@ impl Binding { .and_then(|v| v.as_u64()) .map(|v| v.to_string()); - // Also check Slack and Twitch channel IDs + // Also check Slack, Twitch, and Mattermost channel IDs let slack_channel = message .metadata .get("slack_channel_id") @@ -1457,12 +1459,18 @@ impl Binding { .metadata .get("twitch_channel") .and_then(|v| v.as_str()); + let mattermost_channel = message + .metadata + .get("mattermost_channel_id") + .and_then(|v| v.as_str()); let direct_match = message_channel .as_ref() .is_some_and(|id| self.channel_ids.contains(id)) || slack_channel.is_some_and(|id| self.channel_ids.contains(&id.to_string())) - || twitch_channel.is_some_and(|id| self.channel_ids.contains(&id.to_string())); + || twitch_channel.is_some_and(|id| self.channel_ids.contains(&id.to_string())) + || mattermost_channel + .is_some_and(|id| self.channel_ids.contains(&id.to_string())); let parent_match = parent_channel .as_ref() .is_some_and(|id| self.channel_ids.contains(id)); @@ -1472,6 +1480,19 @@ impl Binding { } } + // Mattermost team filter + if let Some(team_id) = &self.team_id { + if self.channel == "mattermost" { + let message_team = message + .metadata + .get("mattermost_team_id") + .and_then(|v| v.as_str()); + if message_team != Some(team_id.as_str()) { + return false; + } + } + } + if self.channel == "discord" && self.require_mention { let is_guild_message = message .metadata @@ -1534,7 +1555,7 @@ struct AdapterValidationState { fn is_named_adapter_platform(platform: &str) -> bool { matches!( platform, - "discord" | "slack" | "telegram" | "twitch" | "email" + "discord" | "slack" | "telegram" | "twitch" | "email" | "mattermost" ) } @@ -1697,9 +1718,52 @@ fn build_adapter_validation_states( ); } + if let Some(mattermost) = &messaging.mattermost { + let named_instances = validate_instance_names( + "mattermost", + mattermost.instances.iter().map(|i| i.name.as_str()), + )?; + let default_present = + !mattermost.base_url.trim().is_empty() && !mattermost.token.trim().is_empty(); + validate_runtime_keys("mattermost", default_present, &named_instances)?; + if default_present { + validate_mattermost_url(&mattermost.base_url)?; + } + for instance in &mattermost.instances { + if instance.enabled && !instance.base_url.is_empty() { + validate_mattermost_url(&instance.base_url)?; + } + } + states.insert( + "mattermost", + AdapterValidationState { + default_present, + named_instances, + }, + ); + } + Ok(states) } +fn validate_mattermost_url(url: &str) -> Result<()> { + let parsed = url::Url::parse(url) + .map_err(|e| ConfigError::Invalid(format!("invalid mattermost base_url '{url}': {e}")))?; + match parsed.scheme() { + "https" => {} + "http" => { + tracing::warn!(url, "mattermost base_url uses http instead of https"); + } + scheme => { + return Err(ConfigError::Invalid(format!( + "mattermost base_url must use http or https, got: {scheme}" + )) + .into()); + } + } + Ok(()) +} + fn validate_instance_names<'a>( platform: &str, names: impl Iterator, @@ -1800,6 +1864,7 @@ pub struct MessagingConfig { pub email: Option, pub webhook: Option, pub twitch: Option, + pub mattermost: Option, } #[derive(Clone)] @@ -2618,6 +2683,146 @@ fn binding_adapter_selector_matches(binding: &Binding, adapter_selector: Option< } } +/// Mattermost bot configuration. +#[derive(Clone)] +pub struct MattermostConfig { + pub enabled: bool, + pub base_url: String, + pub token: String, + pub team_id: Option, + pub instances: Vec, + pub dm_allowed_users: Vec, + pub max_attachment_bytes: usize, +} + +impl std::fmt::Debug for MattermostConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MattermostConfig") + .field("enabled", &self.enabled) + .field("base_url", &self.base_url) + .field("token", &"[REDACTED]") + .field("team_id", &self.team_id) + .field("instances", &self.instances) + .field("dm_allowed_users", &self.dm_allowed_users) + .field("max_attachment_bytes", &self.max_attachment_bytes) + .finish() + } +} + +/// Named Mattermost bot instance configuration. +#[derive(Clone)] +pub struct MattermostInstanceConfig { + pub name: String, + pub enabled: bool, + pub base_url: String, + pub token: String, + pub team_id: Option, + pub dm_allowed_users: Vec, + pub max_attachment_bytes: usize, +} + +impl std::fmt::Debug for MattermostInstanceConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MattermostInstanceConfig") + .field("name", &self.name) + .field("enabled", &self.enabled) + .field("base_url", &self.base_url) + .field("token", &"[REDACTED]") + .field("team_id", &self.team_id) + .field("dm_allowed_users", &self.dm_allowed_users) + .field("max_attachment_bytes", &self.max_attachment_bytes) + .finish() + } +} + +/// Per-adapter permissions for the Mattermost platform. +/// +/// Shared with the Mattermost adapter via `Arc>` for hot-reloading. +#[derive(Debug, Clone, Default)] +pub struct MattermostPermissions { + /// Allowed team IDs (None = all teams accepted). + pub team_filter: Option>, + /// Allowed channel IDs per team (empty = all channels accepted). + pub channel_filter: HashMap>, + /// User IDs allowed to DM the bot. + pub dm_allowed_users: Vec, +} + +impl MattermostPermissions { + /// Build from the current config's mattermost settings and bindings. + pub fn from_config(config: &MattermostConfig, bindings: &[Binding]) -> Self { + Self::from_bindings_for_adapter(config.dm_allowed_users.clone(), bindings, None) + } + + /// Build permissions for a named Mattermost adapter instance. + pub fn from_instance_config( + instance: &MattermostInstanceConfig, + bindings: &[Binding], + ) -> Self { + Self::from_bindings_for_adapter( + instance.dm_allowed_users.clone(), + bindings, + Some(instance.name.as_str()), + ) + } + + fn from_bindings_for_adapter( + seed_dm_allowed_users: Vec, + bindings: &[Binding], + adapter_selector: Option<&str>, + ) -> Self { + let mm_bindings: Vec<&Binding> = bindings + .iter() + .filter(|b| { + b.channel == "mattermost" + && binding_adapter_selector_matches(b, adapter_selector) + }) + .collect(); + + let team_filter = { + let team_ids: Vec = mm_bindings + .iter() + .filter_map(|b| b.team_id.clone()) + .collect(); + if team_ids.is_empty() { + None + } else { + Some(team_ids) + } + }; + + let channel_filter = { + let mut filter: HashMap> = HashMap::new(); + for binding in &mm_bindings { + if let Some(team_id) = &binding.team_id { + if !binding.channel_ids.is_empty() { + filter + .entry(team_id.clone()) + .or_default() + .extend(binding.channel_ids.clone()); + } + } + } + filter + }; + + let mut dm_allowed_users = seed_dm_allowed_users; + for binding in &mm_bindings { + for id in &binding.dm_allowed_users { + if !dm_allowed_users.contains(id) { + dm_allowed_users.push(id.clone()); + } + } + } + + Self { + team_filter, + channel_filter, + dm_allowed_users, + } + } +} + #[derive(Debug, Clone)] pub struct WebhookConfig { pub enabled: bool, @@ -3086,6 +3291,7 @@ struct TomlMessagingConfig { email: Option, webhook: Option, twitch: Option, + mattermost: Option, } #[derive(Deserialize)] @@ -3284,6 +3490,39 @@ fn default_webhook_bind() -> String { "127.0.0.1".into() } +#[derive(Deserialize)] +struct TomlMattermostConfig { + #[serde(default)] + enabled: bool, + base_url: Option, + token: Option, + team_id: Option, + #[serde(default)] + instances: Vec, + #[serde(default)] + dm_allowed_users: Vec, + #[serde(default = "default_max_attachment_bytes")] + max_attachment_bytes: usize, +} + +#[derive(Deserialize)] +struct TomlMattermostInstanceConfig { + name: String, + #[serde(default)] + enabled: bool, + base_url: Option, + token: Option, + team_id: Option, + #[serde(default)] + dm_allowed_users: Vec, + #[serde(default = "default_max_attachment_bytes")] + max_attachment_bytes: usize, +} + +fn default_max_attachment_bytes() -> usize { + 10 * 1024 * 1024 +} + fn default_email_imap_port() -> u16 { 993 } @@ -3326,6 +3565,8 @@ struct TomlBinding { workspace_id: Option, chat_id: Option, #[serde(default)] + team_id: Option, + #[serde(default)] channel_ids: Vec, #[serde(default)] require_mention: bool, @@ -5457,6 +5698,53 @@ impl Config { trigger_prefix: t.trigger_prefix, }) }), + mattermost: toml.messaging.mattermost.and_then(|mm| { + let instances = mm + .instances + .into_iter() + .map(|instance| { + let token = instance.token.as_deref().and_then(resolve_env_value); + let base_url = instance.base_url.as_deref().and_then(resolve_env_value); + if instance.enabled && (token.is_none() || base_url.is_none()) { + tracing::warn!( + adapter = %instance.name, + "mattermost instance is enabled but credentials are missing/unresolvable — disabling" + ); + } + let has_credentials = token.is_some() && base_url.is_some(); + MattermostInstanceConfig { + name: instance.name, + enabled: instance.enabled && has_credentials, + base_url: base_url.unwrap_or_default(), + token: token.unwrap_or_default(), + team_id: instance.team_id, + dm_allowed_users: instance.dm_allowed_users, + max_attachment_bytes: instance.max_attachment_bytes, + } + }) + .collect::>(); + + let token = std::env::var("MATTERMOST_TOKEN") + .ok() + .or_else(|| mm.token.as_deref().and_then(resolve_env_value)); + let base_url = std::env::var("MATTERMOST_BASE_URL") + .ok() + .or_else(|| mm.base_url.as_deref().and_then(resolve_env_value)); + + if (token.is_none() || base_url.is_none()) && instances.is_empty() { + return None; + } + + Some(MattermostConfig { + enabled: mm.enabled, + base_url: base_url.unwrap_or_default(), + token: token.unwrap_or_default(), + team_id: mm.team_id, + instances, + dm_allowed_users: mm.dm_allowed_users, + max_attachment_bytes: mm.max_attachment_bytes, + }) + }), }; let bindings: Vec = toml @@ -5469,6 +5757,7 @@ impl Config { guild_id: b.guild_id, workspace_id: b.workspace_id, chat_id: b.chat_id, + team_id: b.team_id, channel_ids: b.channel_ids, require_mention: b.require_mention, dm_allowed_users: b.dm_allowed_users, @@ -5836,6 +6125,7 @@ pub fn spawn_file_watcher( slack_permissions: Option>>, telegram_permissions: Option>>, twitch_permissions: Option>>, + mattermost_permissions: Option>>, bindings: Arc>>, messaging_manager: Option>, llm_manager: Arc, @@ -6039,6 +6329,14 @@ pub fn spawn_file_watcher( tracing::info!("twitch permissions reloaded"); } + if let Some(ref perms) = mattermost_permissions + && let Some(mattermost_config) = &config.messaging.mattermost + { + let new_perms = MattermostPermissions::from_config(mattermost_config, &config.bindings); + perms.store(Arc::new(new_perms)); + tracing::info!("mattermost permissions reloaded"); + } + // Hot-start adapters that are newly enabled in the config if let Some(ref manager) = messaging_manager { let rt = tokio::runtime::Handle::current(); @@ -6048,6 +6346,7 @@ pub fn spawn_file_watcher( let slack_permissions = slack_permissions.clone(); let telegram_permissions = telegram_permissions.clone(); let twitch_permissions = twitch_permissions.clone(); + let mattermost_permissions = mattermost_permissions.clone(); let instance_dir = instance_dir.clone(); rt.spawn(async move { @@ -6330,6 +6629,71 @@ pub fn spawn_file_watcher( } } } + + if let Some(mattermost_config) = &config.messaging.mattermost + && mattermost_config.enabled { + if !mattermost_config.base_url.is_empty() + && !mattermost_config.token.is_empty() + && !manager.has_adapter("mattermost").await + { + let permissions = match mattermost_permissions { + Some(ref existing) => existing.clone(), + None => { + let permissions = MattermostPermissions::from_config(mattermost_config, &config.bindings); + Arc::new(arc_swap::ArcSwap::from_pointee(permissions)) + } + }; + match crate::messaging::mattermost::MattermostAdapter::new( + "mattermost", + &mattermost_config.base_url, + mattermost_config.token.as_str(), + mattermost_config.team_id.as_deref().map(Arc::from), + mattermost_config.max_attachment_bytes, + permissions, + ) { + Ok(adapter) => { + if let Err(error) = manager.register_and_start(adapter).await { + tracing::error!(%error, "failed to hot-start mattermost adapter from config change"); + } + } + Err(error) => { + tracing::error!(%error, "failed to create mattermost adapter for hot-start"); + } + } + } + + for instance in mattermost_config.instances.iter().filter(|instance| instance.enabled) { + let runtime_key = binding_runtime_adapter_key( + "mattermost", + Some(instance.name.as_str()), + ); + if manager.has_adapter(runtime_key.as_str()).await { + // TODO: named instance permissions not hot-updated (see discord block comment) + continue; + } + + let permissions = Arc::new(arc_swap::ArcSwap::from_pointee( + MattermostPermissions::from_instance_config(instance, &config.bindings), + )); + match crate::messaging::mattermost::MattermostAdapter::new( + runtime_key, + &instance.base_url, + instance.token.as_str(), + instance.team_id.as_deref().map(Arc::from), + instance.max_attachment_bytes, + permissions, + ) { + Ok(adapter) => { + if let Err(error) = manager.register_and_start(adapter).await { + tracing::error!(%error, adapter = %instance.name, "failed to hot-start named mattermost adapter from config change"); + } + } + Err(error) => { + tracing::error!(%error, adapter = %instance.name, "failed to create named mattermost adapter for hot-start"); + } + } + } + } }); } } @@ -7180,6 +7544,7 @@ bind = "127.0.0.1" guild_id: None, workspace_id: workspace_id.map(String::from), chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users, @@ -7926,6 +8291,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -7942,6 +8308,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -7973,6 +8340,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -7990,6 +8358,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -8007,6 +8376,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -8024,6 +8394,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -8041,6 +8412,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -8068,6 +8440,7 @@ startup_delay_secs = 2 email: None, webhook: None, twitch: None, + mattermost: None, }; let bindings = vec![ Binding { @@ -8077,6 +8450,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -8088,6 +8462,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -8110,6 +8485,7 @@ startup_delay_secs = 2 email: None, webhook: None, twitch: None, + mattermost: None, }; let bindings = vec![Binding { agent_id: "main".into(), @@ -8118,6 +8494,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -8172,6 +8549,7 @@ startup_delay_secs = 2 }), webhook: None, twitch: None, + mattermost: None, }; let bindings = vec![Binding { agent_id: "main".into(), @@ -8180,6 +8558,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], @@ -8206,6 +8585,7 @@ startup_delay_secs = 2 email: None, webhook: None, twitch: None, + mattermost: None, }; // Binding targets default adapter, but no default credentials exist let bindings = vec![Binding { @@ -8215,6 +8595,7 @@ startup_delay_secs = 2 guild_id: None, workspace_id: None, chat_id: None, + team_id: None, channel_ids: vec![], require_mention: false, dm_allowed_users: vec![], diff --git a/src/main.rs b/src/main.rs index ae4dceae0..60180d6ab 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1486,6 +1486,7 @@ async fn run( let mut slack_permissions = None; let mut telegram_permissions = None; let mut twitch_permissions = None; + let mut mattermost_permissions = None; initialize_agents( &config, &llm_manager, @@ -1503,6 +1504,7 @@ async fn run( &mut slack_permissions, &mut telegram_permissions, &mut twitch_permissions, + &mut mattermost_permissions, agent_links.clone(), injection_tx.clone(), task_store_registry.clone(), @@ -1520,6 +1522,7 @@ async fn run( slack_permissions, telegram_permissions, twitch_permissions, + mattermost_permissions, bindings.clone(), Some(messaging_manager.clone()), llm_manager.clone(), @@ -1535,6 +1538,7 @@ async fn run( None, None, None, + None, bindings.clone(), None, llm_manager.clone(), @@ -1857,6 +1861,7 @@ async fn run( let mut new_slack_permissions = None; let mut new_telegram_permissions = None; let mut new_twitch_permissions = None; + let mut new_mattermost_permissions = None; match initialize_agents( &new_config, &new_llm_manager, @@ -1874,6 +1879,7 @@ async fn run( &mut new_slack_permissions, &mut new_telegram_permissions, &mut new_twitch_permissions, + &mut new_mattermost_permissions, agent_links.clone(), injection_tx.clone(), task_store_registry.clone(), @@ -1890,6 +1896,7 @@ async fn run( new_slack_permissions, new_telegram_permissions, new_twitch_permissions, + new_mattermost_permissions, bindings.clone(), Some(messaging_manager.clone()), new_llm_manager.clone(), @@ -2010,6 +2017,7 @@ async fn initialize_agents( slack_permissions: &mut Option>>, telegram_permissions: &mut Option>>, twitch_permissions: &mut Option>>, + mattermost_permissions: &mut Option>>, agent_links: Arc>>, injection_tx: tokio::sync::mpsc::Sender, task_store_registry: Arc< @@ -2631,6 +2639,68 @@ async fn initialize_agents( } } + // Shared Mattermost permissions (hot-reloadable via file watcher) + *mattermost_permissions = config.messaging.mattermost.as_ref().map(|mattermost_config| { + let perms = spacebot::config::MattermostPermissions::from_config(mattermost_config, &config.bindings); + Arc::new(ArcSwap::from_pointee(perms)) + }); + + if let Some(mattermost_config) = &config.messaging.mattermost + && mattermost_config.enabled + { + if !mattermost_config.base_url.is_empty() && !mattermost_config.token.is_empty() { + match spacebot::messaging::mattermost::MattermostAdapter::new( + "mattermost", + &mattermost_config.base_url, + mattermost_config.token.as_str(), + mattermost_config.team_id.as_deref().map(Arc::from), + mattermost_config.max_attachment_bytes, + mattermost_permissions.clone().ok_or_else(|| { + anyhow::anyhow!("mattermost permissions not initialized when mattermost is enabled") + })?, + ) { + Ok(adapter) => { + new_messaging_manager.register(adapter).await; + } + Err(error) => { + tracing::error!(%error, "failed to create mattermost adapter"); + } + } + } + + for instance in mattermost_config.instances.iter().filter(|instance| instance.enabled) { + if instance.base_url.is_empty() || instance.token.is_empty() { + tracing::warn!(adapter = %instance.name, "skipping enabled mattermost instance with missing credentials"); + continue; + } + let runtime_key = spacebot::config::binding_runtime_adapter_key( + "mattermost", + Some(instance.name.as_str()), + ); + let perms = Arc::new(ArcSwap::from_pointee( + spacebot::config::MattermostPermissions::from_instance_config( + instance, + &config.bindings, + ), + )); + match spacebot::messaging::mattermost::MattermostAdapter::new( + runtime_key, + &instance.base_url, + instance.token.as_str(), + instance.team_id.as_deref().map(Arc::from), + instance.max_attachment_bytes, + perms, + ) { + Ok(adapter) => { + new_messaging_manager.register(adapter).await; + } + Err(error) => { + tracing::error!(%error, adapter = %instance.name, "failed to create named mattermost adapter"); + } + } + } + } + let webchat_agent_pools = agents .iter() .map(|(agent_id, agent)| (agent_id.to_string(), agent.db.sqlite.clone())) diff --git a/src/messaging.rs b/src/messaging.rs index f57d5d68a..5554c9fa2 100644 --- a/src/messaging.rs +++ b/src/messaging.rs @@ -1,8 +1,9 @@ -//! Messaging adapters (Discord, Slack, Telegram, Twitch, Email, Webhook, WebChat). +//! Messaging adapters (Discord, Slack, Telegram, Twitch, Email, Webhook, WebChat, Mattermost). pub mod discord; pub mod email; pub mod manager; +pub mod mattermost; pub mod slack; pub mod target; pub mod telegram; diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs new file mode 100644 index 000000000..80340c9cf --- /dev/null +++ b/src/messaging/mattermost.rs @@ -0,0 +1,1031 @@ +//! Mattermost messaging adapter using a custom HTTP + WebSocket client. + +use crate::config::MattermostPermissions; +use crate::messaging::apply_runtime_adapter_to_conversation_id; +use crate::messaging::traits::{HistoryMessage, InboundStream, Messaging}; +use crate::{InboundMessage, MessageContent, OutboundResponse, StatusUpdate}; + +use anyhow::Context as _; +use arc_swap::ArcSwap; +use futures::{SinkExt, StreamExt}; +use reqwest::Client; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::sync::Arc; +use std::time::{Duration, Instant}; +use tokio::sync::{OnceCell, RwLock, mpsc}; +use tokio_tungstenite::{ + connect_async, + tungstenite::Message as WsMessage, +}; +use url::Url; + +const MAX_MESSAGE_LENGTH: usize = 16_383; +const STREAM_EDIT_THROTTLE: Duration = Duration::from_millis(500); +const TYPING_INDICATOR_INTERVAL: Duration = Duration::from_secs(5); +const WS_RECONNECT_BASE_DELAY: Duration = Duration::from_secs(1); +const WS_RECONNECT_MAX_DELAY: Duration = Duration::from_secs(60); +const HTTP_TIMEOUT: Duration = Duration::from_secs(30); + +pub struct MattermostAdapter { + runtime_key: Arc, + base_url: Url, + token: Arc, + default_team_id: Option>, + max_attachment_bytes: usize, + client: Client, + permissions: Arc>, + bot_user_id: OnceCell>, + bot_username: OnceCell>, + active_messages: Arc>>, + typing_tasks: Arc>>>, + shutdown_tx: Arc>>>, + ws_task: Arc>>>, +} + +struct ActiveStream { + post_id: Arc, + #[allow(dead_code)] + channel_id: Arc, + last_edit: Instant, + accumulated_text: String, +} + +impl std::fmt::Debug for MattermostAdapter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MattermostAdapter") + .field("runtime_key", &self.runtime_key) + .field("base_url", &self.base_url) + .field("token", &"[REDACTED]") + .field("default_team_id", &self.default_team_id) + .field("max_attachment_bytes", &self.max_attachment_bytes) + .finish() + } +} + +impl MattermostAdapter { + pub fn new( + runtime_key: impl Into>, + base_url: &str, + token: impl Into>, + default_team_id: Option>, + max_attachment_bytes: usize, + permissions: Arc>, + ) -> anyhow::Result { + let base_url = + Url::parse(base_url).context("invalid mattermost base_url")?; + + let client = Client::builder() + .timeout(HTTP_TIMEOUT) + .pool_idle_timeout(Duration::from_secs(30)) + .build() + .context("failed to build HTTP client")?; + + Ok(Self { + runtime_key: runtime_key.into(), + base_url, + token: token.into(), + default_team_id, + max_attachment_bytes, + client, + permissions, + bot_user_id: OnceCell::new(), + bot_username: OnceCell::new(), + active_messages: Arc::new(RwLock::new(HashMap::new())), + typing_tasks: Arc::new(RwLock::new(HashMap::new())), + shutdown_tx: Arc::new(RwLock::new(None)), + ws_task: Arc::new(RwLock::new(None)), + }) + } + + fn api_url(&self, path: &str) -> Url { + let mut url = self.base_url.clone(); + url.path_segments_mut() + .expect("base_url is a valid base URL") + .extend(["api", "v4"]) + .extend(path.trim_start_matches('/').split('/')); + url + } + + fn ws_url(&self) -> Url { + let mut url = self.base_url.clone(); + url.set_scheme(match self.base_url.scheme() { + "https" => "wss", + "http" => "ws", + other => other, + }) + .expect("scheme substitution is valid"); + url.path_segments_mut() + .expect("base_url is a valid base URL") + .extend(["api", "v4", "websocket"]); + url + } + + fn extract_channel_id<'a>(&self, message: &'a InboundMessage) -> crate::Result<&'a str> { + message + .metadata + .get("mattermost_channel_id") + .and_then(|v| v.as_str()) + .ok_or_else(|| { + anyhow::anyhow!("missing mattermost_channel_id metadata").into() + }) + } + + fn validate_id(id: &str) -> crate::Result<()> { + if id.is_empty() || id.len() > 64 { + return Err(anyhow::anyhow!("invalid mattermost ID: empty or too long").into()); + } + if !id.chars().all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') { + return Err(anyhow::anyhow!("invalid mattermost ID format: {id}").into()); + } + Ok(()) + } + + async fn stop_typing(&self, channel_id: &str) { + if let Some(handle) = self.typing_tasks.write().await.remove(channel_id) { + handle.abort(); + } + } + + async fn start_typing(&self, channel_id: &str) { + let Some(user_id) = self.bot_user_id.get().cloned() else { + return; + }; + let channel_id_owned = channel_id.to_string(); + let client = self.client.clone(); + let token = self.token.clone(); + let url = self.api_url(&format!("/users/{user_id}/typing")); + + let handle = tokio::spawn(async move { + let mut interval = tokio::time::interval(TYPING_INDICATOR_INTERVAL); + loop { + interval.tick().await; + let result = client + .post(url.clone()) + .bearer_auth(token.as_ref()) + .json(&serde_json::json!({ + "channel_id": channel_id_owned, + "parent_id": "", + })) + .send() + .await; + if let Err(error) = result { + tracing::warn!(%error, "typing indicator request failed"); + } + } + }); + + self.typing_tasks + .write() + .await + .insert(channel_id.to_string(), handle); + } + + async fn create_post( + &self, + channel_id: &str, + message: &str, + root_id: Option<&str>, + ) -> crate::Result { + Self::validate_id(channel_id)?; + if let Some(rid) = root_id { + Self::validate_id(rid)?; + } + + let response = self + .client + .post(self.api_url("/posts")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ + "channel_id": channel_id, + "message": message, + "root_id": root_id.unwrap_or(""), + })) + .send() + .await + .context("failed to create post")?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost POST /posts failed with status {}: {body}", + status.as_u16() + ) + .into()); + } + + response + .json() + .await + .context("failed to parse post response") + .map_err(Into::into) + } + + async fn edit_post(&self, post_id: &str, message: &str) -> crate::Result<()> { + Self::validate_id(post_id)?; + + let response = self + .client + .put(self.api_url(&format!("/posts/{post_id}"))) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ "message": message })) + .send() + .await + .context("failed to edit post")?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost PUT /posts/{post_id} failed with status {}: {body}", + status.as_u16() + ) + .into()); + } + + Ok(()) + } + + async fn get_channel_posts( + &self, + channel_id: &str, + before_post_id: Option<&str>, + limit: u32, + ) -> crate::Result { + Self::validate_id(channel_id)?; + + let mut url = self.api_url(&format!("/channels/{channel_id}/posts")); + { + let mut query = url.query_pairs_mut(); + query.append_pair("page", "0"); + query.append_pair("per_page", &limit.to_string()); + if let Some(before) = before_post_id { + query.append_pair("before", before); + } + } + + let response = self + .client + .get(url) + .bearer_auth(self.token.as_ref()) + .send() + .await + .context("failed to fetch channel posts")?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost GET /channels/{channel_id}/posts failed with status {}: {body}", + status.as_u16() + ) + .into()); + } + + response + .json() + .await + .context("failed to parse posts response") + .map_err(Into::into) + } +} + +impl Messaging for MattermostAdapter { + fn name(&self) -> &str { + &self.runtime_key + } + + async fn start(&self) -> crate::Result { + let (inbound_tx, inbound_rx) = mpsc::channel(256); + let (shutdown_tx, mut shutdown_rx) = mpsc::channel::<()>(1); + *self.shutdown_tx.write().await = Some(shutdown_tx); + + let me: MattermostUser = self + .client + .get(self.api_url("/users/me")) + .bearer_auth(self.token.as_ref()) + .send() + .await + .context("failed to get bot user")? + .json() + .await + .context("failed to parse user response")?; + + let user_id: Arc = me.id.clone().into(); + let username: Arc = me.username.clone().into(); + + self.bot_user_id.set(user_id.clone()).ok(); + self.bot_username.set(username.clone()).ok(); + + tracing::info!( + adapter = %self.runtime_key, + bot_id = %user_id, + bot_username = %username, + "mattermost adapter connected" + ); + + let ws_url = self.ws_url(); + let runtime_key = self.runtime_key.clone(); + let token = self.token.clone(); + let permissions = self.permissions.clone(); + let bot_user_id = user_id; + let inbound_tx_clone = inbound_tx.clone(); + + let handle = tokio::spawn(async move { + let mut retry_delay = WS_RECONNECT_BASE_DELAY; + + loop { + let connect_result = connect_async(ws_url.as_str()).await; + + match connect_result { + Ok((ws_stream, _)) => { + retry_delay = WS_RECONNECT_BASE_DELAY; + + let (mut write, mut read) = ws_stream.split(); + + let auth_msg = serde_json::json!({ + "seq": 1, + "action": "authentication_challenge", + "data": {"token": token.as_ref()} + }); + + if let Ok(msg) = serde_json::to_string(&auth_msg) { + if write.send(WsMessage::Text(msg.into())).await.is_err() { + tracing::error!(adapter = %runtime_key, "failed to send websocket auth"); + continue; + } + } + + loop { + tokio::select! { + _ = shutdown_rx.recv() => { + tracing::info!(adapter = %runtime_key, "mattermost websocket shutting down"); + let _ = write.send(WsMessage::Close(None)).await; + return; + } + + msg = read.next() => { + match msg { + Some(Ok(WsMessage::Text(text))) => { + if let Ok(event) = serde_json::from_str::(&text) { + if event.event == "posted" { + // The post is double-encoded as a JSON string in the data field. + let post_result = event + .data + .get("post") + .and_then(|v| v.as_str()) + .and_then(|s| serde_json::from_str::(s).ok()); + + if let Some(mut post) = post_result { + if post.user_id != bot_user_id.as_ref() { + // channel_type comes from event.data, not the post struct. + let channel_type = event + .data + .get("channel_type") + .and_then(|v| v.as_str()) + .map(String::from); + post.channel_type = channel_type; + + let team_id = event.broadcast.team_id.clone(); + let perms = permissions.load(); + if let Some(msg) = build_message_from_post( + &post, + &runtime_key, + &bot_user_id, + &team_id, + &perms, + ) { + if inbound_tx_clone.send(msg).await.is_err() { + tracing::debug!("inbound channel closed"); + return; + } + } + } + } + } + } + } + Some(Ok(WsMessage::Ping(data))) => { + if write.send(WsMessage::Pong(data)).await.is_err() { + tracing::warn!(adapter = %runtime_key, "failed to send pong"); + break; + } + } + Some(Ok(WsMessage::Pong(_))) => {} + Some(Ok(WsMessage::Close(_))) => { + tracing::info!(adapter = %runtime_key, "websocket closed by server"); + break; + } + Some(Err(e)) => { + tracing::error!(adapter = %runtime_key, error = %e, "websocket error"); + break; + } + None => break, + _ => {} + } + } + } + } + + tracing::info!(adapter = %runtime_key, "websocket disconnected, reconnecting..."); + } + Err(e) => { + tracing::error!( + adapter = %runtime_key, + error = %e, + delay_ms = retry_delay.as_millis(), + "websocket connection failed, retrying" + ); + } + } + + tokio::select! { + _ = tokio::time::sleep(retry_delay) => { + retry_delay = (retry_delay * 2).min(WS_RECONNECT_MAX_DELAY); + } + _ = shutdown_rx.recv() => { + tracing::info!(adapter = %runtime_key, "mattermost adapter shutting down during reconnect delay"); + return; + } + } + } + }); + + *self.ws_task.write().await = Some(handle); + + let stream = tokio_stream::wrappers::ReceiverStream::new(inbound_rx); + Ok(Box::pin(stream)) + } + + async fn respond( + &self, + message: &InboundMessage, + response: OutboundResponse, + ) -> crate::Result<()> { + let channel_id = self.extract_channel_id(message)?; + + match response { + OutboundResponse::Text(text) => { + self.stop_typing(channel_id).await; + // Use root_id for threading: prefer mattermost_root_id (when triggered from a + // threaded message) or REPLY_TO_MESSAGE_ID (set by channel.rs for branch/worker + // replies). + let root_id = message + .metadata + .get("mattermost_root_id") + .and_then(|v| v.as_str()) + .or_else(|| { + message + .metadata + .get(crate::metadata_keys::REPLY_TO_MESSAGE_ID) + .and_then(|v| v.as_str()) + }); + + for chunk in split_message(&text, MAX_MESSAGE_LENGTH) { + self.create_post(channel_id, &chunk, root_id).await?; + } + } + + OutboundResponse::StreamStart => { + let root_id = message + .metadata + .get("mattermost_root_id") + .and_then(|v| v.as_str()); + self.start_typing(channel_id).await; + // Create a placeholder post with a zero-width space. + let post = self.create_post(channel_id, "\u{200B}", root_id).await?; + self.active_messages.write().await.insert( + message.id.clone(), + ActiveStream { + post_id: post.id.into(), + channel_id: channel_id.to_string().into(), + last_edit: Instant::now(), + accumulated_text: String::new(), + }, + ); + } + + OutboundResponse::StreamChunk(chunk) => { + let mut active_messages = self.active_messages.write().await; + if let Some(active) = active_messages.get_mut(&message.id) { + active.accumulated_text.push_str(&chunk); + + if active.last_edit.elapsed() > STREAM_EDIT_THROTTLE { + let display_text = if active.accumulated_text.len() > MAX_MESSAGE_LENGTH { + let end = active + .accumulated_text + .floor_char_boundary(MAX_MESSAGE_LENGTH - 3); + format!("{}...", &active.accumulated_text[..end]) + } else { + active.accumulated_text.clone() + }; + + if let Err(error) = self.edit_post(&active.post_id, &display_text).await { + tracing::warn!(%error, "failed to edit streaming message"); + } + active.last_edit = Instant::now(); + } + } + } + + OutboundResponse::StreamEnd => { + self.stop_typing(channel_id).await; + if let Some(active) = self.active_messages.write().await.remove(&message.id) { + if let Err(error) = + self.edit_post(&active.post_id, &active.accumulated_text).await + { + tracing::warn!(%error, "failed to finalize streaming message"); + } + } + } + + OutboundResponse::Status(status) => self.send_status(message, status).await?, + + OutboundResponse::Reaction(emoji) => { + let post_id = message + .metadata + .get("mattermost_post_id") + .and_then(|v| v.as_str()) + .ok_or_else(|| { + anyhow::anyhow!("missing mattermost_post_id metadata") + })?; + let emoji_name = emoji.trim_matches(':'); + + let bot_user_id = self + .bot_user_id + .get() + .map(|s| s.as_ref().to_string()) + .unwrap_or_default(); + + let response = self + .client + .post(self.api_url("/reactions")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ + "user_id": bot_user_id, + "post_id": post_id, + "emoji_name": emoji_name, + })) + .send() + .await + .context("failed to add reaction")?; + + if !response.status().is_success() { + tracing::warn!( + status = %response.status(), + emoji = %emoji_name, + "failed to add reaction" + ); + } + } + + OutboundResponse::File { + filename, + data, + mime_type, + caption, + } => { + if data.len() > self.max_attachment_bytes { + return Err(anyhow::anyhow!( + "file too large: {} bytes (max: {})", + data.len(), + self.max_attachment_bytes + ) + .into()); + } + + let part = reqwest::multipart::Part::bytes(data) + .file_name(filename.clone()) + .mime_str(&mime_type) + .context("invalid mime type")?; + + let form = reqwest::multipart::Form::new() + .part("files", part) + .text("channel_id", channel_id.to_string()); + + let response = self + .client + .post(self.api_url("/files")) + .bearer_auth(self.token.as_ref()) + .multipart(form) + .send() + .await + .context("failed to upload file")?; + + if !response.status().is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost file upload failed: {body}" + ) + .into()); + } + + let upload: MattermostFileUpload = response + .json() + .await + .context("failed to parse file upload response")?; + + let file_ids: Vec<_> = + upload.file_infos.iter().map(|f| f.id.as_str()).collect(); + self.client + .post(self.api_url("/posts")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ + "channel_id": channel_id, + "message": caption.unwrap_or_default(), + "file_ids": file_ids, + })) + .send() + .await + .context("failed to create post with file")?; + } + + _ => { + tracing::debug!(?response, "mattermost adapter does not support this response type"); + } + } + + Ok(()) + } + + async fn send_status( + &self, + message: &InboundMessage, + status: StatusUpdate, + ) -> crate::Result<()> { + let channel_id = self.extract_channel_id(message)?; + + match status { + StatusUpdate::Thinking => { + self.start_typing(channel_id).await; + } + StatusUpdate::StopTyping => { + self.stop_typing(channel_id).await; + } + _ => {} + } + + Ok(()) + } + + async fn fetch_history( + &self, + message: &InboundMessage, + limit: usize, + ) -> crate::Result> { + let channel_id = self.extract_channel_id(message)?; + let before_post_id = message + .metadata + .get("mattermost_post_id") + .and_then(|v| v.as_str()); + + let capped_limit = limit.min(200) as u32; + let posts = self + .get_channel_posts(channel_id, before_post_id, capped_limit) + .await?; + + let bot_id = self + .bot_user_id + .get() + .map(|s| s.as_ref().to_string()); + + let mut posts_vec: Vec<_> = posts + .posts + .into_values() + .filter(|p| bot_id.as_deref() != Some(p.user_id.as_str())) + .collect(); + posts_vec.sort_by_key(|p| p.create_at); + + let history: Vec = posts_vec + .into_iter() + .map(|p| HistoryMessage { + author: p.user_id, + content: p.message, + is_bot: false, + }) + .collect(); + + Ok(history) + } + + async fn health_check(&self) -> crate::Result<()> { + let response = self + .client + .get(self.api_url("/system/ping")) + .bearer_auth(self.token.as_ref()) + .send() + .await + .context("health check request failed")?; + + if !response.status().is_success() { + return Err(anyhow::anyhow!( + "mattermost health check failed: status {}", + response.status() + ) + .into()); + } + + Ok(()) + } + + async fn shutdown(&self) -> crate::Result<()> { + if let Some(tx) = self.shutdown_tx.write().await.take() { + let _ = tx.send(()).await; + } + + if let Some(handle) = self.ws_task.write().await.take() { + handle.abort(); + } + + self.typing_tasks.write().await.clear(); + self.active_messages.write().await.clear(); + + tracing::info!(adapter = %self.runtime_key, "mattermost adapter shut down"); + Ok(()) + } + + async fn broadcast(&self, target: &str, response: OutboundResponse) -> crate::Result<()> { + match response { + OutboundResponse::Text(text) => { + for chunk in split_message(&text, MAX_MESSAGE_LENGTH) { + self.create_post(target, &chunk, None).await?; + } + } + OutboundResponse::File { + filename, + data, + mime_type, + caption, + } => { + if data.len() > self.max_attachment_bytes { + return Err(anyhow::anyhow!( + "file too large: {} bytes (max: {})", + data.len(), + self.max_attachment_bytes + ) + .into()); + } + let part = reqwest::multipart::Part::bytes(data) + .file_name(filename) + .mime_str(&mime_type) + .context("invalid mime type")?; + let form = reqwest::multipart::Form::new() + .part("files", part) + .text("channel_id", target.to_string()); + let upload: MattermostFileUpload = self + .client + .post(self.api_url("/files")) + .bearer_auth(self.token.as_ref()) + .multipart(form) + .send() + .await + .context("failed to upload file")? + .json() + .await + .context("failed to parse file upload response")?; + let file_ids: Vec<_> = + upload.file_infos.iter().map(|f| f.id.as_str()).collect(); + self.client + .post(self.api_url("/posts")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!({ + "channel_id": target, + "message": caption.unwrap_or_default(), + "file_ids": file_ids, + })) + .send() + .await + .context("failed to create post with file")?; + } + other => { + tracing::debug!(?other, "mattermost broadcast does not support this response type"); + } + } + Ok(()) + } +} + +fn build_message_from_post( + post: &MattermostPost, + runtime_key: &str, + bot_user_id: &str, + team_id: &Option, + permissions: &MattermostPermissions, +) -> Option { + if post.user_id == bot_user_id { + return None; + } + + if let Some(team_filter) = &permissions.team_filter { + // Fail-closed: no team_id in the event → can't verify team → reject. + let Some(tid) = team_id else { return None }; + if !team_filter.contains(tid) { + return None; + } + } + + if !permissions.channel_filter.is_empty() { + // Fail-closed: no team_id → can't look up allowed channels → reject. + let Some(tid) = team_id else { return None }; + if let Some(allowed_channels) = permissions.channel_filter.get(tid) { + if !allowed_channels.contains(&post.channel_id) { + return None; + } + } + } + + // "D" = direct message, "G" = group DM + let conversation_id = if post.channel_type.as_deref() == Some("D") { + apply_runtime_adapter_to_conversation_id( + runtime_key, + format!( + "mattermost:{}:dm:{}", + team_id.as_deref().unwrap_or(""), + post.user_id + ), + ) + } else { + apply_runtime_adapter_to_conversation_id( + runtime_key, + format!( + "mattermost:{}:{}", + team_id.as_deref().unwrap_or(""), + post.channel_id + ), + ) + }; + + let mut metadata = HashMap::new(); + + metadata.insert( + crate::metadata_keys::MESSAGE_ID.into(), + serde_json::json!(&post.id), + ); + + metadata.insert("mattermost_post_id".into(), serde_json::json!(&post.id)); + metadata.insert( + "mattermost_channel_id".into(), + serde_json::json!(&post.channel_id), + ); + if let Some(tid) = team_id { + metadata.insert("mattermost_team_id".into(), serde_json::json!(tid)); + } + if !post.root_id.is_empty() { + metadata.insert( + "mattermost_root_id".into(), + serde_json::json!(&post.root_id), + ); + } + + Some(InboundMessage { + id: post.id.clone(), + source: "mattermost".into(), + adapter: Some(runtime_key.to_string()), + conversation_id, + sender_id: post.user_id.clone(), + agent_id: None, + content: MessageContent::Text(post.message.clone()), + timestamp: chrono::DateTime::from_timestamp_millis(post.create_at) + .unwrap_or_else(chrono::Utc::now), + metadata, + formatted_author: None, + }) +} + +// --- API Types --- + +#[derive(Debug, Clone, Deserialize)] +struct MattermostUser { + id: String, + username: String, +} + +#[derive(Debug, Clone, Deserialize, Serialize)] +struct MattermostPost { + id: String, + create_at: i64, + #[allow(dead_code)] + update_at: i64, + user_id: String, + channel_id: String, + root_id: String, + message: String, + /// "D" = direct message, "G" = group DM, "O" = public, "P" = private. + /// Not present in REST list responses; injected from WS event data. + #[serde(default)] + channel_type: Option, + #[serde(default)] + #[allow(dead_code)] + file_ids: Vec, +} + +#[derive(Debug, Deserialize)] +struct MattermostPostList { + #[serde(default)] + #[allow(dead_code)] + order: Vec, + #[serde(default)] + posts: HashMap, +} + +#[derive(Debug, Deserialize)] +struct MattermostFileUpload { + #[serde(default)] + file_infos: Vec, +} + +#[derive(Debug, Deserialize)] +struct MattermostFileInfo { + id: String, + #[allow(dead_code)] + name: String, +} + +#[derive(Debug, Deserialize)] +struct MattermostWsEvent { + event: String, + #[serde(default)] + data: serde_json::Value, + #[serde(default)] + broadcast: MattermostWsBroadcast, +} + +#[derive(Debug, Deserialize, Default)] +struct MattermostWsBroadcast { + #[serde(default)] + #[allow(dead_code)] + channel_id: Option, + #[serde(default)] + team_id: Option, + #[serde(default)] + #[allow(dead_code)] + user_id: Option, +} + +fn split_message(text: &str, max_len: usize) -> Vec { + if text.len() <= max_len { + return vec![text.to_string()]; + } + + let mut chunks = Vec::new(); + let mut remaining = text; + + while !remaining.is_empty() { + if remaining.len() <= max_len { + chunks.push(remaining.to_string()); + break; + } + + let search_region = &remaining[..max_len]; + let break_point = search_region + .rfind('\n') + .or_else(|| search_region.rfind(' ')) + .unwrap_or(max_len); + + let end = remaining.floor_char_boundary(break_point); + chunks.push(remaining[..end].to_string()); + remaining = remaining[end..].trim_start_matches('\n').trim_start(); + } + + chunks +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_split_message_short() { + let result = split_message("hello", 100); + assert_eq!(result, vec!["hello"]); + } + + #[test] + fn test_split_message_exact_boundary() { + let text = "a".repeat(100); + let result = split_message(&text, 100); + assert_eq!(result.len(), 1); + } + + #[test] + fn test_split_message_on_newline() { + let text = "line1\nline2\nline3"; + let result = split_message(text, 8); + assert_eq!(result, vec!["line1", "line2", "line3"]); + } + + #[test] + fn test_split_message_on_space() { + let text = "word1 word2 word3"; + let result = split_message(text, 12); + assert_eq!(result, vec!["word1 word2", "word3"]); + } + + #[test] + fn test_split_message_forced_break() { + let text = "abcdefghijklmnopqrstuvwxyz"; + let result = split_message(text, 10); + assert_eq!(result, vec!["abcdefghij", "klmnopqrst", "uvwxyz"]); + } +} diff --git a/src/messaging/target.rs b/src/messaging/target.rs index 68aba7831..4b31453bb 100644 --- a/src/messaging/target.rs +++ b/src/messaging/target.rs @@ -118,6 +118,25 @@ pub fn resolve_broadcast_target(channel: &ChannelInfo) -> Option { + if let Some(channel_id) = channel + .platform_meta + .as_ref() + .and_then(|meta| meta.get("mattermost_channel_id")) + .and_then(json_value_to_string) + { + channel_id + } else { + // conversation id: mattermost:{team_id}:{channel_id} + // or mattermost:{team_id}:dm:{user_id} + let parts: Vec<&str> = channel.id.split(':').collect(); + match parts.as_slice() { + ["mattermost", _team_id, "dm", user_id] => format!("dm:{user_id}"), + ["mattermost", _team_id, channel_id] => (*channel_id).to_string(), + _ => return None, + } + } + } _ => return None, }; @@ -141,6 +160,7 @@ fn normalize_target(adapter: &str, raw_target: &str) -> Option { "telegram" => normalize_telegram_target(trimmed), "twitch" => normalize_twitch_target(trimmed), "email" => normalize_email_target(trimmed), + "mattermost" => normalize_mattermost_target(trimmed), _ => Some(trimmed.to_string()), } } @@ -213,6 +233,16 @@ fn normalize_twitch_target(raw_target: &str) -> Option { } } +fn normalize_mattermost_target(raw_target: &str) -> Option { + let target = strip_repeated_prefix(raw_target, "mattermost"); + // Accept channel IDs (opaque alphanumeric strings) and dm:user_id patterns + if target.is_empty() { + None + } else { + Some(target.to_string()) + } +} + fn normalize_email_target(raw_target: &str) -> Option { let target = strip_repeated_prefix(raw_target, "email").trim(); if target.is_empty() { From d26723ca9beded4a68db1cc8e041dac8e7934f59 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Wed, 11 Mar 2026 20:14:03 +0100 Subject: [PATCH 02/19] fix: close missing braces in mattermost instance loop in initialize_agents --- src/main.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main.rs b/src/main.rs index 87b91fb5f..259df2b03 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3247,6 +3247,9 @@ async fn initialize_agents( tracing::error!(%error, adapter = %instance.name, "failed to create named mattermost adapter"); } } + } + } + // Shared Signal permissions (hot-reloadable via file watcher) *signal_permissions = config.messaging.signal.as_ref().map(|signal_config| { let perms = spacebot::config::SignalPermissions::from_config(signal_config); From 7f6ee4a115343cc89c3ea305d58a5e558daf5504 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Wed, 11 Mar 2026 21:55:08 +0100 Subject: [PATCH 03/19] feat: add Mattermost to configuration UI - Add Mattermost platform to ChannelSettingCard, ChannelEditModal, Settings, and platformIcons - Server URL and access token credential inputs - Backend: read mattermost config into instances list in messaging_status - Backend: write base_url and token to TOML via create_messaging_instance - Mattermost instances configured manually in config.toml appear in the UI --- interface/src/api/client.ts | 2 + interface/src/components/ChannelEditModal.tsx | 2 +- .../src/components/ChannelSettingCard.tsx | 41 +++++++++- interface/src/lib/platformIcons.tsx | 3 +- interface/src/routes/Settings.tsx | 2 +- src/api/messaging.rs | 79 ++++++++++++++++++- 6 files changed, 123 insertions(+), 6 deletions(-) diff --git a/interface/src/api/client.ts b/interface/src/api/client.ts index 8cab46c2d..72ac02710 100644 --- a/interface/src/api/client.ts +++ b/interface/src/api/client.ts @@ -1205,6 +1205,8 @@ export interface CreateMessagingInstanceRequest { webhook_port?: number; webhook_bind?: string; webhook_auth_token?: string; + mattermost_base_url?: string; + mattermost_token?: string; }; } diff --git a/interface/src/components/ChannelEditModal.tsx b/interface/src/components/ChannelEditModal.tsx index e29a72251..66e3c070b 100644 --- a/interface/src/components/ChannelEditModal.tsx +++ b/interface/src/components/ChannelEditModal.tsx @@ -18,7 +18,7 @@ import { import {PlatformIcon} from "@/lib/platformIcons"; import {TagInput} from "@/components/TagInput"; -type Platform = "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook"; +type Platform = "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook" | "mattermost"; interface ChannelEditModalProps { platform: Platform; diff --git a/interface/src/components/ChannelSettingCard.tsx b/interface/src/components/ChannelSettingCard.tsx index 198babecb..f61c4b6bb 100644 --- a/interface/src/components/ChannelSettingCard.tsx +++ b/interface/src/components/ChannelSettingCard.tsx @@ -27,7 +27,7 @@ import {TagInput} from "@/components/TagInput"; import {FontAwesomeIcon} from "@fortawesome/react-fontawesome"; import {faChevronDown, faPlus} from "@fortawesome/free-solid-svg-icons"; -type Platform = "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook"; +type Platform = "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook" | "mattermost"; const PLATFORM_LABELS: Record = { discord: "Discord", @@ -36,6 +36,7 @@ const PLATFORM_LABELS: Record = { twitch: "Twitch", email: "Email", webhook: "Webhook", + mattermost: "Mattermost", }; const DOC_LINKS: Partial> = { @@ -43,6 +44,7 @@ const DOC_LINKS: Partial> = { slack: "https://docs.spacebot.sh/slack-setup", telegram: "https://docs.spacebot.sh/telegram-setup", twitch: "https://docs.spacebot.sh/twitch-setup", + mattermost: "https://docs.spacebot.sh/mattermost-setup", }; // --- Platform Catalog (Left Column) --- @@ -59,6 +61,7 @@ export function PlatformCatalog({onAddInstance}: PlatformCatalogProps) { "twitch", "email", "webhook", + "mattermost", ]; const COMING_SOON = [ @@ -636,6 +639,17 @@ export function AddInstanceCard({platform, isDefault, onCancel, onCreated}: AddI credentials.webhook_bind = credentialInputs.webhook_bind.trim(); if (credentialInputs.webhook_auth_token?.trim()) credentials.webhook_auth_token = credentialInputs.webhook_auth_token.trim(); + } else if (platform === "mattermost") { + if (!credentialInputs.mattermost_base_url?.trim()) { + setMessage({text: "Server URL is required", type: "error"}); + return; + } + if (!credentialInputs.mattermost_token?.trim()) { + setMessage({text: "Access token is required", type: "error"}); + return; + } + credentials.mattermost_base_url = credentialInputs.mattermost_base_url.trim(); + credentials.mattermost_token = credentialInputs.mattermost_token.trim(); } if (!isDefault && !instanceName.trim()) { @@ -925,6 +939,31 @@ export function AddInstanceCard({platform, isDefault, onCancel, onCreated}: AddI )} + {platform === "mattermost" && ( + <> +
+ + setCredentialInputs({...credentialInputs, mattermost_base_url: e.target.value})} + placeholder="https://mattermost.example.com" + /> +
+
+ + setCredentialInputs({...credentialInputs, mattermost_token: e.target.value})} + placeholder="Personal access token from Mattermost account settings" + onKeyDown={(e) => { if (e.key === "Enter") handleSave(); }} + /> +
+ + )} + {docLink && (

Need help?{" "} diff --git a/interface/src/lib/platformIcons.tsx b/interface/src/lib/platformIcons.tsx index a6e982375..1193986b4 100644 --- a/interface/src/lib/platformIcons.tsx +++ b/interface/src/lib/platformIcons.tsx @@ -1,6 +1,6 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { faDiscord, faSlack, faTelegram, faTwitch, faWhatsapp } from "@fortawesome/free-brands-svg-icons"; -import { faLink, faEnvelope, faComments, faComment } from "@fortawesome/free-solid-svg-icons"; +import { faLink, faEnvelope, faComments, faComment, faServer } from "@fortawesome/free-solid-svg-icons"; interface PlatformIconProps { platform: string; @@ -16,6 +16,7 @@ export function PlatformIcon({ platform, className = "text-ink-faint", size = "1 twitch: faTwitch, webhook: faLink, email: faEnvelope, + mattermost: faServer, whatsapp: faWhatsapp, matrix: faComments, imessage: faComment, diff --git a/interface/src/routes/Settings.tsx b/interface/src/routes/Settings.tsx index 9ea2df47d..0a8e60997 100644 --- a/interface/src/routes/Settings.tsx +++ b/interface/src/routes/Settings.tsx @@ -870,7 +870,7 @@ function ThemePreview({ themeId }: { themeId: ThemeId }) { ); } -type Platform = "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook"; +type Platform = "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook" | "mattermost"; function ChannelsSection() { const [expandedKey, setExpandedKey] = useState(null); diff --git a/src/api/messaging.rs b/src/api/messaging.rs index 692dea009..d65744f45 100644 --- a/src/api/messaging.rs +++ b/src/api/messaging.rs @@ -95,6 +95,11 @@ pub(super) struct InstanceCredentials { webhook_bind: Option, #[serde(default)] webhook_auth_token: Option, + // Mattermost credentials + #[serde(default)] + mattermost_base_url: Option, + #[serde(default)] + mattermost_token: Option, } #[derive(Deserialize)] @@ -510,6 +515,60 @@ pub(super) async fn messaging_status( enabled: false, }); + // Populate instances for Mattermost (not in the legacy per-platform status fields) + if let Some(mm) = doc.get("messaging").and_then(|m| m.get("mattermost")) { + let has_url = mm + .get("base_url") + .and_then(|v| v.as_str()) + .is_some_and(|s| !s.is_empty()); + let has_token = mm + .get("token") + .and_then(|v| v.as_str()) + .is_some_and(|s| !s.is_empty()); + let enabled = mm.get("enabled").and_then(|v| v.as_bool()).unwrap_or(false); + + if has_url && has_token { + push_instance_status(&mut instances, bindings, "mattermost", None, true, enabled); + } + + if let Some(named_instances) = mm + .get("instances") + .and_then(|value| value.as_array_of_tables()) + { + for instance in named_instances { + let instance_name = normalize_adapter_selector( + instance.get("name").and_then(|value| value.as_str()), + ); + let instance_enabled = instance + .get("enabled") + .and_then(|value| value.as_bool()) + .unwrap_or(true) + && enabled; + let instance_configured = instance + .get("base_url") + .and_then(|value| value.as_str()) + .is_some_and(|value| !value.is_empty()) + && instance + .get("token") + .and_then(|value| value.as_str()) + .is_some_and(|value| !value.is_empty()); + + if let Some(instance_name) = instance_name + && instance_configured + { + push_instance_status( + &mut instances, + bindings, + "mattermost", + Some(instance_name), + true, + instance_enabled, + ); + } + } + } + } + ( discord_status, slack_status, @@ -1128,7 +1187,7 @@ pub(super) async fn create_messaging_instance( if !matches!( platform.as_str(), - "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook" + "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook" | "mattermost" ) { return Ok(Json(MessagingInstanceActionResponse { success: false, @@ -1269,6 +1328,14 @@ pub(super) async fn create_messaging_instance( platform_table["auth_token"] = toml_edit::value(token.as_str()); } } + "mattermost" => { + if let Some(url) = &credentials.mattermost_base_url { + platform_table["base_url"] = toml_edit::value(url.as_str()); + } + if let Some(token) = &credentials.mattermost_token { + platform_table["token"] = toml_edit::value(token.as_str()); + } + } _ => {} } platform_table["enabled"] = toml_edit::value(enabled); @@ -1378,6 +1445,14 @@ pub(super) async fn create_messaging_instance( instance_table["auth_token"] = toml_edit::value(token.as_str()); } } + "mattermost" => { + if let Some(url) = &credentials.mattermost_base_url { + instance_table["base_url"] = toml_edit::value(url.as_str()); + } + if let Some(token) = &credentials.mattermost_token { + instance_table["token"] = toml_edit::value(token.as_str()); + } + } _ => {} } @@ -1443,7 +1518,7 @@ pub(super) async fn delete_messaging_instance( if !matches!( platform.as_str(), - "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook" + "discord" | "slack" | "telegram" | "twitch" | "email" | "webhook" | "mattermost" ) { return Ok(Json(MessagingInstanceActionResponse { success: false, From 4416b6dac5e2b5680bcd14d0b158a9e8899b759c Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Fri, 13 Mar 2026 20:14:45 +0100 Subject: [PATCH 04/19] feat: enforce dm_allowed_users filter for Mattermost DMs Empty dm_allowed_users blocks all DMs (fail-closed). Non-empty list allows only the specified Mattermost user IDs, consistent with Slack and Telegram adapter behaviour. All other layers (config, TOML schema, permissions merge from bindings, bindings API, UI TagInput) were already in place. --- src/config/types.rs | 29 +++++++ src/messaging/mattermost.rs | 167 ++++++++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+) diff --git a/src/config/types.rs b/src/config/types.rs index 9ad9c18bc..08b5a8486 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -2684,3 +2684,32 @@ impl std::fmt::Debug for MattermostInstanceConfig { .finish() } } + + +#[cfg(test)] +mod mattermost_url_tests { + use super::validate_mattermost_url; + + #[test] + fn accepts_https_url() { + assert!(validate_mattermost_url("https://mattermost.example.com").is_ok()); + } + + #[test] + fn accepts_http_url_with_warning() { + // http is allowed (with a warning), not an error + assert!(validate_mattermost_url("http://mattermost.example.com").is_ok()); + } + + #[test] + fn rejects_invalid_scheme() { + assert!(validate_mattermost_url("ftp://mattermost.example.com").is_err()); + assert!(validate_mattermost_url("ws://mattermost.example.com").is_err()); + } + + #[test] + fn rejects_unparseable_url() { + assert!(validate_mattermost_url("not a url at all").is_err()); + assert!(validate_mattermost_url("").is_err()); + } +} diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index df0329ad2..6c65a71e9 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -836,6 +836,16 @@ fn build_message_from_post( } } + // DM filter: if channel_type is "D", enforce dm_allowed_users (fail-closed) + if post.channel_type.as_deref() == Some("D") { + if permissions.dm_allowed_users.is_empty() { + return None; + } + if !permissions.dm_allowed_users.contains(&post.user_id) { + return None; + } + } + // "D" = direct message, "G" = group DM let conversation_id = if post.channel_type.as_deref() == Some("D") { apply_runtime_adapter_to_conversation_id( @@ -996,6 +1006,163 @@ fn split_message(text: &str, max_len: usize) -> Vec { mod tests { use super::*; + // --- helpers --- + + fn post(user_id: &str, channel_id: &str, channel_type: Option<&str>) -> MattermostPost { + MattermostPost { + id: "post1".into(), + create_at: 0, + update_at: 0, + user_id: user_id.into(), + channel_id: channel_id.into(), + root_id: String::new(), + message: "hello".into(), + channel_type: channel_type.map(String::from), + file_ids: vec![], + } + } + + fn no_filters() -> MattermostPermissions { + MattermostPermissions { + team_filter: None, + channel_filter: HashMap::new(), + dm_allowed_users: vec![], + } + } + + // --- build_message_from_post --- + + #[test] + fn bot_messages_are_filtered() { + let p = post("bot123", "chan1", None); + assert!(build_message_from_post(&p, "mattermost", "bot123", &None, &no_filters()).is_none()); + } + + #[test] + fn non_bot_message_passes_without_filters() { + let p = post("user1", "chan1", None); + assert!(build_message_from_post(&p, "mattermost", "bot123", &Some("team1".into()), &no_filters()).is_some()); + } + + #[test] + fn team_filter_allows_matching_team() { + let p = post("user1", "chan1", None); + let perms = MattermostPermissions { + team_filter: Some(vec!["team1".into()]), + channel_filter: HashMap::new(), + dm_allowed_users: vec![], + }; + assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &perms).is_some()); + } + + #[test] + fn team_filter_rejects_wrong_team() { + let p = post("user1", "chan1", None); + let perms = MattermostPermissions { + team_filter: Some(vec!["team1".into()]), + channel_filter: HashMap::new(), + dm_allowed_users: vec![], + }; + assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team2".into()), &perms).is_none()); + } + + #[test] + fn team_filter_fail_closed_when_team_id_absent() { + let p = post("user1", "chan1", None); + let perms = MattermostPermissions { + team_filter: Some(vec!["team1".into()]), + channel_filter: HashMap::new(), + dm_allowed_users: vec![], + }; + // No team_id in the event — must reject (fail-closed) + assert!(build_message_from_post(&p, "mattermost", "bot", &None, &perms).is_none()); + } + + #[test] + fn channel_filter_allows_matching_channel() { + let p = post("user1", "chan1", None); + let mut cf = HashMap::new(); + cf.insert("team1".into(), vec!["chan1".into()]); + let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; + assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &perms).is_some()); + } + + #[test] + fn channel_filter_rejects_unlisted_channel() { + let p = post("user1", "chan2", None); + let mut cf = HashMap::new(); + cf.insert("team1".into(), vec!["chan1".into()]); + let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; + assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &perms).is_none()); + } + + #[test] + fn channel_filter_fail_closed_when_team_id_absent() { + let p = post("user1", "chan1", None); + let mut cf = HashMap::new(); + cf.insert("team1".into(), vec!["chan1".into()]); + let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; + // No team_id → can't look up allowed channels → reject + assert!(build_message_from_post(&p, "mattermost", "bot", &None, &perms).is_none()); + } + + fn dm_perms(allowed: &[&str]) -> MattermostPermissions { + MattermostPermissions { + team_filter: None, + channel_filter: HashMap::new(), + dm_allowed_users: allowed.iter().map(|s| s.to_string()).collect(), + } + } + + #[test] + fn dm_blocked_when_dm_allowed_users_empty() { + let p = post("user1", "chan1", Some("D")); + assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &no_filters()).is_none()); + } + + #[test] + fn dm_allowed_for_listed_user() { + let p = post("user1", "chan1", Some("D")); + assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &dm_perms(&["user1"])).is_some()); + } + + #[test] + fn dm_blocked_for_unlisted_user() { + let p = post("user2", "chan1", Some("D")); + assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &dm_perms(&["user1"])).is_none()); + } + + #[test] + fn dm_filter_does_not_affect_channel_messages() { + // channel messages (type "O") pass even with empty dm_allowed_users + let p = post("user1", "chan1", Some("O")); + assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &no_filters()).is_some()); + } + + #[test] + fn dm_conversation_id_uses_user_id() { + let p = post("user1", "chan1", Some("D")); + let msg = build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &dm_perms(&["user1"])).unwrap(); + assert!(msg.conversation_id.contains(":dm:user1"), "expected DM conversation_id, got {}", msg.conversation_id); + } + + #[test] + fn channel_conversation_id_uses_channel_id() { + let p = post("user1", "chan1", Some("O")); + let msg = build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &no_filters()).unwrap(); + assert!(msg.conversation_id.contains(":chan1"), "expected channel conversation_id, got {}", msg.conversation_id); + assert!(!msg.conversation_id.contains(":dm:"), "should not be DM, got {}", msg.conversation_id); + } + + #[test] + fn message_id_metadata_is_set() { + let p = post("user1", "chan1", None); + let msg = build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &no_filters()).unwrap(); + assert!(msg.metadata.contains_key(crate::metadata_keys::MESSAGE_ID)); + } + + // --- split_message --- + #[test] fn test_split_message_short() { let result = split_message("hello", 100); From daca014231cfc66bc5bf9d80fb4ab5d5b136b5a4 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Fri, 13 Mar 2026 20:29:28 +0100 Subject: [PATCH 05/19] feat: fix Mattermost adapter gaps identified in GLM-5 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SEC2: add sanitize_reaction_name() — resolves unicode emoji to shortcodes via the emojis crate, strips colon wrappers, lowercases plain names. FN1: user identity resolution — add user_identity_cache, resolve display name from /users/{id} API on first encounter, set sender_display_name metadata and formatted_author. FN2: channel name resolution — add channel_name_cache, resolve display_name from /channels/{id} API on first encounter, set mattermost_channel_name and CHANNEL_NAME metadata. FN3: fetch_history now returns timestamps parsed from post.create_at milliseconds instead of always None. FN4: bot mention detection — check message text for @bot_username, set mattermost_mentions_or_replies_to_bot metadata flag so require_mention routing mode works correctly. Also adds 12 new unit tests covering these fixes. --- src/messaging/mattermost.rs | 272 +++++++++++++++++++++++++++++++++--- 1 file changed, 254 insertions(+), 18 deletions(-) diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index 6c65a71e9..497ab8792 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -37,6 +37,8 @@ pub struct MattermostAdapter { permissions: Arc>, bot_user_id: OnceCell>, bot_username: OnceCell>, + user_identity_cache: Arc>>, + channel_name_cache: Arc>>, active_messages: Arc>>, typing_tasks: Arc>>>, shutdown_tx: Arc>>>, @@ -91,6 +93,8 @@ impl MattermostAdapter { permissions, bot_user_id: OnceCell::new(), bot_username: OnceCell::new(), + user_identity_cache: Arc::new(RwLock::new(HashMap::new())), + channel_name_cache: Arc::new(RwLock::new(HashMap::new())), active_messages: Arc::new(RwLock::new(HashMap::new())), typing_tasks: Arc::new(RwLock::new(HashMap::new())), shutdown_tx: Arc::new(RwLock::new(None)), @@ -330,6 +334,11 @@ impl Messaging for MattermostAdapter { let token = self.token.clone(); let permissions = self.permissions.clone(); let bot_user_id = user_id; + let bot_username_ws = username; + let user_identity_cache = self.user_identity_cache.clone(); + let channel_name_cache = self.channel_name_cache.clone(); + let ws_client = self.client.clone(); + let ws_base_url = self.base_url.clone(); let inbound_tx_clone = inbound_tx.clone(); let handle = tokio::spawn(async move { @@ -389,12 +398,31 @@ impl Messaging for MattermostAdapter { let team_id = event.broadcast.team_id.clone(); let perms = permissions.load(); + + let display_name = resolve_user_display_name( + &user_identity_cache, + &ws_client, + token.as_ref(), + &ws_base_url, + &post.user_id, + ).await; + let channel_name = resolve_channel_name( + &channel_name_cache, + &ws_client, + token.as_ref(), + &ws_base_url, + &post.channel_id, + ).await; + if let Some(msg) = build_message_from_post( &post, &runtime_key, &bot_user_id, + &bot_username_ws, &team_id, &perms, + display_name.as_deref(), + channel_name.as_deref(), ) { if inbound_tx_clone.send(msg).await.is_err() { tracing::debug!("inbound channel closed"); @@ -550,7 +578,7 @@ impl Messaging for MattermostAdapter { .ok_or_else(|| { anyhow::anyhow!("missing mattermost_post_id metadata") })?; - let emoji_name = emoji.trim_matches(':'); + let emoji_name = sanitize_reaction_name(&emoji); let bot_user_id = self .bot_user_id @@ -703,7 +731,7 @@ impl Messaging for MattermostAdapter { author: p.user_id, content: p.message, is_bot: false, - timestamp: None, + timestamp: chrono::DateTime::from_timestamp_millis(p.create_at), }) .collect(); @@ -811,8 +839,11 @@ fn build_message_from_post( post: &MattermostPost, runtime_key: &str, bot_user_id: &str, + bot_username: &str, team_id: &Option, permissions: &MattermostPermissions, + display_name: Option<&str>, + channel_name: Option<&str>, ) -> Option { if post.user_id == bot_user_id { return None; @@ -889,6 +920,31 @@ fn build_message_from_post( ); } + // FN1: sender display name + if let Some(dn) = display_name { + metadata.insert("sender_display_name".into(), serde_json::json!(dn)); + } + + // FN2: channel name + if let Some(cn) = channel_name { + metadata.insert("mattermost_channel_name".into(), serde_json::json!(cn)); + metadata.insert( + crate::metadata_keys::CHANNEL_NAME.into(), + serde_json::json!(cn), + ); + } + + // FN4: bot mention detection — check for @bot_username in message text + let mentions_bot = !bot_username.is_empty() + && post.message.contains(&format!("@{bot_username}")); + metadata.insert( + "mattermost_mentions_or_replies_to_bot".into(), + serde_json::json!(mentions_bot), + ); + + // FN1: formatted_author — "Display Name" when display name is available + let formatted_author = display_name.map(|dn| dn.to_string()); + Some(InboundMessage { id: post.id.clone(), source: "mattermost".into(), @@ -900,7 +956,7 @@ fn build_message_from_post( timestamp: chrono::DateTime::from_timestamp_millis(post.create_at) .unwrap_or_else(chrono::Utc::now), metadata, - formatted_author: None, + formatted_author, }) } @@ -910,6 +966,33 @@ fn build_message_from_post( struct MattermostUser { id: String, username: String, + #[serde(default)] + first_name: String, + #[serde(default)] + last_name: String, + #[serde(default)] + nickname: String, +} + +impl MattermostUser { + fn display_name(&self) -> String { + if !self.nickname.is_empty() { + return self.nickname.clone(); + } + let full = format!("{} {}", self.first_name, self.last_name); + let full = full.trim(); + if !full.is_empty() { + return full.to_string(); + } + self.username.clone() + } +} + +#[derive(Debug, Deserialize)] +struct MattermostChannel { + #[allow(dead_code)] + id: String, + display_name: String, } #[derive(Debug, Clone, Deserialize, Serialize)] @@ -974,6 +1057,74 @@ struct MattermostWsBroadcast { user_id: Option, } +async fn resolve_user_display_name( + cache: &RwLock>, + client: &Client, + token: &str, + base_url: &Url, + user_id: &str, +) -> Option { + if let Some(name) = cache.read().await.get(user_id).cloned() { + return Some(name); + } + let mut url = base_url.clone(); + url.path_segments_mut() + .ok()? + .extend(["api", "v4", "users", user_id]); + let resp = client.get(url).bearer_auth(token).send().await.ok()?; + if !resp.status().is_success() { + return None; + } + let user: MattermostUser = resp.json().await.ok()?; + let name = user.display_name(); + cache.write().await.insert(user_id.to_string(), name.clone()); + Some(name) +} + +async fn resolve_channel_name( + cache: &RwLock>, + client: &Client, + token: &str, + base_url: &Url, + channel_id: &str, +) -> Option { + if let Some(name) = cache.read().await.get(channel_id).cloned() { + return Some(name); + } + let mut url = base_url.clone(); + url.path_segments_mut() + .ok()? + .extend(["api", "v4", "channels", channel_id]); + let resp = client.get(url).bearer_auth(token).send().await.ok()?; + if !resp.status().is_success() { + return None; + } + let channel: MattermostChannel = resp.json().await.ok()?; + let name = channel.display_name; + cache.write().await.insert(channel_id.to_string(), name.clone()); + Some(name) +} + +/// Convert an emoji input to a Mattermost reaction short-code name. +/// +/// Handles three input forms: +/// 1. Unicode emoji (e.g. "👍") → looked up via the `emojis` crate → "thumbsup" +/// 2. Colon-wrapped short-code (e.g. ":thumbsup:") → stripped to "thumbsup" +/// 3. Plain short-code (e.g. "thumbsup") → lowercased and passed through +fn sanitize_reaction_name(emoji: &str) -> String { + let trimmed = emoji.trim(); + if let Some(e) = emojis::get(trimmed) { + if let Some(shortcode) = e.shortcode() { + return shortcode.to_string(); + } + return e.name().replace(' ', "_").to_lowercase(); + } + trimmed + .trim_start_matches(':') + .trim_end_matches(':') + .to_lowercase() +} + fn split_message(text: &str, max_len: usize) -> Vec { if text.len() <= max_len { return vec![text.to_string()]; @@ -1030,18 +1181,26 @@ mod tests { } } + fn bmfp(p: &MattermostPost, bot_id: &str, team_id: Option<&str>, perms: &MattermostPermissions) -> Option { + build_message_from_post(p, "mattermost", bot_id, "botuser", &team_id.map(String::from), perms, None, None) + } + + fn bmfp_named(p: &MattermostPost, bot_id: &str, bot_username: &str, team_id: Option<&str>, perms: &MattermostPermissions, display_name: Option<&str>, channel_name: Option<&str>) -> Option { + build_message_from_post(p, "mattermost", bot_id, bot_username, &team_id.map(String::from), perms, display_name, channel_name) + } + // --- build_message_from_post --- #[test] fn bot_messages_are_filtered() { let p = post("bot123", "chan1", None); - assert!(build_message_from_post(&p, "mattermost", "bot123", &None, &no_filters()).is_none()); + assert!(bmfp(&p, "bot123", None, &no_filters()).is_none()); } #[test] fn non_bot_message_passes_without_filters() { let p = post("user1", "chan1", None); - assert!(build_message_from_post(&p, "mattermost", "bot123", &Some("team1".into()), &no_filters()).is_some()); + assert!(bmfp(&p, "bot123", Some("team1"), &no_filters()).is_some()); } #[test] @@ -1052,7 +1211,7 @@ mod tests { channel_filter: HashMap::new(), dm_allowed_users: vec![], }; - assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &perms).is_some()); + assert!(bmfp(&p, "bot", Some("team1"), &perms).is_some()); } #[test] @@ -1063,7 +1222,7 @@ mod tests { channel_filter: HashMap::new(), dm_allowed_users: vec![], }; - assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team2".into()), &perms).is_none()); + assert!(bmfp(&p, "bot", Some("team2"), &perms).is_none()); } #[test] @@ -1075,7 +1234,7 @@ mod tests { dm_allowed_users: vec![], }; // No team_id in the event — must reject (fail-closed) - assert!(build_message_from_post(&p, "mattermost", "bot", &None, &perms).is_none()); + assert!(bmfp(&p, "bot", None, &perms).is_none()); } #[test] @@ -1084,7 +1243,7 @@ mod tests { let mut cf = HashMap::new(); cf.insert("team1".into(), vec!["chan1".into()]); let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; - assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &perms).is_some()); + assert!(bmfp(&p, "bot", Some("team1"), &perms).is_some()); } #[test] @@ -1093,7 +1252,7 @@ mod tests { let mut cf = HashMap::new(); cf.insert("team1".into(), vec!["chan1".into()]); let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; - assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &perms).is_none()); + assert!(bmfp(&p, "bot", Some("team1"), &perms).is_none()); } #[test] @@ -1103,7 +1262,7 @@ mod tests { cf.insert("team1".into(), vec!["chan1".into()]); let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; // No team_id → can't look up allowed channels → reject - assert!(build_message_from_post(&p, "mattermost", "bot", &None, &perms).is_none()); + assert!(bmfp(&p, "bot", None, &perms).is_none()); } fn dm_perms(allowed: &[&str]) -> MattermostPermissions { @@ -1117,39 +1276,39 @@ mod tests { #[test] fn dm_blocked_when_dm_allowed_users_empty() { let p = post("user1", "chan1", Some("D")); - assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &no_filters()).is_none()); + assert!(bmfp(&p, "bot", Some("team1"), &no_filters()).is_none()); } #[test] fn dm_allowed_for_listed_user() { let p = post("user1", "chan1", Some("D")); - assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &dm_perms(&["user1"])).is_some()); + assert!(bmfp(&p, "bot", Some("team1"), &dm_perms(&["user1"])).is_some()); } #[test] fn dm_blocked_for_unlisted_user() { let p = post("user2", "chan1", Some("D")); - assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &dm_perms(&["user1"])).is_none()); + assert!(bmfp(&p, "bot", Some("team1"), &dm_perms(&["user1"])).is_none()); } #[test] fn dm_filter_does_not_affect_channel_messages() { // channel messages (type "O") pass even with empty dm_allowed_users let p = post("user1", "chan1", Some("O")); - assert!(build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &no_filters()).is_some()); + assert!(bmfp(&p, "bot", Some("team1"), &no_filters()).is_some()); } #[test] fn dm_conversation_id_uses_user_id() { let p = post("user1", "chan1", Some("D")); - let msg = build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &dm_perms(&["user1"])).unwrap(); + let msg = bmfp(&p, "bot", Some("team1"), &dm_perms(&["user1"])).unwrap(); assert!(msg.conversation_id.contains(":dm:user1"), "expected DM conversation_id, got {}", msg.conversation_id); } #[test] fn channel_conversation_id_uses_channel_id() { let p = post("user1", "chan1", Some("O")); - let msg = build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &no_filters()).unwrap(); + let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); assert!(msg.conversation_id.contains(":chan1"), "expected channel conversation_id, got {}", msg.conversation_id); assert!(!msg.conversation_id.contains(":dm:"), "should not be DM, got {}", msg.conversation_id); } @@ -1157,10 +1316,87 @@ mod tests { #[test] fn message_id_metadata_is_set() { let p = post("user1", "chan1", None); - let msg = build_message_from_post(&p, "mattermost", "bot", &Some("team1".into()), &no_filters()).unwrap(); + let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); assert!(msg.metadata.contains_key(crate::metadata_keys::MESSAGE_ID)); } + // --- FN4: bot mention detection --- + + #[test] + fn mention_sets_flag_when_at_bot_username_in_message() { + let mut p = post("user1", "chan1", Some("O")); + p.message = "hey @botuser can you help?".into(); + let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); + assert_eq!( + msg.metadata.get("mattermost_mentions_or_replies_to_bot").and_then(|v| v.as_bool()), + Some(true), + ); + } + + #[test] + fn no_mention_flag_when_bot_not_mentioned() { + let p = post("user1", "chan1", Some("O")); + let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); + assert_eq!( + msg.metadata.get("mattermost_mentions_or_replies_to_bot").and_then(|v| v.as_bool()), + Some(false), + ); + } + + // --- FN1: display name / sender_display_name --- + + #[test] + fn sender_display_name_set_when_provided() { + let p = post("user1", "chan1", Some("O")); + let msg = bmfp_named(&p, "bot", "botuser", Some("team1"), &no_filters(), Some("Alice"), None).unwrap(); + assert_eq!( + msg.metadata.get("sender_display_name").and_then(|v| v.as_str()), + Some("Alice"), + ); + assert_eq!(msg.formatted_author.as_deref(), Some("Alice")); + } + + #[test] + fn sender_display_name_absent_when_not_provided() { + let p = post("user1", "chan1", Some("O")); + let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); + assert!(msg.metadata.get("sender_display_name").is_none()); + assert!(msg.formatted_author.is_none()); + } + + // --- FN2: channel name --- + + #[test] + fn channel_name_metadata_set_when_provided() { + let p = post("user1", "chan1", Some("O")); + let msg = bmfp_named(&p, "bot", "botuser", Some("team1"), &no_filters(), None, Some("general")).unwrap(); + assert_eq!( + msg.metadata.get("mattermost_channel_name").and_then(|v| v.as_str()), + Some("general"), + ); + assert_eq!( + msg.metadata.get(crate::metadata_keys::CHANNEL_NAME).and_then(|v| v.as_str()), + Some("general"), + ); + } + + // --- SEC2: sanitize_reaction_name --- + + #[test] + fn sanitize_reaction_unicode_to_shortcode() { + assert_eq!(sanitize_reaction_name("\u{1F44D}"), "+1"); + } + + #[test] + fn sanitize_reaction_colon_wrapped() { + assert_eq!(sanitize_reaction_name(":thumbsup:"), "thumbsup"); + } + + #[test] + fn sanitize_reaction_plain_lowercased() { + assert_eq!(sanitize_reaction_name("ThumbsUp"), "thumbsup"); + } + // --- split_message --- #[test] From 12ff932b4436b8f7134d79b17d11aab1f109e338 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sat, 14 Mar 2026 08:37:41 +0100 Subject: [PATCH 06/19] fix: address all Mattermost PR review findings mattermost.rs: - Validate base_url has no path/query/fragment in new() - Check /users/me HTTP status before .json() to surface auth errors - start_typing() aborts previous task before spawning to prevent leaks - shutdown() drains typing_tasks with abort() instead of clear() - StreamStart now falls back to REPLY_TO_MESSAGE_ID for thread replies - StreamEnd applies MAX_MESSAGE_LENGTH chunking to avoid oversized edits - Reaction handler fails fast if bot_user_id not initialized - respond() file attachment post checks HTTP status - broadcast() file upload checks HTTP status - broadcast() resolves dm:{user_id} targets via /channels/direct API - split_message uses floor_char_boundary before slicing to prevent UTF-8 panics - Identity/channel lookups log errors via tracing::debug instead of silent None target.rs: - resolve_broadcast_target extracts named adapter key from conversation ID - normalize_mattermost_target strips team/instance prefixes down to bare channel_id or dm:{user_id} api/messaging.rs: - Validate mattermost base_url before persisting to config.toml - Delete default Mattermost instance now clears all credential fields config/load.rs: - Named Mattermost instances with missing credentials are preserved but disabled (map + enabled&&has_credentials) instead of silently dropped --- src/api/messaging.rs | 25 ++++++ src/config/load.rs | 16 ++-- src/messaging/mattermost.rs | 168 +++++++++++++++++++++++++++++++----- src/messaging/target.rs | 55 ++++++++++-- 4 files changed, 225 insertions(+), 39 deletions(-) diff --git a/src/api/messaging.rs b/src/api/messaging.rs index d65744f45..45313adda 100644 --- a/src/api/messaging.rs +++ b/src/api/messaging.rs @@ -1330,6 +1330,15 @@ pub(super) async fn create_messaging_instance( } "mattermost" => { if let Some(url) = &credentials.mattermost_base_url { + if url::Url::parse(url) + .map(|u| u.path() != "/" || u.query().is_some() || u.fragment().is_some()) + .unwrap_or(true) + { + return Ok(Json(MessagingInstanceActionResponse { + success: false, + message: format!("invalid mattermost base_url: must be an origin URL (e.g. https://mm.example.com)"), + })); + } platform_table["base_url"] = toml_edit::value(url.as_str()); } if let Some(token) = &credentials.mattermost_token { @@ -1447,6 +1456,15 @@ pub(super) async fn create_messaging_instance( } "mattermost" => { if let Some(url) = &credentials.mattermost_base_url { + if url::Url::parse(url) + .map(|u| u.path() != "/" || u.query().is_some() || u.fragment().is_some()) + .unwrap_or(true) + { + return Ok(Json(MessagingInstanceActionResponse { + success: false, + message: format!("invalid mattermost base_url: must be an origin URL (e.g. https://mm.example.com)"), + })); + } instance_table["base_url"] = toml_edit::value(url.as_str()); } if let Some(token) = &credentials.mattermost_token { @@ -1615,6 +1633,13 @@ pub(super) async fn delete_messaging_instance( table.remove("bind"); table.remove("auth_token"); } + "mattermost" => { + table.remove("base_url"); + table.remove("token"); + table.remove("team_id"); + table.remove("dm_allowed_users"); + table.remove("max_attachment_bytes"); + } _ => {} } } diff --git a/src/config/load.rs b/src/config/load.rs index 6e246c2aa..a02378ae4 100644 --- a/src/config/load.rs +++ b/src/config/load.rs @@ -2232,25 +2232,25 @@ impl Config { let instances = mm .instances .into_iter() - .filter_map(|instance| { + .map(|instance| { let token = instance.token.as_deref().and_then(resolve_env_value); let base_url = instance.base_url.as_deref().and_then(resolve_env_value); - if token.is_none() || base_url.is_none() { + let has_credentials = token.is_some() && base_url.is_some(); + if instance.enabled && !has_credentials { tracing::warn!( - name = %instance.name, - "skipping mattermost instance with missing credentials" + adapter = %instance.name, + "mattermost instance is enabled but credentials are missing/unresolvable — disabling" ); - return None; } - Some(MattermostInstanceConfig { + MattermostInstanceConfig { name: instance.name, - enabled: instance.enabled, + enabled: instance.enabled && has_credentials, base_url: base_url.unwrap_or_default(), token: token.unwrap_or_default(), team_id: instance.team_id, dm_allowed_users: instance.dm_allowed_users, max_attachment_bytes: instance.max_attachment_bytes, - }) + } }) .collect::>(); diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index 497ab8792..c0bddbd1e 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -39,6 +39,7 @@ pub struct MattermostAdapter { bot_username: OnceCell>, user_identity_cache: Arc>>, channel_name_cache: Arc>>, + dm_channel_cache: Arc>>, active_messages: Arc>>, typing_tasks: Arc>>>, shutdown_tx: Arc>>>, @@ -74,8 +75,13 @@ impl MattermostAdapter { max_attachment_bytes: usize, permissions: Arc>, ) -> anyhow::Result { - let base_url = - Url::parse(base_url).context("invalid mattermost base_url")?; + let base_url = Url::parse(base_url).context("invalid mattermost base_url")?; + if base_url.path() != "/" || base_url.query().is_some() || base_url.fragment().is_some() { + return Err(anyhow::anyhow!( + "mattermost base_url must be an origin URL without path/query/fragment (got: {})", + base_url + ).into()); + } let client = Client::builder() .timeout(HTTP_TIMEOUT) @@ -95,6 +101,7 @@ impl MattermostAdapter { bot_username: OnceCell::new(), user_identity_cache: Arc::new(RwLock::new(HashMap::new())), channel_name_cache: Arc::new(RwLock::new(HashMap::new())), + dm_channel_cache: Arc::new(RwLock::new(HashMap::new())), active_messages: Arc::new(RwLock::new(HashMap::new())), typing_tasks: Arc::new(RwLock::new(HashMap::new())), shutdown_tx: Arc::new(RwLock::new(None)), @@ -152,6 +159,7 @@ impl MattermostAdapter { } async fn start_typing(&self, channel_id: &str) { + self.stop_typing(channel_id).await; let Some(user_id) = self.bot_user_id.get().cloned() else { return; }; @@ -293,6 +301,43 @@ impl MattermostAdapter { .context("failed to parse posts response") .map_err(Into::into) } + + async fn get_or_create_dm_channel(&self, user_id: &str) -> crate::Result { + if let Some(channel_id) = self.dm_channel_cache.read().await.get(user_id).cloned() { + return Ok(channel_id); + } + let bot_user_id = self + .bot_user_id + .get() + .ok_or_else(|| anyhow::anyhow!("bot_user_id not initialized"))? + .as_ref() + .to_string(); + let response = self + .client + .post(self.api_url("/channels/direct")) + .bearer_auth(self.token.as_ref()) + .json(&serde_json::json!([bot_user_id, user_id])) + .send() + .await + .context("failed to create DM channel")?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost POST /channels/direct failed with status {}: {body}", + status.as_u16() + ).into()); + } + let channel: MattermostChannel = response + .json() + .await + .context("failed to parse DM channel response")?; + self.dm_channel_cache + .write() + .await + .insert(user_id.to_string(), channel.id.clone()); + Ok(channel.id) + } } impl Messaging for MattermostAdapter { @@ -305,13 +350,22 @@ impl Messaging for MattermostAdapter { let (shutdown_tx, mut shutdown_rx) = mpsc::channel::<()>(1); *self.shutdown_tx.write().await = Some(shutdown_tx); - let me: MattermostUser = self + let me_response = self .client .get(self.api_url("/users/me")) .bearer_auth(self.token.as_ref()) .send() .await - .context("failed to get bot user")? + .context("failed to get bot user")?; + let me_status = me_response.status(); + if !me_status.is_success() { + let body = me_response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost /users/me failed with status {}: {body}", + me_status.as_u16() + ).into()); + } + let me: MattermostUser = me_response .json() .await .context("failed to parse user response")?; @@ -519,7 +573,13 @@ impl Messaging for MattermostAdapter { let root_id = message .metadata .get("mattermost_root_id") - .and_then(|v| v.as_str()); + .and_then(|v| v.as_str()) + .or_else(|| { + message + .metadata + .get(crate::metadata_keys::REPLY_TO_MESSAGE_ID) + .and_then(|v| v.as_str()) + }); self.start_typing(channel_id).await; // Create a placeholder post with a zero-width space. let post = self.create_post(channel_id, "\u{200B}", root_id).await?; @@ -560,10 +620,19 @@ impl Messaging for MattermostAdapter { OutboundResponse::StreamEnd => { self.stop_typing(channel_id).await; if let Some(active) = self.active_messages.write().await.remove(&message.id) { - if let Err(error) = - self.edit_post(&active.post_id, &active.accumulated_text).await - { - tracing::warn!(%error, "failed to finalize streaming message"); + let chunks = split_message(&active.accumulated_text, MAX_MESSAGE_LENGTH); + let mut first = true; + for chunk in chunks { + if first { + first = false; + if let Err(error) = self.edit_post(&active.post_id, &chunk).await { + tracing::warn!(%error, "failed to finalize streaming message"); + } + } else { + if let Err(error) = self.create_post(channel_id, &chunk, None).await { + tracing::warn!(%error, "failed to create overflow chunk for streaming message"); + } + } } } } @@ -583,8 +652,9 @@ impl Messaging for MattermostAdapter { let bot_user_id = self .bot_user_id .get() - .map(|s| s.as_ref().to_string()) - .unwrap_or_default(); + .ok_or_else(|| anyhow::anyhow!("bot_user_id not initialized; call start() first"))? + .as_ref() + .to_string(); let response = self .client @@ -656,7 +726,7 @@ impl Messaging for MattermostAdapter { let file_ids: Vec<_> = upload.file_infos.iter().map(|f| f.id.as_str()).collect(); - self.client + let post_response = self.client .post(self.api_url("/posts")) .bearer_auth(self.token.as_ref()) .json(&serde_json::json!({ @@ -667,6 +737,14 @@ impl Messaging for MattermostAdapter { .send() .await .context("failed to create post with file")?; + let post_status = post_response.status(); + if !post_status.is_success() { + let body = post_response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost POST /posts (file) failed with status {}: {body}", + post_status.as_u16() + ).into()); + } } _ => { @@ -767,7 +845,9 @@ impl Messaging for MattermostAdapter { handle.abort(); } - self.typing_tasks.write().await.clear(); + for (_, handle) in self.typing_tasks.write().await.drain() { + handle.abort(); + } self.active_messages.write().await.clear(); tracing::info!(adapter = %self.runtime_key, "mattermost adapter shut down"); @@ -775,6 +855,15 @@ impl Messaging for MattermostAdapter { } async fn broadcast(&self, target: &str, response: OutboundResponse) -> crate::Result<()> { + // Resolve DM targets (dm:{user_id}) to a real Mattermost channel ID. + let resolved_target; + let target = if let Some(user_id) = target.strip_prefix("dm:") { + resolved_target = self.get_or_create_dm_channel(user_id).await?; + resolved_target.as_str() + } else { + target + }; + match response { OutboundResponse::Text(text) => { for chunk in split_message(&text, MAX_MESSAGE_LENGTH) { @@ -802,14 +891,23 @@ impl Messaging for MattermostAdapter { let form = reqwest::multipart::Form::new() .part("files", part) .text("channel_id", target.to_string()); - let upload: MattermostFileUpload = self + let upload_response = self .client .post(self.api_url("/files")) .bearer_auth(self.token.as_ref()) .multipart(form) .send() .await - .context("failed to upload file")? + .context("failed to upload file")?; + let upload_status = upload_response.status(); + if !upload_status.is_success() { + let body = upload_response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost file upload failed with status {}: {body}", + upload_status.as_u16() + ).into()); + } + let upload: MattermostFileUpload = upload_response .json() .await .context("failed to parse file upload response")?; @@ -990,7 +1088,6 @@ impl MattermostUser { #[derive(Debug, Deserialize)] struct MattermostChannel { - #[allow(dead_code)] id: String, display_name: String, } @@ -1071,11 +1168,24 @@ async fn resolve_user_display_name( url.path_segments_mut() .ok()? .extend(["api", "v4", "users", user_id]); - let resp = client.get(url).bearer_auth(token).send().await.ok()?; + let resp = match client.get(url).bearer_auth(token).send().await { + Ok(r) => r, + Err(error) => { + tracing::debug!(%error, user_id, "failed to fetch mattermost user"); + return None; + } + }; if !resp.status().is_success() { + tracing::debug!(status = %resp.status(), user_id, "mattermost user fetch returned non-success"); return None; } - let user: MattermostUser = resp.json().await.ok()?; + let user: MattermostUser = match resp.json().await { + Ok(u) => u, + Err(error) => { + tracing::debug!(%error, user_id, "failed to parse mattermost user response"); + return None; + } + }; let name = user.display_name(); cache.write().await.insert(user_id.to_string(), name.clone()); Some(name) @@ -1095,11 +1205,24 @@ async fn resolve_channel_name( url.path_segments_mut() .ok()? .extend(["api", "v4", "channels", channel_id]); - let resp = client.get(url).bearer_auth(token).send().await.ok()?; + let resp = match client.get(url).bearer_auth(token).send().await { + Ok(r) => r, + Err(error) => { + tracing::debug!(%error, channel_id, "failed to fetch mattermost channel"); + return None; + } + }; if !resp.status().is_success() { + tracing::debug!(status = %resp.status(), channel_id, "mattermost channel fetch returned non-success"); return None; } - let channel: MattermostChannel = resp.json().await.ok()?; + let channel: MattermostChannel = match resp.json().await { + Ok(c) => c, + Err(error) => { + tracing::debug!(%error, channel_id, "failed to parse mattermost channel response"); + return None; + } + }; let name = channel.display_name; cache.write().await.insert(channel_id.to_string(), name.clone()); Some(name) @@ -1139,11 +1262,12 @@ fn split_message(text: &str, max_len: usize) -> Vec { break; } - let search_region = &remaining[..max_len]; + let search_end = remaining.floor_char_boundary(max_len); + let search_region = &remaining[..search_end]; let break_point = search_region .rfind('\n') .or_else(|| search_region.rfind(' ')) - .unwrap_or(max_len); + .unwrap_or(search_end); let end = remaining.floor_char_boundary(break_point); chunks.push(remaining[..end].to_string()); diff --git a/src/messaging/target.rs b/src/messaging/target.rs index cb5fa3615..a18e93c21 100644 --- a/src/messaging/target.rs +++ b/src/messaging/target.rs @@ -141,7 +141,8 @@ pub fn resolve_broadcast_target(channel: &ChannelInfo) -> Option { - if let Some(channel_id) = channel + let adapter = extract_mattermost_adapter_from_channel_id(&channel.id); + let raw_target = if let Some(channel_id) = channel .platform_meta .as_ref() .and_then(|meta| meta.get("mattermost_channel_id")) @@ -151,13 +152,19 @@ pub fn resolve_broadcast_target(channel: &ChannelInfo) -> Option = channel.id.split(':').collect(); match parts.as_slice() { - ["mattermost", _team_id, "dm", user_id] => format!("dm:{user_id}"), - ["mattermost", _team_id, channel_id] => (*channel_id).to_string(), + [_, _team_id, "dm", user_id] => format!("dm:{user_id}"), + [_, _team_id, channel_id] => (*channel_id).to_string(), + [_, _instance, _team_id, "dm", user_id] => format!("dm:{user_id}"), + [_, _instance, _team_id, channel_id] => (*channel_id).to_string(), _ => return None, } - } + }; + let target = normalize_mattermost_target(&raw_target)?; + return Some(BroadcastTarget { adapter, target }); } _ => return None, }; @@ -258,13 +265,43 @@ fn normalize_twitch_target(raw_target: &str) -> Option { } } +fn extract_mattermost_adapter_from_channel_id(channel_id: &str) -> String { + // Named instance conv IDs: "mattermost:{instance}:{team_id}:{channel_id}" (4 parts) + // or: "mattermost:{instance}:{team_id}:dm:{user_id}" (5 parts) + // Default conv IDs: "mattermost:{team_id}:{channel_id}" (3 parts) + // or: "mattermost:{team_id}:dm:{user_id}" (4 parts, 3rd part = "dm") + let parts: Vec<&str> = channel_id.split(':').collect(); + match parts.as_slice() { + ["mattermost", instance, _, channel_or_team] if *channel_or_team != "dm" => { + format!("mattermost:{instance}") + } + ["mattermost", instance, _, "dm", _] => { + format!("mattermost:{instance}") + } + _ => "mattermost".to_string(), + } +} + fn normalize_mattermost_target(raw_target: &str) -> Option { let target = strip_repeated_prefix(raw_target, "mattermost"); - // Accept channel IDs (opaque alphanumeric strings) and dm:user_id patterns - if target.is_empty() { - None - } else { - Some(target.to_string()) + // Parse out just the channel_id or dm:{user_id}, discarding any team/instance prefix. + match target.split(':').collect::>().as_slice() { + // Already bare: "channel_id" or "dm:user_id" + [channel_id] if !channel_id.is_empty() => Some((*channel_id).to_string()), + ["dm", user_id] if !user_id.is_empty() => Some(format!("dm:{user_id}")), + // With team prefix: "team_id:channel_id" or "team_id:dm:user_id" + [_team_id, channel_id] if !channel_id.is_empty() && *channel_id != "dm" => { + Some((*channel_id).to_string()) + } + [_team_id, "dm", user_id] if !user_id.is_empty() => Some(format!("dm:{user_id}")), + // With instance+team prefix: "instance:team_id:channel_id" or "instance:team_id:dm:user_id" + [_instance, _team_id, channel_id] if !channel_id.is_empty() && *channel_id != "dm" => { + Some((*channel_id).to_string()) + } + [_instance, _team_id, "dm", user_id] if !user_id.is_empty() => { + Some(format!("dm:{user_id}")) + } + _ => None, } } From f307f25989ff5c35d5cf6913ded14f408aadaba7 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sat, 14 Mar 2026 09:42:04 +0100 Subject: [PATCH 07/19] docs: add docstrings to Mattermost adapter and target functions --- src/messaging/mattermost.rs | 97 +++++++++++++++++++++++++++++++++++++ src/messaging/target.rs | 18 +++++++ 2 files changed, 115 insertions(+) diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index c0bddbd1e..82b8e0a41 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -67,6 +67,17 @@ impl std::fmt::Debug for MattermostAdapter { } impl MattermostAdapter { + /// Create a new [`MattermostAdapter`]. + /// + /// `base_url` must be an origin URL with no path, query, or fragment + /// (e.g. `https://mm.example.com`). Returns an error if the URL is + /// malformed or includes a path component. + /// + /// `runtime_key` is the adapter's unique identifier within the messaging + /// manager (e.g. `"mattermost"` or `"mattermost:myinstance"`). + /// + /// `default_team_id` is used as a fallback when a WS event does not carry + /// a team ID in its broadcast envelope. pub fn new( runtime_key: impl Into>, base_url: &str, @@ -142,6 +153,12 @@ impl MattermostAdapter { }) } + /// Validate a Mattermost resource ID (post, channel, user, etc.). + /// + /// A valid ID is 1–64 ASCII alphanumeric characters, hyphens, or + /// underscores. Returns an error for empty strings, IDs that are too long, + /// or IDs that contain characters outside that set (e.g. colons in + /// `dm:{user_id}` composite targets). fn validate_id(id: &str) -> crate::Result<()> { if id.is_empty() || id.len() > 64 { return Err(anyhow::anyhow!("invalid mattermost ID: empty or too long").into()); @@ -152,12 +169,25 @@ impl MattermostAdapter { Ok(()) } + /// Cancel any active typing indicator for `channel_id`. + /// + /// Aborts the background loop that posts `/users/{id}/typing` and removes + /// it from the task map. No-op if no indicator is running. async fn stop_typing(&self, channel_id: &str) { if let Some(handle) = self.typing_tasks.write().await.remove(channel_id) { handle.abort(); } } + /// Start a repeating typing indicator for `channel_id`. + /// + /// Spawns a background task that posts `/users/{bot_id}/typing` every + /// [`TYPING_INDICATOR_INTERVAL`]. Any previously running indicator for the + /// same channel is aborted first to prevent task leaks. The task runs until + /// [`stop_typing`](Self::stop_typing) is called or the adapter shuts down. + /// + /// Does nothing if `bot_user_id` has not been set (i.e. [`start`](Self::start) + /// has not been called yet). async fn start_typing(&self, channel_id: &str) { self.stop_typing(channel_id).await; let Some(user_id) = self.bot_user_id.get().cloned() else { @@ -193,6 +223,14 @@ impl MattermostAdapter { .insert(channel_id.to_string(), handle); } + /// Create a new post in `channel_id` and return the created post. + /// + /// Pass `root_id` to place the post inside an existing thread; the root ID + /// must be the ID of the first post in that thread (not a reply). Pass + /// `None` to create a top-level post. + /// + /// Both `channel_id` and `root_id` (if provided) are validated with + /// [`validate_id`](Self::validate_id) before the request is sent. async fn create_post( &self, channel_id: &str, @@ -234,6 +272,11 @@ impl MattermostAdapter { .map_err(Into::into) } + /// Replace the text of an existing post in-place. + /// + /// Used by the streaming path to update the placeholder post created by + /// [`StreamStart`](crate::OutboundResponse::StreamStart) as chunks arrive, + /// and to finalize it on [`StreamEnd`](crate::OutboundResponse::StreamEnd). async fn edit_post(&self, post_id: &str, message: &str) -> crate::Result<()> { Self::validate_id(post_id)?; @@ -259,6 +302,13 @@ impl MattermostAdapter { Ok(()) } + /// Fetch up to `limit` posts from `channel_id`, sorted by creation time. + /// + /// Pass `before_post_id` to retrieve posts that appeared before a specific + /// post (exclusive), which is used by [`fetch_history`](Self::fetch_history) + /// to anchor the history window to the triggering message. The Mattermost + /// API always returns the most recent matching posts first; callers are + /// responsible for reversing the order if needed. async fn get_channel_posts( &self, channel_id: &str, @@ -302,6 +352,12 @@ impl MattermostAdapter { .map_err(Into::into) } + /// Return the Mattermost channel ID for a direct message conversation with + /// `user_id`, creating it via the API if it does not exist yet. + /// + /// Results are cached in `dm_channel_cache` so that subsequent calls for + /// the same user avoid a round-trip. Requires [`start`](Self::start) to + /// have been called so that `bot_user_id` is available. async fn get_or_create_dm_channel(&self, user_id: &str) -> crate::Result { if let Some(channel_id) = self.dm_channel_cache.read().await.get(user_id).cloned() { return Ok(channel_id); @@ -933,6 +989,25 @@ impl Messaging for MattermostAdapter { } } +/// Convert a [`MattermostPost`] from a WebSocket event into an [`InboundMessage`], +/// applying all permission filters. +/// +/// Returns `None` (message dropped) when any of the following hold: +/// - The post was authored by the bot itself. +/// - A `team_filter` is configured and the event's team ID does not match, or +/// the team ID is absent (fail-closed). +/// - A `channel_filter` is configured and the channel is not in the allow-list +/// for the event's team, or the team ID is absent (fail-closed). +/// - The channel is a direct message (`channel_type = "D"`) and either +/// `dm_allowed_users` is empty or the sender is not listed. +/// +/// When a message passes all filters the following metadata keys are set: +/// `message_id`, `mattermost_post_id`, `mattermost_channel_id`, +/// `mattermost_team_id` (if known), `mattermost_root_id` (if in a thread), +/// `sender_display_name` (if `display_name` is provided), +/// `mattermost_channel_name` and `channel_name` (if `channel_name` is provided), +/// and `mattermost_mentions_or_replies_to_bot` (true when the message text +/// contains `@{bot_username}`). fn build_message_from_post( post: &MattermostPost, runtime_key: &str, @@ -1073,6 +1148,9 @@ struct MattermostUser { } impl MattermostUser { + /// Resolve the best available display name for this user. + /// + /// Priority order: nickname → "first last" (trimmed) → username. fn display_name(&self) -> String { if !self.nickname.is_empty() { return self.nickname.clone(); @@ -1154,6 +1232,13 @@ struct MattermostWsBroadcast { user_id: Option, } +/// Look up the display name for a Mattermost user by ID. +/// +/// Returns the cached value if already resolved, otherwise calls +/// `GET /api/v4/users/{user_id}` and caches the result. The resolved name +/// follows the same priority order as [`MattermostUser::display_name`]: +/// nickname → "first last" → username. Returns `None` on any network or +/// API error (logged at `DEBUG` level). async fn resolve_user_display_name( cache: &RwLock>, client: &Client, @@ -1191,6 +1276,12 @@ async fn resolve_user_display_name( Some(name) } +/// Look up the display name for a Mattermost channel by ID. +/// +/// Returns the cached value if already resolved, otherwise calls +/// `GET /api/v4/channels/{channel_id}` and caches the result using the +/// channel's `display_name` field. Returns `None` on any network or API +/// error (logged at `DEBUG` level). async fn resolve_channel_name( cache: &RwLock>, client: &Client, @@ -1248,6 +1339,12 @@ fn sanitize_reaction_name(emoji: &str) -> String { .to_lowercase() } +/// Split `text` into chunks no longer than `max_len` bytes. +/// +/// Splits preferentially on newlines, then on spaces, and falls back to a +/// hard character-boundary break when no whitespace is found. All splits +/// respect UTF-8 character boundaries. Leading whitespace is stripped from +/// each chunk after a split. fn split_message(text: &str, max_len: usize) -> Vec { if text.len() <= max_len { return vec![text.to_string()]; diff --git a/src/messaging/target.rs b/src/messaging/target.rs index a18e93c21..8d1539532 100644 --- a/src/messaging/target.rs +++ b/src/messaging/target.rs @@ -265,6 +265,13 @@ fn normalize_twitch_target(raw_target: &str) -> Option { } } +/// Extract the runtime adapter key from a Mattermost conversation ID. +/// +/// Mattermost conversation IDs encode whether a named instance was used: +/// - Default channel: `mattermost:{team_id}:{channel_id}` (3 parts) → `"mattermost"` +/// - Default DM: `mattermost:{team_id}:dm:{user_id}` (4 parts, 3rd = `"dm"`) → `"mattermost"` +/// - Named channel: `mattermost:{instance}:{team_id}:{channel_id}` (4 parts, last ≠ `"dm"`) → `"mattermost:{instance}"` +/// - Named DM: `mattermost:{instance}:{team_id}:dm:{user_id}` (5 parts) → `"mattermost:{instance}"` fn extract_mattermost_adapter_from_channel_id(channel_id: &str) -> String { // Named instance conv IDs: "mattermost:{instance}:{team_id}:{channel_id}" (4 parts) // or: "mattermost:{instance}:{team_id}:dm:{user_id}" (5 parts) @@ -282,6 +289,17 @@ fn extract_mattermost_adapter_from_channel_id(channel_id: &str) -> String { } } +/// Normalize a raw Mattermost target string to a bare channel ID or `dm:{user_id}`. +/// +/// Accepts any of the following forms (with or without a leading `mattermost:` prefix): +/// - `channel_id` → `channel_id` +/// - `dm:{user_id}` → `dm:{user_id}` +/// - `{team_id}:{channel_id}` → `channel_id` +/// - `{team_id}:dm:{user_id}` → `dm:{user_id}` +/// - `{instance}:{team_id}:{channel_id}` → `channel_id` +/// - `{instance}:{team_id}:dm:{user_id}` → `dm:{user_id}` +/// +/// Returns `None` if the input is empty or does not match any recognised shape. fn normalize_mattermost_target(raw_target: &str) -> Option { let target = strip_repeated_prefix(raw_target, "mattermost"); // Parse out just the channel_id or dm:{user_id}, discarding any team/instance prefix. From 67c6a96cc2bd46ffb391f5c99555d19b666ea223 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sat, 14 Mar 2026 09:43:41 +0100 Subject: [PATCH 08/19] fix: wire Mattermost into channel_ids check and require_mention routing channel_ids filter at line 1543 only checked Discord/Slack/Twitch metadata, causing it to return false for Mattermost messages before the Mattermost- specific block ran. Added mattermost_channel_id to the direct_match set. require_mention routing had no mattermost arm in the mention_key switch, so all Mattermost messages were dropped when require_mention = true. Added mattermost -> mattermost_mentions_or_replies_to_bot mapping. --- src/config/types.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/config/types.rs b/src/config/types.rs index 08b5a8486..a61e35868 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -1562,11 +1562,18 @@ impl Binding { .get("twitch_channel") .and_then(|v| v.as_str()); + // Also check Mattermost channel ID + let mattermost_channel = message + .metadata + .get("mattermost_channel_id") + .and_then(|v| v.as_str()); + let direct_match = message_channel .as_ref() .is_some_and(|id| self.channel_ids.contains(id)) || slack_channel.is_some_and(|id| self.channel_ids.contains(&id.to_string())) - || twitch_channel.is_some_and(|id| self.channel_ids.contains(&id.to_string())); + || twitch_channel.is_some_and(|id| self.channel_ids.contains(&id.to_string())) + || mattermost_channel.is_some_and(|id| self.channel_ids.contains(&id.to_string())); let parent_match = parent_channel .as_ref() .is_some_and(|id| self.channel_ids.contains(id)); @@ -1653,6 +1660,7 @@ impl Binding { "slack" => "slack_mentions_or_replies_to_bot", "twitch" => "twitch_mentions_or_replies_to_bot", "telegram" => "telegram_mentions_or_replies_to_bot", + "mattermost" => "mattermost_mentions_or_replies_to_bot", // Unknown platforms: if require_mention is set, default to // requiring a mention (safe default). _ => return false, From b42929f67ae763dd7f361a1b408a5c1ea6d133c8 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sat, 14 Mar 2026 09:47:41 +0100 Subject: [PATCH 09/19] fix: address new Mattermost PR review findings target.rs: Default DM conv IDs (mattermost:team:dm:user, 4 parts) were matching the named-instance arm in extract_mattermost_adapter_from_channel_id and returning mattermost:{team_id} as the adapter. Fixed match arm ordering so the default-DM pattern is checked before the 4-part named-channel pattern. target.rs: normalize_mattermost_target accepted bare 'dm' as a valid channel ID. Added != 'dm' guard to the single-segment arm. mattermost.rs: StreamEnd overflow chunks were posted with root_id = None, escaping the original thread. Now resolves root_id from mattermost_root_id / REPLY_TO_MESSAGE_ID metadata and passes it to all overflow create_post calls. mattermost.rs: WS post JSON decode failures were silently dropped. Now logs at DEBUG level via tracing::debug so malformed payloads are visible. --- src/messaging/mattermost.rs | 23 +++++++++++++++++++++-- src/messaging/target.rs | 16 ++++++++-------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index 82b8e0a41..cb974e987 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -494,7 +494,16 @@ impl Messaging for MattermostAdapter { .data .get("post") .and_then(|v| v.as_str()) - .and_then(|s| serde_json::from_str::(s).ok()); + .map(|s| serde_json::from_str::(s)); + + let post_result = match post_result { + Some(Ok(p)) => Some(p), + Some(Err(error)) => { + tracing::debug!(%error, "failed to parse Mattermost WS post payload"); + None + } + None => None, + }; if let Some(mut post) = post_result { if post.user_id != bot_user_id.as_ref() { @@ -675,6 +684,16 @@ impl Messaging for MattermostAdapter { OutboundResponse::StreamEnd => { self.stop_typing(channel_id).await; + let root_id = message + .metadata + .get("mattermost_root_id") + .and_then(|v| v.as_str()) + .or_else(|| { + message + .metadata + .get(crate::metadata_keys::REPLY_TO_MESSAGE_ID) + .and_then(|v| v.as_str()) + }); if let Some(active) = self.active_messages.write().await.remove(&message.id) { let chunks = split_message(&active.accumulated_text, MAX_MESSAGE_LENGTH); let mut first = true; @@ -685,7 +704,7 @@ impl Messaging for MattermostAdapter { tracing::warn!(%error, "failed to finalize streaming message"); } } else { - if let Err(error) = self.create_post(channel_id, &chunk, None).await { + if let Err(error) = self.create_post(channel_id, &chunk, root_id).await { tracing::warn!(%error, "failed to create overflow chunk for streaming message"); } } diff --git a/src/messaging/target.rs b/src/messaging/target.rs index 8d1539532..6360cb070 100644 --- a/src/messaging/target.rs +++ b/src/messaging/target.rs @@ -279,12 +279,12 @@ fn extract_mattermost_adapter_from_channel_id(channel_id: &str) -> String { // or: "mattermost:{team_id}:dm:{user_id}" (4 parts, 3rd part = "dm") let parts: Vec<&str> = channel_id.split(':').collect(); match parts.as_slice() { - ["mattermost", instance, _, channel_or_team] if *channel_or_team != "dm" => { - format!("mattermost:{instance}") - } - ["mattermost", instance, _, "dm", _] => { - format!("mattermost:{instance}") - } + // Default DM: mattermost:{team_id}:dm:{user_id} — must come before the named-channel arm + ["mattermost", _, "dm", _] => "mattermost".to_string(), + // Named DM: mattermost:{instance}:{team_id}:dm:{user_id} + ["mattermost", instance, _, "dm", _] => format!("mattermost:{instance}"), + // Named channel: mattermost:{instance}:{team_id}:{channel_id} + ["mattermost", instance, _, _] => format!("mattermost:{instance}"), _ => "mattermost".to_string(), } } @@ -304,8 +304,8 @@ fn normalize_mattermost_target(raw_target: &str) -> Option { let target = strip_repeated_prefix(raw_target, "mattermost"); // Parse out just the channel_id or dm:{user_id}, discarding any team/instance prefix. match target.split(':').collect::>().as_slice() { - // Already bare: "channel_id" or "dm:user_id" - [channel_id] if !channel_id.is_empty() => Some((*channel_id).to_string()), + // Already bare: "channel_id" (but not the bare word "dm" without a user_id) + [channel_id] if !channel_id.is_empty() && *channel_id != "dm" => Some((*channel_id).to_string()), ["dm", user_id] if !user_id.is_empty() => Some(format!("dm:{user_id}")), // With team prefix: "team_id:channel_id" or "team_id:dm:user_id" [_team_id, channel_id] if !channel_id.is_empty() && *channel_id != "dm" => { From 1fd2b2bc816c1f39fa60b280ff78a370959d358d Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sat, 14 Mar 2026 09:50:16 +0100 Subject: [PATCH 10/19] fix: avoid unnecessary clone in StreamChunk throttled edit path Use Cow so the common case (text under MAX_MESSAGE_LENGTH) borrows accumulated_text directly instead of cloning it on every throttled edit. --- src/messaging/mattermost.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index cb974e987..2d9f59c00 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -665,16 +665,20 @@ impl Messaging for MattermostAdapter { active.accumulated_text.push_str(&chunk); if active.last_edit.elapsed() > STREAM_EDIT_THROTTLE { - let display_text = if active.accumulated_text.len() > MAX_MESSAGE_LENGTH { - let end = active - .accumulated_text - .floor_char_boundary(MAX_MESSAGE_LENGTH - 3); - format!("{}...", &active.accumulated_text[..end]) - } else { - active.accumulated_text.clone() - }; - - if let Err(error) = self.edit_post(&active.post_id, &display_text).await { + let display_text: std::borrow::Cow<'_, str> = + if active.accumulated_text.len() > MAX_MESSAGE_LENGTH { + let end = active + .accumulated_text + .floor_char_boundary(MAX_MESSAGE_LENGTH - 3); + std::borrow::Cow::Owned(format!( + "{}...", + &active.accumulated_text[..end] + )) + } else { + std::borrow::Cow::Borrowed(active.accumulated_text.as_str()) + }; + + if let Err(error) = self.edit_post(&active.post_id, display_text.as_ref()).await { tracing::warn!(%error, "failed to edit streaming message"); } active.last_edit = Instant::now(); From abc77b2847e7ad9755d83a4de1c059d614dd0d5b Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sat, 14 Mar 2026 22:59:36 +0100 Subject: [PATCH 11/19] fix: address coderabbitai PR review findings in Mattermost adapter - validate_mattermost_url: fail-closed for non-localhost http (was warn-only) - MattermostConfig: add SystemSecrets impl and register in system_secret_registry - StreamChunk: release active_messages write lock before awaiting edit_post - channel_filter: fail-closed when team has no allowlist entry (was fail-open) - WS shutdown: log close frame send failure instead of discarding with let _ = Co-Authored-By: Claude Sonnet 4.6 --- src/config/types.rs | 58 ++++++++++++++++++++++++++++++++++--- src/messaging/mattermost.rs | 52 +++++++++++++++++++-------------- src/secrets/store.rs | 5 ++-- 3 files changed, 87 insertions(+), 28 deletions(-) diff --git a/src/config/types.rs b/src/config/types.rs index a61e35868..8235a1519 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -1919,7 +1919,22 @@ fn validate_mattermost_url(url: &str) -> Result<()> { match parsed.scheme() { "https" => {} "http" => { - tracing::warn!(url, "mattermost base_url uses http instead of https"); + let is_local = match parsed.host() { + Some(url::Host::Domain(h)) => h.eq_ignore_ascii_case("localhost"), + Some(url::Host::Ipv4(addr)) => addr == std::net::Ipv4Addr::LOCALHOST, + Some(url::Host::Ipv6(addr)) => addr == std::net::Ipv6Addr::LOCALHOST, + None => false, + }; + if !is_local { + return Err(ConfigError::Invalid( + "mattermost base_url must use https for non-localhost hosts".to_string(), + ) + .into()); + } + tracing::warn!( + host = parsed.host_str().unwrap_or(""), + "mattermost base_url uses http for localhost" + ); } scheme => { return Err(ConfigError::Invalid(format!( @@ -2679,6 +2694,31 @@ pub struct MattermostInstanceConfig { pub max_attachment_bytes: usize, } +impl SystemSecrets for MattermostConfig { + fn section() -> &'static str { + "mattermost" + } + + fn is_messaging_adapter() -> bool { + true + } + + fn secret_fields() -> &'static [SecretField] { + &[ + SecretField { + toml_key: "token", + secret_name: "MATTERMOST_TOKEN", + instance_pattern: None, + }, + SecretField { + toml_key: "base_url", + secret_name: "MATTERMOST_BASE_URL", + instance_pattern: None, + }, + ] + } +} + impl std::fmt::Debug for MattermostInstanceConfig { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("MattermostInstanceConfig") @@ -2704,9 +2744,18 @@ mod mattermost_url_tests { } #[test] - fn accepts_http_url_with_warning() { - // http is allowed (with a warning), not an error - assert!(validate_mattermost_url("http://mattermost.example.com").is_ok()); + fn accepts_http_localhost_with_warning() { + // http is allowed only for localhost (with a warning) + assert!(validate_mattermost_url("http://localhost:8065").is_ok()); + assert!(validate_mattermost_url("http://127.0.0.1:8065").is_ok()); + assert!(validate_mattermost_url("http://[::1]:8065").is_ok()); + } + + #[test] + fn rejects_http_non_localhost() { + // http is rejected for non-local hosts + assert!(validate_mattermost_url("http://mattermost.example.com").is_err()); + assert!(validate_mattermost_url("http://10.0.0.1").is_err()); } #[test] @@ -2721,3 +2770,4 @@ mod mattermost_url_tests { assert!(validate_mattermost_url("").is_err()); } } + diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index 2d9f59c00..8eefdacf7 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -480,7 +480,9 @@ impl Messaging for MattermostAdapter { tokio::select! { _ = shutdown_rx.recv() => { tracing::info!(adapter = %runtime_key, "mattermost websocket shutting down"); - let _ = write.send(WsMessage::Close(None)).await; + if let Err(error) = write.send(WsMessage::Close(None)).await { + tracing::debug!(%error, "failed to send websocket close frame"); + } return; } @@ -660,28 +662,33 @@ impl Messaging for MattermostAdapter { } OutboundResponse::StreamChunk(chunk) => { - let mut active_messages = self.active_messages.write().await; - if let Some(active) = active_messages.get_mut(&message.id) { - active.accumulated_text.push_str(&chunk); - - if active.last_edit.elapsed() > STREAM_EDIT_THROTTLE { - let display_text: std::borrow::Cow<'_, str> = - if active.accumulated_text.len() > MAX_MESSAGE_LENGTH { + let pending_edit = { + let mut active_messages = self.active_messages.write().await; + if let Some(active) = active_messages.get_mut(&message.id) { + active.accumulated_text.push_str(&chunk); + + if active.last_edit.elapsed() > STREAM_EDIT_THROTTLE { + let display_text = if active.accumulated_text.len() > MAX_MESSAGE_LENGTH + { let end = active .accumulated_text .floor_char_boundary(MAX_MESSAGE_LENGTH - 3); - std::borrow::Cow::Owned(format!( - "{}...", - &active.accumulated_text[..end] - )) + format!("{}...", &active.accumulated_text[..end]) } else { - std::borrow::Cow::Borrowed(active.accumulated_text.as_str()) + active.accumulated_text.clone() }; - - if let Err(error) = self.edit_post(&active.post_id, display_text.as_ref()).await { - tracing::warn!(%error, "failed to edit streaming message"); + active.last_edit = Instant::now(); + Some((active.post_id.clone(), display_text)) + } else { + None } - active.last_edit = Instant::now(); + } else { + None + } + }; + if let Some((post_id, display_text)) = pending_edit { + if let Err(error) = self.edit_post(&post_id, &display_text).await { + tracing::warn!(%error, "failed to edit streaming message"); } } } @@ -1054,12 +1061,13 @@ fn build_message_from_post( } if !permissions.channel_filter.is_empty() { - // Fail-closed: no team_id → can't look up allowed channels → reject. + // Fail-closed: no team_id or no allowlist entry for this team → reject. let Some(tid) = team_id else { return None }; - if let Some(allowed_channels) = permissions.channel_filter.get(tid) { - if !allowed_channels.contains(&post.channel_id) { - return None; - } + let Some(allowed_channels) = permissions.channel_filter.get(tid) else { + return None; + }; + if !allowed_channels.contains(&post.channel_id) { + return None; } } diff --git a/src/secrets/store.rs b/src/secrets/store.rs index 3c4bcc19a..0e9ca9218 100644 --- a/src/secrets/store.rs +++ b/src/secrets/store.rs @@ -1101,8 +1101,8 @@ pub trait SystemSecrets { /// here. pub fn system_secret_registry() -> Vec<&'static SecretField> { use crate::config::{ - DefaultsConfig, DiscordConfig, EmailConfig, LlmConfig, SignalConfig, SlackConfig, - TelegramConfig, TwitchConfig, + DefaultsConfig, DiscordConfig, EmailConfig, LlmConfig, MattermostConfig, SignalConfig, + SlackConfig, TelegramConfig, TwitchConfig, }; let mut fields = Vec::new(); @@ -1117,6 +1117,7 @@ pub fn system_secret_registry() -> Vec<&'static SecretField> { fields.extend(TwitchConfig::secret_fields()); fields.extend(EmailConfig::secret_fields()); fields.extend(SignalConfig::secret_fields()); + fields.extend(MattermostConfig::secret_fields()); fields } From 0d98cfb14f49b93fb7b4d6d8a86196987191760f Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sun, 15 Mar 2026 08:23:03 +0100 Subject: [PATCH 12/19] fix: address remaining coderabbitai Mattermost review findings - channel_filter: add test for fail-closed behavior when team is missing from filter - WS event loop: log outer MattermostWsEvent parse failures (was silently dropped) - broadcast: check HTTP status on /posts with file_ids (was unchecked) Co-Authored-By: Claude Sonnet 4.6 --- src/messaging/mattermost.rs | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index 8eefdacf7..6581e0f92 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -489,8 +489,12 @@ impl Messaging for MattermostAdapter { msg = read.next() => { match msg { Some(Ok(WsMessage::Text(text))) => { - if let Ok(event) = serde_json::from_str::(&text) { - if event.event == "posted" { + match serde_json::from_str::(&text) { + Err(error) => { + tracing::debug!(%error, text = text.as_str(), "failed to parse Mattermost WS event envelope"); + } + Ok(event) => { + if event.event == "posted" { // The post is double-encoded as a JSON string in the data field. let post_result = event .data @@ -553,7 +557,8 @@ impl Messaging for MattermostAdapter { } } } - } + } // close Ok(event) arm + } // close match MattermostWsEvent } Some(Ok(WsMessage::Ping(data))) => { if write.send(WsMessage::Pong(data)).await.is_err() { @@ -999,7 +1004,7 @@ impl Messaging for MattermostAdapter { .context("failed to parse file upload response")?; let file_ids: Vec<_> = upload.file_infos.iter().map(|f| f.id.as_str()).collect(); - self.client + let post_response = self.client .post(self.api_url("/posts")) .bearer_auth(self.token.as_ref()) .json(&serde_json::json!({ @@ -1010,6 +1015,14 @@ impl Messaging for MattermostAdapter { .send() .await .context("failed to create post with file")?; + let post_status = post_response.status(); + if !post_status.is_success() { + let body = post_response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "mattermost create post with file failed with status {}: {body}", + post_status.as_u16() + ).into()); + } } other => { tracing::debug!(?other, "mattermost broadcast does not support this response type"); @@ -1517,6 +1530,18 @@ mod tests { assert!(bmfp(&p, "bot", None, &perms).is_none()); } + #[test] + fn channel_filter_fail_closed_when_team_not_in_filter() { + // channel_filter has an entry for team1 but the message comes from team2. + // Even though chan1 is the allowed channel, the missing team2 entry must + // reject (fail-closed), not silently pass through. + let p = post("user1", "chan1", None); + let mut cf = HashMap::new(); + cf.insert("team1".into(), vec!["chan1".into()]); + let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; + assert!(bmfp(&p, "bot", Some("team2"), &perms).is_none()); + } + fn dm_perms(allowed: &[&str]) -> MattermostPermissions { MattermostPermissions { team_filter: None, From 56414161c290bfd9df213a089591b16612c1fb51 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sun, 15 Mar 2026 17:17:09 +0100 Subject: [PATCH 13/19] fix: address duplicate coderabbitai findings in Mattermost adapter - SystemSecrets: add InstancePattern to token and base_url fields so MATTERMOST_{INSTANCE}_TOKEN/BASE_URL are recognized as system secrets - start(): log warning when bot_user_id/bot_username OnceCell is already set instead of silently discarding the error - shutdown(): use tx.send().await.ok() pattern for intentional channel-send discard Co-Authored-By: Claude Sonnet 4.6 --- src/config/types.rs | 10 ++++++++-- src/messaging/mattermost.rs | 10 +++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/config/types.rs b/src/config/types.rs index 8235a1519..1d44c0a13 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -2708,12 +2708,18 @@ impl SystemSecrets for MattermostConfig { SecretField { toml_key: "token", secret_name: "MATTERMOST_TOKEN", - instance_pattern: None, + instance_pattern: Some(InstancePattern { + platform_prefix: "MATTERMOST", + field_suffix: "TOKEN", + }), }, SecretField { toml_key: "base_url", secret_name: "MATTERMOST_BASE_URL", - instance_pattern: None, + instance_pattern: Some(InstancePattern { + platform_prefix: "MATTERMOST", + field_suffix: "BASE_URL", + }), }, ] } diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index 6581e0f92..fa39e0cb3 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -429,8 +429,12 @@ impl Messaging for MattermostAdapter { let user_id: Arc = me.id.clone().into(); let username: Arc = me.username.clone().into(); - self.bot_user_id.set(user_id.clone()).ok(); - self.bot_username.set(username.clone()).ok(); + if self.bot_user_id.set(user_id.clone()).is_err() { + tracing::warn!(adapter = %self.runtime_key, "bot_user_id already initialized — start() called more than once"); + } + if self.bot_username.set(username.clone()).is_err() { + tracing::warn!(adapter = %self.runtime_key, "bot_username already initialized — start() called more than once"); + } tracing::info!( adapter = %self.runtime_key, @@ -929,7 +933,7 @@ impl Messaging for MattermostAdapter { async fn shutdown(&self) -> crate::Result<()> { if let Some(tx) = self.shutdown_tx.write().await.take() { - let _ = tx.send(()).await; + tx.send(()).await.ok(); } if let Some(handle) = self.ws_task.write().await.take() { From 4403e47383961c03c3fe7c50b4415680c532db19 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sun, 15 Mar 2026 19:07:13 +0100 Subject: [PATCH 14/19] fix: address new Mattermost PR review findings - validate_mattermost_url: reject URLs with credentials, non-root path, query string, or fragment (accept origin-only URLs only) - start()/WS task: apply default_team_id fallback when WS event omits team_id - matches_route: remove duplicate Mattermost-only channel_ids filter block (mattermost_channel_id is already covered by direct_match) Co-Authored-By: Claude Sonnet 4.6 --- src/config/types.rs | 38 ++++++++++++++++++++++++++----------- src/messaging/mattermost.rs | 4 +++- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/config/types.rs b/src/config/types.rs index 1d44c0a13..036c8d179 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -1607,17 +1607,6 @@ impl Binding { } } } - // Mattermost channel filter - if !self.channel_ids.is_empty() && self.channel == "mattermost" { - let message_channel = message - .metadata - .get("mattermost_channel_id") - .and_then(|v| v.as_str()); - if !self.channel_ids.iter().any(|id| Some(id.as_str()) == message_channel) { - return false; - } - } - true } @@ -1916,6 +1905,33 @@ pub(super) fn build_adapter_validation_states( fn validate_mattermost_url(url: &str) -> Result<()> { let parsed = url::Url::parse(url) .map_err(|e| ConfigError::Invalid(format!("invalid mattermost base_url '{url}': {e}")))?; + + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err(ConfigError::Invalid( + "mattermost base_url must not contain credentials".to_string(), + ) + .into()); + } + let path = parsed.path(); + if !path.is_empty() && path != "/" { + return Err(ConfigError::Invalid(format!( + "mattermost base_url must be an origin URL (no path), got path: {path}" + )) + .into()); + } + if parsed.query().is_some() { + return Err(ConfigError::Invalid( + "mattermost base_url must not contain a query string".to_string(), + ) + .into()); + } + if parsed.fragment().is_some() { + return Err(ConfigError::Invalid( + "mattermost base_url must not contain a fragment".to_string(), + ) + .into()); + } + match parsed.scheme() { "https" => {} "http" => { diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index fa39e0cb3..cb83951e6 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -454,6 +454,7 @@ impl Messaging for MattermostAdapter { let ws_client = self.client.clone(); let ws_base_url = self.base_url.clone(); let inbound_tx_clone = inbound_tx.clone(); + let default_team_id = self.default_team_id.clone(); let handle = tokio::spawn(async move { let mut retry_delay = WS_RECONNECT_BASE_DELAY; @@ -525,7 +526,8 @@ impl Messaging for MattermostAdapter { .map(String::from); post.channel_type = channel_type; - let team_id = event.broadcast.team_id.clone(); + let team_id = event.broadcast.team_id.clone() + .or_else(|| default_team_id.as_ref().map(|s| s.to_string())); let perms = permissions.load(); let display_name = resolve_user_display_name( From 43aa84c340299327bdfb15e975882244c7e9f3b2 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sun, 15 Mar 2026 21:10:58 +0100 Subject: [PATCH 15/19] fix: address new Mattermost PR review findings - bindings API: add team_id to BindingResponse, CreateBindingRequest, and UpdateBindingRequest so Mattermost team scoping is readable/writable via API - validate_mattermost_url: reject URLs with credentials, non-root path, query string, or fragment; add tests covering all new rejection branches - broadcast File: include root_id in /posts payload so file replies stay in thread - StreamStart: stop typing task before propagating create_post error to prevent leak - config/types.rs: rename |i| closure var to |instance| for readability - mattermost tests: rename bmfp/bmfp_named helpers and p param to descriptive names Co-Authored-By: Claude Sonnet 4.6 --- src/api/bindings.rs | 25 ++++++++++++++ src/config/types.rs | 30 +++++++++++++++- src/messaging/mattermost.rs | 69 +++++++++++++++++++++++-------------- 3 files changed, 97 insertions(+), 27 deletions(-) diff --git a/src/api/bindings.rs b/src/api/bindings.rs index 42446bdaf..438e1be71 100644 --- a/src/api/bindings.rs +++ b/src/api/bindings.rs @@ -14,6 +14,7 @@ pub(super) struct BindingResponse { guild_id: Option, workspace_id: Option, chat_id: Option, + team_id: Option, channel_ids: Vec, require_mention: bool, dm_allowed_users: Vec, @@ -43,6 +44,8 @@ pub(super) struct CreateBindingRequest { #[serde(default)] chat_id: Option, #[serde(default)] + team_id: Option, + #[serde(default)] channel_ids: Vec, #[serde(default)] require_mention: bool, @@ -135,6 +138,8 @@ pub(super) struct UpdateBindingRequest { original_workspace_id: Option, #[serde(default)] original_chat_id: Option, + #[serde(default)] + original_team_id: Option, agent_id: String, channel: String, @@ -147,6 +152,8 @@ pub(super) struct UpdateBindingRequest { #[serde(default)] chat_id: Option, #[serde(default)] + team_id: Option, + #[serde(default)] channel_ids: Vec, #[serde(default)] require_mention: bool, @@ -185,6 +192,7 @@ pub(super) async fn list_bindings( guild_id: b.guild_id, workspace_id: b.workspace_id, chat_id: b.chat_id, + team_id: b.team_id, channel_ids: b.channel_ids, require_mention: b.require_mention, dm_allowed_users: b.dm_allowed_users, @@ -433,6 +441,9 @@ pub(super) async fn create_binding( if let Some(chat_id) = &request.chat_id { binding_table["chat_id"] = toml_edit::value(chat_id.as_str()); } + if let Some(team_id) = &request.team_id { + binding_table["team_id"] = toml_edit::value(team_id.as_str()); + } if !request.channel_ids.is_empty() { let mut arr = toml_edit::Array::new(); for id in &request.channel_ids { @@ -720,12 +731,20 @@ pub(super) async fn update_binding( .is_some_and(|v| v == cid), None => table.get("chat_id").is_none(), }; + let matches_team = match &request.original_team_id { + Some(tid) => table + .get("team_id") + .and_then(|v| v.as_str()) + .is_some_and(|v| v == tid), + None => table.get("team_id").is_none(), + }; if matches_agent && matches_channel && matches_adapter && matches_guild && matches_workspace && matches_chat + && matches_team { match_idx = Some(i); break; @@ -750,6 +769,7 @@ pub(super) async fn update_binding( binding.remove("guild_id"); binding.remove("workspace_id"); binding.remove("chat_id"); + binding.remove("team_id"); if let Some(adapter) = request .adapter @@ -775,6 +795,11 @@ pub(super) async fn update_binding( { binding["chat_id"] = toml_edit::value(chat_id); } + if let Some(ref team_id) = request.team_id + && !team_id.is_empty() + { + binding["team_id"] = toml_edit::value(team_id); + } if !request.channel_ids.is_empty() { let mut arr = toml_edit::Array::new(); diff --git a/src/config/types.rs b/src/config/types.rs index 036c8d179..77e3de637 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -1877,7 +1877,7 @@ pub(super) fn build_adapter_validation_states( if let Some(mattermost) = &messaging.mattermost { let named_instances = validate_instance_names( "mattermost", - mattermost.instances.iter().map(|i| i.name.as_str()), + mattermost.instances.iter().map(|instance| instance.name.as_str()), )?; let default_present = !mattermost.base_url.trim().is_empty() && !mattermost.token.trim().is_empty(); @@ -2791,5 +2791,33 @@ mod mattermost_url_tests { assert!(validate_mattermost_url("not a url at all").is_err()); assert!(validate_mattermost_url("").is_err()); } + + #[test] + fn rejects_credentials_in_url() { + assert!(validate_mattermost_url("https://user:pass@mattermost.example.com").is_err()); + assert!(validate_mattermost_url("https://user@mattermost.example.com").is_err()); + } + + #[test] + fn rejects_non_root_path() { + assert!(validate_mattermost_url("https://mattermost.example.com/some/path").is_err()); + assert!(validate_mattermost_url("https://mattermost.example.com/mattermost").is_err()); + } + + #[test] + fn accepts_root_path() { + assert!(validate_mattermost_url("https://mattermost.example.com/").is_ok()); + assert!(validate_mattermost_url("https://mattermost.example.com").is_ok()); + } + + #[test] + fn rejects_query_string() { + assert!(validate_mattermost_url("https://mattermost.example.com/?token=abc").is_err()); + } + + #[test] + fn rejects_fragment() { + assert!(validate_mattermost_url("https://mattermost.example.com/#section").is_err()); + } } diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index cb83951e6..0637a77d4 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -660,7 +660,13 @@ impl Messaging for MattermostAdapter { }); self.start_typing(channel_id).await; // Create a placeholder post with a zero-width space. - let post = self.create_post(channel_id, "\u{200B}", root_id).await?; + let post = match self.create_post(channel_id, "\u{200B}", root_id).await { + Ok(p) => p, + Err(error) => { + self.stop_typing(channel_id).await; + return Err(error); + } + }; self.active_messages.write().await.insert( message.id.clone(), ActiveStream { @@ -823,6 +829,16 @@ impl Messaging for MattermostAdapter { let file_ids: Vec<_> = upload.file_infos.iter().map(|f| f.id.as_str()).collect(); + let root_id = message + .metadata + .get("mattermost_root_id") + .and_then(|v| v.as_str()) + .or_else(|| { + message + .metadata + .get(crate::metadata_keys::REPLY_TO_MESSAGE_ID) + .and_then(|v| v.as_str()) + }); let post_response = self.client .post(self.api_url("/posts")) .bearer_auth(self.token.as_ref()) @@ -830,6 +846,7 @@ impl Messaging for MattermostAdapter { "channel_id": channel_id, "message": caption.unwrap_or_default(), "file_ids": file_ids, + "root_id": root_id.unwrap_or(""), })) .send() .await @@ -1452,12 +1469,12 @@ mod tests { } } - fn bmfp(p: &MattermostPost, bot_id: &str, team_id: Option<&str>, perms: &MattermostPermissions) -> Option { - build_message_from_post(p, "mattermost", bot_id, "botuser", &team_id.map(String::from), perms, None, None) + fn build_message_from_mattermost_post(post: &MattermostPost, bot_id: &str, team_id: Option<&str>, perms: &MattermostPermissions) -> Option { + build_message_from_post(post, "mattermost", bot_id, "botuser", &team_id.map(String::from), perms, None, None) } - fn bmfp_named(p: &MattermostPost, bot_id: &str, bot_username: &str, team_id: Option<&str>, perms: &MattermostPermissions, display_name: Option<&str>, channel_name: Option<&str>) -> Option { - build_message_from_post(p, "mattermost", bot_id, bot_username, &team_id.map(String::from), perms, display_name, channel_name) + fn build_message_from_mattermost_post_named(post: &MattermostPost, bot_id: &str, bot_username: &str, team_id: Option<&str>, perms: &MattermostPermissions, display_name: Option<&str>, channel_name: Option<&str>) -> Option { + build_message_from_post(post, "mattermost", bot_id, bot_username, &team_id.map(String::from), perms, display_name, channel_name) } // --- build_message_from_post --- @@ -1465,13 +1482,13 @@ mod tests { #[test] fn bot_messages_are_filtered() { let p = post("bot123", "chan1", None); - assert!(bmfp(&p, "bot123", None, &no_filters()).is_none()); + assert!(build_message_from_mattermost_post(&p, "bot123", None, &no_filters()).is_none()); } #[test] fn non_bot_message_passes_without_filters() { let p = post("user1", "chan1", None); - assert!(bmfp(&p, "bot123", Some("team1"), &no_filters()).is_some()); + assert!(build_message_from_mattermost_post(&p, "bot123", Some("team1"), &no_filters()).is_some()); } #[test] @@ -1482,7 +1499,7 @@ mod tests { channel_filter: HashMap::new(), dm_allowed_users: vec![], }; - assert!(bmfp(&p, "bot", Some("team1"), &perms).is_some()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team1"), &perms).is_some()); } #[test] @@ -1493,7 +1510,7 @@ mod tests { channel_filter: HashMap::new(), dm_allowed_users: vec![], }; - assert!(bmfp(&p, "bot", Some("team2"), &perms).is_none()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team2"), &perms).is_none()); } #[test] @@ -1505,7 +1522,7 @@ mod tests { dm_allowed_users: vec![], }; // No team_id in the event — must reject (fail-closed) - assert!(bmfp(&p, "bot", None, &perms).is_none()); + assert!(build_message_from_mattermost_post(&p, "bot", None, &perms).is_none()); } #[test] @@ -1514,7 +1531,7 @@ mod tests { let mut cf = HashMap::new(); cf.insert("team1".into(), vec!["chan1".into()]); let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; - assert!(bmfp(&p, "bot", Some("team1"), &perms).is_some()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team1"), &perms).is_some()); } #[test] @@ -1523,7 +1540,7 @@ mod tests { let mut cf = HashMap::new(); cf.insert("team1".into(), vec!["chan1".into()]); let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; - assert!(bmfp(&p, "bot", Some("team1"), &perms).is_none()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team1"), &perms).is_none()); } #[test] @@ -1533,7 +1550,7 @@ mod tests { cf.insert("team1".into(), vec!["chan1".into()]); let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; // No team_id → can't look up allowed channels → reject - assert!(bmfp(&p, "bot", None, &perms).is_none()); + assert!(build_message_from_mattermost_post(&p, "bot", None, &perms).is_none()); } #[test] @@ -1545,7 +1562,7 @@ mod tests { let mut cf = HashMap::new(); cf.insert("team1".into(), vec!["chan1".into()]); let perms = MattermostPermissions { team_filter: None, channel_filter: cf, dm_allowed_users: vec![] }; - assert!(bmfp(&p, "bot", Some("team2"), &perms).is_none()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team2"), &perms).is_none()); } fn dm_perms(allowed: &[&str]) -> MattermostPermissions { @@ -1559,39 +1576,39 @@ mod tests { #[test] fn dm_blocked_when_dm_allowed_users_empty() { let p = post("user1", "chan1", Some("D")); - assert!(bmfp(&p, "bot", Some("team1"), &no_filters()).is_none()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team1"), &no_filters()).is_none()); } #[test] fn dm_allowed_for_listed_user() { let p = post("user1", "chan1", Some("D")); - assert!(bmfp(&p, "bot", Some("team1"), &dm_perms(&["user1"])).is_some()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team1"), &dm_perms(&["user1"])).is_some()); } #[test] fn dm_blocked_for_unlisted_user() { let p = post("user2", "chan1", Some("D")); - assert!(bmfp(&p, "bot", Some("team1"), &dm_perms(&["user1"])).is_none()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team1"), &dm_perms(&["user1"])).is_none()); } #[test] fn dm_filter_does_not_affect_channel_messages() { // channel messages (type "O") pass even with empty dm_allowed_users let p = post("user1", "chan1", Some("O")); - assert!(bmfp(&p, "bot", Some("team1"), &no_filters()).is_some()); + assert!(build_message_from_mattermost_post(&p, "bot", Some("team1"), &no_filters()).is_some()); } #[test] fn dm_conversation_id_uses_user_id() { let p = post("user1", "chan1", Some("D")); - let msg = bmfp(&p, "bot", Some("team1"), &dm_perms(&["user1"])).unwrap(); + let msg = build_message_from_mattermost_post(&p, "bot", Some("team1"), &dm_perms(&["user1"])).unwrap(); assert!(msg.conversation_id.contains(":dm:user1"), "expected DM conversation_id, got {}", msg.conversation_id); } #[test] fn channel_conversation_id_uses_channel_id() { let p = post("user1", "chan1", Some("O")); - let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); + let msg = build_message_from_mattermost_post(&p, "bot", Some("team1"), &no_filters()).unwrap(); assert!(msg.conversation_id.contains(":chan1"), "expected channel conversation_id, got {}", msg.conversation_id); assert!(!msg.conversation_id.contains(":dm:"), "should not be DM, got {}", msg.conversation_id); } @@ -1599,7 +1616,7 @@ mod tests { #[test] fn message_id_metadata_is_set() { let p = post("user1", "chan1", None); - let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); + let msg = build_message_from_mattermost_post(&p, "bot", Some("team1"), &no_filters()).unwrap(); assert!(msg.metadata.contains_key(crate::metadata_keys::MESSAGE_ID)); } @@ -1609,7 +1626,7 @@ mod tests { fn mention_sets_flag_when_at_bot_username_in_message() { let mut p = post("user1", "chan1", Some("O")); p.message = "hey @botuser can you help?".into(); - let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); + let msg = build_message_from_mattermost_post(&p, "bot", Some("team1"), &no_filters()).unwrap(); assert_eq!( msg.metadata.get("mattermost_mentions_or_replies_to_bot").and_then(|v| v.as_bool()), Some(true), @@ -1619,7 +1636,7 @@ mod tests { #[test] fn no_mention_flag_when_bot_not_mentioned() { let p = post("user1", "chan1", Some("O")); - let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); + let msg = build_message_from_mattermost_post(&p, "bot", Some("team1"), &no_filters()).unwrap(); assert_eq!( msg.metadata.get("mattermost_mentions_or_replies_to_bot").and_then(|v| v.as_bool()), Some(false), @@ -1631,7 +1648,7 @@ mod tests { #[test] fn sender_display_name_set_when_provided() { let p = post("user1", "chan1", Some("O")); - let msg = bmfp_named(&p, "bot", "botuser", Some("team1"), &no_filters(), Some("Alice"), None).unwrap(); + let msg = build_message_from_mattermost_post_named(&p, "bot", "botuser", Some("team1"), &no_filters(), Some("Alice"), None).unwrap(); assert_eq!( msg.metadata.get("sender_display_name").and_then(|v| v.as_str()), Some("Alice"), @@ -1642,7 +1659,7 @@ mod tests { #[test] fn sender_display_name_absent_when_not_provided() { let p = post("user1", "chan1", Some("O")); - let msg = bmfp(&p, "bot", Some("team1"), &no_filters()).unwrap(); + let msg = build_message_from_mattermost_post(&p, "bot", Some("team1"), &no_filters()).unwrap(); assert!(msg.metadata.get("sender_display_name").is_none()); assert!(msg.formatted_author.is_none()); } @@ -1652,7 +1669,7 @@ mod tests { #[test] fn channel_name_metadata_set_when_provided() { let p = post("user1", "chan1", Some("O")); - let msg = bmfp_named(&p, "bot", "botuser", Some("team1"), &no_filters(), None, Some("general")).unwrap(); + let msg = build_message_from_mattermost_post_named(&p, "bot", "botuser", Some("team1"), &no_filters(), None, Some("general")).unwrap(); assert_eq!( msg.metadata.get("mattermost_channel_name").and_then(|v| v.as_str()), Some("general"), From 8db2feb6f6ee817aab140b3f7c65a37867cd0b62 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sun, 15 Mar 2026 21:15:32 +0100 Subject: [PATCH 16/19] chore(docs): rewrite mattermost.md as user-facing documentation Replace implementation plan with concise feature list, setup guide, named instance configuration, and notes on what is missing compared to Slack/Discord. Co-Authored-By: Claude Sonnet 4.6 --- docs/mattermost.md | 1785 ++------------------------------------------ 1 file changed, 78 insertions(+), 1707 deletions(-) diff --git a/docs/mattermost.md b/docs/mattermost.md index 64f334c38..88bf0e1f0 100644 --- a/docs/mattermost.md +++ b/docs/mattermost.md @@ -1,1747 +1,118 @@ -# Mattermost Integration Plan +# Mattermost -## Executive Summary +Spacebot connects to Mattermost via a bot account using the Mattermost REST API and WebSocket event stream. The integration can be configured in the web UI or via `config.toml`. -Add Mattermost messaging platform support to Spacebot following the existing adapter architecture. Implementation will use a custom HTTP + WebSocket client (not the `mattermost_api` crate) to maintain consistency with existing patterns and avoid limitations. +## Features -## Library Assessment: mattermost_api +### Supported +- Receive messages from channels and direct messages +- Send text replies (long messages are automatically split into multiple posts) +- Streaming replies with live edit-in-place updates and typing indicator +- Thread-aware replies (replies stay in the originating thread) +- File/image attachments (up to a configurable size limit) +- Emoji reactions +- Fetch channel history (used for context window) +- Multiple named instances (connect to more than one Mattermost server) +- Per-team and per-channel allowlists +- DM allowlist (fail-closed: DMs are blocked unless the sender is explicitly listed) +- `require_mention` routing (only respond when the bot is @-mentioned) -The [mattermost_api](https://docs.rs/mattermost_api/latest/mattermost_api/) crate (v0.8.2) was evaluated and **rejected** for the following reasons: +### Not supported (compared to Slack/Discord) +- Slash commands — Mattermost slash commands are not handled; use @-mentions instead +- Ephemeral (private) messages +- Message threading via `parent_id` lookup — threads are followed when the inbound message carries a root ID, but the bot cannot independently look up thread context +- User/channel autocomplete +- Presence or status events +- App marketplace / interactive components (buttons, modals) -### Critical Gaps +## Setup -| Feature | Required | Crate Support | -|---------|----------|---------------| -| Edit post (streaming) | Yes | No | -| Channel history fetch | Yes | No | -| Typing indicator | Yes | No | -| Websocket disconnect | Yes | No (loops forever) | -| File upload | Medium | No | -| Reactions | Low | No | +### 1. Create a bot account -### Style Conflicts +In Mattermost: **System Console → Integrations → Bot Accounts → Add Bot Account** -- Uses `#[async_trait]` macro - violates AGENTS.md guideline to use native RPITIT -- `WebsocketHandler` trait requires async_trait -- No control over websocket lifecycle +- Give it a username (e.g. `spacebot`) +- Copy the generated access token -### Verdict +The bot must be added to any team and channel it should respond in. Bot accounts in Mattermost are not automatically visible in channels. -Build a minimal custom client using `reqwest` (HTTP) + `tokio-tungstenite` (WebSocket). This matches the pattern used by other adapters and gives us full control over the implementation. +### 2. Configure in config.toml ---- - -## Adversarial Review - -### Security Issues - -| Issue | Severity | Mitigation | -|-------|----------|------------| -| Token stored in plain String | High | Use `DecryptedSecret` wrapper (see src/secrets/store.rs) | -| URL injection in api_url() | Medium | Validate base_url at config load time, use `url::Url` | -| Unvalidated post_id in edit_post | Medium | Validate alphanumeric format before interpolation | -| WebSocket URL derived from user input | Medium | Parse and validate URL, require https/wss in production | -| No HTTP response body limit | Medium | Set max response size on reqwest client | -| File upload size unbounded | Medium | Enforce max_attachment_bytes from config | - -### Concurrency Bugs - -| Issue | Severity | Mitigation | -|-------|----------|------------| -| Race between start() and respond() | High | Use `OnceCell` for bot_user_id, fail requests until initialized | -| StreamChunk lost if channel_id not in active_messages | Medium | Return error, let caller handle | -| WebSocket spawn detached, no join handle | High | Store `JoinHandle` in adapter, await in shutdown | -| typing_tasks abort without cleanup | Low | Acceptable - abort is correct behavior | -| split read/write without mutex | Medium | Use `futures::stream::SplitSink`/`SplitStream` pattern correctly | - -### Maintainability Issues - -| Issue | Severity | Mitigation | -|-------|----------|------------| -| Missing Debug impl on MattermostConfig | Medium | Add `#[derive(Debug)]` with token redaction | -| Missing Debug impl on MattermostAdapter | Medium | Implement Debug manually with redaction | -| Inline JSON builders | Low | Extract to typed request structs | -| "Full implementation omitted" in plan | High | Complete the InboundMessage building code | -| Magic numbers (500ms throttle) | Low | Extract to constants with comments | -| No reconnection logic | High | Implement exponential backoff reconnect | -| No rate limit handling | Medium | Parse 429 responses, implement backoff | - -### Idiomatic Rust Issues - -| Issue | Severity | Mitigation | -|-------|----------|------------| -| `impl Into` for new() params | Low | Use `impl Into>` or just `String` per style guide | -| Clone on large structs | Medium | Use `Arc` for strings, avoid cloning | -| `Vec` allocations in split_message | Low | Return `Cow<'_, str>` iterator or accept writer | -| Missing `#[non_exhaustive]` on config structs | Low | Add for future-proofing | -| Using `anyhow::Context` directly | Medium | Use crate's `Error` type for consistency | - ---- - -## Architecture - -### Message Flow - -``` -┌─────────────────────────────────────────────────────────────┐ -│ Mattermost Instance │ -│ https://mattermost.example.com/api/v4/* │ -└─────────────────┬───────────────────────────┬───────────────┘ - │ │ - │ HTTP (REST API) │ WebSocket - │ │ - ▼ ▼ - ┌────────────────┐ ┌────────────────┐ - │ reqwest │ │ tokio- │ - │ client │ │ tungstenite │ - └───────┬────────┘ └───────┬────────┘ - │ │ - └───────────┬───────────────┘ - │ - ▼ - ┌────────────────────┐ - │ MattermostAdapter │ - │ (Messaging trait) │ - └─────────┬──────────┘ - │ - ▼ - ┌────────────────────┐ - │ MessagingManager │ - │ (fan-in) │ - └────────────────────┘ -``` - -### Conversation ID Format - -``` -mattermost:{team_id}:{channel_id} # Public/private channel -mattermost:{team_id}:dm:{user_id} # Direct message -mattermost:instance_name:{team_id}:{id} # Named adapter instance -``` - ---- - -## Implementation - -### 1. Configuration (src/config.rs) - -#### Add to MessagingConfig - -```rust -pub struct MessagingConfig { - // ... existing fields - pub mattermost: Option, -} -``` - -#### New Config Structs - -```rust -#[derive(Clone)] -pub struct MattermostConfig { - pub enabled: bool, - pub base_url: String, - pub token: String, - pub team_id: Option, - pub instances: Vec, - pub dm_allowed_users: Vec, - pub max_attachment_bytes: usize, -} - -impl std::fmt::Debug for MattermostConfig { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("MattermostConfig") - .field("enabled", &self.enabled) - .field("base_url", &self.base_url) - .field("token", &"[REDACTED]") - .field("team_id", &self.team_id) - .field("instances", &self.instances) - .field("dm_allowed_users", &self.dm_allowed_users) - .field("max_attachment_bytes", &self.max_attachment_bytes) - .finish() - } -} - -#[derive(Clone)] -pub struct MattermostInstanceConfig { - pub name: String, - pub enabled: bool, - pub base_url: String, - pub token: String, - pub team_id: Option, - pub dm_allowed_users: Vec, - pub max_attachment_bytes: usize, -} - -impl std::fmt::Debug for MattermostInstanceConfig { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("MattermostInstanceConfig") - .field("name", &self.name) - .field("enabled", &self.enabled) - .field("base_url", &self.base_url) - .field("token", &"[REDACTED]") - .field("team_id", &self.team_id) - .field("dm_allowed_users", &self.dm_allowed_users) - .field("max_attachment_bytes", &self.max_attachment_bytes) - .finish() - } -} - -#[derive(Debug, Clone, Default)] -pub struct MattermostPermissions { - pub team_filter: Option>, - pub channel_filter: HashMap>, - pub dm_allowed_users: Vec, -} +```toml +[messaging.mattermost] +enabled = true +base_url = "https://mattermost.example.com" # origin only, no path +token = "your-bot-access-token" +team_id = "team_id_here" # optional: default team for events without one +max_attachment_bytes = 52428800 # optional: default 50 MB +dm_allowed_users = [] # optional: user IDs allowed to DM the bot ``` -#### TOML Deserialization Types - -```rust -#[derive(Deserialize)] -struct TomlMattermostConfig { - #[serde(default)] - enabled: bool, - base_url: Option, - token: Option, - team_id: Option, - #[serde(default)] - instances: Vec, - #[serde(default)] - dm_allowed_users: Vec, - #[serde(default = "default_max_attachment_bytes")] - max_attachment_bytes: usize, -} - -fn default_max_attachment_bytes() -> usize { - 10 * 1024 * 1024 // 10 MB -} +The token can also be supplied via the `MATTERMOST_TOKEN` environment variable and `base_url` via `MATTERMOST_BASE_URL`. -#[derive(Deserialize)] -struct TomlMattermostInstanceConfig { - name: String, - #[serde(default)] - enabled: bool, - base_url: Option, - token: Option, - team_id: Option, - #[serde(default)] - dm_allowed_users: Vec, - #[serde(default = "default_max_attachment_bytes")] - max_attachment_bytes: usize, -} -``` - -#### URL Validation at Config Load +> **Security**: `base_url` must use `https` for non-localhost hosts. Plain `http` is only accepted for `localhost` / `127.0.0.1` / `::1`. -```rust -fn validate_mattermost_url(url: &str) -> Result<()> { - let parsed = url::Url::parse(url) - .map_err(|e| ConfigError::Invalid(format!("invalid mattermost base_url: {e}")))?; - - match parsed.scheme() { - "https" => Ok(()), - "http" => { - tracing::warn!("mattermost base_url uses http instead of https"); - Ok(()) - } - scheme => Err(ConfigError::Invalid(format!( - "mattermost base_url must be http or https, got: {scheme}" - )).into()), - } -} -``` +### 3. Wire up a binding -#### Update is_named_adapter_platform +Bindings connect Mattermost channels to agents: -```rust -fn is_named_adapter_platform(platform: &str) -> bool { - matches!( - platform, - "discord" | "slack" | "telegram" | "twitch" | "email" | "mattermost" - ) -} +```toml +[[bindings]] +agent_id = "my-agent" +channel = "mattermost" +channel_ids = ["channel_id_here"] # leave empty to match all channels +require_mention = false ``` -#### Add to build_adapter_validation_states +To scope a binding to a specific Mattermost team, add `team_id`: -```rust -if let Some(mattermost) = &messaging.mattermost { - let named_instances = validate_instance_names( - "mattermost", - mattermost.instances.iter().map(|i| i.name.as_str()), - )?; - let default_present = !mattermost.base_url.trim().is_empty() - && !mattermost.token.trim().is_empty(); - validate_runtime_keys("mattermost", default_present, &named_instances)?; - - if default_present { - validate_mattermost_url(&mattermost.base_url)?; - } - for instance in &mattermost.instances { - if instance.enabled && !instance.base_url.is_empty() { - validate_mattermost_url(&instance.base_url)?; - } - } - - states.insert( - "mattermost", - AdapterValidationState { - default_present, - named_instances, - }, - ); -} +```toml +[[bindings]] +agent_id = "my-agent" +channel = "mattermost" +team_id = "team_id_here" +channel_ids = ["channel_id_here"] ``` -### 2. Config TOML Schema +## Multiple servers (named instances) ```toml -[messaging.mattermost] +[[messaging.mattermost.instances]] +name = "corp" enabled = true -base_url = "https://mattermost.example.com" -token = "your-bot-personal-access-token" -team_id = "team-id-optional-default" -dm_allowed_users = ["user-id-1", "user-id-2"] -max_attachment_bytes = 10485760 # 10 MB default +base_url = "https://mattermost.corp.example.com" +token = "corp-bot-token" +team_id = "corp_team_id" [[messaging.mattermost.instances]] -name = "internal" +name = "community" enabled = true -base_url = "https://internal.company.com" -token = "env:MATTERMOST_INTERNAL_TOKEN" -team_id = "internal-team-id" -max_attachment_bytes = 5242880 # 5 MB for this instance - -[[bindings]] -agent_id = "default" -channel = "mattermost" -team_id = "team-id-here" -channel_ids = ["channel-id-1", "channel-id-2"] -dm_allowed_users = ["user-id-1"] -``` - -### 3. Adapter Implementation (src/messaging/mattermost.rs) - -```rust -//! Mattermost messaging adapter using custom HTTP + WebSocket client. - -use crate::config::MattermostPermissions; -use crate::messaging::apply_runtime_adapter_to_conversation_id; -use crate::messaging::traits::{HistoryMessage, InboundStream, Messaging}; -use crate::{InboundMessage, MessageContent, OutboundResponse, StatusUpdate}; -use crate::error::{AdapterError, Result}; - -use anyhow::Context as _; -use arc_swap::ArcSwap; -use futures_util::{SinkExt, StreamExt}; -use reqwest::Client; -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; -use std::sync::Arc; -use std::time::{Duration, Instant}; -use tokio::sync::{mpsc, OnceCell, RwLock}; -use tokio_tungstenite::{ - connect_async, - tungstenite::{Error as WsError, Message as WsMessage}, - MaybeTlsStream, WebSocketStream, -}; - -const MAX_MESSAGE_LENGTH: usize = 16_383; -const STREAM_EDIT_THROTTLE: Duration = Duration::from_millis(500); -const TYPING_INDICATOR_INTERVAL: Duration = Duration::from_secs(5); -const WS_RECONNECT_BASE_DELAY: Duration = Duration::from_secs(1); -const WS_RECONNECT_MAX_DELAY: Duration = Duration::from_secs(60); -const HTTP_TIMEOUT: Duration = Duration::from_secs(30); - -pub struct MattermostAdapter { - runtime_key: Arc, - base_url: url::Url, - token: Arc, - default_team_id: Option>, - max_attachment_bytes: usize, - client: Client, - permissions: Arc>, - bot_user_id: OnceCell>, - bot_username: OnceCell>, - active_messages: Arc>>, - typing_tasks: Arc>>>, - shutdown_tx: Arc>>>, - ws_task: Arc>>>, -} - -struct ActiveStream { - post_id: Arc, - channel_id: Arc, - last_edit: Instant, - accumulated_text: String, -} - -impl std::fmt::Debug for MattermostAdapter { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("MattermostAdapter") - .field("runtime_key", &self.runtime_key) - .field("base_url", &self.base_url) - .field("token", &"[REDACTED]") - .field("default_team_id", &self.default_team_id) - .field("max_attachment_bytes", &self.max_attachment_bytes) - .finish() - } -} - -impl MattermostAdapter { - pub fn new( - runtime_key: impl Into>, - base_url: &str, - token: impl Into>, - default_team_id: Option>, - max_attachment_bytes: usize, - permissions: Arc>, - ) -> Result { - let base_url = url::Url::parse(base_url) - .map_err(|e| AdapterError::ConfigInvalid(format!("invalid base_url: {e}")))?; - - let client = Client::builder() - .timeout(HTTP_TIMEOUT) - .pool_idle_timeout(Duration::from_secs(30)) - .build() - .context("failed to build HTTP client")?; - - Ok(Self { - runtime_key: runtime_key.into(), - base_url, - token: token.into(), - default_team_id, - max_attachment_bytes, - client, - permissions, - bot_user_id: OnceCell::new(), - bot_username: OnceCell::new(), - active_messages: Arc::new(RwLock::new(HashMap::new())), - typing_tasks: Arc::new(RwLock::new(HashMap::new())), - shutdown_tx: Arc::new(RwLock::new(None)), - ws_task: Arc::new(RwLock::new(None)), - }) - } - - fn api_url(&self, path: &str) -> url::Url { - let mut url = self.base_url.clone(); - url.path_segments_mut() - .expect("base_url is valid") - .extend(&["api", "v4"]) - .extend(path.trim_start_matches('/').split('/')); - url - } - - fn ws_url(&self) -> url::Url { - let mut url = self.base_url.clone(); - url.set_scheme(match self.base_url.scheme() { - "https" => "wss", - "http" => "ws", - other => other, - }).expect("scheme is valid"); - url.path_segments_mut() - .expect("base_url is valid") - .extend(&["api", "v4", "websocket"]); - url - } - - fn extract_channel_id<'a>(&self, message: &'a InboundMessage) -> Result<&'a str> { - message - .metadata - .get("mattermost_channel_id") - .and_then(|v| v.as_str()) - .ok_or_else(|| AdapterError::MissingMetadata("mattermost_channel_id".into()).into()) - } - - fn extract_post_id<'a>(&self, message: &'a InboundMessage) -> Result<&'a str> { - message - .metadata - .get("mattermost_post_id") - .and_then(|v| v.as_str()) - .ok_or_else(|| AdapterError::MissingMetadata("mattermost_post_id".into()).into()) - } - - fn validate_id(id: &str) -> Result<()> { - if id.is_empty() || id.len() > 64 { - return Err(AdapterError::InvalidId(id.into()).into()); - } - if !id.chars().all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') { - return Err(AdapterError::InvalidId(id.into()).into()); - } - Ok(()) - } - - async fn stop_typing(&self, channel_id: &str) { - if let Some(handle) = self.typing_tasks.write().await.remove(channel_id) { - handle.abort(); - } - } - - async fn start_typing(&self, channel_id: &str) { - // Typing indicators expire after ~5 seconds. Spawn a task that re-sends on - // TYPING_INDICATOR_INTERVAL so the indicator remains visible during long operations. - // The task is stored in typing_tasks so stop_typing can abort it. - let Some(user_id) = self.bot_user_id.get().cloned() else { return }; - let channel_id = channel_id.to_string(); - let client = self.client.clone(); - let token = self.token.clone(); - let url = self.api_url(&format!("/users/{user_id}/typing")); - - let handle = tokio::spawn(async move { - let mut interval = tokio::time::interval(TYPING_INDICATOR_INTERVAL); - loop { - interval.tick().await; - let result = client - .post(url.clone()) - .bearer_auth(token.as_ref()) - .json(&serde_json::json!({ - "channel_id": channel_id, - "parent_id": "", - })) - .send() - .await; - if let Err(error) = result { - tracing::warn!(%error, "typing indicator request failed"); - } - } - }); - - self.typing_tasks - .write() - .await - .insert(channel_id.to_string(), handle); - } - - async fn create_post( - &self, - channel_id: &str, - message: &str, - root_id: Option<&str>, - ) -> Result { - Self::validate_id(channel_id)?; - if let Some(rid) = root_id { - Self::validate_id(rid)?; - } - - let response = self.client - .post(self.api_url("/posts")) - .bearer_auth(self.token.as_ref()) - .json(&serde_json::json!({ - "channel_id": channel_id, - "message": message, - "root_id": root_id.unwrap_or(""), - })) - .send() - .await - .context("failed to create post")?; - - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(AdapterError::ApiError { - endpoint: "/posts".into(), - status: status.as_u16(), - message: body, - }.into()); - } - - response - .json() - .await - .context("failed to parse post response") - .map_err(Into::into) - } - - async fn edit_post(&self, post_id: &str, message: &str) -> Result<()> { - Self::validate_id(post_id)?; - - let response = self.client - .put(self.api_url(&format!("/posts/{post_id}"))) - .bearer_auth(self.token.as_ref()) - .json(&serde_json::json!({ "message": message })) - .send() - .await - .context("failed to edit post")?; - - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(AdapterError::ApiError { - endpoint: format!("/posts/{post_id}"), - status: status.as_u16(), - message: body, - }.into()); - } - - Ok(()) - } - - async fn get_channel_posts( - &self, - channel_id: &str, - before_post_id: Option<&str>, - limit: u32, - ) -> Result { - Self::validate_id(channel_id)?; - - let mut url = self.api_url(&format!("/channels/{channel_id}/posts")); - { - let mut query = url.query_pairs_mut(); - query.append_pair("page", "0"); - query.append_pair("per_page", &limit.to_string()); - if let Some(before) = before_post_id { - query.append_pair("before", before); - } - } - - let response = self.client - .get(url) - .bearer_auth(self.token.as_ref()) - .send() - .await - .context("failed to fetch channel posts")?; - - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(AdapterError::ApiError { - endpoint: format!("/channels/{channel_id}/posts"), - status: status.as_u16(), - message: body, - }.into()); - } - - response - .json() - .await - .context("failed to parse posts response") - .map_err(Into::into) - } - - fn build_inbound_message( - &self, - post: MattermostPost, - team_id: Option, - ) -> Option { - let bot_id = self.bot_user_id.get()?; - if &post.user_id == bot_id.as_ref() { - return None; - } - - let permissions = self.permissions.load(); - - if let Some(team_filter) = &permissions.team_filter { - if let Some(ref tid) = team_id { - if !team_filter.contains(tid) { - return None; - } - } - } - - if let Some(channel_filter) = permissions.channel_filter.get(&post.channel_id) { - if !channel_filter.contains(&post.channel_id) { - return None; - } - } - - // DM channel type is carried in the WebSocket event's `channel_type` field. - // All Mattermost channel IDs start with lowercase alphanumeric, so channel_id prefix - // is NOT a reliable DM indicator. Use the channel_type from the event data instead, - // passed in as an optional parameter. "D" = direct message, "G" = group message. - let conversation_id = if post.channel_type.as_deref() == Some("D") { - apply_runtime_adapter_to_conversation_id( - self.runtime_key.as_ref(), - format!("mattermost:{}:dm:{}", team_id.as_deref().unwrap_or(""), post.user_id), - ) - } else { - apply_runtime_adapter_to_conversation_id( - self.runtime_key.as_ref(), - format!("mattermost:{}:{}", team_id.as_deref().unwrap_or(""), post.channel_id), - ) - }; - - let mut metadata = HashMap::new(); - metadata.insert("mattermost_post_id".into(), serde_json::json!(post.id)); - metadata.insert("mattermost_channel_id".into(), serde_json::json!(post.channel_id)); - if let Some(tid) = team_id { - metadata.insert("mattermost_team_id".into(), serde_json::json!(tid)); - } - if !post.root_id.is_empty() { - metadata.insert("mattermost_root_id".into(), serde_json::json!(post.root_id)); - } - metadata.insert( - "mattermost_create_at".into(), - serde_json::json!(post.create_at), - ); - - Some(InboundMessage { - id: post.id.clone(), - source: "mattermost".into(), - adapter: Some(self.runtime_key.to_string()), - conversation_id, - sender_id: post.user_id.clone(), - agent_id: None, - content: MessageContent::Text(post.message), - timestamp: chrono::DateTime::from_timestamp_millis(post.create_at) - .unwrap_or_else(chrono::Utc::now), - metadata, - formatted_author: None, - }) - } -} - -impl Messaging for MattermostAdapter { - fn name(&self) -> &str { - &self.runtime_key - } - - async fn start(&self) -> Result { - let (inbound_tx, inbound_rx) = mpsc::channel(256); - let (shutdown_tx, mut shutdown_rx) = mpsc::channel::<()>(1); - *self.shutdown_tx.write().await = Some(shutdown_tx); - - let me: MattermostUser = self.client - .get(self.api_url("/users/me")) - .bearer_auth(self.token.as_ref()) - .send() - .await - .context("failed to get bot user")? - .json() - .await - .context("failed to parse user response")?; - - let user_id: Arc = me.id.clone().into(); - let username: Arc = me.username.clone().into(); - - self.bot_user_id.set(user_id.clone()).expect("set once"); - self.bot_username.set(username.clone()).expect("set once"); - - tracing::info!( - adapter = %self.runtime_key, - bot_id = %user_id, - bot_username = %username, - "mattermost connected" - ); - - let ws_url = self.ws_url(); - let runtime_key = self.runtime_key.clone(); - let token = self.token.clone(); - let permissions = self.permissions.clone(); - let bot_user_id = user_id; - let inbound_tx_clone = inbound_tx.clone(); - - let handle = tokio::spawn(async move { - let mut retry_delay = WS_RECONNECT_BASE_DELAY; - - loop { - let connect_result = connect_async(ws_url.as_str()).await; - - match connect_result { - Ok((ws_stream, _)) => { - retry_delay = WS_RECONNECT_BASE_DELAY; - - let (mut write, mut read) = ws_stream.split(); - - let auth_msg = serde_json::json!({ - "seq": 1, - "action": "authentication_challenge", - "data": {"token": token.as_ref()} - }); - - if let Ok(msg) = serde_json::to_string(&auth_msg) { - if write.send(WsMessage::Text(msg)).await.is_err() { - tracing::error!("failed to send websocket auth"); - continue; - } - } - - loop { - tokio::select! { - _ = shutdown_rx.recv() => { - tracing::info!(adapter = %runtime_key, "mattermost websocket shutting down"); - let _ = write.send(WsMessage::Close(None)).await; - return; - } - - msg = read.next() => { - match msg { - Some(Ok(WsMessage::Text(text))) => { - if let Ok(event) = serde_json::from_str::(&text) { - if event.event == "posted" { - if let Some(post_data) = event.data.get("post") { - if let Ok(post) = serde_json::from_value::(post_data.clone()) { - if post.user_id.as_str() != bot_user_id.as_ref() { - let team_id = event - .broadcast - .team_id - .clone(); - - // Load permissions each message so hot-reload takes effect. - let perms = permissions.load(); - if let Some(msg) = build_message_from_post( - &post, - &runtime_key, - &bot_user_id, - &team_id, - &perms, - ) { - if inbound_tx_clone.send(msg).await.is_err() { - tracing::debug!("inbound channel closed"); - return; - } - } - } - } - } - } - } - } - Some(Ok(WsMessage::Ping(data))) => { - if write.send(WsMessage::Pong(data)).await.is_err() { - tracing::warn!("failed to send pong"); - break; - } - } - Some(Ok(WsMessage::Pong(_))) => {} - Some(Ok(WsMessage::Close(_))) => { - tracing::info!(adapter = %runtime_key, "websocket closed by server"); - break; - } - Some(Err(e)) => { - tracing::error!(adapter = %runtime_key, error = %e, "websocket error"); - break; - } - None => break, - _ => {} - } - } - } - } - - tracing::info!(adapter = %runtime_key, "websocket disconnected, reconnecting..."); - } - Err(e) => { - tracing::error!( - adapter = %runtime_key, - error = %e, - delay_ms = retry_delay.as_millis(), - "websocket connection failed, retrying" - ); - } - } - - tokio::select! { - _ = tokio::time::sleep(retry_delay) => { - retry_delay = (retry_delay * 2).min(WS_RECONNECT_MAX_DELAY); - } - _ = shutdown_rx.recv() => { - tracing::info!(adapter = %runtime_key, "mattermost adapter shutting down during reconnect delay"); - return; - } - } - } - }); - - *self.ws_task.write().await = Some(handle); - - let stream = tokio_stream::wrappers::ReceiverStream::new(inbound_rx); - Ok(Box::pin(stream)) - } - - async fn respond( - &self, - message: &InboundMessage, - response: OutboundResponse, - ) -> Result<()> { - let channel_id = self.extract_channel_id(message)?; - - match response { - OutboundResponse::Text(text) => { - self.stop_typing(channel_id).await; - // mattermost_root_id: set when the triggering message was itself inside a thread. - // REPLY_TO_MESSAGE_ID: set by channel.rs on retrigger messages when a branch or - // worker completes — this is the post ID the agent should thread its reply under. - let root_id = message - .metadata - .get("mattermost_root_id") - .and_then(|v| v.as_str()) - .or_else(|| { - message - .metadata - .get(crate::metadata_keys::REPLY_TO_MESSAGE_ID) - .and_then(|v| v.as_str()) - }); - - for chunk in split_message(&text, MAX_MESSAGE_LENGTH) { - self.create_post(channel_id, &chunk, root_id).await?; - } - } - - OutboundResponse::StreamStart => { - // Key by message.id, not channel_id. Keying by channel_id would cause - // concurrent conversations in the same channel to overwrite each other. - let root_id = message.metadata.get("mattermost_root_id").and_then(|v| v.as_str()); - self.start_typing(channel_id).await; - let post = self.create_post(channel_id, "\u{200B}", root_id).await?; - self.active_messages.write().await.insert( - message.id.clone(), - ActiveStream { - post_id: post.id.into(), - channel_id: channel_id.to_string().into(), - last_edit: Instant::now(), - accumulated_text: String::new(), - }, - ); - } - - OutboundResponse::StreamChunk(chunk) => { - let mut active_messages = self.active_messages.write().await; - if let Some(active) = active_messages.get_mut(&message.id) { - active.accumulated_text.push_str(&chunk); - - if active.last_edit.elapsed() > STREAM_EDIT_THROTTLE { - let display_text = if active.accumulated_text.len() > MAX_MESSAGE_LENGTH { - let end = active.accumulated_text.floor_char_boundary(MAX_MESSAGE_LENGTH - 3); - format!("{}...", &active.accumulated_text[..end]) - } else { - active.accumulated_text.clone() - }; - - if let Err(error) = self.edit_post(&active.post_id, &display_text).await { - tracing::warn!(%error, "failed to edit streaming message"); - } - active.last_edit = Instant::now(); - } - } - } - - OutboundResponse::StreamEnd => { - self.stop_typing(channel_id).await; - // Always flush the final text, even if the last chunk was within the throttle window. - if let Some(active) = self.active_messages.write().await.remove(&message.id) { - if let Err(error) = self.edit_post(&active.post_id, &active.accumulated_text).await { - tracing::warn!(%error, "failed to finalize streaming message"); - } - } - } - - OutboundResponse::Status(status) => self.send_status(message, status).await?, - - OutboundResponse::Reaction(emoji) => { - let post_id = self.extract_post_id(message)?; - let emoji_name = emoji.trim_matches(':'); - - let response = self.client - .post(self.api_url("/reactions")) - .bearer_auth(self.token.as_ref()) - .json(&serde_json::json!({ - "post_id": post_id, - "emoji_name": emoji_name, - })) - .send() - .await - .context("failed to add reaction")?; - - if !response.status().is_success() { - tracing::warn!( - status = %response.status(), - emoji = %emoji_name, - "failed to add reaction" - ); - } - } - - OutboundResponse::File { filename, data, mime_type, caption } => { - if data.len() > self.max_attachment_bytes { - return Err(AdapterError::FileTooLarge { - size: data.len(), - max: self.max_attachment_bytes, - }.into()); - } - - let part = reqwest::multipart::Part::bytes(data) - .file_name(filename.clone()) - .mime_str(&mime_type) - .context("invalid mime type")?; - - let form = reqwest::multipart::Form::new() - .part("files", part) - .text("channel_id", channel_id.to_string()); - - let response = self.client - .post(self.api_url("/files")) - .bearer_auth(self.token.as_ref()) - .multipart(form) - .send() - .await - .context("failed to upload file")?; - - if !response.status().is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(AdapterError::ApiError { - endpoint: "/files".into(), - status: response.status().as_u16(), - message: body, - }.into()); - } - - let upload: MattermostFileUpload = response - .json() - .await - .context("failed to parse file upload response")?; - - let file_ids: Vec<_> = upload.file_infos.iter().map(|f| f.id.as_str()).collect(); - self.client - .post(self.api_url("/posts")) - .bearer_auth(self.token.as_ref()) - .json(&serde_json::json!({ - "channel_id": channel_id, - "message": caption.unwrap_or_default(), - "file_ids": file_ids, - })) - .send() - .await - .context("failed to create post with file")?; - } - - _ => { - tracing::debug!(?response, "mattermost adapter does not support this response type"); - } - } - - Ok(()) - } - - async fn send_status( - &self, - message: &InboundMessage, - status: StatusUpdate, - ) -> Result<()> { - let channel_id = self.extract_channel_id(message)?; - - match status { - StatusUpdate::Thinking => { - self.start_typing(channel_id).await; - } - StatusUpdate::StopTyping => { - self.stop_typing(channel_id).await; - } - _ => {} - } - - Ok(()) - } - - async fn fetch_history( - &self, - message: &InboundMessage, - limit: usize, - ) -> Result> { - let channel_id = self.extract_channel_id(message)?; - let before_post_id = message.metadata.get("mattermost_post_id").and_then(|v| v.as_str()); - - let capped_limit = limit.min(200) as u32; - let posts = self.get_channel_posts(channel_id, before_post_id, capped_limit).await?; - - let bot_id = self.bot_user_id.get().map(|s| s.as_ref().to_string()); - - // Sort by create_at before mapping so chronological order is preserved. - // Mapping to HistoryMessage discards the timestamp field. - let mut posts_vec: Vec<_> = posts - .posts - .into_values() - .filter(|p| bot_id.as_deref() != Some(p.user_id.as_str())) - .collect(); - posts_vec.sort_by_key(|p| p.create_at); - - let history: Vec = posts_vec - .into_iter() - .map(|p| HistoryMessage { - author: p.user_id, - content: p.message, - is_bot: false, - }) - .collect(); - - Ok(history) - } - - async fn health_check(&self) -> Result<()> { - let response = self.client - .get(self.api_url("/system/ping")) - .bearer_auth(self.token.as_ref()) - .send() - .await - .context("health check request failed")?; - - if !response.status().is_success() { - return Err(AdapterError::HealthCheckFailed( - format!("status: {}", response.status()) - ).into()); - } - - Ok(()) - } - - async fn shutdown(&self) -> Result<()> { - if let Some(tx) = self.shutdown_tx.write().await.take() { - let _ = tx.send(()).await; - } - - if let Some(handle) = self.ws_task.write().await.take() { - handle.abort(); - } - - self.typing_tasks.write().await.clear(); - self.active_messages.write().await.clear(); - - tracing::info!(adapter = %self.runtime_key, "mattermost adapter shut down"); - Ok(()) - } - - async fn broadcast(&self, target: &str, response: OutboundResponse) -> crate::Result<()> { - // target is a channel_id extracted by resolve_broadcast_target(). - match response { - OutboundResponse::Text(text) => { - for chunk in split_message(&text, MAX_MESSAGE_LENGTH) { - self.create_post(target, &chunk, None).await?; - } - } - OutboundResponse::File { filename, data, mime_type, caption } => { - if data.len() > self.max_attachment_bytes { - return Err(AdapterError::FileTooLarge { - size: data.len(), - max: self.max_attachment_bytes, - }.into()); - } - let part = reqwest::multipart::Part::bytes(data) - .file_name(filename) - .mime_str(&mime_type) - .context("invalid mime type")?; - let form = reqwest::multipart::Form::new() - .part("files", part) - .text("channel_id", target.to_string()); - let upload: MattermostFileUpload = self.client - .post(self.api_url("/files")) - .bearer_auth(self.token.as_ref()) - .multipart(form) - .send() - .await - .context("failed to upload file")? - .json() - .await - .context("failed to parse file upload response")?; - let file_ids: Vec<_> = upload.file_infos.iter().map(|f| f.id.as_str()).collect(); - self.client - .post(self.api_url("/posts")) - .bearer_auth(self.token.as_ref()) - .json(&serde_json::json!({ - "channel_id": target, - "message": caption.unwrap_or_default(), - "file_ids": file_ids, - })) - .send() - .await - .context("failed to create post with file")?; - } - other => { - tracing::debug!(?other, "mattermost broadcast does not support this response type"); - } - } - Ok(()) - } -} - -fn build_message_from_post( - post: &MattermostPost, - runtime_key: &str, - bot_user_id: &str, - team_id: &Option, - permissions: &MattermostPermissions, -) -> Option { - if post.user_id == bot_user_id { - return None; - } - - if let Some(team_filter) = &permissions.team_filter { - if let Some(ref tid) = team_id { - if !team_filter.contains(tid) { - return None; - } - } - } - - let conversation_id = apply_runtime_adapter_to_conversation_id( - runtime_key, - format!("mattermost:{}:{}", team_id.as_deref().unwrap_or(""), post.channel_id), - ); - - let mut metadata = HashMap::new(); - - // Standard keys — required by channel.rs and channel_history.rs. - // metadata_keys::MESSAGE_ID is read by extract_message_id(), which feeds - // branch/worker reply threading via REPLY_TO_MESSAGE_ID in retrigger metadata. - metadata.insert( - crate::metadata_keys::MESSAGE_ID.into(), - serde_json::json!(&post.id), - ); - - // Platform-specific keys used within the adapter (respond, fetch_history, etc.). - metadata.insert("mattermost_post_id".into(), serde_json::json!(&post.id)); - metadata.insert("mattermost_channel_id".into(), serde_json::json!(&post.channel_id)); - if let Some(tid) = team_id { - metadata.insert("mattermost_team_id".into(), serde_json::json!(tid)); - } - if !post.root_id.is_empty() { - metadata.insert("mattermost_root_id".into(), serde_json::json!(&post.root_id)); - } - - Some(InboundMessage { - id: post.id.clone(), - source: "mattermost".into(), - adapter: Some(runtime_key.to_string()), - conversation_id, - sender_id: post.user_id.clone(), - agent_id: None, - content: MessageContent::Text(post.message.clone()), - timestamp: chrono::DateTime::from_timestamp_millis(post.create_at) - .unwrap_or_else(chrono::Utc::now), - metadata, - formatted_author: None, - }) -} - -// --- API Types --- - -#[derive(Debug, Clone, Deserialize)] -struct MattermostUser { - id: String, - username: String, -} - -#[derive(Debug, Clone, Deserialize, Serialize)] -struct MattermostPost { - id: String, - create_at: i64, - update_at: i64, - user_id: String, - channel_id: String, - root_id: String, - message: String, - // "D" = direct message, "G" = group DM, "O" = public, "P" = private. - // Present in WebSocket events; absent in REST list responses. - #[serde(default)] - channel_type: Option, - #[serde(default)] - file_ids: Vec, -} - -#[derive(Debug, Deserialize)] -struct MattermostPostList { - #[serde(default)] - order: Vec, - #[serde(default)] - posts: HashMap, -} - -#[derive(Debug, Deserialize)] -struct MattermostFileUpload { - #[serde(default)] - file_infos: Vec, -} - -#[derive(Debug, Deserialize)] -struct MattermostFileInfo { - id: String, - #[allow(dead_code)] - name: String, -} - -#[derive(Debug, Deserialize)] -struct MattermostWsEvent { - event: String, - #[serde(default)] - data: serde_json::Value, - #[serde(default)] - broadcast: MattermostWsBroadcast, -} - -#[derive(Debug, Deserialize, Default)] -struct MattermostWsBroadcast { - #[serde(default)] - channel_id: Option, - #[serde(default)] - team_id: Option, - #[serde(default)] - user_id: Option, -} - -fn split_message(text: &str, max_len: usize) -> Vec { - if text.len() <= max_len { - return vec![text.to_string()]; - } - - let mut chunks = Vec::new(); - let mut remaining = text; - - while !remaining.is_empty() { - if remaining.len() <= max_len { - chunks.push(remaining.to_string()); - break; - } - - let search_region = &remaining[..max_len]; - let break_point = search_region - .rfind('\n') - .or_else(|| search_region.rfind(' ')) - .unwrap_or(max_len); - - let end = remaining.floor_char_boundary(break_point); - chunks.push(remaining[..end].to_string()); - remaining = remaining[end..].trim_start_matches('\n').trim_start(); - } - - chunks -} - -// --- Error Types (add to src/error.rs) --- - -#[derive(Debug, thiserror::Error)] -pub enum AdapterError { - #[error("missing metadata field: {0}")] - MissingMetadata(String), - - #[error("invalid id format: {0}")] - InvalidId(String), - - #[error("API error at {endpoint}: {status} - {message}")] - ApiError { - endpoint: String, - status: u16, - message: String, - }, - - #[error("file too large: {size} bytes (max: {max})")] - FileTooLarge { size: usize, max: usize }, - - #[error("health check failed: {0}")] - HealthCheckFailed(String), - - #[error("invalid configuration: {0}")] - ConfigInvalid(String), - - #[error("adapter not initialized")] - NotInitialized, -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_split_message_short() { - let result = split_message("hello", 100); - assert_eq!(result, vec!["hello"]); - } - - #[test] - fn test_split_message_exact_boundary() { - let text = "a".repeat(100); - let result = split_message(&text, 100); - assert_eq!(result.len(), 1); - } - - #[test] - fn test_split_message_on_newline() { - let text = "line1\nline2\nline3"; - let result = split_message(text, 8); - assert_eq!(result, vec!["line1", "line2", "line3"]); - } - - #[test] - fn test_split_message_on_space() { - let text = "word1 word2 word3"; - let result = split_message(text, 12); - assert_eq!(result, vec!["word1 word2", "word3"]); - } - - #[test] - fn test_split_message_forced_break() { - let text = "abcdefghijklmnopqrstuvwxyz"; - let result = split_message(text, 10); - assert_eq!(result, vec!["abcdefghij", "klmnopqrst", "uvwxyz"]); - } - - #[test] - fn test_validate_id_valid() { - assert!(MattermostAdapter::validate_id("abc123").is_ok()); - assert!(MattermostAdapter::validate_id("a-b_c").is_ok()); - } - - #[test] - fn test_validate_id_invalid() { - assert!(MattermostAdapter::validate_id("").is_err()); - assert!(MattermostAdapter::validate_id("has spaces").is_err()); - assert!(MattermostAdapter::validate_id("has/slash").is_err()); - } -} +base_url = "https://community.example.com" +token = "community-bot-token" ``` -### 4. Module Registration (src/messaging/mod.rs) - -```rust -pub mod mattermost; - -pub use mattermost::MattermostAdapter; -``` - -### 5. Target Resolution (src/messaging/target.rs) - -Add to `resolve_broadcast_target()`: - -```rust -"mattermost" => { - if let Some(channel_id) = channel - .platform_meta - .as_ref() - .and_then(|meta| meta.get("mattermost_channel_id")) - .and_then(json_value_to_string) - { - channel_id - } else { - let parts: Vec<&str> = channel.id.split(':').collect(); - match parts.as_slice() { - ["mattermost", _, channel_id] => (*channel_id).to_string(), - ["mattermost", _, "dm", user_id] => format!("dm:{user_id}"), - ["mattermost", _, _, channel_id] => (*channel_id).to_string(), - _ => return None, - } - } -} -``` +Named instance tokens can be supplied via `MATTERMOST_CORP_TOKEN` / `MATTERMOST_COMMUNITY_TOKEN` etc. -Add to `normalize_target()`: - -```rust -"mattermost" => normalize_mattermost_target(trimmed), -``` - -Add normalization function: - -```rust -fn normalize_mattermost_target(raw_target: &str) -> Option { - let target = strip_repeated_prefix(raw_target, "mattermost"); - - if let Some((_, channel_id)) = target.split_once(':') { - if !channel_id.is_empty() { - return Some(channel_id.to_string()); - } - return None; - } - - if let Some(user_id) = target.strip_prefix("dm:") { - if !user_id.is_empty() { - return Some(format!("dm:{user_id}")); - } - return None; - } - - if target.is_empty() { - None - } else { - Some(target.to_string()) - } -} -``` - -### 6. Binding Support (src/config.rs) - -The `Binding` struct currently has `guild_id`, `workspace_id`, and `chat_id` but **no `team_id` -field**. Add it: - -```rust -pub struct Binding { - // ... existing fields ... - pub team_id: Option, // NEW — Mattermost team ID filter -} -``` - -Add to the TOML deserialization type (`TomlBinding` or equivalent): - -```rust -#[serde(default)] -team_id: Option, -``` - -Add to `Binding::matches()` after the existing channel-type checks: - -```rust -// Mattermost team filter -if let Some(team_id) = &self.team_id { - if self.channel == "mattermost" { - let message_team = message - .metadata - .get("mattermost_team_id") - .and_then(|v| v.as_str()); - if message_team != Some(team_id.as_str()) { - return false; - } - } -} - -// Mattermost channel filter -if !self.channel_ids.is_empty() && self.channel == "mattermost" { - let message_channel = message - .metadata - .get("mattermost_channel_id") - .and_then(|v| v.as_str()); - if !self.channel_ids.iter().any(|id| Some(id.as_str()) == message_channel) { - return false; - } -} -``` - -### 7. Main.rs Registration - -Add following the existing pattern: - -```rust -// Shared Mattermost permissions (hot-reloadable via file watcher) -*mattermost_permissions = config.messaging.mattermost.as_ref().map(|mm_config| { - let perms = MattermostPermissions::from_config(mm_config, &config.bindings); - Arc::new(ArcSwap::from_pointee(perms)) -}); -if let Some(perms) = &*mattermost_permissions { - api_state.set_mattermost_permissions(perms.clone()).await; -} - -if let Some(mm_config) = &config.messaging.mattermost - && mm_config.enabled -{ - if !mm_config.base_url.is_empty() && !mm_config.token.is_empty() { - match MattermostAdapter::new( - Arc::from("mattermost"), - &mm_config.base_url, - Arc::from(mm_config.token.clone()), - mm_config.team_id.clone().map(Arc::from), - mm_config.max_attachment_bytes, - mattermost_permissions.clone().ok_or_else(|| { - anyhow::anyhow!("mattermost permissions not initialized") - })?, - ) { - Ok(adapter) => { - new_messaging_manager.register(adapter).await; - } - Err(error) => { - tracing::error!(%error, "failed to build mattermost adapter"); - } - } - } - - for instance in mm_config - .instances - .iter() - .filter(|i| i.enabled) - { - if instance.base_url.is_empty() || instance.token.is_empty() { - tracing::warn!(adapter = %instance.name, "skipping enabled mattermost instance with missing config"); - continue; - } - let runtime_key = binding_runtime_adapter_key("mattermost", Some(instance.name.as_str())); - let perms = Arc::new(ArcSwap::from_pointee( - MattermostPermissions::from_instance_config(instance, &config.bindings), - )); - match MattermostAdapter::new( - Arc::from(runtime_key), - &instance.base_url, - Arc::from(instance.token.clone()), - instance.team_id.clone().map(Arc::from), - instance.max_attachment_bytes, - perms, - ) { - Ok(adapter) => { - new_messaging_manager.register(adapter).await; - } - Err(error) => { - tracing::error!(adapter = %instance.name, %error, "failed to build named mattermost adapter"); - } - } - } -} -``` - -### 8. Permissions Implementation - -```rust -impl MattermostPermissions { - pub fn from_config(config: &MattermostConfig, bindings: &[Binding]) -> Self { - Self::from_bindings_for_adapter( - config.dm_allowed_users.clone(), - bindings, - None, - ) - } - - pub fn from_instance_config( - instance: &MattermostInstanceConfig, - bindings: &[Binding], - ) -> Self { - Self::from_bindings_for_adapter( - instance.dm_allowed_users.clone(), - bindings, - Some(instance.name.as_str()), - ) - } - - fn from_bindings_for_adapter( - seed_dm_allowed_users: Vec, - bindings: &[Binding], - adapter_selector: Option<&str>, - ) -> Self { - let mm_bindings: Vec<&Binding> = bindings - .iter() - .filter(|b| { - b.channel == "mattermost" - && binding_adapter_selector_matches(b, adapter_selector) - }) - .collect(); - - let team_filter = { - let team_ids: Vec = mm_bindings - .iter() - .filter_map(|b| b.team_id.clone()) - .collect(); - if team_ids.is_empty() { None } else { Some(team_ids) } - }; - - let channel_filter = { - let mut filter: HashMap> = HashMap::new(); - for binding in &mm_bindings { - if let Some(team_id) = &binding.team_id - && !binding.channel_ids.is_empty() - { - filter - .entry(team_id.clone()) - .or_default() - .extend(binding.channel_ids.clone()); - } - } - filter - }; - - let mut dm_allowed_users = seed_dm_allowed_users; - for binding in &mm_bindings { - for id in &binding.dm_allowed_users { - if !dm_allowed_users.contains(id) { - dm_allowed_users.push(id.clone()); - } - } - } - - Self { - team_filter, - channel_filter, - dm_allowed_users, - } - } -} -``` - -### 9. Error Types (src/error.rs) - -Add to the existing Error enum: - -```rust -#[derive(Debug, thiserror::Error)] -pub enum Error { - // ... existing variants - - #[error("adapter error: {0}")] - Adapter(#[from] crate::messaging::mattermost::AdapterError), -} -``` - ---- - -## API Endpoints Used - -| Endpoint | Method | Purpose | -|----------|--------|---------| -| `/users/me` | GET | Bot identity | -| `/users/{id}/typing` | POST | Typing indicator | -| `/posts` | POST | Create message | -| `/posts/{id}` | PUT | Edit message (streaming) | -| `/posts/{id}` | DELETE | Delete message | -| `/channels/{id}/posts` | GET | History fetch | -| `/files` | POST | File upload | -| `/reactions` | POST | Add reaction | -| `/system/ping` | GET | Health check | -| WebSocket `/api/v4/websocket` | - | Real-time events | - ---- - -## Dependencies - -The project already has `tokio-tungstenite = "0.28"` in `Cargo.toml`. Do **not** add a `0.24` -entry — it conflicts with the existing version. Verify the existing entry includes `native-tls`: +In bindings, reference a named instance with the `adapter` field: ```toml -# Ensure this is already present with the native-tls feature; do not downgrade: -tokio-tungstenite = { version = "0.28", features = ["native-tls"] } - -# Add if not already present: -url = "2.5" +[[bindings]] +agent_id = "my-agent" +channel = "mattermost" +adapter = "corp" +channel_ids = ["channel_id_here"] ``` -`futures-util` is already an indirect dependency; verify before adding explicitly. - ---- - -## Feature Support Matrix - -| Feature | Priority | Status | -|---------|----------|--------| -| Text messages | Required | Planned | -| Streaming edits | Required | Planned | -| Typing indicator | Required | Planned | -| Thread replies | High | Planned | -| File attachments | Medium | Planned | -| Reactions | Medium | Planned | -| History fetch | Medium | Planned | -| Rich formatting (markdown) | Low | Planned | -| Interactive buttons | Low | Future | -| Slash commands | Low | Future | - ---- - -## Testing Strategy - -1. **Unit tests**: Message splitting, ID validation, target normalization, permission filtering -2. **Integration tests**: Mock Mattermost API server (mockito) -3. **Manual testing**: Against real Mattermost instance - ---- +## Web UI -## Migration Notes +All of the above can be configured in the Spacebot web interface under **Settings → Messaging → Mattermost**. The UI supports adding credentials, enabling/disabling the adapter, and managing bindings with team and channel scoping. -- No database migrations required -- Configuration is additive only -- No breaking changes to existing adapters +## Finding IDs ---- +Mattermost does not display IDs in the UI by default. The easiest ways to retrieve them: -## Checklist for Implementation +- **Team ID**: `GET /api/v4/teams/name/{team_name}` → `.id` +- **Channel ID**: `GET /api/v4/teams/{team_id}/channels/name/{channel_name}` → `.id` +- **User ID**: `GET /api/v4/users/username/{username}` → `.id` -- [ ] Add `MattermostConfig` and related types to `src/config.rs` -- [ ] Add TOML deserialization types -- [ ] Add URL validation -- [ ] Update `is_named_adapter_platform` -- [ ] Add `AdapterError` to `src/error.rs` -- [ ] Create `src/messaging/mattermost.rs` with full implementation -- [ ] Add to `src/messaging/mod.rs` -- [ ] Update `src/messaging/target.rs` for broadcast resolution -- [ ] Add main.rs registration -- [ ] Add unit tests -- [ ] Add integration tests with mock server -- [ ] Update README.md with Mattermost configuration docs +Alternatively, enable **Account Settings → Advanced → Enable post formatting** and inspect the network tab when loading a channel — team and channel IDs appear in the request URLs. From aa8c2c7b73ed15f6e982080791bcae53ef5f1132 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sun, 15 Mar 2026 21:36:47 +0100 Subject: [PATCH 17/19] fix: address coderabbitai PR review findings in Mattermost adapter - create_binding: guard empty team_id to match update_binding behavior - DeleteBindingRequest: add team_id to prevent wrong-binding deletion - split_message: filter break_point == 0 to prevent empty chunk push - mentions_bot: set true for DMs; detect thread replies to bot via async root-post fetch in WS handler (resolve_root_post_author) Co-Authored-By: Claude Sonnet 4.6 --- src/api/bindings.rs | 14 ++++++-- src/messaging/mattermost.rs | 69 ++++++++++++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/api/bindings.rs b/src/api/bindings.rs index 438e1be71..ddefbb5a7 100644 --- a/src/api/bindings.rs +++ b/src/api/bindings.rs @@ -118,6 +118,8 @@ pub(super) struct DeleteBindingRequest { workspace_id: Option, #[serde(default)] chat_id: Option, + #[serde(default)] + team_id: Option, } #[derive(Serialize)] @@ -441,8 +443,8 @@ pub(super) async fn create_binding( if let Some(chat_id) = &request.chat_id { binding_table["chat_id"] = toml_edit::value(chat_id.as_str()); } - if let Some(team_id) = &request.team_id { - binding_table["team_id"] = toml_edit::value(team_id.as_str()); + if let Some(team_id) = request.team_id.as_deref().map(str::trim).filter(|s| !s.is_empty()) { + binding_table["team_id"] = toml_edit::value(team_id); } if !request.channel_ids.is_empty() { let mut arr = toml_edit::Array::new(); @@ -939,12 +941,20 @@ pub(super) async fn delete_binding( .is_some_and(|v| v == cid), None => table.get("chat_id").is_none(), }; + let matches_team = match &request.team_id { + Some(tid) => table + .get("team_id") + .and_then(|v: &toml_edit::Item| v.as_str()) + .is_some_and(|v| v == tid), + None => table.get("team_id").is_none(), + }; if matches_agent && matches_channel && matches_adapter && matches_guild && matches_workspace && matches_chat + && matches_team { match_idx = Some(i); break; diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index 0637a77d4..7294a6bdd 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -545,7 +545,7 @@ impl Messaging for MattermostAdapter { &post.channel_id, ).await; - if let Some(msg) = build_message_from_post( + if let Some(mut msg) = build_message_from_post( &post, &runtime_key, &bot_user_id, @@ -555,6 +555,24 @@ impl Messaging for MattermostAdapter { display_name.as_deref(), channel_name.as_deref(), ) { + // Detect thread replies to the bot: + // if root_id is set, fetch the root post + // and check if the bot authored it. + if !post.root_id.is_empty() { + if let Some(root_author) = resolve_root_post_author( + &ws_client, + token.as_ref(), + &ws_base_url, + &post.root_id, + ).await { + if root_author == bot_user_id.as_ref() { + msg.metadata.insert( + "mattermost_mentions_or_replies_to_bot".into(), + serde_json::json!(true), + ); + } + } + } if inbound_tx_clone.send(msg).await.is_err() { tracing::debug!("inbound channel closed"); return; @@ -1174,9 +1192,12 @@ fn build_message_from_post( ); } - // FN4: bot mention detection — check for @bot_username in message text - let mentions_bot = !bot_username.is_empty() - && post.message.contains(&format!("@{bot_username}")); + // FN4: bot mention detection — @mention, DM, or thread reply to bot post. + // Thread-reply-to-bot detection is handled asynchronously in the WS event + // handler and may upgrade this to true after this function returns. + let is_dm = post.channel_type.as_deref() == Some("D"); + let mentions_bot = is_dm + || (!bot_username.is_empty() && post.message.contains(&format!("@{bot_username}"))); metadata.insert( "mattermost_mentions_or_replies_to_bot".into(), serde_json::json!(mentions_bot), @@ -1386,6 +1407,43 @@ async fn resolve_channel_name( Some(name) } +/// Fetch the `user_id` of the author of a Mattermost post by its ID. +/// +/// Used to detect thread replies to the bot: if a post's `root_id` resolves to +/// a post authored by the bot, the reply should be treated as directed at the +/// bot for `require_mention` purposes. Returns `None` on any network or API +/// error (logged at `DEBUG` level). +async fn resolve_root_post_author( + client: &Client, + token: &str, + base_url: &Url, + post_id: &str, +) -> Option { + let mut url = base_url.clone(); + url.path_segments_mut() + .ok()? + .extend(["api", "v4", "posts", post_id]); + let resp = match client.get(url).bearer_auth(token).send().await { + Ok(r) => r, + Err(error) => { + tracing::debug!(%error, post_id, "failed to fetch mattermost root post"); + return None; + } + }; + if !resp.status().is_success() { + tracing::debug!(status = %resp.status(), post_id, "mattermost root post fetch returned non-success"); + return None; + } + let post: MattermostPost = match resp.json().await { + Ok(p) => p, + Err(error) => { + tracing::debug!(%error, post_id, "failed to parse mattermost root post response"); + return None; + } + }; + Some(post.user_id) +} + /// Convert an emoji input to a Mattermost reaction short-code name. /// /// Handles three input forms: @@ -1428,9 +1486,12 @@ fn split_message(text: &str, max_len: usize) -> Vec { let search_end = remaining.floor_char_boundary(max_len); let search_region = &remaining[..search_end]; + // Ignore a break point at position 0 to avoid pushing an empty chunk + // (e.g. when the region starts with a newline or space). let break_point = search_region .rfind('\n') .or_else(|| search_region.rfind(' ')) + .filter(|&pos| pos > 0) .unwrap_or(search_end); let end = remaining.floor_char_boundary(break_point); From 956d11f6b014fae2972444792c0568950ee2d514 Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Sun, 15 Mar 2026 22:14:57 +0100 Subject: [PATCH 18/19] fix: address coderabbitai PR review findings in Mattermost adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - bindings: normalize team_id in update/delete matching — trim whitespace and treat whitespace-only values as absent for consistent comparisons - bindings: trim team_id before persisting in update_binding write path - bindings: rename abbreviated match locals (gid/wid/cid/tid) to full names - ws_url: replace silent scheme passthrough with unreachable! for unsupported schemes Co-Authored-By: Claude Sonnet 4.6 --- src/api/bindings.rs | 48 +++++++++++++++---------------------- src/messaging/mattermost.rs | 2 +- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/api/bindings.rs b/src/api/bindings.rs index ddefbb5a7..4693b66d0 100644 --- a/src/api/bindings.rs +++ b/src/api/bindings.rs @@ -713,33 +713,29 @@ pub(super) async fn update_binding( None => table.get("adapter").is_none(), }; let matches_guild = match &request.original_guild_id { - Some(gid) => table + Some(guild_id) => table .get("guild_id") .and_then(|v| v.as_str()) - .is_some_and(|v| v == gid), + .is_some_and(|v| v == guild_id), None => table.get("guild_id").is_none(), }; let matches_workspace = match &request.original_workspace_id { - Some(wid) => table + Some(workspace_id) => table .get("workspace_id") .and_then(|v| v.as_str()) - .is_some_and(|v| v == wid), + .is_some_and(|v| v == workspace_id), None => table.get("workspace_id").is_none(), }; let matches_chat = match &request.original_chat_id { - Some(cid) => table + Some(chat_id) => table .get("chat_id") .and_then(|v| v.as_str()) - .is_some_and(|v| v == cid), + .is_some_and(|v| v == chat_id), None => table.get("chat_id").is_none(), }; - let matches_team = match &request.original_team_id { - Some(tid) => table - .get("team_id") - .and_then(|v| v.as_str()) - .is_some_and(|v| v == tid), - None => table.get("team_id").is_none(), - }; + let req_team_id = request.original_team_id.as_deref().map(str::trim).filter(|s| !s.is_empty()); + let toml_team_id = table.get("team_id").and_then(|v| v.as_str()).map(str::trim).filter(|s| !s.is_empty()); + let matches_team = req_team_id == toml_team_id; if matches_agent && matches_channel && matches_adapter @@ -797,9 +793,7 @@ pub(super) async fn update_binding( { binding["chat_id"] = toml_edit::value(chat_id); } - if let Some(ref team_id) = request.team_id - && !team_id.is_empty() - { + if let Some(team_id) = request.team_id.as_deref().map(str::trim).filter(|s| !s.is_empty()) { binding["team_id"] = toml_edit::value(team_id); } @@ -921,33 +915,29 @@ pub(super) async fn delete_binding( None => table.get("adapter").is_none(), }; let matches_guild = match &request.guild_id { - Some(gid) => table + Some(guild_id) => table .get("guild_id") .and_then(|v: &toml_edit::Item| v.as_str()) - .is_some_and(|v| v == gid), + .is_some_and(|v| v == guild_id), None => table.get("guild_id").is_none(), }; let matches_workspace = match &request.workspace_id { - Some(wid) => table + Some(workspace_id) => table .get("workspace_id") .and_then(|v: &toml_edit::Item| v.as_str()) - .is_some_and(|v| v == wid), + .is_some_and(|v| v == workspace_id), None => table.get("workspace_id").is_none(), }; let matches_chat = match &request.chat_id { - Some(cid) => table + Some(chat_id) => table .get("chat_id") .and_then(|v: &toml_edit::Item| v.as_str()) - .is_some_and(|v| v == cid), + .is_some_and(|v| v == chat_id), None => table.get("chat_id").is_none(), }; - let matches_team = match &request.team_id { - Some(tid) => table - .get("team_id") - .and_then(|v: &toml_edit::Item| v.as_str()) - .is_some_and(|v| v == tid), - None => table.get("team_id").is_none(), - }; + let req_team_id = request.team_id.as_deref().map(str::trim).filter(|s| !s.is_empty()); + let toml_team_id = table.get("team_id").and_then(|v: &toml_edit::Item| v.as_str()).map(str::trim).filter(|s| !s.is_empty()); + let matches_team = req_team_id == toml_team_id; if matches_agent && matches_channel && matches_adapter diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index 7294a6bdd..a21ce9dc3 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -134,7 +134,7 @@ impl MattermostAdapter { url.set_scheme(match self.base_url.scheme() { "https" => "wss", "http" => "ws", - other => other, + other => unreachable!("unsupported URL scheme: {other}"), }) .expect("scheme substitution is valid"); url.path_segments_mut() From 1a38ac7d34486d4d023f91215ed1248ff7cd6bae Mon Sep 17 00:00:00 2001 From: Kevin Read Date: Mon, 16 Mar 2026 09:58:41 +0100 Subject: [PATCH 19/19] fix: address coderabbitai nitpick findings in Mattermost adapter - bindings: hoist request_team_id normalization before the loop in both update_binding and delete_binding to avoid recomputing it per-iteration - mattermost new(): remove redundant .into() on anyhow::anyhow! return - ws loop: rename e to error in match arms and tracing macros Co-Authored-By: Claude Sonnet 4.6 --- src/api/bindings.rs | 10 ++++++---- src/messaging/mattermost.rs | 10 +++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/api/bindings.rs b/src/api/bindings.rs index 4693b66d0..64da215de 100644 --- a/src/api/bindings.rs +++ b/src/api/bindings.rs @@ -695,6 +695,8 @@ pub(super) async fn update_binding( .and_then(|b| b.as_array_of_tables_mut()) .ok_or(StatusCode::NOT_FOUND)?; + let request_team_id = request.original_team_id.as_deref().map(str::trim).filter(|s| !s.is_empty()); + let mut match_idx: Option = None; for (i, table) in bindings_array.iter().enumerate() { let matches_agent = table @@ -733,9 +735,8 @@ pub(super) async fn update_binding( .is_some_and(|v| v == chat_id), None => table.get("chat_id").is_none(), }; - let req_team_id = request.original_team_id.as_deref().map(str::trim).filter(|s| !s.is_empty()); let toml_team_id = table.get("team_id").and_then(|v| v.as_str()).map(str::trim).filter(|s| !s.is_empty()); - let matches_team = req_team_id == toml_team_id; + let matches_team = request_team_id == toml_team_id; if matches_agent && matches_channel && matches_adapter @@ -897,6 +898,8 @@ pub(super) async fn delete_binding( .and_then(|b| b.as_array_of_tables_mut()) .ok_or(StatusCode::NOT_FOUND)?; + let request_team_id = request.team_id.as_deref().map(str::trim).filter(|s| !s.is_empty()); + let mut match_idx: Option = None; for (i, table) in bindings_array.iter().enumerate() { let matches_agent = table @@ -935,9 +938,8 @@ pub(super) async fn delete_binding( .is_some_and(|v| v == chat_id), None => table.get("chat_id").is_none(), }; - let req_team_id = request.team_id.as_deref().map(str::trim).filter(|s| !s.is_empty()); let toml_team_id = table.get("team_id").and_then(|v: &toml_edit::Item| v.as_str()).map(str::trim).filter(|s| !s.is_empty()); - let matches_team = req_team_id == toml_team_id; + let matches_team = request_team_id == toml_team_id; if matches_agent && matches_channel && matches_adapter diff --git a/src/messaging/mattermost.rs b/src/messaging/mattermost.rs index a21ce9dc3..84bbe46fa 100644 --- a/src/messaging/mattermost.rs +++ b/src/messaging/mattermost.rs @@ -91,7 +91,7 @@ impl MattermostAdapter { return Err(anyhow::anyhow!( "mattermost base_url must be an origin URL without path/query/fragment (got: {})", base_url - ).into()); + )); } let client = Client::builder() @@ -595,8 +595,8 @@ impl Messaging for MattermostAdapter { tracing::info!(adapter = %runtime_key, "websocket closed by server"); break; } - Some(Err(e)) => { - tracing::error!(adapter = %runtime_key, error = %e, "websocket error"); + Some(Err(error)) => { + tracing::error!(adapter = %runtime_key, %error, "websocket error"); break; } None => break, @@ -608,10 +608,10 @@ impl Messaging for MattermostAdapter { tracing::info!(adapter = %runtime_key, "websocket disconnected, reconnecting..."); } - Err(e) => { + Err(error) => { tracing::error!( adapter = %runtime_key, - error = %e, + %error, delay_ms = retry_delay.as_millis(), "websocket connection failed, retrying" );