-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Add instructions to Everything Server #1919
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
Add instructions to Everything Server #1919
Conversation
|
@evalstate after talking about instructions in Inspector I thought it might be good to add this. However since I haven't been actively using this feature in the context of an actual host/LLM using these instructions from a server, I wanted to gut check a couple things:
I can also tinker with these first hand but thought you might have more experience with it. Thanks! |
|
This is great @olaservo. There is an example in the Base Protocol Lifecycle page has Yes, this would usually be added to the System Prompt to describe the scenarios under which the toolset would be used, or how tools/resources may be used together. The MCP Prompts I would not expect to be in here, but Resources may (as they actually do have an LLM facing |
cliffhall
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.
LGTM! 👍
|
Converting back to Draft to revise this to be closer to recent discussions around uses for this field. |
Just realised I didn't answer that (sorry not been tracking the discussion). I would say so -- for example |
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 pinged you in the Discord about a slight change.
The contents of instructions.md just made me aware that the server is erroneously triggering a sampling request on subscriptions. We could fix that by removing line 281 of everything.ts and removing this passage from instructions.md
- subscription automatically triggers sampling request to client
Or I could do that in a subsequent request. I'm approving because I tested and all else seems fine. If you'd like to merge this, I'll follow up. Or if you agree and want to make that slight change here, I'll test and approve again.
cliffhall
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.
Slight rewording of the resources part.
src/everything/instructions.md
Outdated
| Testing and demonstration server for MCP protocol features. Workflow: Subscribe to resources before testing notifications - subscription automatically triggers sampling request to client. Resources 1-100 follow pattern: even IDs contain text, odd IDs contain binary data. Resources paginated at 10 items per page with cursor-based navigation. | ||
|
|
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.
| Testing and demonstration server for MCP protocol features. Workflow: Subscribe to resources before testing notifications - subscription automatically triggers sampling request to client. Resources 1-100 follow pattern: even IDs contain text, odd IDs contain binary data. Resources paginated at 10 items per page with cursor-based navigation. | |
| Testing and demonstration server for MCP protocol features. | |
| Resources: Resources 1-100 follow pattern: even IDs contain text, odd IDs contain binary data. Resources paginated at 10 items per page with cursor-based navigation. |
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.
If removing the verbiage about subscription/sampling, this line should change, too. As for "Workflow:..." there's really no need to subscribe before testing notifications. They come to you automatically.
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.
Makes sense - I simplified this part as you suggested. (Although the GitHub UI wouldn't allow me to apply the suggestion directly so you might want to double check that I got this right.)
cliffhall
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.
Just a whitespace change.
Co-authored-by: Cliff Hall <cliff@futurescale.com>
cliffhall
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.
LGTM! 👍
Description
Adds
instructionsto demonstrate this feature for clients that support it.Edited from my original example to follow best practices notes sourced from related discussions, latest changes summarized by Claude below:
1. Be Concise and Clear
2. Focus on Workflows and Dependencies
_meta.progressToken)3. Avoid Redundancy with Tool Descriptions
4. Include Performance Considerations and Limitations
5. Provide Contextual Guidance
6. Educational Enhancement
Server Details
Motivation and Context
This is not yet a well-known feature of MCP, so including an example here could help increase visibility.
How Has This Been Tested?
Tested in Inspector:
Breaking Changes
None
Types of changes
Checklist