Repository navigation
fix(mcp): send the request's headers when loading MCP resources - #7823
Closed
adityasingh2400 wants to merge 1 commit into
Closed
adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
LoadMcpResourceTool listed and read resources without passing the tool context. McpToolset resolves header_provider and the per-user credential from that context, so resources were fetched unauthenticated while the same toolset's tools were not, and a server that authenticates requests rejected every resource call. Pass tool_context to list_resources and read_resource. Fixes google#7822
Collaborator
|
Thank you @adityasingh2400 for your contribution! 🎉 Your changes have been successfully imported and merged via Copybara in commit 28d4975. Closing this PR as the changes are now in the main branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
LoadMcpResourceToolcalledMcpToolset.list_resources()andread_resource(name)without a context.McpToolset._build_headersonly callsheader_providerwhen it gets a context, and_get_auth_headersuses the context to look up the per-user credential (credential_key). So resources were fetched without the headers the toolset's tools are called with. On a server that authenticates requests, every resource call failed, and the failure was only logged as a warning: the model was never told the resources exist, andload_mcp_resourcereturned nothing.Solution:
Pass
tool_contextasreadonly_contextto both calls, asget_toolsalready does. Resource requests now carry the sameheader_providerheaders and user credential as tool calls, and they reuse the same pooled session.Testing Plan
Unit Tests:
In
tests/unittests/tools/test_load_mcp_resource_tool.py:test_process_llm_request_sends_request_headers: a realMcpToolsetwith aheader_providerand a mocked session manager. It asserts that the provider is called with the tool context and that every session is opened with its headers.test_process_llm_request_with_prior_partsnow expectsread_resource("res1", readonly_context=tool_context).pre-commit runpasses on the changed files, andmypyreports no issues inload_mcp_resource_tool.py.Manual End-to-End (E2E) Tests:
The test server is a streamable-HTTP
MCPServerexposing resourcehandbook, behind middleware that returns 401 withoutAuthorization: Bearer user-token. The client is aRunnerwith anLlmAgentholdingMcpToolset(StreamableHTTPConnectionParams(url=...), header_provider=lambda ctx: {"Authorization": "Bearer user-token"}, use_mcp_resources=True). The scripted model callsload_mcp_resource(resource_names=["handbook"])and then answers. The setup is in #7822.Tool listing worked in both runs, because
get_toolsalready passes the context.Checklist
Additional context
None.