mcp: debounce server change notifications

[jba]

Debounce notifications for changes in feature sets (adding or removing tools, prompts, resources, or resource templates).

Delay sending the notifications until some time has passed.

This PR handles server-side change notifications only. We could do the same for client-side notifications (e.g. roots changed), but at the moment we don't need to.

We get errors about undelivered messages, because the delayed notifications are sent before the standalone GET happens. See #684. We'll deal with them in another PR (if at all).

Fixes #649.

[findleyr]

BUG: notifications sent after a connection closes result in an error message printed out. I'm not sure how to avoid that. It would be racy to check for a closed connection just before sending, though it would reduce the number of messages considerably.

I think there will always be a race, but shouldn't we have deleted the session after it's closed, making this race very unlikely? I.e. it should no longer be present in the sessions set?

[markus-kusano]

only nits

[jba]

BUG: notifications sent after a connection closes result in an error message printed out. I'm not sure how to avoid that. It would be racy to check for a closed connection just before sending, though it would reduce the number of messages considerably.

I think there will always be a race, but shouldn't we have deleted the session after it's closed, making this race very unlikely? I.e. it should no longer be present in the sessions set?

Yes, it's not sessions closing.
Looks like this is related: #669.

[jba]

BUG: notifications sent after a connection closes result in an error message printed out. I'm not sure how to avoid that. It would be racy to check for a closed connection just before sending, though it would reduce the number of messages considerably.

I think there will always be a race, but shouldn't we have deleted the session after it's closed, making this race very unlikely? I.e. it should no longer be present in the sessions set?

Yes, it's not sessions closing. Looks like this is related: #669.

The problem is that when a POST is done, the deliver function is nilled out (https://github.com/modelcontextprotocol/go-sdk/blob/main/mcp/streamable.go#L1092), so there is nowhere to send the notification. Perhaps changed notifications should always go to the hanging GET.

As #669 points out, this can happen for progress notifications too, but in that case dropping the notification makes sense, because that notification is useless.

[findleyr]

BUG: notifications sent after a connection closes result in an error message printed out. I'm not sure how to avoid that. It would be racy to check for a closed connection just before sending, though it would reduce the number of messages considerably.

I think there will always be a race, but shouldn't we have deleted the session after it's closed, making this race very unlikely? I.e. it should no longer be present in the sessions set?

Yes, it's not sessions closing. Looks like this is related: #669.

The problem is that when a POST is done, the deliver function is nilled out (https://github.com/modelcontextprotocol/go-sdk/blob/main/mcp/streamable.go#L1092), so there is nowhere to send the notification. Perhaps changed notifications should always go to the hanging GET.

As #669 points out, this can happen for progress notifications too, but in that case dropping the notification makes sense, because that notification is useless.

Ack, thanks for investigating. I think this is working as intended, then. If the message is made with the request context, then it's invalid after the request completes. To send a background notification, use context.Background() (if this isn't apparent from our documentation, we should fix that).