Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions crates/marmot-app/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use crate::{
AppMessageQuery, AppPerformanceTelemetry, AppQuarantinedGroup, AppRuntime, AppTransportRouting,
GroupInviteDeclineResult, MarmotApp, MarmotRelayPlane, MarmotRelayPlaneAccountAdapter,
MediaAttachmentReference, MediaDownloadResult, MediaUploadRequest, MediaUploadResult,
SendSummary, remember_seen_event, unix_now_seconds,
SelfMembership, SendSummary, remember_seen_event, unix_now_seconds,
};

mod audit;
Expand Down Expand Up @@ -690,20 +690,21 @@ impl AppClient {
self.remember_published_reports(&effects);
self.app.save_state(&self.state)?;
self.queue_own_group_system_projection_updates(&effects);
// A local leave / decline departs the group just like an observed
// self-removal does. The inbound `observe_account_device_effects` path
// suppresses the account unread aggregate for self-removal, but our own
// relay echoes are skipped, so the locally initiated departure must
// suppress here too. No-op if no `account_groups` row exists yet, so it
// never resurrects pruned projection state. This is the source-of-truth
// write for the account unread aggregate, so propagate its error (like
// the nearby projection writes) rather than swallow it: a silently
// failed update would leave `account_unread_total()` returning an
// inflated badge after a leave that otherwise reports success.
// A local leave / decline is a voluntary departure, recorded as `Left`
// so the chat list can distinguish it from an involuntary removal. The
// inbound `observe_account_device_effects` path records this for an
// observed self-removal, but our own relay echoes are skipped, so the
// locally initiated departure must record (and thereby suppress) here
// too. No-op if no `account_groups` row exists yet, so it never
// resurrects pruned projection state. This is the source-of-truth write
// for the account unread aggregate, so propagate its error (like the
// nearby projection writes) rather than swallow it: a silently failed
// update would leave `account_unread_total()` returning an inflated
// badge after a leave that otherwise reports success.
self.app.set_group_self_membership(
&self.state.label,
&hex::encode(group_id.as_slice()),
true,
SelfMembership::Left,
)?;
Ok(send_summary_from_effects(&effects))
}
Expand All @@ -720,12 +721,19 @@ impl AppClient {
/// For each row still carrying the default `'member'`, it asks the engine
/// for the group's roster (`runtime.members`, sourced from the Marmot
/// record's authoritative post-merge member set) and flips the row to
/// `'removed'` only when the call succeeds and the local account id is
/// `Removed` only when the call succeeds and the local account id is
/// definitively absent. Engine errors / unknown groups are skipped so
/// uncertainty never suppresses (matching the projection's existing
/// invariant). The work is gated behind a once-only account-import marker,
/// so subsequent opens are a single marker read and the hot path stays
/// projection-only.
///
/// A backfilled departure is recorded as `Removed`, not `Left`: roster
/// absence cannot tell us *why* the account is gone, and `Removed`
/// ("removed by someone") is the safer unknown bucket — claiming the user
/// voluntarily left a group an admin actually evicted them from is the more
/// misleading error. Both states suppress the unread aggregate identically,
/// so the choice only affects how the chat list labels the departure.
pub(crate) fn backfill_self_membership_once(&self) -> Result<(), AppError> {
if self
.app
Expand Down Expand Up @@ -753,8 +761,11 @@ impl AppClient {
continue;
};
if local_account_removed_from_roster(&members, &local_account_id_hex) {
self.app
.set_group_self_membership(&self.state.label, &group_id_hex, true)?;
self.app.set_group_self_membership(
&self.state.label,
&group_id_hex,
SelfMembership::Removed,
)?;
}
}
self.app.mark_account_import_complete(
Expand Down
88 changes: 75 additions & 13 deletions crates/marmot-app/src/client/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use crate::media::media_imeta_tags_are_valid;
use crate::notifications;
use crate::{
AppError, AppGroupAdminPolicyComponent, AppMessageProjection, SDK_DRAIN_WAIT,
SDK_FIRST_SYNC_WAIT, SyncSummary, TRANSPORT_CURSOR_MAX_FUTURE_SKEW, remember_seen_event,
unix_now_seconds,
SDK_FIRST_SYNC_WAIT, SelfMembership, SyncSummary, TRANSPORT_CURSOR_MAX_FUTURE_SKEW,
remember_seen_event, unix_now_seconds,
};

use super::AppClient;
Expand Down Expand Up @@ -479,12 +479,9 @@ impl AppClient {
self.sync_runtime_groups().await?;
}
if let cgka_traits::engine::GroupEvent::GroupStateChanged {
group_id,
change:
cgka_traits::engine::GroupStateChange::MemberRemoved { member }
| cgka_traits::engine::GroupStateChange::MemberLeft { member },
..
group_id, change, ..
} = event
&& let Some((member, membership)) = member_departure(change)
{
let group_id_hex = hex::encode(group_id.as_slice());
let member_id_hex = hex::encode(member.as_slice());
Expand All @@ -494,28 +491,36 @@ impl AppClient {
&member_id_hex,
);
// Only the local account leaving / being removed suppresses our
// own unread aggregate for the group; a peer removal must not.
// own unread aggregate for the group; a peer departure must not.
// The recorded membership distinguishes a voluntary `Left` from
// an involuntary `Removed` so the chat list can tell them apart.
// This projection write is the source of truth for the account
// unread aggregate, so propagate its error (matching the nearby
// timeline/message projection writes) instead of swallowing it:
// silently leaving the flag stale would keep
// `account_unread_total()` returning an inflated badge after a
// self-removal that sync otherwise reports as successful.
if member_id_hex.eq_ignore_ascii_case(&local_account_id_hex) {
self.app
.set_group_self_membership(&self.state.label, &group_id_hex, true)?;
self.app.set_group_self_membership(
&self.state.label,
&group_id_hex,
membership,
)?;
}
}
// A (re-)join or create restores the local account's membership so a
// re-add after removal un-suppresses the group's unread count. Same
// source-of-truth write as the removal path above: propagate the
// source-of-truth write as the departure path above: propagate the
// error rather than swallow it.
if let cgka_traits::engine::GroupEvent::GroupJoined { group_id, .. }
| cgka_traits::engine::GroupEvent::GroupCreated { group_id } = event
{
let group_id_hex = hex::encode(group_id.as_slice());
self.app
.set_group_self_membership(&self.state.label, &group_id_hex, false)?;
self.app.set_group_self_membership(
&self.state.label,
&group_id_hex,
SelfMembership::Member,
)?;
}
}
// Synthesize durable kind-1210 system rows from authenticated state
Expand Down Expand Up @@ -585,6 +590,63 @@ fn clamped_transport_cursor(
.unwrap_or(clamped)
}

/// Classify a group state change that ends a member's participation, returning
/// the departing member alongside how that departure should be recorded for the
/// member: a `MemberLeft` self-removal is a voluntary [`SelfMembership::Left`];
/// a `MemberRemoved` eviction by another member is [`SelfMembership::Removed`].
/// Returns `None` for changes that are not departures.
fn member_departure(
change: &cgka_traits::engine::GroupStateChange,
) -> Option<(&cgka_traits::MemberId, SelfMembership)> {
use cgka_traits::engine::GroupStateChange;
match change {
GroupStateChange::MemberLeft { member } => Some((member, SelfMembership::Left)),
GroupStateChange::MemberRemoved { member } => Some((member, SelfMembership::Removed)),
_ => None,
}
}

#[cfg(test)]
mod membership_change_tests {
use super::member_departure;
use crate::SelfMembership;
use cgka_traits::MemberId;
use cgka_traits::engine::GroupStateChange;

#[test]
fn member_departure_distinguishes_self_leave_from_eviction() {
let member = MemberId::new(vec![0xaa]);

// A SelfRemove proposal is a voluntary departure.
let left = GroupStateChange::MemberLeft {
member: member.clone(),
};
let (subject, membership) = member_departure(&left).expect("MemberLeft is a departure");
assert_eq!(subject, &member);
assert_eq!(membership, SelfMembership::Left);

// An eviction by another member is an involuntary removal.
let removed = GroupStateChange::MemberRemoved {
member: member.clone(),
};
let (subject, membership) =
member_departure(&removed).expect("MemberRemoved is a departure");
assert_eq!(subject, &member);
assert_eq!(membership, SelfMembership::Removed);
}

#[test]
fn member_departure_ignores_non_departures() {
let member = MemberId::new(vec![0xaa]);
let added = GroupStateChange::MemberAdded {
member: member.clone(),
};
let admin = GroupStateChange::AdminAdded { member };
assert!(member_departure(&added).is_none());
assert!(member_departure(&admin).is_none());
}
}

#[cfg(test)]
mod transport_cursor_tests {
use super::clamped_transport_cursor;
Expand Down
4 changes: 4 additions & 0 deletions crates/marmot-app/src/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ pub(crate) fn stored_group_from_app_group(group: &AppGroupRecord) -> StoredAccou
admin_keys_hex: group.admin_policy.admins.join(","),
archived: group.archived,
pending_confirmation: group.pending_confirmation,
// Ignored by the projection save (owned by `set_group_self_membership`);
// carried for struct completeness and round-trip symmetry.
self_membership: group.self_membership,
welcomer_account_id_hex: group.welcomer_account_id_hex.clone(),
via_welcome_message_id_hex: group.via_welcome_message_id_hex.clone(),
components: stored_components_from_app_group(group),
Expand Down Expand Up @@ -177,6 +180,7 @@ pub(crate) fn app_group_from_stored_group(
}
group.archived = stored.archived;
group.pending_confirmation = stored.pending_confirmation;
group.self_membership = stored.self_membership;
group.welcomer_account_id_hex = stored.welcomer_account_id_hex;
group.via_welcome_message_id_hex = stored.via_welcome_message_id_hex;
Ok(group)
Expand Down
7 changes: 6 additions & 1 deletion crates/marmot-app/src/groups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use cgka_traits::{GroupId, TransportEndpoint, TransportGroupSubscription};
use serde::{Deserialize, Serialize};

use crate::media::media_imeta_tags_are_valid;
use crate::{AccountState, AppError, ReceivedMessage, SendSummary, SyncSummary};
use crate::{AccountState, AppError, ReceivedMessage, SelfMembership, SendSummary, SyncSummary};

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
pub struct AppGroupRecord {
Expand All @@ -52,6 +52,10 @@ pub struct AppGroupRecord {
pub archived: bool,
#[serde(default)]
pub pending_confirmation: bool,
/// Whether the local account is still a member of this group, and if not,
/// whether it left voluntarily (`Left`) or was removed (`Removed`).
#[serde(default)]
pub self_membership: SelfMembership,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub welcomer_account_id_hex: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
Expand Down Expand Up @@ -302,6 +306,7 @@ impl AppGroupRecord {
encrypted_media: AppGroupEncryptedMediaComponent::disabled(),
archived: false,
pending_confirmation: false,
self_membership: SelfMembership::Member,
welcomer_account_id_hex: None,
via_welcome_message_id_hex: None,
}
Expand Down
6 changes: 3 additions & 3 deletions crates/marmot-app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub use relay_telemetry_export::{
};
pub use storage_sqlite::{
ChatListAvatar, ChatListMessagePreview, ChatListQuery, ChatListRow, MAX_TIMELINE_LIMIT,
TimelineMessageQuery, TimelineMessageRecord, TimelinePage, TimelinePagination,
SelfMembership, TimelineMessageQuery, TimelineMessageRecord, TimelinePage, TimelinePagination,
TimelineReactionSummary, TimelineReplyPreview, TimelineUserReaction,
};
pub use transport_nostr_adapter::{
Expand Down Expand Up @@ -1554,12 +1554,12 @@ impl MarmotApp {
&self,
account_ref: &str,
group_id_hex: &str,
removed: bool,
membership: SelfMembership,
) -> Result<(), AppError> {
let account = self.account_home().account(account_ref)?;
self.ensure_account_state(&account.label)?;
self.account_storage(&account.label)?
.set_group_self_membership(group_id_hex, removed)?;
.set_group_self_membership(group_id_hex, membership)?;
Ok(())
}

Expand Down
1 change: 1 addition & 0 deletions crates/marmot-app/src/runtime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,7 @@ fn chat_list_test_row(group_id_hex: &str, title: &str) -> ChatListRow {
last_read_message_id_hex: None,
last_read_timeline_at: None,
updated_at: 0,
self_membership: crate::SelfMembership::Member,
}
}

Expand Down
15 changes: 13 additions & 2 deletions crates/marmot-app/tests/relay_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use marmot_app::{
AuditLogSettings, AuditLogTrackerConfig, AuditLogUploadSource, MarmotApp, MarmotAppConfig,
MarmotAppEvent, MarmotAppRuntime, MediaAttachmentReference, MediaLocator,
MediaUploadAttachmentRequest, MediaUploadRequest, MissingRelayListKind, NotificationWakeSource,
PushPlatform, RuntimeMessageUpdate, SignOutOptions, TimelineMessageQuery, TimelinePagination,
UserDirectorySearch, UserProfileMetadata, tag_value,
PushPlatform, RuntimeMessageUpdate, SelfMembership, SignOutOptions, TimelineMessageQuery,
TimelinePagination, UserDirectorySearch, UserProfileMetadata, tag_value,
};
use nostr::base64::Engine as _;
use nostr::base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
Expand Down Expand Up @@ -3008,6 +3008,17 @@ async fn local_leave_suppresses_account_unread_total() {
0,
"a local leave must suppress the leaver's frozen account unread total"
);

// The same leave records the chat-list row as a voluntary `Left` (not an
// involuntary `Removed`), end to end through the projection refresh, so the
// chat list can label the departure.
let bob_row = app
.chat_list("bob", true)
.unwrap()
.into_iter()
.find(|row| row.group_id_hex == group_id_hex)
.expect("bob's chat-list row survives the leave");
assert_eq!(bob_row.self_membership, SelfMembership::Left);
}

#[tokio::test]
Expand Down
6 changes: 5 additions & 1 deletion crates/marmot-uniffi/src/conversions/chat_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use marmot_app::{ChatListAvatar, ChatListMessagePreview, ChatListRow, RuntimeChatListUpdate};

use super::common::markdown_content_tokens;
use super::common::{SelfMembershipFfi, markdown_content_tokens};
use crate::markdown::MarkdownDocumentFfi;

#[derive(Clone, Debug, uniffi::Record)]
Expand Down Expand Up @@ -72,6 +72,9 @@ pub struct ChatListRowFfi {
pub last_read_message_id_hex: Option<String>,
pub last_read_timeline_at: Option<u64>,
pub updated_at: u64,
/// Whether the local account is still a member of this group, and if not,
/// whether it left voluntarily or was removed.
pub self_membership: SelfMembershipFfi,
}

impl From<ChatListRow> for ChatListRowFfi {
Expand All @@ -93,6 +96,7 @@ impl From<ChatListRow> for ChatListRowFfi {
last_read_message_id_hex: value.last_read_message_id_hex,
last_read_timeline_at: value.last_read_timeline_at,
updated_at: value.updated_at,
self_membership: value.self_membership.into(),
}
}
}
Expand Down
22 changes: 22 additions & 0 deletions crates/marmot-uniffi/src/conversions/common.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,31 @@
//! Shared FFI helpers used across the conversion sub-modules.

use cgka_traits::{GroupId, app_event::MARMOT_APP_EVENT_KIND_CHAT};
use marmot_app::SelfMembership;

use crate::markdown::{MarkdownDocumentFfi, parse_markdown_document};

/// The local account's own membership in a group: an active `Member`, or a
/// terminal state describing how it left — `Left` (a voluntary self-removal or
/// declined invite) or `Removed` (evicted by another member). Surfaced on both
/// the chat-list row and the group-detail record.
#[derive(Clone, Copy, Debug, uniffi::Enum)]
pub enum SelfMembershipFfi {
Member,
Left,
Removed,
}

impl From<SelfMembership> for SelfMembershipFfi {
fn from(value: SelfMembership) -> Self {
match value {
SelfMembership::Member => SelfMembershipFfi::Member,
SelfMembership::Left => SelfMembershipFfi::Left,
SelfMembership::Removed => SelfMembershipFfi::Removed,
}
}
}

/// One Nostr tag from an inner Marmot app event, e.g. `["e", "<id>"]` or an
/// `["imeta", …]` media descriptor. Host apps branch on the inner event `kind`
/// plus these tags instead of a fixed payload enum.
Expand Down
Loading
Loading