Repository navigation
MSC4140: merge develop into the drop-state-interference branch - #3
Merged
Conversation
Signed-off-by: dependabot[bot] <support@github.com>
…19591) Follows: #19487 Part of: MSC4354 whose experimental feature tracking issue is #19409 This PR implements the Sliding Sync (MSC4186) extension described in MSC4354, allowing sliding sync clients to receive sticky events in a reliable way. The logic is much the same as for oldschool sync (implementation in #19487), although in the sliding sync extension, the client can choose their own limit and must control their own pagination through an extra token in the extension request/response bodies. Note this does not yet send down existing sticky events in the room when the room has been newly-joined. This newly-discovered gap is tracked at #19662 and will be addressed for both current sync and MSC4186 SSS soon. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> Co-authored-by: Eric Eastwood <erice@element.io>
After looking into it, just a couple of things to pick a bone at in the old wording, which I thought could be clarified for when I next come to look at this again. - the claim that there's a fundamental difference; I'd argue there isn't really, it's just by convention on some mainstream distros. So I have changed this to 'typically' - statements that some distros fetch dependencies at build time (probably does happen, but traditional distros make a point of not doing this for the reasons you'd expect). - This was probably meant to be talking about Debian, but my observation based on sample size of 3 is that some crates are packaged natively, others are vendored in the respective application's source package (like they do for us) and sometime they patch the bounds a bit There could probably be room to talk about how distros vendoring packages is a maintenance burden on them, but I guess it's a bit moot as we would struggle to conform to wide enough bounds to make everyone happy (and anyway; I expect the distros that vendor packages have the tooling to make this easy to update and we do keep on top of security updates and release frequently...) --- Spawning from discussion in [`#element-backend-internal:matrix.org`](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$VttYPPUevn2S_W_rrzg2ZOXWI6aKebk2ganTgrLEWUc?via=jki.re&via=element.io&via=matrix.org) --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
…19863) Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
This reports the total count of users (split by appservice) which is meant to be the monthless counterpart to the MAU metric. Context: > So this is largely for billing purposes and wanting to know the change in the number of users. If a user is deactivated then we no longer want to count them. Consumers *might* want to count appservice users, and maybe count them based on the service (perhaps you change more for users under bridge X or bridge Y). > > *-- #19848 (comment)
…9892) `_synapse/mas/sync_devices` checks the device list against the set of devices MAS knows about, but the dehydrated device (MSC3814) is invisible to MAS, so it gets automatically deleted on each sync, which prevents dehydrated devices from working. This change excludes the dehydrated device from the check. There is similar special case code in the admin devices API (which gives it a special flag) and MAS's own legacy sync path (which filters it out). This code was initially written by @ara4n and Claude, but both he and I have read it and think it makes sense. I am far from a Synapse expert, so feel free to tell me it's all wrong and point me in the right direction. A system-level test for this bug is being written here: element-hq/element-web#34034 but if you think we should have one somewhere else, please let me know. Closes #19889 ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) Co-authored-by: Matthew <matthew@element.io>
…19896) Change `/org.matrix.msc3814.v1/dehydrated_device/[device_id]/events` to accept GET requests instead of POST. The original version of [MSC3814](matrix-org/matrix-spec-proposals#3814) said we should delete keys after returning them from this endpoint, but it is being updated to say we should not delete them, and therefore the appropriate verb is GET. Synapse already doesn't delete anything, so we just need to change to a GET with a `next_batch` query param. (Currently it is a POST with `next_batch` in the JSON content.) This code was initially written by @ara4n and Claude, but both he and I have read it and think it makes sense. I am far from a Synapse expert, so feel free to tell me it's all wrong and point me in the right direction. I don't know what system tests will be affected by this, but I guess we will see when the CI runs (right?). This is a change to an unstable endpoint so no need for notifications about breaking changes or similar. Part of element-hq/element-meta#2704 ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Matthew Hodgson <matthew@matrix.org>
… repeated deadlocks. (#19826) Got paged today for this. The sliding sync worker in question had loads of deadlocks in the logs. I restarted it and it got unwedged, but we should have a more robust defence, which this PR proposes. ``` psycopg2.errors.DeadlockDetected: deadlock detected DETAIL: Process 257324 waits for ShareLock on transaction 688227036; blocked by process 254908. Process 254908 waits for ShareLock on transaction 688222971; blocked by process 256179. Process 256179 waits for ExclusiveLock on tuple (302352,92) of relation 2962200779 of database 16403; blocked by process 257213. Process 257213 waits for ShareLock on transaction 688225005; blocked by process 254905. Process 254905 waits for ShareLock on transaction 688228814; blocked by process 257324. HINT: See server log for query details. CONTEXT: while inserting index tuple (183070,103) in relation "sliding_sync_connection_lazy_members" ``` I wonder if an unfortunate side effect is that these repeated attempts leave a lot of dead tuples on the table, which would then harm the performance of the next attempt to insert the tuples, I suspect making it more likely that they will deadlock again (?). --- By acquring a `FOR NO KEY UPDATE` lock upfront before beginning work, we can ensure that one of the transactions gets queued behind the other one, meaning the first one can succeed unimpeded. `FOR NO KEY UPDATE` blocks other `FOR NO KEY UPDATE` locks and is the weakest lock level that blocks itself. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
…19890) Introduced in: #17847 This 10-second wall-clock timeout was troublesome as it fails flakily on slow/struggling CI runners, like the default ones for private GitHub repositories. The loop also silently relied on the reactor advance in `make_request`, whereas we could just deterministically advance the reactor the known amount of times instead. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Signed-off-by: dependabot[bot] <support@github.com>
…te (#19901) The `flag_existing_quarantined_media` background update (added in #19558, shipped in v1.152.0) back-populates the `quarantined_media_changes` table with media that was already quarantined. It has two bugs. ### 1. Some quarantined remote media is silently skipped The remote-media query paged through `remote_media_cache` with: ```sql WHERE quarantined_by IS NOT NULL AND media_origin >= ? AND media_id > ? ``` This ANDs the two key columns independently rather than comparing them as a tuple. Once an origin has been fully processed (e.g. `media_id` reaches `zzz` for origin `a.example`), rows in a *later* origin whose `media_id` is `<=` the last processed `media_id` (e.g. `b.example` / `aaa`) fail the `media_id > ?` test and are never flagged. Fixed by using a proper row-value tuple comparison: ```sql WHERE quarantined_by IS NOT NULL AND (media_origin, media_id) > (?, ?) ``` Both the minimum supported SQLite (3.37.2) and PostgreSQL support row-value comparisons. ### 2. Exhausted queries keep re-running every iteration `flag_quarantined` ran *both* the local and remote queries on every iteration. When one table was exhausted but the other still had rows, the update kept returning a positive count, so the finished table's (now empty) query needlessly re-ran on every subsequent iteration until the whole update completed. This could add significant time to the transaction, as since the rows were deleted it could scan a significant portion of the table each time. Fixed by tracking per-table completion (`local_done` / `remote_done`) in the background-update progress and skipping a table's query once it has returned an empty batch. The progress dict was also restructured into an incremental build for readability.
Modern 'MAS integration' (as we call it now) is, of course, preserved. Fixes: #19549 --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
…nc Rust (Tokio runtime/thread pool) (#19871) This means you can use `get_success(...)` anywhere regardless of what kind of work needs to be done. Spawning from adding some more async Rust things in #19846 and wanting something more standard instead of the custom `till_deferred_has_result(...)` that has crept in to a few files. Alternative to #19867 spurred on by [this comment](#19867 (comment)) from @erikjohnston ### How does this work? Previously, `get_success(...)` just ran in a hot-loop advancing the Twisted reactor clock which didn't give any time for other threads to do some work or acquire the GIL if necessary (whenever there is a hand-off from Rust to Python, we need the GIL). Now, `get_success(...)` loops until we see a result (until we hit the ~0.1s real-time timeout). In the loop, we call [`time.sleep(0)`](https://docs.python.org/3/library/time.html#time.sleep) which will "Suspend execution of the calling thread [...]" (CPU and GIL) to allow other threads to do some work. Then like before, we advance the Twisted reactor clock to run any scheduled callbacks which includes anything the other threads may have scheduled. ### Does this slow down the entire test suite? Seems just as fast as before. There is minutes variance in what we had before and after but both are within the same range of each other. (see PR for actual before/after timings)
…ch spec conventions (#19897) To match [the spec requirements for pagination](https://spec.matrix.org/v1.9/appendices/#pagination): * rename the query parameter of `/org.matrix.msc3814.v1/dehydrated_device/[device_id]/events` to `from`, and * omit `next_batch` from the response content if there are no more events to fetch This matches [the changes that are coming in MSC3814](https://github.com/matrix-org/matrix-spec-proposals/pull/3814/files#r1540896531). The previous change #19896 kept the old POST API to support old clients. This preserves that compatibility, so the POST API still works how it used to. Depends on #19896 Part of element-hq/element-meta#2799 ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: Quentin Gliech <quenting@element.io>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Bumps [gitpython](https://github.com/gitpython-developers/GitPython) from 3.1.58 to 3.1.59. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/gitpython-developers/GitPython/releases">gitpython's releases</a>.</em></p> <blockquote> <h2>3.1.59 - Security</h2> <h2>What's Changed</h2> <ul> <li>prepare changelog for upcoming release by <a href="https://github.com/Byron"><code>@Byron</code></a> in <a href="https://redirect.github.com/gitpython-developers/GitPython/pull/2207">gitpython-developers/GitPython#2207</a></li> <li>Block file-reading Git options by <a href="https://github.com/Byron"><code>@Byron</code></a> in <a href="https://redirect.github.com/gitpython-developers/GitPython/pull/2208">gitpython-developers/GitPython#2208</a></li> <li>index: write blobs via git hash-object, not gitdb's odb.store by <a href="https://github.com/caroescm"><code>@caroescm</code></a> in <a href="https://redirect.github.com/gitpython-developers/GitPython/pull/2209">gitpython-developers/GitPython#2209</a></li> <li>Block separate git directories during clone by <a href="https://github.com/Byron"><code>@Byron</code></a> in <a href="https://redirect.github.com/gitpython-developers/GitPython/pull/2210">gitpython-developers/GitPython#2210</a></li> <li>fix: harden config parsing boundaries by <a href="https://github.com/Byron"><code>@Byron</code></a> in <a href="https://redirect.github.com/gitpython-developers/GitPython/pull/2211">gitpython-developers/GitPython#2211</a></li> <li><code>repo.index.add()</code> now respects worktree filters <a href="https://redirect.github.com/gitpython-developers/GitPython/pull/2209">gitpython-developers/GitPython#2209</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/gitpython-developers/GitPython/compare/3.1.58...3.1.59">https://github.com/gitpython-developers/GitPython/compare/3.1.58...3.1.59</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/gitpython-developers/GitPython/commit/66340d77aab9a7468f4aed3681d4ef1e3c0ec931"><code>66340d7</code></a> prepare changelog prior to release</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/a5e047d0db7047c4249c0de335585470b14d50c4"><code>a5e047d</code></a> Merge pull request <a href="https://redirect.github.com/gitpython-developers/GitPython/issues/2211">#2211</a> from gitpython-developers/config-sanitize-more</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/ef7568e3b317ce617eacda39b8b54dcdff8c3b5c"><code>ef7568e</code></a> fix: ignore includes in submodule configuration</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/4b4e47fc1224e23b0c8ee7220a7192818f2e4abb"><code>4b4e47f</code></a> fix: preserve multiline config values when writing</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/b473abb0f7de754392e1ec923f2fe296509013ab"><code>b473abb</code></a> Merge pull request <a href="https://redirect.github.com/gitpython-developers/GitPython/issues/2210">#2210</a> from gitpython-developers/fix-clone-unsafe-option</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/5ff52cccca770fd69c6caf0b8f281d3e45d599be"><code>5ff52cc</code></a> Merge pull request <a href="https://redirect.github.com/gitpython-developers/GitPython/issues/2209">#2209</a> from caroescm/fix-index-add-chmod</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/b68afff45af0f49e79a3e2d2162018986b37ad5d"><code>b68afff</code></a> Block separate git directories during clone</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/93677a00ab9dcb06cc08595fd1f88a4b4a0fa23b"><code>93677a0</code></a> fix: <code>index.add()</code> now supports filters (<a href="https://redirect.github.com/gitpython-developers/GitPython/issues/2021">#2021</a>)</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/9729ed3b948f2bde09f1f188c5311e172212b67e"><code>9729ed3</code></a> Merge pull request <a href="https://redirect.github.com/gitpython-developers/GitPython/issues/2208">#2208</a> from gitpython-developers/security-fixes</li> <li><a href="https://github.com/gitpython-developers/GitPython/commit/ce9d8e8d150e06ae2e2cc2efa229071cd3048a93"><code>ce9d8e8</code></a> prepare next release</li> <li>Additional commits viewable in <a href="https://github.com/gitpython-developers/GitPython/compare/3.1.58...3.1.59">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/element-hq/synapse/network/alerts). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
### Pull Request Checklist Replacement for #19752 as that has bit-rotted with #19895 being merged. `/_synapse/mas` is mounted on a worker on matrix.org and can be seen to be workerisable via * https://github.com/element-hq/synapse/blob/v1.160.0/synapse/app/generic_worker.py#L202 * https://github.com/element-hq/synapse/blob/v1.160.0/synapse/rest/synapse/client/__init__.py#L71 * https://github.com/element-hq/synapse/blob/v1.160.0/synapse/rest/synapse/mas/__init__.py#L46-L71 <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). 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. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
…le (#20210) ### Pull Request Checklist Missed off of https://github.com/element-hq/synapse/pull/19926/changes IMO. My reading of https://github.com/element-hq/synapse/blob/v1.161.0rc1/synapse/rest/client/delayed_events.py is that the new endpoint (pattern `r"/org\.matrix\.msc4140/delayed_events/(?P<delay_id>[^/]+)$"` is workerisable) given how `register_servlets` flows. However given `UpdateDelayedEventServlet` covers the same pattern but for `POST` requests, this should go in the `GET` only part of the documentation. <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). 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. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
Adds the serving functions needed for MSC4242: State DAGs. This PR adds MSC4242 support to /make_join, /send_join and /get_missing_events, as well as calculates the destinations for /send events correctly using `prev_state_events`. Built on top of #19718 for the storage functions it makes. Split out from #19425 Part of a series of 5x PRs to land the federation part of [MSC4242](matrix-org/matrix-spec-proposals#4242) ([storage](#19718), [fedclient](#20127), serving (this PR), inbound-joins, inbound-pulls). Whilst this is mostly a port of the code in #19425 there are a few changes: - `/get_missing_events` accepts message events when walking the state DAG, in which case it resolves the first hop to be that event's `prev_state_events`. The original PR made the client `/event` the message event and then set `latest=[prev_state_events]` on its own. This is not very efficient (extra round trip to fetch the event) and there's no reason why the server can't do the message->prev_state_events lookup, so we do so. This matches the MSC examples. - We cap the amount of events fetched via `/get_missing_events`. The MSC allows it, so it's a good safety check. - We sort the returned state DAG in `/send_join` by depth then event ID so it's "mostly" sorted. This is more a formality than anything else, the MSC does not mandate this, but it makes `/send_join` responses deterministic. - `notify_on_event_delivered_over_federation` is a new thing since #19425, so we include state DAG events in it like we do with state/auth_chain. This PR does remove the forced `m.federate: false` setting for MSC4242 rooms, so it makes it possible for federated MSC4242 rooms to be made. This is mostly so we can test via the endpoints. Given you must opt-in to MSC4242 via the experimental features config option, it seems reasonable to loosen this setting. The forced no-federation flag existed prior to review saying that the MSC4242 room version could itself be gated behind an experimental feature. Reviewable commit-by-commit. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). 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. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Eric Eastwood <erice@element.io>
Fixes: #20167 The `Schema Diff` workflow posts a PR comment showing the effective schema diff. For PRs from forks, `GITHUB_TOKEN` is downgraded to read-only, so the comment-posting step was silently failing. ### Changes - `schema_diff.yml`: only post the comment directly when the PR is from the same repository. For forked PRs, upload the diff (and PR number) as a short-lived artifact instead of trying to comment. - `schema_diff_comment.yml` (new): triggered by `workflow_run` after `Schema Diff` completes, with `pull-requests: write` permission (granted because this workflow always runs in the context of the base repository). It downloads the artifact, if present, and posts the comment on behalf of the forked PR. This avoids `pull_request_target`, per the security concerns raised in the issue (zizmor flags it as dangerous). The new workflow only ever treats the downloaded artifact as inert comment text -- it is never executed. --------- Co-authored-by: Olivier 'reivilibre <oliverw@element.io>
Part of: #19415 TLDR: return the `M_UNKNOWN_DEVICE` error code instead of the unstable `ORG.MATRIX.MSC4326.M_UNKNOWN_DEVICE` identifier. > **[Added in `v1.17`]** Application services MAY similarly masquerade as a specific device ID belonging the user ID through use of the `device_id` query string parameter on the request. If the given device ID is not known to belong to the user, the server will return a 400 `M_UNKNOWN_DEVICE` error. > > — [Matrix v1.19, Application Service API — Identity assertion](https://spec.matrix.org/v1.19/application-service-api/#identity-assertion) Synapse returns the correct 400, but with the unstable identifier. MSC4326 was stabilized in Matrix 1.17 and its experimental flag was already removed in #19033; only the error code identifier was left behind. Before: ``` GET /_matrix/client/v3/sync?user_id=@alice:test&device_id=NOT_A_REAL_DEVICE_ID # appservice token 400 {"errcode": "ORG.MATRIX.MSC4326.M_UNKNOWN_DEVICE"} ``` After: ``` GET /_matrix/client/v3/sync?user_id=@alice:test&device_id=NOT_A_REAL_DEVICE_ID # appservice token 400 {"errcode": "M_UNKNOWN_DEVICE"} ``` I verified that no implementation was currently handling the prefixed error code.
Part of: #19414 When [MSC4133](matrix-org/matrix-spec-proposals#4133) (custom profile fields) was implemented, the returned value for an unset display name changed from `200 {}` to `200 { displayname: null }`. This happened first on the unstable `uk.tcpip.msc4133` path in #17488 (1.123.0), then on the stable path when #18635 (1.135.0) unified the `displayname`, `avatar_url` and custom field servlets. Neither PR discussed the change in review, so it looks like an unintended side effect of the refactor rather than a deliberate decision. The v1.16 spec mandated to change from returning `200 {}` to `404` but change was not identified as breaking and was eventually not implemented in other clients and server. This PR has a sister MSC that proposes to return to the pre-1.16 error codes: [MSC4537](matrix-org/matrix-spec-proposals#4537). Before: ``` GET /_matrix/client/v3/profile/@alice:test/displayname 200 {"displayname": null} ``` After: ``` GET /_matrix/client/v3/profile/@alice:test/displayname 200 {} ``` ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). 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. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
… changes, making federation support more reliable. (#20204) Part of: MSC4354 Experimental feature tracking issue: #19409 Related Complement tests currently in https://github.com/matrix-org/complement/pull/806/files#diff-6c9d6d169485d0848c6b20dd9b43f6fe669a8a710e42f953d08fa25a99cc8f4cR509 --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
…omplement suite fails. (#20161) Supersedes: element-hq/synapse-private#155 It would be useful to have the in-repo Complement suite give a status, even when the normal suite fails (e.g. flakes). --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Andrew Morgan <andrew@amorgan.xyz> Signed-off-by: Skye Elliot <actuallyori@gmail.com>
…eparation and completion. (#20166) A key refactoring for, and split out of, #20165 Would be easier to land first to isolate the diff. Should be a standalone change with no behavioural change. Motivation is that #20165 will round-robin between 'main queue' transactions and 'sticky event' transactions. To keep the data flow clear, I wanted to insert a typed struct (well, `attrs` dataclass) as an interface between the 'preparation' of a transaction and its 'completion'. Doing this whilst keeping the asynchronous context manager style did not lead to a readable result in my opinion. (I would also say the async context manager is a touch 'magic' / obscures control flow, but I suspect this is largely down to opinion.) Replace _TransactionQueueManager with prepare/complete transaction methods --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
…room version "12" creation events (#19768)
…ith a `null` value. (#20145) Instead, treat them as absent fields as they feel like they should be. The database implementation detail that these fields have a dedicated column with `NULL` when unset is kept to the storage layer. The goal here is to reduce the amount of special casing needed for these two original profile fields and treat them a little bit more like regular profile fields. Follows: #20003 Follows: #20147 (needed as a bugfix to continue sending them down oldschool sync when they get deleted. Without #20147, this PR would break that — which matches how custom profile fields were broken too.) --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Adds a `redis.username` config option. Details: A `username` without a `password` (or `password_path`) is refused at startup. Redis has no wire form for a username without a password, and txredisapi only sends `AUTH` when a password is set, so the username would otherwise be silently ignored. An explicitly empty password is accepted, since that is how a `nopass` ACL user is configured. This relies on txredisapi 1.4.12, the first release to accept a `username` kwarg. That upstream support was contributed by @karolyi specifically to unblock this. Fixes #19238. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [X] Pull request is based on the develop branch * [X] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). 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. * [X] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…token falls inside a persist batch (#20171) This PR fixes the issue described as comment here: #18793 (comment) In Element Call, this shows up as ghost participants: someone who left the call keeps being displayed until a later state change refreshes the room. The bug is not specific to Element Call: any state event can be affected, RTC membership just changes often enough to make it visible. ## What happens Alice has a client syncing against a homeserver where events are persisted by one worker (the event persister) and `/sync` is served by another (the sync worker). Her client is parked in a long-poll: `GET /sync?since=s99&timeout=30000`. Bob joins a call at the same moment Carol sends a message. Carol's message reaches the persister first; Bob's `m.call.member` arrives while that write is still in flight, so the per-room persist queue groups them into one transaction: ``` events (each gets its own stream ordering): stream_ordering 100: m.room.message Carol stream_ordering 101: m.call.member Bob (state) current_state_delta_stream (how state_after finds state changes): stream_id 100 ────► (m.call.member, @bob) -> $bob_join_call ▲ └─ stamped with the batch MINIMUM (100), not the event's own 101 (see `_update_current_state_txn`) ``` The transaction commits: both events and the delta row are now in the database, atomically. The persister then announces the new events over replication, one RDATA token per stream ordering — rows are only merged into one token when they share a position, and 100 and 101 don't. So the sync worker's events-stream position steps 99 → 100 → 101, and on reaching 100 it pokes the notifier. Alice's long-poll wakes at exactly that moment. Her response is built at the worker's *current* position — `end = 100` — with RDATA 101 still in the queue: ``` Sync A (since=99, end=100): timeline: events 99 < ordering ≤ 100 → [Carol's message] state_after: deltas 99 < stream_id ≤ 100 → [$bob_join_call] ← delivered EARLY next_batch: s100 ← mid-batch token ``` No race on the client's side is needed: the server *hands out* the mid-batch token as `next_batch`. Alice's client re-polls with it, as every sync client does. The worker has meanwhile processed RDATA 101: ``` Sync B (since=100, end=101): timeline: events 100 < ordering ≤ 101 → [Bob's m.call.member @101] ✓ state_after: deltas 100 < stream_id ≤ 101 → [] row is stamped 100 ✗ ``` A state event in the timeline with an empty `state_after`. An MSC4222 client trusts `state_after` over timeline state events, so Alice's copy of Bob's call membership never updates from this response. On a single process this cannot happen: the batch's stream IDs are released as a whole, so the position visible to `/sync` jumps 99 → 101 and `s100` is never handed out. Only a process that learns its position from replication — any sync worker — ticks through the middle of a batch. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). 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. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
) This partially fixes the bug #20116 Device list update EDUs from non-compliant (grandfathered historical) user IDs are currently accepted over federation, stored, and surfaced to clients in `/sync`'s `device_lists.changed` array. > For current room versions, servers must still accept events using such user IDs over federation; however they SHOULD NOT forward such user IDs to clients when referenced outside the context of an event. For example, device list updates from non-compliant user IDs would be dropped by the receiving server. > > -- [Matrix spec](https://spec.matrix.org/v1.14/appendices/#historical-user-ids), clarified in Matrix v1.14 by [matrix-spec#1506](matrix-org/matrix-spec#1506) ### Problem Example A remote server sends an `m.device_list_update` EDU for `@héllo:remote.example` (non-ASCII localpart, outside the compliant U+0021–U+007E range). Synapse: - accepts and processes the update (resyncing the user's device list if needed) - stores it in the remote device list cache - forwards `@héllo:remote.example` to local clients via `device_lists.changed` in `/sync` (**the leak** — a non-compliant user ID referenced outside event context) --- ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). 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. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
Part of #18731 (Support Matrix 1.15). Since Matrix 1.15, the [spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv1roomsroomidhierarchy) defines `allowed_room_ids` on the room summaries returned by `GET /_matrix/client/v1/rooms/{roomId}/hierarchy`, so that clients can tell whether a restricted room can be joined or only knocked at: > **`allowed_room_ids`** — `[Room ID]` — If the room is a [restricted room](https://spec.matrix.org/v1.19/client-server-api/#restricted-rooms), these are the room IDs which are specified by the join rules. Empty or omitted otherwise. Added in `v1.15` > > — [Matrix Spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv1roomsroomidhierarchy) Synapse currently strips the field from the client `/hierarchy` response before returning it — a guard added in matrix-org/synapse#12175 (2022), correct at the time, when the spec defined the field for federation only and it was leaking into client responses. The other two surfaces that define the field (`/room_summary` and the federation `/hierarchy`) already return it. This PR removes the strip in `_RoomEntry.as_json` (and the now-unused `for_client` parameter), so client hierarchy entries include `allowed_room_ids` for restricted rooms, both local and received over federation. The first commit adds the failing test coverage, the second removes the strip. --- ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). 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. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
…#20224) See https://github.com/element-hq/synapse-private/blob/pro/.dockerignore Spawning from element-hq/synapse-private#169 where because we now have 'Synapse Variants' builds for the Synapse Pro container images, we can revert our special `docker/` changes back to match this repo. In this case, the new comments here make some sense so let's keep it ⏩
Resolve the conflict in the delayed events handler: - Keep the removal of `notify_new_event`, `_unsafe_process_new_event` and `_handle_state_deltas`. Develop only touched them to swap `Sentinel.UNSET_SENTINEL` for `Absent`. - Imports: take develop's `Codes` and `SynapseError`, which `add` now needs for its 403 `M_FORBIDDEN` and `DELAY_TOO_LARGE` errors, and drop `Measure` and `Absent`, which only the removed code used.
3 of 15 tasks
AndrewFerr
merged commit Oct 5, 2026
2aee8ae
into
AndrewFerr:msc4140-drop-state-cancel-rule
5 checks passed
AndrewFerr
deleted the
barodeur/msc4140-drop-state-cancel-rule-merge-base
branch
October 5, 2026 15:49
Owner
|
Merged, along with another merge with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings #19495 up to date with
develop. This is a single merge commit on top of the PR's head, nothing else.Two PRs merged into
developsince then touch the code that element-hq#19495 removes, and conflict with it insynapse/handlers/delayed_events.py:Codesto thesynapse.api.errorsimport, on the same line where MSC4140: drop delayed state interference element-hq/synapse#19495 dropsSynapseError. The merge keeps develop's import:addnow raisesSynapseErrorwithCodes.FORBIDDENandCodes.DELAY_TOO_LARGE(#20156).Sentinel.UNSET_SENTINELwithAbsentinside_handle_state_deltasand removes theSentinelimport next toMeasure. The merge keeps the removal of_handle_state_deltas,_unsafe_process_new_eventandnotify_new_event, and drops theAbsentandMeasureimports, which nothing else uses.The other four files (the store, the schema comment, the test and the changelog) merged cleanly.
I recommend
git show --remerge-diff 407758a8a6to see only the relevant changes.git show --remerge-diff 407758a8a6(the deleted body of the three methods is elided)