Limit concurrent peers per real-time collaboration room to 2#75381
Closed
Limit concurrent peers per real-time collaboration room to 2#75381
Conversation
|
Size Change: +119 B (0%) Total Size: 8.75 MB
ℹ️ View Unchanged
|
Contributor
Author
|
The initial modal is basic, so will probably require some design, if we follow this route. |
chriszarate
reviewed
Feb 12, 2026
| /** | ||
| * Default maximum number of peers allowed in a single room. | ||
| */ | ||
| const DEFAULT_MAX_PEERS_PER_ROOM = 2; |
Contributor
There was a problem hiding this comment.
For simplicity, I would lean towards leaving the server code alone and agnostic to limits, and implement them purely in the client. This isn't a security feature, just a performance / roll-out restriction.
Contributor
Author
There was a problem hiding this comment.
I was thinking this could be an opportunity to educate users at this point, either directing them to the risks of increasing this value or giving them information on how to utilize an alternative provider.
chriszarate
reviewed
Feb 12, 2026
| // If the server signals that the room is full, notify | ||
| // listeners and stop syncing for this room. | ||
| if ( room.read_only ) { | ||
| doAction( 'sync.roomFull', room.room ); |
Contributor
There was a problem hiding this comment.
This should build on @shekharnwagh's work to emit a distinct status to indicate that the limit has been reached, which can be consumed by the existing modal.
Contributor
Author
There was a problem hiding this comment.
given this doesn't appear to be in trunk yet, should we get that ported over before fixing this?
Contributor
6475424 to
cd27e8f
Compare
cd27e8f to
f6562d8
Compare
chriszarate
reviewed
Mar 4, 2026
chriszarate
reviewed
Mar 4, 2026
Enforce a per-room peer limit (default: 2) in the HTTP polling sync server. When a new client tries to join a full room, the server returns a read_only response and the client emits a 'connection-limit-exceeded' status through the existing provider event system, which is handled by the SyncConnectionModal. The limit is configurable via the 'real_time_collaboration_max_peers_per_room' PHP filter.
Move the per-room peer limit check from the response body (read_only flag) into check_permissions(), returning HTTP 429 when a room is full. The client detects this in the catch block and emits connection-limit-exceeded through the existing provider status event system. Also incorporates wp_user_id tracking and client ID verification from PR #76056, and deduplicates by WordPress user so multiple tabs from the same user do not count as separate peers.
Guard the instanceof Response check with a typeof check since the Response global is not available in Jest/jsdom.
6a8fd25 to
ef22f6e
Compare
|
Flaky tests detected in 830f9b0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23153546757
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
chriszarate
reviewed
Mar 5, 2026
chriszarate
reviewed
Mar 5, 2026
| $wp_user_id = get_current_user_id(); | ||
| $rooms = $request['rooms']; | ||
| $wp_user_id = get_current_user_id(); | ||
| $current_time = time(); |
Contributor
There was a problem hiding this comment.
move $current_time inside the if block where it is used
Contributor
Author
There was a problem hiding this comment.
would that make much difference - I'd imagine we'd be talking fractions of a second difference?
Contributor
There was a problem hiding this comment.
Not perf-related, just a general principle of "Declare variables as close as possible to their first use."
Keeps it close to the implementation and makes code easier to review. When I saw this added variable, I scanned the whole scope to see where it might be used. Take or leave
Contributor
Author
There was a problem hiding this comment.
I think it got put there as it won't be reassigned during the for loops, but I can see the confusion and argument either way.
chriszarate
reviewed
Mar 5, 2026
chriszarate
reviewed
Mar 5, 2026
Co-authored-by: Chris Zarate <chris.zarate@automattic.com>
chriszarate
reviewed
Mar 6, 2026
chriszarate
reviewed
Mar 6, 2026
| } | ||
|
|
||
| // Limit the number of tabs a single user can open for this post. | ||
| if ( ! $is_client_tracked && $same_user_client_count >= self::DEFAULT_MAX_CLIENTS_PER_USER ) { |
Contributor
There was a problem hiding this comment.
Should this be filterable also?
Contributor
Author
There was a problem hiding this comment.
almost certainly, otherwise it'll be odd!
chriszarate
reviewed
Mar 12, 2026
| @@ -1,3 +1,4 @@ | |||
| https://github.com/WordPress/wordpress-develop/pull/11120 | |||
|
|
|||
| * https://github.com/WordPress/gutenberg/pull/76056 | |||
| * https://github.com/WordPress/gutenberg/pull/75381 | |||
Contributor
There was a problem hiding this comment.
Must be added to changelog for new wordpress-develop pull request. This one is closed / merged.
chriszarate
reviewed
Mar 12, 2026
| // If the server returned 429, the room is full. Notify all | ||
| // rooms and stop polling — this is permanent, not transient. | ||
| if ( | ||
| typeof Response !== 'undefined' && |
Contributor
There was a problem hiding this comment.
Response is a type. I'm not sure what this evaluates too, probably:
'undefined' !== 'undefined'
so this never triggers.
`Response should be explicitly imported also.
Suggested change
| typeof Response !== 'undefined' && |
Co-authored-by: Chris Zarate <chris.zarate@automattic.com>
chriszarate
reviewed
Mar 16, 2026
chriszarate
reviewed
Mar 16, 2026
Contributor
|
Closing in favor of #76565 |
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
3 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.
What?
Add a per-room peer limit to the HTTP polling sync server. When a room reaches capacity (default: 2 peers), new clients receive a blocking modal directing them back to the post list.
Fixes #75315.
Why?
Real-time collaboration should have configurable limits to prevent server resource exhaustion from too many simultaneous editors on a single document.
How?
DEFAULT_MAX_PEERS_PER_ROOMconstant andreal_time_collaboration_max_peers_per_roomfilter. Modifiedprocess_awareness_update()to track existing clients and reject new ones when the limit is reached, returning aread_onlyflag.read_onlyresponse, firessync.roomFullaction, and unregisters the room.PostCollaborationLimitModalcomponent listens for the action and shows a non-dismissible modal with "Go back" button.Testing Instructions
Testing Instructions for Keyboard
🤖 Generated with Claude Code