-
Notifications
You must be signed in to change notification settings - Fork 943
Added UI to provide additional _meta values #770
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?
Conversation
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!
Tested locally with prompts/list resources/list, resources/subscribe, resources/unsubscribe, resources/templates/list, completion/complete, resources/read, tools/call, and tools/list.
However,
Can we just make this "Metadata"? I understand it's "data for the _meta param" but Metadata is a word. Meta Data is weird.
And make MetaDataTab be MetadataTab.
|
I tried to satisfy all you suggestions. For the cli test, we would need to update the everything server first to somehow react to metadata and/or return the submitted values in the echo tool to make sure the values are passed along. The added test for now only check if passing values is successful. |
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.
Left some inline notes, but also, can you remove the metadata-test-output folder and files and add the folder to .gitignore?
|
Hey @ln-12! Thanks for clearing up the test logging. In testing this locally though, it seems like there's been a slight regression since the first time I tested this. If I remember correctly,
What I'm now seeing is that I set a pair on the Metadata tab, and then I do tools/list and I see the pair ( |
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.
Test logging issue and naming issue straightened out, but now there's a regression in sending general metadata to tool calls.
|
Good catch, I accidentally shadowed the metadata state field when renaming the function parameter. I checked it again locally and all combinations of general and tool metadata work as expected for me now. |
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.
Looking good so far, tool-specific metadata is being sent in addition to general metadata.
However, it allows me to send reserved keys, and that should be prohibited. Please review the spec on the matter of reserved keys and make sure they're all disallowed.
In this case I'm sending a key of modelcontextprotocol.io/flip and it is allowed.




This PR adds a UI section inside the tools tab which enabled the user to provide extra key-value pairs for the
_metaobject. Fixes #559.UI:

Request:

Motivation and Context
Currently there is no way to provide additional
_metavalues in the UI.How Has This Been Tested?
I provided values in the UI and checked that they are sent in the request. Also, I added a test.
Breaking Changes
No
Types of changes
Checklist