diff --git a/crates/openfang-channels/src/bridge.rs b/crates/openfang-channels/src/bridge.rs index 29e70252..a1aecae6 100644 --- a/crates/openfang-channels/src/bridge.rs +++ b/crates/openfang-channels/src/bridge.rs @@ -679,6 +679,34 @@ fn sender_user_id(message: &ChannelMessage) -> &str { .unwrap_or(&message.sender.platform_id) } +/// Extract the channel/conversation ID from a message, for bindings whose +/// `match_rule.channel_id` is set. +/// +/// On Discord and Slack, `sender.platform_id` already holds the channel/ +/// conversation ID (per `discord.rs` and `slack.rs`, where the user ID lives +/// in metadata under `sender_user_id`). On other adapters where the platform +/// ID is the user, callers can opt-in by stashing the channel ID under the +/// `sender_channel_id` metadata key. +fn sender_channel_id(message: &ChannelMessage) -> Option<&str> { + if let Some(v) = message + .metadata + .get("sender_channel_id") + .and_then(|v| v.as_str()) + { + return Some(v); + } + // On Discord/Slack, the metadata `sender_user_id` is set and differs from + // `sender.platform_id` — in that case, platform_id IS the channel ID. + let user_in_meta = message + .metadata + .get("sender_user_id") + .and_then(|v| v.as_str()); + match user_in_meta { + Some(uid) if uid != message.sender.platform_id => Some(&message.sender.platform_id), + _ => None, + } +} + /// If an error contains "Agent not found", try to re-resolve the channel's default agent /// by name (the name stored at bridge startup). Returns `Some(new_id)` on success. async fn try_reresolution( @@ -1001,10 +1029,13 @@ async fn dispatch_message( // Route to agent (standard path). // Use sender_user_id() so user-keyed bindings (peer_id) match for adapters like // Discord/Slack where sender.platform_id is the channel ID, not the user ID. - let agent_id = router.resolve( + // Pass the channel/conversation ID separately so bindings with `channel_id` + // can match (e.g. "messages in Discord channel X → agent Y"). + let agent_id = router.resolve_with_channel_id( &message.channel, sender_user_id(message), message.sender.openfang_user.as_deref(), + sender_channel_id(message), ); let agent_id = match agent_id { @@ -1444,11 +1475,13 @@ async fn dispatch_with_blocks( prefix_style: PrefixStyle, ) { // Route to agent (same logic as text path). - // Use sender_user_id() so user-keyed bindings match for Discord/Slack. - let agent_id = router.resolve( + // Use sender_user_id() so user-keyed bindings match for Discord/Slack; + // pass channel_id so per-room bindings match too. + let agent_id = router.resolve_with_channel_id( &message.channel, sender_user_id(message), message.sender.openfang_user.as_deref(), + sender_channel_id(message), ); let agent_id = match agent_id { diff --git a/crates/openfang-channels/src/router.rs b/crates/openfang-channels/src/router.rs index 666768a7..b1de44e3 100644 --- a/crates/openfang-channels/src/router.rs +++ b/crates/openfang-channels/src/router.rs @@ -18,6 +18,10 @@ pub struct BindingContext { pub peer_id: String, /// Guild/server ID. pub guild_id: Option, + /// Channel/conversation ID (e.g. Discord channel, Slack conversation, + /// Telegram chat, IRC channel name). Populated by bridges so bindings can + /// route by room independent of which user posted. + pub channel_id: Option, /// User's roles. pub roles: Vec, } @@ -143,6 +147,19 @@ impl AgentRouter { channel_type: &ChannelType, platform_user_id: &str, user_key: Option<&str>, + ) -> Option { + self.resolve_with_channel_id(channel_type, platform_user_id, user_key, None) + } + + /// Resolve with an explicit channel/conversation ID, so bindings whose + /// `match_rule.channel_id` is set can match. Used by bridges that know the + /// room/conversation the message arrived in (Discord/Slack/Telegram/IRC). + pub fn resolve_with_channel_id( + &self, + channel_type: &ChannelType, + platform_user_id: &str, + user_key: Option<&str>, + channel_id: Option<&str>, ) -> Option { let channel_key = format!("{channel_type:?}"); @@ -152,6 +169,7 @@ impl AgentRouter { account_id: None, peer_id: platform_user_id.to_string(), guild_id: None, + channel_id: channel_id.map(|s| s.to_string()), roles: Vec::new(), }; if let Some(agent_id) = self.resolve_binding(&ctx) { @@ -329,6 +347,11 @@ impl AgentRouter { return false; } } + if let Some(ref cid) = rule.channel_id { + if ctx.channel_id.as_ref() != Some(cid) { + return false; + } + } if !rule.roles.is_empty() { // User must have at least one of the specified roles let has_role = rule.roles.iter().any(|r| ctx.roles.contains(r)); @@ -639,7 +662,128 @@ mod tests { guild_id: Some("guild".to_string()), roles: vec!["admin".to_string()], account_id: Some("bot".to_string()), + channel_id: Some("ch_42".to_string()), }; - assert_eq!(full.specificity(), 17); // 8+4+2+2+1 + assert_eq!(full.specificity(), 25); // 8+8+4+2+2+1 + + // peer_id alone vs channel_id alone — both worth 8. + let peer_only = BindingMatchRule { + peer_id: Some("u".to_string()), + ..Default::default() + }; + let channel_id_only = BindingMatchRule { + channel_id: Some("c".to_string()), + ..Default::default() + }; + assert_eq!(peer_only.specificity(), 8); + assert_eq!(channel_id_only.specificity(), 8); + + // Combined peer_id + channel_id (16) outranks either alone (8). + let peer_and_channel = BindingMatchRule { + peer_id: Some("u".to_string()), + channel_id: Some("c".to_string()), + ..Default::default() + }; + assert_eq!(peer_and_channel.specificity(), 16); + } + + #[test] + fn test_binding_channel_id_match() { + // A binding scoped to a specific Discord channel should match messages + // from that channel and reject messages from other channels. + let router = AgentRouter::new(); + let agent_id = AgentId::new(); + router.register_agent("ops-bot".to_string(), agent_id); + router.load_bindings(&[AgentBinding { + agent: "ops-bot".to_string(), + match_rule: openfang_types::config::BindingMatchRule { + channel: Some("discord".to_string()), + channel_id: Some("1477803840265781391".to_string()), + ..Default::default() + }, + }]); + + // Same channel, any user — matches. + let resolved = router.resolve_with_channel_id( + &ChannelType::Discord, + "any-user", + None, + Some("1477803840265781391"), + ); + assert_eq!(resolved, Some(agent_id)); + + // Different channel — no match. + let resolved = router.resolve_with_channel_id( + &ChannelType::Discord, + "any-user", + None, + Some("9999999999999999999"), + ); + assert_eq!(resolved, None); + + // Missing channel_id on the wire — no match (the binding is restrictive). + let resolved = router.resolve_with_channel_id(&ChannelType::Discord, "any-user", None, None); + assert_eq!(resolved, None); + } + + #[test] + fn test_binding_channel_id_plus_peer_outranks_channel_id_alone() { + // user A in #medical → researcher; anyone else in #medical → general. + let router = AgentRouter::new(); + let researcher = AgentId::new(); + let general = AgentId::new(); + router.register_agent("researcher".to_string(), researcher); + router.register_agent("general".to_string(), general); + router.load_bindings(&[ + AgentBinding { + agent: "general".to_string(), + match_rule: openfang_types::config::BindingMatchRule { + channel_id: Some("ch-medical".to_string()), + ..Default::default() + }, + }, + AgentBinding { + agent: "researcher".to_string(), + match_rule: openfang_types::config::BindingMatchRule { + channel_id: Some("ch-medical".to_string()), + peer_id: Some("user-a".to_string()), + ..Default::default() + }, + }, + ]); + + // user-a in #medical → researcher (more specific wins) + let r = router.resolve_with_channel_id( + &ChannelType::Discord, + "user-a", + None, + Some("ch-medical"), + ); + assert_eq!(r, Some(researcher)); + + // user-b in #medical → general (channel_id alone matches) + let r = router.resolve_with_channel_id( + &ChannelType::Discord, + "user-b", + None, + Some("ch-medical"), + ); + assert_eq!(r, Some(general)); + } + + #[test] + fn test_binding_match_rule_unknown_field_rejected() { + // Typos like `channnel_id` must fail loudly at deserialization rather + // than silently producing a wide-open binding. This is the highest- + // leverage line in the patch from issue #1127. + let bad = r#"{ "channnel_id": "ch-1" }"#; + let r: Result = serde_json::from_str(bad); + assert!(r.is_err(), "unknown field must be rejected by serde"); + + // Sanity: known fields still parse. + let good = r#"{ "channel_id": "ch-1", "channel": "discord" }"#; + let r: openfang_types::config::BindingMatchRule = serde_json::from_str(good).unwrap(); + assert_eq!(r.channel_id.as_deref(), Some("ch-1")); + assert_eq!(r.channel.as_deref(), Some("discord")); } } diff --git a/crates/openfang-types/src/config.rs b/crates/openfang-types/src/config.rs index d06c6924..c6048911 100644 --- a/crates/openfang-types/src/config.rs +++ b/crates/openfang-types/src/config.rs @@ -742,16 +742,32 @@ pub struct AgentBinding { } /// Match rule for agent bindings. All specified (non-None) fields must match. +/// +/// `#[serde(deny_unknown_fields)]` is intentional: a typo like `channnel_id` or +/// `chan_id` would otherwise be silently dropped, producing a wide-open binding +/// that matches every message. Failing loudly at config load is the safer default. #[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct BindingMatchRule { /// Channel type (e.g., "discord", "telegram", "slack"). + #[serde(default)] pub channel: Option, /// Specific account/bot ID within the channel. + #[serde(default)] pub account_id: Option, /// Peer/user ID for DM routing. + #[serde(default)] pub peer_id: Option, /// Guild/server ID (Discord/Slack). + #[serde(default)] pub guild_id: Option, + /// Channel/conversation ID — the per-channel routing dimension. + /// On Discord this is the channel/thread ID; on Slack it is the conversation + /// ID (`C…`/`D…`/`G…`); on Telegram it is the chat ID; on IRC it is the + /// channel name. Bridges populate this from the message's channel/conversation + /// identifier so bindings can route by room independent of which user posted. + #[serde(default)] + pub channel_id: Option, /// Role-based routing (user must have at least one). #[serde(default)] pub roles: Vec, @@ -760,11 +776,17 @@ pub struct BindingMatchRule { impl BindingMatchRule { /// Calculate specificity score for binding priority ordering. /// Higher = more specific = checked first. + /// + /// Weights: peer_id and channel_id are both 8 so a binding that combines + /// both (a specific user in a specific room) cleanly outranks either alone. pub fn specificity(&self) -> u32 { let mut score = 0u32; if self.peer_id.is_some() { score += 8; } + if self.channel_id.is_some() { + score += 8; + } if self.guild_id.is_some() { score += 4; }