Skip to content

feat(bindings_mobile): expose the app data change callback and update guard - #4012

Merged
tylerhawkes merged 1 commit into
tyler/appdata-cb-corefrom
tyler/appdata-cb-mobile
Aug 19, 2026
Merged

feat(bindings_mobile): expose the app data change callback and update guard#4012
tylerhawkes merged 1 commit into
tyler/appdata-cb-corefrom
tyler/appdata-cb-mobile

Conversation

@tylerhawkes

@tylerhawkes tylerhawkes commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Stack

Merge bottom-up — each PR is based on its parent below.
Android is the priority path: #4019#4011#4012#4016.

Already merged: #4018 (proto regen). Related follow-up, independent of this stack: #4020.


Exposes the unstable app-data change callback over uniffi, for iOS and Android.

Shape

FfiUnstableChangeCallbacks is a uniffi Record whose every field carries #[uniffi(default = ...)] — the convention already used by FfiCatchUpOptions and FfiUpdateAppDataOptions. Adding a callback for another mutable field later is additive: the generated Swift/Kotlin constructors keep their existing shape and compiled apps do not break.

Confirmed by the generated Swift, which comes out as:

public init(appData: FfiAppDataChangeCallback? = nil)

FfiAppDataChangeCallback is an async foreign trait (#[uniffi::export(with_foreign)] + #[xmtp_common::async_trait]), bridged to the core trait by FfiAppDataChangeCallbackBridge — the same pattern as FfiAuthCallbackBridge.

Registration

A new trailing change_callbacks: Option<FfiUnstableChangeCallbacks> on create_client, alongside fork_recovery_opts and worker_config. Passing None registers nothing and costs one Option check per processed message.

This does add a parameter to the exported create_client signature, so every Swift and Kotlin caller must pass a value. The SDK wrappers in the follow-up PRs default it to nil, so app authors see no change.

Note on the test call sites

The 22 updated create_client calls in tests and benches are all mechanical None additions.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AdY7WKkNbJmdzpmUWvW1my

Note

Expose app data change callbacks and compare-and-swap guard in mobile FFI bindings

  • Adds a new change_callbacks parameter to create_client in mls.rs, allowing callers to register an async FfiAppDataChangeCallback that fires when group app data changes.
  • Introduces change_callbacks.rs with FfiAppDataChange, FfiAppDataChangeCallback, FfiUnstableChangeCallbacks, and a bridge adapter that converts core events to foreign FFI callbacks.
  • Adds an optional expected_value field to FfiUpdateAppDataOptions, wiring it through to the inner update_app_data call as a compare-and-swap guard; mismatches may now return an error instead of overwriting.
  • All existing call sites pass None for the new parameters, keeping current behavior unchanged.

Macroscope summarized cce6731.

@tylerhawkes
tylerhawkes requested a review from a team as a code owner August 19, 2026 16:39
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Claude finished @tylerhawkes's task —— View job


PR Review Completed

  • Read implementation files
  • Review code quality and potential issues
  • Assess test coverage concerns
  • Post final review

Review Feedback

2026-08-19 20:04 UTC

The implementation follows established patterns in the codebase (mirroring FfiAuthCallbackBridge) and the approach is sound. I have one minor concern:

Test Coverage

The 10% patch coverage (18 missing lines in change_callbacks.rs) is expected for FFI bridge code — the callback mechanism can only be tested end-to-end from Swift/Kotlin in the SDK layers above. However, I notice:

bindings/mobile/src/mls/change_callbacks.rs:88-90 — The FfiAppDataChangeCallbackBridge::on_app_data_changed implementation is untested within this layer. While full callback testing happens in the SDKs, consider whether a minimal smoke test could verify the conversion (AppDataChangeFfiAppDataChange) doesn't panic. The From implementation at lines 24-32 is straightforward, but having at least one unit test exercising the bridge's async dispatch path would improve confidence.

This is a nice-to-have rather than a blocker — the real validation happens in #4015 and #4016.


Code quality, API design, security, and performance look good. No bugs identified.


macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Aug 19, 2026
@macroscopeapp

macroscopeapp Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at f7a9599

Macroscope's review found this PR approvable — Additive FFI bindings exposing existing core functionality to mobile clients. All new parameters have backwards-compatible defaults. The author owns all changed files and the changes are well-scoped with minimal risk.

No code changes detected at cce6731. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 10.00000% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.88%. Comparing base (c625372) to head (cce6731).

Files with missing lines Patch % Lines
bindings/mobile/src/mls/change_callbacks.rs 0.00% 18 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                    @@
##           tyler/appdata-cb-core    #4012      +/-   ##
=========================================================
- Coverage                  85.91%   85.88%   -0.04%     
=========================================================
  Files                        418      419       +1     
  Lines                      68328    68348      +20     
=========================================================
- Hits                       58705    58700       -5     
- Misses                      9623     9648      +25     

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

@tylerhawkes
tylerhawkes force-pushed the tyler/appdata-cb-mobile branch from c8c03a8 to 7f9f852 Compare August 19, 2026 16:48
@tylerhawkes
tylerhawkes force-pushed the tyler/appdata-cb-mobile branch 2 times, most recently from 8583651 to 2a822f7 Compare August 19, 2026 17:51
@macroscopeapp
macroscopeapp Bot dismissed their stale review August 19, 2026 17:51

Dismissing prior approval to re-evaluate 2a822f7

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Aug 19, 2026
@tylerhawkes
tylerhawkes force-pushed the tyler/appdata-cb-mobile branch from 2a822f7 to 5b80031 Compare August 19, 2026 18:26
@macroscopeapp
macroscopeapp Bot dismissed their stale review August 19, 2026 18:26

Dismissing prior approval to re-evaluate 5b80031

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Aug 19, 2026
@tylerhawkes
tylerhawkes force-pushed the tyler/appdata-cb-mobile branch from 5b80031 to f7a9599 Compare August 19, 2026 18:51
@tylerhawkes tylerhawkes changed the title feat(bindings_mobile): expose the app data change callback over uniffi feat(bindings_mobile): expose the app data change callback and update guard Aug 19, 2026
@macroscopeapp
macroscopeapp Bot dismissed their stale review August 19, 2026 18:54

Dismissing prior approval to re-evaluate f7a9599

@tylerhawkes
tylerhawkes force-pushed the tyler/appdata-cb-mobile branch from f70bd28 to cce6731 Compare August 19, 2026 20:03
@tylerhawkes
tylerhawkes merged commit b68212a into main Aug 19, 2026
61 of 70 checks passed
@tylerhawkes
tylerhawkes deleted the tyler/appdata-cb-mobile branch August 19, 2026 20:41
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.

2 participants