-
Notifications
You must be signed in to change notification settings - Fork 103
feat(server): add server lifecycle events #175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(server): add server lifecycle events #175
Conversation
275a0bb to
3d9d559
Compare
|
Nice one, thanks - before going into detailed review we should verify if we want that granularity with the events or if it should rather be one level higher like you could still check for the method there, like public function onRequestEvent(RequestEvent $event): void
{
if ('tools/list' !== $event->getMethod()) {
return;
}
}or similar. not super sure about it myself, but i just think of change scenarios and number of events - cc @CodeWithKyrian @Nyholm |
|
Hey @EdouardCourty, had a brief alignment with @CodeWithKyrian and we would like to go with more generic events - being a more robust solution with changes over time adding more handlers/notification - or with users adding their own. So it would be
dispatched on do you see that you could solve your use case with that as well? |
Alright, this works! |
|
That you presumably would dispatch the events rather in the |
|
Thank you.
Can you please elaborate on why? Do you want to be able to modify the request before it is given to the tool? Or do you want to change the result of the tool? Ie, What do you want to achieve after this PR is merged? |
Observing and reacting. I want to be able to trigger specific logic on each request (like storing these in an external database, or log them in a specific way) or response. For your other question, I think it could be useful to be able to modify the requests and responses before and after they are processed, leaving the choice to the developers that implement this SDK to use it or not. |
Nyholm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this PR.
I would like you to:
- Remove the abstract classes
- Make the event classes final
- Remove "use once" variables
- Follow up on this comment: #175 (comment)
If you would like help, let me know.
| $toolCallRequestEvent = new CallToolRequestEvent($request); | ||
| $this->eventDispatcher?->dispatch($toolCallRequestEvent); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you skip the variable here and for other similar places?
| $toolCallRequestEvent = new CallToolRequestEvent($request); | |
| $this->eventDispatcher?->dispatch($toolCallRequestEvent); | |
| $this->eventDispatcher?->dispatch($new CallToolRequestEvent($request)); |
| /** | ||
| * @author Edouard Courty <[email protected]> | ||
| */ | ||
| abstract class AbstractGetPromptEvent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this class
| /** | ||
| * @author Edouard Courty <[email protected]> | ||
| */ | ||
| abstract class AbstractCallToolEvent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this class
| /** | ||
| * @author Edouard Courty <[email protected]> | ||
| */ | ||
| abstract class AbstractReadResourceEvent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this class
Add Server Lifecycle Events
This PR introduces a comprehensive PSR-14 compatible event system for the MCP PHP SDK, allowing developers to hook into the server's lifecycle for tools, prompts, resources, ping, and initialize operations.
This will fix #169
Motivation and Context
Currently, there's no standardized way to observe or react to server operations like tool calls, prompt retrievals, or resource reads.
How Has This Been Tested?
Breaking Changes
None. This is a purely additive change. The event dispatcher is optional - servers without an event dispatcher configured will continue to work as before.
Types of changes
Checklist
Additional context
New Event Classes
Server Events:
InitializeRequestEvent- Fired when client sends initialize requestPingRequestEvent- Fired when client sends ping requestTool Events:
CallToolRequestEvent- Before tool executionCallToolResultEvent- After successful executionCallToolExceptionEvent- On uncaught exceptionPrompt Events:
GetPromptRequestEvent- Before prompt executionGetPromptResultEvent- After successful executionGetPromptExceptionEvent- On uncaught exceptionResource Events:
ReadResourceRequestEvent- Before resource readReadResourceResultEvent- After successful readReadResourceExceptionEvent- On uncaught exceptionFull documentation available in
docs/events.md.