Skip to content

fix: Tombstone MDN before sending it (#8252)#8257

Draft
iequidoo wants to merge 2 commits into
mainfrom
iequidoo/tombstone-mdn-before-sending
Draft

fix: Tombstone MDN before sending it (#8252)#8257
iequidoo wants to merge 2 commits into
mainfrom
iequidoo/tombstone-mdn-before-sending

Conversation

@iequidoo

@iequidoo iequidoo commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Otherwise, when it appears on IMAP, it will mark chat messages as seen/noticed even if
markfresh_chat() is called meanwhile.

test: Add function to queue MDN into smtp is extracted from #8008 , better to be reviewed there.
Fix #8252

@iequidoo iequidoo marked this pull request as ready for review May 20, 2026 13:14
@iequidoo iequidoo requested review from WofWca and link2xt May 20, 2026 13:14

@WofWca WofWca left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sounds like this should work but I'm probably not the right person to ask.

IDK about introducing different behavior for tests though, it doesn't seem like something we often do at least. At least some comments about could be useful? But maybe it's obvious to someone who knows what they're reading.

@iequidoo

iequidoo commented May 20, 2026

Copy link
Copy Markdown
Collaborator Author

IDK about introducing different behavior for tests though, it doesn't seem like something we often do at least.

The smtp code (send_mdn_rfc724_mid() particularly) isn't tested at all in Rust currently. It's better to test it somehow than not test it at all, and running an SMTP server in Rust tests isn't what we want.

@link2xt

link2xt commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

IDK about introducing different behavior for tests though

This is already discussed in #8008, we indeed don't want to have conditional compilation or hooks just for tests. Just checking that MDN is in smtp_mdns table would be enough.

Comment thread src/smtp.rs
SendResult::Success => {
if !recipients.is_empty() {
info!(context, "Successfully sent MDN for {rfc724_mid}.");
message::insert_tombstone(context, &rendered_msg.rfc724_mid).await?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the actual fix and it is likely good as long as other code is not changed just for testing.

@iequidoo iequidoo force-pushed the iequidoo/tombstone-mdn-before-sending branch from 44261e2 to 842d1f4 Compare June 14, 2026 01:42
@iequidoo iequidoo marked this pull request as draft June 14, 2026 01:43
Otherwise, when it appears on IMAP, it will mark chat messages as seen/noticed even if
markfresh_chat() is called meanwhile.
@iequidoo iequidoo force-pushed the iequidoo/tombstone-mdn-before-sending branch from 842d1f4 to 48b7697 Compare June 14, 2026 01:50
@iequidoo

Copy link
Copy Markdown
Collaborator Author

For the record, i've removed conditional compilation, but just in case if changes still look complex, i'm going to replace the changes to smtp and the exisitng Rust tests with a JSON-RPC Python test, it should fail because MDNs aren't appearing on IMAP instantly.

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.

markfresh_chat() (Mark as Unread) doesn't work sometimes, because of late self-sent MDNs

3 participants