mdk-core 0.8.0

//! MDK groups
//!
//! This module provides functionality for managing MLS groups in Nostr:
//! - Group creation and configuration
//! - Member management (adding/removing members)
//! - Group state updates and synchronization
//! - Group metadata handling
//! - Group secret management
//!
//! Groups in MDK have both an MLS group ID and a Nostr group ID. The MLS group ID
//! is used internally by the MLS protocol, while the Nostr group ID is used for
//! relay-based message routing and group discovery.

use std::collections::{BTreeMap, BTreeSet};

use mdk_storage_traits::GroupId;
use mdk_storage_traits::MdkStorageProvider;
use mdk_storage_traits::groups::types as group_types;
use mdk_storage_traits::messages::types as message_types;
use nostr::prelude::*;
use openmls::prelude::*;
use openmls::treesync::errors::LeafNodeValidationError;
use openmls_basic_credential::SignatureKeyPair;
use tls_codec::Serialize as TlsSerialize;

use sha2::{Digest, Sha256};

mod openmls_compat;

use self::openmls_compat::exported_leaf_capabilities;
use super::MDK;
use super::extension::NostrGroupDataExtension;
use crate::constant::{GROUP_CONTEXT_REQUIRED_EXTENSIONS, SUPPORTED_PROPOSALS};
use crate::error::Error;
use crate::messages::EventTag;
use crate::messages::crypto::encrypt_message_with_exporter_secret;
use crate::util::{ContentEncoding, encode_content};

/// Result of creating a new MLS group
#[derive(Debug)]
pub struct GroupResult {
    /// The stored group
    pub group: group_types::Group,
    /// A vec of Kind:444 Welcome Events to be published for members added during creation.
    pub welcome_rumors: Vec<UnsignedEvent>,
}

/// Result of updating a group
#[derive(Debug)]
pub struct UpdateGroupResult {
    /// A Kind:445 Event containing the proposal or commit message. To be published to the group relays.
    pub evolution_event: Event,
    /// A vec of Kind:444 Welcome Events to be published for any members added as part of the update.
    pub welcome_rumors: Option<Vec<UnsignedEvent>>,
    /// The MLS group ID this update applies to
    pub mls_group_id: GroupId,
}

/// Upgrade readiness of a single proposal type — whether adding it to
/// the group's `RequiredCapabilities` via
/// [`MDK::upgrade_group_capabilities`] would be safe.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ProposalUpgradability {
    /// Already in the group's `RequiredCapabilities`. Not an upgrade
    /// candidate.
    AlreadyRequired,
    /// Every current leaf advertises this proposal. Safe to add.
    Available,
    /// One or more members' leaves don't advertise this proposal. The
    /// `blockers` list names them. The missing proposal itself is
    /// implicit from the containing entry.
    Blocked {
        /// Nostr public keys of members whose leaves don't advertise
        /// the containing proposal type, ordered by leaf index.
        blockers: Vec<PublicKey>,
    },
}

/// Per-proposal-type upgrade readiness for a group's
/// `RequiredCapabilities`. Returned by
/// [`MDK::group_capability_upgrade_status`].
///
/// `per_proposal` contains one entry per proposal type in MDK's
/// `SUPPORTED_PROPOSALS`, in the order that constant declares them.
/// Ordering is stable across calls at the same epoch.
#[derive(Debug, Clone)]
pub struct CapabilityUpgradeStatus {
    /// One (proposal, status) pair per proposal type MDK advertises.
    pub per_proposal: Vec<(ProposalType, ProposalUpgradability)>,
}

/// Snapshot of a single group member's advertised MLS capabilities.
///
/// Returned by [`MDK::group_member_capabilities`]. Capabilities are the
/// proposal types, extension types, and ciphersuites the member's current
/// leaf advertises support for, as well as whether the member is an admin
/// per MDK's group-data extension.
#[derive(Debug, Clone)]
pub struct MemberCapabilities {
    /// The member's Nostr public key.
    pub member: PublicKey,
    /// Whether the member is currently an admin of the group.
    pub is_admin: bool,
    /// Proposal types this member's leaf advertises.
    pub proposals: BTreeSet<ProposalType>,
    /// Extension types this member's leaf advertises.
    pub extensions: BTreeSet<ExtensionType>,
    /// Ciphersuites this member's leaf advertises, in advertised order.
    ///
    /// Kept as a `Vec` because ciphersuite order can carry preference;
    /// proposal and extension capabilities are sets.
    pub ciphersuites: Vec<VerifiableCiphersuite>,
}

/// Configuration data for the Group
#[derive(Debug, Clone)]
pub struct NostrGroupConfigData {
    /// Group name
    pub name: String,
    /// Group description
    pub description: String,
    /// URL to encrypted group image
    pub image_hash: Option<[u8; 32]>,
    /// Key to decrypt the image
    pub image_key: Option<[u8; 32]>,
    /// Nonce to decrypt the image
    pub image_nonce: Option<[u8; 12]>,
    /// Relays used by the group
    pub relays: Vec<RelayUrl>,
    /// Group admins
    pub admins: Vec<PublicKey>,
}

/// Configuration for updating group data with optional fields
#[derive(Debug, Clone, Default)]
pub struct NostrGroupDataUpdate {
    /// Group name (optional)
    pub name: Option<String>,
    /// Group description (optional)
    pub description: Option<String>,
    /// URL to encrypted group image (optional, use Some(None) to clear)
    pub image_hash: Option<Option<[u8; 32]>>,
    /// Key to decrypt the image (optional, use Some(None) to clear)
    pub image_key: Option<Option<[u8; 32]>>,
    /// Nonce to decrypt the image (optional, use Some(None) to clear)
    pub image_nonce: Option<Option<[u8; 12]>>,
    /// Upload key seed for the image (optional, use Some(None) to clear)
    pub image_upload_key: Option<Option<[u8; 32]>>,
    /// Relays used by the group (optional)
    pub relays: Option<Vec<RelayUrl>>,
    /// Group admins (optional)
    pub admins: Option<Vec<PublicKey>>,
    /// Nostr group ID for message routing (optional, for rotation per MIP-01)
    pub nostr_group_id: Option<[u8; 32]>,
}

/// Pending member changes from proposals that need admin approval
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct PendingMemberChanges {
    /// Public keys of members that will be added when proposals are committed
    pub additions: Vec<PublicKey>,
    /// Public keys of members that will be removed when proposals are committed
    pub removals: Vec<PublicKey>,
}

/// Public information about a leaf node in the ratchet tree
///
/// Contains only public information (encryption key, signature key, credential
/// identity). No secret key material is included.
///
/// # Security Note
///
/// The ratchet tree holds public keys and tree structure, not secrets.
/// The MLS spec assumes this data can be shared (e.g. in Welcome messages).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct LeafNodeInfo {
    /// The leaf index in the ratchet tree
    pub index: u32,
    /// The member's public HPKE encryption key (hex-encoded)
    pub encryption_key: String,
    /// The member's public signature key (hex-encoded)
    pub signature_key: String,
    /// The member's credential identity (hex-encoded, typically a Nostr public key)
    pub credential_identity: String,
}

/// Public information about the ratchet tree of an MLS group
///
/// Provides a view into the MLS group's tree structure.
/// Contains only public information — no secrets or private key material.
///
/// # Security Note
///
/// The ratchet tree holds public keys and tree structure, not secrets.
/// The MLS spec assumes this data can be shared (e.g. in Welcome messages).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct RatchetTreeInfo {
    /// SHA-256 fingerprint of the TLS-serialized ratchet tree (hex-encoded).
    /// Useful for comparing tree state across clients.
    pub tree_hash: String,
    /// The full ratchet tree serialized via TLS encoding (hex-encoded)
    pub serialized_tree: String,
    /// Leaf nodes with their indices and public keys
    pub leaf_nodes: Vec<LeafNodeInfo>,
}

impl NostrGroupConfigData {
    /// Creates NostrGroupConfigData
    pub fn new(
        name: String,
        description: String,
        image_hash: Option<[u8; 32]>,
        image_key: Option<[u8; 32]>,
        image_nonce: Option<[u8; 12]>,
        relays: Vec<RelayUrl>,
        admins: Vec<PublicKey>,
    ) -> Self {
        Self {
            name,
            description,
            image_hash,
            image_key,
            image_nonce,
            relays,
            admins,
        }
    }
}

impl NostrGroupDataUpdate {
    /// Creates a new empty update configuration
    pub fn new() -> Self {
        Self::default()
    }

    mdk_macros::setters! {
        /// Sets the name to be updated
        name: impl Into<String>;
        /// Sets the description to be updated
        description: impl Into<String>;
        /// Sets the image hash to be updated
        image_hash: Option<[u8; 32]>;
        /// Sets the image key to be updated
        image_key: Option<[u8; 32]>;
        /// Sets the image nonce to be updated
        image_nonce: Option<[u8; 12]>;
        /// Sets the image upload key to be updated
        image_upload_key: Option<[u8; 32]>;
        /// Sets the relays to be updated
        relays: Vec<RelayUrl>;
        /// Sets the admins to be updated
        admins: Vec<PublicKey>;
        /// Sets the nostr_group_id to be updated (for ID rotation per MIP-01)
        nostr_group_id: [u8; 32];
    }
}

impl<Storage> MDK<Storage>
where
    Storage: MdkStorageProvider,
{
    /// Extracts a public key from an MLS credential.
    pub(crate) fn pubkey_from_credential(
        &self,
        credential: &Credential,
    ) -> Result<PublicKey, Error> {
        let basic = BasicCredential::try_from(credential.clone())?;
        self.parse_credential_identity(basic.identity())
    }

    /// Gets the current user's public key from an MLS group
    ///
    /// # Arguments
    ///
    /// * `group` - Reference to the MLS group
    ///
    /// # Returns
    ///
    /// * `Ok(PublicKey)` - The current user's public key
    /// * `Err(Error)` - If the user's leaf node is not found or there is an error extracting the public key
    pub(crate) fn get_own_pubkey(&self, group: &MlsGroup) -> Result<PublicKey, Error> {
        let own_leaf = group.own_leaf().ok_or(Error::OwnLeafNotFound)?;
        self.pubkey_from_credential(own_leaf.credential())
    }

    /// Checks if the LeafNode is an admin of an MLS group
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    /// * `leaf_node` - The leaf to check as an admin
    ///
    /// # Returns
    ///
    /// * `Ok(true)` - The leaf node is an admin
    /// * `Ok(false)` - The leaf node is not an admin
    /// * `Err(Error)` - If the public key cannot be extracted or the group is not found
    pub(crate) fn is_leaf_node_admin(
        &self,
        group_id: &GroupId,
        leaf_node: &LeafNode,
    ) -> Result<bool, Error> {
        let pubkey = self.pubkey_for_leaf_node(leaf_node)?;
        let mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        let group_data = NostrGroupDataExtension::from_group(&mls_group)?;
        Ok(group_data.admins.contains(&pubkey))
    }

    /// Extracts the public key from a leaf node
    ///
    /// # Arguments
    ///
    /// * `leaf_node` - Reference to the leaf node
    ///
    /// # Returns
    ///
    /// * `Ok(PublicKey)` - The public key extracted from the leaf node
    /// * `Err(Error)` - If the credential cannot be converted or the public key cannot be extracted
    pub(crate) fn pubkey_for_leaf_node(&self, leaf_node: &LeafNode) -> Result<PublicKey, Error> {
        self.pubkey_from_credential(leaf_node.credential())
    }

    /// Extracts the public key from a member
    ///
    /// # Arguments
    ///
    /// * `member` - Reference to the member
    ///
    /// # Returns
    ///
    /// * `Ok(PublicKey)` - The public key extracted from the member
    /// * `Err(Error)` - If the public key cannot be extracted or there is an error converting the public key to hex
    pub(crate) fn pubkey_for_member(&self, member: &Member) -> Result<PublicKey, Error> {
        self.pubkey_from_credential(&member.credential)
    }

    /// Loads the signature key pair for the current member in an MLS group
    ///
    /// # Arguments
    ///
    /// * `group` - Reference to the MLS group
    ///
    /// # Returns
    ///
    /// * `Ok(SignatureKeyPair)` - The member's signature key pair
    /// * `Err(Error)` - If the key pair cannot be loaded
    pub(crate) fn load_mls_signer(&self, group: &MlsGroup) -> Result<SignatureKeyPair, Error> {
        let own_leaf: &LeafNode = group.own_leaf().ok_or(Error::OwnLeafNotFound)?;
        let public_key: &[u8] = own_leaf.signature_key().as_slice();

        SignatureKeyPair::read(
            self.provider.storage(),
            public_key,
            group.ciphersuite().signature_algorithm(),
        )
        .ok_or(Error::CantLoadSigner)
    }

    /// Loads an MLS group from storage by its ID
    fn load_mls_group_impl(&self, group_id: &GroupId) -> Result<Option<MlsGroup>, Error> {
        MlsGroup::load(self.provider.storage(), group_id.inner())
            .map_err(|e| Error::Provider(e.to_string()))
    }

    /// Loads an MLS group from storage by its ID
    ///
    /// This method provides access to the underlying OpenMLS `MlsGroup` object,
    /// which can be useful for inspection, debugging, and advanced operations.
    ///
    /// **Note:** This method is only available with the `debug-examples` feature flag.
    /// It is intended for debugging and example purposes only.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID to load
    ///
    /// # Returns
    ///
    /// * `Ok(Some(MlsGroup))` - The loaded group if found
    /// * `Ok(None)` - If no group exists with the given ID
    /// * `Err(Error)` - If there is an error loading the group
    #[cfg(feature = "debug-examples")]
    pub fn load_mls_group(&self, group_id: &GroupId) -> Result<Option<MlsGroup>, Error> {
        self.load_mls_group_impl(group_id)
    }

    /// Loads an MLS group from storage by its ID (internal version)
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID to load
    ///
    /// # Returns
    ///
    /// * `Ok(Some(MlsGroup))` - The loaded group if found
    /// * `Ok(None)` - If no group exists with the given ID
    /// * `Err(Error)` - If there is an error loading the group
    #[cfg(not(feature = "debug-examples"))]
    pub(crate) fn load_mls_group(&self, group_id: &GroupId) -> Result<Option<MlsGroup>, Error> {
        self.load_mls_group_impl(group_id)
    }

    fn derive_exporter_secret_for_group(
        &self,
        group_id: &crate::GroupId,
        group: &MlsGroup,
        exporter_label: &str,
        exporter_context: &[u8],
    ) -> Result<group_types::GroupExporterSecret, Error> {
        let export_secret: [u8; 32] = group
            .export_secret(self.provider.crypto(), exporter_label, exporter_context, 32)?
            .try_into()
            .map_err(|_| Error::Group("Failed to convert export secret to [u8; 32]".to_string()))?;

        Ok(group_types::GroupExporterSecret {
            mls_group_id: group_id.clone(),
            epoch: group.epoch().as_u64(),
            secret: mdk_storage_traits::Secret::new(export_secret),
        })
    }

    pub(crate) fn legacy_exporter_secret(
        &self,
        group_id: &crate::GroupId,
    ) -> Result<group_types::GroupExporterSecret, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        self.derive_exporter_secret_for_group(group_id, &group, "nostr", b"nostr")
    }

    /// Exports the current epoch's secret key from an MLS group for MIP-03 message encryption.
    ///
    /// Uses `MLS-Exporter("marmot", "group-event", 32)` per MIP-03. The secret is the
    /// ChaCha20-Poly1305 encryption key for kind:445 Group Message Events.
    /// The current epoch's secret is always re-derived from live MLS state before being
    /// stored. This self-heals stale rows migrated from pre-0.7.0 caches.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    ///
    /// # Returns
    ///
    /// * `Ok(GroupExporterSecret)` - The exported secret
    /// * `Err(Error)` - If the group is not found or there is an error exporting the secret
    pub(crate) fn exporter_secret(
        &self,
        group_id: &crate::GroupId,
    ) -> Result<group_types::GroupExporterSecret, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        let stored_secret = self
            .storage()
            .get_group_exporter_secret(group_id, group.epoch().as_u64())
            .map_err(|e| Error::Group(e.to_string()))?;
        let group_exporter_secret =
            self.derive_exporter_secret_for_group(group_id, &group, "marmot", b"group-event")?;

        // Only write back to storage when the value has changed (or is absent).
        // This avoids an unconditional write on every message decryption in the steady state
        // while still self-healing any stale pre-0.7.0 row that differs from live MLS state.
        let secret_changed = stored_secret
            .as_ref()
            .map(|s| s.secret != group_exporter_secret.secret)
            .unwrap_or(true);

        if secret_changed {
            self.storage()
                .save_group_exporter_secret(group_exporter_secret.clone())
                .map_err(|e| Error::Group(e.to_string()))?;

            if let Some(stored_secret) = stored_secret
                && let Err(e) = self
                    .storage()
                    .save_group_legacy_exporter_secret(stored_secret)
            {
                tracing::warn!(
                    target: "mdk_core::groups::exporter_secret",
                    "Failed to preserve legacy exporter secret for compatibility: {}",
                    e
                );
            }
        }

        Ok(group_exporter_secret)
    }

    /// Exports the current epoch's secret key from an MLS group for MIP-04 encrypted media.
    ///
    /// Uses `MLS-Exporter("marmot", "encrypted-media", 32)` per MIP-04. This is a separate
    /// exporter from [`Self::exporter_secret`] (which uses `"group-event"` for MIP-03).
    /// The result is used as HKDF input keying material for per-file encryption key derivation.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    ///
    /// # Returns
    ///
    /// * `Ok(GroupExporterSecret)` - The exported secret
    /// * `Err(Error)` - If the group is not found or there is an error exporting the secret
    #[cfg(feature = "mip04")]
    pub(crate) fn mip04_exporter_secret(
        &self,
        group_id: &crate::GroupId,
    ) -> Result<group_types::GroupExporterSecret, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        self.derive_exporter_secret_for_group(group_id, &group, "marmot", b"encrypted-media")
    }

    /// Retrieves a MDK group by its MLS group ID
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID to look up
    ///
    /// # Returns
    ///
    /// * `Ok(Some(Group))` - The group if found
    /// * `Ok(None)` - If no group exists with the given ID
    /// * `Err(Error)` - If there is an error accessing storage
    pub fn get_group(&self, group_id: &GroupId) -> Result<Option<group_types::Group>, Error> {
        self.storage()
            .find_group_by_mls_group_id(group_id)
            .map_err(|e| Error::Group(e.to_string()))
    }

    /// Retrieves all MDK groups from storage
    ///
    /// # Returns
    ///
    /// * `Ok(Vec<Group>)` - List of all groups
    /// * `Err(Error)` - If there is an error accessing storage
    pub fn get_groups(&self) -> Result<Vec<group_types::Group>, Error> {
        self.storage()
            .all_groups()
            .map_err(|e| Error::Group(e.to_string()))
    }

    /// Returns the group IDs of active groups that need a self-update.
    ///
    /// A group needs a self-update if:
    /// - `self_update_state` is `SelfUpdateState::Required` (post-join requirement, MIP-02), or
    /// - `self_update_state` is `SelfUpdateState::CompletedAt` and the timestamp is older than `threshold_secs` (periodic rotation, MIP-00)
    ///
    /// # Arguments
    /// * `threshold_secs` - Maximum age in seconds before a group's key rotation is considered stale
    ///
    /// # Returns
    /// * `Ok(Vec<GroupId>)` - Group IDs needing self-update
    /// * `Err(Error)` - If there is an error accessing storage
    pub fn groups_needing_self_update(&self, threshold_secs: u64) -> Result<Vec<GroupId>, Error> {
        self.storage()
            .groups_needing_self_update(threshold_secs)
            .map_err(|e| Error::Group(e.to_string()))
    }

    /// Gets the Nostr identities represented by all live members in an MLS group
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    ///
    /// # Returns
    ///
    /// * `Ok(BTreeSet<PublicKey>)` - Set of represented member identities
    /// * `Err(Error)` - If the group is not found or there is an error accessing member data
    pub fn get_members(&self, group_id: &GroupId) -> Result<BTreeSet<PublicKey>, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        self.live_member_identities(&group)
    }

    /// Returns the proposal types required of every member of this group,
    /// read from the live `MlsGroup`'s `RequiredCapabilities` extension.
    ///
    /// Branch UI on this set to predict group behavior — for example,
    /// when `ProposalType::SelfRemove` is present, non-admin members can
    /// leave without an admin commit.
    ///
    /// An empty set is the LCD outcome for mixed or empty-invitee groups
    /// and is distinct from `Err(Error::GroupNotFound)`, so callers can
    /// rely on an empty set meaning "no required proposals," not "unknown."
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    ///
    /// # Returns
    ///
    /// * `Ok(BTreeSet<ProposalType>)` - Proposal types listed in the group's
    ///   `RequiredCapabilities` extension (empty if none)
    /// * `Err(Error)` - `Error::GroupNotFound` if no MLS record exists for
    ///   `group_id`, or a storage error if loading the record fails
    pub fn group_required_proposals(
        &self,
        group_id: &GroupId,
    ) -> Result<BTreeSet<ProposalType>, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        Ok(match group.extensions().required_capabilities() {
            Some(rc) => rc.proposal_types().iter().copied().collect(),
            None => BTreeSet::new(),
        })
    }

    /// Returns per-member advertised capabilities for every active leaf
    /// in the group.
    ///
    /// Any member may call this — it reads public leaf state only. Useful
    /// for client UIs that want to show "who supports what" or diagnose
    /// which members are blocking a pending
    /// [`MDK::upgrade_group_capabilities`] request.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    ///
    /// # Returns
    ///
    /// * `Ok(Vec<MemberCapabilities>)` - One entry per active leaf, ordered
    ///   by leaf index (stable for a given epoch)
    /// * `Err(Error::GroupNotFound)` - If no MLS record exists for `group_id`
    pub fn group_member_capabilities(
        &self,
        group_id: &GroupId,
    ) -> Result<Vec<MemberCapabilities>, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        let group_data = NostrGroupDataExtension::from_group(&group)?;
        let capabilities_by_leaf = exported_leaf_capabilities(&group)?;
        group
            .members()
            .map(|member| {
                let public_key = self.pubkey_for_member(&member)?;
                let capabilities = capabilities_by_leaf
                    .get(&member.index)
                    .ok_or_else(|| missing_leaf_capabilities_error(member.index.u32()))?;
                Ok(MemberCapabilities {
                    is_admin: group_data.admins.contains(&public_key),
                    member: public_key,
                    proposals: capabilities.proposals().iter().copied().collect(),
                    extensions: capabilities.extensions().iter().copied().collect(),
                    ciphersuites: capabilities.ciphersuites().to_vec(),
                })
            })
            .collect()
    }

    fn leaves_missing_proposal(
        &self,
        group: &MlsGroup,
        proposal: ProposalType,
    ) -> Result<Vec<PublicKey>, Error> {
        let capabilities_by_leaf = exported_leaf_capabilities(group)?;
        group
            .members()
            .try_fold(Vec::new(), |mut blockers, member| {
                let capabilities = capabilities_by_leaf
                    .get(&member.index)
                    .ok_or_else(|| missing_leaf_capabilities_error(member.index.u32()))?;

                if !capabilities.proposals().contains(&proposal) {
                    blockers.push(self.pubkey_for_member(&member)?);
                }

                Ok(blockers)
            })
    }

    /// Returns per-proposal upgrade readiness for the group's
    /// `RequiredCapabilities`. Any member may call this.
    ///
    /// For each proposal type in MDK's `SUPPORTED_PROPOSALS`, the result
    /// reports whether the proposal is already required, could be added
    /// safely (every leaf advertises it), or is blocked (and which
    /// members are blocking).
    ///
    /// See [`MDK::upgrade_group_capabilities`] to act on an `Available`
    /// proposal.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    ///
    /// # Returns
    ///
    /// * `Ok(CapabilityUpgradeStatus)` with `per_proposal` ordered the same as
    ///   `SUPPORTED_PROPOSALS`
    /// * `Err(Error::GroupNotFound)` - If no MLS record exists for `group_id`
    pub fn group_capability_upgrade_status(
        &self,
        group_id: &GroupId,
    ) -> Result<CapabilityUpgradeStatus, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        let currently_required: BTreeSet<ProposalType> = group
            .extensions()
            .required_capabilities()
            .map(|rc| rc.proposal_types().iter().copied().collect())
            .unwrap_or_default();

        let per_proposal = SUPPORTED_PROPOSALS
            .iter()
            .copied()
            .map(|pt| {
                if currently_required.contains(&pt) {
                    return Ok((pt, ProposalUpgradability::AlreadyRequired));
                }
                let blockers = self.leaves_missing_proposal(&group, pt)?;
                Ok(if blockers.is_empty() {
                    (pt, ProposalUpgradability::Available)
                } else {
                    (pt, ProposalUpgradability::Blocked { blockers })
                })
            })
            .collect::<Result<Vec<_>, Error>>()?;

        Ok(CapabilityUpgradeStatus { per_proposal })
    }

    /// Proposes a `GroupContextExtensions` commit that adds each of
    /// `proposals_to_add` to the group's `RequiredCapabilities`.
    ///
    /// Admin-only. One MLS commit, all-or-nothing: if any requested
    /// proposal fails re-validation when this method runs (e.g. status
    /// was checked earlier and another admin upgraded concurrently, or a
    /// new blocker joined since), the whole call errors with the
    /// relevant per-proposal variant and no commit is sent.
    ///
    /// See [`MDK::group_capability_upgrade_status`] to discover which
    /// proposal types are currently `Available`.
    ///
    /// # Errors
    ///
    /// - [`Error::EmptyUpgradeSet`] if `proposals_to_add` is empty.
    /// - [`Error::ProposalNotInSupportedSet`] if any requested proposal
    ///   is not in MDK's `SUPPORTED_PROPOSALS`.
    /// - [`Error::NotAdmin`] if the current member is not an admin.
    /// - [`Error::ProposalAlreadyRequired`] if a requested proposal is
    ///   already in `RequiredCapabilities` (typically concurrent
    ///   upgrade).
    /// - [`Error::ProposalNotAvailableForUpgrade`] if a requested
    ///   proposal is blocked when this method runs, with the blocker
    ///   identities.
    pub fn upgrade_group_capabilities(
        &self,
        group_id: &GroupId,
        proposals_to_add: &BTreeSet<ProposalType>,
    ) -> Result<UpdateGroupResult, Error> {
        // Cheap input validation before any storage access.
        if proposals_to_add.is_empty() {
            return Err(Error::EmptyUpgradeSet);
        }
        for pt in proposals_to_add {
            if !SUPPORTED_PROPOSALS.contains(pt) {
                return Err(Error::ProposalNotInSupportedSet(*pt));
            }
        }

        let mut mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        // Admin authority is the same gate `remove_members` uses.
        let own_leaf = mls_group.own_leaf().ok_or(Error::OwnLeafNotFound)?;
        if !self.is_leaf_node_admin(group_id, own_leaf)? {
            return Err(Error::NotAdmin);
        }

        // Re-check every requested proposal against every current leaf.
        // In this OpenMLS revision, GroupContextExtensions construction
        // validates required proposal support for our own update-path
        // leaf, but MDK must reject proposal upgrades blocked by any
        // existing member.
        let current_required_capabilities = mls_group.extensions().required_capabilities();
        let currently_required: BTreeSet<ProposalType> = current_required_capabilities
            .map(|rc| rc.proposal_types().iter().copied().collect())
            .unwrap_or_default();
        for pt in proposals_to_add {
            if currently_required.contains(pt) {
                return Err(Error::ProposalAlreadyRequired(*pt));
            }
            let blockers = self.leaves_missing_proposal(&mls_group, *pt)?;
            if !blockers.is_empty() {
                return Err(Error::ProposalNotAvailableForUpgrade {
                    proposal: *pt,
                    blockers,
                });
            }
        }

        // Build the upgraded extensions by replacing only the proposal set
        // inside `RequiredCapabilities`. Preserve all other required
        // capability fields from the current group context.
        let mut new_required = currently_required;
        new_required.extend(proposals_to_add.iter().copied());
        let required_extension_types = current_required_capabilities
            .map(|rc| rc.extension_types().to_vec())
            .unwrap_or_else(|| GROUP_CONTEXT_REQUIRED_EXTENSIONS.to_vec());
        let required_credential_types = current_required_capabilities
            .map(|rc| rc.credential_types().to_vec())
            .unwrap_or_default();
        let new_required_proposals: Vec<ProposalType> = new_required.into_iter().collect();
        let required_capabilities_extension =
            Extension::RequiredCapabilities(RequiredCapabilitiesExtension::new(
                &required_extension_types,
                &new_required_proposals,
                &required_credential_types,
            ));
        let mut upgraded_extensions = mls_group.extensions().clone();
        upgraded_extensions.add_or_replace(required_capabilities_extension)?;

        let signer: SignatureKeyPair = self.load_mls_signer(&mls_group)?;
        let (commit_message, _welcome, _group_info) = mls_group.update_group_context_extensions(
            &self.provider,
            upgraded_extensions,
            &signer,
        )?;

        self.ensure_mixed_wire_format(&mut mls_group);

        let serialized_commit = commit_message.tls_serialize_detached()?;
        let commit_event =
            self.build_message_event(&mls_group.group_id().into(), serialized_commit, None)?;

        self.track_processed_message(
            commit_event.id,
            &mls_group,
            message_types::ProcessedMessageState::ProcessedCommit,
        )?;

        tracing::debug!(
            target: "mdk_core::groups::upgrade_group_capabilities",
            added = ?proposals_to_add,
            "Proposed GroupContextExtensions commit adding proposal types to RequiredCapabilities"
        );

        Ok(UpdateGroupResult {
            evolution_event: commit_event,
            welcome_rumors: None,
            mls_group_id: group_id.clone(),
        })
    }

    /// Returns the local member's current MLS leaf index for a group.
    pub fn own_leaf_index(&self, group_id: &GroupId) -> Result<u32, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        Ok(group.own_leaf_index().u32())
    }

    /// Returns the current active MLS leaf positions and their bound Nostr public keys.
    ///
    /// Removed-member tree holes are omitted by design. Callers should not
    /// expect every index in a contiguous `0..n` range to be present.
    pub fn group_leaf_map(&self, group_id: &GroupId) -> Result<BTreeMap<u32, PublicKey>, Error> {
        let group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        group
            .members()
            .try_fold(BTreeMap::new(), |mut acc, member| {
                let credentials: BasicCredential = BasicCredential::try_from(member.credential)?;
                let identity_bytes: &[u8] = credentials.identity();
                let public_key = self.parse_credential_identity(identity_bytes)?;
                acc.insert(member.index.u32(), public_key);
                Ok(acc)
            })
    }

    /// Returns public information about the ratchet tree of an MLS group
    ///
    /// This includes a SHA-256 fingerprint of the TLS-serialized ratchet tree,
    /// the full serialized tree as hex, and a list of leaf nodes with their
    /// indices and public keys.
    ///
    /// # Security Note
    ///
    /// The ratchet tree holds public keys and tree structure, not secrets.
    /// Per the MLS spec, ratchet tree data is public information that can be
    /// shared (e.g., in Welcome messages).
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    ///
    /// # Returns
    ///
    /// * `Ok(RatchetTreeInfo)` - Public information about the ratchet tree
    /// * `Err(Error)` - If the group is not found or there is an error accessing the tree
    pub fn get_ratchet_tree_info(&self, group_id: &GroupId) -> Result<RatchetTreeInfo, Error> {
        let mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        // Export and TLS-serialize the ratchet tree
        let ratchet_tree = mls_group.export_ratchet_tree();
        let serialized_bytes = ratchet_tree.tls_serialize_detached()?;

        // Compute SHA-256 fingerprint of the serialized tree for easy comparison
        let tree_hash = hex::encode(Sha256::digest(&serialized_bytes));
        let serialized_tree = hex::encode(&serialized_bytes);

        // Extract leaf nodes with their public keys
        let leaf_nodes: Vec<LeafNodeInfo> = mls_group
            .members()
            .map(|member| {
                let index = member.index.u32();
                let basic_cred = BasicCredential::try_from(member.credential).map_err(|e| {
                    tracing::warn!(
                        leaf_index = index,
                        error = %e,
                        "Failed to parse credential for leaf node in ratchet tree"
                    );
                    Error::Group(format!("invalid credential at leaf index {index}: {e}"))
                })?;
                let credential_identity = hex::encode(basic_cred.identity());

                Ok(LeafNodeInfo {
                    index,
                    encryption_key: hex::encode(&member.encryption_key),
                    signature_key: hex::encode(&member.signature_key),
                    credential_identity,
                })
            })
            .collect::<Result<Vec<_>, Error>>()?;

        Ok(RatchetTreeInfo {
            tree_hash,
            serialized_tree,
            leaf_nodes,
        })
    }

    /// Gets the public keys of members that will be added from pending proposals in an MLS group
    ///
    /// This method examines pending Add proposals in the group and extracts the public keys
    /// of members that would be added if these proposals are committed. This is useful for
    /// showing admins which member additions are pending approval.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID to examine for pending proposals
    ///
    /// # Returns
    ///
    /// * `Ok(Vec<PublicKey>)` - List of public keys for members in pending Add proposals
    /// * `Err(Error)` - If there's an error loading the group or extracting member information
    pub fn pending_added_members_pubkeys(
        &self,
        group_id: &GroupId,
    ) -> Result<Vec<PublicKey>, Error> {
        let mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        let mut added_pubkeys = Vec::new();

        for proposal in mls_group.pending_proposals() {
            if let Proposal::Add(add_proposal) = proposal.proposal() {
                let leaf_node = add_proposal.key_package().leaf_node();
                let pubkey = self.pubkey_for_leaf_node(leaf_node)?;
                added_pubkeys.push(pubkey);
            }
        }

        Ok(added_pubkeys)
    }

    /// Gets the public keys of members that will be removed from pending proposals in an MLS group
    ///
    /// This method examines pending Remove proposals in the group and extracts the public keys
    /// of members that would be removed if these proposals are committed. This is useful for
    /// showing admins which member removals are pending approval.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID to examine for pending proposals
    ///
    /// # Returns
    ///
    /// * `Ok(Vec<PublicKey>)` - List of public keys for members in pending Remove proposals
    /// * `Err(Error)` - If there's an error loading the group or extracting member information
    pub fn pending_removed_members_pubkeys(
        &self,
        group_id: &GroupId,
    ) -> Result<Vec<PublicKey>, Error> {
        let mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        let mut removed_pubkeys = Vec::new();

        for proposal in mls_group.pending_proposals() {
            if let Proposal::Remove(remove_proposal) = proposal.proposal() {
                let removed_leaf_index = remove_proposal.removed();
                if let Some(member) = mls_group.member_at(removed_leaf_index) {
                    let pubkey = self.pubkey_for_member(&member)?;
                    removed_pubkeys.push(pubkey);
                }
            }
        }

        Ok(removed_pubkeys)
    }

    /// Gets all pending member changes (additions and removals) from pending proposals
    ///
    /// This method provides a combined view of all pending member changes in a group,
    /// which is useful for showing admins a complete picture of proposed membership changes
    /// that need approval.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID to examine for pending proposals
    ///
    /// # Returns
    ///
    /// * `Ok(PendingMemberChanges)` - Struct containing lists of pending additions and removals
    /// * `Err(Error)` - If there's an error loading the group or extracting member information
    pub fn pending_member_changes(
        &self,
        group_id: &GroupId,
    ) -> Result<PendingMemberChanges, Error> {
        let mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        let mut additions = Vec::new();
        let mut removals = Vec::new();

        for proposal in mls_group.pending_proposals() {
            match proposal.proposal() {
                Proposal::Add(add_proposal) => {
                    let leaf_node = add_proposal.key_package().leaf_node();
                    let pubkey = self.pubkey_for_leaf_node(leaf_node)?;
                    additions.push(pubkey);
                }
                Proposal::Remove(remove_proposal) => {
                    let removed_leaf_index = remove_proposal.removed();
                    if let Some(member) = mls_group.member_at(removed_leaf_index) {
                        let pubkey = self.pubkey_for_member(&member)?;
                        removals.push(pubkey);
                    }
                }
                _ => {}
            }
        }

        Ok(PendingMemberChanges {
            additions,
            removals,
        })
    }

    /// Add members to a group
    ///
    /// NOTE: This function doesn't merge the pending commit. Clients must call this function manually only after successful publish of the commit message to relays.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    /// * `key_package_events` - The nostr key package events (Kind:443) for each new member to add
    ///
    /// # Returns
    ///
    /// * `Ok(UpdateGroupResult)`
    /// * `Err(Error)` - If there is an error adding members
    pub fn add_members(
        &self,
        group_id: &GroupId,
        key_package_events: &[Event],
    ) -> Result<UpdateGroupResult, Error> {
        let mut mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        let mls_signer: SignatureKeyPair = self.load_mls_signer(&mls_group)?;

        let own_leaf = mls_group.own_leaf().ok_or(Error::OwnLeafNotFound)?;
        if !self.is_leaf_node_admin(&mls_group.group_id().into(), own_leaf)? {
            return Err(Error::NotAdmin);
        }

        // Parse key packages from events
        let mut key_packages_vec: Vec<KeyPackage> = Vec::new();
        for event in key_package_events {
            // TODO: Error handling for failure here
            let key_package: KeyPackage = self.parse_key_package(event)?;
            key_packages_vec.push(key_package);
        }

        // Distinguish the specific "invitee KP advertises too few proposals"
        // failure from other add_members errors so callers can react to it
        // programmatically. At runtime (openmls 0.8.1) the capability
        // mismatch travels as
        // `AddMembersError::CreateCommitError(CreateCommitError::ProposalValidationError(
        // ProposalValidationError::LeafNodeValidation(LeafNodeValidationError::UnsupportedProposals)))`.
        // If a future openmls version re-routes the leaf-node validation
        // error into `CreateCommitError::LeafNodeValidation(...)` directly
        // (a sibling variant exists), add that arm too. Anything else keeps
        // its flat-string shape.
        let (commit_message, welcome_message, _group_info) = mls_group
            .add_members(&self.provider, &mls_signer, &key_packages_vec)
            .map_err(|e| match e {
                AddMembersError::CreateCommitError(CreateCommitError::ProposalValidationError(
                    ProposalValidationError::LeafNodeValidation(
                        LeafNodeValidationError::UnsupportedProposals,
                    ),
                )) => Error::InviteeMissingRequiredProposal,
                other => Error::Group(other.to_string()),
            })?;

        let serialized_commit_message = commit_message
            .tls_serialize_detached()
            .map_err(|e| Error::Group(e.to_string()))?;

        let commit_event = self.build_message_event(
            &mls_group.group_id().into(),
            serialized_commit_message,
            None,
        )?;

        self.track_processed_message(
            commit_event.id,
            &mls_group,
            message_types::ProcessedMessageState::ProcessedCommit,
        )?;

        let serialized_welcome_message = welcome_message
            .tls_serialize_detached()
            .map_err(|e| Error::Group(e.to_string()))?;

        // Get relays for this group
        let group_relays = self
            .get_relays(&mls_group.group_id().into())?
            .into_iter()
            .collect::<Vec<_>>();

        let welcome_rumors = self.build_welcome_rumors_for_key_packages(
            &mls_group,
            serialized_welcome_message,
            key_package_events.to_vec(),
            &group_relays,
        )?;

        // let serialized_group_info = group_info
        //     .map(|g| {
        //         g.tls_serialize_detached()
        //             .map_err(|e| Error::Group(e.to_string()))
        //     })
        //     .transpose()?;

        Ok(UpdateGroupResult {
            evolution_event: commit_event,
            welcome_rumors, // serialized_group_info,
            mls_group_id: group_id.clone(),
        })
    }

    /// Remove members from a group
    ///
    /// If any removed members are in the group's admin list, the admin list is
    /// updated atomically within the same MLS commit.
    ///
    /// NOTE: This function doesn't merge the pending commit. Clients must call
    /// this function manually only after successful publish of the commit
    /// message to relays.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    /// * `pubkeys` - The Nostr public keys of the members to remove
    ///
    /// # Returns
    ///
    /// * `Ok(UpdateGroupResult)`
    ///
    /// # Errors
    ///
    /// * `Error::GroupNotFound` - If the group does not exist
    /// * `Error::OwnLeafNotFound` - If the caller's leaf node is missing
    /// * `Error::NotAdmin` - If the caller is not an admin
    /// * `Error::Group` - If no matching members are found, the caller attempts to
    ///   remove themselves, or the removal would leave the group with no admins
    /// * `Error::Extension` - If updating the admin list extension fails
    pub fn remove_members(
        &self,
        group_id: &GroupId,
        pubkeys: &[PublicKey],
    ) -> Result<UpdateGroupResult, Error> {
        let mut mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        let signer: SignatureKeyPair = self.load_mls_signer(&mls_group)?;

        let own_leaf = mls_group.own_leaf().ok_or(Error::OwnLeafNotFound)?;
        if !self.is_leaf_node_admin(group_id, own_leaf)? {
            return Err(Error::NotAdmin);
        }

        // Prevent self-removal — MLS does not allow a member to commit their own removal
        let own_pubkey = self.get_own_pubkey(&mls_group)?;
        if pubkeys.contains(&own_pubkey) {
            return Err(Error::Group(
                "Cannot remove yourself from the group".to_string(),
            ));
        }

        // Convert pubkeys to leaf indices
        let mut leaf_indices = Vec::new();

        for member in mls_group.members() {
            let pubkey = self.pubkey_for_member(&member)?;
            if pubkeys.contains(&pubkey) {
                leaf_indices.push(member.index);
            }
        }

        if leaf_indices.is_empty() {
            return Err(Error::Group(
                "No matching members found to remove".to_string(),
            ));
        }

        // TODO: Get a list of users to be added from any proposals and create welcome events for them

        // If any removed members are admins, prepare an updated extension to
        // strip them from the admin list in the same MLS commit.
        let group_data = NostrGroupDataExtension::from_group(&mls_group)?;
        let has_admin_removals = pubkeys.iter().any(|pk| group_data.admins.contains(pk));

        let updated_extensions = if has_admin_removals {
            let mut updated_data = group_data;
            for pk in pubkeys {
                updated_data.remove_admin(pk);
            }
            // Defensive check: the self-removal guard above makes this
            // unreachable when the caller is also an admin, but we protect
            // against future code paths that might skip that guard.
            if updated_data.admins.is_empty() {
                return Err(Error::Group(
                    "Cannot remove all admins from the group".to_string(),
                ));
            }
            let extension = Self::get_unknown_extension_from_group_data(&updated_data)?;
            let mut extensions = mls_group.extensions().clone();
            extensions.add_or_replace(extension)?;
            Some(extensions)
        } else {
            None
        };

        // Build a single commit containing removal proposals and, if needed,
        // a GroupContextExtensions proposal to update the admin list.
        let mut builder = mls_group
            .commit_builder()
            .propose_removals(leaf_indices.iter().cloned());

        if let Some(ext) = updated_extensions {
            builder = builder
                .propose_group_context_extensions(ext)
                .map_err(|e| Error::Group(e.to_string()))?;
        }

        // The PSK validation callback accepts all PSKs unconditionally. This is
        // safe here because this commit only contains removal and (optionally)
        // GroupContextExtensions proposals — no external PSK proposals are involved.
        let bundle = builder
            .load_psks(self.provider.storage())
            .map_err(|e| Error::Group(e.to_string()))?
            .build(
                self.provider.rand(),
                self.provider.crypto(),
                &signer,
                |_| true,
            )
            .map_err(|e| Error::Group(e.to_string()))?
            .stage_commit(&self.provider)
            .map_err(|e| Error::Group(e.to_string()))?;

        let welcome_option = bundle.to_welcome_msg();
        let (commit_message, _, _group_info) = bundle.into_contents();

        let serialized_commit_message = commit_message
            .tls_serialize_detached()
            .map_err(|e| Error::Group(e.to_string()))?;

        let commit_event = self.build_message_event(
            &mls_group.group_id().into(),
            serialized_commit_message,
            None,
        )?;

        self.track_processed_message(
            commit_event.id,
            &mls_group,
            message_types::ProcessedMessageState::ProcessedCommit,
        )?;

        // For now, if we find welcomes, throw an error.
        if welcome_option.is_some() {
            return Err(Error::Group(
                "Found welcomes when removing users".to_string(),
            ));
        }
        // let serialized_welcome_message = welcome_option
        //     .map(|w| {
        //         w.tls_serialize_detached()
        //             .map_err(|e| Error::Group(e.to_string()))
        //     })
        //     .transpose()?;

        // let serialized_group_info = group_info
        //     .map(|g| {
        //         g.tls_serialize_detached()
        //             .map_err(|e| Error::Group(e.to_string()))
        //     })
        //     .transpose()?;

        Ok(UpdateGroupResult {
            evolution_event: commit_event,
            welcome_rumors: None, // serialized_group_info,
            mls_group_id: group_id.clone(),
        })
    }

    fn update_group_data_extension(
        &self,
        mls_group: &mut MlsGroup,
        group_id: &GroupId,
        group_data: &NostrGroupDataExtension,
    ) -> Result<UpdateGroupResult, Error> {
        let own_leaf = mls_group.own_leaf().ok_or(Error::OwnLeafNotFound)?;
        if !self.is_leaf_node_admin(group_id, own_leaf)? {
            return Err(Error::NotAdmin);
        }

        let extension = Self::get_unknown_extension_from_group_data(group_data)?;
        let mut extensions = mls_group.extensions().clone();
        extensions.add_or_replace(extension)?;

        let signature_keypair = self.load_mls_signer(mls_group)?;
        let (message_out, _, _) = mls_group.update_group_context_extensions(
            &self.provider,
            extensions,
            &signature_keypair,
        )?;
        let commit_event = self.build_message_event(
            &mls_group.group_id().into(),
            message_out.tls_serialize_detached()?,
            None,
        )?;

        self.track_processed_message(
            commit_event.id,
            mls_group,
            message_types::ProcessedMessageState::ProcessedCommit,
        )?;

        Ok(UpdateGroupResult {
            evolution_event: commit_event,
            welcome_rumors: None,
            mls_group_id: group_id.clone(),
        })
    }

    /// Updates group data with the specified configuration
    ///
    /// This method allows updating one or more fields of the group data in a single operation.
    /// Only the fields specified in the update configuration will be modified.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    /// * `update` - Configuration specifying which fields to update and their new values
    ///
    /// # Returns
    ///
    /// * `Ok(UpdateGroupResult)` - Update result containing the evolution event
    /// * `Err(Error)` - If the group is not found or the operation fails
    ///
    /// # Examples
    ///
    /// ```rust,ignore
    /// // Update only the name
    /// let update = NostrGroupDataUpdate::new().name("New Group Name");
    /// mls.update_group_data(&group_id, update)?;
    ///
    /// // Update name and description together
    /// let update = NostrGroupDataUpdate::new()
    ///     .name("New Name")
    ///     .description("New Description");
    /// mls.update_group_data(&group_id, update)?;
    ///
    /// // Update image, clearing the existing one
    /// // Note: Setting image_hash to None automatically clears image_key, image_nonce, and image_upload_key
    /// let update = NostrGroupDataUpdate::new().image_hash(None);
    /// mls.update_group_data(&group_id, update)?;
    ///
    /// // Rotate the nostr_group_id for message routing (per MIP-01)
    /// let new_id = [0u8; 32]; // Generate a new random ID
    /// let update = NostrGroupDataUpdate::new().nostr_group_id(new_id);
    /// mls.update_group_data(&group_id, update)?;
    /// ```
    pub fn update_group_data(
        &self,
        group_id: &GroupId,
        update: NostrGroupDataUpdate,
    ) -> Result<UpdateGroupResult, Error> {
        let mut mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        let mut group_data = NostrGroupDataExtension::from_group(&mls_group)?;

        // Apply updates only for fields that are specified
        if let Some(name) = update.name {
            group_data.name = name;
        }

        if let Some(description) = update.description {
            group_data.description = description;
        }

        if let Some(image_hash) = update.image_hash {
            group_data.image_hash = image_hash;
            // When clearing the image (setting hash to None), also clear all related cryptographic material
            if image_hash.is_none() {
                group_data.image_key = None;
                group_data.image_nonce = None;
                group_data.image_upload_key = None;
            }
        }

        if let Some(image_key) = update.image_key {
            group_data.image_key = image_key;
        }

        if let Some(image_nonce) = update.image_nonce {
            group_data.image_nonce = image_nonce;
        }

        if let Some(image_upload_key) = update.image_upload_key {
            group_data.image_upload_key = image_upload_key;
        }

        if let Some(relays) = update.relays {
            group_data.relays = relays.into_iter().collect();
        }

        if let Some(ref admins) = update.admins {
            // Prune non-members and validate at least one admin remains
            group_data.admins = self.prune_and_validate_admin_update(group_id, admins)?;
        }

        if let Some(nostr_group_id) = update.nostr_group_id {
            group_data.nostr_group_id = nostr_group_id;
        }

        self.update_group_data_extension(&mut mls_group, group_id, &group_data)
    }

    /// Retrieves the set of relay URLs associated with an MLS group
    ///
    /// # Arguments
    ///
    /// * `group_id` - The MLS group ID
    ///
    /// # Returns
    ///
    /// * `Ok(BTreeSet<RelayUrl>)` - Set of relay URLs where group messages are published
    /// * `Err(Error)` - If there is an error accessing storage or the group is not found
    pub fn get_relays(&self, group_id: &GroupId) -> Result<BTreeSet<RelayUrl>, Error> {
        let relays = self
            .storage()
            .group_relays(group_id)
            .map_err(|e| Error::Group(e.to_string()))?;
        Ok(relays.into_iter().map(|r| r.relay_url).collect())
    }

    fn get_unknown_extension_from_group_data(
        group_data: &NostrGroupDataExtension,
    ) -> Result<Extension, Error> {
        let serialized_group_data = group_data.as_raw().tls_serialize_detached()?;

        Ok(Extension::Unknown(
            group_data.extension_type(),
            UnknownExtension(serialized_group_data),
        ))
    }

    /// Creates a new MLS group with the specified members and settings.
    ///
    /// This function creates a new MLS group with the given name, description, members, and administrators.
    /// It generates the necessary cryptographic credentials, configures the group with Nostr-specific extensions,
    /// and adds the specified members.
    ///
    /// # Single-Member Groups
    ///
    /// This method supports creating groups with only the creator (no additional members).
    /// When `member_key_package_events` is empty, the group is created with just the creator,
    /// and `welcome_rumors` in the result will be empty. This is useful for:
    /// - "Message to self" functionality
    /// - Setting up groups before inviting members
    ///
    /// # Arguments
    ///
    /// * `creator_public_key` - The Nostr public key of the group creator
    /// * `member_key_package_events` - A vector of Nostr events (Kind:443) containing key packages
    ///   for the initial group members. Can be empty to create a single-member group.
    /// * `config` - Group configuration including name, description, admins, and relays
    ///
    /// # Returns
    ///
    /// A `GroupResult` containing:
    /// - The created group
    /// - A Vec of UnsignedEvents (`welcome_rumors`) representing the welcomes to be sent to new
    ///   members. Empty if no members were added.
    ///
    /// # Errors
    ///
    /// Returns an `Error` if:
    /// - Credential generation fails
    /// - Group creation fails
    /// - Adding members fails (when members are provided)
    /// - Message serialization fails
    pub fn create_group(
        &self,
        creator_public_key: &PublicKey,
        member_key_package_events: Vec<Event>,
        config: NostrGroupConfigData,
    ) -> Result<GroupResult, Error> {
        // Get member pubkeys
        let member_pubkeys = member_key_package_events
            .clone()
            .into_iter()
            .map(|e| e.pubkey)
            .collect::<Vec<PublicKey>>();

        let admins = config.admins.clone();

        // Validate group members
        self.validate_group_members(creator_public_key, &member_pubkeys, &admins)?;

        let group_data = NostrGroupDataExtension::new(
            config.name,
            config.description,
            admins,
            config.relays.clone(),
            config.image_hash,
            config.image_key,
            config.image_nonce,
            None, // image_upload_key - will be set when image is uploaded
        );

        // Parse invitee KeyPackages up front — the LCD intersection below
        // needs their advertised capabilities to compute the group's
        // `RequiredCapabilities`.
        let key_packages_vec: Vec<KeyPackage> = member_key_package_events
            .iter()
            .map(|event| self.parse_key_package(event))
            .collect::<Result<_, _>>()?;

        // LCD: require only proposals every invitee's leaf can satisfy. An
        // all-modern group keeps [SelfRemove]; any legacy invitee strips it.
        // MDK itself supports every entry in SUPPORTED_PROPOSALS by
        // construction, so the creator's own leaf never needs an explicit
        // intersection term.
        //
        // Empty-invitee groups (single-member setup / "message to self") stay
        // permissive: `Iterator::all` is vacuously true over an empty set, so
        // a naive intersection would freeze in SUPPORTED_PROPOSALS and close
        // the group to later legacy `add_members` calls. We don't know the
        // future composition yet, and `RequiredCapabilities` is immutable
        // until a `GroupContextExtensions` upgrade ships, so empty stays
        // empty.
        let required_proposals: Vec<ProposalType> = if key_packages_vec.is_empty() {
            Vec::new()
        } else {
            SUPPORTED_PROPOSALS
                .iter()
                .copied()
                .filter(|pt| {
                    key_packages_vec
                        .iter()
                        .all(|kp| kp.leaf_node().capabilities().proposals().contains(pt))
                })
                .collect()
        };

        let extension = Self::get_unknown_extension_from_group_data(&group_data)?;
        let required_capabilities_extension =
            Extension::RequiredCapabilities(RequiredCapabilitiesExtension::new(
                &GROUP_CONTEXT_REQUIRED_EXTENSIONS,
                &required_proposals,
                &[],
            ));
        let extensions = Extensions::from_vec(vec![extension, required_capabilities_extension])?;

        // Build the group config
        let capabilities = self.capabilities();
        let sender_ratchet_config = SenderRatchetConfiguration::new(
            self.config.out_of_order_tolerance,
            self.config.maximum_forward_distance,
        );
        let group_config = MlsGroupCreateConfig::builder()
            .ciphersuite(self.ciphersuite)
            .wire_format_policy(MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY)
            .use_ratchet_tree_extension(true)
            .capabilities(capabilities)
            .with_group_context_extensions(extensions)
            .sender_ratchet_configuration(sender_ratchet_config)
            .max_past_epochs(self.config.max_past_epochs)
            .build();

        // Generate the creator's signer last, right before it is handed to
        // OpenMLS. `generate_credential_with_key` persists the fresh signer
        // into provider storage as a side effect, so any preflight failure
        // above (member validation, KeyPackage parse, extension build) must
        // happen before this line — otherwise an early error would leave an
        // orphan signer in storage.
        let (credential, signer) = self.generate_credential_with_key(creator_public_key)?;

        let mut mls_group =
            MlsGroup::new(&self.provider, &signer, &group_config, credential.clone())?;

        // Handle member addition and welcome message creation
        // For single-member groups (no additional members), we skip adding members
        // and return an empty welcome_rumors vec
        let welcome_rumors = if key_packages_vec.is_empty() {
            // Single-member group: no members to add, no welcome messages needed
            Vec::new()
        } else {
            // Add members to the group
            let (_, welcome_out, _group_info) =
                mls_group.add_members(&self.provider, &signer, &key_packages_vec)?;

            // IMPORTANT: Privacy-preserving group creation
            //
            // We intentionally DO NOT publish the initial commit to relays. Instead, we:
            // 1. Merge the pending commit locally (immediately below)
            // 2. Send Welcome messages directly to invited members
            //
            // This differs from the MLS specification (RFC 9420), which recommends waiting
            // for Delivery Service confirmation before applying commits. However, that
            // guidance assumes a centralized Delivery Service model.
            //
            // For initial group creation with Nostr relays, not publishing the commit is
            // the correct choice for security and privacy reasons:
            //
            // - PRIVACY: Publishing the commit would expose additional metadata on relays
            //   (timing, event patterns, correlation opportunities) with no functional benefit
            // - SECURITY: Invited members receive complete group state via Welcome messages;
            //   they do not need the commit to join the group
            // - NO RACE CONDITIONS: At creation time, only the creator exists in the group,
            //   so there are no other members who need to process this commit
            //
            // This approach minimizes observable events on relays while maintaining full
            // MLS security properties. The Welcome messages contain all cryptographic
            // material needed for invitees to participate in the group.
            //
            // NOTE: This is specific to initial group creation. For commits in established
            // groups (adding/removing members, updates), commits MUST be published to relays
            // so existing members can process them and stay in sync.
            mls_group.merge_pending_commit(&self.provider)?;

            // Serialize the welcome message and send it to the members
            let serialized_welcome_message = welcome_out.tls_serialize_detached()?;

            self.build_welcome_rumors_for_key_packages(
                &mls_group,
                serialized_welcome_message,
                member_key_package_events,
                &config.relays,
            )?
            .ok_or(Error::Welcome("Error creating welcome rumors".to_string()))?
        };

        // Save the NostrMLS Group
        let group = group_types::Group {
            mls_group_id: mls_group.group_id().clone().into(),
            nostr_group_id: group_data.nostr_group_id,
            name: group_data.name,
            description: group_data.description,
            admin_pubkeys: group_data.admins,
            last_message_id: None,
            last_message_at: None,
            last_message_processed_at: None,
            epoch: mls_group.epoch().as_u64(),
            state: group_types::GroupState::Active,
            image_hash: config.image_hash,
            image_key: config.image_key.map(mdk_storage_traits::Secret::new),
            image_nonce: config.image_nonce.map(mdk_storage_traits::Secret::new),
            self_update_state: group_types::SelfUpdateState::CompletedAt(Timestamp::now()),
        };

        self.storage().save_group(group.clone()).map_err(
            |e: mdk_storage_traits::groups::error::GroupError| Error::Group(e.to_string()),
        )?;

        // Save the group relays after saving the group
        self.storage()
            .replace_group_relays(&group.mls_group_id, config.relays.into_iter().collect())
            .map_err(|e| Error::Group(e.to_string()))?;

        Ok(GroupResult {
            group,
            welcome_rumors,
        })
    }

    /// Updates the current member's leaf node in an MLS group.
    /// Does not currently support updating any group attributes.
    ///
    /// This function performs a self-update operation in the specified MLS group by:
    /// 1. Loading the group from storage
    /// 2. Generating a new signature keypair
    /// 3. Storing the keypair
    /// 4. Creating and applying a self-update proposal
    ///
    /// NOTE: This function doesn't merge the pending commit. Clients must call this function manually only after successful publish of the commit message to relays.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The ID of the MLS group
    ///
    /// # Returns
    ///
    /// An UpdateGroupResult
    ///
    /// # Errors
    ///
    /// Returns a Error if:
    /// - The group cannot be loaded from storage
    /// - The specified group is not found
    /// - Failed to generate or store signature keypair
    /// - Failed to perform self-update operation
    pub fn self_update(&self, group_id: &GroupId) -> Result<UpdateGroupResult, Error> {
        let mut mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        tracing::debug!(target: "mdk_core::groups::self_update", "Current epoch: {:?}", mls_group.epoch().as_u64());

        // Load current signer
        let current_signer: SignatureKeyPair = self.load_mls_signer(&mls_group)?;

        // Get own leaf
        let own_leaf = mls_group.own_leaf().ok_or(Error::OwnLeafNotFound)?;

        let new_signature_keypair = SignatureKeyPair::new(self.ciphersuite.signature_algorithm())?;

        new_signature_keypair
            .store(self.provider.storage())
            .map_err(|e| Error::Provider(e.to_string()))?;

        let pubkey = BasicCredential::try_from(own_leaf.credential().clone())?
            .identity()
            .to_vec();

        let new_credential: BasicCredential = BasicCredential::new(pubkey);
        let new_credential_with_key = CredentialWithKey {
            credential: new_credential.into(),
            signature_key: new_signature_keypair.public().into(),
        };

        let new_signer_bundle = NewSignerBundle {
            signer: &new_signature_keypair,
            credential_with_key: new_credential_with_key.clone(),
        };

        let leaf_node_params = LeafNodeParameters::builder()
            .with_credential_with_key(new_credential_with_key)
            .with_capabilities(self.capabilities())
            .with_extensions(own_leaf.extensions().clone())
            .build();

        let commit_message_bundle = mls_group.self_update_with_new_signer(
            &self.provider,
            &current_signer,
            new_signer_bundle,
            leaf_node_params,
        )?;

        self.ensure_mixed_wire_format(&mut mls_group);

        // Serialize the message
        let serialized_commit_message = commit_message_bundle.commit().tls_serialize_detached()?;

        let commit_event = self.build_message_event(
            &mls_group.group_id().into(),
            serialized_commit_message,
            None,
        )?;

        self.track_processed_message(
            commit_event.id,
            &mls_group,
            message_types::ProcessedMessageState::ProcessedCommit,
        )?;

        let serialized_welcome_message = commit_message_bundle
            .welcome()
            .map(|w| {
                w.tls_serialize_detached()
                    .map_err(|e| Error::Group(e.to_string()))
            })
            .transpose()?;

        // For now, if we find welcomes, throw an error.
        if serialized_welcome_message.is_some() {
            return Err(Error::Group(
                "Found welcomes when performing a self update".to_string(),
            ));
        }

        Ok(UpdateGroupResult {
            evolution_event: commit_event,
            welcome_rumors: None, // serialized_group_info,
            mls_group_id: group_id.clone(),
        })
    }

    /// Converges a member's local wire-format policy to
    /// `MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY`, MDK's current default.
    ///
    /// Called from commit-emitting operations as a piggy-backed hygiene step:
    /// every rotation brings a member's local state forward so that a future
    /// `GroupContextExtensions` upgrade adding `SelfRemove` to the group's
    /// `RequiredCapabilities` finds members already able to accept SelfRemove
    /// `PublicMessage` proposals. No-op when the policy is already converged,
    /// so it's cheap to call unconditionally.
    ///
    /// Failures to persist the updated configuration are logged at `warn` and
    /// swallowed. The in-memory config is still correct for the current
    /// session; the next rotation will retry. The caller's primary operation
    /// is not affected.
    ///
    /// # Note on full-config rebuild
    ///
    /// `MlsGroup::set_configuration` replaces the entire `MlsGroupJoinConfig`.
    /// Ideally we would preserve every non-wire-format field from the group's
    /// stored config, but openmls 0.8.1 only exposes getters for three of the
    /// six fields (`wire_format_policy`, `padding_size`,
    /// `sender_ratchet_configuration`). Falling back to builder defaults for
    /// the hidden fields would regress known MDK settings
    /// (`max_past_epochs`, `use_ratchet_tree_extension`). MDK's existing
    /// invariant — every site that writes an `MlsGroupJoinConfig`
    /// (`create_group`, `join_group_from_welcome`, `try_self_remove`) rebuilds
    /// it from `self.config` — keeps the stored config in sync with the
    /// current MDK instance, so rebuilding here matches expectations for
    /// the other fields.
    fn ensure_mixed_wire_format(&self, mls_group: &mut MlsGroup) {
        if mls_group.configuration().wire_format_policy() == MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY {
            return;
        }

        let sender_ratchet_config = SenderRatchetConfiguration::new(
            self.config.out_of_order_tolerance,
            self.config.maximum_forward_distance,
        );
        let converged_config = MlsGroupJoinConfig::builder()
            .wire_format_policy(MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY)
            .use_ratchet_tree_extension(true)
            .sender_ratchet_configuration(sender_ratchet_config)
            .max_past_epochs(self.config.max_past_epochs)
            .build();

        tracing::debug!(
            target: "mdk_core::groups::ensure_mixed_wire_format",
            "Converging local wire-format policy to MIXED_CIPHERTEXT"
        );

        if let Err(e) = mls_group.set_configuration(self.storage(), &converged_config) {
            tracing::warn!(
                target: "mdk_core::groups::ensure_mixed_wire_format",
                "Failed to persist converged wire-format policy: {e}. \
                 In-memory config is correct for this session; \
                 persisted config may be stale."
            );
        }
    }

    /// Attempts to create a SelfRemove proposal, falling back to Remove for legacy groups.
    ///
    /// SelfRemove proposals MUST be sent as MLS PublicMessage per the MLS Extensions draft.
    /// Since groups default to MIXED_CIPHERTEXT (outgoing: ciphertext), this method
    /// temporarily switches the wire format policy to allow the PublicMessage, then
    /// restores it. This preserves PrivateMessage for all other group operations.
    ///
    /// For legacy groups (PURE_CIPHERTEXT, created before SelfRemove support), peers
    /// reject PublicMessage on the incoming side. These groups fall back to a Remove
    /// proposal where sender == removed member.
    fn try_self_remove(
        &self,
        group: &mut MlsGroup,
        signer: &SignatureKeyPair,
    ) -> Result<MlsMessageOut, Error> {
        // RequiredCapabilities gate. RFC 9420 §7.2 guarantees every leaf's
        // advertised proposals cover RequiredCapabilities, so if SelfRemove
        // is required, OpenMLS's commit-time validate_proposal_type_support
        // is guaranteed to accept it on every peer. If SelfRemove is NOT
        // required, fall back to legacy Remove — a mixed-composition group's
        // non-admin-committable SelfRemove path would orphan on at least one
        // peer. This is also our wire-format guarantee: modern groups are
        // born with `RequiredCapabilities = [SelfRemove]` and
        // `MIXED_CIPHERTEXT` in lockstep (PR #236), and legacy groups have
        // neither, so this single check covers both invariants in practice.
        // Sound but over-conservative for formerly-mixed groups that became
        // all-modern: `RequiredCapabilities` is frozen at creation and only
        // a `GroupContextExtensions` commit can mutate it.
        let self_remove_required = group
            .extensions()
            .required_capabilities()
            .is_some_and(|rc| rc.proposal_types().contains(&ProposalType::SelfRemove));

        if !self_remove_required {
            tracing::debug!(
                target: "mdk_core::groups::leave_group",
                "SelfRemove not in group's RequiredCapabilities, \
                 falling back to Remove proposal"
            );
            return group
                .leave_group(&self.provider, signer)
                .map_err(|e| Error::Group(e.to_string()));
        }

        // MIXED_CIPHERTEXT groups: temporarily switch to plaintext for SelfRemove.
        let sender_ratchet_config = SenderRatchetConfiguration::new(
            self.config.out_of_order_tolerance,
            self.config.maximum_forward_distance,
        );

        let plaintext_config = MlsGroupJoinConfig::builder()
            .wire_format_policy(MIXED_PLAINTEXT_WIRE_FORMAT_POLICY)
            .use_ratchet_tree_extension(true)
            .sender_ratchet_configuration(sender_ratchet_config)
            .max_past_epochs(self.config.max_past_epochs)
            .build();

        let ciphertext_config = MlsGroupJoinConfig::builder()
            .wire_format_policy(MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY)
            .use_ratchet_tree_extension(true)
            .sender_ratchet_configuration(sender_ratchet_config)
            .max_past_epochs(self.config.max_past_epochs)
            .build();

        group
            .set_configuration(self.storage(), &plaintext_config)
            .map_err(|e| Error::Group(format!("Failed to switch wire format: {e}")))?;

        let result = group.leave_group_via_self_remove(&self.provider, signer);

        // Restore ciphertext mode. set_configuration updates in-memory state
        // unconditionally (always succeeds) and then persists to storage (can fail).
        // If the persist fails, the in-memory config is still correct for this
        // session. The stale persisted config would only matter after an app restart
        // where the member is still in the group (SelfRemove wasn't committed).
        if let Err(e) = group.set_configuration(self.storage(), &ciphertext_config) {
            tracing::error!(
                target: "mdk_core::groups::leave_group",
                "Failed to persist restored ciphertext wire format: {e}. \
                 In-memory config is correct; persisted config may be stale."
            );
        }

        result.map_err(|e| Error::Group(e.to_string()))
    }

    /// Self-demote from admin status in a group.
    ///
    /// Per MIP-03, admins MUST demote themselves before sending a SelfRemove
    /// proposal. This method removes the caller's public key from `admin_pubkeys`
    /// via a GroupContextExtensions commit.
    ///
    /// If the caller is the last admin, they MUST designate a successor first using
    /// `update_group_data` to add another admin before calling this method.
    ///
    /// After this method succeeds and the commit is merged, the caller can use
    /// `leave_group` to send a SelfRemove proposal.
    pub fn self_demote(&self, group_id: &GroupId) -> Result<UpdateGroupResult, Error> {
        let mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        let own_leaf = mls_group.own_leaf().ok_or(Error::OwnLeafNotFound)?;
        let own_pubkey = self.pubkey_for_leaf_node(own_leaf)?;
        let group_data = NostrGroupDataExtension::from_group(&mls_group)?;

        if !group_data.admins.contains(&own_pubkey) {
            return Err(Error::NotAdmin);
        }

        // Count only admins who are actual group members (ignores stale entries
        // from admins who departed without a GroupContextExtensions update).
        let active_admins: Vec<_> = group_data
            .admins
            .into_iter()
            .filter(|pk| {
                mls_group.members().any(|member| {
                    BasicCredential::try_from(member.credential)
                        .ok()
                        .and_then(|cred| self.parse_credential_identity(cred.identity()).ok())
                        .is_some_and(|member_pk| &member_pk == pk)
                })
            })
            .collect();

        if active_admins.len() <= 1 {
            return Err(Error::Group(
                "Cannot self-demote: last active admin. \
                 Designate another admin first using update_group_data."
                    .to_string(),
            ));
        }

        let new_admins: Vec<_> = active_admins
            .into_iter()
            .filter(|pk| pk != &own_pubkey)
            .collect();

        self.update_group_data(group_id, NostrGroupDataUpdate::new().admins(new_admins))
    }

    /// Create a proposal to leave the group
    ///
    /// Sends a SelfRemove proposal (new protocol) if the group supports it, otherwise
    /// falls back to a Remove proposal for legacy groups using PURE_CIPHERTEXT policy.
    /// The proposal must be committed by another member — the departing member cannot
    /// commit their own removal.
    ///
    /// # Arguments
    ///
    /// * `group_id` - The ID of the MLS group
    ///
    /// # Returns
    /// * `Ok(UpdateGroupResult)` - Contains the leave proposal event that must be processed by another member
    pub fn leave_group(&self, group_id: &GroupId) -> Result<UpdateGroupResult, Error> {
        let mut group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        // Per MIP-03, admins MUST self-demote before SelfRemoving.
        // Use self_demote() first, then call leave_group() again.
        let own_leaf = group.own_leaf().ok_or(Error::OwnLeafNotFound)?;
        if self.is_leaf_node_admin(group_id, own_leaf)? {
            return Err(Error::Group(
                "Admins must self-demote before leaving. \
                 Use self_demote() first."
                    .to_string(),
            ));
        }

        let signer: SignatureKeyPair = self.load_mls_signer(&group)?;

        let leave_message = self.try_self_remove(&mut group, &signer)?;

        let serialized_message_out = leave_message
            .tls_serialize_detached()
            .map_err(|e| Error::Group(e.to_string()))?;

        let evolution_event =
            self.build_message_event(&group.group_id().into(), serialized_message_out, None)?;

        self.track_processed_message(
            evolution_event.id,
            &group,
            message_types::ProcessedMessageState::Processed,
        )?;

        Ok(UpdateGroupResult {
            evolution_event,
            welcome_rumors: None,
            mls_group_id: group_id.clone(),
        })
    }

    /// Delete all locally stored messages for a group.
    ///
    /// Removes decrypted message content from storage. The group remains
    /// active and can continue to receive new messages. Processing records
    /// are preserved to prevent re-processing of already-seen events.
    ///
    /// Returns the number of messages deleted.
    ///
    /// This is a local-only operation — no MLS proposals or Nostr events
    /// are published.
    pub fn delete_messages_for_group(&self, group_id: &GroupId) -> Result<usize, Error> {
        self.storage()
            .delete_messages_for_group(group_id)
            .map_err(|e| Error::Group(e.to_string()))
    }

    /// Delete all local state for a group.
    ///
    /// Removes everything MDK stores for this group: messages, processed
    /// message records, MLS tree state, epoch secrets, key material, relay
    /// associations, proposals, and snapshots.
    ///
    /// After deletion, the group cannot receive or decrypt new messages.
    /// Call `leave_group()` first if you want to notify other members of
    /// your departure before cleaning up local state.
    ///
    /// Idempotent: deleting a nonexistent group is a no-op.
    ///
    /// This is a local-only operation — no MLS proposals or Nostr events
    /// are published.
    pub fn delete_group(&self, group_id: &GroupId) -> Result<(), Error> {
        self.storage().delete_group(group_id)?;
        self.epoch_snapshots.remove_group(group_id);
        Ok(())
    }

    /// Clear (rollback) a pending commit without merging it.
    ///
    /// This should be called when the Kind:445 publish fails after creating a commit
    /// via `add_members`, `remove_members`, or `self_update`. Without clearing, the
    /// pending commit blocks all future group operations with "Can't execute operation
    /// because a pending commit exists".
    ///
    /// The group returns to its pre-commit state — no epoch advance, no member changes.
    ///
    /// When the pending commit is a `self_update`, the new `SignatureKeyPair` that was
    /// stored in the provider before creating the commit is deleted from storage, since
    /// the group reverts to the previous signer.
    ///
    /// # Arguments
    /// * `group_id` - the MlsGroup GroupId value
    pub fn clear_pending_commit(&self, group_id: &GroupId) -> Result<(), Error> {
        let mut mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        // Detect whether the pending commit is a self-update BEFORE clearing,
        // since clear_pending_commit() consumes the staged commit.
        // If it is, capture the new leaf's signature public key so we can delete
        // the orphaned keypair from storage after the rollback.
        let self_update_new_pubkey: Option<Vec<u8>> =
            mls_group.pending_commit().and_then(|staged_commit| {
                let has_update_signal = staged_commit.update_path_leaf_node().is_some()
                    || staged_commit.update_proposals().next().is_some();
                let no_non_update_proposals = staged_commit
                    .queued_proposals()
                    .all(|p| matches!(p.proposal(), Proposal::Update(_)));
                if has_update_signal && no_non_update_proposals {
                    staged_commit
                        .update_path_leaf_node()
                        .map(|leaf| leaf.signature_key().as_slice().to_vec())
                } else {
                    None
                }
            });

        mls_group
            .clear_pending_commit(self.provider.storage())
            .map_err(|e| Error::Provider(e.to_string()))?;

        // If this was a self-update, delete the orphaned new signature keypair.
        // OpenMLS reverts to the old signer after clear_pending_commit; the new
        // keypair was stored eagerly in self_update() and is now unreachable.
        if let Some(pubkey_bytes) = self_update_new_pubkey {
            SignatureKeyPair::delete(
                self.provider.storage(),
                &pubkey_bytes,
                self.ciphersuite.signature_algorithm(),
            )
            .map_err(|e| Error::Provider(e.to_string()))?;
        }

        Ok(())
    }

    /// Merge any pending commits.
    /// This should be called AFTER publishing the Kind:445 message that contains a commit message to mitigate race conditions
    ///
    /// # Arguments
    /// * `group_id` - the MlsGroup GroupId value
    ///
    /// Returns
    /// * `Ok(())` - if the commits were merged successfully
    /// * Err(GroupError) - if something goes wrong
    pub fn merge_pending_commit(&self, group_id: &GroupId) -> Result<(), Error> {
        let mut mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;

        // Detect whether the pending commit is a self-update BEFORE merging,
        // since merge_pending_commit() consumes the staged commit.
        let is_self_update = mls_group.pending_commit().is_some_and(|staged_commit| {
            // Must contain at least one update signal
            let has_update_signal = staged_commit.update_path_leaf_node().is_some()
                || staged_commit.update_proposals().next().is_some();
            // No non-update proposals present (add/remove/psk/etc.).
            // Note: `all()` is vacuously true on an empty iterator, which is
            // the expected case for path-based self-updates where the update
            // comes via `update_path_leaf_node()` rather than explicit Update
            // proposals in `queued_proposals()`.
            let no_non_update_proposals = staged_commit
                .queued_proposals()
                .all(|p| matches!(p.proposal(), Proposal::Update(_)));
            has_update_signal && no_non_update_proposals
        });

        mls_group.merge_pending_commit(&self.provider)?;

        // Save MIP-03 and MIP-04 exporter secrets for the new epoch
        self.exporter_secret(group_id)?;
        #[cfg(feature = "mip04")]
        {
            let mip04_secret = self.mip04_exporter_secret(group_id)?;
            self.storage()
                .save_group_mip04_exporter_secret(mip04_secret)
                .map_err(|_| Error::Group("Failed to save MIP-04 exporter secret".to_string()))?;
        }

        let min_epoch_to_keep = mls_group
            .epoch()
            .as_u64()
            .saturating_sub(self.config.max_past_epochs as u64);
        self.storage()
            .prune_group_exporter_secrets_before_epoch(group_id, min_epoch_to_keep)
            .map_err(|_| Error::Group("Failed to prune exporter secrets".to_string()))?;

        // Sync the stored group metadata with the updated MLS group state
        self.sync_group_metadata_from_mls(group_id)?;

        // If this was actually a self-update commit, record the timestamp.
        // This correctly handles:
        // - Post-join self-updates (transitions Required → CompletedAt)
        // - Periodic rotation self-updates (updates CompletedAt timestamp)
        // - Non-self-update commits (leaves self_update_state untouched)
        if is_self_update {
            let mut group = self.get_group(group_id)?.ok_or(Error::GroupNotFound)?;
            group.self_update_state = group_types::SelfUpdateState::CompletedAt(Timestamp::now());
            self.storage()
                .save_group(group)
                .map_err(|e| Error::Group(e.to_string()))?;
        }

        Ok(())
    }

    /// Synchronizes the stored group metadata with the current MLS group state
    ///
    /// This helper method ensures that all fields in the stored `group_types::Group`
    /// remain consistent with the MLS group state and extensions after operations.
    /// It should be called after any operation that changes the group state or extensions.
    ///
    /// # Arguments
    /// * `group_id` - The MLS group ID to synchronize
    ///
    /// # Returns
    /// * `Ok(())` - if synchronization succeeds
    /// * `Err(Error)` - if the group is not found or synchronization fails
    pub fn sync_group_metadata_from_mls(&self, group_id: &GroupId) -> Result<(), Error> {
        let mls_group = self.load_mls_group(group_id)?.ok_or(Error::GroupNotFound)?;
        let mut stored_group = self.get_group(group_id)?.ok_or(Error::GroupNotFound)?;

        // Validate the mandatory group-data extension FIRST before making any state changes.
        // Receiver-side commits are checked before merge; this guard covers local merge
        // and storage-sync paths so invalid MLS metadata is not copied into storage.
        let group_data = NostrGroupDataExtension::from_group(&mls_group)?;
        self.validate_active_admins_in_group(&mls_group, &group_data.admins)?;
        // Only after successful validation, update epoch and metadata from MLS group
        stored_group.epoch = mls_group.epoch().as_u64();

        // Update extension data from NostrGroupDataExtension
        stored_group.name = group_data.name;
        stored_group.description = group_data.description;
        stored_group.image_hash = group_data.image_hash;
        stored_group.image_key = group_data.image_key.map(mdk_storage_traits::Secret::new);
        stored_group.image_nonce = group_data.image_nonce.map(mdk_storage_traits::Secret::new);
        stored_group.admin_pubkeys = group_data.admins;
        stored_group.nostr_group_id = group_data.nostr_group_id;

        // Sync relays atomically - replace entire relay set with current extension data
        self.storage()
            .replace_group_relays(group_id, group_data.relays)
            .map_err(|e| Error::Group(e.to_string()))?;

        self.storage()
            .save_group(stored_group)
            .map_err(|e| Error::Group(e.to_string()))?;

        Ok(())
    }

    /// Validates the members and admins of a group during creation
    ///
    /// # Arguments
    /// * `creator_pubkey` - The public key of the group creator
    /// * `member_pubkeys` - List of public keys for group members
    /// * `admin_pubkeys` - List of public keys for group admins
    ///
    /// # Returns
    /// * `Ok(true)` if validation passes
    /// * `Err(GroupError::InvalidParameters)` if validation fails
    ///
    /// # Validation Rules
    /// - Creator must be an admin but not included in member list
    /// - All admins must also be members (except creator)
    ///
    /// # Errors
    /// Returns `GroupError::InvalidParameters` with descriptive message if:
    /// - Creator is not an admin
    /// - Creator is in member list
    /// - Any admin, other than the creator, is not a member
    fn validate_group_members(
        &self,
        creator_pubkey: &PublicKey,
        member_pubkeys: &[PublicKey],
        admin_pubkeys: &[PublicKey],
    ) -> Result<bool, Error> {
        // Creator must be an admin
        if !admin_pubkeys.contains(creator_pubkey) {
            return Err(Error::Group("Creator must be an admin".to_string()));
        }

        // Creator must not be included as a member
        if member_pubkeys.contains(creator_pubkey) {
            return Err(Error::Group(
                "Creator must not be included as a member".to_string(),
            ));
        }

        // Check that admins are valid pubkeys and are members
        for pubkey in admin_pubkeys.iter() {
            if !member_pubkeys.contains(pubkey) && creator_pubkey != pubkey {
                return Err(Error::Group("Admin must be a member".to_string()));
            }
        }
        Ok(true)
    }

    /// Prunes non-member public keys from the proposed admin list and validates
    /// that at least one valid admin remains.
    ///
    /// # Errors
    /// Returns `Error::UpdateGroupContextExts` if no valid admins remain after pruning.
    fn prune_and_validate_admin_update(
        &self,
        group_id: &GroupId,
        new_admins: &[PublicKey],
    ) -> Result<BTreeSet<PublicKey>, Error> {
        let current_members = self.get_members(group_id)?;

        let valid_admins: BTreeSet<PublicKey> = new_admins
            .iter()
            .filter(|admin| current_members.contains(admin))
            .copied()
            .collect();

        if valid_admins.is_empty() {
            return Err(Error::UpdateGroupContextExts(
                "Admin set cannot be empty".to_string(),
            ));
        }

        Ok(valid_admins)
    }

    /// Records a processed message so the client can track message state.
    fn track_processed_message(
        &self,
        event_id: EventId,
        mls_group: &MlsGroup,
        state: message_types::ProcessedMessageState,
    ) -> Result<(), Error> {
        let processed_message = message_types::ProcessedMessage {
            wrapper_event_id: event_id,
            message_event_id: None,
            processed_at: Timestamp::now(),
            epoch: Some(mls_group.epoch().as_u64()),
            mls_group_id: Some(mls_group.group_id().into()),
            state,
            failure_reason: None,
        };
        self.storage()
            .save_processed_message(processed_message)
            .map_err(|e| Error::Message(e.to_string()))
    }

    /// Creates a ChaCha20-Poly1305 encrypted message event Kind: 445 signed with an ephemeral keypair.
    ///
    /// Per MIP-03, the encryption key is derived via `MLS-Exporter("marmot", "group-event", 32)`,
    /// a random 12-byte nonce is generated per event. No AAD is used per MIP-03.
    /// The content format is `base64(nonce || ciphertext)`.
    pub(crate) fn build_message_event(
        &self,
        group_id: &GroupId,
        serialized_content: Vec<u8>,
        tags: Option<Vec<EventTag>>,
    ) -> Result<Event, Error> {
        let group = self.get_group(group_id)?.ok_or(Error::GroupNotFound)?;

        // Derive the encryption key via MLS exporter (stable per epoch)
        let secret: group_types::GroupExporterSecret = self.exporter_secret(group_id)?;
        let encrypted_content = encrypt_message_with_exporter_secret(&secret, &serialized_content)?;

        // Generate ephemeral key for signing (MUST NOT be reused across events)
        let ephemeral_nostr_keys: Keys = Keys::generate();

        let tag: Tag = Tag::custom(TagKind::h(), [hex::encode(group.nostr_group_id)]);
        let encoding_tag: Tag = Tag::custom(TagKind::Custom("encoding".into()), ["base64"]);

        let mut builder = EventBuilder::new(Kind::MlsGroupMessage, encrypted_content)
            .tag(tag)
            .tag(encoding_tag);

        if let Some(tags) = tags {
            builder = builder.tags(tags.into_iter().map(|t| t.into_tag()));
        }

        let event = builder.sign_with_keys(&ephemeral_nostr_keys)?;

        Ok(event)
    }

    pub(crate) fn build_welcome_rumors_for_key_packages(
        &self,
        group: &MlsGroup,
        serialized_welcome: Vec<u8>,
        key_package_events: Vec<Event>,
        group_relays: &[RelayUrl],
    ) -> Result<Option<Vec<UnsignedEvent>>, Error> {
        let committer_pubkey = self.get_own_pubkey(group)?;
        let mut welcome_rumors_vec = Vec::new();

        for event in key_package_events {
            // SECURITY: Always use base64 encoding with explicit encoding tag per MIP-00/MIP-02.
            // This prevents downgrade attacks and parsing ambiguity across clients.
            let encoding = ContentEncoding::Base64;

            let encoded_welcome = encode_content(&serialized_welcome, encoding);

            tracing::debug!(
                target: "mdk_core::groups",
                "Encoded welcome using {} format",
                encoding.as_tag_value()
            );

            let tags = vec![
                Tag::from_standardized(TagStandard::Relays(group_relays.to_vec())),
                Tag::event(event.id),
                Tag::client(format!("MDK/{}", env!("CARGO_PKG_VERSION"))),
                Tag::custom(
                    TagKind::Custom("encoding".into()),
                    [encoding.as_tag_value()],
                ),
            ];

            // Build welcome event rumors for each new user
            let welcome_rumor = EventBuilder::new(Kind::MlsWelcome, encoded_welcome)
                .tags(tags)
                .build(committer_pubkey);

            welcome_rumors_vec.push(welcome_rumor);
        }

        let welcome_rumors = if !welcome_rumors_vec.is_empty() {
            Some(welcome_rumors_vec)
        } else {
            None
        };

        Ok(welcome_rumors)
    }
}

fn missing_leaf_capabilities_error(leaf_index: u32) -> Error {
    Error::Group(format!("missing capabilities for leaf index {leaf_index}"))
}

#[cfg(test)]
mod tests {
    use std::collections::BTreeSet;
    use std::iter::once;

    use mdk_memory_storage::MdkMemoryStorage;
    use mdk_storage_traits::groups::GroupStorage;
    use mdk_storage_traits::messages::{MessageStorage, types as message_types};
    use nostr::{Keys, PublicKey};
    use openmls::prelude::{BasicCredential, ProposalType};
    use openmls_basic_credential::SignatureKeyPair;

    use super::NostrGroupDataExtension;
    use crate::Error;
    use crate::constant::NOSTR_GROUP_DATA_EXTENSION_TYPE;
    use crate::groups::{NostrGroupDataUpdate, ProposalUpgradability};
    use crate::messages::MessageProcessingResult;
    use crate::test_util::*;
    use crate::tests::create_test_mdk;

    #[test]
    fn test_validate_group_members() {
        let mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();
        let member_pks: Vec<PublicKey> = members.iter().map(|k| k.public_key()).collect();

        // Test valid configuration
        assert!(
            mdk.validate_group_members(&creator_pk, &member_pks, &admins)
                .is_ok()
        );

        // Test creator not in admin list
        let bad_admins = vec![member_pks[0]];
        assert!(
            mdk.validate_group_members(&creator_pk, &member_pks, &bad_admins)
                .is_err()
        );

        // Test creator in member list
        let bad_members = vec![creator_pk, member_pks[0]];
        assert!(
            mdk.validate_group_members(&creator_pk, &bad_members, &admins)
                .is_err()
        );

        // Test admin not in member list
        let non_member = Keys::generate().public_key();
        let bad_admins = vec![creator_pk, non_member];
        assert!(
            mdk.validate_group_members(&creator_pk, &member_pks, &bad_admins)
                .is_err()
        );
    }

    #[test]
    fn test_create_group_basic() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Verify group was created with correct members
        let members = creator_mdk
            .get_members(group_id)
            .expect("Failed to get members");

        assert_eq!(members.len(), 3); // creator + 2 initial members
        assert!(members.contains(&creator_pk));
        for member_keys in &initial_members {
            assert!(members.contains(&member_keys.public_key()));
        }
    }

    /// Test creating a group with only the creator (no additional members).
    /// This is useful for "message to self" functionality, setting up groups
    /// before inviting members, and multi-device scenarios.
    #[test]
    fn test_create_single_member_group() {
        let creator_mdk = create_test_mdk();
        let creator = Keys::generate();
        let creator_pk = creator.public_key();

        // Create a group with no additional members - only the creator
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                Vec::new(), // No additional members
                create_nostr_group_config_data(vec![creator_pk]),
            )
            .expect("Failed to create single-member group");

        let group_id = &create_result.group.mls_group_id;

        // Verify welcome_rumors is empty (no members to welcome)
        assert!(
            create_result.welcome_rumors.is_empty(),
            "Single-member group should have no welcome rumors"
        );

        // Verify only the creator is in the group
        let members = creator_mdk
            .get_members(group_id)
            .expect("Failed to get members");

        assert_eq!(
            members.len(),
            1,
            "Single-member group should have exactly 1 member"
        );
        assert!(
            members.contains(&creator_pk),
            "Creator should be in the group"
        );

        // Verify group metadata was saved correctly
        let group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        assert_eq!(group.name, "Test Group");
        assert!(group.admin_pubkeys.contains(&creator_pk));
    }

    #[test]
    fn test_get_members() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Test get_members
        let members = creator_mdk
            .get_members(group_id)
            .expect("Failed to get members");

        assert_eq!(members.len(), 3); // creator + 2 initial members
        assert!(members.contains(&creator_pk));
        for member_keys in &initial_members {
            assert!(members.contains(&member_keys.public_key()));
        }
    }

    #[test]
    fn test_add_members_epoch_advancement() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the initial group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Get initial epoch
        let initial_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");
        let initial_epoch = initial_group.epoch;

        // Create key package event for new member
        let new_member = Keys::generate();
        let new_key_package_event = create_key_package_event(&creator_mdk, &new_member);

        // Add the new member
        let _add_result = creator_mdk
            .add_members(group_id, &[new_key_package_event])
            .expect("Failed to add member");

        // Merge the pending commit for the member addition
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for member addition");

        // Verify the MLS group epoch was advanced by checking the actual MLS group
        let mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let final_mls_epoch = mls_group.epoch().as_u64();

        assert!(
            final_mls_epoch > initial_epoch,
            "MLS group epoch should advance after adding members (initial: {}, final: {})",
            initial_epoch,
            final_mls_epoch
        );

        // Verify the new member was added
        let final_members = creator_mdk
            .get_members(group_id)
            .expect("Failed to get members");
        assert!(
            final_members.contains(&new_member.public_key()),
            "New member should be in the group"
        );
        assert_eq!(
            final_members.len(),
            4, // creator + 2 initial + 1 new = 4 total
            "Should have 4 total members"
        );
    }

    #[test]
    fn test_get_own_pubkey() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");

        // Test get_own_pubkey
        let own_pubkey = creator_mdk
            .get_own_pubkey(&mls_group)
            .expect("Failed to get own pubkey");

        assert_eq!(
            own_pubkey, creator_pk,
            "Own pubkey should match creator pubkey"
        );
    }

    #[test]
    fn test_admin_check() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Test admin check - verify creator is in admin list
        let stored_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        assert!(
            stored_group.admin_pubkeys.contains(&creator_pk),
            "Creator should be admin"
        );
    }

    #[test]
    fn test_admin_permission_checks() {
        let admin_mdk = create_test_mdk();
        let non_admin_mdk = create_test_mdk();

        // Generate keys
        let admin_keys = Keys::generate();
        let non_admin_keys = Keys::generate();
        let member1_keys = Keys::generate();

        let admin_pk = admin_keys.public_key();
        let _non_admin_pk = non_admin_keys.public_key();
        let member1_pk = member1_keys.public_key();

        // Create key package events for initial members
        let non_admin_event = create_key_package_event(&admin_mdk, &non_admin_keys);
        let member1_event = create_key_package_event(&admin_mdk, &member1_keys);

        // Create group with admin as creator, non_admin and member1 as members
        // Only admin is an admin
        let create_result = admin_mdk
            .create_group(
                &admin_pk,
                vec![non_admin_event.clone(), member1_event.clone()],
                create_nostr_group_config_data(vec![admin_pk]), // Only admin is an admin
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        admin_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Create a new member to add
        let new_member_keys = Keys::generate();
        let _new_member_pk = new_member_keys.public_key();
        let new_member_event = create_key_package_event(&non_admin_mdk, &new_member_keys);

        // Test that admin can add members (should work)
        let add_result = admin_mdk.add_members(group_id, &[new_member_event]);
        assert!(add_result.is_ok(), "Admin should be able to add members");

        // Merge the pending commit for the member addition
        admin_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for member addition");

        // Test that admin can remove members (should work)
        let remove_result = admin_mdk.remove_members(group_id, &[member1_pk]);
        assert!(
            remove_result.is_ok(),
            "Admin should be able to remove members"
        );

        // Note: Testing non-admin permissions would require the non-admin user to actually
        // be part of the MLS group, which would require processing the welcome message.
        // For now, we've verified that admin permissions work correctly.
    }

    /// Test that admin authorization reads from the current MLS group state (NostrGroupDataExtension)
    /// rather than from potentially stale stored metadata.
    ///
    /// This test addresses issue #50: Admin Authorization Uses Stale Stored Metadata Instead of MLS State
    /// See: <https://github.com/marmot-protocol/mdk/issues/50>
    #[test]
    fn test_admin_check_uses_mls_state_not_stale_storage() {
        let creator_mdk = create_test_mdk();

        // Generate keys
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();

        let alice_pk = alice_keys.public_key();
        let bob_pk = bob_keys.public_key();
        let _charlie_pk = charlie_keys.public_key();

        // Create key package events for members
        let bob_event = create_key_package_event(&creator_mdk, &bob_keys);
        let charlie_event = create_key_package_event(&creator_mdk, &charlie_keys);

        // Create group with Alice as the ONLY admin
        let create_result = creator_mdk
            .create_group(
                &alice_pk,
                vec![bob_event, charlie_event],
                create_nostr_group_config_data(vec![alice_pk]), // Only Alice is admin
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id;

        // Merge the pending commit
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Get the MLS group to access leaf nodes
        let mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");

        // Get Alice's leaf node (she's the creator/own leaf)
        let alice_leaf = mls_group.own_leaf().expect("Group should have own leaf");

        // Verify initial state: Alice is admin per MLS state
        assert!(
            creator_mdk
                .is_leaf_node_admin(&group_id.clone(), alice_leaf)
                .unwrap(),
            "Alice should be admin in MLS state"
        );

        // Now simulate stale storage by directly modifying stored_group.admin_pubkeys
        // to remove Alice as admin (even though MLS state has her as admin)
        let mut stored_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        // Add Bob to the stored admin list (simulating stale/incorrect storage)
        stored_group.admin_pubkeys.insert(bob_pk);
        // Remove Alice from stored admin list (simulating stale storage)
        stored_group.admin_pubkeys.remove(&alice_pk);

        // Save the modified (now stale) storage
        creator_mdk
            .storage()
            .save_group(stored_group.clone())
            .expect("Failed to save modified group");

        // Verify storage is now "stale" (has incorrect admin set)
        let stale_stored_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");
        assert!(
            stale_stored_group.admin_pubkeys.contains(&bob_pk),
            "Stale storage should have Bob as admin"
        );
        assert!(
            !stale_stored_group.admin_pubkeys.contains(&alice_pk),
            "Stale storage should NOT have Alice as admin"
        );

        // The critical test: is_leaf_node_admin should read from MLS state, NOT stale storage
        // Alice should still be admin (per MLS state) even though stale storage says otherwise
        assert!(
            creator_mdk
                .is_leaf_node_admin(&group_id.clone(), alice_leaf)
                .unwrap(),
            "is_leaf_node_admin should use MLS state, not stale storage"
        );
    }

    #[test]
    fn test_pubkey_for_member() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");

        // Test pubkey_for_member by checking all members
        let members: Vec<_> = mls_group.members().collect();
        let mut found_pubkeys = Vec::new();

        for member in &members {
            let pubkey = creator_mdk
                .pubkey_for_member(member)
                .expect("Failed to get pubkey for member");
            found_pubkeys.push(pubkey);
        }

        // Verify we found the expected public keys
        assert!(
            found_pubkeys.contains(&creator_pk),
            "Should find creator pubkey"
        );
        for member_keys in &initial_members {
            assert!(
                found_pubkeys.contains(&member_keys.public_key()),
                "Should find member pubkey: {:?}",
                member_keys.public_key()
            );
        }
        assert_eq!(found_pubkeys.len(), 3, "Should have 3 members total");
    }

    // TODO: Fix remaining test cases that need to be updated to match new API

    #[test]
    fn test_remove_members_group_not_found() {
        let mdk = create_test_mdk();
        let non_existent_group_id = crate::GroupId::from_slice(&[1, 2, 3, 4, 5]);
        let dummy_pubkey = Keys::generate().public_key();

        let result = mdk.remove_members(&non_existent_group_id, &[dummy_pubkey]);
        assert!(
            matches!(result, Err(crate::Error::GroupNotFound)),
            "Should return GroupNotFound error for non-existent group"
        );
    }

    #[test]
    fn test_remove_members_no_matching_members() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Try to remove a member that doesn't exist in the group
        let non_member = Keys::generate().public_key();
        let result = creator_mdk.remove_members(group_id, &[non_member]);

        assert!(
            matches!(
                result,
                Err(crate::Error::Group(ref msg)) if msg.contains("No matching members found")
            ),
            "Should return error when no matching members found"
        );
    }

    #[test]
    fn test_remove_members_epoch_advancement() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Get initial epoch
        let initial_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");
        let initial_epoch = initial_group.epoch;

        // Remove a member
        let member_to_remove = initial_members[0].public_key();
        let _remove_result = creator_mdk
            .remove_members(group_id, &[member_to_remove])
            .expect("Failed to remove member");

        // Merge the pending commit for the member removal
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for member removal");

        // Verify the MLS group epoch was advanced
        let mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let final_mls_epoch = mls_group.epoch().as_u64();

        assert!(
            final_mls_epoch > initial_epoch,
            "MLS group epoch should advance after removing members (initial: {}, final: {})",
            initial_epoch,
            final_mls_epoch
        );

        // Verify the member was removed
        let final_members = creator_mdk
            .get_members(group_id)
            .expect("Failed to get members");
        assert!(
            !final_members.contains(&member_to_remove),
            "Removed member should not be in the group"
        );
        assert_eq!(
            final_members.len(),
            2, // creator + 1 remaining member
            "Should have 2 total members after removal"
        );
    }

    #[test]
    fn test_remove_members_strips_admin_status() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();
        let member1_pk = initial_members[0].public_key();

        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // admins = [creator_pk, member1_pk]
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Verify member1 is an admin before removal
        let group_before = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");
        assert!(
            group_before.admin_pubkeys.contains(&member1_pk),
            "member1 should be admin before removal"
        );

        // Remove member1 (who is an admin)
        creator_mdk
            .remove_members(group_id, &[member1_pk])
            .expect("Failed to remove member");

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Verify member1 is no longer an admin
        let group_after = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        let expected_admins: BTreeSet<PublicKey> = once(creator_pk).collect();
        assert_eq!(
            group_after.admin_pubkeys, expected_admins,
            "Admin set should contain only the creator after removing admin member"
        );

        // Re-add member1 with a fresh key package and verify they do NOT
        // reappear as admin (regression test for issue #514).
        let re_add_kp = create_key_package_event(&creator_mdk, &initial_members[0]);
        creator_mdk
            .add_members(group_id, &[re_add_kp])
            .expect("Failed to re-add member");
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit after re-add");

        let group_readded = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");
        assert_eq!(
            group_readded.admin_pubkeys, expected_admins,
            "Re-added member should NOT regain admin status"
        );
        assert!(
            creator_mdk
                .get_members(group_id)
                .expect("Failed to get members")
                .contains(&member1_pk),
            "member1 should be back in the group"
        );
    }

    #[test]
    fn test_remove_non_admin_member_preserves_admin_list() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, _) = create_test_group_members();
        let creator_pk = creator.public_key();
        let member1_pk = initial_members[0].public_key();
        let member2_pk = initial_members[1].public_key();

        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Only creator is admin
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(vec![creator_pk]),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let admin_list_before = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist")
            .admin_pubkeys
            .clone();

        // Remove member2 (non-admin)
        creator_mdk
            .remove_members(group_id, &[member2_pk])
            .expect("Failed to remove member");

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let group_after = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        assert_eq!(
            group_after.admin_pubkeys, admin_list_before,
            "Admin list should be unchanged when removing a non-admin member"
        );

        // Verify the member was actually removed
        let members = creator_mdk
            .get_members(group_id)
            .expect("Failed to get members");
        assert!(
            !members.contains(&member2_pk),
            "member2 should have been removed from the group"
        );
        assert!(
            members.contains(&member1_pk),
            "member1 should still be in the group"
        );
    }

    #[test]
    fn test_remove_multiple_admins_strips_all() {
        let creator_mdk = create_test_mdk();
        let creator = Keys::generate();
        let creator_pk = creator.public_key();
        let member1 = Keys::generate();
        let member1_pk = member1.public_key();
        let member2 = Keys::generate();
        let member2_pk = member2.public_key();
        let member3 = Keys::generate();

        let members = vec![&member1, &member2, &member3];
        let mut key_package_events = Vec::new();
        for m in &members {
            key_package_events.push(create_key_package_event(&creator_mdk, m));
        }

        // Creator, member1, and member2 are admins; member3 is not
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                key_package_events,
                create_nostr_group_config_data(vec![creator_pk, member1_pk, member2_pk]),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Remove both admin members in a single call
        creator_mdk
            .remove_members(group_id, &[member1_pk, member2_pk])
            .expect("Failed to remove members");

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let group_after = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        let expected_admins: BTreeSet<PublicKey> = once(creator_pk).collect();
        assert_eq!(
            group_after.admin_pubkeys, expected_admins,
            "Only creator should remain as admin after bulk admin removal"
        );
    }

    #[test]
    fn test_remove_members_rejects_self_removal() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        let mut key_package_events = Vec::new();
        for m in &initial_members {
            key_package_events.push(create_key_package_event(&creator_mdk, m));
        }

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let result = creator_mdk.remove_members(group_id, &[creator_pk]);
        assert!(result.is_err(), "Self-removal should be rejected");
        assert!(
            result
                .unwrap_err()
                .to_string()
                .contains("Cannot remove yourself"),
            "Error should mention self-removal"
        );
    }

    #[test]
    fn test_leave_group_records_processed_state() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        let mut key_package_events = Vec::new();
        for m in &initial_members {
            key_package_events.push(create_key_package_event(&creator_mdk, m));
        }

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Admin must self-demote before leaving (per MIP-03)
        creator_mdk
            .self_demote(group_id)
            .expect("Failed to self-demote");
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge self-demote commit");

        let leave_result = creator_mdk
            .leave_group(group_id)
            .expect("Failed to leave group");

        // Verify the processed message was recorded with Processed state
        // (not ProcessedCommit), since leave_group creates a proposal.
        // Note: Check the leave event, not the self-demote event.
        let processed = creator_mdk
            .storage()
            .find_processed_message_by_event_id(&leave_result.evolution_event.id)
            .expect("Failed to query processed message")
            .expect("ProcessedMessage should exist");

        assert_eq!(
            processed.state,
            message_types::ProcessedMessageState::Processed,
            "leave_group should record Processed state, not ProcessedCommit"
        );
        assert_eq!(processed.wrapper_event_id, leave_result.evolution_event.id);
        assert!(processed.failure_reason.is_none());
    }

    #[test]
    fn test_self_update_success() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Verify initial group state
        let initial_members_set = creator_mdk
            .get_members(group_id)
            .expect("Failed to get initial members");
        assert_eq!(initial_members_set.len(), 3); // creator + 2 initial members

        // Get initial group state
        let initial_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let initial_epoch = initial_mls_group.epoch().as_u64();

        // Perform self update
        let update_result = creator_mdk
            .self_update(group_id)
            .expect("Failed to perform self update");

        // Merge the pending commit for the self update
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for self update");

        // Verify the result contains the expected data
        assert!(
            !update_result.evolution_event.content.is_empty(),
            "Evolution event should not be empty"
        );
        // Note: self_update typically doesn't produce a welcome message unless there are special circumstances
        // assert!(update_result.serialized_welcome_message.is_none(), "Welcome message should typically be None for self-update");

        // Verify the group state was updated correctly
        let final_members = creator_mdk
            .get_members(group_id)
            .expect("Failed to get final members");
        assert_eq!(
            final_members.len(),
            3,
            "Member count should remain the same after self update"
        );

        // Verify all original members are still in the group
        assert!(
            final_members.contains(&creator_pk),
            "Creator should still be in group"
        );
        for initial_member_keys in &initial_members {
            assert!(
                final_members.contains(&initial_member_keys.public_key()),
                "Initial member should still be in group"
            );
        }

        // Verify the epoch was advanced
        let final_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let final_epoch = final_mls_group.epoch().as_u64();

        assert!(
            final_epoch > initial_epoch,
            "Epoch should advance after self update (initial: {}, final: {})",
            initial_epoch,
            final_epoch
        );
    }

    #[test]
    fn test_self_update_group_not_found() {
        let mdk = create_test_mdk();
        let non_existent_group_id = crate::GroupId::from_slice(&[1, 2, 3, 4, 5]);

        let result = mdk.self_update(&non_existent_group_id);
        assert!(
            matches!(result, Err(crate::Error::GroupNotFound)),
            "Should return GroupNotFound error for non-existent group"
        );
    }

    #[test]
    fn test_self_update_key_rotation() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Get initial signature key from the leaf node
        let initial_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let initial_own_leaf = initial_mls_group
            .own_leaf()
            .expect("Failed to get initial own leaf");
        let initial_signature_key = initial_own_leaf.signature_key().as_slice().to_vec();

        // Perform self update (this should rotate the signing key)
        let _update_result = creator_mdk
            .self_update(group_id)
            .expect("Failed to perform self update");

        // Merge the pending commit for the self update
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for self update");

        // Get the new signature key
        let final_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let final_own_leaf = final_mls_group
            .own_leaf()
            .expect("Failed to get final own leaf");
        let final_signature_key = final_own_leaf.signature_key().as_slice().to_vec();

        // Verify the signature key has been rotated
        assert_ne!(
            initial_signature_key, final_signature_key,
            "Signature key should be different after self update"
        );

        // Verify the public key identity remains the same
        let initial_credential = BasicCredential::try_from(initial_own_leaf.credential().clone())
            .expect("Failed to extract initial credential");
        let final_credential = BasicCredential::try_from(final_own_leaf.credential().clone())
            .expect("Failed to extract final credential");

        assert_eq!(
            initial_credential.identity(),
            final_credential.identity(),
            "Public key identity should remain the same after self update"
        );
    }

    #[test]
    fn test_self_update_exporter_secret_rotation() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Get initial exporter secret
        let initial_secret = creator_mdk
            .exporter_secret(group_id)
            .expect("Failed to get initial exporter secret");

        // Perform self update
        let _update_result = creator_mdk
            .self_update(group_id)
            .expect("Failed to perform self update");

        // Merge the pending commit for the self update
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for self update");

        // Get the new exporter secret
        let final_secret = creator_mdk
            .exporter_secret(group_id)
            .expect("Failed to get final exporter secret");

        // Verify the exporter secret has been rotated
        assert_ne!(
            initial_secret.secret, final_secret.secret,
            "Exporter secret should be different after self update"
        );

        // Verify the epoch has advanced
        assert!(
            final_secret.epoch > initial_secret.epoch,
            "Epoch should advance after self update (initial: {}, final: {})",
            initial_secret.epoch,
            final_secret.epoch
        );

        // Verify the group ID remains the same
        assert_eq!(
            initial_secret.mls_group_id, final_secret.mls_group_id,
            "Group ID should remain the same"
        );
    }

    #[test]
    fn test_exporter_secret_rederives_current_epoch_instead_of_trusting_storage() {
        let mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&mdk, &creator, &members, &admins);

        let legacy_secret = mdk
            .legacy_exporter_secret(&group_id)
            .expect("Failed to derive legacy exporter secret");
        mdk.storage()
            .save_group_exporter_secret(legacy_secret.clone())
            .expect("Failed to persist legacy exporter secret");

        let refreshed_secret = mdk
            .exporter_secret(&group_id)
            .expect("Failed to derive refreshed exporter secret");

        assert_ne!(
            refreshed_secret.secret, legacy_secret.secret,
            "Current exporter secret should ignore stale stored bytes"
        );

        let stored_secret = mdk
            .storage()
            .get_group_exporter_secret(&group_id, refreshed_secret.epoch)
            .expect("Failed to load stored exporter secret")
            .expect("Stored exporter secret should exist");
        assert_eq!(
            stored_secret.secret, refreshed_secret.secret,
            "Storage should be healed to the freshly derived exporter secret"
        );
    }

    #[test]
    fn test_update_group_data() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Get initial group data for comparison
        let initial_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let initial_group_data = NostrGroupDataExtension::from_group(&initial_mls_group).unwrap();

        // Test 1: Update only the name
        let new_name = "Updated Name".to_string();
        let update = NostrGroupDataUpdate::new().name(new_name.clone());
        let update_result = creator_mdk
            .update_group_data(group_id, update)
            .expect("Failed to update group name");

        assert!(!update_result.evolution_event.content.is_empty());
        assert!(update_result.welcome_rumors.is_none());

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let updated_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let updated_group_data = NostrGroupDataExtension::from_group(&updated_mls_group).unwrap();

        assert_eq!(updated_group_data.name, new_name);
        assert_eq!(
            updated_group_data.description,
            initial_group_data.description
        );
        assert_eq!(updated_group_data.image_hash, initial_group_data.image_hash);

        // Test 2: Update multiple fields at once
        let new_description = "Updated Description".to_string();
        let new_image_hash =
            mdk_storage_traits::test_utils::crypto_utils::generate_random_bytes(32)
                .try_into()
                .unwrap();
        let new_image_key = mdk_storage_traits::test_utils::crypto_utils::generate_random_bytes(32)
            .try_into()
            .unwrap();
        let new_image_upload_key =
            mdk_storage_traits::test_utils::crypto_utils::generate_random_bytes(32)
                .try_into()
                .unwrap();

        let update = NostrGroupDataUpdate::new()
            .description(new_description.clone())
            .image_hash(Some(new_image_hash))
            .image_key(Some(new_image_key))
            .image_upload_key(Some(new_image_upload_key));

        let update_result = creator_mdk
            .update_group_data(group_id, update)
            .expect("Failed to update multiple fields");

        assert!(!update_result.evolution_event.content.is_empty());

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let final_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let final_group_data = NostrGroupDataExtension::from_group(&final_mls_group).unwrap();

        assert_eq!(final_group_data.name, new_name); // Should remain from previous update
        assert_eq!(final_group_data.description, new_description);
        assert_eq!(final_group_data.image_hash, Some(new_image_hash));
        assert_eq!(final_group_data.image_key, Some(new_image_key));
        assert_eq!(
            final_group_data.image_upload_key,
            Some(new_image_upload_key)
        );

        // Test 3: Clear optional fields
        let update = NostrGroupDataUpdate::new().image_hash(None);

        let update_result = creator_mdk
            .update_group_data(group_id, update)
            .expect("Failed to clear optional fields");

        assert!(!update_result.evolution_event.content.is_empty());

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let cleared_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let cleared_group_data = NostrGroupDataExtension::from_group(&cleared_mls_group).unwrap();

        assert_eq!(cleared_group_data.name, new_name);
        assert_eq!(cleared_group_data.description, new_description);
        assert_eq!(cleared_group_data.image_hash, None);
        assert_eq!(cleared_group_data.image_key, None);
        assert_eq!(cleared_group_data.image_nonce, None);
        assert_eq!(cleared_group_data.image_upload_key, None);

        // Test 4: Empty update (should succeed but not change anything)
        let empty_update = NostrGroupDataUpdate::new();
        let update_result = creator_mdk
            .update_group_data(group_id, empty_update)
            .expect("Failed to apply empty update");

        assert!(!update_result.evolution_event.content.is_empty());

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let unchanged_mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let unchanged_group_data =
            NostrGroupDataExtension::from_group(&unchanged_mls_group).unwrap();

        assert_eq!(unchanged_group_data.name, cleared_group_data.name);
        assert_eq!(
            unchanged_group_data.description,
            cleared_group_data.description
        );
        assert_eq!(
            unchanged_group_data.image_hash,
            cleared_group_data.image_hash
        );
        assert_eq!(unchanged_group_data.image_key, cleared_group_data.image_key);
    }

    #[test]
    fn test_sync_group_metadata_from_mls() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins.clone()),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Get initial stored group state
        let initial_stored_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get initial stored group")
            .expect("Stored group should exist");

        // Modify the MLS group directly (simulating state change without sync)
        let mut mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");

        // Create a new group data extension with different values
        let mut new_group_data = NostrGroupDataExtension::from_group(&mls_group).unwrap();
        new_group_data.name = "Synchronized Name".to_string();
        new_group_data.description = "Synchronized Description".to_string();

        // Apply the extension update to MLS group (but not to stored group)
        let extension =
            super::MDK::<MdkMemoryStorage>::get_unknown_extension_from_group_data(&new_group_data)
                .unwrap();
        let mut extensions = mls_group.extensions().clone();
        extensions.add_or_replace(extension).unwrap();

        let signature_keypair = creator_mdk.load_mls_signer(&mls_group).unwrap();
        let (_message_out, _, _) = mls_group
            .update_group_context_extensions(&creator_mdk.provider, extensions, &signature_keypair)
            .unwrap();

        // Merge the pending commit to advance epoch
        mls_group
            .merge_pending_commit(&creator_mdk.provider)
            .unwrap();

        // At this point, MLS group has changed but stored group is stale
        let stale_stored_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get stale stored group")
            .expect("Stored group should exist");

        // Verify stored group is stale
        assert_eq!(stale_stored_group.name, initial_stored_group.name);
        assert_eq!(
            stale_stored_group.description,
            initial_stored_group.description
        );
        assert_eq!(stale_stored_group.epoch, initial_stored_group.epoch);

        // Now test our sync function
        creator_mdk
            .sync_group_metadata_from_mls(group_id)
            .expect("Failed to sync group metadata");

        // Verify stored group is now synchronized
        let synced_stored_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get synced stored group")
            .expect("Stored group should exist");

        assert_eq!(synced_stored_group.name, "Synchronized Name");
        assert_eq!(synced_stored_group.description, "Synchronized Description");
        assert!(synced_stored_group.epoch > initial_stored_group.epoch);
        assert_eq!(
            synced_stored_group.admin_pubkeys,
            admins.into_iter().collect::<BTreeSet<_>>()
        );

        // Verify other fields remain unchanged
        assert_eq!(
            synced_stored_group.mls_group_id,
            initial_stored_group.mls_group_id
        );
        assert_eq!(
            synced_stored_group.last_message_id,
            initial_stored_group.last_message_id
        );
        assert_eq!(
            synced_stored_group.last_message_at,
            initial_stored_group.last_message_at
        );
        assert_eq!(synced_stored_group.state, initial_stored_group.state);
    }

    #[test]
    fn test_sync_group_metadata_rejects_adminless_group_data() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let group_id = create_test_group(&creator_mdk, &creator, &initial_members, &admins);

        let initial_stored_group = creator_mdk
            .get_group(&group_id)
            .expect("Failed to get initial stored group")
            .expect("Stored group should exist");

        let mut mls_group = creator_mdk
            .load_mls_group(&group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let mut group_data =
            NostrGroupDataExtension::from_group(&mls_group).expect("Group data should parse");
        group_data.admins.clear();

        let extension =
            super::MDK::<MdkMemoryStorage>::get_unknown_extension_from_group_data(&group_data)
                .expect("Failed to build adminless group-data extension");
        let mut extensions = mls_group.extensions().clone();
        extensions
            .add_or_replace(extension)
            .expect("Failed to replace group-data extension");

        let signature_keypair = creator_mdk
            .load_mls_signer(&mls_group)
            .expect("Failed to load signer");
        let (_message_out, _, _) = mls_group
            .update_group_context_extensions(&creator_mdk.provider, extensions, &signature_keypair)
            .expect("OpenMLS should allow the adminless extension update");
        mls_group
            .merge_pending_commit(&creator_mdk.provider)
            .expect("OpenMLS should merge the adminless extension update");

        // This guard does not roll back the mutated MLS group. It prevents the
        // invalid extension from being copied into stored group metadata.
        let result = creator_mdk.sync_group_metadata_from_mls(&group_id);

        assert!(
            matches!(result, Err(crate::Error::Group(ref msg)) if msg.contains("no admins")),
            "sync should reject adminless group data, got: {:?}",
            result
        );

        let stored_group = creator_mdk
            .get_group(&group_id)
            .expect("Failed to reload stored group")
            .expect("Stored group should still exist");
        assert_eq!(
            stored_group.admin_pubkeys, initial_stored_group.admin_pubkeys,
            "Rejected sync should not alter stored admins"
        );
    }

    #[test]
    fn test_extension_updates_create_processed_messages() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Test that each extension update creates a ProcessedMessage
        let test_cases = vec![
            ("update_group_name", "New Name"),
            ("update_group_description", "New Description"),
        ];

        for (operation, _value) in test_cases {
            let update_result = match operation {
                "update_group_name" => {
                    let update = NostrGroupDataUpdate::new().name("New Name".to_string());
                    creator_mdk.update_group_data(group_id, update)
                }
                "update_group_description" => {
                    let update =
                        NostrGroupDataUpdate::new().description("New Description".to_string());
                    creator_mdk.update_group_data(group_id, update)
                }
                _ => panic!("Unknown operation"),
            };

            let update_result = update_result.unwrap_or_else(|_| panic!("Failed to {}", operation));
            let commit_event_id = update_result.evolution_event.id;

            // Verify ProcessedMessage was created with correct state
            let processed_message = creator_mdk
                .storage()
                .find_processed_message_by_event_id(&commit_event_id)
                .expect("Failed to query processed message")
                .expect("ProcessedMessage should exist");

            assert_eq!(processed_message.wrapper_event_id, commit_event_id);
            assert_eq!(processed_message.message_event_id, None);
            assert_eq!(
                processed_message.state,
                message_types::ProcessedMessageState::ProcessedCommit
            );
            assert_eq!(processed_message.failure_reason, None);

            // Clean up by merging the commit
            creator_mdk
                .merge_pending_commit(group_id)
                .unwrap_or_else(|_| panic!("Failed to merge pending commit for {}", operation));
        }
    }

    #[test]
    fn test_stored_group_sync_after_all_operations() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Helper function to verify stored group epoch matches MLS group epoch
        let verify_epoch_sync = || {
            let mls_group = creator_mdk.load_mls_group(group_id).unwrap().unwrap();
            let stored_group = creator_mdk.get_group(group_id).unwrap().unwrap();
            assert_eq!(
                stored_group.epoch,
                mls_group.epoch().as_u64(),
                "Stored group epoch should match MLS group epoch"
            );
        };

        // Test 1: After group creation (should already be synced)
        verify_epoch_sync();

        // Test 2: After adding members
        let new_member = Keys::generate();
        let new_key_package_event = create_key_package_event(&creator_mdk, &new_member);
        let _add_result = creator_mdk
            .add_members(group_id, &[new_key_package_event])
            .expect("Failed to add member");

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for add member");
        verify_epoch_sync();

        // Test 3: After self update
        let _self_update_result = creator_mdk
            .self_update(group_id)
            .expect("Failed to perform self update");

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for self update");
        verify_epoch_sync();

        // Test 4: After extension updates
        let update = NostrGroupDataUpdate::new().name("Final Name".to_string());
        let _name_result = creator_mdk
            .update_group_data(group_id, update)
            .expect("Failed to update group name");

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit for name update");
        verify_epoch_sync();

        // Test 5: Verify stored group metadata matches extension data
        let final_mls_group = creator_mdk.load_mls_group(group_id).unwrap().unwrap();
        let final_stored_group = creator_mdk.get_group(group_id).unwrap().unwrap();
        let final_group_data = NostrGroupDataExtension::from_group(&final_mls_group).unwrap();

        assert_eq!(final_stored_group.name, final_group_data.name);
        assert_eq!(final_stored_group.description, final_group_data.description);
        assert_eq!(final_stored_group.admin_pubkeys, final_group_data.admins);
        assert_eq!(
            final_stored_group.nostr_group_id,
            final_group_data.nostr_group_id
        );
    }

    #[test]
    fn test_sync_group_metadata_error_cases() {
        let creator_mdk = create_test_mdk();

        // Test with non-existent group
        let non_existent_group_id = crate::GroupId::from_slice(&[1, 2, 3, 4, 5]);
        let result = creator_mdk.sync_group_metadata_from_mls(&non_existent_group_id);
        assert!(matches!(result, Err(crate::Error::GroupNotFound)));
    }

    #[test]
    fn test_sync_group_metadata_propagates_extension_parse_failure() {
        use openmls::prelude::{Extension, UnknownExtension};

        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins.clone()),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id;

        // Merge the pending commit
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Load the MLS group and corrupt the group-data extension
        let mut mls_group = creator_mdk
            .load_mls_group(group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");

        // Create a corrupted extension with invalid data
        let corrupted_extension_data = vec![0xFF, 0xFF, 0xFF]; // Invalid TLS-serialized data
        let corrupted_extension = Extension::Unknown(
            NOSTR_GROUP_DATA_EXTENSION_TYPE,
            UnknownExtension(corrupted_extension_data),
        );

        // Replace the group-data extension with the corrupted one
        let mut extensions = mls_group.extensions().clone();
        extensions.add_or_replace(corrupted_extension).unwrap();

        let signature_keypair = creator_mdk.load_mls_signer(&mls_group).unwrap();
        let (_message_out, _, _) = mls_group
            .update_group_context_extensions(&creator_mdk.provider, extensions, &signature_keypair)
            .unwrap();

        // Merge the pending commit to apply the corrupted extension
        mls_group
            .merge_pending_commit(&creator_mdk.provider)
            .unwrap();

        // Now test that sync_group_metadata_from_mls properly propagates the parse error
        let result = creator_mdk.sync_group_metadata_from_mls(group_id);

        // The function should return an error, not silently ignore the parse failure
        assert!(
            result.is_err(),
            "sync_group_metadata_from_mls should propagate extension parse errors"
        );

        // Verify it's a deserialization error (the specific error from deserialize_bytes)
        match result {
            Err(e) => {
                let error_msg = e.to_string();
                assert!(
                    error_msg.contains("TLS")
                        || error_msg.contains("deserialize")
                        || error_msg.contains("EndOfStream"),
                    "Expected deserialization error, got: {}",
                    error_msg
                );
            }
            Ok(_) => panic!("Expected error but got Ok"),
        }
    }

    /// Test getting group that doesn't exist
    #[test]
    fn test_get_nonexistent_group() {
        let mdk = create_test_mdk();
        let non_existent_id = crate::GroupId::from_slice(&[9, 9, 9, 9]);

        let result = mdk.get_group(&non_existent_id);

        assert!(result.is_ok(), "Should succeed");
        assert!(
            result.unwrap().is_none(),
            "Should return None for non-existent group"
        );
    }

    /// Member self-removal proposal
    ///
    /// Tests that leave_group creates a valid leave proposal.
    /// Note: A member cannot unilaterally leave - they create a proposal
    /// that must be committed by another member (typically an admin).
    ///
    /// Requirements tested:
    /// - leave_group creates valid MLS proposal events
    /// - leave_group works for group members
    /// - The proposal can be processed by other members
    #[test]
    fn test_member_self_removal() {
        use crate::test_util::create_key_package_event;

        // Create Alice (admin) and Bob (member)
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();

        let admins = vec![alice_keys.public_key()];

        // Bob creates his key package
        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        // Alice creates the group
        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_key_package],
                create_nostr_group_config_data(admins),
            )
            .expect("Alice should be able to create group");

        let group_id = create_result.group.mls_group_id.clone();

        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge Alice's create commit");

        // Bob processes and accepts welcome
        let bob_welcome_rumor = &create_result.welcome_rumors[0];
        let bob_welcome = bob_mdk
            .process_welcome(&nostr::EventId::all_zeros(), bob_welcome_rumor)
            .expect("Bob should be able to process welcome");
        bob_mdk
            .accept_welcome(&bob_welcome)
            .expect("Bob should be able to accept welcome");

        // Verify initial member count
        let initial_members = alice_mdk
            .get_members(&group_id)
            .expect("Failed to get members");
        assert_eq!(initial_members.len(), 2, "Group should have 2 members");

        // Bob calls leave_group
        let bob_leave_result = bob_mdk.leave_group(&group_id);
        assert!(
            bob_leave_result.is_ok(),
            "Bob should be able to call leave_group: {:?}",
            bob_leave_result.err()
        );

        // Verify leave generates proper MLS evolution event
        let bob_leave_event = bob_leave_result.unwrap().evolution_event;
        assert_eq!(
            bob_leave_event.kind,
            nostr::Kind::MlsGroupMessage,
            "Leave should generate MLS group message event"
        );

        // Verify the leave event has required tags
        assert!(
            bob_leave_event.tags.iter().any(|t| t.kind()
                == nostr::TagKind::SingleLetter(nostr::SingleLetterTag::from_char('h').unwrap())),
            "Leave event should have group ID tag"
        );

        // (1) Verify Bob is still in the group from Alice's perspective
        // The leave is only a proposal and hasn't been applied yet
        let members_after_leave_call = alice_mdk
            .get_members(&group_id)
            .expect("Failed to get members after leave call");
        assert_eq!(
            members_after_leave_call.len(),
            2,
            "Bob should still be in group - leave hasn't been processed yet"
        );
        assert!(
            members_after_leave_call.contains(&bob_keys.public_key()),
            "Bob should still be in member list until another member processes the leave"
        );

        // (2) Alice processes Bob's leave event
        // OpenMLS behavior: Alice receives the leave proposal
        let process_result = alice_mdk.process_message(&bob_leave_event);
        assert!(
            process_result.is_ok(),
            "Alice should be able to process Bob's leave event: {:?}",
            process_result.err()
        );

        // (3) Check if merge is needed
        let _merge_result = alice_mdk.merge_pending_commit(&group_id);

        // (4) Verify Bob's leave was processed successfully
        // The leave_group call by Bob creates a valid leave event that Alice can process
        // Whether Bob is immediately removed depends on OpenMLS implementation details
        let final_members = alice_mdk
            .get_members(&group_id)
            .expect("Failed to get members");

        // The test verifies that leave_group creates a valid event structure
        // that other members can process without errors
        assert!(
            final_members.len() <= 2,
            "Group should have at most 2 members after processing leave"
        );
    }

    /// Member removal and re-addition
    ///
    /// Tests that attempting to add an existing member with the same KeyPackage fails,
    /// but the member can be successfully re-added after removal using a new KeyPackage.
    ///
    /// Requirements tested:
    /// - Cannot add existing member with same KeyPackage (OpenMLS deterministic behavior)
    /// - Member can be removed from group
    /// - Member can be successfully re-added after removal with new KeyPackage
    #[test]
    fn test_cannot_add_existing_member() {
        use crate::test_util::create_key_package_event;

        // Create Alice (admin) and Bob (member)
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();

        let admins = vec![alice_keys.public_key()];

        // Bob creates his key package
        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        // Alice creates group with Bob as member
        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_key_package.clone()],
                create_nostr_group_config_data(admins),
            )
            .expect("Alice should be able to create group");

        let group_id = create_result.group.mls_group_id.clone();

        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge Alice's create commit");

        // Bob processes and accepts welcome
        let bob_welcome_rumor = &create_result.welcome_rumors[0];
        let bob_welcome = bob_mdk
            .process_welcome(&nostr::EventId::all_zeros(), bob_welcome_rumor)
            .expect("Bob should be able to process welcome");
        bob_mdk
            .accept_welcome(&bob_welcome)
            .expect("Bob should be able to accept welcome");

        // Verify initial member count
        let initial_members = alice_mdk
            .get_members(&group_id)
            .expect("Failed to get members");
        assert_eq!(initial_members.len(), 2, "Group should have 2 members");

        // Step 1: Alice attempts to add Bob again using the same KeyPackage
        // OpenMLS should reject this because Bob is already in the group
        let add_duplicate_result = alice_mdk.add_members(&group_id, &[bob_key_package]);
        assert!(
            add_duplicate_result.is_err(),
            "Should not be able to add existing member with same KeyPackage"
        );

        // Verify member count unchanged
        let members_after_duplicate = alice_mdk
            .get_members(&group_id)
            .expect("Failed to get members");
        assert_eq!(
            members_after_duplicate.len(),
            2,
            "Member count should not change after rejected duplicate add"
        );

        // Step 2: Alice removes Bob
        let remove_result = alice_mdk
            .remove_members(&group_id, &[bob_keys.public_key()])
            .expect("Should be able to remove Bob");

        alice_mdk
            .process_message(&remove_result.evolution_event)
            .expect("Failed to process remove");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge remove commit");

        // Verify Bob is removed
        let members_after_remove = alice_mdk
            .get_members(&group_id)
            .expect("Failed to get members");
        assert_eq!(
            members_after_remove.len(),
            1,
            "Group should have 1 member after removal"
        );
        assert!(
            !members_after_remove.contains(&bob_keys.public_key()),
            "Bob should not be in group"
        );

        // Step 3: Alice adds Bob back (should succeed)
        let bob_new_key_package = create_key_package_event(&bob_mdk, &bob_keys);
        let readd_result = alice_mdk.add_members(&group_id, &[bob_new_key_package]);

        assert!(
            readd_result.is_ok(),
            "Should be able to re-add Bob after removal: {:?}",
            readd_result.err()
        );

        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge re-add commit");

        // Verify Bob is back in the group
        let final_members = alice_mdk
            .get_members(&group_id)
            .expect("Failed to get members");
        assert_eq!(final_members.len(), 2, "Group should have 2 members again");
        assert!(
            final_members.contains(&bob_keys.public_key()),
            "Bob should be back in group"
        );
    }

    /// Test that non-admins cannot add members to a group
    #[test]
    fn test_non_admin_cannot_add_members() {
        let creator_mdk = create_test_mdk();
        let creator = Keys::generate();
        let non_admin_keys = Keys::generate();

        // Only creator is admin
        let admins = vec![creator.public_key()];

        // Non-admin creates their own MDK and key package
        let non_admin_mdk = create_test_mdk();
        let non_admin_key_package = create_key_package_event(&non_admin_mdk, &non_admin_keys);

        // Creator creates group with non-admin as member
        let create_result = creator_mdk
            .create_group(
                &creator.public_key(),
                vec![non_admin_key_package],
                create_nostr_group_config_data(admins.clone()),
            )
            .expect("Failed to create group");

        let group_id = create_result.group.mls_group_id.clone();

        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");

        // Non-admin joins the group
        let non_admin_welcome_rumor = &create_result.welcome_rumors[0];
        let non_admin_welcome = non_admin_mdk
            .process_welcome(&nostr::EventId::all_zeros(), non_admin_welcome_rumor)
            .expect("Non-admin should process welcome");
        non_admin_mdk
            .accept_welcome(&non_admin_welcome)
            .expect("Non-admin should accept welcome");

        // Verify non-admin is not an admin
        assert!(
            !admins.contains(&non_admin_keys.public_key()),
            "Non-admin should not be in admin list"
        );

        // Get initial member count
        let initial_member_count = creator_mdk
            .get_members(&group_id)
            .expect("Failed to get members")
            .len();

        // Try to have the non-admin add a new member
        let new_member_keys = Keys::generate();
        let new_member_key_package = create_key_package_event(&non_admin_mdk, &new_member_keys);

        let result = non_admin_mdk.add_members(&group_id, &[new_member_key_package]);

        assert!(
            matches!(result, Err(Error::NotAdmin)),
            "Should fail with typed admin permission error, got: {:?}",
            result
        );

        // Verify that the members list did not change
        let final_member_count = creator_mdk
            .get_members(&group_id)
            .expect("Failed to get members")
            .len();
        assert_eq!(
            initial_member_count, final_member_count,
            "Member count should not change when non-admin attempts to add members"
        );
    }

    /// Test that non-admins cannot remove members from a group
    #[test]
    fn test_non_admin_cannot_remove_members() {
        let creator_mdk = create_test_mdk();
        let creator = Keys::generate();
        let non_admin_keys = Keys::generate();
        let other_member_keys = Keys::generate();

        // Only creator is admin
        let admins = vec![creator.public_key()];

        // Create MDKs and key packages for members
        let non_admin_mdk = create_test_mdk();
        let other_member_mdk = create_test_mdk();
        let non_admin_key_package = create_key_package_event(&non_admin_mdk, &non_admin_keys);
        let other_member_key_package =
            create_key_package_event(&other_member_mdk, &other_member_keys);

        // Creator creates group with non-admin and other member
        let create_result = creator_mdk
            .create_group(
                &creator.public_key(),
                vec![non_admin_key_package, other_member_key_package],
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = create_result.group.mls_group_id.clone();

        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");

        // Non-admin joins the group
        let non_admin_welcome_rumor = &create_result.welcome_rumors[0];
        let non_admin_welcome = non_admin_mdk
            .process_welcome(&nostr::EventId::all_zeros(), non_admin_welcome_rumor)
            .expect("Non-admin should process welcome");
        non_admin_mdk
            .accept_welcome(&non_admin_welcome)
            .expect("Non-admin should accept welcome");

        // Get initial member count
        let initial_member_count = creator_mdk
            .get_members(&group_id)
            .expect("Failed to get members")
            .len();

        // Try to have the non-admin remove another member
        let result = non_admin_mdk.remove_members(&group_id, &[other_member_keys.public_key()]);

        assert!(
            matches!(result, Err(Error::NotAdmin)),
            "Should fail with typed admin permission error, got: {:?}",
            result
        );

        // Verify that the members list did not change
        let final_members_list = creator_mdk
            .get_members(&group_id)
            .expect("Failed to get members");
        let final_member_count = final_members_list.len();

        assert_eq!(
            initial_member_count, final_member_count,
            "Member count should not change when non-admin attempts to remove members"
        );

        // Verify the specific member is still present
        assert!(
            final_members_list
                .iter()
                .any(|m| m == &other_member_keys.public_key()),
            "Target member should still be in the group"
        );
    }

    /// Test that non-admins cannot update group extensions
    #[test]
    fn test_non_admin_cannot_update_group_extensions() {
        let creator_mdk = create_test_mdk();
        let creator = Keys::generate();
        let non_admin_keys = Keys::generate();

        // Only creator is admin
        let admins = vec![creator.public_key()];

        // Non-admin creates their own MDK and key package
        let non_admin_mdk = create_test_mdk();
        let non_admin_key_package = create_key_package_event(&non_admin_mdk, &non_admin_keys);

        // Creator creates group with non-admin as member
        let create_result = creator_mdk
            .create_group(
                &creator.public_key(),
                vec![non_admin_key_package],
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = create_result.group.mls_group_id.clone();

        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");

        // Non-admin joins the group
        let non_admin_welcome_rumor = &create_result.welcome_rumors[0];
        let non_admin_welcome = non_admin_mdk
            .process_welcome(&nostr::EventId::all_zeros(), non_admin_welcome_rumor)
            .expect("Non-admin should process welcome");
        non_admin_mdk
            .accept_welcome(&non_admin_welcome)
            .expect("Non-admin should accept welcome");

        // Get initial group metadata
        let initial_group = creator_mdk
            .get_group(&group_id)
            .expect("Failed to get group")
            .expect("Group should exist");
        let initial_name = initial_group.name.clone();
        let initial_description = initial_group.description.clone();

        // Try to have the non-admin update group name
        let update = NostrGroupDataUpdate::new().name("Hacked Name".to_string());
        let result = non_admin_mdk.update_group_data(&group_id, update);

        assert!(
            matches!(result, Err(Error::NotAdmin)),
            "Should fail with typed admin permission error, got: {:?}",
            result
        );

        // Verify that the group metadata did not change
        let final_group = creator_mdk
            .get_group(&group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        assert_eq!(
            initial_name, final_group.name,
            "Group name should not change when non-admin attempts to update"
        );
        assert_eq!(
            initial_description, final_group.description,
            "Group description should not change when non-admin attempts to update"
        );
    }

    /// Test that non-admins cannot self-demote.
    #[test]
    fn test_non_admin_cannot_self_demote() {
        let creator_mdk = create_test_mdk();
        let creator = Keys::generate();
        let non_admin_keys = Keys::generate();

        let non_admin_mdk = create_test_mdk();
        let non_admin_key_package = create_key_package_event(&non_admin_mdk, &non_admin_keys);

        let create_result = creator_mdk
            .create_group(
                &creator.public_key(),
                vec![non_admin_key_package],
                create_nostr_group_config_data(vec![creator.public_key()]),
            )
            .expect("Failed to create group");

        let group_id = create_result.group.mls_group_id.clone();
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");

        let non_admin_welcome = non_admin_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("Non-admin should process welcome");
        non_admin_mdk
            .accept_welcome(&non_admin_welcome)
            .expect("Non-admin should accept welcome");

        let result = non_admin_mdk.self_demote(&group_id);

        assert!(
            matches!(result, Err(Error::NotAdmin)),
            "Should fail with typed admin permission error, got: {:?}",
            result
        );
    }

    /// Test creator validation errors
    #[test]
    fn test_creator_validation_errors() {
        let mdk = create_test_mdk();
        let creator = Keys::generate();
        let member1 = Keys::generate();
        let member2 = Keys::generate();

        let creator_pk = creator.public_key();
        let member_pks = vec![member1.public_key(), member2.public_key()];

        // Test 1: Creator not in admin list
        let bad_admins = vec![member1.public_key()];
        let result = mdk.validate_group_members(&creator_pk, &member_pks, &bad_admins);
        assert!(
            matches!(result, Err(crate::Error::Group(ref msg)) if msg.contains("Creator must be an admin")),
            "Should error when creator is not an admin"
        );

        // Test 2: Creator in member list
        let bad_members = vec![creator_pk, member1.public_key()];
        let admins = vec![creator_pk];
        let result = mdk.validate_group_members(&creator_pk, &bad_members, &admins);
        assert!(
            matches!(result, Err(crate::Error::Group(ref msg)) if msg.contains("Creator must not be included as a member")),
            "Should error when creator is in member list"
        );

        // Test 3: Admin not in member list
        let non_member_admin = Keys::generate().public_key();
        let bad_admins = vec![creator_pk, non_member_admin];
        let result = mdk.validate_group_members(&creator_pk, &member_pks, &bad_admins);
        assert!(
            matches!(result, Err(crate::Error::Group(ref msg)) if msg.contains("Admin must be a member")),
            "Should error when admin is not a member"
        );
    }

    /// Test that admin update validation rejects empty admin sets
    #[test]
    fn test_admin_update_rejects_empty_admin_set() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Attempt to update with empty admin set - should fail
        let empty_admins: Vec<PublicKey> = vec![];
        let update = NostrGroupDataUpdate::new().admins(empty_admins);
        let result = creator_mdk.update_group_data(group_id, update);

        assert!(
            matches!(result, Err(crate::Error::UpdateGroupContextExts(ref msg)) if msg.contains("Admin set cannot be empty")),
            "Should error when admin set is empty, got: {:?}",
            result
        );
    }

    /// Test that admin update errors when all proposed admins are non-members (pruned to empty)
    #[test]
    fn test_admin_update_rejects_all_non_member_admins() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // All proposed admins are non-members - pruning leaves empty set
        let non_member1 = Keys::generate().public_key();
        let non_member2 = Keys::generate().public_key();
        let all_non_members = vec![non_member1, non_member2];
        let update = NostrGroupDataUpdate::new().admins(all_non_members);
        let result = creator_mdk.update_group_data(group_id, update);

        assert!(
            matches!(result, Err(crate::Error::UpdateGroupContextExts(ref msg)) if msg.contains("Admin set cannot be empty")),
            "Should error when all admins are pruned, got: {:?}",
            result
        );
    }

    /// Test that admin update prunes non-member admins
    #[test]
    fn test_admin_update_prunes_non_member_admins() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Attempt to update with a non-member admin - non-member should be pruned
        let non_member = Keys::generate().public_key();
        let admins_with_non_member = vec![creator_pk, non_member];
        let update = NostrGroupDataUpdate::new().admins(admins_with_non_member);
        let result = creator_mdk.update_group_data(group_id, update);

        assert!(
            result.is_ok(),
            "Should succeed after pruning non-member admin, got: {:?}",
            result
        );

        // Merge and verify only the valid admin remains
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let synced_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        assert!(
            synced_group.admin_pubkeys.contains(&creator_pk),
            "Creator should remain as admin"
        );
        assert!(
            !synced_group.admin_pubkeys.contains(&non_member),
            "Non-member should have been pruned from admin set"
        );
    }

    /// Test that admin update validation accepts valid admin sets
    #[test]
    fn test_admin_update_accepts_valid_member_admins() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Get current members
        let members = creator_mdk
            .get_members(group_id)
            .expect("Failed to get members");

        // Update admins to include all current members - should succeed
        let new_admins: Vec<PublicKey> = members.into_iter().collect();
        let update = NostrGroupDataUpdate::new().admins(new_admins.clone());
        let result = creator_mdk.update_group_data(group_id, update);

        assert!(
            result.is_ok(),
            "Should succeed when all admins are current members, got: {:?}",
            result
        );

        // Merge the pending commit
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Sync from MLS to get updated admin set
        creator_mdk
            .sync_group_metadata_from_mls(group_id)
            .expect("Failed to sync");

        let synced_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        let expected_admins: BTreeSet<PublicKey> = new_admins.into_iter().collect();
        assert_eq!(
            synced_group.admin_pubkeys, expected_admins,
            "Admin pubkeys should be updated to the new set"
        );
    }

    /// Test that admin update prunes previously removed members
    #[test]
    fn test_admin_update_prunes_previously_removed_member() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // Capture member public keys before they're used
        let member1_pk = initial_members[0].public_key();

        // Create key package events for initial members
        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        // Create the group
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id.clone();

        // Merge the pending commit to apply the member additions
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Remove member1 from the group
        creator_mdk
            .remove_members(group_id, &[member1_pk])
            .expect("Failed to remove member");

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Attempt to make the removed member an admin - removed member should be pruned
        let admins_with_removed = vec![creator_pk, member1_pk];
        let update = NostrGroupDataUpdate::new().admins(admins_with_removed);
        let result = creator_mdk.update_group_data(group_id, update);

        assert!(
            result.is_ok(),
            "Should succeed after pruning removed member, got: {:?}",
            result
        );

        // Merge and verify the removed member was pruned
        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let synced_group = creator_mdk
            .get_group(group_id)
            .expect("Failed to get group")
            .expect("Group should exist");

        assert!(
            synced_group.admin_pubkeys.contains(&creator_pk),
            "Creator should remain as admin"
        );
        assert!(
            !synced_group.admin_pubkeys.contains(&member1_pk),
            "Removed member should have been pruned from admin set"
        );
    }

    /// Test getting all groups when none exist
    #[test]
    fn test_get_groups_empty() {
        let mdk = create_test_mdk();

        let groups = mdk.get_groups().expect("Should succeed");

        assert_eq!(groups.len(), 0, "Should have no groups initially");
    }

    /// Test getting all groups returns created groups
    #[test]
    fn test_get_groups_with_data() {
        let creator_mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();

        // Create a group
        let group_id = create_test_group(&creator_mdk, &creator, &members, &admins);

        // Get all groups
        let groups = creator_mdk.get_groups().expect("Should succeed");

        assert_eq!(groups.len(), 1, "Should have 1 group");
        assert_eq!(groups[0].mls_group_id, group_id, "Group ID should match");
    }

    /// Test getting relays for a group
    #[test]
    fn test_get_relays() {
        let creator_mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();

        // Create a group (create_nostr_group_config_data includes test relays)
        let group_id = create_test_group(&creator_mdk, &creator, &members, &admins);

        // Get relays for the group
        let relays = creator_mdk
            .get_relays(&group_id)
            .expect("Should get relays");

        // Verify relays were stored (test config includes relays)
        assert!(!relays.is_empty(), "Group should have relays");
    }

    /// Test getting members for non-existent group
    #[test]
    fn test_get_members_nonexistent_group() {
        let mdk = create_test_mdk();
        let non_existent_id = crate::GroupId::from_slice(&[9, 9, 9, 9]);

        let result = mdk.get_members(&non_existent_id);

        // Should fail because group doesn't exist
        assert!(result.is_err(), "Should fail for non-existent group");
    }

    /// Test group name and description updates
    #[test]
    fn test_group_metadata_updates() {
        let creator_mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&creator_mdk, &creator, &members, &admins);

        // Update group name
        let update = NostrGroupDataUpdate::new().name("New Name".to_string());
        let result = creator_mdk.update_group_data(&group_id, update);
        assert!(result.is_ok(), "Should be able to update group name");
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");

        // Update group description
        let update = NostrGroupDataUpdate::new().description("New Description".to_string());
        let result = creator_mdk.update_group_data(&group_id, update);
        assert!(result.is_ok(), "Should be able to update group description");
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");

        // Update both at once
        let update = NostrGroupDataUpdate::new()
            .name("Final Name".to_string())
            .description("Final Description".to_string());
        let result = creator_mdk.update_group_data(&group_id, update);
        assert!(
            result.is_ok(),
            "Should be able to update both name and description"
        );
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");
    }

    /// Test group with empty name
    #[test]
    fn test_group_with_empty_name() {
        let creator_mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&creator_mdk, &creator, &members, &admins);

        // Update to empty name (should be valid)
        let update = NostrGroupDataUpdate::new().name("".to_string());
        let result = creator_mdk.update_group_data(&group_id, update);
        assert!(result.is_ok(), "Empty group name should be valid");
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");
    }

    /// Test group with long name within allowed limits
    ///
    /// Security fix (Issue #82): Group names are now limited to prevent memory exhaustion.
    /// This test verifies that names within the limit work correctly.
    #[test]
    fn test_group_with_long_name() {
        let creator_mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&creator_mdk, &creator, &members, &admins);

        // Update to a long name within the allowed limit (256 bytes)
        let long_name = "a".repeat(256);
        let update = NostrGroupDataUpdate::new().name(long_name);
        let result = creator_mdk.update_group_data(&group_id, update);
        assert!(result.is_ok(), "Group name at limit should be valid");
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");
    }

    /// Test that nostr_group_id can be rotated via update_group_data
    ///
    /// MIP-01 allows nostr_group_id rotation via proposals. This test verifies
    /// that the update API supports rotating the nostr_group_id for message routing.
    #[test]
    fn test_update_nostr_group_id() {
        let creator_mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&creator_mdk, &creator, &members, &admins);

        // Get the initial nostr_group_id
        let initial_mls_group = creator_mdk
            .load_mls_group(&group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let initial_group_data = NostrGroupDataExtension::from_group(&initial_mls_group).unwrap();
        let initial_nostr_group_id = initial_group_data.nostr_group_id;

        // Create a new nostr_group_id
        let new_nostr_group_id: [u8; 32] = [42u8; 32];

        // Update the nostr_group_id via the update API
        let update = NostrGroupDataUpdate::new().nostr_group_id(new_nostr_group_id);
        let result = creator_mdk.update_group_data(&group_id, update);
        assert!(result.is_ok(), "Should be able to update nostr_group_id");

        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("Failed to merge commit");

        // Verify the nostr_group_id was updated in the MLS extension
        let final_mls_group = creator_mdk
            .load_mls_group(&group_id)
            .expect("Failed to load MLS group")
            .expect("MLS group should exist");
        let final_group_data = NostrGroupDataExtension::from_group(&final_mls_group).unwrap();

        assert_ne!(
            final_group_data.nostr_group_id, initial_nostr_group_id,
            "nostr_group_id should have changed"
        );
        assert_eq!(
            final_group_data.nostr_group_id, new_nostr_group_id,
            "nostr_group_id should match the new value"
        );

        // Verify the stored group metadata was synced
        let stored_group = creator_mdk
            .get_group(&group_id)
            .expect("Failed to get group")
            .expect("Group should exist");
        assert_eq!(
            stored_group.nostr_group_id, new_nostr_group_id,
            "Stored group nostr_group_id should be synced"
        );
    }

    // ============================================================================
    // Proposal/Commit Edge Cases
    // ============================================================================

    /// Operation from Removed Member
    ///
    /// Validates that operations (adds/removes/updates) from a removed member
    /// are properly rejected to prevent security issues.
    #[test]
    fn test_operation_from_removed_member() {
        use crate::test_util::create_key_package_event;

        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();
        let dave_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();
        let dave_mdk = create_test_mdk();

        // Create key packages
        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        // Alice creates group with Bob, both are admins
        let admin_pubkeys = vec![alice_keys.public_key(), bob_keys.public_key()];
        let config = create_nostr_group_config_data(admin_pubkeys);

        let create_result = alice_mdk
            .create_group(&alice_keys.public_key(), vec![bob_key_package], config)
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();

        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // Bob joins
        let bob_welcome_rumor = &create_result.welcome_rumors[0];
        let bob_welcome = bob_mdk
            .process_welcome(&nostr::EventId::all_zeros(), bob_welcome_rumor)
            .expect("Bob should process welcome");
        bob_mdk
            .accept_welcome(&bob_welcome)
            .expect("Bob should accept welcome");

        // Step 1: Bob successfully adds Charlie (proves Bob has admin permissions)
        let charlie_key_package = create_key_package_event(&charlie_mdk, &charlie_keys);
        let bob_add_charlie = bob_mdk
            .add_members(&group_id, &[charlie_key_package])
            .expect("Bob should be able to add Charlie as admin");

        bob_mdk
            .merge_pending_commit(&group_id)
            .expect("Bob should merge commit");

        // Alice processes Bob's add commit
        alice_mdk
            .process_message(&bob_add_charlie.evolution_event)
            .expect("Alice should process Bob's commit");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // Verify Charlie is in the group
        let members_after_charlie = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert_eq!(
            members_after_charlie.len(),
            3,
            "Should have 3 members (Alice, Bob, Charlie)"
        );
        assert!(
            members_after_charlie.contains(&charlie_keys.public_key()),
            "Charlie should be in the group"
        );

        // Step 2: Alice removes Bob
        let _remove_bob = alice_mdk
            .remove_members(&group_id, &[bob_keys.public_key()])
            .expect("Alice should remove Bob");

        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge removal commit");

        // Verify Bob is removed
        let members_after_removal = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert_eq!(
            members_after_removal.len(),
            2,
            "Should have 2 members after Bob's removal"
        );
        assert!(
            !members_after_removal.contains(&bob_keys.public_key()),
            "Bob should not be in Alice's member list"
        );

        // Step 3: Bob attempts to add Dave (should fail - Bob is removed)
        // Bob hasn't processed his own removal yet, so he still has the group locally
        let dave_key_package = create_key_package_event(&dave_mdk, &dave_keys);
        let bob_add_dave = bob_mdk.add_members(&group_id, &[dave_key_package]);

        // Either Bob's operation fails locally, or if it succeeds,
        // Alice will reject it when processing
        if let Ok(bob_add_result) = bob_add_dave {
            // Bob was able to create a commit locally
            // Process it with Alice and merge if needed
            let alice_process_result = alice_mdk.process_message(&bob_add_result.evolution_event);

            // If processing succeeded, try to merge
            if alice_process_result.is_ok() {
                let _merge_result = alice_mdk.merge_pending_commit(&group_id);
            }
        }
        // If bob_add_dave failed locally, that's also acceptable - Bob's removal
        // was effective

        // Verify Dave was NOT added - this is the key assertion
        // Even if Bob could create a commit, it shouldn't result in Dave being added
        let final_members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert_eq!(
            final_members.len(),
            2,
            "Should still have 2 members (Alice and Charlie)"
        );
        assert!(
            !final_members.contains(&dave_keys.public_key()),
            "Dave should not be in the group"
        );
    }

    /// Rapid Sequential Member Operations
    ///
    /// Validates that rapid sequential member add/remove operations
    /// maintain state consistency and proper epoch advancement.
    #[test]
    fn test_rapid_sequential_member_operations() {
        use crate::test_util::create_key_package_event;

        let alice_keys = Keys::generate();
        let alice_mdk = create_test_mdk();

        let admin_pubkeys = vec![alice_keys.public_key()];
        let config = create_nostr_group_config_data(admin_pubkeys);

        // Create initial member
        let bob_keys = Keys::generate();
        let bob_mdk = create_test_mdk();
        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        let create_result = alice_mdk
            .create_group(&alice_keys.public_key(), vec![bob_key_package], config)
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // Bob processes welcome and joins
        let bob_welcome_rumor = &create_result.welcome_rumors[0];
        let bob_welcome = bob_mdk
            .process_welcome(&nostr::EventId::all_zeros(), bob_welcome_rumor)
            .expect("Bob should process welcome");
        bob_mdk
            .accept_welcome(&bob_welcome)
            .expect("Bob should accept welcome");

        let initial_epoch = alice_mdk
            .get_group(&group_id)
            .expect("Should get group")
            .expect("Group should exist")
            .epoch;

        // Rapidly add multiple members and have Bob process each commit
        let mut member_add_events = Vec::new();
        for i in 0..3 {
            let member_keys = Keys::generate();
            let member_mdk = create_test_mdk();
            let member_key_package = create_key_package_event(&member_mdk, &member_keys);

            let add_result = alice_mdk
                .add_members(&group_id, &[member_key_package])
                .unwrap_or_else(|_| panic!("Should add member {}", i));

            alice_mdk
                .merge_pending_commit(&group_id)
                .unwrap_or_else(|_| panic!("Should merge commit {}", i));

            member_add_events.push(add_result.evolution_event);
        }

        // Bob processes all the add commits
        for (i, event) in member_add_events.iter().enumerate() {
            bob_mdk
                .process_message(event)
                .unwrap_or_else(|_| panic!("Bob should process add commit {}", i));
            bob_mdk
                .merge_pending_commit(&group_id)
                .unwrap_or_else(|_| panic!("Bob should merge commit {}", i));
        }

        // Verify epoch advanced from Alice's perspective
        let after_adds_epoch = alice_mdk
            .get_group(&group_id)
            .expect("Should get group")
            .expect("Group should exist")
            .epoch;

        assert!(
            after_adds_epoch > initial_epoch,
            "Epoch should advance after additions"
        );

        // Verify member count from Alice's perspective
        let alice_members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");

        // Should have Alice + Bob + 3 new members = 5 total
        assert_eq!(
            alice_members.len(),
            5,
            "Alice should see 5 members after additions"
        );

        // Verify Bob's perspective matches Alice's
        let bob_group = bob_mdk
            .get_group(&group_id)
            .expect("Bob should have group")
            .expect("Group should exist for Bob");
        assert_eq!(
            bob_group.epoch, after_adds_epoch,
            "Bob's epoch should match Alice's"
        );

        let bob_members = bob_mdk
            .get_members(&group_id)
            .expect("Bob should get members");
        assert_eq!(bob_members.len(), 5, "Bob should see 5 members");

        // Verify both see the same members
        for member in &alice_members {
            assert!(
                bob_members.contains(member),
                "Bob should see member {:?}",
                member
            );
        }
    }

    /// Member Operation State Consistency
    ///
    /// Validates that member operations maintain consistent state across
    /// group metadata, member lists, and epoch tracking.
    #[test]
    fn test_member_operation_state_consistency() {
        use crate::test_util::create_key_package_event;

        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();

        let admin_pubkeys = vec![alice_keys.public_key()];
        let config = create_nostr_group_config_data(admin_pubkeys);

        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        let create_result = alice_mdk
            .create_group(&alice_keys.public_key(), vec![bob_key_package], config)
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // Check initial state
        let initial_group = alice_mdk
            .get_group(&group_id)
            .expect("Should get group")
            .expect("Group should exist");
        let initial_members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        let initial_epoch = initial_group.epoch;

        assert_eq!(initial_members.len(), 2, "Should have 2 initial members");

        // Add Charlie
        let charlie_key_package = create_key_package_event(&charlie_mdk, &charlie_keys);
        alice_mdk
            .add_members(&group_id, &[charlie_key_package])
            .expect("Should add Charlie");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Should merge commit");

        // Verify state after add
        let after_add_group = alice_mdk
            .get_group(&group_id)
            .expect("Should get group")
            .expect("Group should exist");
        let after_add_members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");

        assert_eq!(
            after_add_members.len(),
            3,
            "Should have 3 members after add"
        );
        assert!(
            after_add_group.epoch > initial_epoch,
            "Epoch should advance after add"
        );
        assert!(
            after_add_members.contains(&charlie_keys.public_key()),
            "Charlie should be in members list"
        );

        // Remove Charlie
        alice_mdk
            .remove_members(&group_id, &[charlie_keys.public_key()])
            .expect("Should remove Charlie");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Should merge commit");

        // Verify state after removal
        let after_remove_group = alice_mdk
            .get_group(&group_id)
            .expect("Should get group")
            .expect("Group should exist");
        let after_remove_members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");

        assert_eq!(
            after_remove_members.len(),
            2,
            "Should have 2 members after removal"
        );
        assert!(
            after_remove_group.epoch > after_add_group.epoch,
            "Epoch should advance after removal"
        );
        assert!(
            !after_remove_members.contains(&charlie_keys.public_key()),
            "Charlie should not be in members list"
        );

        // Verify Alice and Bob still present
        assert!(
            after_remove_members.contains(&alice_keys.public_key()),
            "Alice should still be in group"
        );
        assert!(
            after_remove_members.contains(&bob_keys.public_key()),
            "Bob should still be in group"
        );
    }

    /// Test that remove_members correctly handles ratchet tree holes
    ///
    /// This is a regression test for a bug where enumerate() was used to derive
    /// LeafNodeIndex instead of using member.index. When the ratchet tree has holes
    /// (from prior removals), the enumeration index diverges from the actual leaf index.
    ///
    /// Scenario: Alice creates group with Bob, Charlie, Dave. Remove Charlie (creates hole).
    /// Then remove Dave - must remove Dave (leaf 3), not the wrong member.
    #[test]
    fn test_remove_members_with_tree_holes() {
        use crate::test_util::create_key_package_event;

        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();
        let dave_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();
        let dave_mdk = create_test_mdk();

        let admin_pubkeys = vec![alice_keys.public_key()];
        let config = create_nostr_group_config_data(admin_pubkeys);

        // Create key packages for all members
        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_key_package = create_key_package_event(&charlie_mdk, &charlie_keys);
        let dave_key_package = create_key_package_event(&dave_mdk, &dave_keys);

        // Alice creates group with Bob, Charlie, Dave
        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_key_package, charlie_key_package, dave_key_package],
                config,
            )
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // Verify initial state: Alice, Bob, Charlie, Dave
        let initial_members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert_eq!(initial_members.len(), 4, "Should have 4 members initially");

        // Step 1: Remove Charlie (creates a hole in the ratchet tree)
        alice_mdk
            .remove_members(&group_id, &[charlie_keys.public_key()])
            .expect("Should remove Charlie");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Should merge commit");

        let after_charlie_removal = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert_eq!(
            after_charlie_removal.len(),
            3,
            "Should have 3 members after removing Charlie"
        );
        assert!(
            !after_charlie_removal.contains(&charlie_keys.public_key()),
            "Charlie should be removed"
        );

        // Step 2: Remove Dave (the bug would cause wrong member removal here)
        // With the bug: enumerate() would give Dave index 2, but his actual leaf index is 3
        alice_mdk
            .remove_members(&group_id, &[dave_keys.public_key()])
            .expect("Should remove Dave");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Should merge commit");

        // Verify final state: only Alice and Bob remain
        let final_members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert_eq!(
            final_members.len(),
            2,
            "Should have 2 members after removals"
        );
        assert!(
            final_members.contains(&alice_keys.public_key()),
            "Alice should still be in group"
        );
        assert!(
            final_members.contains(&bob_keys.public_key()),
            "Bob should still be in group"
        );
        assert!(
            !final_members.contains(&dave_keys.public_key()),
            "Dave should be removed"
        );
    }

    /// Empty Group Operations
    ///
    /// Validates proper handling of edge cases with minimal group configurations.
    #[test]
    fn test_empty_group_operations() {
        use crate::test_util::create_key_package_event;

        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();

        let admin_pubkeys = vec![alice_keys.public_key()];
        let config = create_nostr_group_config_data(admin_pubkeys);

        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        let create_result = alice_mdk
            .create_group(&alice_keys.public_key(), vec![bob_key_package], config)
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // Test: Remove with empty list (should return error)
        let empty_remove_result = alice_mdk.remove_members(&group_id, &[]);
        assert!(
            empty_remove_result.is_err(),
            "Removing empty member list should fail"
        );

        // Verify no state change after failed empty remove
        let members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert_eq!(members.len(), 2, "Member count should not change");

        // Test: Add with empty list (should return error)
        let empty_add_result = alice_mdk.add_members(&group_id, &[]);
        assert!(
            empty_add_result.is_err(),
            "Adding empty member list should fail"
        );

        // Verify no state change after failed empty add
        let members = alice_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert_eq!(members.len(), 2, "Member count should not change");
    }

    /// Tests that pending_added_members_pubkeys returns empty when there are no pending proposals.
    /// Note: pending_proposals() in MLS only contains proposals received via process_message,
    /// not commits created locally. This test verifies the method works for empty groups.
    #[test]
    fn test_pending_added_members_pubkeys_empty() {
        use crate::test_util::create_key_package_event;

        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();

        let admins = vec![alice_keys.public_key()];

        // Create key package for Bob
        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        // Alice creates the group with Bob
        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_key_package],
                create_nostr_group_config_data(admins),
            )
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // There should be no pending added members (proposals are from process_message)
        let pending = alice_mdk
            .pending_added_members_pubkeys(&group_id)
            .expect("Should get pending added members");
        assert!(
            pending.is_empty(),
            "No pending additions when no proposals have been received"
        );
    }

    /// Tests that SelfRemove proposals are auto-committed by non-admin receivers,
    /// so no pending removals accumulate.
    ///
    /// With SelfRemove (new protocol), any member auto-commits the departure.
    /// The proposal never enters a "pending" state.
    #[test]
    fn test_self_remove_auto_committed_no_pending_removals() {
        use crate::test_util::create_key_package_event;

        // Setup: Alice (admin), Bob (non-admin), Charlie (non-admin)
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();

        // Only Alice is admin
        let admins = vec![alice_keys.public_key()];

        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_key_package = create_key_package_event(&charlie_mdk, &charlie_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_key_package, charlie_key_package],
                create_nostr_group_config_data(admins),
            )
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // Bob and Charlie join the group
        let bob_welcome = &create_result.welcome_rumors[0];
        let charlie_welcome = &create_result.welcome_rumors[1];

        let bob_welcome_preview = bob_mdk
            .process_welcome(&nostr::EventId::all_zeros(), bob_welcome)
            .expect("Bob should process welcome");
        bob_mdk
            .accept_welcome(&bob_welcome_preview)
            .expect("Bob should accept welcome");

        let charlie_welcome_preview = charlie_mdk
            .process_welcome(&nostr::EventId::all_zeros(), charlie_welcome)
            .expect("Charlie should process welcome");
        charlie_mdk
            .accept_welcome(&charlie_welcome_preview)
            .expect("Charlie should accept welcome");

        // Bob leaves (sends SelfRemove proposal)
        let bob_leave_result = bob_mdk
            .leave_group(&group_id)
            .expect("Bob should be able to leave");

        // Charlie (non-admin) processes Bob's SelfRemove — auto-commits it
        let process_result = charlie_mdk
            .process_message(&bob_leave_result.evolution_event)
            .expect("Charlie should process Bob's SelfRemove");

        assert!(
            matches!(process_result, MessageProcessingResult::Proposal(_)),
            "SelfRemove should be auto-committed by non-admin, got: {:?}",
            process_result
        );

        // No pending removals — the proposal was committed immediately
        let pending = charlie_mdk
            .pending_removed_members_pubkeys(&group_id)
            .expect("Should get pending removed members");
        assert!(
            pending.is_empty(),
            "No pending removals after SelfRemove auto-commit"
        );

        // After merging, Bob should no longer be in the group from Charlie's POV
        charlie_mdk
            .merge_pending_commit(&group_id)
            .expect("Charlie should merge pending commit");

        let members = charlie_mdk
            .get_members(&group_id)
            .expect("Should get members");
        assert!(
            !members.contains(&bob_keys.public_key()),
            "Bob should no longer be in the group after SelfRemove"
        );
    }

    /// Tests that SelfRemove works end-to-end with multiple members and the group
    /// remains functional afterward.
    ///
    /// Models the realistic relay flow: Bob sends SelfRemove, Charlie auto-commits
    /// (first to the relay), Alice and Dave process Charlie's commit. All remaining
    /// members converge and can exchange messages.
    #[test]
    fn test_self_remove_group_remains_functional() {
        use crate::test_util::create_key_package_event;

        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();
        let dave_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();
        let dave_mdk = create_test_mdk();

        let admins = vec![alice_keys.public_key()];

        let bob_kp = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_kp = create_key_package_event(&charlie_mdk, &charlie_keys);
        let dave_kp = create_key_package_event(&dave_mdk, &dave_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_kp, charlie_kp, dave_kp],
                create_nostr_group_config_data(admins),
            )
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        for (mdk, keys, idx) in [
            (&bob_mdk, &bob_keys, 0),
            (&charlie_mdk, &charlie_keys, 1),
            (&dave_mdk, &dave_keys, 2),
        ] {
            let welcome = &create_result.welcome_rumors[idx];
            let preview = mdk
                .process_welcome(&nostr::EventId::all_zeros(), welcome)
                .unwrap_or_else(|_| panic!("{:?} should process welcome", keys.public_key()));
            mdk.accept_welcome(&preview)
                .unwrap_or_else(|_| panic!("{:?} should accept welcome", keys.public_key()));
        }

        // Bob sends SelfRemove
        let bob_leave = bob_mdk.leave_group(&group_id).expect("Bob should leave");

        // Charlie is first online — auto-commits Bob's SelfRemove
        let charlie_result = charlie_mdk
            .process_message(&bob_leave.evolution_event)
            .expect("Charlie should process SelfRemove");

        let charlie_commit = match charlie_result {
            MessageProcessingResult::Proposal(update) => update.evolution_event,
            other => panic!("Charlie should auto-commit, got: {:?}", other),
        };

        charlie_mdk
            .merge_pending_commit(&group_id)
            .expect("Charlie should merge commit");

        // Alice and Dave receive Charlie's commit from the relay.
        // They process Bob's SelfRemove proposal first (needed to resolve the
        // reference in Charlie's commit), then the commit itself.
        for (name, mdk) in [("Alice", &alice_mdk), ("Dave", &dave_mdk)] {
            // Process Bob's proposal (stores it; will auto-commit but Charlie's
            // commit supersedes our auto-commit when we process it next)
            let _ = mdk.process_message(&bob_leave.evolution_event);

            // Process Charlie's commit — this is the "winning" commit from the relay
            let commit_result = mdk.process_message(&charlie_commit);
            assert!(
                commit_result.is_ok(),
                "{name} should process Charlie's commit: {:?}",
                commit_result.err()
            );
        }

        // All members agree: Bob is gone
        for (name, mdk) in [
            ("Alice", &alice_mdk),
            ("Charlie", &charlie_mdk),
            ("Dave", &dave_mdk),
        ] {
            let members = mdk.get_members(&group_id).expect("Should get members");
            assert!(
                !members.contains(&bob_keys.public_key()),
                "Bob should be removed from {name}'s group"
            );
        }

        // Group is still functional: Charlie sends, Alice and Dave can read
        let rumor = crate::test_util::create_test_rumor(&charlie_keys, "post-departure message");
        let charlie_msg = charlie_mdk
            .create_message(&group_id, rumor, None)
            .expect("Charlie should send a message after SelfRemove");

        for (name, mdk) in [("Alice", &alice_mdk), ("Dave", &dave_mdk)] {
            let result = mdk.process_message(&charlie_msg);
            assert!(
                result.is_ok(),
                "{name} should read Charlie's post-departure message: {:?}",
                result.err()
            );
        }
    }

    /// Tests that pending_member_changes returns empty when there are no pending proposals.
    #[test]
    fn test_pending_member_changes_empty() {
        use crate::test_util::create_key_package_event;

        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();

        // Create group with Alice as admin and Bob as member
        let admins = vec![alice_keys.public_key()];
        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_key_package],
                create_nostr_group_config_data(admins),
            )
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // There should be no pending changes
        let changes = alice_mdk
            .pending_member_changes(&group_id)
            .expect("Should get pending member changes");
        assert!(changes.additions.is_empty(), "No pending additions");
        assert!(changes.removals.is_empty(), "No pending removals");
    }

    /// Tests that SelfRemove auto-commit leaves no pending member changes.
    ///
    /// With SelfRemove, the departure is committed immediately by any member,
    /// so pending_member_changes should show no pending removals.
    #[test]
    fn test_no_pending_member_changes_after_self_remove() {
        use crate::test_util::create_key_package_event;

        // Setup: Alice (admin), Bob (non-admin), Charlie (non-admin)
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();

        let admins = vec![alice_keys.public_key()];

        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_key_package = create_key_package_event(&charlie_mdk, &charlie_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_key_package, charlie_key_package],
                create_nostr_group_config_data(admins),
            )
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");

        // Bob and Charlie join
        let bob_welcome = &create_result.welcome_rumors[0];
        let charlie_welcome = &create_result.welcome_rumors[1];

        let bob_welcome_preview = bob_mdk
            .process_welcome(&nostr::EventId::all_zeros(), bob_welcome)
            .expect("Bob should process welcome");
        bob_mdk
            .accept_welcome(&bob_welcome_preview)
            .expect("Bob should accept welcome");

        let charlie_welcome_preview = charlie_mdk
            .process_welcome(&nostr::EventId::all_zeros(), charlie_welcome)
            .expect("Charlie should process welcome");
        charlie_mdk
            .accept_welcome(&charlie_welcome_preview)
            .expect("Charlie should accept welcome");

        // Bob leaves (SelfRemove)
        let bob_leave_result = bob_mdk.leave_group(&group_id).expect("Bob should leave");

        // Charlie (non-admin) auto-commits Bob's SelfRemove
        charlie_mdk
            .process_message(&bob_leave_result.evolution_event)
            .expect("Charlie should process Bob's SelfRemove");

        // No pending changes — SelfRemove was committed immediately
        let changes = charlie_mdk
            .pending_member_changes(&group_id)
            .expect("Should get pending member changes");
        assert!(changes.additions.is_empty(), "No pending additions");
        assert!(
            changes.removals.is_empty(),
            "No pending removals after SelfRemove auto-commit"
        );
    }

    /// Tests that pending member methods return error for non-existent group.
    #[test]
    fn test_pending_member_methods_group_not_found() {
        let alice_mdk = create_test_mdk();
        let fake_group_id = mdk_storage_traits::GroupId::from_slice(&[0u8; 16]);

        let result = alice_mdk.pending_added_members_pubkeys(&fake_group_id);
        assert!(result.is_err(), "Should error for non-existent group");

        let result = alice_mdk.pending_removed_members_pubkeys(&fake_group_id);
        assert!(result.is_err(), "Should error for non-existent group");

        let result = alice_mdk.pending_member_changes(&fake_group_id);
        assert!(result.is_err(), "Should error for non-existent group");
    }

    /// Tests that `clear_pending_commit` rolls back a pending add-member commit,
    /// allowing subsequent group operations to succeed.
    #[test]
    fn test_clear_pending_commit_after_failed_add() {
        let mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&mdk, &creator, &members, &admins);

        let initial_members = mdk.get_members(&group_id).expect("get members");
        let initial_count = initial_members.len();

        // Add a new member — creates a pending commit but do NOT merge
        // (simulates a failed relay publish)
        let new_member = Keys::generate();
        let kp_event = create_key_package_event(&mdk, &new_member);
        let _add_result = mdk
            .add_members(&group_id, &[kp_event])
            .expect("add_members should succeed");

        // A second add_members should fail because there is already a pending commit
        let another_member = Keys::generate();
        let kp_event2 = create_key_package_event(&mdk, &another_member);
        let err = mdk.add_members(&group_id, &[kp_event2]);
        assert!(err.is_err(), "Should fail due to existing pending commit");

        // Clear the pending commit (simulates cleanup after failed publish)
        mdk.clear_pending_commit(&group_id)
            .expect("clear_pending_commit should succeed");

        // Verify the member was NOT added
        let after_clear = mdk.get_members(&group_id).expect("get members");
        assert_eq!(
            after_clear.len(),
            initial_count,
            "Member count should be unchanged after clearing pending commit"
        );
        assert!(
            !after_clear.contains(&new_member.public_key()),
            "New member should not be in group after clearing pending commit"
        );

        // Verify the group is usable again — a new operation should succeed
        let kp_event3 = create_key_package_event(&mdk, &another_member);
        mdk.add_members(&group_id, &[kp_event3])
            .expect("add_members should succeed after clearing pending commit");
        mdk.merge_pending_commit(&group_id)
            .expect("merge should succeed after clearing pending commit");

        let final_members = mdk.get_members(&group_id).expect("get members");
        assert_eq!(
            final_members.len(),
            initial_count + 1,
            "Member should be added after clearing stale commit and retrying"
        );
        assert!(
            final_members.contains(&another_member.public_key()),
            "New member should be in group after successful retry"
        );
    }

    /// Tests that `clear_pending_commit` rolls back a pending remove-member commit.
    #[test]
    fn test_clear_pending_commit_after_failed_remove() {
        let mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&mdk, &creator, &members, &admins);

        let initial_members = mdk.get_members(&group_id).expect("get members");
        let initial_count = initial_members.len();
        let member_to_remove = members[0].public_key();

        // Remove a member — creates a pending commit but do NOT merge
        let _remove_result = mdk
            .remove_members(&group_id, &[member_to_remove])
            .expect("remove_members should succeed");

        // Clear the pending commit
        mdk.clear_pending_commit(&group_id)
            .expect("clear_pending_commit should succeed");

        // Verify the member was NOT removed
        let after_clear = mdk.get_members(&group_id).expect("get members");
        assert_eq!(
            after_clear.len(),
            initial_count,
            "Member count should be unchanged after clearing pending remove commit"
        );
        assert!(
            after_clear.contains(&member_to_remove),
            "Member should still be in group after clearing pending remove commit"
        );

        // Verify the group is usable again — retry the removal
        mdk.remove_members(&group_id, &[member_to_remove])
            .expect("remove_members should succeed after clearing pending commit");
        mdk.merge_pending_commit(&group_id)
            .expect("merge should succeed after clearing pending commit");

        let final_members = mdk.get_members(&group_id).expect("get members");
        assert_eq!(
            final_members.len(),
            initial_count - 1,
            "Member should be removed after clearing stale commit and retrying"
        );
        assert!(
            !final_members.contains(&member_to_remove),
            "Removed member should not be in group after successful retry"
        );
    }

    /// Tests that `clear_pending_commit` is a no-op when there is no pending commit.
    #[test]
    fn test_clear_pending_commit_no_pending() {
        let mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&mdk, &creator, &members, &admins);

        // Clearing when there's nothing pending should succeed (no-op)
        mdk.clear_pending_commit(&group_id)
            .expect("clear_pending_commit should succeed even with no pending commit");

        // Group should still be functional
        let member_count = mdk.get_members(&group_id).expect("get members").len();
        assert!(member_count > 0, "Group should still have members");
    }

    /// Tests that `clear_pending_commit` returns an error for a non-existent group.
    #[test]
    fn test_clear_pending_commit_group_not_found() {
        let mdk = create_test_mdk();
        let fake_group_id = mdk_storage_traits::GroupId::from_slice(&[0u8; 16]);

        let result = mdk.clear_pending_commit(&fake_group_id);
        assert!(
            result.is_err(),
            "clear_pending_commit should error for non-existent group"
        );
    }

    /// Tests that `self_update` followed by `merge_pending_commit` rotates the signing key and
    /// leaves no orphaned keypairs in storage — the new key becomes the active signer.
    #[test]
    fn test_self_update_then_merge_no_orphan() {
        let mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&mdk, &creator, &members, &admins);

        // Capture the pre-update signature key so we can verify rotation.
        let pre_update_mls_group = mdk
            .load_mls_group(&group_id)
            .expect("load mls group")
            .expect("group exists");
        let pre_update_pubkey = pre_update_mls_group
            .own_leaf()
            .expect("own leaf")
            .signature_key()
            .as_slice()
            .to_vec();

        // Perform self_update — stores the new keypair and creates a pending commit.
        mdk.self_update(&group_id).expect("self_update");

        // Capture the pending new public key before merging.
        let pending_mls_group = mdk
            .load_mls_group(&group_id)
            .expect("load mls group")
            .expect("group exists");
        let new_pubkey = pending_mls_group
            .pending_commit()
            .expect("pending commit exists")
            .update_path_leaf_node()
            .expect("update path leaf node in self_update commit")
            .signature_key()
            .as_slice()
            .to_vec();

        // The new key should differ from the pre-update key.
        assert_ne!(
            pre_update_pubkey, new_pubkey,
            "self_update should rotate the signature key"
        );

        // The new keypair must be in storage at this point (used by OpenMLS for the commit).
        let stored_before_merge = SignatureKeyPair::read(
            &mdk.provider.storage,
            &new_pubkey,
            mdk.ciphersuite.signature_algorithm(),
        );
        assert!(
            stored_before_merge.is_some(),
            "new keypair must be in storage before merge_pending_commit"
        );

        // Merge — advances the epoch and makes the new key the active signer.
        mdk.merge_pending_commit(&group_id)
            .expect("merge_pending_commit");

        // After a successful merge the new key must still be in storage (it is now the active signer).
        let stored_after_merge = SignatureKeyPair::read(
            &mdk.provider.storage,
            &new_pubkey,
            mdk.ciphersuite.signature_algorithm(),
        );
        assert!(
            stored_after_merge.is_some(),
            "new keypair must remain in storage after successful merge (it is the active signer)"
        );

        // The group should still be functional.
        let members_after = mdk.get_members(&group_id).expect("get members");
        assert!(!members_after.is_empty(), "group should still have members");
    }

    /// Tests that `self_update` followed by `clear_pending_commit` rolls back the commit and
    /// removes the orphaned new signature keypair from storage.
    #[test]
    fn test_self_update_then_clear_removes_orphaned_keypair() {
        let mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&mdk, &creator, &members, &admins);

        // Perform self_update — stores the new keypair and creates a pending commit.
        mdk.self_update(&group_id).expect("self_update");

        // Capture the new public key that was stored during self_update.
        let pending_mls_group = mdk
            .load_mls_group(&group_id)
            .expect("load mls group")
            .expect("group exists");
        let new_pubkey = pending_mls_group
            .pending_commit()
            .expect("pending commit exists")
            .update_path_leaf_node()
            .expect("update path leaf node in self_update commit")
            .signature_key()
            .as_slice()
            .to_vec();

        // The new keypair must be present in storage before we clear.
        let stored_before_clear = SignatureKeyPair::read(
            &mdk.provider.storage,
            &new_pubkey,
            mdk.ciphersuite.signature_algorithm(),
        );
        assert!(
            stored_before_clear.is_some(),
            "new keypair must be in storage before clear_pending_commit"
        );

        // Clear — rolls back the MLS state and must delete the orphaned keypair.
        mdk.clear_pending_commit(&group_id)
            .expect("clear_pending_commit");

        // The orphaned new keypair must now be gone from storage.
        let stored_after_clear = SignatureKeyPair::read(
            &mdk.provider.storage,
            &new_pubkey,
            mdk.ciphersuite.signature_algorithm(),
        );
        assert!(
            stored_after_clear.is_none(),
            "orphaned new keypair must be deleted from storage after clear_pending_commit"
        );

        // The group must be functional and able to perform a new self_update.
        mdk.self_update(&group_id)
            .expect("self_update should succeed after clearing pending commit");
        mdk.merge_pending_commit(&group_id)
            .expect("merge should succeed after retry");

        // After the successful retry the active signer is the newly rotated key.
        let final_mls_group = mdk
            .load_mls_group(&group_id)
            .expect("load mls group")
            .expect("group exists");
        let final_pubkey = final_mls_group
            .own_leaf()
            .expect("own leaf")
            .signature_key()
            .as_slice()
            .to_vec();

        let active_signer = SignatureKeyPair::read(
            &mdk.provider.storage,
            &final_pubkey,
            mdk.ciphersuite.signature_algorithm(),
        );
        assert!(
            active_signer.is_some(),
            "active signer keypair must be present in storage after successful retry"
        );
    }

    /// A `create_group` call that fails while parsing an invitee KeyPackage must
    /// not leave the creator's signer behind in provider storage. The signer is
    /// only useful paired with the group we failed to build; persisting it on
    /// the error path is leaked state.
    #[test]
    fn test_create_group_with_invalid_invitee_kp_does_not_persist_signer() {
        let creator_mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        // One good invitee and one malformed event: the malformed KP declares
        // `members[1]`'s identity in its credential but is signed by an
        // unrelated key, which `parse_key_package` rejects with
        // `KeyPackageIdentityMismatch`.
        let good_kp = create_key_package_event(&creator_mdk, &members[0]);
        let impostor = Keys::generate();
        let malformed_kp =
            create_key_package_event_with_key(&creator_mdk, &members[1].public_key(), &impostor);

        let signers_before = creator_mdk.provider.storage.signature_key_count();

        let result = creator_mdk.create_group(
            &creator_pk,
            vec![good_kp, malformed_kp],
            create_nostr_group_config_data(admins),
        );
        assert!(
            result.is_err(),
            "create_group must reject a malformed invitee KeyPackage"
        );

        let signers_after = creator_mdk.provider.storage.signature_key_count();
        assert_eq!(
            signers_before, signers_after,
            "failed create_group must not persist a signer (orphan signer leaked)"
        );
    }

    #[test]
    fn test_get_ratchet_tree_info() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id;

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        let debug_info = creator_mdk
            .get_ratchet_tree_info(group_id)
            .expect("Failed to get ratchet tree info");

        // tree_hash should be a 64-character hex string (SHA-256 = 32 bytes)
        assert_eq!(debug_info.tree_hash.len(), 64);
        assert!(
            debug_info.tree_hash.chars().all(|c| c.is_ascii_hexdigit()),
            "tree_hash should be valid hex"
        );

        // serialized_tree should be non-empty hex
        assert!(
            !debug_info.serialized_tree.is_empty(),
            "serialized tree should not be empty"
        );
        assert!(
            debug_info
                .serialized_tree
                .chars()
                .all(|c| c.is_ascii_hexdigit()),
            "serialized tree should be valid hex"
        );

        // Should have 3 leaf nodes: creator + 2 members
        assert_eq!(
            debug_info.leaf_nodes.len(),
            3,
            "should have 3 leaf nodes (creator + 2 members)"
        );

        // Each leaf node should have valid data
        for leaf in &debug_info.leaf_nodes {
            assert!(
                !leaf.encryption_key.is_empty(),
                "encryption key should not be empty"
            );
            assert!(
                !leaf.signature_key.is_empty(),
                "signature key should not be empty"
            );
            // Credential identity should be a 32-byte hex pubkey (64 hex chars)
            assert_eq!(
                leaf.credential_identity.len(),
                64,
                "credential identity should be a 32-byte hex pubkey"
            );
        }

        // Verify the creator's pubkey is among the leaf nodes
        let creator_hex = creator_pk.to_hex();
        assert!(
            debug_info
                .leaf_nodes
                .iter()
                .any(|l| l.credential_identity == creator_hex),
            "creator pubkey should be in leaf nodes"
        );
    }

    #[test]
    fn test_get_ratchet_tree_info_nonexistent_group() {
        let mdk = create_test_mdk();
        let fake_group_id = mdk_storage_traits::GroupId::from_slice(&[0u8; 32]);

        let result = mdk.get_ratchet_tree_info(&fake_group_id);
        assert!(result.is_err(), "should error for nonexistent group");
    }

    #[test]
    fn test_get_ratchet_tree_info_deterministic() {
        let creator_mdk = create_test_mdk();
        let (creator, initial_members, admins) = create_test_group_members();
        let creator_pk = creator.public_key();

        let mut initial_key_package_events = Vec::new();
        for member_keys in &initial_members {
            let key_package_event = create_key_package_event(&creator_mdk, member_keys);
            initial_key_package_events.push(key_package_event);
        }

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                initial_key_package_events,
                create_nostr_group_config_data(admins),
            )
            .expect("Failed to create group");

        let group_id = &create_result.group.mls_group_id;

        creator_mdk
            .merge_pending_commit(group_id)
            .expect("Failed to merge pending commit");

        // Call twice — should return identical results
        let info1 = creator_mdk
            .get_ratchet_tree_info(group_id)
            .expect("first call");
        let info2 = creator_mdk
            .get_ratchet_tree_info(group_id)
            .expect("second call");

        assert_eq!(info1, info2, "ratchet tree info should be deterministic");
    }

    #[test]
    fn test_own_leaf_index_and_group_leaf_map() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let bob_key_package = create_key_package_event(&bob_mdk, &bob_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_key_package],
                create_nostr_group_config_data(vec![alice_keys.public_key()]),
            )
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        let bob_welcome_rumor = &create_result.welcome_rumors[0];
        let bob_welcome = bob_mdk
            .process_welcome(&nostr::EventId::all_zeros(), bob_welcome_rumor)
            .expect("Bob should process welcome");

        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");
        bob_mdk
            .accept_welcome(&bob_welcome)
            .expect("Bob should accept welcome");

        assert_eq!(alice_mdk.own_leaf_index(&group_id).unwrap(), 0);
        assert_eq!(bob_mdk.own_leaf_index(&group_id).unwrap(), 1);

        let leaf_map = alice_mdk.group_leaf_map(&group_id).unwrap();
        assert_eq!(leaf_map.get(&0), Some(&alice_keys.public_key()));
        assert_eq!(leaf_map.get(&1), Some(&bob_keys.public_key()));
    }

    #[test]
    fn test_group_leaf_map_preserves_tree_holes() {
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();
        let dave_keys = Keys::generate();

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();
        let dave_mdk = create_test_mdk();

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![
                    create_key_package_event(&bob_mdk, &bob_keys),
                    create_key_package_event(&charlie_mdk, &charlie_keys),
                    create_key_package_event(&dave_mdk, &dave_keys),
                ],
                create_nostr_group_config_data(vec![alice_keys.public_key()]),
            )
            .expect("Alice should create group");

        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Alice should merge commit");
        alice_mdk
            .remove_members(&group_id, &[charlie_keys.public_key()])
            .expect("Should remove Charlie");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("Should merge Charlie removal");

        let leaf_map = alice_mdk.group_leaf_map(&group_id).unwrap();
        assert_eq!(leaf_map.get(&0), Some(&alice_keys.public_key()));
        assert_eq!(leaf_map.get(&1), Some(&bob_keys.public_key()));
        assert_eq!(leaf_map.get(&3), Some(&dave_keys.public_key()));
        assert!(!leaf_map.contains_key(&2));
    }

    // ============================================================================
    // LCD admission at create_group.
    //
    // These tests pin the observable group-context shape resulting from the
    // least-common-denominator intersection across invitee KeyPackage
    // capabilities. All three assertions are against
    // `MlsGroup::extensions().required_capabilities().proposal_types()` as
    // read back from persistent storage via `load_mls_group`.
    // ============================================================================

    /// All-modern invitees: the resulting group's `RequiredCapabilities`
    /// MUST contain `SelfRemove`. LCD should preserve the #7.2 guarantee
    /// whenever every invitee can honour it.
    #[test]
    fn test_create_group_all_modern_invitees_requires_self_remove() {
        let creator_mdk = create_test_mdk();
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let creator = Keys::generate();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let creator_pk = creator.public_key();

        let alice_kp = create_key_package_event(&alice_mdk, &alice);
        let bob_kp = create_key_package_event(&bob_mdk, &bob);

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                vec![alice_kp, bob_kp],
                create_nostr_group_config_data(vec![creator_pk]),
            )
            .expect("all-modern group creation should succeed");

        let group_id = create_result.group.mls_group_id.clone();
        let mls_group = creator_mdk
            .load_mls_group(&group_id)
            .expect("load ok")
            .expect("group exists");

        let proposal_types: Vec<ProposalType> = mls_group
            .extensions()
            .required_capabilities()
            .map(|rc| rc.proposal_types().to_vec())
            .unwrap_or_default();

        assert!(
            proposal_types.contains(&ProposalType::SelfRemove),
            "all-modern group should require SelfRemove (got {:?})",
            proposal_types
        );
    }

    /// Empty-invitee setup groups (single-member / "message to self") must
    /// NOT freeze in `RequiredCapabilities = [SelfRemove]`. A naive LCD
    /// over an empty invitee set is vacuously true, which would pin
    /// `SUPPORTED_PROPOSALS` and close the group to any later legacy
    /// `add_members()` call. This test pins the empty-stays-empty
    /// permissive default and guards against someone "simplifying" the
    /// `is_empty()` branch out of `create_group`.
    #[test]
    fn test_create_group_empty_invitees_yields_empty_required_capabilities() {
        let creator_mdk = create_test_mdk();
        let creator = Keys::generate();
        let creator_pk = creator.public_key();

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                Vec::new(),
                create_nostr_group_config_data(vec![creator_pk]),
            )
            .expect("single-member group creation should succeed");

        let group_id = create_result.group.mls_group_id.clone();
        let mls_group = creator_mdk
            .load_mls_group(&group_id)
            .expect("load ok")
            .expect("group exists");

        let proposal_types = mls_group
            .extensions()
            .required_capabilities()
            .map(|rc| rc.proposal_types().to_vec())
            .unwrap_or_default();

        assert!(
            proposal_types.is_empty(),
            "empty-invitee group must stay permissive (got {:?})",
            proposal_types
        );
    }

    // ============================================================================
    // `MDK::group_required_proposals` accessor.
    //
    // Read-side parity with the LCD-admission tests above: these pin the
    // public accessor's observable behavior so callers can branch UI on
    // per-group capability state without walking `openmls` internals.
    // ============================================================================

    /// All-modern invitees: accessor returns exactly `SUPPORTED_PROPOSALS`.
    /// Growth-safe — adding a proposal type to `SUPPORTED_PROPOSALS`
    /// automatically extends this test's truth.
    #[test]
    fn test_group_required_proposals_all_modern() {
        use crate::constant::SUPPORTED_PROPOSALS;

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_kp = create_key_package_event(&charlie_mdk, &charlie_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_kp, charlie_kp],
                create_nostr_group_config_data(vec![alice_keys.public_key()]),
            )
            .expect("alice creates all-modern group");
        let group_id = create_result.group.mls_group_id;

        let expected: BTreeSet<_> = SUPPORTED_PROPOSALS.iter().copied().collect();
        assert_eq!(
            alice_mdk
                .group_required_proposals(&group_id)
                .expect("accessor succeeds"),
            expected,
        );
    }

    /// Mixed invitees (one modern, one legacy): LCD strips all non-default
    /// proposals, so the accessor returns the empty set. The empty set is
    /// unambiguous here — it's the LCD outcome, not a "group missing"
    /// signal (that path returns `Err(Error::GroupNotFound)`).
    #[test]
    fn test_group_required_proposals_mixed_is_empty() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let legacy_keys = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob_keys);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice_keys.public_key()]),
            )
            .expect("alice creates mixed group");
        let group_id = create_result.group.mls_group_id;

        assert!(
            alice_mdk
                .group_required_proposals(&group_id)
                .expect("accessor succeeds")
                .is_empty(),
            "mixed-invitee group must yield empty LCD"
        );
    }

    /// Storage/MLS divergence contract: the accessor returns
    /// `Error::GroupNotFound` rather than collapsing "MLS record missing"
    /// into a silent empty set. This is the critical error-path contract
    /// that distinguishes the method from a field-on-`Group` design.
    #[test]
    fn test_group_required_proposals_missing_group() {
        let mdk = create_test_mdk();
        let fabricated = crate::GroupId::from_slice(&[1, 2, 3, 4, 5]);

        let err = mdk
            .group_required_proposals(&fabricated)
            .expect_err("missing group must error");
        assert!(
            matches!(err, Error::GroupNotFound),
            "expected Error::GroupNotFound, got {:?}",
            err
        );
    }

    /// `self_update` rotates the committing member's leaf but must not
    /// touch the group's `RequiredCapabilities` extension. Sanity-checks
    /// that the accessor reads the live `MlsGroup`, not a stale snapshot.
    #[test]
    fn test_group_required_proposals_survives_self_update() {
        use crate::constant::SUPPORTED_PROPOSALS;

        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_kp],
                create_nostr_group_config_data(vec![alice_keys.public_key()]),
            )
            .expect("alice creates all-modern group");
        let group_id = create_result.group.mls_group_id;
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation");

        alice_mdk
            .self_update(&group_id)
            .expect("alice rotates her leaf");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges the rotation");

        let expected: BTreeSet<_> = SUPPORTED_PROPOSALS.iter().copied().collect();
        assert_eq!(
            alice_mdk
                .group_required_proposals(&group_id)
                .expect("accessor succeeds post-rotation"),
            expected,
            "self_update must not change required capabilities"
        );
    }

    // ============================================================================
    // `MDK::group_member_capabilities` inspection API.
    //
    // Reports per-member advertised capabilities. Any member may call it —
    // read-only access to public leaf state.
    // ============================================================================

    /// Roster shape: one entry per live leaf, admin flag reflects group
    /// data, proposal advertisements reflect leaf capabilities.
    #[test]
    fn test_group_member_capabilities_reports_per_member() {
        use crate::groups::MemberCapabilities;

        let creator_mdk = create_test_mdk();
        let alice_mdk = create_test_mdk();
        let creator = Keys::generate();
        let alice = Keys::generate();
        let legacy = Keys::generate();

        let alice_kp = create_key_package_event(&alice_mdk, &alice);
        let legacy_kp = create_legacy_key_package_event(&creator_mdk, &legacy);

        let create_result = creator_mdk
            .create_group(
                &creator.public_key(),
                vec![alice_kp, legacy_kp],
                create_nostr_group_config_data(vec![creator.public_key()]),
            )
            .expect("create group");
        let group_id = create_result.group.mls_group_id;

        let roster: Vec<MemberCapabilities> = creator_mdk
            .group_member_capabilities(&group_id)
            .expect("roster");
        assert_eq!(roster.len(), 3, "three live leaves expected");

        let entry_for = |pk: &PublicKey| -> &MemberCapabilities {
            roster.iter().find(|m| m.member == *pk).unwrap_or_else(|| {
                panic!("no roster entry for {pk:?}");
            })
        };

        let creator_entry = entry_for(&creator.public_key());
        assert!(creator_entry.is_admin, "creator is admin");
        assert!(
            creator_entry.proposals.contains(&ProposalType::SelfRemove),
            "modern creator advertises SelfRemove"
        );

        let alice_entry = entry_for(&alice.public_key());
        assert!(!alice_entry.is_admin, "alice is non-admin");
        assert!(
            alice_entry.proposals.contains(&ProposalType::SelfRemove),
            "modern alice advertises SelfRemove"
        );

        let legacy_entry = entry_for(&legacy.public_key());
        assert!(!legacy_entry.is_admin, "legacy is non-admin");
        assert!(
            !legacy_entry.proposals.contains(&ProposalType::SelfRemove),
            "capability-poor legacy leaf does not advertise SelfRemove"
        );
    }

    /// The inspection method is callable by non-admins. Denying reads
    /// to non-admins would force UIs to guess at affordances.
    #[test]
    fn test_group_member_capabilities_is_callable_by_non_admins() {
        let creator_mdk = create_test_mdk();
        let alice_mdk = create_test_mdk();
        let creator = Keys::generate();
        let alice = Keys::generate();

        let alice_kp = create_key_package_event(&alice_mdk, &alice);
        let create_result = creator_mdk
            .create_group(
                &creator.public_key(),
                vec![alice_kp],
                create_nostr_group_config_data(vec![creator.public_key()]),
            )
            .expect("create group");
        let group_id = create_result.group.mls_group_id.clone();
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("creator merges");

        let alice_welcome = alice_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("alice processes welcome");
        alice_mdk
            .accept_welcome(&alice_welcome)
            .expect("alice accepts");

        // Alice is the non-admin caller.
        let roster = alice_mdk
            .group_member_capabilities(&group_id)
            .expect("non-admin may read");
        assert_eq!(roster.len(), 2);
    }

    // ============================================================================
    // `MDK::group_capability_upgrade_status` inspection API.
    //
    // Per-proposal upgrade readiness. Any member may call it.
    // ============================================================================

    /// All-modern group: `RequiredCapabilities = [SelfRemove]` via LCD.
    /// The status call must report every `SUPPORTED_PROPOSALS` entry as
    /// `AlreadyRequired` — there's nothing to offer the admin.
    #[test]
    fn test_upgrade_status_all_required_for_all_modern_group() {
        use crate::constant::SUPPORTED_PROPOSALS;

        let creator_mdk = create_test_mdk();
        let alice_mdk = create_test_mdk();
        let creator = Keys::generate();
        let alice = Keys::generate();

        let alice_kp = create_key_package_event(&alice_mdk, &alice);
        let create_result = creator_mdk
            .create_group(
                &creator.public_key(),
                vec![alice_kp],
                create_nostr_group_config_data(vec![creator.public_key()]),
            )
            .expect("all-modern group creation");
        let group_id = create_result.group.mls_group_id;

        let status = creator_mdk
            .group_capability_upgrade_status(&group_id)
            .expect("status call succeeds");
        assert_eq!(
            status.per_proposal.len(),
            SUPPORTED_PROPOSALS.len(),
            "one entry per SUPPORTED_PROPOSALS"
        );
        assert!(
            !status.per_proposal.is_empty(),
            "SUPPORTED_PROPOSALS unexpectedly empty"
        );
        let self_remove_entry = status
            .per_proposal
            .iter()
            .find(|(pt, _)| *pt == ProposalType::SelfRemove)
            .expect("SelfRemove entry present");
        assert!(
            matches!(self_remove_entry.1, ProposalUpgradability::AlreadyRequired),
            "SelfRemove should already be required, got {:?}",
            self_remove_entry.1
        );
        for (_, upgradability) in &status.per_proposal {
            assert!(
                matches!(upgradability, ProposalUpgradability::AlreadyRequired),
                "all-modern group should report AlreadyRequired, got {upgradability:?}"
            );
        }
    }

    /// Mixed group that becomes all-modern: status flips from `Blocked`
    /// to `Available` once the legacy member leaves and remaining members
    /// have refreshed their leaf capabilities.
    #[test]
    fn test_upgrade_status_available_after_legacy_member_leaves() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let legacy = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy);

        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice.public_key()]),
            )
            .expect("mixed group creation");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation");

        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob processes welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");

        // Alice removes the legacy member; merge + propagate to Bob.
        let remove_result = alice_mdk
            .remove_members(&group_id, &[legacy.public_key()])
            .expect("alice removes legacy");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges removal");
        bob_mdk
            .process_message(&remove_result.evolution_event)
            .expect("bob processes removal commit");

        let status = alice_mdk
            .group_capability_upgrade_status(&group_id)
            .expect("status call succeeds");

        let self_remove_entry = status
            .per_proposal
            .iter()
            .find(|(pt, _)| *pt == ProposalType::SelfRemove)
            .expect("SelfRemove entry present");
        assert!(
            matches!(self_remove_entry.1, ProposalUpgradability::Available),
            "formerly-mixed group should now be Available, got {:?}",
            self_remove_entry.1
        );
    }

    /// Mixed group with legacy member still present: status reports the
    /// specific legacy member as the blocker for `SelfRemove`.
    #[test]
    fn test_upgrade_status_blocked_names_blocker() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let legacy = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy);

        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice.public_key()]),
            )
            .expect("mixed group");
        let group_id = create_result.group.mls_group_id;

        let status = alice_mdk
            .group_capability_upgrade_status(&group_id)
            .expect("status call succeeds");

        let self_remove_entry = status
            .per_proposal
            .iter()
            .find(|(pt, _)| *pt == ProposalType::SelfRemove)
            .expect("SelfRemove entry present");
        match &self_remove_entry.1 {
            ProposalUpgradability::Blocked { blockers } => {
                assert_eq!(
                    blockers,
                    &vec![legacy.public_key()],
                    "legacy member is the sole blocker"
                );
            }
            other => panic!("expected Blocked, got {other:?}"),
        }
    }

    /// Status call is available to non-admin callers: read-only, no
    /// authorization required.
    #[test]
    fn test_upgrade_status_is_callable_by_non_admins() {
        let creator_mdk = create_test_mdk();
        let alice_mdk = create_test_mdk();
        let creator = Keys::generate();
        let alice = Keys::generate();

        let alice_kp = create_key_package_event(&alice_mdk, &alice);
        let create_result = creator_mdk
            .create_group(
                &creator.public_key(),
                vec![alice_kp],
                create_nostr_group_config_data(vec![creator.public_key()]),
            )
            .expect("create");
        let group_id = create_result.group.mls_group_id.clone();
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("creator merges");

        let alice_welcome = alice_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("alice welcome");
        alice_mdk
            .accept_welcome(&alice_welcome)
            .expect("alice accepts");

        alice_mdk
            .group_capability_upgrade_status(&group_id)
            .expect("non-admin may call status");
    }

    // ============================================================================
    // `MDK::upgrade_group_capabilities` action. Admin-only.
    // ============================================================================

    /// Happy path: mixed → legacy member removed → admin upgrades →
    /// `RequiredCapabilities` now contains `SelfRemove` on every peer,
    /// and a subsequent non-admin leave uses the modern SelfRemove path
    /// (receiver observes `MessageProcessingResult::Proposal`, not
    /// `PendingProposal`).
    #[test]
    fn test_upgrade_group_capabilities_happy_path() {
        // Alice (admin), Bob (non-admin leaver), plus one legacy invitee
        // who gets removed before the upgrade.
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let legacy = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy);

        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice.public_key()]),
            )
            .expect("mixed creation");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation");

        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");

        // Remove legacy member; propagate.
        let remove_result = alice_mdk
            .remove_members(&group_id, &[legacy.public_key()])
            .expect("alice removes legacy");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges removal");
        bob_mdk
            .process_message(&remove_result.evolution_event)
            .expect("bob processes removal");

        // Sanity: pre-upgrade admin count.
        let pre_admins = NostrGroupDataExtension::from_group(
            &alice_mdk.load_mls_group(&group_id).unwrap().unwrap(),
        )
        .unwrap()
        .admins
        .len();

        // Upgrade.
        let upgrade_result = alice_mdk
            .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove]))
            .expect("admin upgrade succeeds");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges upgrade");
        bob_mdk
            .process_message(&upgrade_result.evolution_event)
            .expect("bob processes upgrade");

        // Post-upgrade: admin count unchanged (sanity check against
        // depletion-validation interaction).
        let post_admins = NostrGroupDataExtension::from_group(
            &alice_mdk.load_mls_group(&group_id).unwrap().unwrap(),
        )
        .unwrap()
        .admins
        .len();
        assert_eq!(pre_admins, post_admins, "admin count unchanged");

        // Both peers reflect the upgraded RequiredCapabilities.
        assert!(
            alice_mdk
                .group_required_proposals(&group_id)
                .unwrap()
                .contains(&ProposalType::SelfRemove),
            "alice sees SelfRemove now required"
        );
        assert!(
            bob_mdk
                .group_required_proposals(&group_id)
                .unwrap()
                .contains(&ProposalType::SelfRemove),
            "bob sees SelfRemove now required"
        );

        // Modern SelfRemove path: Bob (non-admin) leaves; Alice (admin)
        // receives an auto-committable `Proposal`, not a
        // `PendingProposal`.
        let bob_leave = bob_mdk.leave_group(&group_id).expect("bob leaves modern");
        let observed = alice_mdk
            .process_message(&bob_leave.evolution_event)
            .expect("alice processes bob's leave");
        assert!(
            matches!(observed, MessageProcessingResult::Proposal { .. }),
            "post-upgrade leave must be modern SelfRemove (Proposal auto-commit), got {observed:?}"
        );
    }

    /// Convergence: `upgrade_group_capabilities` normalises a stale
    /// PURE_CIPHERTEXT local policy to `MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY`.
    /// Same hygiene as `self_update`: an admin upgrading a formerly-mixed
    /// group whose own persisted state still carries PURE (e.g. from an
    /// older MDK build) must end up able to receive `PublicMessage`
    /// SelfRemove proposals from departing members on the upgraded group.
    /// Reload-from-storage is load-bearing — in-memory state alone does
    /// not prove the persist-side convergence worked.
    #[test]
    fn test_upgrade_group_capabilities_converges_wire_format_to_mixed_ciphertext() {
        use openmls::prelude::{
            MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY, MlsGroupJoinConfig,
            PURE_CIPHERTEXT_WIRE_FORMAT_POLICY,
        };

        // Mixed group → legacy invitee removed → SelfRemove now Available.
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let legacy = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy);

        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice.public_key()]),
            )
            .expect("mixed creation");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation");

        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");

        let remove_result = alice_mdk
            .remove_members(&group_id, &[legacy.public_key()])
            .expect("alice removes legacy");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges removal");
        bob_mdk
            .process_message(&remove_result.evolution_event)
            .expect("bob processes removal");

        // Force alice's local policy to PURE_CIPHERTEXT, simulating
        // persisted state from an older MDK build that defaulted to PURE.
        let mut mls_group = alice_mdk
            .load_mls_group(&group_id)
            .expect("load for setup")
            .expect("group exists");
        let pure_config = MlsGroupJoinConfig::builder()
            .wire_format_policy(PURE_CIPHERTEXT_WIRE_FORMAT_POLICY)
            .build();
        mls_group
            .set_configuration(&alice_mdk.provider.storage, &pure_config)
            .expect("force PURE wire format");
        drop(mls_group);

        // Sanity: the stale precondition is actually persisted.
        let pre = alice_mdk
            .load_mls_group(&group_id)
            .expect("load pre")
            .expect("group pre");
        assert_eq!(
            pre.configuration().wire_format_policy(),
            PURE_CIPHERTEXT_WIRE_FORMAT_POLICY,
            "sanity: upgrader starts on stale PURE wire format"
        );
        drop(pre);

        alice_mdk
            .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove]))
            .expect("admin upgrade succeeds");

        let post = alice_mdk
            .load_mls_group(&group_id)
            .expect("load post")
            .expect("group post");
        assert_eq!(
            post.configuration().wire_format_policy(),
            MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY,
            "upgrade_group_capabilities should converge wire format to MIXED_CIPHERTEXT"
        );
    }

    /// Empty `proposals_to_add` is a caller bug, surfaced explicitly.
    #[test]
    fn test_upgrade_group_capabilities_rejects_empty_set() {
        let alice_mdk = create_test_mdk();
        let alice = Keys::generate();
        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![],
                create_nostr_group_config_data(vec![alice.public_key()]),
            )
            .expect("single-member group");
        let group_id = create_result.group.mls_group_id;

        assert!(matches!(
            alice_mdk.upgrade_group_capabilities(&group_id, &BTreeSet::new()),
            Err(Error::EmptyUpgradeSet)
        ));
    }

    /// Non-admin callers are rejected before any state mutation.
    #[test]
    fn test_upgrade_group_capabilities_rejects_non_admin() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let legacy = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy);
        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice.public_key()]),
            )
            .expect("mixed");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges");
        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("accept");

        // Remove legacy so the SelfRemove upgrade would otherwise be `Available`.
        let remove = alice_mdk
            .remove_members(&group_id, &[legacy.public_key()])
            .expect("remove");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges removal");
        bob_mdk
            .process_message(&remove.evolution_event)
            .expect("bob processes removal");

        // Bob (non-admin) tries to upgrade.
        assert!(matches!(
            bob_mdk
                .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove])),
            Err(Error::NotAdmin)
        ));

        // RequiredCapabilities unchanged.
        assert!(
            !bob_mdk
                .group_required_proposals(&group_id)
                .unwrap()
                .contains(&ProposalType::SelfRemove),
            "group still permissive"
        );
    }

    /// Requesting a `Blocked` proposal surfaces blocker identities on
    /// the error payload, not just a generic rejection.
    #[test]
    fn test_upgrade_group_capabilities_surfaces_blocker_identities() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let legacy = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy);
        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice.public_key()]),
            )
            .expect("mixed");
        let group_id = create_result.group.mls_group_id;

        match alice_mdk
            .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove]))
        {
            Err(Error::ProposalNotAvailableForUpgrade { proposal, blockers }) => {
                assert_eq!(proposal, ProposalType::SelfRemove);
                assert_eq!(blockers, vec![legacy.public_key()]);
            }
            other => panic!("expected ProposalNotAvailableForUpgrade, got {other:?}"),
        }
    }

    /// Requesting an `AlreadyRequired` proposal is a caller bug.
    #[test]
    fn test_upgrade_group_capabilities_rejects_already_required() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp],
                create_nostr_group_config_data(vec![alice.public_key()]),
            )
            .expect("all-modern");
        let group_id = create_result.group.mls_group_id;

        assert!(matches!(
            alice_mdk
                .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove])),
            Err(Error::ProposalAlreadyRequired(ProposalType::SelfRemove))
        ));
    }

    /// TOCTOU: another admin upgrades concurrently before this one can.
    /// The second admin's attempt sees `RequiredCapabilities` already
    /// contains the proposal and errors with `ProposalAlreadyRequired`,
    /// not a generic "no longer available."
    #[test]
    fn test_upgrade_group_capabilities_toctou_already_required() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let legacy = Keys::generate();

        // Two admins so both can trigger the upgrade.
        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy);
        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice.public_key(), bob.public_key()]),
            )
            .expect("mixed group with two admins");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation");

        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");

        // Remove legacy; propagate.
        let remove = alice_mdk
            .remove_members(&group_id, &[legacy.public_key()])
            .expect("remove legacy");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges removal");
        bob_mdk
            .process_message(&remove.evolution_event)
            .expect("bob processes removal");

        // Bob (admin B) upgrades first; alice processes.
        let b_result = bob_mdk
            .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove]))
            .expect("bob upgrades");
        bob_mdk
            .merge_pending_commit(&group_id)
            .expect("bob merges upgrade");
        alice_mdk
            .process_message(&b_result.evolution_event)
            .expect("alice processes bob's upgrade");

        // Alice now tries — should surface AlreadyRequired.
        assert!(matches!(
            alice_mdk
                .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove])),
            Err(Error::ProposalAlreadyRequired(ProposalType::SelfRemove))
        ));
    }

    /// Concurrent admins: both propose from the same pre-upgrade epoch.
    /// MLS commit ordering admits one; the other admin's local state
    /// recovers through the existing epoch-divergence machinery (the
    /// losing admin's upgrade call succeeded locally but merge fails
    /// when a conflicting commit lands, mirroring any other race like
    /// concurrent `add_members`).
    ///
    /// The invariant we pin: the committed transcript contains exactly
    /// one `GroupContextExtensions` commit adding `SelfRemove`, not two.
    /// After convergence, `RequiredCapabilities` includes `SelfRemove`
    /// exactly once.
    #[test]
    fn test_upgrade_group_capabilities_concurrent_admins_converge() {
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let alice = Keys::generate();
        let bob = Keys::generate();
        let legacy = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy);
        let create_result = alice_mdk
            .create_group(
                &alice.public_key(),
                vec![bob_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice.public_key(), bob.public_key()]),
            )
            .expect("mixed group with two admins");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation");

        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");

        let remove = alice_mdk
            .remove_members(&group_id, &[legacy.public_key()])
            .expect("remove");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges removal");
        bob_mdk
            .process_message(&remove.evolution_event)
            .expect("bob processes removal");

        // Both admins independently propose the upgrade from the same
        // epoch. Each local call succeeds (they don't see each other's
        // pending state).
        let alice_upgrade = alice_mdk
            .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove]))
            .expect("alice proposes upgrade");
        let _bob_upgrade = bob_mdk
            .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove]))
            .expect("bob proposes upgrade");

        // Alice's commit wins: she merges hers; bob processes alice's
        // event, which forces his pending commit to be dropped in
        // favor of alice's committed epoch.
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges her upgrade");
        bob_mdk
            .process_message(&alice_upgrade.evolution_event)
            .expect("bob adopts alice's upgrade commit");
        let bob_group = bob_mdk
            .load_mls_group(&group_id)
            .expect("bob group load")
            .expect("bob group exists");
        assert!(
            bob_group.pending_commit().is_none(),
            "bob's losing pending commit should be cleared after adopting alice's commit"
        );

        // Both now see SelfRemove required, exactly once.
        let alice_required = alice_mdk.group_required_proposals(&group_id).unwrap();
        let bob_required = bob_mdk.group_required_proposals(&group_id).unwrap();
        assert!(alice_required.contains(&ProposalType::SelfRemove));
        assert!(bob_required.contains(&ProposalType::SelfRemove));
        let alice_count = alice_required
            .iter()
            .filter(|p| **p == ProposalType::SelfRemove)
            .count();
        assert_eq!(
            alice_count, 1,
            "exactly one SelfRemove entry after convergence"
        );
    }

    /// Admin-leave demotion flow in a mixed group. Admins cannot leave
    /// directly; they must `self_demote()` first, have the demotion
    /// committed, then leave. In a mixed group, the subsequent leave must
    /// follow the legacy-Remove fallback (observable: a non-admin receiver
    /// sees `PendingProposal`, not `Proposal`).
    #[test]
    fn test_admin_in_mixed_group_demotes_then_leaves_legacy() {
        // Two admins so self-demote is allowed (not last-admin).
        // Charlie is a modern non-admin observer; a legacy invitee forces
        // LCD → RequiredCapabilities = [].
        let alice_mdk = create_test_mdk(); // admin leaving
        let bob_mdk = create_test_mdk(); // admin who stays
        let charlie_mdk = create_test_mdk(); // non-admin modern observer
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();
        let legacy_keys = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_kp = create_key_package_event(&charlie_mdk, &charlie_keys);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_kp, charlie_kp, legacy_kp],
                create_nostr_group_config_data(vec![
                    alice_keys.public_key(),
                    bob_keys.public_key(),
                ]),
            )
            .expect("alice creates mixed group with two admins");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation");

        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob processes welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");
        let charlie_welcome = charlie_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[1],
            )
            .expect("charlie processes welcome");
        charlie_mdk
            .accept_welcome(&charlie_welcome)
            .expect("charlie accepts");

        // Step 1: Alice (admin) cannot leave directly.
        let direct_err = alice_mdk
            .leave_group(&group_id)
            .expect_err("admin cannot leave without demoting");
        assert!(
            direct_err.to_string().contains("self-demote"),
            "expected demotion-required error substring; got: {direct_err}"
        );

        // Step 2: Alice self-demotes; the demotion commit must propagate
        // to Charlie (so his group state reflects Alice's non-admin status
        // when he later processes her leave).
        let demote_result = alice_mdk
            .self_demote(&group_id)
            .expect("alice self-demotes");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges her demotion");
        charlie_mdk
            .process_message(&demote_result.evolution_event)
            .expect("charlie processes demotion commit");

        // Step 3: Alice (now non-admin) leaves. Mixed group → legacy Remove
        // fallback → non-admin receiver sees PendingProposal.
        let alice_leave = alice_mdk
            .leave_group(&group_id)
            .expect("alice leaves after demotion");

        let result = charlie_mdk
            .process_message(&alice_leave.evolution_event)
            .expect("charlie processes alice's leave");
        assert!(
            matches!(result, MessageProcessingResult::PendingProposal { .. }),
            "demoted-admin leave in a mixed group should fall back to legacy Remove \
             (non-admin receiver sees PendingProposal); got {:?}",
            result
        );
    }

    /// `self_update()` must inject `MDK::capabilities()` into the new leaf,
    /// not clone the (possibly stale) capabilities already there. A member
    /// whose leaf currently advertises an empty proposals set must, after a
    /// self-update, advertise MDK's full set (which includes `SelfRemove`).
    #[test]
    fn test_self_update_refreshes_leaf_capabilities() {
        let alice_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();
        let alice_keys = Keys::generate();
        let charlie_keys = Keys::generate();

        // Alice invites Charlie using a capability-poor KeyPackage. Charlie's
        // leaf on entry therefore advertises NO proposals (no SelfRemove).
        // The group's RequiredCapabilities will also end up empty (LCD).
        let legacy_kp = create_legacy_key_package_event(&charlie_mdk, &charlie_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![legacy_kp],
                create_nostr_group_config_data(vec![alice_keys.public_key()]),
            )
            .expect("alice creates mixed group");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges");

        let charlie_welcome = charlie_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("charlie processes welcome");
        charlie_mdk
            .accept_welcome(&charlie_welcome)
            .expect("charlie accepts");

        // Precondition: Charlie's leaf advertises NO proposals.
        let pre = charlie_mdk
            .load_mls_group(&group_id)
            .expect("load pre")
            .expect("group pre");
        let pre_proposals: Vec<ProposalType> = pre
            .own_leaf()
            .expect("own leaf pre")
            .capabilities()
            .proposals()
            .to_vec();
        assert!(
            pre_proposals.is_empty(),
            "sanity: capability-poor leaf starts with NO proposals; got {:?}",
            pre_proposals
        );

        // Run a self-update and commit.
        charlie_mdk
            .self_update(&group_id)
            .expect("self-update succeeds");
        charlie_mdk
            .merge_pending_commit(&group_id)
            .expect("charlie merges self-update");

        // Postcondition: leaf now advertises MDK's modern proposals set,
        // which includes SelfRemove. This is the bug fix.
        let post = charlie_mdk
            .load_mls_group(&group_id)
            .expect("load post")
            .expect("group post");
        let post_proposals: Vec<ProposalType> = post
            .own_leaf()
            .expect("own leaf post")
            .capabilities()
            .proposals()
            .to_vec();
        assert!(
            post_proposals.contains(&ProposalType::SelfRemove),
            "self_update should refresh leaf proposals to current MDK capabilities; \
             expected SelfRemove, got {:?}",
            post_proposals
        );
    }

    /// Convergence: `self_update` normalises a stale PURE_CIPHERTEXT local
    /// policy to `MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY`. The PURE precondition
    /// simulates persisted state from pre-MIXED-welcomes MDK builds — today's
    /// welcome-processing creates MIXED, but local storage from older
    /// installations can still carry PURE. Bringing members forward on
    /// rotation reduces the wire-format reconciliation burden at a future
    /// `GroupContextExtensions`-upgrade event.
    #[test]
    fn test_self_update_converges_wire_format_to_mixed_ciphertext() {
        use openmls::prelude::{
            MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY, MlsGroupJoinConfig,
            PURE_CIPHERTEXT_WIRE_FORMAT_POLICY,
        };

        let mdk = create_test_mdk();
        let (creator, members, admins) = create_test_group_members();
        let group_id = create_test_group(&mdk, &creator, &members, &admins);

        // Force the creator's local policy to PURE_CIPHERTEXT, simulating
        // persisted state from an older MDK build that defaulted to PURE.
        let mut mls_group = mdk
            .load_mls_group(&group_id)
            .expect("load for setup")
            .expect("group exists");
        let pure_config = MlsGroupJoinConfig::builder()
            .wire_format_policy(PURE_CIPHERTEXT_WIRE_FORMAT_POLICY)
            .build();
        mls_group
            .set_configuration(&mdk.provider.storage, &pure_config)
            .expect("force PURE wire format");
        drop(mls_group);

        // Sanity: the stale precondition is actually persisted.
        let pre = mdk
            .load_mls_group(&group_id)
            .expect("load pre")
            .expect("group pre");
        assert_eq!(
            pre.configuration().wire_format_policy(),
            PURE_CIPHERTEXT_WIRE_FORMAT_POLICY,
            "sanity: member starts on stale PURE wire format"
        );
        drop(pre);

        mdk.self_update(&group_id).expect("self_update");

        let post = mdk
            .load_mls_group(&group_id)
            .expect("load post")
            .expect("group post");
        assert_eq!(
            post.configuration().wire_format_policy(),
            MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY,
            "self_update should converge wire format to MIXED_CIPHERTEXT"
        );
    }

    /// A formerly-mixed group can regain modern `SelfRemove` UX once
    /// every legacy member has left and an admin opts in by calling
    /// [`MDK::upgrade_group_capabilities`]. This test documents the
    /// end-to-end UX: pre-upgrade a modern leave falls back to legacy
    /// `Remove`; after the admin's upgrade commit lands on every peer,
    /// the same leave uses the modern auto-committable path.
    ///
    /// Replaces the old `test_mixed_group_stays_legacy_after_legacy_members_leave_over_conservative`
    /// which pinned the accepted over-conservatism before the
    /// `GroupContextExtensions` upgrade path was available.
    #[test]
    fn test_formerly_mixed_group_can_be_upgraded_after_legacy_members_leave() {
        // Alice (admin), Bob and Charlie (modern non-admins), plus a
        // legacy invitee who gets removed before the upgrade.
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();
        let legacy_keys = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_kp = create_key_package_event(&charlie_mdk, &charlie_keys);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_kp, charlie_kp, legacy_kp],
                create_nostr_group_config_data(vec![alice_keys.public_key()]),
            )
            .expect("mixed group creation");
        let group_id = create_result.group.mls_group_id.clone();
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges");

        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob processes welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");
        let charlie_welcome = charlie_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[1],
            )
            .expect("charlie processes welcome");
        charlie_mdk
            .accept_welcome(&charlie_welcome)
            .expect("charlie accepts");

        // Alice admin-removes the legacy member; propagate to peers so
        // every member's local state reflects the all-modern composition.
        let remove = alice_mdk
            .remove_members(&group_id, &[legacy_keys.public_key()])
            .expect("alice removes legacy");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges removal");
        bob_mdk
            .process_message(&remove.evolution_event)
            .expect("bob processes removal");
        charlie_mdk
            .process_message(&remove.evolution_event)
            .expect("charlie processes removal");

        // Alice performs the upgrade; propagate.
        let upgrade = alice_mdk
            .upgrade_group_capabilities(&group_id, &BTreeSet::from([ProposalType::SelfRemove]))
            .expect("alice upgrades");
        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges upgrade");
        bob_mdk
            .process_message(&upgrade.evolution_event)
            .expect("bob processes upgrade");
        charlie_mdk
            .process_message(&upgrade.evolution_event)
            .expect("charlie processes upgrade");

        // Every peer sees SelfRemove required.
        for mdk in [&alice_mdk, &bob_mdk, &charlie_mdk] {
            assert!(
                mdk.group_required_proposals(&group_id)
                    .unwrap()
                    .contains(&ProposalType::SelfRemove),
                "peer sees SelfRemove in RequiredCapabilities post-upgrade"
            );
        }

        // Bob (non-admin) leaves; Charlie processes. Post-upgrade this
        // uses the modern `SelfRemove` auto-commit path — receiver sees
        // `Proposal { .. }`, not `PendingProposal { .. }`.
        let bob_leave = bob_mdk.leave_group(&group_id).expect("bob leaves");
        let observed = charlie_mdk
            .process_message(&bob_leave.evolution_event)
            .expect("charlie processes bob's leave");
        assert!(
            matches!(observed, MessageProcessingResult::Proposal { .. }),
            "post-upgrade leave uses modern SelfRemove (Proposal auto-commit), got {observed:?}"
        );
    }

    /// Leave-path: after a modern SelfRemove send, the group's wire-format
    /// policy must be back to MIXED_CIPHERTEXT in persistent storage.
    /// Reloading from storage is load-bearing — in-memory state alone does
    /// not prove the persist-side restoration worked.
    #[test]
    fn test_self_remove_restores_ciphertext_config_on_disk() {
        use openmls::prelude::{IncomingWireFormatPolicy, OutgoingWireFormatPolicy};

        // All-modern 3-member group so RequiredCapabilities = [SelfRemove].
        let alice_mdk = create_test_mdk();
        let bob_mdk = create_test_mdk();
        let charlie_mdk = create_test_mdk();
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();

        let bob_kp = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_kp = create_key_package_event(&charlie_mdk, &charlie_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_kp, charlie_kp],
                create_nostr_group_config_data(vec![alice_keys.public_key()]),
            )
            .expect("alice creates group");
        let group_id = create_result.group.mls_group_id.clone();

        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation commit");

        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob processes welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");

        // Bob sends the SelfRemove. Internally `try_self_remove` flips to
        // plaintext, sends, then restores ciphertext.
        bob_mdk
            .leave_group(&group_id)
            .expect("bob sends SelfRemove");

        // Reload from storage and assert restoration.
        let reloaded = bob_mdk
            .load_mls_group(&group_id)
            .expect("reload ok")
            .expect("group still exists for bob");
        let policy = reloaded.configuration().wire_format_policy();
        assert_eq!(
            policy.outgoing(),
            OutgoingWireFormatPolicy::AlwaysCiphertext,
            "outgoing policy should be restored to AlwaysCiphertext"
        );
        assert_eq!(
            policy.incoming(),
            IncomingWireFormatPolicy::Mixed,
            "incoming policy should be restored to Mixed (MIXED_CIPHERTEXT)"
        );
    }

    /// Leave-path: a mixed group (RequiredCapabilities = []) must fall back
    /// to legacy `Remove` when a modern non-admin member leaves. Observable
    /// via the receiver — a non-admin receiver sees `PendingProposal`
    /// (waiting for an admin to commit), NOT `Proposal` (the SelfRemove
    /// auto-commit signature).
    #[test]
    fn test_leave_mixed_group_falls_back_to_legacy_remove() {
        let alice_mdk = create_test_mdk(); // admin
        let bob_mdk = create_test_mdk(); // non-admin, the leaver, modern
        let charlie_mdk = create_test_mdk(); // non-admin receiver, modern
        let alice_keys = Keys::generate();
        let bob_keys = Keys::generate();
        let charlie_keys = Keys::generate();
        let legacy_keys = Keys::generate();
        let admins = vec![alice_keys.public_key()];

        // Mixed group: bob + charlie are modern, one capability-poor invitee
        // poisons LCD into RequiredCapabilities = [].
        let bob_kp = create_key_package_event(&bob_mdk, &bob_keys);
        let charlie_kp = create_key_package_event(&charlie_mdk, &charlie_keys);
        let legacy_kp = create_legacy_key_package_event(&alice_mdk, &legacy_keys);

        let create_result = alice_mdk
            .create_group(
                &alice_keys.public_key(),
                vec![bob_kp, charlie_kp, legacy_kp],
                create_nostr_group_config_data(admins),
            )
            .expect("mixed group creation should succeed");
        let group_id = create_result.group.mls_group_id.clone();

        alice_mdk
            .merge_pending_commit(&group_id)
            .expect("alice merges creation commit");

        // Bob + Charlie join via welcomes (legacy invitee does not participate;
        // the capability-poor KP fixture bypasses MDK's welcome flow anyway).
        let bob_welcome = bob_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[0],
            )
            .expect("bob processes welcome");
        bob_mdk.accept_welcome(&bob_welcome).expect("bob accepts");

        let charlie_welcome = charlie_mdk
            .process_welcome(
                &nostr::EventId::all_zeros(),
                &create_result.welcome_rumors[1],
            )
            .expect("charlie processes welcome");
        charlie_mdk
            .accept_welcome(&charlie_welcome)
            .expect("charlie accepts");

        // Bob leaves the mixed group.
        let bob_leave = bob_mdk.leave_group(&group_id).expect("Bob should leave");

        // Charlie (non-admin) processing a legacy Remove: it goes into the
        // pending store (needs admin commit). For SelfRemove it would
        // auto-commit on any member (MessageProcessingResult::Proposal).
        let result = charlie_mdk
            .process_message(&bob_leave.evolution_event)
            .expect("charlie processes bob's leave");

        assert!(
            matches!(result, MessageProcessingResult::PendingProposal { .. }),
            "mixed-group leave must fall back to legacy Remove and surface as PendingProposal \
             for a non-admin receiver; got {:?}",
            result
        );
    }

    /// Admission: after an all-modern group is created, adding a member
    /// whose KeyPackage omits `SelfRemove` must be rejected and surface as
    /// the typed `Error::InviteeMissingRequiredProposal`. The underlying
    /// openmls chain is `AddMembersError::CreateCommitError(
    /// CreateCommitError::LeafNodeValidation(
    /// LeafNodeValidationError::UnsupportedProposals))`; the typed MDK
    /// variant lets callers distinguish legacy-joiner rejection from
    /// other add-member failures without substring-matching openmls's
    /// `Display` output.
    #[test]
    fn test_add_members_rejects_legacy_kp_in_all_modern_group() {
        let creator_mdk = create_test_mdk();
        let alice_mdk = create_test_mdk();
        let legacy_mdk = create_test_mdk();
        let creator = Keys::generate();
        let alice = Keys::generate();
        let legacy = Keys::generate();
        let creator_pk = creator.public_key();

        // All-modern bootstrap — RequiredCapabilities will contain SelfRemove.
        let alice_kp = create_key_package_event(&alice_mdk, &alice);
        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                vec![alice_kp],
                create_nostr_group_config_data(vec![creator_pk]),
            )
            .expect("all-modern group creation should succeed");
        let group_id = create_result.group.mls_group_id.clone();
        creator_mdk
            .merge_pending_commit(&group_id)
            .expect("merge creation commit");

        // Attempting to add a capability-poor invitee must fail.
        let legacy_kp = create_legacy_key_package_event(&legacy_mdk, &legacy);
        let err = creator_mdk
            .add_members(&group_id, &[legacy_kp])
            .expect_err("adding capability-poor KP to all-modern group must fail");

        assert!(
            matches!(err, Error::InviteeMissingRequiredProposal),
            "expected typed InviteeMissingRequiredProposal; got: {err:?}"
        );
    }

    /// All-legacy invitees: creation still succeeds and the resulting
    /// group's `RequiredCapabilities` is empty. Degenerate case proving
    /// that LCD does not blow up when no invitee advertises anything in
    /// `SUPPORTED_PROPOSALS`.
    #[test]
    fn test_create_group_all_legacy_invitees_yields_empty_required_capabilities() {
        let creator_mdk = create_test_mdk();
        let creator = Keys::generate();
        let legacy1 = Keys::generate();
        let legacy2 = Keys::generate();
        let creator_pk = creator.public_key();

        let kp1 = create_legacy_key_package_event(&creator_mdk, &legacy1);
        let kp2 = create_legacy_key_package_event(&creator_mdk, &legacy2);

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                vec![kp1, kp2],
                create_nostr_group_config_data(vec![creator_pk]),
            )
            .expect("all-legacy group creation should succeed");

        let group_id = create_result.group.mls_group_id.clone();
        let mls_group = creator_mdk
            .load_mls_group(&group_id)
            .expect("load ok")
            .expect("group exists");

        let proposal_types = mls_group
            .extensions()
            .required_capabilities()
            .map(|rc| rc.proposal_types().to_vec())
            .unwrap_or_default();

        assert!(
            proposal_types.is_empty(),
            "all-legacy group should have empty RequiredCapabilities.proposal_types (got {:?})",
            proposal_types
        );
    }

    /// Tracer for whitenoise-rs#749: creating a group whose invitee set
    /// contains a capability-poor (legacy) KeyPackage must succeed AND the
    /// resulting group's `RequiredCapabilities` must NOT advertise
    /// `SelfRemove`. This is the core #749 regression guard.
    #[test]
    fn test_create_group_mixed_invitees_drops_self_remove_requirement() {
        let creator_mdk = create_test_mdk();
        let modern_mdk = create_test_mdk();
        let creator = Keys::generate();
        let modern_member = Keys::generate();
        let legacy_member = Keys::generate();
        let creator_pk = creator.public_key();

        let modern_kp = create_key_package_event(&modern_mdk, &modern_member);
        let legacy_kp = create_legacy_key_package_event(&creator_mdk, &legacy_member);

        let create_result = creator_mdk
            .create_group(
                &creator_pk,
                vec![modern_kp, legacy_kp],
                create_nostr_group_config_data(vec![creator_pk]),
            )
            .expect("mixed-invitee group creation should succeed");

        let group_id = create_result.group.mls_group_id.clone();
        let mls_group = creator_mdk
            .load_mls_group(&group_id)
            .expect("load ok")
            .expect("group exists");

        let proposal_types: Vec<ProposalType> = mls_group
            .extensions()
            .required_capabilities()
            .map(|rc| rc.proposal_types().to_vec())
            .unwrap_or_default();

        assert!(
            !proposal_types.contains(&ProposalType::SelfRemove),
            "mixed group must not require SelfRemove (got {:?})",
            proposal_types
        );
    }
}