feat(xmtp_api_d14n): XIP-83 bidi half-close + live integration tests

[tylerhawkes]

What

Builds on the merged XIP-83 BidiConnection actor (#3772). Adds the bounded-sync half-close API and the first live integration tests that exercise a real BidiConnection against a running node, plus the small forward that lets the standard test client open one.

Changes

1. TrackedStatsClient forwards XmtpMlsBidiStreams (xmtp_api_d14n/src/queries/api_stats.rs, native-only). The stats wrapper forwarded every client trait except bidi, so the feature-switched TestClient (TrackedStatsClient<V3Client>) could not open a BidiConnection. One delegating impl, no per-call stat (the bidi stream is opened once and mutated in place, not counted per RPC).

2. BidiConnection::finish() — half-close / bounded sync (XIP-83 server req 9 / client req 4). A new Command::Finish makes the actor drop its outbound (request) half so the stream half-closes; the server finishes the in-flight catch-up wave, emits its TopicsLive / CatchUpComplete markers, and closes its side, so the consumer drains next() to None. After finish(), mutate/probe return Closed. Opened with Mutate::history_only, this is the bounded catch-up ("sync") flow over the same connection. The response→event translation is factored into a shared emit_response_events so the live loop and the post-finish drain deliver identical events.

3. Live v3 integration tests (xmtp_mls/src/subscriptions/bidi_tests.rs). v3-only and native-only (the bidi transport is implemented for the v3 client and needs full-duplex HTTP/2; TestClient is only bidi-capable under the v3 config). They open a real BidiConnection against whichever backend the test feature switch selects and derive every assertion from the stream itself — using each frame's is_commit flag to count application messages independently of the MLS commits a membership change produces, rather than a side query:

   • bidi_connection_delivers_live_welcome_over_the_wire — handshake, live welcome after a membership change, ping/pong probe.
   • bidi_catch_up_precedes_live_marker_then_streams_live — the happy path in one test: pre-subscription history is caught up strictly before the TopicsLive marker; messages published while the stream starts arrive exactly once on either side of the marker; post-marker sends stream live and are strictly newer than every catch-up cursor; CatchUpComplete echoes the mutate_id.
   • bidi_history_only_catches_up_then_delivers_nothing_live — history_only catches up and emits the markers but registers nothing for live delivery.
   • bidi_history_only_half_close_drains_then_server_closes — bounded sync: history_only + finish(), the server closes after the wave, and mutate then returns Closed.

Testing

All four integration tests pass against a local node-go:main carrying the bidi Subscribe support. The 13 BidiConnection actor unit tests still pass after the refactor. just lint-rust is clean; the v3-only test module was additionally linted with cargo clippy -p xmtp_mls --tests (the workspace lint runs --all-features, which enables d14n and so skips this module).

Note

Add XIP-83 bidi half-close support and live integration tests to BidiConnection

• Adds a finish() method to BidiConnection that half-closes the outbound stream: the actor stops accepting new commands, flushes any pending frames best-effort, drops the outbound channel, then drains inbound until the server closes while continuing to emit events.
• Introduces Command::Finish in the actor loop and an AtomicBool latch so mutate() and probe() return Closed immediately after finish() is delivered.
• Extracts emit_response_events and drain_after_finish as helpers in connection.rs to cleanly separate teardown logic from the main actor loop.
• Adds native-only integration tests in bidi_tests.rs covering live welcome delivery, catch-up + live streaming, history-only mode, and bounded sync via history_only + finish.
• Implements XmtpMlsBidiStreams for TrackedStatsClient on native targets, forwarding subscribe_bidi to the inner client.

^{Macroscope summarized 5d2c9c4.}

[claude]

Claude finished @tylerhawkes's task —— View job

---

Code Review Completed

Review Date: June 29, 2026 at 21:59 UTC

Based on my analysis of the XIP-83 bidi half-close implementation, I've identified the following areas that warrant attention:

Outstanding Critical Issues

The three unresolved review comments from macroscopeapp identify real correctness concerns that should be addressed:

1. ResetFlagOnCancel race condition (connection.rs:129): Two concurrent finish() calls can race such that one succeeds while the other's cancellation resets the finished flag back to false, leaving the connection in an inconsistent state where mutate()/probe() accept commands that will be dropped.

2. Hanging probe after finish (connection.rs:476): When Command::Finish breaks the loop, outstanding probes in the probes map are not resolved before entering drain_after_finish. This can leave caller probe() futures hanging indefinitely, especially on slow-closing streams.

3. Best-effort flush semantics (connection.rs:597): The non-blocking flush in drain_after_finish will silently drop accepted frames if the wire is temporarily full. While documented as acceptable due to cursor-based replay, this could surprise callers who received Ok(()) from mutate().

Code Quality Observations

Positive aspects:

• The integration tests in bidi_tests.rs are excellent: they derive assertions entirely from the stream (using is_commit flags), test real wire behavior against a live node, and cover the key scenarios comprehensively
• The emit_response_events refactoring properly shares event translation between the live loop and post-finish drain
• Error handling distinguishes Closed vs ProbeTimedOut appropriately
• The API stats forwarding is minimal and correctly gated with if_native!

Minor improvements:

• Consider adding a unit test for the ResetFlagOnCancel guard's behavior under cancellation (beyond the existing reset_flag_on_cancel_clears_only_while_armed test, focus on the concurrent finish() scenario)
• The next_within helper in tests is good for fail-fast behavior, but consider whether 10s timeouts are appropriate for CI environments that may be slower
• Lines 623-628 in drain_after_finish: the comment explains the non-blocking flush rationale well, but consider whether a limited retry (e.g., 2-3 attempts with brief delays) would improve the success rate without risking wedge

Test Coverage

The codecov report shows 91.52% patch coverage with 15 lines missing. Given the nature of this feature (actor-based concurrency with multiple exit paths), this is reasonable. The four integration tests provide strong end-to-end validation.

Performance & Security

No performance or security concerns identified. The bounded buffers and backpressure mechanisms are sound.

---

Summary: The implementation is well-structured and thoroughly tested. The three unresolved review comments represent real edge cases that should be addressed before merge, particularly the race condition in ResetFlagOnCancel and the hanging probe issue.

---

[macroscopeapp]

Approvability

Verdict: Needs human review

2 blocking correctness issues found. This PR adds new XIP-83 bidi half-close functionality with significant concurrent state management and stream lifecycle changes. The new runtime behavior and unresolved review comments about potential concurrency issues warrant human review.

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

[codecov]

Codecov Report

❌ Patch coverage is 91.01796% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.54%. Comparing base (7a0f50f) to head (5d2c9c4).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/xmtp_api_d14n/src/queries/v3/connection.rs	90.90%	15 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #3789    +/-   ##
========================================
  Coverage   84.54%   84.54%            
========================================
  Files         410      410            
  Lines       60716    60843   +127     
========================================
+ Hits        51331    51440   +109     
- Misses       9385     9403    +18     

☔ 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.