-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Feat: Dynamic authentication handling in MCPToolset from Adk ReadonlyContext.state using Callback #1198
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
Feat: Dynamic authentication handling in MCPToolset from Adk ReadonlyContext.state using Callback #1198
Conversation
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. |
afd46e2
to
f257b9a
Compare
Hi @seanzhou1023, Apologies for tagging you directly. I noticed you've recently reviewed several of the MCPTools. If you have a moment, could you take a look at this pull request? Your feedback would be greatly appreciated. |
a. fix function call long running id matching logic b. fix error code conversion logic c. add input required and auth required status conversion logic d. add a2a Task/Message to ADK event converter f. set task id and context id from input argument PiperOrigin-RevId: 775860563
Merge google#1509 Fixs: google#1302 Previous PR: google#1450 COPYBARA_INTEGRATE_REVIEW=google#1509 from simonwei97:fix/litellm-gen-config-converting-err 3120887 PiperOrigin-RevId: 775903671
Additionally, few other small changes. * Updated a test fixture to support the latest eval data schema. Somehow I missed doing that previously. * Updated the `evaluation_generator.py` to use `run_async`, instead of `run`. * Also, raise an informed error when dependencies required eval are not installed. * Also, changed the behavior of AgentEvaluator.evaluate method to run all the evals, instead of failing at the first eval metric failure. PiperOrigin-RevId: 775919127
…l naming Merge google#1518 ## Description Fixes [google#1512](google#1512) by updating google_search_tool.py to support new Gemini LIVE model naming ## Changes - Update the model name checking in google_search_tool.py COPYBARA_INTEGRATE_REVIEW=google#1518 from rayira:rayira-patch-1 4c98f88 PiperOrigin-RevId: 775941268
PiperOrigin-RevId: 775975982
PiperOrigin-RevId: 775983689
PiperOrigin-RevId: 775991652
PiperOrigin-RevId: 776026554
Update minified files for adk-web PiperOrigin-RevId: 776176499
…fied (WIP) PiperOrigin-RevId: 776211580
Merge google#1679 Contributing doc says to do the following: ```sh uv sync --extra test --extra eval pytest ./tests/unittests ``` If you follow this the tests will fail: ```sh tests/unittests/a2a/executor/test_task_result_aggregator.py:27: in <module> from a2a.types import Message E ModuleNotFoundError: No module named 'a2a' ``` This makes sense since the `a2a` package is not part of ADK's core dep, it is an extra: https://github.com/google/adk-python/blob/e79651cd86ba3f0c998109f2140f1db2cab78708/pyproject.toml#L79-L83 Thus for a2a tests to pass we must include the extra in the sync command: ```sh uv sync --extra test --extra eval --extra a2a pytest ./tests/unittests ``` COPYBARA_INTEGRATE_REVIEW=google#1679 from jackwotherspoon:main c1a5367 PiperOrigin-RevId: 776617515
New version removed graph.graph we rely on. temporarily fix the version before we fix the issue. PiperOrigin-RevId: 776619611
…nd update the import error message PiperOrigin-RevId: 776631415
…ul error message to ask user upgrade python when python version < 3.10 PiperOrigin-RevId: 776631647
This will fail a PR check if multiple commits are added, and will give the user instructions on how to fix the issue. PiperOrigin-RevId: 776638574
…ontext_id PiperOrigin-RevId: 776639713
Merge google#2109 Fixes google#2105 ## Problem When integrating Google ADK with Langfuse using the @observe decorator, the usage details displayed in Langfuse web UI were incorrect. The root cause was in the telemetry implementation where total_token_count was being mapped to gen_ai.usage.output_tokens instead of candidates_token_count. - Expected mapping: - candidates_token_count → completion_tokens (output tokens) - prompt_token_count → prompt_tokens (input tokens) - Previous incorrect mapping: - total_token_count → completion_tokens (wrong!) - prompt_token_count → prompt_tokens (correct) ## Solution Updated trace_call_llm function in telemetry.py to use candidates_token_count for output token tracking instead of total_token_count, ensuring proper token count reporting to observability tools like Langfuse. ## Testing plan - Updated test expectations in test_telemetry.py - Verified telemetry tests pass - Manual verification with Langfuse integration ## Screenshots **Before** <img width="1187" height="329" alt="Screenshot from 2025-07-22 20-20-33" src="https://github.com/user-attachments/assets/ad5fc957-64a2-4524-bd31-0cebb15a5270" /> **After** <img width="1187" height="329" alt="Screenshot from 2025-07-22 20-21-40" src="https://github.com/user-attachments/assets/3920df2a-be75-47e0-9bd0-f961bb72c838" /> _Notes_: From the screenshot, there's another problem: thoughts_token_count field is not mapped, but this should be another issue imo COPYBARA_INTEGRATE_REVIEW=google#2109 from tl-nguyen:fix-telemetry-token-count-mapping 3d043f5 PiperOrigin-RevId: 786827802
Merge google#1607 - Added CLI functionality so that we can deploy and Agent onto a GKE cluster - Related documentation google/adk-docs#445 COPYBARA_INTEGRATE_REVIEW=google#1607 from vicentefb:GkeDeployAgent 42f35d9 PiperOrigin-RevId: 786857789
Merge google#869 How to reproduce the error: ``` from google.adk.code_executors import UnsafeLocalCodeExecutor from google.adk.code_executors.code_execution_utils import CodeExecutionInput result = UnsafeLocalCodeExecutor().execute_code( invocation_context=None, code_execution_input=CodeExecutionInput( code=''' import math def pi(): return math.pi print(pi()) ''' ) ) print(result) ``` output: ``` CodeExecutionResult(stdout='', stderr="name 'math' is not defined", output_files=[]) ``` COPYBARA_INTEGRATE_REVIEW=google#869 from qieqieplus:main 63f557b PiperOrigin-RevId: 787145189
…n config PiperOrigin-RevId: 787148485
PiperOrigin-RevId: 787164869
…stances PiperOrigin-RevId: 787179405
PiperOrigin-RevId: 787245794
This endpoint could be used by ADK Web to dynamically know: - What are the available eval metrics in an App - A description of those metrics - A value range supported by those metrics We also update the metric registry to make it mandatory to supply these details. The goal is to improve usability and interpretability of the eval metrics. PiperOrigin-RevId: 787277695
PiperOrigin-RevId: 787735915
…o_mcp_passing_env_from_adk_state
….com/pandasanjay/adk-python into stdio_mcp_passing_env_from_adk_state
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
This PR implements dynamic authentication handling in MCPToolset by enabling the passing of authentication information and environment variables from the ADK ReadonlyContext.state to MCP servers at runtime. The key improvement allows for user-specific token management and flexible authentication flows across different MCP transport types (STDIO, SSE, HTTP Stream).
- Adds callback-based authentication and environment variable extraction from session state
- Implements dynamic environment variable injection for STDIO transport connections
- Enhances test coverage with comprehensive unit tests for new authentication and environment functionality
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/google/adk/tools/mcp_tool/mcp_toolset.py |
Core implementation adding callback support and authentication extraction logic |
src/google/adk/tools/mcp_tool/mcp_session_manager.py |
Adds connection parameter update functionality to support dynamic environment injection |
tests/unittests/tools/mcp_tool/test_mcp_toolset_env.py |
Unit tests for environment variable extraction and injection functionality |
tests/unittests/tools/mcp_tool/test_mcp_toolset_auth.py |
Unit tests for authentication extraction callback functionality |
tests/unittests/tools/mcp_tool/test_mcp_tool_auth.py |
Unit tests for MCPTool authentication handling |
tests/integration/test_mcp_env_integration.py |
Integration tests for environment variable functionality |
tests/integration/utils/test_runner.py |
Test runner updates to support async session management |
pyproject.toml |
MCP dependency version update from 1.8.0 to 1.9.4 |
contributing/samples/mcp_stdio_user_auth_passing_sample/ |
Sample demonstrating user token passing via environment variables |
Comments suppressed due to low confidence (3)
tests/integration/utils/test_runner.py:53
- This line is attempting to call create_session synchronously but the method signature expects an async call. The original code structure suggests this should be removed since session creation is now handled lazily in _ensure_session().
self.current_session_id = None
tests/integration/test_mcp_env_integration.py:63
- The attribute name '_context_to_env_mapper_callback' doesn't match the actual attribute name '_get_env_from_context_fn' used in the MCPToolset class. This will cause an AttributeError.
""",
pyproject.toml:41
- The MCP version 1.9.4 may not exist. The current knowledge cutoff is January 2025, and this could be a version that doesn't exist yet or was never released.
"mcp>=1.9.4;python_version>='3.10'", # For MCP Toolset
self._connection_params = new_connection_params | ||
|
||
# Clear existing sessions since connection params changed | ||
# Sessions will be recreated on next request |
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.
The comment mentions clearing sessions but the actual implementation doesn't clear the sessions dictionary. This could lead to stale sessions being reused with outdated connection parameters.
# Sessions will be recreated on next request | |
# Sessions will be recreated on next request | |
self._sessions.clear() |
Copilot uses AI. Check for mistakes.
### Option 1: Run the complete example | ||
```bash | ||
cd /home/sanjay-dev/Workspace/adk-python | ||
python -m contributing.samples.stdio_mcp_user_auth_passing_sample.main |
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.
The module path in the command doesn't match the actual directory name 'mcp_stdio_user_auth_passing_sample'. It should be 'mcp_stdio_user_auth_passing_sample.main'.
python -m contributing.samples.stdio_mcp_user_auth_passing_sample.main | |
python -m contributing.samples.mcp_stdio_user_auth_passing_sample.main |
Copilot uses AI. Check for mistakes.
|
||
## Directory Structure | ||
``` | ||
contributing/samples/stdio_mcp_user_auth_passing_sample/ |
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.
The directory path in the documentation doesn't match the actual directory name 'mcp_stdio_user_auth_passing_sample'. It should be 'mcp_stdio_user_auth_passing_sample/'.
contributing/samples/stdio_mcp_user_auth_passing_sample/ | |
contributing/samples/mcp_stdio_user_auth_passing_sample/ |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
….com/pandasanjay/adk-python into stdio_mcp_passing_env_from_adk_state
9850f83
to
32ee080
Compare
…o_mcp_passing_env_from_adk_state
This changes has been moved to #2208 |
Add authentication information to MCPToolset from Adk ReadonlyContext.state
This PR implements dynamic authentication handling in MCPToolset by passing authentication information (such as user-specific tokens) directly from the Adk session.state. With this approach, user tokens can be injected and sent from the ADK Context state to MCP, supporting user-specific authentication flows.
Key Improvements:
Dynamic Token Management:
Improved Unit Tests: Enhancements for better readability and maintainability.
New Sample: Demonstrates passing user tokens to MCP via environment variables, showcasing the dynamic flow from session.state.
Flexible & Secure Integration: Allows for more flexible and secure integration scenarios, supporting distinct authentication credentials per session or user.
#805 - Partially added ability to pass Authentication Details from ReadonlyContext, if the same JWT token can be reused.
#1402 - Fully support for the STDIO Transport.