fix(xmtp_api_d14n,xmtp_mls): harden the bidi lifecycle, reconnect backoff, and catch-up ahead of enablement

[tylerhawkes]

Pre-enablement hardening for the XIP-83 bidi surface, from a fresh adversarial re-review of the now-merged transport + catch-up + lifecycle code (the interaction bugs that per-PR reviews miss). Bidi still ships default-OFF (XMTP_BIDI_STREAMS_ENABLED), so none of these block a release — they harden the fleet behavior that matters the moment the flag is flipped. Six atomic commits, one per finding.

The two that matter most for enablement

• Order the lifecycle command with the suspend flag under one lock (#1, medium). suspend_bidi_streams/resume_bidi_streams set the process suspend_requested flag under the registry lock but sent each transport's Cmd::Suspend/Cmd::Resume after dropping it. On a multi-threaded runtime a concurrent suspend+resume could race between lock-release and their sends, delivering the commands to a transport opposite to the flag — a wire left live while backgrounded (battery) or parked while foregrounded (no delivery). Now the synchronous command push (enqueue_suspend/enqueue_resume) happens inside the same locked section as the flag; only the reply await runs outside.

• Grow the reconnect backoff on a flapping wire (#2, medium, pre-existing). The backoff reset to the 100 ms floor the instant a wire opened, before any frame. A server that accepts then immediately closes (an overloaded node RSTing after accept) pinned every client at the floor forever, amplifying load against the struggling backend. Now only a wire that stays up ≥ MIN_STABLE_UPTIME resets the backoff; one that dies inside that window is flapping and keeps backing off. The explicit resume() reset is unchanged.

Latent hardening

• Guard the catch-up open against an empty subscription set (#3, low). An empty subs would seed an adds-nothing open frame that never earns a CatchUpComplete, stalling ~90 s → Exhausted. Unreachable today (the welcome topic is always present), but the safety sat far from the open; short-circuit it to Complete.

• Time-box the catch-up half-close (#4, low). catch_up_attempt awaited conn.finish() with no deadline; a wedged actor could hang it. Wrap the half-close and drain under one budget, matching the transport's own close_gracefully.

• Conclude a superseded resume waiter on suspend (#5, low). A resume() awaiting catch-up that a suspend() preempts was left parked until some later resume, stranding the caller (and its task) for the whole background sojourn. A suspend supersedes pending resumes; conclude them at the suspend instead.

Coverage

• Chunking × reconnect and × suspend/resume (#6, test). The multi-wave-per-lease chunking across a wire death or suspend/resume was exercised only by the property test (16-case default); every scripted lifecycle test used the production cap, so none chunked. Adds two deterministic tests at a shrunk cap (over-cap lease survives a wire death; survives a suspend/resume), and raises the property test's default case count.

Verification

• Both affected crates compile; clippy clean; cargo fmt clean.
• All 239 transport scripted tests pass (incl. the two new chunking tests and the updated preempt test); catch-up + lifecycle tests pass against the live node; property model passes with the transport changes.

🤖 Generated with Claude Code

Note

Harden bidi transport lifecycle with reconnect backoff, suspend/resume ordering, and catch-up fixes

• Reconnect backoff in LedgerTask.wire_died now resets to the floor only if the wire stayed up for at least MIN_STABLE_UPTIME (10s); short-lived flaps continue to grow the backoff instead.
• Transport.enqueue_suspend and enqueue_resume allow callers to enqueue lifecycle commands under the SHARED_WIRES lock and await acknowledgment outside it, closing a race where a resume could be enqueued between a flag flip and its suspend command.
• Pending resume() waiters are now resolved immediately when a suspend is processed (both in the normal handler and when a suspend preempts an in-flight dial).
• CatchUp::catch_up_to_live returns early when the subscription set is empty (avoiding a stalled frame) and enforces a timeout on the post-finish drain to prevent indefinite blocking on a wedged connection.
• Behavioral Change: over-cap leases that were still catching up on wire death are now fully re-issued in fresh chunks on reopen rather than resuming mid-stream.

^{Macroscope summarized a238169.}

[claude]

Claude finished @tylerhawkes's task —— View job

---

Code Review Complete

Review Date: July 22, 2026 20:17 UTC

I've reviewed the 4 changed files across 6 commits focused on hardening the XIP-83 bidi lifecycle before enablement. The changes address critical race conditions and improve resilience. Here's my feedback:

---

Critical Issues

None found. The two high-priority fixes (lifecycle command ordering and reconnect backoff) are solid implementations that address real concurrency issues.

---

Code Quality & Best Practices

Excellent overall. The code demonstrates deep understanding of:

• Async Rust concurrency patterns
• Lock ordering disciplines
• State machine invariants
• Defensive programming

Specific observations:

1. Lock ordering fix (router_callbacks.rs:311-326) ✅
   The synchronized command enqueue under the registry lock is the correct solution. The pattern of splitting enqueue_*() from the await is clean and well-documented.

2. Flapping wire backoff (bidi_transport.rs:263-267) ✅
   MIN_STABLE_UPTIME is a well-chosen heuristic. The 10s threshold distinguishes transient RSTs from genuine reconnects without being overly conservative.

3. Empty subscription guard (catch_up.rs:382-388) ✅
   Good defensive programming. The comment at line 382-385 clearly explains why this "impossible" case is worth guarding.

4. Catch-up timeout (catch_up.rs:511-521) ✅
   Time-boxing the half-close and drain together mirrors the transport's own GRACEFUL_CLOSE_BUDGET pattern. Consistent design.

5. Resume waiter preemption (router_callbacks.rs:370-404) ✅
   The settle_lifecycle refactoring that latches on Closed ensures suspended-then-resumed wires that fail discovery still route correctly.

---

Performance Considerations

No concerns. The changes are correctness fixes, not hot paths:

• Lock hold times remain minimal (synchronous push only)
• Backoff logic adds negligible overhead
• Timeout budget is appropriate for the use case

---

Security Concerns

None. These are internal state management fixes with no API surface changes or data exposure.

---

Test Coverage

Strong. The PR includes:

• 2 new deterministic chunking tests (lines 176-247 in bidi_transport.rs)
• Updated property test defaults (line 718: 16→32 cases)
• Updated preempt test for the resume waiter fix

Recommendation: The property tests now run 32 cases by default, which is good for CI. Consider documenting the PROPTEST_CASES env var in CLAUDE.md for developers who want faster local runs during iteration.

---

Minor Observations

1. Documentation quality (throughout): The commit messages and inline comments are exceptional. The PR description's "two that matter most" framing helps reviewers prioritize.

2. Atomicity (6 commits): Each commit is self-contained and well-scoped. This aids bisection and makes the review much easier.

3. Type safety (bidi_transport.rs:432-439): The distinct LeaseId and WaveId types prevent silent mix-ups. Good use of newtype pattern.

---

Conclusion

This is production-ready hardening code. The fixes address real concurrency issues that could manifest under mobile app lifecycle transitions and network flapping. The test coverage gives confidence, and the bidi-default-OFF deployment strategy means any latent issues won't block release.

Recommend: Approve and merge.

---

Todo List:

• Read and analyze crates/xmtp_api_d14n/src/queries/bidi_transport.rs
• Read and analyze crates/xmtp_api_d14n/src/queries/bidi_transport_props.rs
• Read and analyze crates/xmtp_mls/src/subscriptions/catch_up.rs
• Read and analyze crates/xmtp_mls/src/subscriptions/router_callbacks.rs
• Provide review feedback

---

[macroscopeapp]

Approvability

Verdict: Approved

Hardening changes to bidi transport reconnect backoff and lifecycle management. All modified files are owned by the PR author (admin). The changes add defensive checks, improve race condition handling, and include comprehensive tests.

^{You can customize Macroscope's approvability policy. Learn more.}

[codecov]

Codecov Report

❌ Patch coverage is 94.91525% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.74%. Comparing base (622cf37) to head (a238169).

Files with missing lines	Patch %	Lines
crates/xmtp_api_d14n/src/queries/bidi_transport.rs	96.62%	3 Missing ⚠️
crates/xmtp_mls/src/subscriptions/catch_up.rs	81.81%	2 Missing ⚠️
...tes/xmtp_mls/src/subscriptions/router_callbacks.rs	94.44%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3887      +/-   ##
==========================================
+ Coverage   85.64%   85.74%   +0.09%     
==========================================
  Files         426      426              
  Lines       68005    68102      +97     
==========================================
+ Hits        58245    58394     +149     
+ Misses       9760     9708      -52     

☔ View full report in Codecov by Harness.
📢 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.