Skip to content

fix(mobile): tag DM recipients on send - #2863

Closed
Baltsat wants to merge 1 commit into
block:mainfrom
Baltsat:fix/2835-mobile-dm-recipient-p-tag
Closed

fix(mobile): tag DM recipients on send#2863
Baltsat wants to merge 1 commit into
block:mainfrom
Baltsat:fix/2835-mobile-dm-recipient-p-tag

Conversation

@Baltsat

@Baltsat Baltsat commented Jul 25, 2026

Copy link
Copy Markdown

Summary

Mobile DM sends only serialized p tags from picker-resolved mentions, so a plain DM could reach the relay without addressing the counterparty. This adds the known DM participants to the existing mention-normalization path.

Changes

  • append DM participant pubkeys after explicit mentions
  • reuse the existing case-insensitive dedupe and self-exclusion logic
  • pass DM participants from both the channel composer and thread composer
  • preserve existing tag order: h, thread e tags, recipient p tags, then media tags
  • add focused coverage for plain DM sends, mixed-case dedupe/self exclusion, non-DM behavior, and threaded media ordering

Verification

  • dart format --output=none --set-exit-if-changed on all four changed files
  • git diff --check
  • Flutter analysis and tests were not available in the local environment; GitHub Actions is the integration gate

AI assistance: ChatGPT/Codex was used for implementation. The generated patch was independently reviewed, corrected, and reduced to one product commit before publication.

Closes #2835

@Baltsat
Baltsat requested a review from a team as a code owner July 25, 2026 17:16
Copilot AI review requested due to automatic review settings July 25, 2026 17:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Baltsat
Baltsat force-pushed the fix/2835-mobile-dm-recipient-p-tag branch from b58c4ab to 8c77451 Compare July 25, 2026 17:23
Comment thread .github/workflows/validate-issue-2835.yml Fixed
Include DM participants in outgoing mobile p-tags while preserving explicit mention handling and thread/media tag order.

Closes block#2835

Signed-off-by: Konstantin Baltsat <[email protected]>
@github-actions
github-actions Bot force-pushed the fix/2835-mobile-dm-recipient-p-tag branch from 3dc0c48 to af2216f Compare July 25, 2026 17:58
@svndco

svndco commented Jul 26, 2026

Copy link
Copy Markdown

Independent review

I checked this out at af2216fbf6e04b7b3b8e182502789cc0b0abe8d9 and ran the full mobile package gates:

  • flutter test: 589 passed, 1 skipped
  • just mobile-check: formatting, analysis, and file-size checks passed

The send-path wiring covers both the channel composer and DM thread composer, and the regression tests correctly cover plain DMs, self exclusion/deduplication, non-DM behavior, and threaded tag ordering.

One consistency fix is worth folding in before merge: normalizedMentions deduplicates using pk.toLowerCase() but emits the original pk, despite the comment saying this matches desktop normalization. Desktop's normalizeMentionPubkeys emits the lowercase value, and this path also retains an empty pubkey if supplied. PR #2927 has the tighter loop (lower.isNotEmpty and emit lower). I recommend adopting that small normalization piece here, retaining this PR's stronger threaded/media-tag tests, and then closing #2927 as the duplicate.

Baltsat commented Jul 28, 2026

Copy link
Copy Markdown
Author

Closing this in favor of #3258, which now covers the same recipient-tag behavior at the central send path with dedicated wire tests. Keeping both implementations open would duplicate review effort. Thanks for the review and guidance here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mobile: DM omits recipient p-tag unless a mention is picked, so agents are never woken

4 participants