[Server] Wire up notifications for changing prompt, resource & tools lists #234
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.
This is a working draft for emitting change notifications on dynamically added prompts, resources and tools.
The main issue here is that we need that indirection between Registry, source of truth on changing lists, and Protocol, that is able so send notifications. That's why we have those events, but we never really adopted them.
I introduced a slim EventDispatcher/ListenerProvider setup, but it feels a bit flaky - see wire up in example. The service landscape within the SDK just grows in complexity.
TBD
Event Dispatcher
The issue I had was mostly about PSR-14 - we want to empower library users to inject their own event dispatcher, to easily register subscriber/listener/whatever - but with the current implementation we also want to add our own listeners for triggering the notifications (again, we need a decoupling between registry and protocol currently).
With PSR-14 the
EventDispatcherInterfaceused in theRegistrydoesn't offer us to register our own listeners after a user injected it viaBuilder::setEventDispatcher()Session ID State
I need to call the
Protocol::sendNotification(...)from the listeners, but didn't have the session at hand - and there is basically no state in the Protocol - which is great, but we don't have a service concept to retrieve the session - it is only state in the transport or arguments looped through half the SDK.I went for introducing the session as state in the
Protocol- needs better null-checks tho, but wanted to check with @CodeWithKyrian first - the other option would be to keep the transport as state, since it also has the session already as state.WDYT?
edit: this currently messes up the session handling within a fiber intermediate client request.