fix(xmtp_api_d14n): keep commit-log on the retained v3 service after cutover

[tylerhawkes]

What

Fixes a latent correctness bug in the shipping MigrationClient: commit-log (fork detection) silently dies the moment a client crosses the v3→d14n cutover.

Commit-log is deliberately not part of d14n's network ordering — the plan keeps a centralized v3-shaped commit-log service running through and past the cutover ("basically keep that part of v3 running"). But MigrationClient routed publish_commit_log / query_commit_log through choose_client, so a migrated client's commit-log calls followed its reads/writes to xmtpd — where the commit-log methods are deliberate no-ops (publish drops, query returns empty). Fork detection would go dark at migration with no error.

Fix

Route commit-log to the v3 client unconditionally, independent of cutover state — the one surface that must never follow the cutover. Also drop write_with_refresh for it: the retained service never emits the migration-rejection signal that reroutes writes to xmtpd.

async fn publish_commit_log(&self, request) -> Result<(), Self::Error> {
    self.v3_client.publish_commit_log(request).await   // was: choose_client()
}
async fn query_commit_log(&self, request) -> Result<..., Self::Error> {
    self.v3_client.query_commit_log(request).await     // was: choose_client()
}

Test

A deterministic unit test: with has_migrated = true (so every other call routes to xmtpd), both commit-log calls must still hit v3. The d14n mock is left bare, so a wrongly-routed call panics — I verified the test fails on the old routing and passes on the fix.

Scope

This is Phase 2 of 3 (Phase 1: #3888). This PR is the routing fix + its unit proof. Lifting the ci-d14n not (test(/commit_log/)) exclusion so the full commit-log suite runs live through a migration-backed client is deferred — it needs the migration test harness from #3888 to give commit-log a v3 target in the d14n lane, and is better as its own change.

Verification

• xmtp_api_d14n compiles; clippy + fmt clean.
• All 14 queries::combined::tests pass (13 existing + the new one).
• Negative check: the new test fails on the pre-fix routing (panics on the bare d14n mock), confirming it's not vacuous.

🤖 Generated with Claude Code

Note

Fix commit-log routing to always use v3 client after cutover

After migration, publish_commit_log and query_commit_log were still routing through choose_client(), which could send commit-log traffic to the wrong backend. Both methods in combined.rs now call v3_client directly, bypassing the migration-aware router entirely. A new test in tests.rs verifies that both calls land on v3 (and not d14n) when has_migrated = true.

^{Macroscope summarized c656399.}

[claude]

Claude finished @tylerhawkes's task —— View job

---

Code Review Complete ✓

Review for PR #3889 — 2026-07-22 22:55 UTC

Summary

The fix is correct and well-tested. The commit-log routing change addresses a critical correctness issue with clear implementation and comprehensive test coverage.

Analysis

Code Quality: ✓ Excellent

• The inline documentation (lines 208-216 in combined.rs) is exemplary — it clearly explains why commit-log is exceptional and what would break if it followed the normal routing
• The fix is minimal and surgical: only the two affected methods changed
• Consistent with Rust idioms and project patterns

Correctness: ✓ Verified

• The unconditional routing to v3_client is architecturally correct per the PR description (commit-log service is retained through cutover)
• Removing write_with_refresh wrapper is justified — the v3 commit-log service won't emit the migration-rejection signal that triggers rerouting

Test Coverage: ✓ Strong

• The new test commit_log_stays_on_v3_after_migration (lines 309-340 in tests.rs) is well-designed:
  • Uses has_migrated = true to ensure other calls would route to d14n
  • v3 mock counts calls; d14n mock is bare (would panic if wrongly called)
  • Tests both publish_commit_log and query_commit_log
  • Verifies exactly 2 calls hit v3 (neither hit d14n)
• Negative testing confirmed by author (test fails on old routing)

Performance: No concerns

• Removing the write_with_refresh wrapper eliminates unnecessary error-handling overhead for commit-log (slight improvement)
• Direct call to v3_client avoids the choose_client() state checks

Security: No concerns

• No changes to authentication, encryption, or data handling
• Fork-detection mechanism correctly preserved

Minor Observations

None — the implementation is clean and complete.

---

[macroscopeapp]

Approvability

Verdict: Approved

Targeted fix ensuring commit-log operations route to v3 unconditionally during d14n migration (preventing silent fork-detection failure). Author owns all changed files, change is well-documented with explanatory comments, and includes comprehensive test coverage.

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.66%. Comparing base (ebe0e1f) to head (c656399).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3889      +/-   ##
==========================================
- Coverage   85.74%   85.66%   -0.09%     
==========================================
  Files         426      426              
  Lines       68102    68098       -4     
==========================================
- Hits        58394    58333      -61     
- Misses       9708     9765      +57     

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

[blacksmith-sh]

Found 1 test failure on Blacksmith runners:

Failure

Test	View Logs
org.xmtp.android.library.ArchiveTest/testStreamsAndMessages	View Logs