Honor group-admin moderation deletes in timeline ingest#874
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (13)
WalkthroughThis PR adds admin-moderated message deletion. A ChangesAdmin moderation delete support
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Ready to review this PR? Stage has broken it down into 8 individual chapters for you: 
Chapters generated by Stage for commit f5757ff on Jul 17, 2026 6:04am UTC. |
Contributor
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/marmot-app/src/client/mod.rs (1)
1108-1190: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStale pre-send moderation grant can be locked in
record_local_app_event_projectionruns beforesync_runtime_groups().await?and again after publish for the sameevent.id, butcrates/storage-sqlite/src/timeline.rskeepsmoderation_grant = app_events.moderation_granton conflict. That means the first call can persist a stale grant and the later call cannot correct it if admin/member state changed in between.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/marmot-app/src/client/mod.rs` around lines 1108 - 1190, The pre-send call to record_local_app_event_projection in send_app_event_with_local_projection can persist a stale moderation grant before sync_runtime_groups refreshes state. Ensure the post-publish projection can replace the existing event’s moderation_grant with the current value, updating the app_events conflict/upsert handling rather than preserving the original grant unconditionally.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/marmot-app/src/client/mod.rs`:
- Around line 1108-1190: The pre-send call to record_local_app_event_projection
in send_app_event_with_local_projection can persist a stale moderation grant
before sync_runtime_groups refreshes state. Ensure the post-publish projection
can replace the existing event’s moderation_grant with the current value,
updating the app_events conflict/upsert handling rather than preserving the
original grant unconditionally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cfcad042-7d9e-4716-8186-c332860162f3
📒 Files selected for processing (14)
crates/marmot-app/src/client/mod.rscrates/marmot-app/src/client/projection.rscrates/marmot-app/src/client/sync.rscrates/marmot-app/src/conversions.rscrates/marmot-app/src/groups.rscrates/marmot-app/src/lib.rscrates/marmot-app/src/projection/tests.rscrates/marmot-app/src/tests.rscrates/storage-sqlite/src/account_projection/tests.rscrates/storage-sqlite/src/chat_list/tests.rscrates/storage-sqlite/src/migrations.rscrates/storage-sqlite/src/migrations/0027_app_event_moderation_grant.rscrates/storage-sqlite/src/timeline.rscrates/storage-sqlite/src/timeline/tests.rs
jgmontoya
reviewed
Jul 15, 2026
jgmontoya
previously approved these changes
Jul 16, 2026
mubarakcoded
force-pushed
the
fix/admin-delete-timeline-ingest
branch
from
July 17, 2026 06:04
b5761cb to
f5757ff
Compare
dannym-arx
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Timeline ingest previously honored a delete only when its author matched the target message's author, so a group admin deleting another member's message tombstoned it locally while every other member silently ignored it — and the deleter's own store resurrected the message on reprojection.
Design
The moderation verdict is evaluated once, at ingest, against the signed MLS group state, and persisted with the event:
app_events.moderation_grant(migration 0027) is stamped true only for a delete whose authenticated sender is in the group's admin set (admin_pubkeys, mirrored from signed group state) and whose group is not a direct (two-member, unnamed) conversation. Lookup failures fail closed to self-retraction semantics.delete.sender == row.senderOR the delete carries the persisted grant. Reaction retraction keeps its sender-equality forged-delete guard — moderation authority applies to messages, not reaction removal.Verification
just fast-cigreen (fmt, naming gate, check, clippy, workspace-wide including OTLP builds).cargo test -p storage-sqlite: 213 passed, including new coverage for admin tombstone via incremental projection, out-of-order delete-before-target delivery, full timeline rebuild preserving the moderated tombstone, and the reaction guard staying sender-equality.cargo test -p marmot-app: 331 passed, including grant-helper cases (direct conversations never grant, whitespace-only names still count as unnamed, named two-member groups do grant, empty admin set grants nothing).Closes #873
Summary by CodeRabbit