Skip to content

Conversation

anoadragon453
Copy link
Member

@anoadragon453 anoadragon453 commented Aug 14, 2025

Two things are happening in this PR:

  1. Room creators of v12 rooms were being set to PL0 upon downgrading their room from v12 -> anything lower (Tombstoning a V12 room to any room version < 12 can lead to loss of powerlevel #18810)

    • This was due to there being two places where we set the power levels in upgraded rooms. The second attempt to send a power levels event was not MSC4289-aware - it would drop the request back down to PL0 when copying the old room's power levels a second time. (Users would see the user who upgraded the room being set to Admin, and then Default).
    • The second attempt has been removed entirely. It didn't appear to do anything useful; we already copied over state from the old room's power levels once before. So simply removing it fixes both the bug, and saves a state event.
  2. additional_creators listed in the old v12 room were not included in the power levels of the upgraded room, effectively dropping them to PL0 on upgrade. I noticed this while writing Complement tests for this PR.

    • Additional logic was added to cover setting the power levels for the additional_creators in the upgraded room.

Associated Complement tests: matrix-org/complement#794

Note that MSC4289 does not actually state what a user's power level should be when downgrading a room. I've taken the liberty of making this "the minimum power level needed to send m.room.power_levels events. But note that this is currently not covered by the spec. It really shouldn't be 0 though, as that makes the room unmoderatable.

Fixes #18810

Recommended to review commit-by-commit.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

The second attempt to send a power levels event was not MSC4289-aware - it would drop the request back down to PL0 when copying the old room's power levels a second time.

The second attempt to send power levels didn't appear to do anything useful anyways. So simply removing it fixes both the bug, and saves a state event.

Fixes #18810
@anoadragon453 anoadragon453 marked this pull request as ready for review August 14, 2025 15:28
@anoadragon453 anoadragon453 requested a review from a team as a code owner August 14, 2025 15:28
@ll-SKY-ll
Copy link
Contributor

ll-SKY-ll commented Aug 14, 2025

If you take the minimum PL needed to send powerlevel events, wouldnt that create a case where if i have a room like this:

  • @alice:matrix.org has PL 3000
  • @Sky:matrix.org is creator
  • PL to send m.room.power_levels = 200

and then "upgrade" it to < 12, i end up with Alice havin PL 3000 while I, the former "creator" end up with PL 200, which seems wrong to me overall

wouldnt it be safer to just use the max powerlevel here, since that best reflects the previous powerlevel structure from the V12 room?

@FSG-Cat
Copy link
Contributor

FSG-Cat commented Aug 14, 2025

Due to that creators have infinite power in v12 it would be safest to set them to PL 2^53-1 and if there happens to be a user at that PL put them at 2^53-2 and increment all the events at that PL down to the same value. That way you effectively get as close as is realistic to the behavior of the v12 room.

Since v12 rooms always have access to the full powerlevel spectrum this assumption is essentially the only way to make this fail safe and not eligible for screwing up somehow.

@anoadragon453
Copy link
Member Author

@ll-SKY-ll @FSG-Cat you're both spot on, see matrix-org/complement#794 (comment).

@anoadragon453
Copy link
Member Author

Setting to draft status while the exact power level is worked out.

@anoadragon453 anoadragon453 marked this pull request as draft August 14, 2025 17:16
@anoadragon453
Copy link
Member Author

portdb failures are unrelated to this PR.

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.

Tombstoning a V12 room to any room version < 12 can lead to loss of powerlevel
3 participants