[Server] Support server notifications #42

Bellangelo · 2025-09-06T23:25:53Z

Introduce a NotificationPublisher that queues server-initiated MCP notifications. The Server now flushes this queue each tick and sends any pending notifications to the client.

We initially wired this via an EventDispatcher between the Registry and notifications, but, as per the discussion in this PR (see final comment), we simplified the design to invoke NotificationPublisher directly at the mutation points. As a result, the EventDispatcher wiring and the interim internal events used for that wiring have been removed.

Motivation and Context

Closes #12

How Has This Been Tested?

Unit and Integration tests. I have added extra tests for the Registry to increase its coverage.

Breaking Changes

None for typical users constructing the server via ServerBuilder.
(If someone manually instantiates Server, they now pass the NotificationPublisher )

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Bellangelo · 2025-09-06T23:28:56Z

Hi @chr-hertel, I created a PoC of a solution. There are a few things that do not work 100% correct but I wanted a quick validation around the idea. I had to implement a few new things so I wasn't sure if I understood correctly the issue.

chr-hertel · 2025-09-06T23:39:52Z

Nice one @Bellangelo - thanks for working on this! :)

So, i just gave it a quick read, maybe i missed something. so using an event dispatcher in the registry was an idea to publish events, but i wonder if we'd go with your NotificationPublisher idea - and i like it btw 👍 - if we'd need that dispatcher at all.

with the publisher there is a central stack of notifications that get's emptied by the server - who else would listen to those events? and if there's only one consumer of events why not push from the emitting service (registry) to the publisher directly?

Bellangelo · 2025-09-06T23:59:14Z

with the publisher there is a central stack of notifications that get's emptied by the server - who else would listen to those events? and if there's only one consumer of events why not push from the emitting service (registry) to the publisher directly?

Good question @chr-hertel, my initial thought for the PSR event dispatcher was that the developer building the server might also want to listen to these events. That’s why I’m dispatching through the developer’s event dispatcher here:
https://github.com/modelcontextprotocol/php-sdk/pull/42/files#diff-1758bd2c4379f6f88e9048d7b437c38d17a64e3d71dd2e9493322c7e61d4a87fR224-R229

If my understanding is off, then you’re right and we could call the NotificationPublisher directly instead of going through the dispatcher. The trade-off is that this would couple components that are otherwise unrelated. However, it might be unnecessary complexity at this stage of the project.

chr-hertel · 2025-09-07T00:05:53Z

yup, valid point, server application code might listen to that as well - but on the other hand i'd expect that the impulse for change is rather coming from the application side anyways, since the server itself doesn't have inner logic to change the lists - as of now.
currently i'm more leaning to keeping it simple for now - like having the direct coupling and removing the EventDispatcher for now, yea

huangdijia · 2025-09-07T00:27:51Z

Transitionally depending on symfony is not a good thing; it lacks flexibility

Bellangelo · 2025-09-07T10:18:32Z

hi @chr-hertel, it should be ready now.

butschster · 2025-09-08T15:16:32Z

Hi @Bellangelo 👋

I noticed that in addition to the MessageFactory instantiation https://github.com/Bellangelo/php-sdk/blob/support-server-notifications/src/JsonRpc/Handler.php#L58, the NotificationPublisher class also has a constructor that takes just one argument. I was wondering why you decided to introduce the extra static make() method for creating it, instead of relying directly on the constructor. (https://github.com/modelcontextprotocol/php-sdk/pull/42/files#diff-1758bd2c4379f6f88e9048d7b437c38d17a64e3d71dd2e9493322c7e61d4a87fR211)

Is there a specific reason or design goal behind this approach? I’d love to better understand the intention here. 🙂

Bellangelo · 2025-09-09T11:07:56Z

Hi @Bellangelo 👋

Is there a specific reason or design goal behind this approach? I’d love to better understand the intention here. 🙂

Hey @butschster, I did this to minimize how many things the dependents know about their dependencies. If the NotificationPublisher grows more, you won't have to change the ServerBuilder.

chr-hertel · 2025-09-09T20:48:11Z

src/Capability/Registry.php

        $this->tools[$toolName] = new ToolReference($tool, $handler, $isManual);

-        $this->eventDispatcher?->dispatch(new ToolListChangedEvent());
+        $this->notificationPublisher->enqueue(ToolListChangedNotification::class);


wouldn't it be a bit easier to just go with

Suggested change

$this->notificationPublisher->enqueue(ToolListChangedNotification::class);

$this->notificationPublisher->enqueue(new ToolListChangedNotification());

@chr-hertel Hmm.. I thought that they would need to pass through the MessageFactory. If they don't need to pass, should I also remove these notifications from the MessageFactory? Because they won't be send as a response to a prompt.

MessageFactory removed from the NotificationPublisher in this commit 3019972

the MessageFactory is intended to handle incoming json/array payload - in this case it's outgoing

I know but it seems that it contains outgoing notifications https://github.com/modelcontextprotocol/php-sdk/blob/main/src/JsonRpc/MessageFactory.php#L39. So we should remove these from the MessageFactory, right?

chr-hertel · 2025-09-09T22:40:55Z

src/JsonRpc/MessageFactory.php

     * @return class-string<HasMethodInterface>
     */
-    private function getType(string $method): string
+    private function getType(string $method/**/): string


Suggested change

private function getType(string $method/**/): string

private function getType(string $method): string

Fixed in 759a379

chr-hertel · 2025-09-09T22:43:04Z

src/Server/NotificationPublisher.php

+        $out = $this->queue;
+        $this->queue = [];
+
+        yield from $out;


should work as well

Suggested change

$out = $this->queue;

$this->queue = [];

yield from $out;

yield from $this->queue;

$this->queue = [];

Fixed in f226b2b

chr-hertel · 2025-09-09T22:44:50Z

src/Capability/Registry.php

    private array $resourceTemplates = [];

    public function __construct(
+        private readonly NotificationPublisher $notificationPublisher,


Suggested change

private readonly NotificationPublisher $notificationPublisher,

private readonly NotificationPublisher $notificationPublisher = new NotificationPublisher(),

Fixed in 4c2415b

chr-hertel

Ready for merging from my point of view - thanks @Bellangelo!

@CodeWithKyrian any thoughts? :)

chr-hertel · 2025-09-14T18:45:16Z

Needs a rebase to solve the conflicts

Bellangelo · 2025-09-15T20:58:31Z

Needs a rebase to solve the conflicts

@chr-hertel Done.

CodeWithKyrian · 2025-09-21T22:21:18Z

@Bellangelo please rebase and adjust to fit the new architecture.

I’m also wondering if there’s a possibility of using the session as a message queue, since it can be persistent (depending on the store) and is unique to each client connection. Just rough thoughts at this point.

@chr-hertel, any thoughts?

chr-hertel · 2025-09-22T21:22:53Z

@CodeWithKyrian that would be even required for making this work on the http transport, right?
@Bellangelo can you have a look into this? changed with merging #49

Bellangelo requested review from chr-hertel, Nyholm and CodeWithKyrian as code owners September 6, 2025 23:25

Bellangelo marked this pull request as draft September 6, 2025 23:26

Bellangelo marked this pull request as ready for review September 7, 2025 10:17

Bellangelo changed the title ~~PoC - Support server notifications~~ Support server notifications Sep 7, 2025

Bellangelo mentioned this pull request Sep 8, 2025

Implement SEP-973: Icons and metadata support #19

Open

chr-hertel reviewed Sep 9, 2025

View reviewed changes

Bellangelo added 11 commits September 10, 2025 01:58

Add method to create messages by type

76f0641

Parse and queue notifications

b61828f

Send notifications

e4780c6

Clean-up and fix tests

f571e71

Install Symfony dispatcher to handle events for internal usage

b5622c6

Use Symfony dispatcher for internal usage

be26b49

Test the new notification publisher

4af2d4a

Fix code style

d6187dd

Add auth annotations

413660b

Test new message factory method

1fa635d

Remove event dispatcher

138fea6

Bellangelo added 11 commits September 10, 2025 01:58

Server accepts different argument now

fde283c

Fix return type

6beda2d

No need for an id

de10bcd

Clean-up phpdoc

8c81487

Update tests

8b201e1

Fix sending notifications

4900058

Use generator to return notifications

7a31c24

Test Registry

efd3c0a

Let the caller handle the construction of the notification

8c55c25

Remove unused method

f39c614

Fix code style

759a379

Bellangelo force-pushed the support-server-notifications branch from 2d2b84f to 759a379 Compare September 9, 2025 22:58

Bellangelo added 2 commits September 10, 2025 02:01

Simplify logic

f226b2b

Make NotificationPublisher optional

4c2415b

Bellangelo requested a review from chr-hertel September 9, 2025 23:10

chr-hertel approved these changes Sep 14, 2025

View reviewed changes

chr-hertel added the Status: Needs Decision label Sep 14, 2025

chr-hertel added the Status: Needs Work label Sep 14, 2025

Bellangelo added 4 commits September 15, 2025 23:45

Merge branch 'main' into support-server-notifications

271befb

Update tests based on new constructor

4a29253

Unify tests

244a81e

Fix styling

dab3bf3

chr-hertel removed the Status: Needs Work label Sep 16, 2025

chr-hertel changed the title ~~Support server notifications~~ [Server] Support server notifications Sep 21, 2025

chr-hertel added Status: Needs Work and removed Status: Needs Decision labels Sep 29, 2025

	$this->notificationPublisher->enqueue(ToolListChangedNotification::class);
	$this->notificationPublisher->enqueue(new ToolListChangedNotification());

	private function getType(string $method/**/): string
	private function getType(string $method): string

	private readonly NotificationPublisher $notificationPublisher,
	private readonly NotificationPublisher $notificationPublisher = new NotificationPublisher(),

[Server] Support server notifications #42

Are you sure you want to change the base?

[Server] Support server notifications #42

Conversation

Bellangelo commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Checklist

Additional context

Uh oh!

Bellangelo commented Sep 6, 2025

Uh oh!

chr-hertel commented Sep 6, 2025

Uh oh!

Bellangelo commented Sep 6, 2025

Uh oh!

chr-hertel commented Sep 7, 2025

Uh oh!

huangdijia commented Sep 7, 2025

Uh oh!

Bellangelo commented Sep 7, 2025

Uh oh!

butschster commented Sep 8, 2025

Uh oh!

Bellangelo commented Sep 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chr-hertel left a comment

Choose a reason for hiding this comment

Uh oh!

chr-hertel commented Sep 14, 2025

Uh oh!

Bellangelo commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CodeWithKyrian commented Sep 21, 2025

Uh oh!

chr-hertel commented Sep 22, 2025

Uh oh!

Uh oh!

Bellangelo commented Sep 6, 2025 •

edited

Loading

Bellangelo commented Sep 15, 2025 •

edited

Loading