Attempt fork repros

[cameronvoell]

Add a concurrency/recovery test for xmtp_mls::groups::tests::test_concurrent_metadata_updates_with_recovery and bump workspace and bindings versions to 1.7.0-dev

Introduce a new multi-threaded async test validating concurrent group metadata updates and recovery in xmtp_mls/src/groups/tests/mod.rs, and bump workspace and bindings versions to 1.7.0-dev across manifests.

📍Where to Start

Start with the test xmtp_mls::groups::tests::test_concurrent_metadata_updates_with_recovery in xmtp_mls/src/groups/tests/mod.rs.

---

Macroscope summarized 1ac0c57.

[graphite-app]

How to use the Graphite Merge Queue

Add the label mergequeue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

[claude]

Claude finished @cameronvoell's task —— View job

---

Code Review Complete

• Read and analyze changed files
• Review code quality and best practices
• Check for potential bugs and issues
• Provide feedback

---

Review Feedback

Reviewed on: 2025-11-08 01:08 UTC

Test Implementation Analysis

The new test test_concurrent_metadata_updates_with_recovery in xmtp_mls/src/groups/tests/mod.rs:4912 adds valuable coverage for a critical race condition scenario. Here are my observations:

✅ Strengths

1. Well-structured test: The test follows a clear narrative and tests an important edge case (concurrent metadata updates from the same epoch)
2. Comprehensive verification: Checks epoch progression, message exchange, and recovery after the race condition
3. Proper async handling: Uses tokio::join! for concurrent operations
4. Follows conventions: Uses #[xmtp_common::test] as specified in CLAUDE.md

⚠️ Potential Issues

1. Unclear test outcome (xmtp_mls/src/groups/tests/mod.rs:4948-4949):

   // At least one sync should succeed
   let alix_sync_ok = alix_sync_result.is_ok();
   let bo_sync_ok = bo_sync_result.is_ok();

   These variables are assigned but never used. The test doesn't verify that at least one succeeded or document what should happen if both fail.

2. Non-deterministic behavior: The test comment states "one commit will succeed, the other will fail" but doesn't assert this. This makes it unclear what the expected behavior is and whether the test could pass even if both updates somehow succeed or both fail.

3. Missing assertion (xmtp_mls/src/groups/tests/mod.rs:4943-4944): Should verify that exactly one of the sync operations succeeded:

   assert!(alix_sync_ok || bo_sync_ok, "At least one sync should succeed");
   // Or more strictly: assert!(alix_sync_ok ^ bo_sync_ok, "Exactly one sync should succeed");

4. Unclear fork verification: The test title mentions "does not cause a fork" but there's no explicit assertion that verifies fork prevention. The epoch equality checks help, but the test would benefit from documenting what specific fork behavior is being prevented.

5. Test name mismatch: The commit message says "verify self commit on previous epoch does not cause a fork" but the test name is test_concurrent_metadata_updates_with_recovery, which doesn't clearly indicate it's testing fork prevention.

💡 Suggestions

1. Add assertions for the sync results to make expected behavior explicit
2. Consider adding a comment explaining what "fork" means in this context and how the test verifies it doesn't occur
3. Add logging or better documentation of which sync succeeded/failed to aid debugging if the test fails
4. Consider renaming the test to better reflect its purpose (e.g., test_concurrent_epoch_updates_prevent_fork)

Other Changes

The Cargo.lock and package.json changes appear to be from merging main and are standard dependency updates with no concerns.

---

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.46%. Comparing base (7253f22) to head (1ac0c57).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2753      +/-   ##
==========================================
+ Coverage   77.32%   77.46%   +0.13%     
==========================================
  Files         365      367       +2     
  Lines       53843    53872      +29     
==========================================
+ Hits        41636    41731      +95     
+ Misses      12207    12141      -66     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.