feat(mobile): p-tag all DM participants so agents respond to plain DMs - #3258
feat(mobile): p-tag all DM participants so agents respond to plain DMs#3258zacharytamas wants to merge 4 commits into
Conversation
Match desktop's DM contract: kind-9 messages published in a DM channel now carry ["p", pk] tags for every other DM participant (union of explicit @mentions and the channel's participantPubkeys, minus the sender, deduplicated), so the buzz-acp harness's mention-gated #p subscription sees plain DM messages and agents respond without an @mention. The fan-out lives in SendMessage.call — mobile's single publish-time chokepoint for kind-9 — via a new injected readChannel closure wired from the cached channelsProvider state (synchronous; never blocks a send). If the channel isn't cached yet or isn't a DM, behavior is unchanged. Non-DM channels keep explicit-mentions-only semantics. Wire-level tests assert the p tags on the recorded signed event for plain DM sends, non-DM sends, and the not-yet-cached fallback. Signed-off-by: Zachary <[email protected]>
…d thread replies Two wire-level tests on SendMessage: a case-mismatched explicit mention overlapping a DM roster entry dedupes to a single p tag, and a DM thread reply carries the participant fan-out alongside its root/reply e-tags. Signed-off-by: Zachary <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98d370395b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
98d3703 to
4808153
Compare
Baltsat
left a comment
There was a problem hiding this comment.
The central SendMessage path is a cleaner place for this behavior. Two concrete points from the earlier review on #2863 still appear to apply here:
seenMentions.add(pk.toLowerCase())deduplicates case-insensitively, but emits the originalpkand also accepts an empty string.svndcoasked for canonical lowercase output and empty-value filtering; the current tests do not cover either case.- When
_readChannel(channelId)returnsnull, a plain DM still emits no recipientptags. Is that cache-miss behavior acceptable, or should this path have an authoritative fallback so the original #2835 failure cannot recur during provider loading?
It would also help to link/close #2835 in the PR body. I closed #2863 to avoid competing production patches.
Signed-off-by: Zachary <[email protected]>
|
Addressed both review points in
Verification: focused provider suite 8/8; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1750a15fcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c0cbff5a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .read(sendMessageProvider) | ||
| .call( | ||
| channelId: channelId, | ||
| channel: channel, |
There was a problem hiding this comment.
Send thread replies with the refreshed DM roster
When DM membership changes after this thread page is pushed—for example, a participant or agent is added from desktop—channelsProvider refreshes and rebuilds this widget, but the send path still passes the constructor snapshot channel. SendMessage therefore reads stale participantPubkeys, so plain thread replies omit the new participant's p tag indefinitely until the user closes and reopens the thread. Resolve the current channel from the already-watched channelsAsync, with the route channel only as a fallback, before sending.
Useful? React with 👍 / 👎.
Signed-off-by: Zachary <[email protected]>
0c0cbff to
7c9fa80
Compare
|
Scope correction: the earlier route- |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c9fa80bee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| channel = await _readChannel(channelId); | ||
| } catch (_) { | ||
| channel = null; |
There was a problem hiding this comment.
Preserve the DM roster when channel lookup fails
When _readChannel throws or returns null while a DM route remains usable—for example, during a channelsProvider refresh error—this fallback treats the DM as a non-DM and publishes a plain message without any p tags. ACP agents subscribe through #p, so the send appears successful but the agent never receives or responds to it; the added failure tests only exercise explicit mentions, which mask this behavior. Fresh evidence relative to the earlier lookup discussion is that the current catch-and-null path now explicitly swallows the failure and selects an empty roster, so the route-resolved Channel should be supplied to preserve DM fan-out.
Useful? React with 👍 / 👎.
What problems was I solving
On desktop, DMing an agent "just works": a plain message p-tags every other DM participant, and the buzz-acp harness's mention-gated
#psubscription treats those tags as the trigger. Mobile only tagged explicit@mentions, so a DM'd agent stayed silent unless the sender selected it from the mention picker.After this PR, mobile resolves the current channel before publishing and adds every other DM participant as a
ptag. A plain, unmentioned DM therefore wakes the recipient agent when channel state is available.What user-facing changes did I ship
etags alongside recipientptags.channelsProvider.future, preventing initial loading from silently dropping DM recipients.Implementation
The change remains centralized in two files:
mobile/lib/features/channels/send_message_provider.dartmobile/test/features/channels/send_message_provider_test.dartNo channel-page, thread-navigation, message-row, or action-sheet interfaces are changed.
Verification
Closes #2835