channels: harden channel_id binding — strict wrapper, explicit adapter allowlist, full guild+channel context #1144

Description

d336314 introduced channel_id matching on BindingMatchRule with deny_unknown_fields and a sender_channel_id heuristic in the bridge. In production use on a fork tracking main, three follow-on gaps surfaced. Each is small, independently defensible, and has tests:

1. deny_unknown_fields is missing on the AgentBinding wrapper

BindingMatchRule rejects unknown fields, but AgentBinding itself does not. A common typo — match_rules (plural) instead of match_rule — is silently accepted, the typo'd field is dropped, and the binding loads with no rules attached (matches everything).

Fix: add #[serde(deny_unknown_fields)] to AgentBinding. One line. The same protection it already gives the inner rule.

Note on KernelConfig: we deliberately do not apply deny_unknown_fields at the top level. Forward-compat with new top-level sections matters more there than typo-catching, and a comment in the source explains the asymmetry.

2. Replace the sender_channel_id heuristic with an explicit adapter allowlist

d336314's heuristic in bridge.rs reads:

if metadata[\"sender_user_id\"] exists AND differs from sender.platform_id, treat platform_id as the channel id.

This is correct for Discord and Slack today because their bridges happen to populate sender_user_id. It fails open for any adapter whose bridge doesn't — and there's no signal at the type level that says "for this adapter, platform_id IS the channel."

Fix: make the contract explicit on ChannelMessage itself.

• New pub const CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL: &[ChannelType] — single source of truth listing every adapter where platform_id semantically is the channel id (Discord, Slack, Telegram, Matrix, Mattermost, Teams, Webex, Rocket.Chat, Nextcloud, Pumble, Revolt, Guilded, Feishu, Lark, Keybase, Google Chat, LINE, Twist, Flock, Twitch — 19 adapters per current code).
• New ChannelMessage::channel_id() -> Option<&str> — checks the allowlist, falls back to metadata[\"channel_id\"], else None. Case-folds ChannelType::Custom(_) so user-defined adapter names with mixed casing don't silently miss.

Result: deterministic. No reliance on whether a given bridge happened to populate sender_user_id. Coverage extends to adapters the heuristic would have missed.

3. binding_context_for should carry both channel_id AND guild_id

d336314's dispatch path calls router.resolve_with_channel_id(channel_type, platform_user_id, user_key, sender_channel_id(message)). The new BindingContext carries channel_id but guild_id is dropped — so a binding written as match_rule = { guild_id = \"...\", channel_id = \"...\" } will only match on channel_id, and guild_id-only bindings on the channel-aware path stop working.

Fix: introduce binding_context_for(message) -> BindingContext in bridge.rs that builds the full context (both fields populated from message.channel_id() and message.metadata). Both dispatch_message and dispatch_with_blocks route through it.

4. Startup validation warnings (small but useful)

At config load, warn (not error) when:

• A binding sets channel_id but no channel (almost always a mistake).
• A binding sets channel_id for an adapter not in CHANNELS_WITH_PLATFORM_ID_AS_CHANNEL (the rule will never match).

Catches misconfig at boot rather than at first dispatched message.

5. Documentation

docs/channel-adapters.md updated to:

• Promote bindings to step 1 in the routing-resolution order.
• Distinguish peer_id vs channel_id (different specificity weights, different semantics).
• Publish the specificity table (peer_id=8, channel_id=8, guild_id=4, account_id=2, channel=2, roles=1, max=25).
• List the adapter allowlist with the metadata[\"channel_id\"] escape hatch for adapters not on it.

Expected Behavior

• Typo'd binding wrappers (match_rules plural, etc.) fail loudly at config load.
• channel_id matching works deterministically across all adapters where it semantically applies — not just the ones whose bridges currently populate sender_user_id.
• Bindings combining guild_id and channel_id match on both.
• Misconfigured bindings produce a warning at startup, not silent no-op at runtime.

Steps to Reproduce (current behavior)

1. Wrapper typo: Write a binding with match_rules = { ... } (plural). Config loads. Binding has no rules. Matches everything. No warning.
2. Heuristic gap: Use any non-Discord/Slack adapter where platform_id is the channel id but the bridge does not populate metadata[\"sender_user_id\"]. channel_id rules never match.
3. Guild drop: Write a binding with match_rule = { guild_id = \"G\", channel_id = \"C\" }. On the dispatch path, guild_id is not part of the context — only channel_id is checked.

OpenFang Version

v0.6.2 (head 15ed29c), against d336314.

Operating System

Reproduced on macOS 14.x (Apple Silicon). Not OS-dependent — pure config/routing logic.

Logs / Screenshots

N/A — this is a configuration/routing semantics issue. Tests in the accompanying PR cover all four behaviors.

Test coverage in the proposed PR

• crates/openfang-channels/src/types.rs — +1 test (allowlist + case-folding).
• crates/openfang-channels/src/bridge.rs — +8 tests (allowlist match, metadata fallback, Custom(...) case-folding, guild+channel combo, etc.).
• crates/openfang-types/src/config.rs — +5 tests (wrapper-level deny_unknown_fields, validation warnings, channel_id-without-channel, allowlist negative case, default on every Option).

Files touched

• crates/openfang-types/src/config.rs
• crates/openfang-channels/src/types.rs
• crates/openfang-channels/src/bridge.rs
• docs/channel-adapters.md

Note for reviewers

Each of items 1–4 is independently mergeable. If any one of them is contentious, it can be cherry-picked or dropped without disturbing the others. Item 1 is a one-liner with obvious value; item 2 is the largest change and the one most worth scrutinizing.