Where does notification go?

[sei-li-miidas]

https://github.com/mark3labs/mcp-go/blob/main/server/sse.go#L113-L125

All sessions are waiting for notifications, Golang doesn't guarantee that the session which sends the notification will receive it.
And because of

// Only forward notifications meant for this session
if serverNotification.Context.SessionID == sessionID {

the notification may just got lost?
Is it on purpose?

BTW.
Thought the official SDK
https://github.com/modelcontextprotocol/typescript-sdk/blob/main/src/shared/protocol.ts#L535-L548
also only sends notification to one client, the current client.
I think it's better there is way to send notification to all clients.
When tools change, server wants to notify every client.

[ezynda3]

So I agree if something like tools changed then we need to send to all clients. But if for example someone triggered a long running process then the notifications should only apply to that one client. Need to figure out the best way to separate the two.

[xinwo]

@ezynda3
below is on purpose or just a bug?

https://github.com/mark3labs/mcp-go/blob/main/server/sse.go#L113-L125

All sessions are waiting for notifications, because of

// Only forward notifications meant for this session
if serverNotification.Context.SessionID == sessionID {

if the notification goes to different session, it just got lost?

[xinwo]

Need to figure out the best way to separate the two.

Currently there are below notifications

notifications/initialized
notifications/progress
notifications/message

pattern1: only go to current client.

notifications/resources/list_changed
notifications/resources/updated
notifications/prompts/list_changed
notifications/tools/list_changed

pattern2: go to all clients.

How about at
https://github.com/mark3labs/mcp-go/blob/main/server/sse.go#L113-L125
loop the session store.
If the notificaiton only goes to current client, find the session and send it.
If the notification should go to all clients, send to all sessions.

There are two concerns doing it this way.

1. If there are thousands of clients, it could take quite long to iterate the session store.
2. It doesn't work if there are multiple MCP servers.

[torkve]

So I agree if something like tools changed then we need to send to all clients. But if for example someone triggered a long running process then the notifications should only apply to that one client. Need to figure out the best way to separate the two.

I also encountered the issue, and I believe it should be separate cases:

1. Context-less notification (e.g. list of tools changed): such things are done not in the connection handling loop, and want to notify all the connected clients. I'm not sure if this API should be exposed to the user at all.
2. Context-aware notification: it is something that is spawned during handling of client request (long operation and so on). I also believe that such operations should always use or inherit the context.Context created during session accept, hence you can always store session notification channel in this context and notification of the client would always be safe for the user.

[torkve]

Regarding the issue, I see that there was some attempt to make that very thing I propose: https://github.com/mark3labs/mcp-go/blob/main/server/server.go#L94-L116
However, ctx is not modified in the WithContext. Also it appears to me, that clientMu has no effect at all. So I propose:

1. WithContext to save NotificationContext into the ctx.
2. SendNotificationToClient to remain public method, sending notification to the session from ctx (however it have to accept ctx parameter now) .
3. Methods like AddTool should use some new private method, that will send notification to all clients, not some "current".

If you agree, I can make a PR today.

[sei-li-miidas]

I'm a little curious about how to trigger a notification from outside.
Of course AddTool and something like can send notifications, but how to call it when server is running?

In my case, my prompts and tool definitions are defined in the database, when I update them, I want to notify all clients.
I created a tool for that, in the tool it just calls mcpServer.SendNotificationToClient("notifications/tools/list_changed", nil), I thought it will send notification to all clients, and found out that it doesn't...

[torkve]

It should be done exactly by AddTool/SetTools/RemoveTool, I suppose.

[sei-li-miidas]

But currently how to call them when server is running?

[torkve]

This should be fixed, of course :) There is no sense to send notifications (as it is done now) when server is not running, because you have no clients connected.

[torkve]

I created PR #46 with the fix of this issue.

Regarding the problem mentioned by @sei-li-miidas above, I suggest to make a separate issue: it should be fixed by also making initialized flag session-local rather than per-server one.
I can also fix it later, if this one is accepted.