Uh oh!
There was an error while loading. Please reload this page.
Remove invalid WARN message for missing notification handler in stateless servers - #835
Conversation
There was a problem hiding this comment.
Thanks for your contribution! This is directionally good, but the tests need reworking.
Both the tests in this PR pass even without your changes, because of the server's default handleNotification which does return Mono.empty.
The only difference with and without your code is suppressing a WARN log message.
Also, there's no need to use mock requests, as the SDK's client can trigger the notifications.
Please update your test to:
- Use the bundled
client - Read from the logs and check the absence of the specific warn message
Please also consider adding an appropriate log message for roots/list_changed. After all the client is sending a notification we can't handle, we should at least log it, probably at the DEBUG level, not WARN.
1439baa to
18fef50Compare18fef50 to
055bc10CompareCo-authored-by: Daniel Garnier-Moiroux <git@garnier.wf> Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
055bc10 to
0f95783CompareKehrlann
commented
Jul 10, 2026
Polished and merged, thanks for your contribution! |
Uh oh!
There was an error while loading. Please reload this page.
Motivation and Context
#777
According to the MCP specification, notifications from client to server are allowed.
In the case of notification/roots/list_changed, if the client supports listChanged, sending this notification is MUST.
The initialized notification is part of the lifecycle specification.
Since the MCP spec does not define any specific error-handling requirements for notification/roots/list_changed on the server side, it seems more appropriate to accept the notification rather than return a "Missing handler" error.
Because the server is stateless, we have implemented notification/roots/list_changed as a no-op.
How Has This Been Tested?
I wrote test code.
Breaking Changes
nop
Types of changes
Checklist
Additional context