feat(android-sdk): surface app data change callbacks and the update guard

[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 ← this PR
      • 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
      • 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.

---

Surfaces the unstable app-data change callback on the Android SDK.

Public surface

ClientOptions(
    appContext = context,
    dbEncryptionKey = key,
    unstableChangeCallbacks = UnstableChangeCallbacks(appData = MyReconciler()),
)

unstableChangeCallbacks defaults to null, so this is source-compatible for every existing caller.

AppDataChangeHandler is an SDK-level interface with a suspend fun taking an AppDataChange with a hex groupId; a private bridge adapts it to the generated FfiAppDataChangeCallback so FfiAppDataChange stays out of the public surface. toFfi() returns null when nothing is registered, so a client that does not use this never pays for the before/after snapshot on the message-processing path.

The identity-probe client passes null explicitly — it never processes messages, so there is nothing to notify about.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AdY7WKkNbJmdzpmUWvW1my

Note

Surface app data change callbacks and update guard in Android SDK

• Adds UnstableChangeCallbacks with AppDataChangeHandler interface and FFI bridge, allowing callers to receive notifications when a group's appData changes.
• Extends ClientOptions with an optional unstableChangeCallbacks field; callbacks are forwarded to the FFI client on construction for both in-memory and persistent DB clients (identity-probe client explicitly skips them).
• Adds an optional expectedAppData parameter to Group.updateAppData implementing a compare-and-swap guard — the core rejects the update if the committed value differs from the expected value.

^{Macroscope summarized c072dae.}

[claude]

Claude finished @tylerhawkes's task —— View job

---

PR Review — 2026-08-19 20:04 UTC

Summary

This PR successfully surfaces the app-data change callback and compare-and-swap guard to the Android SDK. The implementation is clean, follows Kotlin best practices, and maintains source compatibility. Below are observations on code quality, potential issues, and testing.

---

Code Quality & Best Practices

✅ Strengths:

1. Clean API design — UnstableChangeCallbacks is well-structured as a data class with nullable properties, making it easy to extend with additional callbacks in the future (Client.kt:67)

2. Good encapsulation — FfiAppDataChangeHandlerBridge keeps FFI types out of the public API surface (UnstableChangeCallbacks.kt:63-75)

3. Proper coroutine usage — suspend fun onAppDataChanged correctly marks the callback as async, allowing callers to perform async operations like database writes or network calls (UnstableChangeCallbacks.kt:28)

4. Thread safety — Consistently uses withContext(Dispatchers.IO) for FFI calls (Client.kt:606, Group.kt:631)

5. Consistent error handling — The updateAppData function wraps FFI exceptions in XMTPException with context (Group.kt:636-638)

6. Source compatibility — The unstableChangeCallbacks parameter defaults to null, so existing callers don't need changes (Client.kt:67)

⚠️ Observations:

1. Naming clarity — UnstableChangeCallbacks.toFfi() returns null when nothing is registered, but this behavior isn't documented in a KDoc comment. Consider adding a comment explaining why null is returned (to avoid callback overhead when not needed) (UnstableChangeCallbacks.kt:53-56)

2. Opt-in annotation inconsistency — The PR doesn't add an @UnstableApi annotation to UnstableChangeCallbacks, even though the name and PR description clearly mark it as unstable. Consider adding this annotation for consistency with Group.unstable (Group.kt:648-652)

---

Potential Bugs & Edge Cases

⚠️ Race condition consideration:

The expectedAppData guard helps prevent lost updates, but there's a subtle race condition scenario:

// Thread A reads appData = "v1"
val current = group.appData()  // "v1"

// Thread B updates to "v2" and commits
group.updateAppData("v2")

// Thread A tries to update with guard, expecting "v1"
group.updateAppData("new-value", expectedAppData = current)  // Will fail correctly

This is correct behavior — the guard works as intended. However, the documentation in Group.kt:618-627 could be clearer about retry semantics. When the guard fails, should callers:

• Re-read, re-merge, and retry?
• Surface the error to the user?
• Use a different conflict resolution strategy?

Recommendation: Add a KDoc example showing the retry pattern, similar to this:

suspend fun mergeAppData(group: Group, newData: String) {
    var attempts = 0
    while (attempts < MAX_RETRIES) {
        val current = group.appData()
        val merged = merge(current, newData)
        try {
            group.updateAppData(merged, expectedAppData = current)
            return  // Success
        } catch (e: XMTPException) {
            if (e.message?.contains("Permission denied") == true) throw e
            attempts++
            // Guard failed, retry the merge
        }
    }
    throw XMTPException("Failed to update appData after $MAX_RETRIES attempts")
}

---

Security Concerns

✅ No major security issues identified.

1. Callback isolation — The callback bridge correctly isolates FFI types from user code
2. No injection risks — String values are passed through safely
3. Permission enforcement — Permissions are checked at the FFI layer (Group.kt:637 wraps permission errors)

Minor observation: The groupId is exposed as hex-encoded in AppDataChange (UnstableChangeCallbacks.kt:69). This is consistent with the rest of the Android SDK and prevents raw byte array leakage, which is good.

---

Performance Considerations

✅ Generally efficient:

1. Lazy FFI conversion — toFfi() returns null when no callbacks are registered, avoiding callback overhead (UnstableChangeCallbacks.kt:53-56). This is excellent.

2. Minimal allocations — The bridge allocates only when a callback is registered

⚠️ Blocking consideration:

The FFI callback is awaited during message processing (per the Rust doc in change_callbacks.rs:5-6). If a handler implementation is slow (e.g., complex merge logic, database writes), it will block message processing for that group.

Recommendation: Document this in the KDoc for AppDataChangeHandler:

/**
 * Notified when a processed message changed a group's `appData`.
 *
 * [onAppDataChanged] is awaited before message processing continues, so a
 * semantic merge — including republishing via `Group.updateAppData` — can
 * finish first. It fires for changes this client made as well as remote ones,
 * so the merge must be idempotent.
 *
 * **Performance note:** This callback blocks message processing for the group
 * until it completes. Keep implementations fast or offload heavy work to a
 * background queue.
 */

---

Test Coverage

❌ No tests found for the new functionality:

1. No tests verify that unstableChangeCallbacks are invoked when appData changes
2. No tests verify the expectedAppData guard prevents concurrent updates
3. No tests verify that passing null for callbacks works correctly
4. No tests verify the callback fires for both local and remote changes

Recommendation: Add tests to GroupTest.kt covering:

@Test
fun testAppDataChangeCallback() = runBlocking {
    val changes = mutableListOf<AppDataChange>()
    val handler = object : AppDataChangeHandler {
        override suspend fun onAppDataChanged(change: AppDataChange) {
            changes.add(change)
        }
    }
    
    val client = Client.create(
        account = fixtures.aliceAccount,
        options = fixtures.clientOptions.copy(
            unstableChangeCallbacks = UnstableChangeCallbacks(appData = handler)
        )
    )
    
    val group = client.conversations.newGroup(listOf(fixtures.bob.walletAddress))
    group.updateAppData("test-data")
    group.sync()
    
    assertEquals(1, changes.size)
    assertEquals(group.id, changes[0].groupId)
    assertNull(changes[0].oldValue)
    assertEquals("test-data", changes[0].newValue)
}

@Test
fun testAppDataUpdateGuard() = runBlocking {
    val group = fixtures.aliceClient.conversations.newGroup(listOf(fixtures.bob.walletAddress))
    
    group.updateAppData("v1")
    group.sync()
    
    // This should succeed
    group.updateAppData("v2", expectedAppData = "v1")
    group.sync()
    
    // This should fail (expected value doesn't match)
    assertFailsWith<XMTPException> {
        group.updateAppData("v3", expectedAppData = "v1")
    }
}

---

Final Verdict

Approve with recommendations:

The code is well-structured, safe, and follows Android SDK conventions. The main gaps are:

1. Missing test coverage — Add tests before merging
2. Documentation improvements — Add retry pattern examples and performance notes
3. Consider adding @UnstableApi annotation for consistency

---

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new feature (app data change callbacks) with new interfaces, callback mechanisms, and compare-and-swap semantics. While additive and backwards-compatible, new user-facing capabilities and FFI integrations warrant human review.

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

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