fix(runtime): channel_send without recipient replies to the group, not the speaker#6261
Conversation
github-actions
Bot
added
size/M
d0d64ea to
d90c568
Compare
github-actions
Bot
added
has-conflicts
A no-recipient channel_send auto-filled the recipient from sender_id (the individual who spoke) instead of sender_chat_id (the room / group). In a group the send then targeted the speaker's user id as if it were a conversation — e.g. a Matrix file send routed to @user:hs rather than the room !room:hs, which the homeserver rejected with 403 not in room. That failure was visible only on the sidecar's stderr, so the tool still reported success and the agent believed the message was delivered. The librefang#6117 cross-chat guard already computed the correct target (sender_chat_id first, sender_id only as a DM fallback) but the send fallback disagreed, using the raw sender_id. Unify both through a shared resolve_send_target helper so the auto-filled target is exactly the expected_chat the guard validates — a no-recipient send can no longer resolve a chat the guard would reject. Adds unit tests for the group, DM, explicit-recipient, guard-parity, and no-identity cases.
d90c568 to
7e85ffd
Compare
|
Rebased onto |
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
LGTM (rebased onto current main to clear the .secrets.baseline conflict; reviewed independently). Correct fix: a no-recipient channel_send in a group was auto-filling the recipient from sender_id (the speaker) instead of sender_chat_id (the room), so sends routed to the speaker's user id and silently failed (e.g. Matrix 403 not-in-room). The PR unifies the auto-fill and the #6117 guard's expected_chat through one resolve_send_target helper so the two fallbacks can't disagree, with 5 new unit tests. CI is fully green.
2 participants
Problem
A
channel_sendcall without an explicitrecipientis meant to reply to the current conversation. In a group/room it instead targeted the speaker rather than the room.tool_channel_send(crates/librefang-runtime/src/tool_runner/channel.rs) auto-filled the recipient fromsender_id(the individual who spoke), while the #6117 cross-chat dispatch guard a few lines below computed itsexpected_chatfromsender_chat_idfirst (sender_chat_id.filter(non-empty).or(sender_id)). The two fallbacks disagreed.In a group the speaker's id is not a conversation, so the send routed to it as if it were one. Concretely on Matrix a no-recipient file send resolved to the speaker MXID
@user:hsinstead of the room!room:hs; the homeserver rejected thePUT m.room.messagewith:The sidecar
Sendcommand is fire-and-forget (noResponse.errorcorrelation — the known long-term gap noted insdk/python/librefang/sidecar/runtime.py), so that 403 surfaced only on the sidecar's stderr. The tool returnedFile '…' sent to … via <channel>and the agent reported success while nothing was delivered.The explicit-recipient path was unaffected — the guard correctly rejects a mismatched explicit recipient; only the implicit (auto-fill) path resolved a target the guard never validated.
Fix
Resolve the send target and the guard's
expected_chatthrough one shared helper,resolve_send_target(explicit, sender_chat_id, sender_id):sender_chat_id— room / group / chat);sender_id(DM, where the chat id coincides with the sender or none is stamped).Because the auto-fill now returns exactly what the guard treats as canonical, a no-recipient send can no longer resolve a chat the guard would reject. DM replies are unchanged (they fall through to
sender_id). Out-of-band callers (cron / trigger, no identity stamped) still surface the existing "Missing 'recipient'" error.This is purely the recipient-resolution defect. The separate fire-and-forget "phantom success" gap (a failed sidecar send still reports success) needs the
Response.errorprotocol change already tracked inruntime.pyand is not in scope here.Changes
resolve_send_targethelper unifies the send fallback with the guard'sexpected_chat.Verification
cargo check -p librefang-runtime --lib— clean.cargo clippy -p librefang-runtime --lib— no warnings.cargo fmt -p librefang-runtime -- --check— clean.cargo test -p librefang-runtime --lib channel::tests— 5/5 pass:auto_fill_replies_to_group_not_speaker(the regression: room over speaker),auto_fill_falls_back_to_sender_for_dm,explicit_recipient_wins,auto_fill_matches_guard_expected_chat(send target ≡ guard expectation),no_identity_resolves_none.