Skip to content

Document and fix room_config param when user_may_create_room callback is invoked for a room upgrade #18721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 23 commits into from
Sep 24, 2025
Merged

Document and fix room_config param when user_may_create_room callback is invoked for a room upgrade #18721

merged 23 commits into from
Sep 24, 2025

Conversation

@hughns
Copy link
Member

@hughns hughns commented Jul 23, 2025 •
edited
Loading

Fixes #18792.

The way that the initial_state was being constructed was completely wrong.<

It should now match that described at https://spec.matrix.org/v1.15/client-server-api/#post_matrixclientv3createroom_request_stateevent

I've also made the documentation much more explicit.

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)

@hughns hughns marked this pull request as ready for review July 23, 2025 14:42
@hughns hughns requested a review from a team as a code owner July 23, 2025 14:42
MadLittleMods
MadLittleMods reviewed Jul 23, 2025
View reviewed changes
@MadLittleMods MadLittleMods mentioned this pull request Jul 23, 2025
Merged
3 tasks
MadLittleMods
MadLittleMods approved these changes Jul 25, 2025
View reviewed changes
@github-actions github-actions bot deployed to PR Documentation Preview July 28, 2025 14:24 Active
@hughns
Copy link
Member Author

hughns commented Jul 28, 2025

Having given this some more consideration, I don't think it makes sense to try and pass the room_config to user_may_create_room in the case of the a room upgrade.

If anything it would make sense to introduce something like user_may_upgrade_room alongside user_may_create_room to make the upgrade request explicit, and could pass a different set of params that perhaps relate to the existing room state rather than a pseudo room config.

It wasn't documented that the user_may_create_room callback was called in the case of a room upgrade, so I have fixed that.

Then, what I propose doing is simply setting the room_config param to None in the case of a room upgrade.

This cleans up the module API without attempting to make some overly complicated mapping. If the is a need to hook into upgrades then that could be added as a new callback.

I kept the changelog as bugfix, but perhaps it might be a feature.

@hughns hughns requested a review from MadLittleMods July 28, 2025 14:32
@github-actions github-actions bot deployed to PR Documentation Preview July 28, 2025 14:38 Active
@hughns hughns changed the title Fix structure of initial_state when calling user_may_create_room module API callback Document and fix room_config param when user_may_create_room callback is invoked for a room upgrade Jul 28, 2025
@hughns
Copy link
Member Author

hughns commented Jul 28, 2025

What I've done in this last commit is revert the location of the upgrade logic to be how it was pre #18486

@github-actions github-actions bot deployed to PR Documentation Preview July 28, 2025 15:04 Active
MadLittleMods
MadLittleMods reviewed Jul 28, 2025
View reviewed changes
@hughns hughns enabled auto-merge (squash) July 28, 2025 16:45
@hughns hughns closed this Jul 28, 2025
auto-merge was automatically disabled July 28, 2025 16:48

Pull request was closed

@hughns hughns reopened this Jul 28, 2025
@github-actions github-actions bot deployed to PR Documentation Preview July 28, 2025 17:12 Active
@hughns
Copy link
Member Author

hughns commented Jul 28, 2025

I didn’t mean to close this. But I am now confused as to which iteration @MadLittleMods has approved for merging.

MadLittleMods added a commit that referenced this pull request Aug 4, 2025
@MadLittleMods
Improve order of validation and ratelimiting in room creation (#18723)
ddbcd85 
Spawning from looking at this stuff while reviewing
#18721
@hughns hughns marked this pull request as draft August 8, 2025 12:40
@github-actions github-actions bot deployed to PR Documentation Preview August 29, 2025 13:31 Active
@hughns hughns marked this pull request as ready for review August 29, 2025 13:39
@github-actions github-actions bot deployed to PR Documentation Preview September 12, 2025 16:31 Active
MadLittleMods
MadLittleMods reviewed Sep 12, 2025
View reviewed changes
hughns and others added 3 commits September 17, 2025 20:38
@hughns
Another attempt at line spacing in tests
a165f99 
Try again to follow @MadLittleMods's instructions:

> expected_room_config = { and self.create_room should be next to each other with a newline after that grouping.

If this is still wrong then please point to an example of it correctly (I see inconsitencies when I look at other tests in the codebase)
@hughns @MadLittleMods
Update docs/modules/spam_checker_callbacks.md
dcc5ce3 
Co-authored-by: Eric Eastwood <[email protected]>
@hughns
Linewrap
91becca
@github-actions github-actions bot deployed to PR Documentation Preview September 17, 2025 19:41 Active
@github-actions github-actions bot deployed to PR Documentation Preview September 17, 2025 19:46 Active
@hughns
Copy link
Member Author

hughns commented Sep 17, 2025

This updated implementation is not perfect, but it is at least has more accurate documentation and the implementation matches the documentation.

I am not going to add any more fields to the (now documented) synthesised subset of the createRoom object that is passed when a room upgrade call is made.

MadLittleMods
MadLittleMods approved these changes Sep 17, 2025
View reviewed changes
@anoadragon453 anoadragon453 removed their request for review September 18, 2025 13:21
@github-actions github-actions bot deployed to PR Documentation Preview September 24, 2025 17:36 Active
@github-actions github-actions bot deployed to PR Documentation Preview September 24, 2025 17:43 Active
@hughns hughns enabled auto-merge (squash) September 24, 2025 17:45
@github-actions github-actions bot deployed to PR Documentation Preview September 24, 2025 17:45 Active
@hughns hughns merged commit fd8fa97 into develop Sep 24, 2025
77 of 80 checks passed
@hughns hughns deleted the hughns/user-may-create-room-initial-state-format branch September 24, 2025 21:42
Michael-Ixo pushed a commit to ixoworld/synapse that referenced this pull request Oct 23, 2025
@MichaelPretorius
Merge tag 'v1.140.0' into ixo
748b7bc 
Deployments that make use of the
[synapse-s3-storage-provider](https://github.com/matrix-org/synapse-s3-storage-provider)
module must upgrade to
[v1.6.0](https://github.com/matrix-org/synapse-s3-storage-provider/releases/tag/v1.6.0).
Using older versions of the module with this release of Synapse will prevent
users from being able to upload or download media.

No significant changes since 1.140.0rc1.

- Add [a new Media Query by ID Admin API](https://element-hq.github.io/synapse/v1.140/admin_api/media_admin_api.html#query-a-piece-of-media-by-id) that allows server admins to query and investigate the metadata of local or cached remote media via
  the `origin/media_id` identifier found in a [Matrix Content URI](https://spec.matrix.org/v1.14/client-server-api/#matrix-content-mxc-uris). ([\element-hq#18911](element-hq#18911))
- Add [a new Fetch Event Admin API](https://element-hq.github.io/synapse/v1.140/admin_api/fetch_event.html) to fetch an event by ID. ([\element-hq#18963](element-hq#18963))
- Update [MSC4284: Policy Servers](matrix-org/matrix-spec-proposals#4284) implementation to support signatures when available. ([\element-hq#18934](element-hq#18934))
- Add experimental implementation of the `GET /_matrix/client/v1/rtc/transports` endpoint for the latest draft of [MSC4143: MatrixRTC](matrix-org/matrix-spec-proposals#4143). ([\element-hq#18967](element-hq#18967))
- Expose a `defer_to_threadpool` function in the Synapse Module API that allows modules to run a function on a separate thread in a custom threadpool. ([\element-hq#19032](element-hq#19032))

- Fix room upgrade `room_config` argument and documentation for `user_may_create_room` spam-checker callback. ([\element-hq#18721](element-hq#18721))
- Compute a user's last seen timestamp from their devices' last seen timestamps instead of IPs, because the latter are automatically cleared according to `user_ips_max_age`. ([\element-hq#18948](element-hq#18948))
- Fix bug where ephemeral events were not filtered by room ID. Contributed by @frastefanini. ([\element-hq#19002](element-hq#19002))
- Update Synapse main process version string to include git info. ([\element-hq#19011](element-hq#19011))

- Explain how `Deferred` callbacks interact with logcontexts. ([\element-hq#18914](element-hq#18914))
- Fix documentation for `rc_room_creation` and `rc_reports` to clarify that a `per_user` rate limit is not supported. ([\element-hq#18998](element-hq#18998))

- Remove deprecated `LoggingContext.set_current_context`/`LoggingContext.current_context` methods which already have equivalent bare methods in `synapse.logging.context`. ([\element-hq#18989](element-hq#18989))
- Drop support for unstable field names from the long-accepted [MSC2732](matrix-org/matrix-spec-proposals#2732) (Olm fallback keys) proposal. ([\element-hq#18996](element-hq#18996))

- Cleanly shutdown `SynapseHomeServer` object, allowing artifacts of embedded small hosts to be properly garbage collected. ([\element-hq#18828](element-hq#18828))
- Update OEmbed providers to use 'X' instead of 'Twitter' in URL previews, following a rebrand. Contributed by @HammyHavoc. ([\element-hq#18767](element-hq#18767))
- Fix `server_name` in logging context for multiple Synapse instances in one process. ([\element-hq#18868](element-hq#18868))
- Wrap the Rust HTTP client with `make_deferred_yieldable` so it follows Synapse logcontext rules. ([\element-hq#18903](element-hq#18903))
- Fix the GitHub Actions workflow that moves issues labeled "X-Needs-Info" to the "Needs info" column on the team's internal triage board. ([\element-hq#18913](element-hq#18913))
- Disconnect background process work from request trace. ([\element-hq#18932](element-hq#18932))
- Reduce overall number of calls to `_get_e2e_cross_signing_signatures_for_devices` by increasing the batch size of devices the query is called with, reducing DB load. ([\element-hq#18939](element-hq#18939))
- Update error code used when an appservice tries to masquerade as an unknown device using [MSC4326](matrix-org/matrix-spec-proposals#4326). Contributed by @tulir @ Beeper. ([\element-hq#18947](element-hq#18947))
- Fix `no active span when trying to log` tracing error on startup (when OpenTracing is enabled). ([\element-hq#18959](element-hq#18959))
- Fix `run_coroutine_in_background(...)` incorrectly handling logcontext. ([\element-hq#18964](element-hq#18964))
- Add debug logs wherever we change current logcontext. ([\element-hq#18966](element-hq#18966))
- Update dockerfile metadata to fix broken link; point to documentation website. ([\element-hq#18971](element-hq#18971))
- Note that the code is additionally licensed under the [Element Commercial license](https://github.com/element-hq/synapse/blob/develop/LICENSE-COMMERCIAL) in SPDX expression field configs. ([\element-hq#18973](element-hq#18973))
- Fix logcontext handling in `timeout_deferred` tests. ([\element-hq#18974](element-hq#18974))
- Remove internal `ReplicationUploadKeysForUserRestServlet` as a follow-up to the work in element-hq#18581 that moved device changes off the main process. ([\element-hq#18988](element-hq#18988))
- Switch task scheduler from raw logcontext manipulation to using the dedicated logcontext utils. ([\element-hq#18990](element-hq#18990))
- Remove `MockClock()` in tests. ([\element-hq#18992](element-hq#18992))
- Switch back to our own custom `LogContextScopeManager` instead of OpenTracing's `ContextVarsScopeManager` which was causing problems when using the experimental `SYNAPSE_ASYNC_IO_REACTOR` option with tracing enabled. ([\element-hq#19007](element-hq#19007))
- Remove `version_string` argument from `HomeServer` since it's always the same. ([\element-hq#19012](element-hq#19012))
- Remove duplicate call to `hs.start_background_tasks()` introduced from a bad merge. ([\element-hq#19013](element-hq#19013))
- Split homeserver creation (`create_homeserver`) and setup (`setup`). ([\element-hq#19015](element-hq#19015))
- Swap near-end-of-life `macos-13` GitHub Actions runner for the `macos-15-intel` variant. ([\element-hq#19025](element-hq#19025))
- Introduce `RootConfig.validate_config()` which can be subclassed in `HomeServerConfig` to do cross-config class validation. ([\element-hq#19027](element-hq#19027))
- Allow any command of the `release.py` script to accept a `--gh-token` argument. ([\element-hq#19035](element-hq#19035))

* Bump Swatinem/rust-cache from 2.8.0 to 2.8.1. ([\element-hq#18949](element-hq#18949))
* Bump actions/cache from 4.2.4 to 4.3.0. ([\element-hq#18983](element-hq#18983))
* Bump anyhow from 1.0.99 to 1.0.100. ([\element-hq#18950](element-hq#18950))
* Bump authlib from 1.6.3 to 1.6.4. ([\element-hq#18957](element-hq#18957))
* Bump authlib from 1.6.4 to 1.6.5. ([\element-hq#19019](element-hq#19019))
* Bump bcrypt from 4.3.0 to 5.0.0. ([\element-hq#18984](element-hq#18984))
* Bump docker/login-action from 3.5.0 to 3.6.0. ([\element-hq#18978](element-hq#18978))
* Bump lxml from 6.0.0 to 6.0.2. ([\element-hq#18979](element-hq#18979))
* Bump phonenumbers from 9.0.13 to 9.0.14. ([\element-hq#18954](element-hq#18954))
* Bump phonenumbers from 9.0.14 to 9.0.15. ([\element-hq#18991](element-hq#18991))
* Bump prometheus-client from 0.22.1 to 0.23.1. ([\element-hq#19016](element-hq#19016))
* Bump pydantic from 2.11.9 to 2.11.10. ([\element-hq#19017](element-hq#19017))
* Bump pygithub from 2.7.0 to 2.8.1. ([\element-hq#18952](element-hq#18952))
* Bump regex from 1.11.2 to 1.11.3. ([\element-hq#18981](element-hq#18981))
* Bump serde from 1.0.224 to 1.0.226. ([\element-hq#18953](element-hq#18953))
* Bump serde from 1.0.226 to 1.0.228. ([\element-hq#18982](element-hq#18982))
* Bump setuptools-rust from 1.11.1 to 1.12.0. ([\element-hq#18980](element-hq#18980))
* Bump twine from 6.1.0 to 6.2.0. ([\element-hq#18985](element-hq#18985))
* Bump types-pyyaml from 6.0.12.20250809 to 6.0.12.20250915. ([\element-hq#19018](element-hq#19018))
* Bump types-requests from 2.32.4.20250809 to 2.32.4.20250913. ([\element-hq#18951](element-hq#18951))
* Bump typing-extensions from 4.14.1 to 4.15.0. ([\element-hq#18956](element-hq#18956))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MadLittleMods MadLittleMods MadLittleMods approved these changes

Assignees

No one assigned

Labels

A-Docs A-Modules

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

room_config param to user_may_create_room spam-checker callback has invalid data when called for room upgrades

3 participants

@hughns @MadLittleMods