This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.
You reply to an email notification from your team chat β a quick "sounds good" to a thread you missed. It never arrives. No bounce, no error, no red anything. And the server's own logs report, cheerfully, "Successfully processed email."
That reply had already been thrown away. And it kept happening for a full year β in a tool thousands of organizations trust β while six people tried and failed to fix it.
TL;DR β Replying by email to a group-DM notification silently dropped your message whenever a group member had been deactivated (while logging success). The fix is ~15 lines; the PR is #39766, green across Zulip's CI matrix. Sentry's Seer independently reconstructed my diagnosis from one captured event, and Gemini stress-tested my test coverage.
Project Overview
The feature at the center of this is Zulip's email gateway β Zulip being the open-source team chat thousands of organizations run themselves. Miss a message, get a notification email, and just reply to it: your reply is parsed and dropped straight back into the conversation. Brilliant β until the conversation is a group DM and one of its members has since been deactivated. Then your reply vanishes, and the log lies about it.
Bug Fix or Performance Improvement
Issue #35273, opened by a Zulip core maintainer in July 2025 β one year old this week. When someone replies by email to a group-DM notification and any member of that group has been deactivated, message validation throws:
django.core.exceptions.ValidationError:
["'deactivated-recipient@example.com' is no longer using Zulip."]
...
zerver.lib.exceptions.JsonableError:
'deactivated-recipient@example.com' is no longer using Zulip.
The exception is caught deep in the internal send path and logged β so nothing crashes, nothing retries, and the sender is never told. The reply is simply gone. And here's the lie from the intro β my favorite detail of the whole bug. Right after eating your message, the mirror logs
INFO Successfully processed email from user 8 to ...
A success log for a message that was never delivered. That's the worst class of bug there is: silent data loss that every party β sender, server, and logs β believes succeeded. Nobody files a report for a message they don't know they lost, which is exactly why it hid in a shipping product for a year.
The web app can't hit this β it refuses to compose to a group containing a deactivated user at all. Only the email path, replying to a notification that predates the deactivation, can walk into it.
The bug, by the numbers: open 1 year Β· 6 contributors claimed it and drifted off Β· 4 PRs opened, 0 merged Β· the fix is ~15 lines of logic + 3 new tests Β· 75/75 email-mirror tests green Β· CI green on 5 Python versions (3.10β3.14).
Behind those numbers is a pattern. Every one of the six claimants was auto-unassigned for inactivity; none of the four PRs cleared review. The most promising got a "Looks great!" from a maintainer with a short punch-list β use .values_list("email", flat=True), add a test where the conversation remains a group DM, extract the duplicated test setup into a helper, and also handle the 1:1 DM case β and then stalled in commit-style churn. So before writing a line, I read all four attempts and that review, and treated the punch-list as my spec.
The reading paid off with the key insight: the "1:1 case" no longer exists. Since the earlier attempts, Zulip finished migrating all direct messages β including 1:1s β to direct-message groups and dropped personal recipient rows entirely. The separate 1:1 fix the reviewer asked for in January is now structurally impossible and structurally unnecessary: one fix in the group branch covers everything.
Code
[ai] email_mirror: Ignore deactivated users in group DM email replies.
#39766
Fixes: #35273
Replying by email to a message notification email for a group direct message raised an unhandled JsonableError when any member of the group had since been deactivated, and the reply was silently lost (while the mirror still logged "Successfully processed email").
This filters the group's members to active users when constructing the recipient list, so the reply is delivered to the remaining active participants β consistent with the web app, which does not offer composing to such a group at all. If nobody but the sender remains active, the reply is dropped with an INFO log rather than an exception.
Two notes for review:
- Since personal
Recipientrows no longer exist (1:1 direct messages are direct message groups), the same branch covers the 1:1 case raised in the review of #37449; a separate commit for it is no longer meaningful. - The filtering query adds one database query to the reply path, so the query-count assertions in the two existing tests are updated from 22 to 23.
A possible follow-up (not included, to keep this scoped like the previously reviewed approach): notifying the sender via the notification bot when the reply is dropped, as send_mm_reply_to_stream does for channel replies.
Prior work on this issue: #36882, #37449 (whose review feedback is addressed here: values_list-based filtering, a test where the conversation remains a group DM, and the repeated test setup extracted into helpers), #38117, #38933.
How changes were tested:
- [x]
./tools/test-backend zerver.tests.test_email_mirror.TestMissedMessageEmailMessagesβ 12/12 pass (re-run after rebasing on current main); the three new tests fail without the lib change (silent message loss, no drop log). - [x]
./tools/lint zerver/lib/email_mirror.py zerver/tests/test_email_mirror.py - [x] Coverage:
zerver/lib/email_mirror.pyshows no uncovered lines under this test class. - [x] Reproduced end-to-end in the dev environment (group DM β deactivate a member β email reply): before the fix the reply is lost; after, it is delivered to the remaining active members.
Screenshots and screen captures:
N/A (backend-only change).
Self-review checklist
- [x] Self-reviewed the changes for clarity and maintainability (variable names, code reuse, readability, etc.).
- [x] Followed the AI use policy.
Communicate decisions, questions, and potential concerns.
- [x] Explains differences from previous plans (e.g., issue description).
- [x] Highlights technical choices and bugs encountered.
- [x] Calls out remaining decisions and concerns.
- [x] Automated tests verify logic where appropriate.
Individual commits are ready for review (see commit discipline).
- [x] Each commit is a coherent idea.
- [x] Commit message(s) explain reasoning and motivation for changes.
Completed manual review and testing of the following:
- [ ] Visual appearance of the changes.
- [ ] Responsiveness and internationalization.
- [ ] Strings and tooltips.
- [x] End-to-end functionality of the changes.
- [ ] Accessibility.
- [ ] Theming.
The whole change is about two dozen lines in zerver/lib/email_mirror.py (roughly half of them comments): drop the group's deactivated members before constructing the recipient list, deliver to whoever remains, and if nobody but the sender is left, log and drop instead of throwing. Here's the logic, comments elided:
elif recipient.type == Recipient.DIRECT_MESSAGE_GROUP:
display_recipient = get_display_recipient(recipient)
user_ids = [user_dict["id"] for user_dict in display_recipient]
emails = list(
UserProfile.objects.filter(id__in=user_ids)
.exclude(is_active=False, is_mirror_dummy=False)
.order_by("id")
.values_list("email", flat=True)
)
if len(emails) <= 1:
logger.info(
"Dropping message notification email reply from user %s to a group direct message with no active recipients",
user_profile.id,
)
return
recipient_str = ", ".join(emails)
internal_send_group_direct_message(user_profile.realm, user_profile, body, emails=emails)
Two subtleties in that query. The .exclude(is_active=False, is_mirror_dummy=False) drops deactivated members but keeps "mirror-dummy" users β inactive placeholder accounts that Zulip's own recipient validation still treats as valid recipients. A naive is_active=True filter gets that wrong and silently drops them; a reviewer caught exactly that, which is why the query mirrors the validation's rule instead. And .order_by("id") preserves the deterministic ordering get_display_recipient provides, since we no longer iterate it directly.
Plus three new backend tests (delivered-and-stays-a-group-DM, keeps-a-mirror-dummy-member, only-sender-remains-so-drop) and the test-helper refactor the reviewer asked for β extracted into its own preparatory commit, per Zulip's commit discipline. The filtering adds exactly one indexed query, so the two existing tests' query-count assertions moved from 22 to 23 β stated in the PR rather than hidden.
My Improvements
The discipline was: make the deactivation tests fail on main for the right reasons before making them pass (the third test guards the opposite direction β that a valid inactive "mirror-dummy" member is kept, not over-filtered). On unfixed code they fail exactly as production does β recipients' latest message is still the original group DM (the reply was eaten), and the drop-path test finds the lying "Successfully processed" log where the graceful drop should be:
AssertionError: Actual and expected outputs do not match; showing diff.
- original group direct message
+ Reply with deactivated group member body
With the fix: the whole test_email_mirror module is green (75/75, my three new tests included), ./tools/lint (including mypy) clean, and coverage shows zero uncovered lines in email_mirror.py under this suite β including the drop path.
Design choice worth surfacing: when members are filtered out, the reply is delivered to the reduced group (the active members' conversation), matching what the previously-reviewed attempt did and what the maintainers endorsed in-thread. And when nobody but the sender remains, we log-and-drop rather than crash; a follow-up could notify the sender via Zulip's notification bot, the way stream-reply failures already do β I flagged that in the PR rather than scope-creeping into it.
Best Use of Sentry
Zulip ships Sentry integration natively (zproject/sentry.py) β enabling it in the dev environment took exactly one environment variable, which made a live before/after demo of this bug almost embarrassingly easy.
The whole loop, start to finish; the detail on each step is below.
Before: I ran the real scenario end-to-end β group DM, deactivate a member, reply by email through process_message() β with the unfixed code and a Sentry DSN. The swallowed JsonableError arrives in Sentry as a first-class error event with the full traceback from the issue:
The trace pins it exactly: except ValidationError as e: β¦ raise JsonableError(e.messages[0]) in check_message β the reply is turned into an exception, caught upstream, logged, and dropped.
Seer: I connected my Zulip fork and pointed Sentry's Seer at the captured event. Its root-cause analysis independently landed on the same mechanism I'd fixed:
"The recipient list for the conversation still includes the deactivated user11, causing a
JsonableErrorand silently dropping the email reply."
That's exactly the bug β and Seer reconstructed the reproduction steps (create group DM β deactivate a member β reply by email) matching the ones I'd used, from a single captured event. A nice independent check on the diagnosis before shipping the fix.
After: identical scenario, fixed code β the reply is delivered to the remaining active members, the success log now lists only the active recipients, no new event is captured, and I marked the issue resolved (you can watch the whole beforeβafter in the recording above).
What sold me on the workflow: this bug's signature β error event + success log + missing message β is invisible in any one log file, but obvious the moment the exception becomes a first-class object in Sentry with grouping and history.
Best Use of Google AI
Green tests are a trap: they prove the cases you wrote pass, not the cases you forgot. So I handed Gemini 2.5 Flash (via the Google AI Studio API) the exact patch and my three test descriptions and asked the one question I can't ask myself objectively β what am I not testing?
It went straight for the boundary that worried me most, the len(emails) <= 1 drop path, and reasoned it out instead of hand-waving:
Sender is the only active member in a group DM β Correctly dropped and logged.
len(emails) <= 1correctly handles this, as the sender's email will be inemails.
That's the load-bearing assumption of the whole drop branch β the sender is always in the list, so "nobody left" means length 1, not 0. Then it flagged the email vs delivery_email question under restricted email-visibility β the exact trap I'd already hit building the repro, where the send path resolves recipients by .email, not .delivery_email β and correctly scoped it to the downstream function rather than crying wolf on my diff. It also named two recipient shapes my tests don't exercise (bot members, cross-realm members) and reasoned both already-correct, which is why I kept the PR scoped to the deactivation case instead of expanding it.
No bug fell out β and on a year-old issue that had already beaten six people, that was the win. Gemini bought me the thing you skip when the suite is green: a fast, adversarial second read that turns "my tests pass" into "I know exactly which cases I'm not covering, and why they're safe." The full prompt and response are saved alongside the code.
The takeaway
Six people had the right instinct on this issue; what was missing wasn't cleverness, it was finishing the job β reading the one review that already said exactly what to do, and acting on all of it. If you're hunting for a first open-source contribution, the highest-leverage move is often to pick up the issue everyone started and nobody closed. The trail of dead PRs isn't a warning sign β it's a spec.
Have you ever shipped (or inherited) a "Successfully processed" log line sitting right on top of a dropped message? Tell me the worst one in the comments. π
This is one of my two Summer Bug Smash entries. The other is a Smash Stories deep-dive β the data-loss bug I diagnosed six times in one evening β different chaos, same lesson about trusting your evidence over your theories.




Top comments (0)