Skip to content

Fix: a duplicate group invite demoted a member who had already joined - #2218

Merged
mpretty-cyro merged 1 commit into
session-foundation:devfrom
mpretty-cyro:fix/duplicate-invite-demotes-member
Sep 28, 2026
Merged

mpretty-cyro merged 1 commit into
session-foundation:devfrom
mpretty-cyro:fix/duplicate-invite-demotes-member

Conversation

@mpretty-cyro

@mpretty-cyro mpretty-cyro commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Issue #2215, reported with a reliable repro and against every Session Android version.

Receiving side

GroupManagerV2Impl.handleInvitation had no check for a group we are already in. It built a fresh ClosedGroupInfo and wrote it over the existing record, so a second invitation set invited back to true and reset joinedAtSecs to 0 — a joined member was demoted to "invited" and their join time was lost.

It now checks the same thing handlePromotion already checks — but conditionally, not literally. A literal group == null check would break re-invitation after a kick: handleKicked keeps the record with kicked = true, and an invitation that rebuilds it is the only way back in. The guard is therefore "already joined": present, and not invited, kicked or destroyed.

Why a bare guard would have been a regression

A second invitation still has to be answered. Our invite response is sent once, on approval, inside a runCatching whose failure is swallowed, and the admin marks us INVITE_ACCEPTED only on receiving it. So an admin whose copy was lost sees us as "Invite sent" indefinitely, and re-inviting is the only repair available to them — which means the reported demotion is what that repair did on our side.

Simply ignoring the duplicate would have removed the repair and left the admin stuck, with Resend visibly doing nothing. The already-joined path therefore sends the invite response and drops the invitation from our swarm — the parts of approval the admin depends on — without touching membership or joinedAtSecs. An admin member sends no response, because their membership is not established that way.

Admin side

inviteMembersInternal called setInvited() whatever state the member was in, and InviteContactsJob then wrote the send result over the top. Re-inviting someone who had accepted reset them in both places, so fixing only the first would have changed nothing observable.

Both now skip anyone already in the group: INVITE_ACCEPTED, or isAdminOrBeingPromoted(status) — the predicate BaseGroupMembersViewModel already uses for showAsAdmin/canPromote, which is admin || status in { PROMOTION_SENT, PROMOTION_ACCEPTED }. Naming the accepted statuses by hand would have missed a member whose promotion is still in flight, and the outcome of sending them another invitation says nothing about a membership they already have.

Note this half is defensive rather than load-bearing: promotions never reach InviteContactsJob (promoteMember sends and records its own results), canResendInvite is offered only for INVITE_SENT/INVITE_FAILED, and the invite picker excludes existing members. It closes the hole rather than a reachable regression.

Tests

GroupManagerV2ImplTest, synthetic group config only:

test reverting
a second invitation leaves a joined group untouched the guard
a second invitation is still answered, so the admin can stop showing us as invited the acknowledgement
an admin member answers a second invitation without sending a response the acknowledgement
a first invitation is written to the user's groups — control, passes either way

The control exists so the negative tests cannot pass by never reaching the code.

The admin side has no test: GroupMember's state setters are native, so no JVM unit test can reach them.

Known limits, not addressed here

  • The guard protects the device that handles the message. A linked device on an older build can still rebuild the demoted record and sync it back.
  • Answering a duplicate invitation deletes it from our swarm, but a record already synced by another device is not undone.

Rebased on dev d29053ab48. Unit suite: 315 pass, 0 fail.

@mpretty-cyro mpretty-cyro self-assigned this Sep 20, 2026
@mpretty-cyro
mpretty-cyro marked this pull request as ready for review September 20, 2026 20:39
@mpretty-cyro
mpretty-cyro force-pushed the fix/duplicate-invite-demotes-member branch from bfae257 to acb973d Compare September 28, 2026 03:34
Issue session-foundation#2215, reported with a reliable repro and against every Session Android version.

handleInvitation built a fresh ClosedGroupInfo and wrote it over the existing record,
so a second invitation for a group we were already in set invited back to true and
discarded joinedAtSecs. It now checks the same thing handlePromotion already checks,
and still rebuilds when we were kicked or the group was destroyed - a fresh invitation
is how we get back in.

A second invitation still has to be answered, though, and that is why this is not
simply a guard. Our invite response is sent once, on approval, with its failure
swallowed, and the admin marks us accepted only on receiving it - so an admin whose
copy was lost has no repair except re-inviting, and the reported demotion is what that
repair did on our side. The already-joined path now sends the response and drops the
invitation from our swarm without touching membership, so both halves hold.

Admin side of the same defect: inviteMembersInternal called setInvited() whatever
state the member was in, and InviteContactsJob then wrote the send result over the
top - so re-inviting someone who had accepted reset them twice, and fixing only the
first place would have changed nothing. Both now leave anyone already in the group
alone, using isAdminOrBeingPromoted so a promotion in flight counts as membership
rather than as an invitation waiting on an answer.

Tests cover the receiving side. The admin side has none: GroupMember's state setters
are native, so no JVM unit test can reach them.
@mpretty-cyro
mpretty-cyro force-pushed the fix/duplicate-invite-demotes-member branch from acb973d to bc6698e Compare September 28, 2026 03:59
@mpretty-cyro
mpretty-cyro merged commit 7fe6d1c into session-foundation:dev Sep 28, 2026
5 checks passed
@mpretty-cyro
mpretty-cyro deleted the fix/duplicate-invite-demotes-member branch September 28, 2026 04:59
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.

1 participant