Repository navigation
[Server] End a listen stream that can carry nothing right after its acknowledgment - #618
Open
BrocksiNet wants to merge 1 commit into
Open
BrocksiNet wants to merge 1 commit into
BrocksiNet wants to merge 1 commit into
Conversation
…cknowledgment When the agreed filter of a `subscriptions/listen` request is empty (the server announces none of the requested types, or the client asks for none), nothing can ever be delivered on the stream. It still polled every 250 ms until the subscription lifetime ended, holding a PHP-FPM worker for the full lifetime, and a reconnecting client held one almost permanently. Send the acknowledgment with the empty `notifications` filter and close the subscription gracefully with `resultType: complete` right away. The spec allows the server to end a subscription on its own initiative this way. Adds `NotificationFilter::isEmpty()`.
BrocksiNet
requested review from
CodeWithKyrian,
Nyholm,
chr-hertel and
soyuka
as code owners
October 26, 2026 09:42
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
chr-hertel
added
Server
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the change to immediately close empty-filter subscription streams.
No open issues 🎉
What changed in this PR
Ends stateless subscriptions/listen streams immediately after acknowledgment when the agreed notification filter is empty, avoiding unnecessary polling and PHP-FPM worker usage. The behavior applies to both HTTP and stdio.
Changes:
- Adds
NotificationFilter::isEmpty()and reuses a completion-frame helper for both stream exits. - Adds regression tests for empty filters and immediate graceful closure.
- Documents the behavior and records it in the changelog.
| File | Description |
|---|---|
tests/Unit/Server/Stateless/StatelessProtocolTest.php |
Tests immediate acknowledgment and completion over HTTP and stdio. |
tests/Unit/Server/Stateless/NotificationFilterTest.php |
Tests filter emptiness and empty capability intersections. |
src/Server/Stateless/StatelessProtocol.php |
Closes empty-filter streams immediately and extracts completion framing. |
src/Server/Stateless/NotificationFilter.php |
Adds the empty-filter check. |
docs/run/subscriptions.md |
Documents immediate closure for streams that cannot deliver notifications. |
CHANGELOG.md |
Records the fix and new filter method under version 0.9.1. |
Review effort: Balanced
This branch has not been deployed
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
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.
Summary
subscriptions/listenrequest is empty, send the acknowledgment with the emptynotificationsobject and the gracefulresultType: completeclosure right away, instead of polling every 250 ms until the subscription lifetime ends. This applies to HTTP and stdio.NotificationFilter::isEmpty()for the check, and move the closure frame into a helper that both exits of the stream use.2026-07-28subscriptions pattern), so the client can still tell an intended end from a dropped transport.Motivation: under PHP-FPM such a stream holds a worker for the full lifetime even though nothing can ever be delivered, and a reconnecting client holds one almost permanently. We ran into this in Shopware's MCP server: shopware/shopware#21919
Fixes #617
Test plan
resources.subscribe, nothing requested, nonotificationsmember) yields exactly the acknowledgment and the completion, with no poll tick and no waiting on the 60 s lifetime; the same over stdio.NotificationFilterTestcoversisEmpty()and an intersection that ends up empty.make tests(2962 tests, 8 skipped)make phpstanmake cscomposer validate --strict