-
Notifications
You must be signed in to change notification settings - Fork 968
feat: support custom header #549
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
Conversation
|
LGTM |
1 similar comment
|
LGTM |
|
Hi, this older PR is editing a lot of the same UI, so before we move forward with this one, lets wait on this PR to be finalized: #345 If we feel this is urgent and is blocked for too long, we can revisit how to best coordinate these. Thanks for your patience! |
|
In the meantime, I don't love the hash in the |
|
Hi @popomore , it seems like the main outstanding concern we had here was with the UX (see Cliff's comment above). In general I think we might need to revisit the UI design of the sidebar since there is a lot going on there and editing long values can be annoying. Any thoughts on how we might tweak this to address the callouts above? Thanks! |
|
Suggestion: rename "custom headers" to "additional headers" which is the term used in the SDKs. Also headers can be used for other purposes than authentication, so I suggest changing the comments to be less categorical about the purpose of the headers. For example, I have seen them used for localization purposes. |
|
@cliffhall @olaservo I’m not sure whether using a custom header field or a hash is the better approach — I tend to prefer using a hash for this purpose. @PederHP I agree with the use of additional headers. I’ll make the changes after the requirements are confirmed. |
I don't see the functionality of "#" in the button name, and I still question the need for the Custom (or Additional) headers field that has the big JSON ball inside it. Will you be editing that separately somewhere and pasting it in? Surely you don't expect the user to edit JSON inside it? If it just accumulates the headers that are created in the forms below in a read only way, what would you be doing with the resultant content? Copying it and pasting it? Where and for what? We have very little real estate in the sidebar, and this field seems unnecessary. I suggest the button that has "# Headers" be called "Additional Headers" and it open or close the section with "Header 1", "Header 2" boxes etc. Those boxes should be in a vertical box that scrolls on its own if the whole sidebar becomes filled, as it easily could if you add a lot of headers. That should be the extent of the UI unless there is some demonstrable need for the JSON field. |
|
when this? |
|
@popomore Any thoughts on my last comment, and whether you'd like to carry on with this PR? |
|
I think this feature is really good. |
|
Fills a real gap: Setting headers is critical for testing servers with auth or special config. This makes the inspector much more practical for real-world scenarios . ⸻ Suggested improvements |
|
@peterswimm I think this PR has been abandoned. But there is another PR in the queue that looks better IMO. |
Summary
This PR adds custom header support to the MCP Inspector, allowing users to configure and send custom HTTP headers when connecting to MCP servers. This enhancement improves flexibility for testing servers that require authentication tokens, API keys, or other custom headers.
Ref #249
Motivation and Context
Many MCP servers require custom headers for authentication (e.g., Bearer tokens, API keys) or other configuration purposes. The current inspector lacks the ability to set these headers, making it difficult to test servers that have authentication requirements or need specific header configurations.
This feature addresses the need to:
Changes Made
How Has This Been Tested?
Breaking Changes
No breaking changes - this is a backward-compatible feature addition
Types of changes
Checklist
Additional context
UI
