Skip to content

fix(job): guard group conversation in MixinJob.createConversation#6416

Open
SeniorZhai wants to merge 2 commits into
masterfrom
fix/skip-group-in-check-conversation-exist
Open

fix(job): guard group conversation in MixinJob.createConversation#6416
SeniorZhai wants to merge 2 commits into
masterfrom
fix/skip-group-in-check-conversation-exist

Conversation

@SeniorZhai
Copy link
Copy Markdown
Member

Summary

MixinJob.createConversation builds a minimal ConversationRequest with only conversationId, category, and a single-participant list — no random_id, name, or full participants. That request shape is only valid for CONTACT (1v1) conversations, whose conversationId is deterministically derived from the two userIds.

When a GROUP conversation falls into this path — SendMessageJob.sendPlainMessage/sendEncryptedMessage/sendSignalMessage calls checkConversationExist(conversation) for any non-SUCCESS local conversation regardless of category — the resulting POST /conversations hits the server with category=GROUP but no random_id. The server rejects it as either "random_id missing" or "invalid UUID", which is what we've been seeing in error reports.

Note that MixinJob.checkConversation (the other entry) already short-circuits groups to jobSenderKey.syncConversation, so the bug only fires from the three SendMessageJob.send* paths.

This PR adds an explicit guard inside createConversation itself: if the conversation is a group, report the unexpected state (so the issue surfaces if any future caller slips through) and return conversation.expireIn without hitting the API. Group creation is left to the proper flows that already supply a valid randomId:

  • GroupViewModel.createGroupConversation (new group from NewGroupFragment)
  • ConversationListViewModel.createGroupConversation (retry from ConversationListFragment on FAILURE state)

Audit of all group-create paths

Verified every code path that can POST /conversations:

Entry Source of request randomId
ConversationJob.createGroupconversationApi.create GroupViewModel.createGroupConversation UUID.randomUUID().toString()
ConversationListViewModel.createGroupConversation UUID.randomUUID().toString()
ConversationViewModel.createConversationcreateSuspend hardcoded CONTACT category n/a ✓
MixinJob.createConversationconversationApi.create SendMessageJob.checkConversationExist guarded by this PR

Other ConversationRequest constructions (in SearchViewModel, BottomSheetViewModel, ContactViewModel, etc.) are all MUTE/UPDATE requests that hit /conversations/{id}/mute or /conversations/{id} and do not require random_id.

Test plan

  • Send a message in a group whose local status is not SUCCESS — no POST /conversations request is fired and no "invalid UUID" error in logs
  • New group creation via NewGroupFragment still works end-to-end
  • Retry a FAILURE group from the conversation list — ConversationListViewModel.createGroupConversation still drives recreation with a fresh randomId
  • Contact (1v1) auto-create on first message still works (path unchanged)
  • Crash reporting captures the new reportException if any unexpected caller hits the guard

🤖 Generated with Claude Code

…nExist

The createConversation path in MixinJob builds a minimal ConversationRequest
without random_id, name, announcement, or full participants. When a GROUP
conversation falls into SendMessageJob's checkConversationExist with a
non-SUCCESS status, it triggers /conversations create with category=GROUP
but no random_id, which the server rejects (missing or invalid UUID).

Short-circuit groups here so creation stays on the proper flow via
GroupViewModel / ConversationListViewModel.createGroupConversation, which
always supplies a valid randomId.
createConversation builds a minimal ConversationRequest without random_id,
name, or full participants, which is only valid for CONTACT (1v1) where
the conversationId is deterministically derived from two userIds. If a
GROUP slips into this path (e.g. SendMessageJob.checkConversationExist on
a non-SUCCESS local group), the /conversations POST hits the server with
category=GROUP but no random_id, yielding 'invalid UUID' or missing-param
errors and a broken group on the server.

Bail out early for groups and report the unexpected state so the proper
GroupViewModel / ConversationListViewModel flow remains the only path
that creates groups.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Guards MixinJob.createConversation against being called for GROUP conversations to prevent sending an invalid minimal POST /conversations request (missing random_id) that the server rejects, which was causing “invalid UUID”/missing-param errors when sending messages in non-SUCCESS local groups.

Changes:

  • Add an early return in MixinJob.createConversation for group conversations.
  • Report the unexpected call path via reportException instead of attempting the API create.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +185 to +190
if (conversation.isGroupConversation()) {
reportException(
"Skip MixinJob.createConversation for group",
IllegalStateException("conversation_id=${conversation.conversationId}"),
)
return conversation.expireIn
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