feat: Handle Calling events and forward to apps #WPB-28341

[spoonman01]

---

PR Submission Checklist for internal contributors

• The PR Title

  • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
  • contains a reference JIRA issue number like SQPIT-764
  • answers the question: If merged, this PR will: ... ³

• The PR Description

  • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

---

What's new in this PR?

Issues

Briefly describe the issue you have solved or implemented with this pull request. If the PR contains multiple issues, use a bullet list.

Causes (Optional)

Briefly describe the causes behind the issues. This could be helpful to understand the adopted solutions behind some nasty bugs or complex issues.

Solutions

Briefly describe the solutions you have implemented for the issues explained above.

Dependencies (Optional)

If there are some other pull requests related to this one (e.g. new releases of frameworks), specify them here.

Needs releases with:

• GitHub link to other pull request

Testing

Test Coverage (Optional)

• I have added automated test to this contribution

How to Test

Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.

Notes (Optional)

Specify here any other facts that you think are important for this issue.

Attachments (Optional)

Attachments like images, videos, etc. (drag and drop in the text box)

---

PR Post Submission Checklist for internal contributors (Optional)

• Wire's Github Workflow has automatically linked the PR to a JIRA issue

---

PR Post Merge Checklist for internal contributors

• If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

---

References

1. https://sparkbox.com/foundry/semantic_commit_messages
2. https://github.com/wireapp/.github#usage
3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

[claude]

Review: Calling events & conference subconversation support (1/2)

Solid work overall. The design constraints (SDK never creates conferences, never commits pending proposals, never auto-rejoins, snapshots owned by the app) are applied consistently and documented well in docs/calling.md / AGENTS.md. SubconversationServiceTest covers the interesting state transitions, and MlsCryptoClientTest.conferenceKeysFollowMembershipAndEpochs (two real CoreCrypto clients, external commit, removal -> isActive == false) is exactly the right kind of test here.

Worth calling out a real bug fix hidden in this PR: WireApplicationManager.sendMessageSuspending previously swallowed a stale-epoch ClientError and never resent, and never rethrew other errors. Lines 212-222 now re-read the group ID, re-encrypt, resend, and rethrow otherwise. CallingMessageSendingTest covers all three paths.

---

CORRECTNESS

1. getSharedSecret() silently returns 32 zero bytes after close() — model/calling/SubconversationEpochInfo.kt:43

close() zeroes the buffer in place, so use-after-close returns a valid-looking all-zero key rather than failing. The tests currently assert this as expected, but consider the realistic integration: WireEventsHandlerDefault.onSubconversationEpochChanged closes the snapshot by default, and an AVS adapter that hands the snapshot to another thread or a queue (a very natural shape) reads the secret after the callback returns. Failure mode is media keyed from a fully predictable all-zero secret, with no exception anywhere.

Suggest a closed flag plus check(!closed) in getSharedSecret(). That turns a silent crypto failure into an obvious bug, and matches the documented contract.

2. forgetParent(...) runs before the primary state update and can abort it — EventsRouter.kt:168, :208, :279

forgetParent -> forget -> crypto.wipeConversation, and operation {} wraps CoreCryptoException into WireException. The channel consumer catches and logs it (EventsRouter.kt:420), so nothing crashes, but processDeletedConversation / deleteMembers / resetMlsConversation are then silently skipped, leaving stale local conversation state. Either move forgetParent after the primary update, or make it non-throwing (catch + log inside the service) since conference cleanup is best-effort here.

3. One unparseable MLS identity drops all remaining buffered messages — EventsRouter.kt:461

parseMlsClientIdentity() uses require(...) + UUID.fromString(...), both throwing. Inside the new forEach over listOf(message) + message.bufferedMessages, a single malformed identity aborts the loop and discards every subsequent buffered message. Before this PR only one message was processed, so this is new exposure. A per-message try/catch that logs and continues would be safer.

Same shape in MlsCryptoClient.getConferenceEpochInfo:307-309: one member whose credential does not match uuid:client@domain makes the whole snapshot throw, failing the join or losing an epoch callback. Since getCredentialType can return X509 when E2EI is enabled — is the identity format guaranteed identical there? If not, skip-with-warning beats failing the snapshot.

4. leave() does not emit onSubconversationLeft when only remote membership exists — SubconversationService.kt:117-125

After a restart with the local CoreCrypto group gone, restore() returns null (!crypto.conversationExists), so conference == null; the backend leave is still issued but forget — and therefore the callback — never runs. docs/calling.md says the callback fires "after local departure, removal, or parent invalidation", so this is a docs/behaviour mismatch. Also untested.

[claude]

Review: Calling events & conference subconversation support (2/2)

PERFORMANCE

5. Every conference event for a call the app has not joined costs a backend round trip — SubconversationService.kt:134 -> restore() at :186

decrypt does conferences[id] ?: restore(id), and restore unconditionally calls api.getConference(id). There is no negative caching, so for any conversation with an ongoing call the app is not part of, every conference MLS event triggers a fresh GET /subconversations/conference that ends in return null.

This matters more than it looks because decrypt is awaited from inside the per-conversation event channel (EventsRouter.kt:445, same channel key as text messages — extractChannelKey at :432). All other events in that conversation queue behind repeated network round trips, so a busy conversation with a long call the bot is not in will visibly lag on normal message delivery. A short-lived negative cache (or the existing Caffeine pattern) fixes both the amplification and the head-of-line blocking.

6. publishEpoch exports a fresh secret key on every message, then throws it away — SubconversationService.kt:242-250

snapshot() runs a full CoreCrypto transaction (epoch + getClientIds + exportSecretKey) and only then compares info.epoch == conference.lastEpoch and closes it. Reading crypto.conversationEpoch(groupId) first (cheap, already on the interface) and snapshotting only when the epoch actually advanced avoids a transaction and an unnecessary key export per decrypted message. Touching key material less often is also the safer default.

7. Redundant GET /subconversations/conference in leave() — SubconversationService.kt:119-122

restore(conversationId) already fetches the conference, then line 122 fetches it again immediately — two identical requests back to back (three on a cold cache). Fetch once and pass the response down.

---

RESOURCE MANAGEMENT

8. locks and callbacks grow without bound — SubconversationService.kt:77-78

Both are plain ConcurrentHashMaps keyed by conversation, only cleared in close(). lock() is hit by forgetParent, which EventsRouter calls on every DeleteConversation / self-MemberLeave / MlsReset event, so a Mutex accumulates per conversation even when no call ever happened there. dispatch() additionally retains a Channel plus a live consumer coroutine per conversation, forever.

EventsRouter already solved exactly this with Caffeine.newBuilder().maximumSize(...).expireAfterAccess(...) and a removal listener (EventsRouter.kt:107-113). Reusing that here would keep the two services consistent and bound the growth — though the removal listener would need cancel() rather than close() so onUndeliveredElement still discards queued secrets.

Minor, same method: dispatch() launches a coroutine inside ConcurrentHashMap.computeIfAbsent, i.e. under a bin lock. Safe here (no re-entrancy into the map), but the kind of thing that bites later.

---

API DESIGN

9. WireMessage.Calling.create fabricates a sender — model/WireMessage.kt:696

sender = QualifiedId(UUID(0, 0), "") is a public, readable field on the returned object. An app that logs or inspects message.sender on an outgoing message gets a nil UUID and empty domain with no indication it is a placeholder. If the sealed-interface contract forces sender to be non-null, at least document the sentinel — the current KDoc ("The SDK supplies the authenticated sender") reads as though the field will be populated.

10. "conference" is hardcoded in two places — EventsRouter.kt:443 and CallingApiClient.kt:45

Worth a shared internal constant, since the whole feature is defined by that one subconversation name. Also processConferenceMessage returns silently for any other value; a logger.debug there would help when debugging against a backend that grows a new subconversation type.

[claude]

Smaller notes and test coverage

• EventsRouter.kt:463 — the sender-mismatch log lost its parameters (was "MLS message sender {} does not match event envelope sender {}", now a bare string). If that was for PII reasons, obfuscateId() / obfuscateClientId() already exist in utils/Extensions.kt and would keep the message debuggable.
• SubconversationEpochInfo.kt:34-38 — fully-qualified java.util.Collections.unmodifiableMap / unmodifiableList inline. java.util.* is explicitly allowed by CLAUDE.md, so an import (or Map.copyOf / List.copyOf) reads better. Same for the inline com.wire.crypto.MlsTransport, kotlin.test.assertContentEquals and io.mockk.verify FQNs in the new tests.
• Modules.kt:114-115 — } onClose with the lambda on the next line is inconsistent with every other registration in that file.
• SubconversationService.kt:140, :144 — if reconcile() itself throws (e.g. network failure) it replaces the original decryption exception, so the apps onCallingError` sees the wrong cause. Worth guarding.
• SubconversationService.kt:251 — publishEpoch advances conference.lastEpoch before the callback is enqueued; if trySend fails the snapshot is discarded but the epoch is marked delivered. Only reachable at shutdown, so low impact.
• LoggingConfiguration — adding credential for the /calls/config/v2 SFT token is the right instinct. Note replaceJsonStringProperty only masks string properties, so it is worth eyeballing what actually lands in the logs for a real config response.

Test coverage

Good overall. Gaps I noticed:

• No coverage of getCallingConfiguration() on WireApplicationManager (only the API client is tested).
• No test for leave() when remote membership exists but local crypto state is gone (finding 4).
• No test for concurrent join / decrypt on the same conversation — the per-conversation Mutex is central to the design and currently unexercised.
• No test that locks / callbacks stay bounded (would follow naturally from finding 8).
• CallingApiClientTest uses a mock client without the real error-mapping plugin, so WireException.ClientError translation for the calling endpoints is not exercised. Shared infra, so probably fine.

One process note: the PR description is still the unfilled template. Given the size of the surface this adds (new public API, new callbacks, new HTTP endpoints), filling in Issues / Solutions / How to Test would help reviewers and future archaeology.

Note on verification: I reviewed statically — Gradle was not runnable in this environment, so I did not execute the new tests or ktlintCheck / detekt.