[tylerhawkes]

Stack

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

• feat(xmtp_db): add IntentState::Superseded for abandoned guarded intents #4019 db — IntentState::Superseded (base: main)
  • feat(xmtp_mls): report app data changes and guard updates against clobbering #4011 xmtp_mls — app-data change callbacks + compare-and-swap guard
    • feat(bindings_mobile): expose the app data change callback and update guard #4012 bindings/mobile · Android path
      • feat(android-sdk): surface app data change callbacks and the update guard #4016 sdks/android · Android path
      • feat(ios-sdk): surface app data change callbacks and the update guard #4015 sdks/ios
    • feat(bindings_node): expose the app data change callback and update guard #4013 bindings/node ← this PR
      • feat(node-sdk): surface app data change callbacks and the update guard #4017 sdks/js/node-sdk
    • feat(bindings_wasm): expose the app data change callback and update guard #4014 bindings/wasm

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

---

Exposes the unstable app-data change callback to the node bindings.

Shape

UnstableChangeCallbacks is a #[napi] class rather than an object, because ThreadsafeFunction is not ToNapiValue and so cannot be an object field. Its constructor takes the callbacks as optional arguments, so adding a handler for another mutable field later is an additional optional parameter — additive for existing TS callers.

new UnstableChangeCallbacks(async (change) => { /* merge */ })

The JS function returns a promise, which is awaited before message processing continues, so a semantic merge can finish before the next message is handled.

group_id is hex-encoded on this boundary, matching how the node bindings surface every other id.

Error handling

A host callback that throws is logged and swallowed. The commit is already durable by the time the callback runs, so there is nothing to roll back; the only sane recovery is to let the next change re-trigger the merge. Propagating would abort message processing for a group over a host-side bug.

Registration

A trailing changeCallbacks parameter on createClient, createClientWithBackend, and the shared inner constructor. undefined registers nothing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AdY7WKkNbJmdzpmUWvW1my

Note

Expose app data change callbacks and compare-and-swap guard for group updates in Node bindings

• Adds an UnstableChangeCallbacks registry (core, FFI, and Node/mobile/Android bindings) that lets callers register an async callback to receive AppDataChange notifications after a group's app_data commits, dispatched outside group locks.
• Introduces an optional compare-and-swap guard to update_app_data: callers may supply an expected_value; if the committed value no longer matches at publish time, the intent is marked Superseded and a non-retryable AppDataSuperseded error is returned.
• Adds IntentState::Superseded to the DB schema and intent state machine, with a new set_group_intent_superseded DB method that only transitions from ToPublish.
• Callbacks are wired through ClientBuilder → XmtpMlsLocalContext → XmtpSharedContext trait and dispatched in sync_with_conn (after mutex release) and in the streaming ProcessMessageFactory path.
• Risk: AppDataSuperseded is a new non-retryable error surface visible to all callers of update_app_data; existing callers must pass None as the new second argument.

📊 Macroscope summarized 314ae86. 21 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

[claude]

Claude finished @tylerhawkes's task —— View job

---

Code Review

Review Date: 2026-08-19 20:04 UTC

Overall, this is a well-structured implementation that correctly exposes the app data change callback to Node.js bindings. The code quality is high and follows established patterns. I have a few minor observations:

Observations

1. Error handling clarity (change_callbacks.rs:91-98): The dual-layer error handling distinguishes between call_async failures (JS runtime issues) and promise rejections (host callback logic). Both are logged and swallowed, which is correct given the commit is already durable. Consider whether the distinction between these error types provides useful debugging value, or if a unified log message would suffice.

2. Memory considerations (change_callbacks.rs:49): The Arc<ThreadsafeFunction<...>> wrapping means each client with callbacks registered holds a reference-counted, thread-safe JS function handle. This is appropriate for the cross-thread callback invocation pattern, but worth noting for SDK consumers running many clients in a single process (already addressed by the use_single_connection DB option).

3. Test coverage gap: The test_utils change at line 52 passes None for change_callbacks. While unit tests for the core callback trait exist in xmtp_mls, there's no integration test demonstrating the node bindings wiring (callback registration → JS function invocation → awaiting promise). Given that this introduces async boundary crossing and error swallowing semantics, an integration test would provide valuable regression protection.

4. Type safety (metadata.rs:154): expected_value: Option<String> correctly mirrors the Rust core API. The TS signature will be expected_value?: string | undefined, which matches the compare-and-swap semantics documented in the PR description.

Summary

The implementation correctly handles the async callback contract, properly converts types at the FFI boundary (hex-encoding group_id), and follows the repository's error handling patterns. The error swallowing behavior is well-justified in code comments. The only gap is integration test coverage for the Node.js callback path.

---

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new feature: app_data change callbacks and a compare-and-swap guard mechanism. It adds new callback infrastructure across multiple platforms (core, mobile, node, Android), a new intent state (Superseded), new error handling, and modifies sync/publish logic. Despite full code ownership by the author, new features with this scope warrant human review. This assessment was made without Macroscope's correctness findings, which could not be posted.

Not approved because:

• Macroscope's correctness review could not post its findings

^{You can add or adjust custom eligibility rules. Learn more.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.88%. Comparing base (61114a7) to head (314ae86).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4013   +/-   ##
=======================================
  Coverage   85.88%   85.88%           
=======================================
  Files         419      419           
  Lines       68348    68348           
=======================================
  Hits        58700    58700           
  Misses       9648     9648           

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

Dismissing prior approval to re-evaluate b76a2e4

Dismissing prior approval to re-evaluate 615d16b

Dismissing prior approval to re-evaluate bec4f9c