Skip to content

fix: prevent refresh_state cursor increment when commit log is disabled#3416

Open
xmtp-coder-agent wants to merge 2 commits intoxmtp:mainfrom
xmtp-coder-agent:fix/issue-2983
Open

fix: prevent refresh_state cursor increment when commit log is disabled#3416
xmtp-coder-agent wants to merge 2 commits intoxmtp:mainfrom
xmtp-coder-agent:fix/issue-2983

Conversation

@xmtp-coder-agent
Copy link
Copy Markdown
Contributor

@xmtp-coder-agent xmtp-coder-agent commented Apr 8, 2026

Resolves #2983

Summary

  • Changes publish_commit_log return type from Result<()> to Result<bool> across the XmtpMlsClient trait and all implementations
  • D14N implementation now returns Ok(false) (no-op), V3 returns Ok(true) (actually published)
  • CommitLogWorker::publish_commit_logs_to_remote only updates the refresh_state cursor when the publish actually occurred (Ok(true))

Problem

When using the D14N client, publish_commit_log returns Ok(()) without actually publishing anything (commit log is not yet implemented for D14N). The caller treats this as success and increments the EntityKind::CommitLogUpload cursor in refresh_state, even though no data was uploaded.

Test plan

  • cargo check passes for all affected crates
  • cargo clippy passes with no warnings
  • Existing commit log tests continue to pass (V3 path unchanged)
  • D14N path no longer increments refresh_state for entity kind 3

🤖 Generated with Claude Code

Note

Prevent refresh_state cursor increment when commit log publishing is disabled

  • Changes the XmtpMlsClient::publish_commit_log trait method to return Result<bool> instead of Result<()>, where false signals a no-op (e.g. commit log disabled) and true signals successful publication.
  • The d14n client in d14n/mls.rs now explicitly returns Ok(false) with a debug log when commit log publishing is disabled.
  • CommitLogWorker::publish_commit_logs_to_remote in commit_log.rs skips updating upload cursors when the API returns false, preventing cursor advancement on no-op publishes.
  • Behavioral Change: all XmtpMlsClient implementors must now handle a boolean return from publish_commit_log; the v3 client returns Ok(true) on success.

Macroscope summarized fc58d64.

Change publish_commit_log return type from Result<()> to Result<bool>
so callers can distinguish a real publish (true) from a no-op (false).
D14N returns false since commit log is not yet implemented, preventing
the CommitLogWorker from incorrectly advancing the refresh_state cursor.

Resolves xmtp#2983

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@xmtp-coder-agent xmtp-coder-agent requested a review from a team as a code owner April 8, 2026 20:16
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.07%. Comparing base (bc560bb) to head (fc58d64).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
crates/xmtp_api_d14n/src/queries/combined.rs 0.00% 1 Missing ⚠️
crates/xmtp_proto/src/api_client/impls.rs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3416      +/-   ##
==========================================
+ Coverage   83.06%   83.07%   +0.01%     
==========================================
  Files         378      378              
  Lines       51829    51834       +5     
==========================================
+ Hits        43050    43061      +11     
+ Misses       8779     8773       -6     

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CommitLogUpload increments refresh_state even if commit_log disabled in d14n

2 participants