mirror of
https://github.com/RightNow-AI/openfang.git
synced 2026-07-29 22:22:35 +00:00
Merge pull request #1147 from benhoverter/harden-channel-id-binding
feat(channels): harden channel_id binding — adapter allowlist, strict validation, single source of truth for routing
This commit is contained in:
@@ -4,7 +4,7 @@
|
||||
//! `BridgeManager` which owns running adapters and dispatches messages.
|
||||
|
||||
use crate::formatter;
|
||||
use crate::router::AgentRouter;
|
||||
use crate::router::{AgentRouter, BindingContext};
|
||||
use crate::types::{
|
||||
default_phase_emoji, AgentPhase, ChannelAdapter, ChannelContent, ChannelMessage, ChannelUser,
|
||||
LifecycleReaction,
|
||||
@@ -679,31 +679,33 @@ 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.
|
||||
/// Build a `BindingContext` for routing the given inbound message.
|
||||
///
|
||||
/// 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,
|
||||
/// Populates `channel_id` so per-channel bindings (e.g. `channel_id = "<discord_channel>"`)
|
||||
/// can route to dedicated agents. The channel ID source is delegated to
|
||||
/// [`ChannelMessage::channel_id`] — the single source of truth shared with
|
||||
/// config validation (see `CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL` in
|
||||
/// `openfang-types::config`). `peer_id` uses the resolved user ID, not
|
||||
/// `sender.platform_id`, so user-scoped bindings still match correctly on
|
||||
/// Discord/Slack/etc. where `platform_id` holds the channel.
|
||||
///
|
||||
/// This replaces the earlier heuristic `sender_channel_id()` (which inferred
|
||||
/// "platform_id is the channel" from "metadata has `sender_user_id`"). The
|
||||
/// allowlist is explicit, the metadata-fallback path is documented, and
|
||||
/// adapters can be added or removed in one place (`openfang-types::config`)
|
||||
/// without touching this file.
|
||||
fn binding_context_for(message: &ChannelMessage) -> BindingContext {
|
||||
BindingContext {
|
||||
channel: channel_type_str(&message.channel).to_string(),
|
||||
account_id: None,
|
||||
peer_id: sender_user_id(message).to_string(),
|
||||
channel_id: message.channel_id(),
|
||||
guild_id: message
|
||||
.metadata
|
||||
.get("guild_id")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(String::from),
|
||||
roles: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1029,13 +1031,15 @@ 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.
|
||||
// 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(
|
||||
// Use resolve_with_context so channel_id-scoped (and guild_id-scoped)
|
||||
// bindings can route per channel — see binding_context_for() for the
|
||||
// single-source-of-truth allowlist.
|
||||
let binding_ctx = binding_context_for(message);
|
||||
let agent_id = router.resolve_with_context(
|
||||
&message.channel,
|
||||
sender_user_id(message),
|
||||
message.sender.openfang_user.as_deref(),
|
||||
sender_channel_id(message),
|
||||
&binding_ctx,
|
||||
);
|
||||
|
||||
let agent_id = match agent_id {
|
||||
@@ -1485,12 +1489,13 @@ async fn dispatch_with_blocks(
|
||||
) {
|
||||
// Route to agent (same logic as text path).
|
||||
// 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(
|
||||
// resolve_with_context lets channel_id-scoped bindings match per room.
|
||||
let binding_ctx = binding_context_for(message);
|
||||
let agent_id = router.resolve_with_context(
|
||||
&message.channel,
|
||||
sender_user_id(message),
|
||||
message.sender.openfang_user.as_deref(),
|
||||
sender_channel_id(message),
|
||||
&binding_ctx,
|
||||
);
|
||||
|
||||
let agent_id = match agent_id {
|
||||
@@ -2190,6 +2195,122 @@ mod tests {
|
||||
assert_eq!(GroupPolicy::default(), GroupPolicy::MentionOnly);
|
||||
}
|
||||
|
||||
// -- binding_context_for / ChannelMessage::channel_id() coverage --
|
||||
//
|
||||
// These tests pin the routing-time behavior so future adapter additions to
|
||||
// CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL cannot silently regress the bridge.
|
||||
|
||||
fn make_msg_for_ctx(
|
||||
channel: ChannelType,
|
||||
platform_id: &str,
|
||||
metadata: Vec<(&str, serde_json::Value)>,
|
||||
) -> ChannelMessage {
|
||||
let mut md = std::collections::HashMap::new();
|
||||
for (k, v) in metadata {
|
||||
md.insert(k.to_string(), v);
|
||||
}
|
||||
ChannelMessage {
|
||||
channel,
|
||||
platform_message_id: "msg-1".to_string(),
|
||||
sender: crate::types::ChannelUser {
|
||||
platform_id: platform_id.to_string(),
|
||||
display_name: "Tester".to_string(),
|
||||
openfang_user: None,
|
||||
},
|
||||
content: ChannelContent::Text("hi".to_string()),
|
||||
target_agent: None,
|
||||
timestamp: chrono::Utc::now(),
|
||||
is_group: true,
|
||||
thread_id: None,
|
||||
metadata: md,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_binding_context_for_discord_uses_platform_id_as_channel() {
|
||||
let msg = make_msg_for_ctx(ChannelType::Discord, "1234567890", vec![]);
|
||||
let ctx = binding_context_for(&msg);
|
||||
assert_eq!(ctx.channel, "discord");
|
||||
assert_eq!(ctx.channel_id.as_deref(), Some("1234567890"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_binding_context_for_telegram_uses_platform_id_as_channel() {
|
||||
// Regression guard: Telegram is on the channel-ID allowlist.
|
||||
let msg = make_msg_for_ctx(ChannelType::Telegram, "-100123", vec![]);
|
||||
let ctx = binding_context_for(&msg);
|
||||
assert_eq!(ctx.channel, "telegram");
|
||||
assert_eq!(ctx.channel_id.as_deref(), Some("-100123"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_binding_context_for_matrix_uses_room_id_from_platform_id() {
|
||||
let msg = make_msg_for_ctx(ChannelType::Matrix, "!room:server.tld", vec![]);
|
||||
let ctx = binding_context_for(&msg);
|
||||
assert_eq!(ctx.channel_id.as_deref(), Some("!room:server.tld"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_binding_context_for_custom_supported_adapter() {
|
||||
// Custom("twitch") is on the allowlist.
|
||||
let msg = make_msg_for_ctx(
|
||||
ChannelType::Custom("twitch".to_string()),
|
||||
"channel-foo",
|
||||
vec![],
|
||||
);
|
||||
let ctx = binding_context_for(&msg);
|
||||
assert_eq!(ctx.channel, "twitch");
|
||||
assert_eq!(ctx.channel_id.as_deref(), Some("channel-foo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_binding_context_for_user_id_adapter_returns_none() {
|
||||
// Reddit's platform_id is the post author, not a subreddit/conversation.
|
||||
// The bridge must not surface that as `channel_id` (would silently match
|
||||
// user-scoped bindings against a user ID).
|
||||
let msg = make_msg_for_ctx(
|
||||
ChannelType::Custom("reddit".to_string()),
|
||||
"u/some-user",
|
||||
vec![],
|
||||
);
|
||||
let ctx = binding_context_for(&msg);
|
||||
assert_eq!(ctx.channel, "reddit");
|
||||
assert!(ctx.channel_id.is_none());
|
||||
// peer_id still falls through to platform_id (sender_user_id default).
|
||||
assert_eq!(ctx.peer_id, "u/some-user");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_binding_context_for_metadata_fallback() {
|
||||
// For non-allowlisted adapters, metadata["channel_id"] is the
|
||||
// documented escape hatch — verify the bridge honors it.
|
||||
let msg = make_msg_for_ctx(
|
||||
ChannelType::Custom("reddit".to_string()),
|
||||
"u/some-user",
|
||||
vec![("channel_id", serde_json::json!("r/rust"))],
|
||||
);
|
||||
let ctx = binding_context_for(&msg);
|
||||
assert_eq!(ctx.channel_id.as_deref(), Some("r/rust"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_binding_context_for_metadata_guild_id() {
|
||||
let msg = make_msg_for_ctx(
|
||||
ChannelType::Discord,
|
||||
"1234567890",
|
||||
vec![("guild_id", serde_json::json!("99999"))],
|
||||
);
|
||||
let ctx = binding_context_for(&msg);
|
||||
assert_eq!(ctx.guild_id.as_deref(), Some("99999"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_channel_message_channel_id_email_returns_none() {
|
||||
// Email's platform_id is the sender address — not a channel.
|
||||
let msg = make_msg_for_ctx(ChannelType::Email, "alice@example.com", vec![]);
|
||||
assert!(msg.channel_id().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_channel_type_str() {
|
||||
assert_eq!(channel_type_str(&ChannelType::Telegram), "telegram");
|
||||
|
||||
@@ -97,6 +97,60 @@ pub struct ChannelMessage {
|
||||
pub metadata: HashMap<String, serde_json::Value>,
|
||||
}
|
||||
|
||||
// Re-export the adapter allowlist from openfang-types so config validation
|
||||
// and routing share a single source of truth (no drift between the two).
|
||||
pub use openfang_types::config::CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL;
|
||||
|
||||
impl ChannelMessage {
|
||||
/// Return the platform-native channel/conversation ID for this message,
|
||||
/// suitable for matching against an `AgentBinding`'s `channel_id` field.
|
||||
///
|
||||
/// Resolution order:
|
||||
/// 1. For adapters in [`CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL`],
|
||||
/// `sender.platform_id` already *is* the channel ID (these adapters
|
||||
/// overload the field because it doubles as the send target).
|
||||
/// 2. Otherwise, fall back to `metadata["channel_id"]` if present (any
|
||||
/// adapter can opt in by populating that key).
|
||||
/// 3. Otherwise, `None`.
|
||||
///
|
||||
/// This is the central routing-time accessor — config validation and the
|
||||
/// router both consult it (directly or via the same allowlist) so the two
|
||||
/// cannot drift.
|
||||
pub fn channel_id(&self) -> Option<String> {
|
||||
// For builtin variants the string is already lowercase by construction.
|
||||
// For `Custom(s)`, adapters _should_ register lowercase names but we
|
||||
// case-fold here so a stray `Custom("Twitch")` cannot silently slip
|
||||
// past the allowlist (and out of step with the validation path, which
|
||||
// already lowercases user input). Allocates only on the Custom arm.
|
||||
let channel_str: std::borrow::Cow<'_, str> = match &self.channel {
|
||||
ChannelType::Telegram => "telegram".into(),
|
||||
ChannelType::Discord => "discord".into(),
|
||||
ChannelType::Slack => "slack".into(),
|
||||
ChannelType::WhatsApp => "whatsapp".into(),
|
||||
ChannelType::Signal => "signal".into(),
|
||||
ChannelType::Matrix => "matrix".into(),
|
||||
ChannelType::Email => "email".into(),
|
||||
ChannelType::Teams => "teams".into(),
|
||||
ChannelType::Mattermost => "mattermost".into(),
|
||||
ChannelType::WebChat => "webchat".into(),
|
||||
ChannelType::CLI => "cli".into(),
|
||||
ChannelType::Mqtt => "mqtt".into(),
|
||||
ChannelType::Custom(s) => s.to_lowercase().into(),
|
||||
};
|
||||
if CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL
|
||||
.iter()
|
||||
.any(|c| *c == channel_str.as_ref())
|
||||
{
|
||||
Some(self.sender.platform_id.clone())
|
||||
} else {
|
||||
self.metadata
|
||||
.get("channel_id")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(String::from)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Agent lifecycle phase for UX indicators.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
@@ -365,6 +419,34 @@ mod tests {
|
||||
assert_eq!(back, ChannelType::Email);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_channel_id_custom_arm_is_case_insensitive() {
|
||||
// A stray capitalized Custom variant must still resolve through the
|
||||
// allowlist. The validation path lowercases user input; the routing
|
||||
// path needs the same case-fold to stay in sync.
|
||||
let make = |name: &str| ChannelMessage {
|
||||
channel: ChannelType::Custom(name.to_string()),
|
||||
platform_message_id: "m".to_string(),
|
||||
sender: ChannelUser {
|
||||
platform_id: "C123".to_string(),
|
||||
display_name: "x".to_string(),
|
||||
openfang_user: None,
|
||||
},
|
||||
content: ChannelContent::Text("hi".to_string()),
|
||||
target_agent: None,
|
||||
timestamp: Utc::now(),
|
||||
is_group: false,
|
||||
thread_id: None,
|
||||
metadata: HashMap::new(),
|
||||
};
|
||||
assert_eq!(make("twitch").channel_id().as_deref(), Some("C123"));
|
||||
assert_eq!(make("Twitch").channel_id().as_deref(), Some("C123"));
|
||||
assert_eq!(make("TWITCH").channel_id().as_deref(), Some("C123"));
|
||||
// Lark spelling (Feishu Intl) must also match.
|
||||
assert_eq!(make("lark").channel_id().as_deref(), Some("C123"));
|
||||
assert_eq!(make("Lark").channel_id().as_deref(), Some("C123"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_channel_content_variants() {
|
||||
let text = ChannelContent::Text("hello".to_string());
|
||||
|
||||
@@ -733,7 +733,12 @@ impl Default for VaultConfig {
|
||||
}
|
||||
|
||||
/// Agent binding — routes specific channel/account/peer patterns to agents.
|
||||
///
|
||||
/// `deny_unknown_fields` so typos at the binding level (e.g. `match_rule` →
|
||||
/// `match_rules`) fail loudly at config-load instead of silently leaving the
|
||||
/// rule defaulted to "match everything".
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct AgentBinding {
|
||||
/// Target agent name or ID.
|
||||
pub agent: String,
|
||||
@@ -741,6 +746,50 @@ pub struct AgentBinding {
|
||||
pub match_rule: BindingMatchRule,
|
||||
}
|
||||
|
||||
/// Lowercased channel-name strings whose adapters place a channel/conversation/
|
||||
/// room/space/chat ID directly in `ChannelMessage::sender.platform_id` (these
|
||||
/// adapters overload that field because it doubles as the send target).
|
||||
///
|
||||
/// Single source of truth shared between:
|
||||
/// - Config validation (warn the user when their `channel_id` binding targets
|
||||
/// an adapter that doesn't populate `ctx.channel_id`).
|
||||
/// - `ChannelMessage::channel_id()` in `openfang-channels::types` (routing-time
|
||||
/// accessor that reads from this list to decide where to source the ID).
|
||||
///
|
||||
/// Adapters not listed fall back to `metadata["channel_id"]` if present, then
|
||||
/// `None`. Hybrid adapters whose `platform_id` flips between channel and user
|
||||
/// based on `is_group` (IRC, Zulip) are intentionally excluded — a single
|
||||
/// channel-scoped binding would silently match DMs.
|
||||
///
|
||||
/// Compared against the lowercased `channel` string from `BindingMatchRule`
|
||||
/// or from `channel_type_str()` at routing time.
|
||||
pub const CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL: &[&str] = &[
|
||||
"discord",
|
||||
"slack",
|
||||
"telegram",
|
||||
"matrix",
|
||||
"mattermost",
|
||||
"teams",
|
||||
"webex",
|
||||
"rocketchat",
|
||||
"nextcloud",
|
||||
"pumble",
|
||||
"revolt",
|
||||
"guilded",
|
||||
"feishu",
|
||||
// Feishu Intl region emits `Custom("lark")` from `feishu.rs` (region.as_str()
|
||||
// returns "lark" when configured for international). Listed alongside
|
||||
// "feishu" so both regional spellings resolve identically at routing and
|
||||
// validation time.
|
||||
"lark",
|
||||
"keybase",
|
||||
"google_chat",
|
||||
"line",
|
||||
"twist",
|
||||
"flock",
|
||||
"twitch",
|
||||
];
|
||||
|
||||
/// Match rule for agent bindings. All specified (non-None) fields must match.
|
||||
///
|
||||
/// `#[serde(deny_unknown_fields)]` is intentional: a typo like `channnel_id` or
|
||||
@@ -755,7 +804,18 @@ pub struct BindingMatchRule {
|
||||
/// Specific account/bot ID within the channel.
|
||||
#[serde(default)]
|
||||
pub account_id: Option<String>,
|
||||
/// Peer/user ID for DM routing.
|
||||
/// Peer/user ID. Matches `BindingContext::peer_id`, which the bridge
|
||||
/// populates from `ChannelMessage::sender_user_id()` — i.e. the platform's
|
||||
/// *user* identity (Discord user ID, Slack user ID, etc.).
|
||||
///
|
||||
/// Note: the legacy `AgentRouter::resolve()` entry point (kept for tests
|
||||
/// and any caller without bridge context) builds a synthetic context
|
||||
/// where `peer_id` is filled from the raw `platform_user_id` argument. On
|
||||
/// adapters that overload `platform_id` as the channel ID (Discord, Slack,
|
||||
/// …), passing those callers a channel-scoped value will match here. New
|
||||
/// code should route through `resolve_with_context` and a bridge-built
|
||||
/// `BindingContext`. For platform-native channel/conversation matching,
|
||||
/// use `channel_id` instead.
|
||||
#[serde(default)]
|
||||
pub peer_id: Option<String>,
|
||||
/// Guild/server ID (Discord/Slack).
|
||||
@@ -766,6 +826,8 @@ pub struct BindingMatchRule {
|
||||
/// 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.
|
||||
/// Pair with `channel` to disambiguate across platforms (channel IDs are
|
||||
/// not portable).
|
||||
#[serde(default)]
|
||||
pub channel_id: Option<String>,
|
||||
/// Role-based routing (user must have at least one).
|
||||
@@ -1050,6 +1112,14 @@ impl Default for ThinkingConfig {
|
||||
}
|
||||
|
||||
/// Top-level kernel configuration.
|
||||
///
|
||||
/// `deny_unknown_fields` is intentionally *not* applied here. Strict-field
|
||||
/// validation is scoped to bindings (`AgentBinding` + `BindingMatchRule`),
|
||||
/// where silent no-op routing is the failure mode worth catching loudly.
|
||||
/// Adding it to the top-level kernel struct would also reject forward-compat
|
||||
/// keys, downstream-fork-only keys, and "I'm trying out a future field early"
|
||||
/// workflows — a wider behavior change than this PR's mandate. If we want it
|
||||
/// later, it ships as its own decision.
|
||||
#[derive(Clone, Serialize, Deserialize)]
|
||||
#[serde(default)]
|
||||
pub struct KernelConfig {
|
||||
@@ -3723,6 +3793,42 @@ impl KernelConfig {
|
||||
SearchProvider::DuckDuckGo | SearchProvider::Auto => {}
|
||||
}
|
||||
|
||||
// --- Binding validation (channel_id) ---
|
||||
// Use the shared allowlist (CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL) so this
|
||||
// validation cannot drift from the routing-time accessor in
|
||||
// ChannelMessage::channel_id(). Adapters not on the list may still
|
||||
// populate ctx.channel_id via metadata["channel_id"], but we cannot
|
||||
// detect that statically — so we warn conservatively and document the
|
||||
// metadata escape hatch in docs/channel-adapters.md.
|
||||
for (idx, binding) in self.bindings.iter().enumerate() {
|
||||
let rule = &binding.match_rule;
|
||||
if let Some(ref cid) = rule.channel_id {
|
||||
match rule.channel.as_deref() {
|
||||
None => {
|
||||
warnings.push(format!(
|
||||
"Binding #{} (agent='{}') sets channel_id='{}' without channel; \
|
||||
channel IDs are not portable across platforms. Pair with channel = \"discord\" (or similar).",
|
||||
idx, binding.agent, cid
|
||||
));
|
||||
}
|
||||
Some(ch) => {
|
||||
let ch_lower = ch.to_lowercase();
|
||||
if !CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL
|
||||
.iter()
|
||||
.any(|p| *p == ch_lower)
|
||||
{
|
||||
warnings.push(format!(
|
||||
"Binding #{} (agent='{}') sets channel_id='{}' for channel='{}', \
|
||||
but the {} adapter does not populate ctx.channel_id from sender.platform_id; \
|
||||
this binding will only match if the adapter writes channel_id into message metadata.",
|
||||
idx, binding.agent, cid, ch, ch
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// --- Production bounds validation ---
|
||||
// Clamp dangerous zero/extreme values to safe defaults instead of crashing.
|
||||
warnings
|
||||
@@ -3782,6 +3888,106 @@ mod tests {
|
||||
assert!(toml_str.contains("log_level"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_binding_match_rule_deny_unknown_fields_rejects_typo() {
|
||||
// Typo (`channnel_id` with three n's) should fail to parse rather than
|
||||
// silently producing a no-op rule that matches every message.
|
||||
let toml_input = r#"
|
||||
channel = "discord"
|
||||
channnel_id = "12345"
|
||||
"#;
|
||||
let result: Result<BindingMatchRule, _> = toml::from_str(toml_input);
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"expected deny_unknown_fields to reject typo'd field, got: {:?}",
|
||||
result
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_agent_binding_deny_unknown_fields() {
|
||||
// A typo at the AgentBinding level (`match_rules` plural instead of
|
||||
// `match_rule`) must fail config load — silent default would make the
|
||||
// binding a wildcard.
|
||||
let toml_str = r#"
|
||||
agent = "x"
|
||||
[match_rules]
|
||||
channel = "discord"
|
||||
"#;
|
||||
let result: Result<AgentBinding, _> = toml::from_str(toml_str);
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"expected deny_unknown_fields rejection, got Ok"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_warns_channel_id_without_channel() {
|
||||
let mut config = KernelConfig::default();
|
||||
config.bindings.push(AgentBinding {
|
||||
agent: "ghost".to_string(),
|
||||
match_rule: BindingMatchRule {
|
||||
channel_id: Some("99999".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
});
|
||||
let warnings = config.validate();
|
||||
assert!(
|
||||
warnings
|
||||
.iter()
|
||||
.any(|w| w.contains("channel_id") && w.contains("without channel")),
|
||||
"expected channel_id-without-channel warning, got: {:?}",
|
||||
warnings
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_warns_channel_id_for_unsupported_adapter() {
|
||||
// Reddit's `platform_id` is the post author, not a subreddit; no
|
||||
// per-conversation channel_id, so the binding can never match.
|
||||
let mut config = KernelConfig::default();
|
||||
config.bindings.push(AgentBinding {
|
||||
agent: "ghost".to_string(),
|
||||
match_rule: BindingMatchRule {
|
||||
channel: Some("reddit".to_string()),
|
||||
channel_id: Some("r/something".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
});
|
||||
let warnings = config.validate();
|
||||
assert!(
|
||||
warnings
|
||||
.iter()
|
||||
.any(|w| w.contains("reddit") && w.contains("does not populate")),
|
||||
"expected unsupported-adapter warning, got: {:?}",
|
||||
warnings
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_no_warning_for_supported_adapters() {
|
||||
// Discord, Slack, Telegram all overload platform_id as the channel ID —
|
||||
// these must not warn.
|
||||
for ch in ["discord", "slack", "telegram"] {
|
||||
let mut config = KernelConfig::default();
|
||||
config.bindings.push(AgentBinding {
|
||||
agent: "ok".to_string(),
|
||||
match_rule: BindingMatchRule {
|
||||
channel: Some(ch.to_string()),
|
||||
channel_id: Some("X".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
});
|
||||
let warnings = config.validate();
|
||||
assert!(
|
||||
!warnings.iter().any(|w| w.contains("does not populate")),
|
||||
"did not expect channel_id-coverage warning for {}, got: {:?}",
|
||||
ch,
|
||||
warnings
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_discord_config_defaults() {
|
||||
let dc = DiscordConfig::default();
|
||||
|
||||
@@ -622,10 +622,63 @@ Features:
|
||||
|
||||
The `AgentRouter` determines which agent receives an incoming message. The routing logic is:
|
||||
|
||||
1. **Per-channel default**: Each channel config has a `default_agent` field. Messages from that channel go to that agent.
|
||||
2. **User-agent binding**: If a user has previously been associated with a specific agent (via commands or configuration), messages from that user route to that agent.
|
||||
3. **Command prefix**: Users can switch agents by sending a command like `/agent coder` in the chat. Subsequent messages will be routed to the "coder" agent.
|
||||
4. **Fallback**: If no routing applies, messages go to the first available agent.
|
||||
1. **Bindings** (most specific first). Declarative `[[bindings]]` rules in `config.toml` map message attributes (channel, channel_id, peer_id, guild_id, account_id, roles) to agents. The router scores each rule by specificity and picks the highest-scoring match.
|
||||
2. **Per-channel default**: Each channel config has a `default_agent` field. Messages from that channel go to that agent.
|
||||
3. **User-agent binding**: If a user has previously been associated with a specific agent (via commands or configuration), messages from that user route to that agent.
|
||||
4. **Command prefix**: Users can switch agents by sending a command like `/agent coder` in the chat. Subsequent messages will be routed to the "coder" agent.
|
||||
5. **Fallback**: If no routing applies, messages go to the first available agent.
|
||||
|
||||
### Bindings
|
||||
|
||||
A binding has an `agent` (the target) and a `match_rule` (the criteria). All non-empty fields in the rule must match.
|
||||
|
||||
```toml
|
||||
# Route a specific Discord channel to a dedicated agent.
|
||||
[[bindings]]
|
||||
agent = "researcher-medical"
|
||||
match_rule = { channel = "discord", channel_id = "1234567890" }
|
||||
|
||||
[[bindings]]
|
||||
agent = "researcher-business"
|
||||
match_rule = { channel = "discord", channel_id = "9876543210" }
|
||||
|
||||
# Catch-all for the same user on any other channel.
|
||||
[[bindings]]
|
||||
agent = "assistant"
|
||||
match_rule = { channel = "discord", peer_id = "user_discord_id" }
|
||||
```
|
||||
|
||||
**`peer_id` vs `channel_id`** — these are easy to confuse and the difference matters:
|
||||
|
||||
- `peer_id` matches the **user** (Discord user ID, Slack user ID, etc.).
|
||||
- `channel_id` matches the **channel/conversation** (Discord text channel, Slack conversation, Telegram chat).
|
||||
|
||||
Use `peer_id` for "messages from this person." Use `channel_id` for "messages in this room."
|
||||
|
||||
**Specificity scores** (higher wins):
|
||||
|
||||
| Field | Score |
|
||||
| ------------ | ----- |
|
||||
| `peer_id` | 8 |
|
||||
| `channel_id` | 8 |
|
||||
| `guild_id` | 4 |
|
||||
| `roles` | 2 |
|
||||
| `account_id` | 2 |
|
||||
| `channel` | 1 |
|
||||
|
||||
A binding's score is the sum of its set fields. `peer_id` and `channel_id` are equally specific, so a rule with both (16) beats either alone (8). Ties are broken by declaration order in the config.
|
||||
|
||||
**Adapter coverage for `channel_id`** — the following adapters populate `ctx.channel_id` directly from `sender.platform_id` (their "user" field is overloaded as a channel/conversation/room/space ID because that field doubles as the send target):
|
||||
|
||||
`discord`, `slack`, `telegram`, `matrix`, `mattermost`, `teams`, `webex`, `rocketchat`, `nextcloud`, `pumble`, `revolt`, `guilded`, `feishu`, `lark`, `keybase`, `google_chat`, `line`, `twist`, `flock`, `twitch`.
|
||||
|
||||
(Feishu Intl region emits `Custom("lark")` rather than `Custom("feishu")`; both spellings are recognized.)
|
||||
|
||||
Adapters not on this list (Reddit, Bluesky, Mastodon, Signal, Email, ntfy, Discourse, etc.) carry a *user* ID in `platform_id` and have no per-conversation concept, or use a hybrid scheme (IRC, Zulip flip between channel and user based on `is_group`). Bindings targeting `channel_id` on those platforms will only match if the adapter writes a `channel_id` key into message metadata.
|
||||
|
||||
The kernel emits a startup warning when a binding sets `channel_id` for a non-supporting adapter, so misconfigurations surface early instead of silently routing nowhere. The single source of truth for this list is `CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL` in `openfang-types::config`, consumed by both routing (`ChannelMessage::channel_id()`) and config validation.
|
||||
|
||||
**Strict parsing** — `AgentBinding` and `BindingMatchRule` use `#[serde(deny_unknown_fields)]`. Typos at the binding level (e.g. `match_rules` for `match_rule`, `channnel_id` for `channel_id`) fail config load with a clear error rather than parsing into a no-op rule that silently matches every message. Existing configs that work today are unaffected; only configs with stray/misspelled fields inside a `[[bindings]]` block need a fix. The top-level `KernelConfig` deliberately stays permissive so unrecognized top-level keys (forward-compat, downstream forks) don't break startup.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user