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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/xmtp_mls/src/groups/app_data/component_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,11 @@ pub(crate) fn component_type(id: ComponentId) -> Option<ComponentType> {
| ComponentId::MESSAGE_DISAPPEAR_IN_NS
| ComponentId::COMMIT_LOG_SIGNER => Some(ComponentType::Bytes),

// External-commit policy: proto-encoded ExternalCommitPolicyEntry,
// replaced atomically via the generic AppDataUpdate intent. No
// per-id Component impl needed; helpers decode bytes via prost.
ComponentId::EXTERNAL_COMMIT_POLICY => Some(ComponentType::Bytes),

// Immutable metadata (not flowable through AppDataUpdate writes,
// but we still advertise the type for completeness).
ComponentId::CONVERSATION_TYPE
Expand Down
8 changes: 3 additions & 5 deletions crates/xmtp_mls/src/groups/app_data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,11 @@ pub(crate) fn stage_app_data_propose_and_commit<Provider: OpenMlsProvider>(
// information than the on-wire commit, but the producer of each
// folded-in proposal already accepted that outcome by leaving it
// pending instead of issuing its own commit.
// Step 1: publish the standalone proposal. This adds the proposal
// to the local pending-proposal store AND returns the wire-form
// MlsMessageOut so the caller can broadcast it.
let openmls_id = component_id.as_u16();
let operation = AppDataUpdateOperation::Update(payload.into());

// Step 1: publish a standalone proposal. This adds the proposal to
// the local pending-proposal store AND returns the wire-form
// MlsMessageOut for the proposal so the caller can broadcast it.
let (proposal_msg, _proposal_ref) = mls_group
.propose_app_data_update(provider, signer, openmls_id, operation)
.map_err(GroupAppDataError::Propose)?;
Expand Down Expand Up @@ -326,7 +325,6 @@ pub(crate) fn stage_app_data_propose_and_commit<Provider: OpenMlsProvider>(
let app_data_updates =
accumulate_app_data_updates(mls_group, pending_iter).inspect_err(|e| {
tracing::error!(
component_id = %component_id,
error = %e,
"Failed to compute AppDataUpdates for standalone propose+commit"
);
Expand Down
7 changes: 7 additions & 0 deletions crates/xmtp_mls/src/groups/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,12 @@ pub enum GroupError {
/// AppDataUpdate path. Not retryable.
#[error("component source error: {0}")]
ComponentSource(#[from] super::app_data::component_source::ComponentSourceError),
/// An `EXTERNAL_COMMIT_POLICY` value violates the XIP-82
/// field-coupling invariants (enable requires key + slot id; revoke
/// leaves every per-invite field absent). Not retryable — the caller
/// supplied a malformed policy.
#[error("external commit policy error: {0}")]
ExternalCommitPolicy(#[from] super::external_commit_policy::ExternalCommitPolicyError),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low groups/error.rs:289

GroupError::ExternalCommitPolicy wraps ExternalCommitPolicyError without #[error_code(inherit)], and ExternalCommitPolicyError doesn't implement ErrorCode. When the Node/WASM/mobile bindings call error_code(), every variant of ExternalCommitPolicyError is flattened to the single code GroupError::ExternalCommitPolicy. SDK consumers cannot distinguish MissingSymmetricKey, InvalidExternalGroupIdLength, MissingMembershipGrant, etc. without parsing the free-form message string. Consider adding #[error_code(inherit)] to the variant (if the inner type gains ErrorCode) or implementing ErrorCode directly on ExternalCommitPolicyError.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @crates/xmtp_mls/src/groups/error.rs around line 289:

`GroupError::ExternalCommitPolicy` wraps `ExternalCommitPolicyError` without `#[error_code(inherit)]`, and `ExternalCommitPolicyError` doesn't implement `ErrorCode`. When the Node/WASM/mobile bindings call `error_code()`, every variant of `ExternalCommitPolicyError` is flattened to the single code `GroupError::ExternalCommitPolicy`. SDK consumers cannot distinguish `MissingSymmetricKey`, `InvalidExternalGroupIdLength`, `MissingMembershipGrant`, etc. without parsing the free-form message string. Consider adding `#[error_code(inherit)]` to the variant (if the inner type gains `ErrorCode`) or implementing `ErrorCode` directly on `ExternalCommitPolicyError`.

/// AppData commit error.
///
/// Failed to build or stage a commit that bundles an inline AppDataUpdate
Expand Down Expand Up @@ -587,6 +593,7 @@ impl RetryableError for GroupError {
Self::MinVersionDowngrade { .. } => false,
Self::InvalidMinVersion { .. } => false,
Self::ComponentSource(_) => false,
Self::ExternalCommitPolicy(_) => false,
Self::AppDataCommit(e) => e.is_retryable(),
// Bootstrap synthesis can fail on a transient identity-update
// API blip — delegate to the inner error so we retry on
Expand Down
Loading