feat: list a direct conversation's participants (#189)

A direct conversation has participants like any conversation, and listing them is part of the conversation surface, but group_members could only answer for groups: the roster accessor sat on the group-only trait, so the direct arm returned an unsupported-function error even though the membership lives in the conversation's inner group.

members moves to the Convo trait and DirectV1Convo delegates it to that inner group. add_member stays on the group-only trait, so the direct arm keeps rejecting it.
This commit is contained in:
osmaczko 2026-07-24 19:45:22 +02:00 committed by GitHub
parent 639632b775
commit 7474f023ed
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 100 additions and 81 deletions

View File

@ -35,6 +35,10 @@ pub(crate) trait Convo<S: ExternalServices>: Identified + Send {
/// Advances any time-driven protocol work (de-mls consensus deadlines) and
/// reports what it observed, mirroring [`Self::handle_frame`].
fn wakeup(&mut self, service_ctx: &mut ServiceContext<S>) -> Result<ConvoOutcome, ChatError>;
/// Each current member's MLS leaf-credential content (hex-encoded), self
/// included.
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError>;
}
/// Group-only operations.
@ -45,10 +49,6 @@ pub(crate) trait GroupConvo<S: ExternalServices>: Convo<S> + std::fmt::Debug + S
members: &[IdentIdRef],
) -> Result<(), ChatError>;
/// Each current member's MLS leaf-credential content (hex-encoded), self
/// included.
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError>;
/// Each member this conversation invited and the group has not committed
/// yet, in the same encoding as [`Self::members`]. Covers only invites
/// [`Self::add_member`] made here, and is empty for a conversation kind

View File

@ -61,4 +61,8 @@ where
) -> Result<crate::ConvoOutcome, ChatError> {
self.inner_group.wakeup(service_ctx)
}
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
Convo::<S>::members(&self.inner_group)
}
}

View File

@ -295,6 +295,14 @@ impl<S: ExternalServices> Convo<S> for GroupV1Convo {
fn wakeup(&mut self, _: &mut ServiceContext<S>) -> Result<ConvoOutcome, ChatError> {
Ok(ConvoOutcome::empty(self.id().to_string()))
}
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
Ok(self
.mls_group
.members()
.map(|m| m.credential.serialized_content().to_vec())
.collect())
}
}
impl<S: ExternalServices> GroupConvo<S> for GroupV1Convo {
@ -344,14 +352,6 @@ impl<S: ExternalServices> GroupConvo<S> for GroupV1Convo {
self.send_payload(cx, commit.to_bytes()?)
}
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
Ok(self
.mls_group
.members()
.map(|m| m.credential.serialized_content().to_vec())
.collect())
}
/// Always empty: `add_member` merges its own commit, so an added member is
/// on the roster by the time the call returns.
fn pending_members(&self) -> Result<Vec<Vec<u8>>, ChatError> {

View File

@ -291,6 +291,16 @@ where
let events = self.after_op(ctx)?; // publish what poll produced + re-arm alarm
Ok(self.outcome_from_events(&events))
}
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
// Guarantee the local member is listed so callers see the full roster.
let mut members = self.conversation.members()?;
let self_id = self.conversation.member_id_bytes().to_vec();
if !members.contains(&self_id) {
members.push(self_id);
}
Ok(members)
}
}
impl<S> GroupConvo<S> for GroupV2Convo
@ -376,16 +386,6 @@ where
result.and(flushed)
}
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
// Guarantee the local member is listed so callers see the full roster.
let mut members = self.conversation.members()?;
let self_id = self.conversation.member_id_bytes().to_vec();
if !members.contains(&self_id) {
members.push(self_id);
}
Ok(members)
}
fn pending_members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
Ok(self
.pending_invites

View File

@ -263,52 +263,36 @@ impl<'a, S: ExternalServices + 'static> Core<S> {
convo_id: &str,
members: &[IdentIdRef],
) -> Result<(), ChatError> {
if self.cached_convos.contains_key(convo_id) {
let convo = self
.cached_convos
.get_mut(convo_id)
.ok_or_else(|| ChatError::NoConvo(convo_id.to_string()))?;
let convo = self
.cached_convos
.get_mut(convo_id)
.ok_or_else(|| ChatError::NoConvo(convo_id.to_string()))?;
match convo {
ConvoTypeOwned::Group(group_convo) => {
group_convo.add_member(&mut self.services, members)
}
ConvoTypeOwned::Direct(convo) => Err(ChatError::UnsupportedFunction(
convo.id().into(),
"Add Member".into(),
)),
match convo {
ConvoTypeOwned::Group(group_convo) => {
group_convo.add_member(&mut self.services, members)
}
} else {
let mut convo = self.load_group_convo(convo_id)?;
convo.add_member(&mut self.services, members)
ConvoTypeOwned::Direct(convo) => Err(ChatError::UnsupportedFunction(
convo.id().into(),
"Add Member".into(),
)),
}
}
/// Each member's MLS leaf-credential content (hex-encoded); errors if
/// `convo_id` names a direct (non-group) conversation.
/// Each member's MLS leaf-credential content (hex-encoded), for a direct
/// conversation as for a group.
pub fn group_members(&mut self, convo_id: &str) -> Result<Vec<Vec<u8>>, ChatError> {
if self.cached_convos.contains_key(convo_id) {
let convo = self
.cached_convos
.get(convo_id)
.ok_or_else(|| ChatError::NoConvo(convo_id.to_string()))?;
let convo = self
.cached_convos
.get(convo_id)
.ok_or_else(|| ChatError::NoConvo(convo_id.to_string()))?;
match convo {
ConvoTypeOwned::Group(group_convo) => group_convo.members(),
ConvoTypeOwned::Direct(convo) => Err(ChatError::UnsupportedFunction(
convo.id().into(),
"List Members".into(),
)),
}
} else {
let convo = self.load_group_convo(convo_id)?;
convo.members()
}
convo.members()
}
/// Each member invited here and still awaiting the group's commit, in the
/// same encoding as [`Self::group_members`]; errors if `convo_id` names a
/// direct (non-group) conversation.
/// same encoding as [`Self::group_members`]. A direct conversation has no
/// pending members and reports none.
pub fn group_pending_members(&mut self, convo_id: &str) -> Result<Vec<Vec<u8>>, ChatError> {
let convo = self
.cached_convos
@ -317,10 +301,7 @@ impl<'a, S: ExternalServices + 'static> Core<S> {
match convo {
ConvoTypeOwned::Group(group_convo) => group_convo.pending_members(),
ConvoTypeOwned::Direct(convo) => Err(ChatError::UnsupportedFunction(
convo.id().into(),
"List Pending Members".into(),
)),
ConvoTypeOwned::Direct(_) => Ok(Vec::new()),
}
}
@ -464,17 +445,6 @@ impl<'a, S: ExternalServices + 'static> Core<S> {
})
}
/// Rebuilds a group conversation; errors if `convo_id` names a non-group.
fn load_group_convo(&mut self, convo_id: &str) -> Result<Box<dyn GroupConvo<S>>, ChatError> {
let record = self.load_conversation_meta(convo_id)?;
match record.kind {
ConversationKind::GroupV1 => Ok(Box::new(self.load_mls_convo(&record.local_convo_id)?)),
ConversationKind::Unknown(_) => {
Err(ChatError::UnsupportedConvoType(record.kind.as_str().into()))
}
}
}
/// Rebuilds a group conversation from storage so an operation can run against it.
fn load_mls_convo(&mut self, convo_id: &str) -> Result<GroupV1Convo, ChatError> {
let group_id_bytes = hex::decode(convo_id).map_err(ChatError::generic)?;
@ -563,4 +533,11 @@ impl<S: ExternalServices> Convo<S> for ConvoTypeOwned<S> {
ConvoTypeOwned::Direct(convo) => convo.wakeup(service_ctx),
}
}
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
match self {
ConvoTypeOwned::Group(group_convo) => group_convo.members(),
ConvoTypeOwned::Direct(convo) => convo.members(),
}
}
}

View File

@ -229,14 +229,15 @@ where
.map_err(Into::into)
}
/// The group's roster, one [`GroupMember`] per account (self included),
/// committed members first and this client's uncommitted invites after
/// them, flagged `pending`. An account's several devices collapse to a
/// single entry surfacing that account; a member whose account claim the
/// directory can't confirm stays on the roster individually, keyed by its
/// device. An account that is both committed and pending collapses to its
/// committed entry. Costs one directory lookup per member that claims an
/// account, the same per-member cost a received message's sender check pays.
/// The conversation's roster, one [`GroupMember`] per account (self
/// included), for a direct conversation as for a group: committed members
/// first and this client's uncommitted invites after them, flagged
/// `pending`. An account's several devices collapse to a single entry
/// surfacing that account; a member whose account claim the directory can't
/// confirm stays on the roster individually, keyed by its device. An account
/// that is both committed and pending collapses to its committed entry.
/// Costs one directory lookup per member that claims an account, the same
/// per-member cost a received message's sender check pays.
pub fn group_members(&mut self, convo_id: &str) -> Result<Vec<GroupMember>, ClientError> {
let (committed, pending) = {
let mut core = self.core.lock();

View File

@ -304,6 +304,43 @@ fn group_metadata_on_direct_conversation_errors() {
.expect_err("direct conversation has no group metadata");
}
/// A direct conversation reports its participants like any conversation. Add
/// Member errors on the creator's handle, though the joiner holds the same
/// conversation as a plain group, so the rejection is not conversation-wide.
#[test]
fn direct_conversation_lists_its_participants() {
let bus = MessageBus::default();
let reg = EphemeralRegistry::new();
let (mut saro, _saro_events) =
create_test_client(bus.clone(), reg.clone()).expect("client create");
let (raya, _raya_events) = create_test_client(bus.clone(), reg.clone()).expect("client create");
let saro_addr = saro.addr().to_string();
let raya_addr = raya.addr().to_string();
let convo_id = saro
.create_direct_conversation(&raya_addr)
.expect("convo create");
let roster = saro.group_members(&convo_id).expect("group_members");
let mut accounts: Vec<Option<&str>> = roster
.iter()
.map(|m| m.account.as_ref().map(|a| a.as_str()))
.collect();
accounts.sort();
let mut expected = vec![Some(saro_addr.as_str()), Some(raya_addr.as_str())];
expected.sort();
assert_eq!(accounts, expected);
let err = saro
.add_group_members(&convo_id, &[&raya_addr])
.expect_err("add member is unsupported on a direct conversation");
assert!(matches!(
err,
logos_generic_chat::ClientError::Chat(libchat::ChatError::UnsupportedFunction(..))
));
}
#[derive(Debug)]
struct FailingDelivery {
inbound_tx: Sender<Vec<u8>>,