Skip to content

fix: MCPToolset does not include authentication information during initialization and tool listing #2173

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 1 commit into
base: main
Choose a base branch
from

Conversation

chyok
Copy link

@chyok chyok commented Jul 25, 2025

PR Description

Summary

This PR adds a redundant get_header method to MCPToolset, with the exact same functionality as the existing get_header in mcp_tool.

This is a minimal-change, fast-fix approach to address the reported authentication issue, enabling us to resolve the bug in stages:

  • Stage 1 (this PR): Introduce get_header in MCPToolset to quickly unblock users and fix the bug.
  • Stage 2 (future work): Refactor by extracting get_header into a shared utility to avoid duplication or refactor the module.

With this PR, the MCPToolset will now correctly handle authentication headers during MCP initialization and tool listing.

Linked Issue

This PR addresses #2168

Testing Plan

Unit Tests

  • All unit tests for the new get_header method have been added under tests/unittests/tools/mcp_tool/test_mcp_toolset.py
image
  • All tests were run using pytest and passed successfully.

Manual End-to-End (E2E) Test

  • Verified the MCPToolset integration using the ADK runner.
  • Confirmed that after this fix, the MCP tool can be initialized and used without authentication errors.

image

Additional Notes

  • This is an interim fix to minimize code changes and risk.
  • In follow-up PRs, we will refactor and deduplicate the get_header method by moving it into a shared utility module.

Copy link

google-cla bot commented Jul 25, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@chyok chyok force-pushed the fix-mcptoolset-initialization-and-list-tool-authentication branch 2 times, most recently from bb1f3cd to 3cdcb31 Compare July 25, 2025 09:47
@chyok chyok force-pushed the fix-mcptoolset-initialization-and-list-tool-authentication branch from ab9fdba to 3d5cadf Compare July 27, 2025 02:56
@chyok
Copy link
Author

chyok commented Jul 28, 2025

Hi @seanzhou1023 , would it be acceptable to allow a small amount of duplicated code, or do we still prefer to extract it into a separate function?
I can try to refactor the logic for credentials to the header, but it might introduce larger changes, which could make the review process more difficult.

@chyok chyok force-pushed the fix-mcptoolset-initialization-and-list-tool-authentication branch from 3d5cadf to ca20316 Compare July 28, 2025 13:36
@chyok chyok force-pushed the fix-mcptoolset-initialization-and-list-tool-authentication branch from ca20316 to 8160486 Compare July 29, 2025 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant