DO NOT MERGE: re-merge async storage traits (#1923) onto fork main + sqlx Postgres CI by tylerhawkes · Pull Request #57 · xmtp/openmls

tylerhawkes · 2026-08-05T17:22:00Z

Draft, and titled DO NOT MERGE on purpose. This is opened to get CI running against the merge and to make the diff reviewable — not to land yet. See "Before this can merge" at the bottom.

What this is

tyler/maybe-async carried the async storage traits (upstream openmls#1923) plus a Postgres sqlx provider, but it was based on fork rev e1256743. Fork main has since advanced to 3fabbf23 through #55 (upstream merge to openmls main 65396d8) and #56 (the fork storage-format work), and 3fabbf23 was not an ancestor of that branch.

Repinning libxmtp to the old branch would have silently reverted persisted wire format — storage_tag numbering on ExtensionType/Extension, PastEpochDeletionPolicy as a portable u64, tolerant deserialization of fork-only trailing fields. It also would not have resolved at all: libxmtp pins extensions-draft and draft-ietf-mls-pq-ciphersuites, which the branch only had under the older upstream name extensions-draft-08.

So this re-merges upstream raphael/async (18a72436, unchanged since the original merge) onto current fork main, then carries forward the branch's own work.

The merge

78 conflict regions across 20 files. Resolutions are uniform: the fork/upstream side evolved docs, attributes and signatures; the async side added maybe_async annotations and awaits; both survive.

Every storage-format-critical file is byte-identical to 3fabbf23 — extensions/mod.rs, codec.rs, metadata.rs, message_secrets.rs, config.rs, messages/codec.rs, proposals.rs, key_package_in.rs. That was the main risk and it did not materialize.

Three things needed more than mechanical resolution:

extensions-draft-08 → extensions-draft throughout, since upstream renamed it after the async branch forked. compat_tests is deliberately left alone — it refers to the published openmls_0_8_1 crate, whose feature genuinely carries the old name.
memory_storage lost its sync/async split, because that split was the branch's own work rather than upstream's. Restored. Its Cargo.toml had hardcoded openmls_traits/sync, which flips the global maybe-async/is_sync on for every graph containing the crate; openmls_rust_crypto now forwards the track instead.
MlsGroup::resolve_app_data_commit had to be restructured, not annotated. It passed a synchronous closure to ProcessedMessage::resolve_app_data_commit, and staging now touches storage. Split into take_unresolved_app_data_commit / set_staged_commit so the await happens between them.

CI

The sqlx provider workflow only ever tested SQLite. Postgres is what the provider exists for, so it now has its own jobs against a real postgres:16 service container:

job	coverage
`sqlite`	`sqlite`, `sqlite,extensions-draft` × ubuntu + macOS
`postgres`	`postgres`, `postgres,extensions-draft`, `sqlite,postgres` (ubuntu only — service containers are unavailable on macOS runners)
`async-group-flow`	runs `openmls_async_tests` end to end

Two fixes were needed to make those jobs mean anything:

tests/lifecycle.rs is SQLite-only but was ungated, so a postgres-only build failed to compile it.
The Postgres tests skip themselves when OPENMLS_SQLX_POSTGRES_URL is unset — and a skipped run still prints test result: ok. 3 passed in 0.00s, which is indistinguishable from a real run. A misconfigured job would have reported green having tested nothing. OPENMLS_SQLX_REQUIRE_POSTGRES turns that skip into a failure, and the workflow sets it.

openmls_async_tests now runs in CI at all. It is its own workspace, so no other job compiled it — which is exactly why it rotted to 53 errors once before. Running it exercises a full storage-backed group flow against the async traits; merely building it enforces the Send guard the provider needs, since a server spawns one task per stream.

Separately, openmls has no default feature, so the storage track is now named explicitly wherever CI builds or tests it. This is not a fix for a broken job — nothing was failing, because openmls_test depends on openmls_traits with sync, which sets the global is_sync and quietly gives every test build the blocking shape. That accident is load-bearing today. Naming the track keeps each job honest about what it covers, and keeps it working when openmls_test stops hardcoding it.

Verified

openmls checks in sync and async, with and without libxmtp's feature set
cargo test -p openmls -F sync → 331 passed; -F sync,extensions-draft,extensions-draft-test-dependencies → 389 passed; 0 failures, 24 suites each
memory_storage on both tracks; sqlx_storage on sqlite, postgres, and both, with and without extensions-draft
openmls_async_tests runs to completion
clippy -D warnings across the full package list under -F sync
both workspace jobs unchanged

Before this can merge

The 15 virtual-clients-draft storage methods the fork added after the async branch was cut are still synchronous, so virtual-clients-draft + async does not build. Off libxmtp's path (it does not enable that feature), but it should be async-ified before this lands.
openmls_test hardcoding openmls_traits/sync means openmls's own test suite can only run the sync shape. Async test infra is its own piece of work.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PE4EuVF1vYZk1NrWy7Gqon

Note

Convert OpenMLS storage traits and MlsGroup API to async with maybe-async and add PostgreSQL backend to sqlx_storage

Converts StorageProvider and PublicStorageProvider traits in traits/src/storage.rs and traits/src/public_storage.rs to async using maybe-async, making all storage operations return futures.
Converts all MlsGroup, PublicGroup, SignatureKeyPair, EncryptionKeyPair, LeafNode, and related methods to async throughout the openmls crate; sync behavior is preserved via the sync cargo feature (maybe-async/is_sync).
Adds a PostgresStorageProvider to sqlx_storage behind a postgres feature flag, with dedicated migrations in sqlx_storage/migrations_pg/ and a custom _openmls_sqlx_migrations table with advisory locking.
Adds a new openmls_async_tests binary crate as an end-to-end async integration test using the SQLx storage backend.
Restructures CI in .github/workflows/build_test_sqlx_provider.yml to run separate sqlite/postgres jobs with a Postgres 16 service container; adds sync feature to all other CI workflow build/test invocations.
Risk: All callers of StorageProvider, MlsGroup, and related APIs must now use async/await or opt into the sync feature; this is a breaking API change for all storage provider implementors.

📊 Macroscope summarized e1ce848. 48 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

tyler/maybe-async was based on fork rev e125674; fork main has since advanced to 3fabbf2 through PR #55 (upstream merge to openmls main 65396d8) and PR #56 (the fork storage-format work). 3fabbf2 was not an ancestor of the branch, so repinning libxmtp to it would have reverted the persisted wire format -- storage_tag numbering on ExtensionType/Extension, PastEpochDeletionPolicy as a portable u64, tolerant deserialization of fork-only trailing fields -- and would not have resolved at all, since libxmtp pins features (extensions-draft, draft-ietf-mls-pq-ciphersuites) that the branch did not have under those names. This re-merges upstream raphael/async (18a7243, unchanged since the original merge) onto 3fabbf2, then carries forward the branch's own work: the sqlx Postgres provider (provider.rs, sql.rs, migrations_pg, the migrator rewrite, tests/postgres.rs) and the RefCell -> tokio::sync::Mutex change that makes the provider's futures Send. 78 conflict regions across 20 files. The resolutions are uniform: the fork or upstream side evolved docs, attributes and signatures, the async side added maybe_async annotations and awaits, and both survive. Every storage-format file is byte-identical to 3fabbf2. Notable, beyond mechanical conflict resolution: - extensions-draft-08 was renamed extensions-draft upstream after the async branch forked. Renamed throughout, except compat_tests, which refers to the published openmls_0_8_1 crate whose feature genuinely carries the old name. - memory_storage lost its sync/async split in the re-merge, because that split is the branch's own work rather than upstream's. Restored: the Cargo.toml hardcoded openmls_traits/sync, which flips maybe-async/is_sync on for every graph containing the crate -- the same track-selecting-feature trap recorded for the workspace-hack, one layer down. openmls_rust_crypto now forwards the track instead. - MlsGroup::resolve_app_data_commit had to be restructured rather than annotated: it passed a synchronous closure to ProcessedMessage::resolve_app_data_commit, and staging now touches storage, which is async. Split into take_unresolved_app_data_commit / set_staged_commit so the await happens between them. Known gap: the 15 virtual-clients-draft storage methods the fork added after the async branch was cut are still synchronous, so virtual-clients-draft combined with async does not build. libxmtp does not enable virtual-clients-draft, so this is off the critical path; async-ifying them is its own change. Also needs doing before this is CI-clean: openmls has no default feature, so every tests.yml matrix entry needs sync added -- the branch predates the current matrix and has no precedent to copy. Verified: openmls checks in sync and async, with and without libxmtp's feature set; memory_storage checks on both tracks; sqlx_storage checks on both backends; openmls_async_tests compiles, including its Send assertion on the full group flow.

macroscopeapp · 2026-08-05T17:26:16Z

@@ -606,7 +625,8 @@ impl MlsGroup {
    }

    /// Set the past epoch secret deletion policy for the group.


🟠 High mls_group/mod.rs:627

set_past_epoch_deletion_policy writes the new join config and the resized message secrets store in two separate await points. If the future is dropped between the two, storage records the stricter policy (e.g. MaxEpochs(0)) while the old, larger set of past-epoch secrets is retained. MlsGroup::load reads both records without reconciling them, so after a restart the group advertises the stricter policy but still holds the secrets it should have deleted, weakening forward secrecy. Consider persisting both values atomically or adding a load-time reconciliation step.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/mod.rs around line 627: `set_past_epoch_deletion_policy` writes the new join config and the resized message secrets store in two separate `await` points. If the future is dropped between the two, storage records the stricter policy (e.g. `MaxEpochs(0)`) while the old, larger set of past-epoch secrets is retained. `MlsGroup::load` reads both records without reconciling them, so after a restart the group advertises the stricter policy but still holds the secrets it should have deleted, weakening forward secrecy. Consider persisting both values atomically or adding a load-time reconciliation step.

macroscopeapp · 2026-08-05T17:26:16Z

    ) -> Result<MlsMessageOut, CreateMessageError> {
-        let (_, output) =
-            self.create_message_internal::<_, CreateMessageError>(provider, signer, message)?;
+        let (_, output) = self


🟠 High mls_group/application.rs:55

After this PR makes create_message async, dropping the returned future after encrypt advances the in-memory ratchet but before write_message_secrets completes leaves the in-memory message_secrets_store ahead of durable storage. Reloading the group from the stale persisted state and sending another message reuses the same ratchet generation, producing MLS key/nonce reuse that weakens ciphertext confidentiality. encrypt consumes the generation inside try_from_authenticated_content and only then awaits the storage write, so a cancellation (timeout, select!, etc.) between those steps silently desynchronizes the two. Consider making the ratchet advance and its persistence transactional, or otherwise guard the await so cancellation cannot leave memory ahead of durable state.

Also found in 2 other location(s)

openmls/src/group/mls_group/mod.rs:841

encrypt advances the in-memory secret-tree ratchet in try_from_authenticated_content before awaiting write_message_secrets. If the future is cancelled at this await (for example by a timeout or select!), the caller retains an advanced MlsGroup while storage can still contain the pre-encryption ratchet. A subsequent restart/reload can therefore reuse the same encryption generation/key, weakening MLS nonce/key-reuse protections. Persist the ratchet transition in a cancellation-safe transaction or otherwise ensure cancellation cannot leave memory ahead of durable state.

openmls/src/group/mls_group/proposal.rs:465

propose_add_member_impl is not cancellation-safe at the new await of content_to_mls_message. For ciphertext framing, the nested encrypt call advances the in-memory secret tree before awaiting write_message_secrets; dropping this future while that storage write is pending returns no message but can leave memory advanced and persisted secrets stale. Reloading the group can then reuse the same ratchet generation/key (and nonce), while the proposal has also already been queued. The state mutation and persistence need cancellation-safe/transactional handling before this API can safely be async.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/application.rs around line 55: After this PR makes `create_message` async, dropping the returned future after `encrypt` advances the in-memory ratchet but before `write_message_secrets` completes leaves the in-memory `message_secrets_store` ahead of durable storage. Reloading the group from the stale persisted state and sending another message reuses the same ratchet generation, producing MLS key/nonce reuse that weakens ciphertext confidentiality. `encrypt` consumes the generation inside `try_from_authenticated_content` and only then awaits the storage write, so a cancellation (timeout, `select!`, etc.) between those steps silently desynchronizes the two. Consider making the ratchet advance and its persistence transactional, or otherwise guard the await so cancellation cannot leave memory ahead of durable state. Also found in 2 other location(s): - openmls/src/group/mls_group/mod.rs:841 -- `encrypt` advances the in-memory secret-tree ratchet in `try_from_authenticated_content` before awaiting `write_message_secrets`. If the future is cancelled at this await (for example by a timeout or `select!`), the caller retains an advanced `MlsGroup` while storage can still contain the pre-encryption ratchet. A subsequent restart/reload can therefore reuse the same encryption generation/key, weakening MLS nonce/key-reuse protections. Persist the ratchet transition in a cancellation-safe transaction or otherwise ensure cancellation cannot leave memory ahead of durable state. - openmls/src/group/mls_group/proposal.rs:465 -- `propose_add_member_impl` is not cancellation-safe at the new await of `content_to_mls_message`. For ciphertext framing, the nested `encrypt` call advances the in-memory secret tree before awaiting `write_message_secrets`; dropping this future while that storage write is pending returns no message but can leave memory advanced and persisted secrets stale. Reloading the group can then reuse the same ratchet generation/key (and nonce), while the proposal has also already been queued. The state mutation and persistence need cancellation-safe/transactional handling before this API can safely be async.

macroscopeapp · 2026-08-05T17:26:16Z

🟠 High

openmls/openmls/src/group/mls_group/proposal.rs

Line 517 in e1ce848

let proposal_ref = proposal.proposal_reference();

propose_remove_member_impl persists the proposal to storage and adds it to the in-memory ProposalStore before awaiting content_to_mls_message. If the future is cancelled at that await, the caller gets no proposal message but the group still retains the queued proposal (and, in the ciphertext path, a consumed encryption generation). Retrying then produces inconsistent state — duplicate proposals or wasted ratchet generations — with no error surfaced. Consider deferring the storage write and proposal_store_mut().add() until after framing succeeds, or implementing a rollback on cancellation.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/proposal.rs around line 517: `propose_remove_member_impl` persists the proposal to storage and adds it to the in-memory `ProposalStore` *before* awaiting `content_to_mls_message`. If the future is cancelled at that await, the caller gets no proposal message but the group still retains the queued proposal (and, in the ciphertext path, a consumed encryption generation). Retrying then produces inconsistent state — duplicate proposals or wasted ratchet generations — with no error surfaced. Consider deferring the storage write and `proposal_store_mut().add()` until after framing succeeds, or implementing a rollback on cancellation.

macroscopeapp · 2026-08-05T17:26:16Z

    ) -> Result<JoinBuilder<'a, Provider>, WelcomeError<Provider::StorageError>> {
        let processed_welcome =
-            ProcessedWelcome::new_from_welcome(provider, mls_group_config, welcome)?;
+            ProcessedWelcome::new_from_welcome(provider, mls_group_config, welcome).await?;


🟠 High mls_group/creation.rs:578

build_from_welcome calls ProcessedWelcome::new_from_welcome, whose async path deletes the matching key package in keys_for_welcome before later await points (such as PSK loading). If the future is dropped after that deletion, the consumed Welcome is lost and the key package stays deleted, so retrying the Welcome fails and the invitation is irrecoverably lost. The deletion should be deferred until the entire new_from_welcome future completes successfully, or cancellation must restore the key package.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/creation.rs around line 578: `build_from_welcome` calls `ProcessedWelcome::new_from_welcome`, whose async path deletes the matching key package in `keys_for_welcome` before later `await` points (such as PSK loading). If the future is dropped after that deletion, the consumed `Welcome` is lost and the key package stays deleted, so retrying the Welcome fails and the invitation is irrecoverably lost. The deletion should be deferred until the entire `new_from_welcome` future completes successfully, or cancellation must restore the key package.

macroscopeapp · 2026-08-05T17:26:16Z

    ) -> Result<(), MergePendingCommitError<Provider::StorageError>> {
        match &self.group_state {
            MlsGroupState::PendingCommit(_) => {
                let old_state = mem::replace(&mut self.group_state, MlsGroupState::Operational);


🟠 High mls_group/processing.rs:579

merge_pending_commit replaces self.group_state with MlsGroupState::Operational on line 579 before awaiting merge_staged_commit. If the future is dropped while suspended in the merge (task abort, timeout, or scheduler cancellation), the staged commit held in pending_commit_state is dropped, so it is permanently lost, yet the group is already marked Operational. The group ends up with no pending commit and no way to recover the staged commit, leaving the local state inconsistent with the rest of the group. The fix is to keep MlsGroupState::PendingCommit installed until merge_staged_commit completes successfully (moving the mem::replace after the await), or implement cancellation-safe rollback that restores the pending state on failure.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/processing.rs around line 579: `merge_pending_commit` replaces `self.group_state` with `MlsGroupState::Operational` on line 579 *before* awaiting `merge_staged_commit`. If the future is dropped while suspended in the merge (task abort, timeout, or scheduler cancellation), the staged commit held in `pending_commit_state` is dropped, so it is permanently lost, yet the group is already marked `Operational`. The group ends up with no pending commit and no way to recover the staged commit, leaving the local state inconsistent with the rest of the group. The fix is to keep `MlsGroupState::PendingCommit` installed until `merge_staged_commit` completes successfully (moving the `mem::replace` after the await), or implement cancellation-safe rollback that restores the pending state on failure.

macroscopeapp · 2026-08-05T17:26:16Z

        storage: &Storage,
        mls_group_config: &MlsGroupJoinConfig,
    ) -> Result<(), Storage::Error> {
        self.mls_group_config = mls_group_config.clone();


🟡 Medium mls_group/mod.rs:293

set_configuration overwrites self.mls_group_config before awaiting write_mls_join_config. If the storage write is cancelled (timeout or task drop), the future is abandoned, but the in-memory group already holds the new config — so the group diverges from persisted state and the new config is silently lost on reload. Move the assignment after the .await completes successfully.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/mod.rs around line 293: `set_configuration` overwrites `self.mls_group_config` *before* awaiting `write_mls_join_config`. If the storage write is cancelled (timeout or task drop), the future is abandoned, but the in-memory group already holds the new config — so the group diverges from persisted state and the new config is silently lost on reload. Move the assignment after the `.await` completes successfully.

macroscopeapp · 2026-08-05T17:26:16Z

🟠 High

openmls/openmls/src/group/mls_group/processing.rs

Line 374 in e1ce848

.write_message_secrets(self.group_id(), &self.message_secrets_store)

process_message advances the in-memory sender ratchet in decrypt_message and then suspends at the write_message_secrets .await. If the future is dropped at that point (e.g. a request timeout or select!), the consumed generation stays consumed in memory while storage may retain the old secret tree. A retry of the same valid message in the current process then fails as an already-consumed generation, and a reload can resurrect the consumed key, weakening forward secrecy and replay protection. Consider making the ratchet mutation and its persistence cancellation-safe or transactional so the in-memory state and storage cannot diverge on drop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/processing.rs around line 374: `process_message` advances the in-memory sender ratchet in `decrypt_message` and then suspends at the `write_message_secrets` `.await`. If the future is dropped at that point (e.g. a request timeout or `select!`), the consumed generation stays consumed in memory while storage may retain the old secret tree. A retry of the same valid message in the current process then fails as an already-consumed generation, and a reload can resurrect the consumed key, weakening forward secrecy and replay protection. Consider making the ratchet mutation and its persistence cancellation-safe or transactional so the in-memory state and storage cannot diverge on drop.

macroscopeapp · 2026-08-05T17:26:16Z

+            )
+            .await?;
        Ok((framing.message, proposal_ref))
    }


🟠 High mls_group/proposal.rs:655

The .await on queue_proposal introduces a cancellation point between the storage write and the in-memory proposal_store_mut().add. If the future is dropped after queue_proposal completes but before add runs, the proposal is persisted in storage but missing from the live MlsGroup. A commit from that instance can omit the proposal, and it only reappears after the group is reloaded from storage. Consider making the storage write and in-memory update atomic, or reordering so the in-memory state is updated before (or together with) the persisted state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/proposal.rs around line 655: The `.await` on `queue_proposal` introduces a cancellation point between the storage write and the in-memory `proposal_store_mut().add`. If the future is dropped after `queue_proposal` completes but before `add` runs, the proposal is persisted in storage but missing from the live `MlsGroup`. A commit from that instance can omit the proposal, and it only reappears after the group is reloaded from storage. Consider making the storage write and in-memory update atomic, or reordering so the in-memory state is updated before (or together with) the persisted state.

macroscopeapp · 2026-08-05T17:26:16Z

🟠 High

openmls/openmls/src/group/mls_group/mod.rs

Line 449 in e1ce848

// If the proposal store is not empty...

clear_pending_proposals empties the in-memory ProposalStore before awaiting clear_proposal_queue. If the future is dropped while the storage call is pending (or the call returns an error), the group is left with an empty in-memory store while the proposals remain persisted in storage. Any commit that references those proposals can no longer resolve them, and retrying clear_pending_proposals is a no-op for storage because the in-memory store is already empty. Move the self.proposal_store_mut().empty() call to after the awaited clear_proposal_queue succeeds.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/mod.rs around line 449: `clear_pending_proposals` empties the in-memory `ProposalStore` *before* awaiting `clear_proposal_queue`. If the future is dropped while the storage call is pending (or the call returns an error), the group is left with an empty in-memory store while the proposals remain persisted in storage. Any commit that references those proposals can no longer resolve them, and retrying `clear_pending_proposals` is a no-op for storage because the in-memory store is already empty. Move the `self.proposal_store_mut().empty()` call to after the awaited `clear_proposal_queue` succeeds.

macroscopeapp · 2026-08-05T17:26:17Z

        match self.group_state {
            MlsGroupState::PendingCommit(ref pending_commit_state) => {
                if let PendingCommitState::Member(_) = **pending_commit_state {
                    self.group_state = MlsGroupState::Operational;


🟠 High mls_group/mod.rs:426

clear_pending_commit mutates self.group_state to Operational before awaiting write_group_state. If the future is dropped (cancelled) while that storage write is pending, the in-memory group state loses its pending commit but storage still holds the old PendingCommit state — a partial update the caller cannot detect. Reloading the group then restores the pending commit, producing an inconsistent view. Consider persisting first or restoring the old state if the write fails.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/mod.rs around line 426: `clear_pending_commit` mutates `self.group_state` to `Operational` before awaiting `write_group_state`. If the future is dropped (cancelled) while that storage write is pending, the in-memory group state loses its pending commit but storage still holds the old `PendingCommit` state — a partial update the caller cannot detect. Reloading the group then restores the pending commit, producing an inconsistent view. Consider persisting first or restoring the old state if the write fails.

macroscopeapp · 2026-08-05T17:29:17Z

        let welcome = bundle.to_welcome_msg();
        let (commit, _, group_info) = bundle.into_contents();

        provider


🟠 High mls_group/membership.rs:299

remove_members performs a redundant write_group_state after stage_commit has already persisted the pending commit. If the .await on this second write is cancelled, the caller never receives the commit message but the group is left in PendingCommit, so subsequent operations fail with PendingCommit until the application explicitly clears the orphaned state. Remove this extra write — stage_commit already persists group_state.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/membership.rs around line 299: `remove_members` performs a redundant `write_group_state` after `stage_commit` has already persisted the pending commit. If the `.await` on this second write is cancelled, the caller never receives the commit message but the group is left in `PendingCommit`, so subsequent operations fail with `PendingCommit` until the application explicitly clears the orphaned state. Remove this extra write — `stage_commit` already persists `group_state`.

macroscopeapp · 2026-08-05T17:29:18Z


    /// Deletes the [`PublicGroup`] from storage.
-    pub fn delete<Storage: PublicStorageProvider>(
+    #[maybe_async::maybe_async]


🟠 High public_group/mod.rs:519

PublicGroup::delete removes four storage entries with individual .await calls. If the future is dropped after any of these completes, the persisted group is left partially deleted — for example, the tree is removed while confirmation tag, context, and interim transcript hash remain. A subsequent load then sees an inconsistent record. Each delete_* call is separate with no rollback, so cancellation produces partial state even when the storage provider itself never errors. Consider making the deletion atomic or cancellation-safe (e.g., guard against partial completion) so the group is either fully removed or not removed at all.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/public_group/mod.rs around line 519: `PublicGroup::delete` removes four storage entries with individual `.await` calls. If the future is dropped after any of these completes, the persisted group is left partially deleted — for example, the tree is removed while confirmation tag, context, and interim transcript hash remain. A subsequent load then sees an inconsistent record. Each `delete_*` call is separate with no rollback, so cancellation produces partial state even when the storage provider itself never errors. Consider making the deletion atomic or cancellation-safe (e.g., guard against partial completion) so the group is either fully removed or not removed at all.

The sqlx provider workflow only ever tested SQLite. Postgres is what the provider exists for, so it now has its own jobs against a real server. sqlx provider matrix: - sqlite, sqlite+extensions-draft, on ubuntu and macos - postgres, postgres+extensions-draft, and sqlite+postgres, on ubuntu with a postgres:16 service container. Linux only, because service containers are unavailable on macos runners. - the both-backends-on combination gets its own entry: the two are additive rather than exclusive, so it is a distinct configuration. Two things were needed to make the Postgres jobs mean anything. `tests/lifecycle.rs` is SQLite-only but was ungated, so a postgres-only build failed to compile it. Gated on `sqlite`. More importantly, the Postgres tests skip themselves when OPENMLS_SQLX_POSTGRES_URL is unset -- and a skipped run still prints `test result: ok. 3 passed` in 0.00s, which is indistinguishable from a real one at a glance. A CI job that could not reach its server would have reported green having tested nothing. OPENMLS_SQLX_REQUIRE_POSTGRES turns that skip into a failure, and the workflow sets it. Verified both directions: the guard fires when the URL is missing, and the tests still pass against a real server. `openmls_async_tests` now runs in CI. It is its own workspace, so no other job compiled it -- which is exactly why it rotted to 53 errors once before. Running it exercises a full storage-backed group flow against the async traits, and building it enforces the Send guard the provider needs, since a server spawns one task per stream. Separately, openmls has no default feature, so the storage track is now named explicitly wherever CI builds or tests it -- tests.yml, tests_nightly.yml, clippy.yml, coverage.yml, build.yml. This is not a fix for a broken job: nothing was failing, because `openmls_test` depends on `openmls_traits` with `sync`, which sets the global `maybe-async/is_sync` and quietly gives every test build the blocking shape. That accident is load-bearing today and is the reason a trackless job passes. Naming the track keeps each job honest about what it covers, and keeps them working when openmls_test stops hardcoding it. The workspace jobs are left trackless on purpose: `openmls_sqlite_storage` is a member and is inherently synchronous, so the workspace build genuinely is a sync build. Both workspace commands verified unchanged. Verified locally: all workflows parse; every new feature combination compiles; clippy passes with -D warnings across the full package list under -F sync; the sqlx crate builds and tests on sqlite, postgres, and both, with and without extensions-draft; and openmls_async_tests runs to completion. Also rustfmt'd the merge's hand-written conflict resolutions, which `cargo fmt --check` caught, and added a format check for `sqlx_storage` and `openmls_async_tests`. `cargo fmt` at the repo root only reaches the main workspace, so neither crate had ever been format-checked and both had drifted. Three fixes found by running the PR's own CI, each a consumer that never selected a track and so got the async shape: - `targeted_messages.rs` was missing an `.await` on `read_epoch_keypairs`, and its caller needed the `maybe_async` annotation. That feature is outside libxmtp's set, so the merge never compiled it. - `openmls-wasm` calls the group API from plain `fn`s; it now depends on openmls and the provider crates with `sync`. - `cargo hack`'s feature powerset explored combinations with neither track on. It now forces `--features sync` and excludes `async` from the set: the two are a single choice and `is_sync` is global, so a combination enabling neither checks a shape that never ships. Verified locally -- 64 combinations, all pass.

macroscopeapp · 2026-08-05T17:45:51Z

🟠 High

openmls/openmls/src/group/mls_group/staged_commit.rs

Line 1 in 491a43c

use core::fmt::Debug;

merge_commit mutates the in-memory group state (e.g. merge_diff, replacing group_epoch_secrets and message_secrets) before awaiting the storage writes that persist those changes. If the future is cancelled at any of the subsequent .await points (lines 696, 703, 742, 796–834), the caller is left holding an MlsGroup whose epoch is advanced in memory while the persisted state still reflects the old epoch. Reloading the group from storage then produces a stale tree, secrets, and keypairs, causing decryption failures and loss of cryptographic state. The in-memory mutation should only be committed after all awaited persistence writes have completed successfully, or the new state should be staged separately and atomically swapped into place once the writes succeed.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @openmls/src/group/mls_group/staged_commit.rs around line 1: `merge_commit` mutates the in-memory group state (e.g. `merge_diff`, replacing `group_epoch_secrets` and `message_secrets`) *before* awaiting the storage writes that persist those changes. If the future is cancelled at any of the subsequent `.await` points (lines 696, 703, 742, 796–834), the caller is left holding an `MlsGroup` whose epoch is advanced in memory while the persisted state still reflects the old epoch. Reloading the group from storage then produces a stale tree, secrets, and keypairs, causing decryption failures and loss of cryptographic state. The in-memory mutation should only be committed after all awaited persistence writes have completed successfully, or the new state should be staged separately and atomically swapped into place once the writes succeed.

raphaelrobert and others added 6 commits January 7, 2026 11:15

introduce maybe_async

e09e4e8

Merge branch 'main' into raphael/async

3fdc9fc

Merge branch 'main' into raphael/async

3ce702f

Merge branch 'main' into raphael/async

5bbbfb6

follow-up to merging main branch

18a7243

github-actions Bot added the size/xl label Aug 5, 2026

tylerhawkes force-pushed the tyler/maybe-async-remerge branch from e1ce848 to 1b6f3ef Compare August 5, 2026 17:24

macroscopeapp Bot reviewed Aug 5, 2026

View reviewed changes

tylerhawkes force-pushed the tyler/maybe-async-remerge branch from 1b6f3ef to 491a43c Compare August 5, 2026 17:42

macroscopeapp Bot reviewed Aug 5, 2026

View reviewed changes

Navigation Menu

Uh oh!

DO NOT MERGE: re-merge async storage traits (#1923) onto fork main + sqlx Postgres CI - #57

DO NOT MERGE: re-merge async storage traits (#1923) onto fork main + sqlx Postgres CI#57
tylerhawkes wants to merge 7 commits into
mainfrom
tyler/maybe-async-remerge

tylerhawkes commented Aug 5, 2026 •

edited by macroscopeapp Bot

Loading

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

macroscopeapp Bot Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -606,7 +625,8 @@ impl MlsGroup {
		}

		/// Set the past epoch secret deletion policy for the group.

Uh oh!

Conversation

tylerhawkes commented Aug 5, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this is

The merge

CI

Verified

Before this can merge

Convert OpenMLS storage traits and MlsGroup API to async with maybe-async and add PostgreSQL backend to sqlx_storage

🗂️ Filtered Issues

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tylerhawkes commented Aug 5, 2026 •

edited by macroscopeapp Bot

Loading