admin/openfang
1
0
Fork 0
mirror of https://github.com/RightNow-AI/openfang.git synced 2026-08-14 08:52:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(channels): harden channel_id binding (extends d336314)

Browse Source 
Layers richer config validation, an explicit adapter allowlist, and a
stricter bridge routing path on top of upstream `d336314` ("binding
rule"), which shipped the same `channel_id` field as our PR #1127 in a
parallel implementation. Replaces upstream's `sender_user_id`/
`platform_id` heuristic with a single source of truth shared between
config validation and routing.

## What changes vs upstream `d336314`

**Data model** (`openfang-types/src/config.rs`)
- `#[serde(deny_unknown_fields)]` on `AgentBinding` so a typo at the
  binding level (e.g. `match_rules` plural) fails loudly instead of
  silently leaving the rule defaulted to "match everything". Upstream
  has it on `BindingMatchRule` only.
- New `pub const CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL` (19 adapters:
  discord, slack, telegram, matrix, mattermost, teams, webex,
  rocketchat, nextcloud, pumble, revolt, guilded, feishu, lark,
  keybase, google_chat, line, twist, flock, twitch). Single source of
  truth shared with the bridge — no drift between routing and
  validation paths possible. Hybrid adapters (IRC, Zulip) are
  excluded; see source comment.
- Startup validation: warn when a binding sets `channel_id` for a
  non-supporting adapter, or when `channel_id` is set without
  `channel`. Documents the metadata escape hatch in the warning.
- Top-level `KernelConfig` keeps no `deny_unknown_fields` — comment
  explains the §5.5 scoping decision so a future reader doesn't
  "tighten" it without realizing it would break forward-compat keys.

**Bridge** (`openfang-channels/src/bridge.rs`)
- Replaces upstream's `sender_channel_id()` heuristic ("if metadata has
  `sender_user_id` and it differs from `platform_id`, assume
  `platform_id` IS the channel") with `binding_context_for(message)`,
  which delegates to `ChannelMessage::channel_id()`. The heuristic
  worked for Discord/Slack but would fail silently on Matrix, Teams,
  Mattermost, Telegram, etc. — adapters whose `platform_id` IS the
  channel ID but whose metadata does not happen to set
  `sender_user_id` differently.
- Routes both dispatch paths (text + blocks) through
  `resolve_with_context` so `guild_id` and `channel_id` bindings can
  match. (Upstream's `resolve_with_channel_id` only handled
  channel_id.)

**Channels types** (`openfang-channels/src/types.rs`)
- New `ChannelMessage::channel_id()` accessor: reads `platform_id` for
  allowlisted adapters, falls back to `metadata["channel_id"]` for
  opt-in adapters, else `None`. Case-folds `Custom(...)` variants so
  a stray `Custom("Twitch")` cannot silently slip past the allowlist
  (the validation path lowercases user input — accessor must match).

**Tests** (+8 in `bridge.rs`, +5 in `config.rs`, +1 in `types.rs`)
- Bridge: Discord/Telegram/Matrix/custom-supported/user-id-only-adapter
  /metadata-fallback/guild-id-from-metadata/Email-returns-None
  coverage of `binding_context_for` and `channel_id()`.
- Config: typo rejection on both `BindingMatchRule` and `AgentBinding`;
  channel_id-without-channel warning; unsupported-adapter warning;
  no-warning for discord/slack/telegram.
- Types: `channel_id()` case-insensitivity for Custom variants
  including the Lark/Feishu Intl spelling.

**Docs** (`docs/channel-adapters.md`)
- Routing section rewritten: bindings are step 1 in the resolution
  order. New "Bindings" subsection documents the rule shape, the
  `peer_id` vs `channel_id` distinction (the easy confusion), full
  specificity table, the adapter allowlist with the metadata escape
  hatch, and the strict-field parsing rule.

## Why this layering instead of replacing d336314

Upstream's commit and our PR #1127 are functionally equivalent on
Discord and Slack. Shipping a richer extension on top is less churn
than ripping out the upstream commit and substituting ours, and keeps
the API surface upstream just added (`resolve_with_channel_id`)
intact for any third-party consumers.

`cargo check --workspace` and `cargo test -p openfang-types -p
openfang-channels --lib` (850 tests) pass.
This commit is contained in:
Ben Hoverter
2026-04-30 17:10:16 -07:00
parent 15ed29c667
commit faf2cf9211
4 changed files with 499 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/channel-adapters.md
+57 -4
View File
@@ -608,10 +608,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.
---