feat(channels): deterministic inbound dispatch — channel-instance binding lookup (#5671 Model A)#6131
Merged
Merged
Conversation
added 3 commits
June 16, 2026 16:00
…tch (#5671) Adds migration v44 with two SQLite tables backing Model A inbound dispatch and a `ChannelBindingStore` over them. `channel_instance_defaults` holds one row per `[[sidecar_channels]]` instance (the default agent, seeded from config at boot). `conversation_bindings` holds per-`(instance, conversation)` overrides written by the forthcoming `/agent` command. Both store the agent *name* rather than the per-spawn `AgentId` uuid, since config and the agent registry resolve agents by stable name. `ChannelBindingStore::resolve(instance, conversation_id)` performs the two-level lookup — conversation override first, then instance default, then `None` — that replaces the non-deterministic `list_agents().first()` fallback in the channel bridge. Wired into `MemorySubstrate` with a `channel_bindings()` accessor. Covered by 7 unit tests including the per-instance scoping guard (#5672 cross-bot leak) and override-wins-over-default precedence.
After the memory substrate opens, seed `channel_instance_defaults` from every `[[sidecar_channels]] agent`, giving the deterministic two-level dispatch lookup its first level. Config is the source of truth for the default, so this re-seeds (upsert) on every boot; per-conversation `/agent` overrides live in a separate table and are left untouched. An instance with no `agent` is skipped — its traffic keeps falling through to the legacy resolver chain until the operator sets one, so this is non-breaking for existing deployments.
Replaces the topology-blind agent-selection logic in `resolve_or_fallback` with a deterministic lookup keyed by `(instance, conversation)`: `instance` is the sidecar's config `name` (stamped onto inbound messages as `metadata["account_id"]`), and `conversation_id` is uniformly `sender.platform_id` — the chat id for groups, the peer id for DMs. A configured channel-instance binding (per-conversation override, else instance default) now wins outright; the operator bound this agent, so neither the legacy chain nor the channel allowlist gets a vote. This removes the production DM-rotation bug where two turns of the same DM resolved to different agents (and different sessions) once the 5-minute thread-ownership claim expired and selection fell to the non-deterministic `list_agents().first()` branch. The legacy chain is kept as a transitional fallback for instances with no binding configured, now emitting a deprecation WARN at the non-deterministic tail so operators can see they should set `agent` on the instance. The HITL/AITL engagement gate (`should_process_group_message`) and the send path are untouched — engagement still runs after selection. New `ChannelBridgeHandle::resolve_bound_agent` defaults to `None` (so existing test doubles and the legacy path are unaffected); the kernel adapter overrides it with the DB-backed lookup, resolving the bound agent name to an `AgentId` and falling back gracefully when the name no longer maps to a live agent. Two dispatch precedence tests cover bound-wins and no-binding-falls-through.
github-actions
Bot
added
area/channels
…ine breaks All new `///` / `//!` blocks in channel_binding_store.rs and bridge.rs were wrapped at ~80 columns, breaking mid-sentence. CLAUDE.md requires line breaks only at sentence boundaries.
houko
commented
Jun 16, 2026
houko
left a comment
houko
left a comment
Contributor
Author
There was a problem hiding this comment.
crates/librefang-channels/src/bridge.rs — binding key for group messages (resolve_or_fallback)
The dispatch comment and the PR body both state:
conversation_idis the chat id for groups and the peer id for DMs — uniformlymessage.sender.platform_id
But in a Telegram group, sender.platform_id is the sender's user ID, not the group chat ID.
Using it as the binding key would mean two different users in the same group resolve as two different conversations, and a binding seeded for the group-chat ID would never match any inbound message.
For DMs the choice is correct (sender.platform_id == peer ID == the conversation).
Could you confirm one of the following:
- The Telegram (and other) adapters intentionally populate
sender.platform_idwith the chat ID for group messages (i.e. not the sender's user ID), making the field uniform across DMs and groups? - Or the group case should use a different field — e.g. something from
message.metadata, or a forthcomingchat_idfield — and the current code is a latent bug that only goes unnoticed because configured instances with[[sidecar_channels]] agentset are always DM-only in practice?
The two new tests cover only is_group: false, so neither CI nor the test suite catches the group path today.
If groups simply can't reach the bound path (because the engagement gate keeps them silent before resolve_or_fallback is called), a short comment at that call site would make the intent explicit and prevent a future reader from "fixing" the field to the group chat ID inadvertently.
Generated by Claude Code
…_agent (#5671) Verifies the real kernel adapter override reads the channel-binding store and resolves the bound agent name to a live AgentId against a fully-booted test kernel. The trait default returns None, so a missing or incorrect override would silently disable deterministic dispatch while the bridge's own mock-handle unit tests stay green — this guards that default-None-disables-feature trap. Also covers the no-binding (fall-through) and dangling-name (graceful None) paths.
github-actions
Bot
added
ready-for-review
# Conflicts: # crates/librefang-channels/src/bridge.rs
github-actions
Bot
added
ready-for-review
added 2 commits
June 17, 2026 16:58
…spatch Reorder resolve_or_fallback so explicit #5323 group addressing (@-mention / alias, then sticky holder) runs before the #5671 channel-instance binding lookup. New precedence: adapter thread-route > @-mention > binding > legacy fallback chain. A fresh @-mention now overrides a configured instance binding, while the binding still applies to otherwise-unaddressed traffic and wins over the router/assistant/first-available fallback. Update the precedence doc comment to match the new order and add a test asserting an in-group @-mention of a different agent overrides the binding.
…) and default (below) Resolve_or_fallback previously consulted a single collapsed binding lookup. The two binding levels carry different authority: an explicit per-conversation /agent override is a deliberate user command, while the instance default seeded from [[sidecar_channels]] agent is a weak standing fallback. Ranking them as one block forced a choice between letting the instance default steamroll an in-flight #5323 sticky conversation or letting a stale sticky holder ignore an explicit /agent. Split the ChannelBridgeHandle trait method resolve_bound_agent into resolve_conversation_override and resolve_instance_default, backed by the store's already-distinct conversation_binding / instance_default getters. New dispatch precedence: thread-route > @mention (#5323) > explicit /agent override > sticky holder (#5323) > instance default > legacy fallback. Update the precedence doc comment to the 6-tier order, and add tests pinning the split: sticky holder outranks the instance default, the explicit override outranks the sticky holder, and the kernel injection-site test now seeds both levels through the real substrate.
github-actions
Bot
added
size/XL
# Conflicts: # crates/librefang-memory/src/migration.rs
github-actions
Bot
added
ready-for-review
houko
enabled auto-merge (squash)
June 17, 2026 08:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Implements the Model A dispatch rewrite from the #5671 RFC: channel inbound routing becomes a deterministic two-level table lookup, removing the production DM-rotation bug. This is PR-B/C/D of the RFC rollout; PR-A (the
SidecarChannelConfig.agent/available_agentsschema fields) already landed in #6105.The bug being fixed
resolve_or_fallbackapplied the same topology-blind priority chain to every inbound message. With no binding, no"assistant"agent, and an expired thread-ownership claim (5-minute TTL), agent selection fell tolist_agents().first()— non-deterministic byHashMapiteration order. Two turns of the same DM, sent hours apart, resolved to different agents with different sessions (no shared history). The RFC has the production journal showing a Telegram DM rotatingresearcher→legal-assistantacross two messages.A DM has one participant talking to one agent — there is nothing for a resolver to decide. The fix makes selection a table lookup instead of a chain.
Changes (three atomic commits)
feat(memory)— migration v44 addschannel_instance_defaults(one row per[[sidecar_channels]]instance) andconversation_bindings(per-(instance, conversation)override). NewChannelBindingStoreover both;resolve(instance, conversation_id)does the two-level lookup (override → instance default →None). Stores the agent name, not the per-spawnAgentIduuid, since config and the registry resolve by stable name. Wired intoMemorySubstrate.feat(kernel)— at boot, seedchannel_instance_defaultsfrom each[[sidecar_channels]] agent. Re-seeds (upsert) every boot since config owns the default; per-conversation overrides are untouched.feat(channels)—resolve_or_fallbackconsults the lookup first, keyed byinstance = metadata["account_id"]andconversation_id = sender.platform_id(uniform: chat id for groups, peer id for DMs). A configured binding wins outright. NewChannelBridgeHandle::resolve_bound_agent(defaultNone, kernel adapter overrides with the DB lookup).Scope decisions
WARNat the non-deterministiclist_agents().first()tail. Configured instances become deterministic; unconfigured ones keep today's behaviour. This is the migration story raised in the RFC review rather than a hard delete.AgentManifest.channelsas a dispatch filter, and the deterministic DB lookup replaces the router'sset_user_defaultcache (which only existed to paper over the non-deterministic chain). The operator's explicit[[sidecar_channels]] agentbinding is authoritative.should_process_group_message) and the send path are unchanged — engagement still runs after selection, so groups stay silent-by-default and DMs always engage./agentslash command (the only writer ofconversation_bindings— the table lands empty but read-active), the REST mirror, and the dashboard UI.Collision to resolve
#4961is a DRAFT addingAgentManifest.channelsas a dispatch filter — that is Model B, which this PR's direction deprecates. It should be redirected before it lands, per the RFC thread.Verification
cargo test -p librefang-memory— 7 newChannelBindingStoretests (per-instance scoping / bug(channels): channel-side /commands ignore account_id — multi-bot deployments route commands to wrong agent (and /agent leaks user-defaults across bots) #5672 leak guard, override-wins-over-default, fall-through) + 31 migration tests pass.cargo test -p librefang-channels—bound_agent_wins_over_legacy_fallbackandno_binding_falls_through_to_legacy_chainpass.cargo check -p librefang-kernel -p librefang-api --lib— clean.cargo clippy -p librefang-memory -p librefang-channels -p librefang-kernel -p librefang-api --all-targets -- -D warnings— zero warnings.cargo fmtapplied.Refs #5671. Supersedes the inbound-routing portion of #5323; eliminates the #5294 layers b/c non-determinism by construction.