Skip to content

Conversation

@zhouribin
Copy link
Contributor

📝 Description

This PR fixes a bug where members who quit and rejoin a group cannot recall their new messages or view new history correctly.

🐛 Issue

When a member quits a group, their userMaxSeq is capped at the current group maxSeq to restrict access to future messages. However, when the user rejoins, this userMaxSeq limit was not being cleared.

As a result, new messages (which have a higher seq) are filtered out by the visibility logic, causing "RecordNotFoundError" errors on the server side and preventing operations like message recall.

💡 Solution

I implemented a mechanism to reset the conversation visibility for a user immediately after they successfully rejoin a group.

Specific changes:

  1. Introduced a helper function setMemberJoinSeq in internal/rpc/group/group.go.
  2. This function performs the following:
    • Sets the user's minSeq to currentGroupMaxSeq + 1 (ensuring they don't see messages from before they rejoined).
    • Resets the user's userMaxSeq to 0 (removing the read limit so they can receive new messages).
  3. Applied this reset logic to all group join paths:
    • Direct Join (JoinGroup)
    • Invitation (InviteUserToGroup)
    • Admin Approval (GroupApplicationResponse)

🔍 Verification

  • Verified that rejoining members can now successfully pull new messages.
  • Confirmed that message recall works as expected for messages sent after rejoining.
  • Checked that userMinSeq is correctly updated and userMaxSeq is reset to 0 in the conversation settings.

@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 17, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2025

🤖 All Contributors have signed the CLA.
The signed information is recorded here
Posted by the CLA Assistant Lite bot.

@zhouribin
Copy link
Contributor Author

💕 Thank you for your contribution and please kindly read and sign our CLA. CLA Docs

I have read the CLA Document and I hereby sign the CLA

You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

I have read the CLA Document and I hereby sign the CLA

@zhouribin
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

OpenIM-Robot added a commit to OpenIM-Robot/cla that referenced this pull request Dec 17, 2025
@zhouribin
Copy link
Contributor Author

zhouribin commented Dec 22, 2025

  • Thanks for the follow-up review. Here is the updated clarification based on the latest change:

    • Change: In setMemberJoinSeq , replace 0 with math.MaxInt64 to set user MaxSeq to an “unlimited” value ( internal/rpc/group/group.go:972–975 ).
    • Why:
      • maxSeq=0 triggers Conversation service validation ( 1001 ArgsError ), which breaks invites.
      • Any small positive userMaxSeq shrinks the query upper bound below conversation maxSeq , which breaks revoke after join.
      • math.MaxInt64 preserves the “no upper limit” semantics and avoids both issues.
    • Safety:
      • History visibility remains controlled by the notification flow: when EnableHistoryForNewMembers=false , we set minSeq = maxSeq + 1 ( internal/rpc/group/notification.go:583–592 ).
      • Quit behavior is unchanged: user MaxSeq is solidified to the current conversation maxSeq ( internal/rpc/group/group.go:963–970 ).
    • Entry points: setMemberJoinSeq is invoked in all three paths for consistency:
      • GroupApplicationResponse ( internal/rpc/group/group.go:843–845 )
      • JoinGroup ( internal/rpc/group/group.go:908–910 )
      • InviteUserToGroup ( internal/rpc/group/group.go:459–461 )
    • Client consistency: using conversation.SetConversationMaxSeq ( pkg/rpcli/conversation.go:17–23 ) to keep conversation and seq_user in sync.
    • Commit: 36915fc on branch fix/group-member-rejoin-bug .

    @FGadvancer

@FGadvancer
Copy link
Collaborator

Thank you very much for submitting this PR.
Could you please also submit the same PR to the 3.8.3-patch branch?
This would allow us to release it to release as soon as possible.

@FGadvancer
Copy link
Collaborator

One more note regarding the condition maxSeq <= 0:
we may want to consider removing the = in the future and using a stricter comparison instead.
This would likely require a protocol-level change.

For now, keeping the value unified as 0 could help make the judgment logic more consistent across modules going forward.

@zhouribin
Copy link
Contributor Author

Thank you very much for submitting this PR. Could you please also submit the same PR to the 3.8.3-patch branch? This would allow us to release it to release as soon as possible.

Hi @FGadvancer

@withchao withchao enabled auto-merge December 23, 2025 06:15
@withchao withchao added this pull request to the merge queue Dec 23, 2025
Merged via the queue into openimsdk:main with commit 6f33c0a Dec 23, 2025
7 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 23, 2025
@zhouribin zhouribin deleted the fix/group-member-rejoin-bug branch December 24, 2025 07:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants