Skip to content

feat: add request header #143

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

wlxwlxwlx
Copy link

@wlxwlxwlx wlxwlxwlx commented Apr 14, 2025

Add the request header to mcp.Request so that users can use the header to handle business logic in the hooks function.

Summary by CodeRabbit

  • New Features
    • Added support for passing and accessing HTTP headers in server requests, enabling features like authorization and header-based filtering.
  • Tests
    • Updated tests to include header information in server request handling.
    • Added a new test to verify header-based filtering of tools based on authorization tokens.

Copy link
Contributor

coderabbitai bot commented Apr 14, 2025

Walkthrough

A new Header field was added to the Request struct, and the HandleMessage method in the server was updated to accept and propagate a header map. All relevant server, SSE, stdio, and client code, as well as tests, were updated to accommodate the new header parameter, including a new test for header-based filtering.

Changes

Files/Paths Change Summary
mcp/types.go Added Header map[string]string field to the Request struct.
server/request_handler.go Updated HandleMessage method to accept a header map and assign it to the request struct.
server/sse.go, server/streamable_http.go Extracted HTTP headers into a map and passed it to HandleMessage.
server/stdio.go, client/transport/inprocess.go Updated calls to HandleMessage to include an empty header map.
client/client.go Removed an extraneous blank line in sendRequest; no logic changes.
server/server_test.go Updated all HandleMessage test calls to include a header map; added TestMCPServer_HandleWithHeader to verify header-based filtering.
server/resource_test.go, server/session_test.go Updated all HandleMessage test calls to include an authorization header.

Suggested reviewers

  • robert-jackson-glean
  • ezynda3

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (1.64.8)

Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2
Failed executing command with error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e2e7099 and fa50388.

📒 Files selected for processing (8)
  • client/client.go (0 hunks)
  • mcp/types.go (1 hunks)
  • server/request_handler.go (10 hunks)
  • server/server_test.go (24 hunks)
  • server/session_test.go (5 hunks)
  • server/sse.go (2 hunks)
  • server/stdio.go (1 hunks)
  • server/streamable_http.go (1 hunks)
💤 Files with no reviewable changes (1)
  • client/client.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • mcp/types.go
  • server/stdio.go
  • server/session_test.go
  • server/sse.go
  • server/request_handler.go
🔇 Additional comments (3)
server/streamable_http.go (1)

310-315: LGTM! Clean header extraction and forwarding implementation.

The header extraction logic correctly converts HTTP headers to a map and forwards them to the MCP server. Note that this implementation takes only the first value from multi-value headers (v[0]), which is appropriate for most use cases, especially authentication headers.

server/server_test.go (2)

11-11: Excellent consistency in updating all test cases.

All HandleMessage calls have been systematically updated to include the new header parameter, ensuring comprehensive test coverage for the new API signature. The consistent use of {"Authorization": "Bearer test"} across all tests maintains uniformity.

Also applies to: 145-146, 351-352, 459-460, 499-503, 525-526, 605-611, 686-691, 809-813, 974-974, 979-979, 999-999, 1064-1068, 1200-1204, 1293-1297, 1383-1384, 1432-1436, 1455-1458, 1645-1645, 1647-1651, 1654-1658, 1664-1668, 1674-1681, 1826-1834, 2023-2023


2042-2102: Outstanding example test demonstrating header-based business logic.

This test perfectly showcases the practical value of the header feature by implementing token-based tool filtering. The implementation demonstrates:

  • Realistic authorization logic using bearer tokens
  • Proper access to headers within hooks via message.Header["Authorization"]
  • Tool filtering based on permission mappings
  • Verification that only authorized tools are returned

This test serves as both validation and documentation for the intended use case described in the PR objectives.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
server/stdio.go (1)

238-238: Header parameter now included in HandleMessage call.

The code has been updated to pass an empty header map to match the new signature. This maintains compatibility while not providing any actual headers from the stdio interface.

Consider allowing headers to be customized in stdio mode by providing a configuration option similar to WithStdioContextFunc. This could allow testing with headers or implementing specific behaviors based on predefined headers.

-response := s.server.HandleMessage(ctx, map[string]string{}, rawMessage)
+response := s.server.HandleMessage(ctx, s.getRequestHeaders(), rawMessage)

With a corresponding function and field:

func (s *StdioServer) getRequestHeaders() map[string]string {
    if s.defaultHeaders != nil {
        return s.defaultHeaders
    }
    return map[string]string{}
}
server/sse.go (1)

357-360: Header extraction from HTTP request.

The code correctly extracts headers from the HTTP request, but only uses the first value for each header key.

Consider handling multi-value headers more appropriately. HTTP headers can have multiple values, and currently, only the first one is used.

-for k, v := range r.Header {
-    header[k] = v[0]
-}
+// Option 1: Join multiple values (common approach)
+for k, values := range r.Header {
+    header[k] = strings.Join(values, ", ")
+}
+
+// OR Option 2: Use a multi-map if needed elsewhere in the codebase
+// This would require changing the Header field type in Request struct
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c509bf9 and 012fe6d.

📒 Files selected for processing (5)
  • mcp/types.go (1 hunks)
  • server/request_handler.go (10 hunks)
  • server/server_test.go (18 hunks)
  • server/sse.go (3 hunks)
  • server/stdio.go (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
server/sse.go (2)
examples/custom_context/main.go (1)
  • MCPServer (100-102)
server/server.go (1)
  • MCPServer (143-159)
🔇 Additional comments (22)
mcp/types.go (1)

89-91: Good addition of header support to the Request struct.

The addition of the Header field to the Request struct enables passing metadata via headers, which will be useful for implementing business logic in hooks functions as mentioned in the PR objectives.

server/request_handler.go (3)

15-17: Good update to include header parameter in HandleMessage signature.

This change appropriately adds a header map[string]string parameter to the method signature, allowing headers to be passed in for processing.


70-70: Properly initializing Request.Header from parameter.

The header parameter is correctly assigned to the request.Header field, ensuring it's available for the initialization flow.


90-90: Consistent header assignment across all request types.

Good implementation - consistently applying the header parameter to all request types ensures headers are available throughout the application.

Also applies to: 116-116, 142-142, 168-168, 194-194, 220-220, 246-246, 272-272

server/sse.go (3)

56-64: Improved code formatting for struct fields.

The reformatting of variable declarations in the SSEServer struct improves readability and consistency.


161-166: Consistent field formatting in constructor.

The reformatting of the initialization values in the NewSSEServer function matches the struct definition and improves readability.


362-362: Updated HandleMessage call with header parameter.

The code correctly passes the extracted headers to the HandleMessage method, allowing request headers to propagate through the system.

server/server_test.go (15)

142-143: Good implementation of the new header parameter

The header parameter has been properly added to the HandleMessage call with an "Authorization" header, which aligns with the PR objective of adding request header support.


316-317: Consistent header implementation in the tools test cases

The header parameter has been properly implemented in both test message handling calls, maintaining consistency across the test case.

Also applies to: 337-338


425-426: Appropriate header implementation in message handling tests

The header parameter follows the same pattern used across the PR, ensuring consistent behavior testing.


465-469: Correctly updated the pagination test cases

The header parameter is appropriately inserted between the context and message parameters.


488-489: Properly updated notification handling test

The header parameter maintains the consistent pattern with the "Authorization" token.


568-569: Correctly updated client notification test case

The header parameter is correctly implemented for the SendNotificationToClient test.


693-697: Properly updated prompt handling test cases

The header parameter follows the same implementation pattern used throughout the PR.


751-755: Correctly updated error handling tests

The header parameter has been properly integrated into the invalid message test cases.


876-880: Header parameter properly implemented in handler tests

The undefined handlers test cases have been correctly updated with the header parameter.


962-966: Successfully updated capability tests

The header parameter is consistently added to the HandleMessage calls in the capability test cases.


1043-1044: Proper header implementation in instruction tests

The header parameter has been correctly integrated into the instruction test cases.


1092-1097: Correctly updated resource template tests

Both HandleMessage calls in the resource template tests have been updated with the header parameter, maintaining consistency.

Also applies to: 1117-1119


1292-1298: All hook tests properly updated

All HandleMessage calls in the WithHooks test cases have been correctly updated with the header parameter, ensuring comprehensive testing of the new functionality.

Also applies to: 1301-1305, 1311-1315, 1321-1328


1377-1385: Recovery test cases properly updated

The header parameter has been correctly added to the HandleMessage call in the recovery test case.


142-1385: Overall implementation is consistent and complete

The PR effectively adds header support to all HandleMessage calls throughout the test file. The implementation consistently uses map[string]string{"Authorization": "Bearer test"} as the header value, which provides good test coverage for the new header parameter. This change aligns perfectly with the PR objective of enhancing the mcp.Request with header functionality for business logic in hooks.

The implementation maintains the existing behavior of the tests while adding the new header functionality, which is an excellent approach.

🧰 Tools
🪛 golangci-lint (1.64.8)

342-342: S1040: type assertion to the same type: toolsList already has type mcp.JSONRPCMessage

(gosimple)

@wlxwlxwlx
Copy link
Author

This PR enables the request headers received by the server launched in SSE to be read within the hook function. I have already resolved the conflicts and hope that it can be merged.

@ezynda3 ezynda3 changed the title F/add request header feat: add request header May 1, 2025

This comment was marked as resolved.

Copy link
Collaborator

@rwjblue-glean rwjblue-glean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. It seems somewhat odd to have to have a headers for STDIO clients (conceptually headers don't seem to make sense in that context -- at least not to me).

RE: this

Add the request header to mcp.Request so that users can use the header to handle business logic in the hooks function.

I see a lot of wiring in the tests for adding headers arguments but I don't see a new test explicitly confirming that headers is set on the request in the hooks (e.g. the case you actually care about). Maybe I missed it on review? If not, would you mind adding a new test for this explicitly?

@rwjblue-glean rwjblue-glean added the status: needs submitter response Waiting for feedback from issue opener label May 11, 2025
@wlxwlxwlx
Copy link
Author

I added a testing. I retrieve the token from the header to filter different permissions and allow returning different tools lists.

@pottekkat pottekkat added type: enhancement New feature or enhancement request area: sdk SDK improvements unrelated to MCP specification labels May 16, 2025
@0xNF
Copy link

0xNF commented Jun 2, 2025

Is there any update to this? I'd like to get Auth header to differentiate users for my remote servers, and I'd prefer not to use a fork to do so

@arun11299
Copy link

arun11299 commented Jun 6, 2025

If the only concern is with the naming of the field (i.e Header) then can we close that by renaming it to something like "RequestMetadata" ? I do agree that "Header" is not apt as this is more of a protocol addition rather than transport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: sdk SDK improvements unrelated to MCP specification status: needs submitter response Waiting for feedback from issue opener type: enhancement New feature or enhancement request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants