From 1b86d8eafe6190d7388146679f7d5481be9d828b Mon Sep 17 00:00:00 2001 From: Yifan Wang Date: Thu, 21 May 2026 09:45:36 -0700 Subject: [PATCH 01/31] chore: Add single folder support in adk web Change-Id: If593c0118bb44ec4a4dd836d8c5d5c4025591af2 --- README.md | 2 +- .../adk_project_overview_and_architecture.md | 2 +- src/google/adk/cli/cli_tools_click.py | 10 +- src/google/adk/cli/fast_api.py | 21 +- src/google/adk/cli/utils/agent_loader.py | 47 ++++- tests/unittests/cli/test_fast_api.py | 183 ++++++++++++++++++ .../unittests/cli/utils/test_agent_loader.py | 14 +- 7 files changed, 268 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 190a1c1043..4a26756e09 100644 --- a/README.md +++ b/README.md @@ -93,7 +93,7 @@ root_agent = Workflow( # Interactive CLI adk run path/to/my_agent -# Web UI +# Web UI (supports multi-agent directories or pointing directly to a single agent folder) adk web path/to/agents_dir ``` diff --git a/contributing/adk_project_overview_and_architecture.md b/contributing/adk_project_overview_and_architecture.md index aa23af0db1..8c28aff385 100644 --- a/contributing/adk_project_overview_and_architecture.md +++ b/contributing/adk_project_overview_and_architecture.md @@ -83,7 +83,7 @@ async def health_check(): By default, the ADK API server expects an explicit application context in all requests (e.g., via the `/apps/{app_name}/...` path or in the payload body). -However, if the environment variable `ADK_DEFAULT_APP_NAME` is set, the server will automatically resolve and fall back to the default application whenever a request lacks an explicit app name: +However, if the environment variable `ADK_DEFAULT_APP_NAME` is set, or if the server is running in **single agent mode** (when pointing directly to a directory containing an agent instead of a directory of agents), the server will automatically resolve and fall back to that agent as the default application whenever a request lacks an explicit app name. In single agent mode, the local agent takes precedence over the `ADK_DEFAULT_APP_NAME` environment variable. - **URL Path-Rewriting (Production Endpoints)**: Requests to production endpoints that omit the `/apps/{app_name}` prefix (such as `/users/{user_id}/sessions` or `/app-info`) are automatically rewritten by an internal ASGI middleware to target the default application. (Note: `/dev` and `/builder` endpoints are excluded from rewriting). - **Agent Execution & Streaming**: Requests to `/run`, `/run_sse`, or `/run_live` that omit the `app_name` parameter in their payload body or query string will automatically resolve to the default application. diff --git a/src/google/adk/cli/cli_tools_click.py b/src/google/adk/cli/cli_tools_click.py index 8102f16573..c6a71175a1 100644 --- a/src/google/adk/cli/cli_tools_click.py +++ b/src/google/adk/cli/cli_tools_click.py @@ -1774,8 +1774,9 @@ def cli_web( ): """Starts a FastAPI server with Web UI for agents. - AGENTS_DIR: The directory of agents, where each subdirectory is a single - agent, containing at least `__init__.py` and `agent.py` files. + AGENTS_DIR: The directory of agents (where each subdirectory is a single + agent containing `agent.py` or `root_agent.yaml` files) or a path pointing + directly to a single agent folder. Example: @@ -1893,8 +1894,9 @@ def cli_api_server( ): """Starts a FastAPI server for agents. - AGENTS_DIR: The directory of agents, where each subdirectory is a single - agent, containing at least `__init__.py` and `agent.py` files. + AGENTS_DIR: The directory of agents (where each subdirectory is a single + agent containing `agent.py` or `root_agent.yaml` files) or a path pointing + directly to a single agent folder. Example: diff --git a/src/google/adk/cli/fast_api.py b/src/google/adk/cli/fast_api.py index ed99799ca4..6d8893f5d2 100644 --- a/src/google/adk/cli/fast_api.py +++ b/src/google/adk/cli/fast_api.py @@ -44,6 +44,7 @@ from .service_registry import load_services_module from .utils import envs from .utils.agent_change_handler import AgentChangeEventHandler +from .utils.agent_loader import is_single_agent_directory from .utils.base_agent_loader import BaseAgentLoader from .utils.service_factory import _create_task_store_from_options from .utils.service_factory import create_artifact_service_from_options @@ -459,6 +460,16 @@ def get_fast_api_app( config_agent_utils._set_enforce_denylist(True) + # Detect single agent mode + agents_path = Path(agents_dir).resolve() + is_single_agent = is_single_agent_directory(agents_path) + + original_agents_dir = agents_dir + single_agent_name = None + if is_single_agent: + single_agent_name = agents_path.name + agents_dir = str(agents_path.parent) + # Set up eval managers. if eval_storage_uri: from .utils import evals @@ -476,9 +487,11 @@ def get_fast_api_app( ) # initialize Agent Loader if not passed as argument + this_module = sys.modules[__name__] if agent_loader is None: - this_module = sys.modules[__name__] - agent_loader = this_module.AgentLoader(agents_dir) + agent_loader = this_module.AgentLoader(original_agents_dir) + elif is_single_agent and isinstance(agent_loader, this_module.AgentLoader): + agent_loader._set_single_agent_mode(single_agent_name, agents_dir) # Load services.py from agents_dir for custom service registration. load_services_module(agents_dir) @@ -537,6 +550,10 @@ def get_fast_api_app( default_llm_model=default_llm_model, ) + # In single agent mode, use that agent as the default app. + if is_single_agent: + adk_web_server.default_app_name = single_agent_name + # Callbacks & other optional args for when constructing the FastAPI instance extra_fast_api_args = {} diff --git a/src/google/adk/cli/utils/agent_loader.py b/src/google/adk/cli/utils/agent_loader.py index d4bbfc88f6..5a8c6d4c2c 100644 --- a/src/google/adk/cli/utils/agent_loader.py +++ b/src/google/adk/cli/utils/agent_loader.py @@ -39,6 +39,16 @@ logger = logging.getLogger("google_adk." + __name__) + +def is_single_agent_directory(path: Path | str) -> bool: + """Returns True if the directory contains a single agent configuration or file.""" + p = Path(path).resolve() + return ( + p.joinpath("agent.py").is_file() + or p.joinpath("root_agent.yaml").is_file() + ) + + # Special agents directory for agents with names starting with double underscore SPECIAL_AGENTS_DIR = os.path.join( os.path.dirname(__file__), "..", "built_in_agents" @@ -60,10 +70,36 @@ class AgentLoader(BaseAgentLoader): """ def __init__(self, agents_dir: str): - self.agents_dir = str(Path(agents_dir)) + agents_path = Path(agents_dir).resolve() + is_single_agent = is_single_agent_directory(agents_path) + if is_single_agent: + self._is_single_agent = True + self._single_agent_name = agents_path.name + self.agents_dir = str(agents_path.parent) + else: + self._is_single_agent = False + self._single_agent_name = None + self.agents_dir = str(agents_path) + self._original_sys_path = None self._agent_cache: dict[str, Union[BaseAgent, App]] = {} + @property + def is_single_agent(self) -> bool: + """Returns True if the loader is in single agent mode.""" + return self._is_single_agent + + @property + def single_agent_name(self) -> Optional[str]: + """Returns the name of the agent in single agent mode.""" + return self._single_agent_name + + def _set_single_agent_mode(self, name: str, agents_dir: str) -> None: + """Internal method to force single agent mode. Use with care.""" + self._is_single_agent = True + self._single_agent_name = name + self.agents_dir = agents_dir + def _load_from_module_or_package( self, agent_name: str ) -> Optional[Union[BaseAgent, App]]: @@ -204,6 +240,13 @@ def _validate_agent_name(self, agent_name: str) -> None: name_to_check = agent_name check_dir = self.agents_dir + if self._is_single_agent and not agent_name.startswith("__"): + if agent_name != self._single_agent_name: + raise ValueError( + f"Agent not found: {agent_name!r}. In single agent mode, only " + f"'{self._single_agent_name}' is accessible." + ) + if not self._VALID_AGENT_NAME_RE.match(name_to_check): raise ValueError( f"Invalid agent name: {agent_name!r}. Agent names must be valid" @@ -368,6 +411,8 @@ def load_agent(self, agent_name: str) -> Union[BaseAgent, App]: @override def list_agents(self) -> list[str]: """Lists all agents available in the agent loader (sorted alphabetically).""" + if self._is_single_agent: + return [self._single_agent_name] base_path = Path.cwd() / self.agents_dir agent_names = [ x diff --git a/tests/unittests/cli/test_fast_api.py b/tests/unittests/cli/test_fast_api.py index a80644a9b6..9731e3efd5 100755 --- a/tests/unittests/cli/test_fast_api.py +++ b/tests/unittests/cli/test_fast_api.py @@ -2753,5 +2753,188 @@ def test_run_live_websocket_missing_app_name_raises_error( assert exc_info.value.code == 1008 +def test_is_single_agent_directory(tmp_path): + """Verify that is_single_agent_directory only identifies directories with agent.py or root_agent.yaml.""" + from google.adk.cli.utils.agent_loader import is_single_agent_directory + + # Directory with agent.py (should be identified as agent) + agent_py_dir = tmp_path / "agent_py_dir" + agent_py_dir.mkdir() + (agent_py_dir / "agent.py").write_text("root_agent = 'dummy'") + assert is_single_agent_directory(str(agent_py_dir)) is True + + # Directory with root_agent.yaml (should be identified as agent) + yaml_dir = tmp_path / "yaml_dir" + yaml_dir.mkdir() + (yaml_dir / "root_agent.yaml").write_text("root_agent: dummy") + assert is_single_agent_directory(str(yaml_dir)) is True + + # Normal directory or standard package with __init__.py only (should NOT be identified as agent) + normal_pkg = tmp_path / "normal_pkg" + normal_pkg.mkdir() + (normal_pkg / "__init__.py").write_text( + "from .app import App\nimport something" + ) + assert is_single_agent_directory(str(normal_pkg)) is False + + +def test_agent_loader_single_agent_mode(tmp_path): + """Verify that AgentLoader automatically detects and configures single agent mode.""" + agent_folder = tmp_path / "my_test_agent" + agent_folder.mkdir() + (agent_folder / "agent.py").write_text("root_agent = 'dummy'") + + loader = fast_api_module.AgentLoader(str(agent_folder)) + + assert loader._is_single_agent is True + assert loader._single_agent_name == "my_test_agent" + assert loader.agents_dir == str(tmp_path) + assert loader.list_agents() == ["my_test_agent"] + + +def test_single_agent_mode_detection( + tmp_path, + mock_session_service, + mock_artifact_service, + mock_memory_service, + mock_eval_sets_manager, + mock_eval_set_results_manager, +): + """Verify that pointing agents_dir to a single agent folder enables single agent mode.""" + agent_folder = tmp_path / "my_only_agent" + agent_folder.mkdir() + (agent_folder / "agent.py").write_text("root_agent = None") + + with ( + patch.object(signal, "signal", autospec=True, return_value=None), + patch.object( + fast_api_module, + "create_session_service_from_options", + autospec=True, + return_value=mock_session_service, + ), + patch.object( + fast_api_module, + "create_artifact_service_from_options", + autospec=True, + return_value=mock_artifact_service, + ), + patch.object( + fast_api_module, + "create_memory_service_from_options", + autospec=True, + return_value=mock_memory_service, + ), + patch.object( + fast_api_module, + "LocalEvalSetsManager", + autospec=True, + return_value=mock_eval_sets_manager, + ), + patch.object( + fast_api_module, + "LocalEvalSetResultsManager", + autospec=True, + return_value=mock_eval_set_results_manager, + ), + ): + app = get_fast_api_app( + agents_dir=str(agent_folder), + web=True, + session_service_uri="", + artifact_service_uri="", + memory_service_uri="", + allow_origins=None, + a2a=False, + host="127.0.0.1", + port=8000, + ) + client = TestClient(app) + + response = client.get("/list-apps") + assert response.status_code == 200 + assert response.json() == ["my_only_agent"] + + +def test_single_agent_mode_sets_default_app( + tmp_path, + mock_session_service, + mock_artifact_service, + mock_memory_service, + mock_eval_sets_manager, + mock_eval_set_results_manager, + monkeypatch, +): + """Verify that in single agent mode, the agent is used as default app.""" + # Set environment variable to something else, but single mode should take precedence. + monkeypatch.setenv("ADK_DEFAULT_APP_NAME", "some_other_app") + + agent_folder = tmp_path / "my_only_agent" + agent_folder.mkdir() + (agent_folder / "agent.py").write_text("root_agent = None") + + # Setup session data in the in-memory service + async def setup_session(): + await mock_session_service.create_session( + app_name="my_only_agent", + user_id="test_user", + session_id="test_session", + state={}, + ) + + asyncio.run(setup_session()) + + with ( + patch.object(signal, "signal", autospec=True, return_value=None), + patch.object( + fast_api_module, + "create_session_service_from_options", + autospec=True, + return_value=mock_session_service, + ), + patch.object( + fast_api_module, + "create_artifact_service_from_options", + autospec=True, + return_value=mock_artifact_service, + ), + patch.object( + fast_api_module, + "create_memory_service_from_options", + autospec=True, + return_value=mock_memory_service, + ), + patch.object( + fast_api_module, + "LocalEvalSetsManager", + autospec=True, + return_value=mock_eval_sets_manager, + ), + patch.object( + fast_api_module, + "LocalEvalSetResultsManager", + autospec=True, + return_value=mock_eval_set_results_manager, + ), + ): + app = get_fast_api_app( + agents_dir=str(agent_folder), + web=True, + session_service_uri="", + artifact_service_uri="", + memory_service_uri="", + allow_origins=None, + a2a=False, + host="127.0.0.1", + port=8000, + ) + client = TestClient(app) + + # Accessing /users/{user_id}/sessions/{session_id} should work because of rewrite + response = client.get("/users/test_user/sessions/test_session") + assert response.status_code == 200 + assert response.json()["id"] == "test_session" + + if __name__ == "__main__": pytest.main(["-xvs", __file__]) diff --git a/tests/unittests/cli/utils/test_agent_loader.py b/tests/unittests/cli/utils/test_agent_loader.py index 8454323189..4da5f063ff 100644 --- a/tests/unittests/cli/utils/test_agent_loader.py +++ b/tests/unittests/cli/utils/test_agent_loader.py @@ -308,11 +308,21 @@ def test_agent_loader_with_mocked_windows_path(self, monkeypatch): del self windows_path = "C:\\Users\\dev\\agents\\" + class MockWindowsPath(PureWindowsPath): + + def resolve(self): + return self + with monkeypatch.context() as m: m.setattr( agent_loader_module, "Path", - lambda path_str: PureWindowsPath(path_str), + MockWindowsPath, + ) + m.setattr( + agent_loader_module, + "is_single_agent_directory", + lambda path: False, ) loader = AgentLoader(windows_path) @@ -458,7 +468,7 @@ def __init__(self): def test_sys_path_modification(self): """Test that agents_dir is added to sys.path correctly.""" with tempfile.TemporaryDirectory() as temp_dir: - temp_path = Path(temp_dir) + temp_path = Path(temp_dir).resolve() # Create agent self.create_agent_structure(temp_path, "path_agent", "module") From 1f245535ff1107b428f4fb0837db985457ddf024 Mon Sep 17 00:00:00 2001 From: Yifan Wang Date: Thu, 21 May 2026 09:50:25 -0700 Subject: [PATCH 02/31] fix: update EditFileTool to handle cross-platform line breaks and escape regex characters Change-Id: Id155218fa6feb7e004684d8f96cd0598fa4d2766 --- src/google/adk/tools/environment/_tools.py | 11 +- .../tools/environment/test_tools_edit_file.py | 150 ++++++++++++++++++ 2 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 tests/unittests/tools/environment/test_tools_edit_file.py diff --git a/src/google/adk/tools/environment/_tools.py b/src/google/adk/tools/environment/_tools.py index 67baa8f40d..f8e365ae9a 100644 --- a/src/google/adk/tools/environment/_tools.py +++ b/src/google/adk/tools/environment/_tools.py @@ -17,6 +17,7 @@ from __future__ import annotations import logging +import re from typing import Any from typing import Optional from typing import TYPE_CHECKING @@ -381,7 +382,13 @@ async def run_async( except FileNotFoundError: return {'status': 'error', 'error': f'File not found: {path}'} - count = content.count(old_string) + # Normalize line breaks in old_string to \n and use regex for flexible matching + normalized_old = old_string.replace('\r\n', '\n') + pattern = re.escape(normalized_old).replace('\n', '\r?\n') + + matches = re.findall(pattern, content) + count = len(matches) + if count == 0: return { 'status': 'error', @@ -399,6 +406,6 @@ async def run_async( ), } - new_content = content.replace(old_string, new_string, 1) + new_content = re.sub(pattern, lambda m: new_string, content, count=1) await self._environment.write_file(path, new_content) return {'status': 'ok', 'message': f'Edited {path}'} diff --git a/tests/unittests/tools/environment/test_tools_edit_file.py b/tests/unittests/tools/environment/test_tools_edit_file.py new file mode 100644 index 0000000000..d96dcb873a --- /dev/null +++ b/tests/unittests/tools/environment/test_tools_edit_file.py @@ -0,0 +1,150 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for EditFileTool. + +Verifies that EditFileTool correctly handles line break differences. +""" + +from pathlib import Path + +from google.adk.environment._local_environment import LocalEnvironment +from google.adk.tools.environment._tools import EditFileTool +import pytest +import pytest_asyncio + + +@pytest_asyncio.fixture(name="env") +async def _env(tmp_path: Path): + """Create and initialize a LocalEnvironment backed by a temp directory.""" + environment = LocalEnvironment(working_dir=tmp_path) + await environment.initialize() + yield environment + await environment.close() + + +class TestEditFileTool: + """Tests for EditFileTool behavior.""" + + @pytest.mark.asyncio + async def test_edit_file_handles_line_breaks_linux_file_windows_search( + self, env: LocalEnvironment + ): + """File has \\n, search string has \\r\\n.""" + # Arrange + tool = EditFileTool(env) + await env.write_file("test.txt", "line1\nline2\nline3") + + args = { + "path": "test.txt", + "old_string": "line1\r\nline2", + "new_string": "line1_replaced\nline2_replaced", + } + + # Act + result = await tool.run_async(args=args, tool_context=None) + + # Assert + assert result["status"] == "ok" + data = await env.read_file("test.txt") + assert data == b"line1_replaced\nline2_replaced\nline3" + + @pytest.mark.asyncio + async def test_edit_file_handles_line_breaks_windows_file_linux_search( + self, env: LocalEnvironment + ): + """File has \\r\\n, search string has \\n.""" + # Arrange + tool = EditFileTool(env) + await env.write_file("test.txt", "line1\r\nline2\r\nline3") + + args = { + "path": "test.txt", + "old_string": "line1\nline2", + "new_string": "line1_replaced\r\nline2_replaced", + } + + # Act + result = await tool.run_async(args=args, tool_context=None) + + # Assert + assert result["status"] == "ok" + data = await env.read_file("test.txt") + assert data == b"line1_replaced\r\nline2_replaced\r\nline3" + + @pytest.mark.asyncio + async def test_edit_file_fails_on_multiple_matches( + self, env: LocalEnvironment + ): + """Tool fails if old_string appears multiple times.""" + # Arrange + tool = EditFileTool(env) + await env.write_file("test.txt", "line1\nline2\nline1\nline2") + + args = { + "path": "test.txt", + "old_string": "line1\nline2", + "new_string": "replaced", + } + + # Act + result = await tool.run_async(args=args, tool_context=None) + + # Assert + assert result["status"] == "error" + assert "appears 2 times" in result["error"] + + @pytest.mark.asyncio + async def test_edit_file_exact_match_works(self, env: LocalEnvironment): + """Exact match works as before.""" + # Arrange + tool = EditFileTool(env) + await env.write_file("test.txt", "line1\nline2\nline3") + + args = { + "path": "test.txt", + "old_string": "line1\nline2", + "new_string": "replaced", + } + + # Act + result = await tool.run_async(args=args, tool_context=None) + + # Assert + assert result["status"] == "ok" + data = await env.read_file("test.txt") + assert data == b"replaced\nline3" + + @pytest.mark.asyncio + async def test_edit_file_handles_special_regex_chars( + self, env: LocalEnvironment + ): + """Special regex characters in old_string are escaped.""" + # Arrange + tool = EditFileTool(env) + await env.write_file("test.txt", "line1.content\nline2") + + args = { + "path": "test.txt", + "old_string": "line1.content", + "new_string": "replaced", + } + + # Act + result = await tool.run_async(args=args, tool_context=None) + + # Assert + assert result["status"] == "ok" + data = await env.read_file("test.txt") + assert data == b"replaced\nline2" From 7e38fc811ed58235aa5c120c48c419e0f10a8de2 Mon Sep 17 00:00:00 2001 From: George Weale Date: Thu, 21 May 2026 20:21:04 +0000 Subject: [PATCH 03/31] fix: resolve circular import caused by llm_request Change-Id: Icfa51c1323a751921bb35000cd6aababe9033c09 --- src/google/adk/telemetry/_experimental_semconv.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/google/adk/telemetry/_experimental_semconv.py b/src/google/adk/telemetry/_experimental_semconv.py index 6ffd5beaaa..a415d72de8 100644 --- a/src/google/adk/telemetry/_experimental_semconv.py +++ b/src/google/adk/telemetry/_experimental_semconv.py @@ -45,8 +45,9 @@ from opentelemetry.trace import Span from opentelemetry.util.types import AttributeValue -from ..models.llm_request import LlmRequest -from ..models.llm_response import LlmResponse +if TYPE_CHECKING: + from ..models.llm_request import LlmRequest + from ..models.llm_response import LlmResponse try: from opentelemetry.semconv._incubating.attributes.gen_ai_attributes import GEN_AI_TOOL_DEFINITIONS From 416775dcceae8f57badf0cd55c44148f6426b6db Mon Sep 17 00:00:00 2001 From: asobran Date: Fri, 22 May 2026 14:59:08 +0000 Subject: [PATCH 04/31] fix: Make google-cloud-storage import lazy in skill utils Change-Id: Id86475d867e31e3f8d3ee1429f2b8a7b8c53da34 --- src/google/adk/skills/_utils.py | 16 +++++++++++++- tests/unittests/skills/test__utils.py | 32 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/google/adk/skills/_utils.py b/src/google/adk/skills/_utils.py index 3270025e01..bbe0280f37 100644 --- a/src/google/adk/skills/_utils.py +++ b/src/google/adk/skills/_utils.py @@ -24,7 +24,6 @@ import zipfile from google.auth import credentials as auth -from google.cloud import storage from pydantic import ValidationError import yaml @@ -407,6 +406,14 @@ def _list_skills_in_gcs_dir( Returns: Dictionary mapping skill IDs to their frontmatter. """ + try: + from google.cloud import storage + except ImportError as e: + raise ImportError( + "google-cloud-storage is required to list skills in GCS. Install it" + " with `pip install google-cloud-storage` or `pip install google-adk[gcp]`." + ) from e + client = storage.Client(project=project_id, credentials=credentials) bucket = client.bucket(bucket_name) @@ -466,6 +473,13 @@ def _load_skill_from_gcs_dir( ValueError: If SKILL.md is invalid or the skill name does not match the directory name. """ + try: + from google.cloud import storage + except ImportError as e: + raise ImportError( + "google-cloud-storage is required to load skills from GCS. Install it" + " with `pip install google-cloud-storage` or `pip install google-adk[gcp]`." + ) from e client = storage.Client(project=project_id, credentials=credentials) bucket = client.bucket(bucket_name) diff --git a/tests/unittests/skills/test__utils.py b/tests/unittests/skills/test__utils.py index 1975c0a494..0547c630a5 100644 --- a/tests/unittests/skills/test__utils.py +++ b/tests/unittests/skills/test__utils.py @@ -14,7 +14,9 @@ """Unit tests for skill utilities.""" +import builtins import io +import sys from unittest import mock import zipfile @@ -363,3 +365,33 @@ def test__load_skill_from_zip_bytes(): assert skill.instructions == "Body instructions" assert skill.resources.get_reference("ref1.md") == "ref1 content" assert skill.resources.get_script("script1.sh").src == "echo hello" + + +def test__list_skills_in_gcs_dir_import_error(): + """Tests list_skills_in_gcs_dir raises ImportError when storage missing.""" + real_import = builtins.__import__ + + def mock_import(name, globals=None, locals=None, fromlist=(), level=0): + if name == "google.cloud" and "storage" in (fromlist or ()): + raise ImportError("No module named 'google.cloud.storage'") + return real_import(name, globals, locals, fromlist, level) + + with mock.patch("builtins.__import__", mock_import): + with pytest.raises(ImportError, match="google-cloud-storage is required"): + _list_skills_in_gcs_dir("my-bucket", "skills/") + + +def test__load_skill_from_gcs_dir_import_error(): + """Tests load_skill_from_gcs_dir raises ImportError when storage missing.""" + real_import = builtins.__import__ + + def mock_import(name, globals=None, locals=None, fromlist=(), level=0): + if name == "google.cloud" and "storage" in (fromlist or ()): + raise ImportError("No module named 'google.cloud.storage'") + return real_import(name, globals, locals, fromlist, level) + + with mock.patch("builtins.__import__", mock_import): + with pytest.raises(ImportError, match="google-cloud-storage is required"): + _load_skill_from_gcs_dir("my-bucket", "skills/my-skill/") + + From 5f91a9db032bb9ffd0b59ba24aad04b680481c98 Mon Sep 17 00:00:00 2001 From: Sasha Sobran Date: Thu, 21 May 2026 14:42:25 +0000 Subject: [PATCH 05/31] fix: lazy-import GCS evaluation managers in evals utility Fixes https://github.com/google/adk-python/issues/5787 Change-Id: I0aadfb11fedc49c1d45d4fcd9f89087c27a0e6ea --- pyproject.toml | 1 + src/google/adk/cli/utils/evals.py | 22 ++++++++++++++++++---- tests/unittests/cli/utils/test_evals.py | 4 ++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 43ee273184..cd87d3735c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,6 +47,7 @@ dependencies = [ "packaging>=21", "pydantic>=2.12,<3", "python-dotenv>=1,<2", + "python-multipart>=0.0.9,<1", # go/keep-sorted start "pyyaml>=6.0.2,<7", "requests>=2.32.4,<3", diff --git a/src/google/adk/cli/utils/evals.py b/src/google/adk/cli/utils/evals.py index ff3c148637..d2ca8878f3 100644 --- a/src/google/adk/cli/utils/evals.py +++ b/src/google/adk/cli/utils/evals.py @@ -15,6 +15,7 @@ from __future__ import annotations import os +from typing import Any, TYPE_CHECKING from pydantic import alias_generators from pydantic import BaseModel @@ -22,10 +23,12 @@ from ...evaluation.eval_case import Invocation from ...evaluation.evaluation_generator import EvaluationGenerator -from ...evaluation.gcs_eval_set_results_manager import GcsEvalSetResultsManager -from ...evaluation.gcs_eval_sets_manager import GcsEvalSetsManager from ...sessions.session import Session +if TYPE_CHECKING: + from ...evaluation.gcs_eval_set_results_manager import GcsEvalSetResultsManager + from ...evaluation.gcs_eval_sets_manager import GcsEvalSetsManager + class GcsEvalManagers(BaseModel): model_config = ConfigDict( @@ -34,9 +37,9 @@ class GcsEvalManagers(BaseModel): arbitrary_types_allowed=True, ) - eval_sets_manager: GcsEvalSetsManager + eval_sets_manager: 'GcsEvalSetsManager' - eval_set_results_manager: GcsEvalSetResultsManager + eval_set_results_manager: 'GcsEvalSetResultsManager' def convert_session_to_eval_invocations(session: Session) -> list[Invocation]: @@ -66,8 +69,19 @@ def create_gcs_eval_managers_from_uri( Raises: ValueError: If the eval_storage_uri is not supported. + RuntimeError: If GCP optional dependencies are missing. """ if eval_storage_uri.startswith('gs://'): + try: + from ...evaluation.gcs_eval_set_results_manager import GcsEvalSetResultsManager + from ...evaluation.gcs_eval_sets_manager import GcsEvalSetsManager + except ImportError as e: + raise RuntimeError( + 'GCS evaluation managers require Google Cloud optional dependencies.\n' + 'Please install them using: pip install google-adk[gcp]\n' + 'Or: pip install google-cloud-storage>=2.18' + ) from e + gcs_bucket = eval_storage_uri.split('://')[1] eval_sets_manager = GcsEvalSetsManager( bucket_name=gcs_bucket, project=os.environ['GOOGLE_CLOUD_PROJECT'] diff --git a/tests/unittests/cli/utils/test_evals.py b/tests/unittests/cli/utils/test_evals.py index 84214859c5..071feb1e2d 100644 --- a/tests/unittests/cli/utils/test_evals.py +++ b/tests/unittests/cli/utils/test_evals.py @@ -25,11 +25,11 @@ @mock.patch.dict(os.environ, {'GOOGLE_CLOUD_PROJECT': 'test-project'}) @mock.patch( - 'google.adk.cli.utils.evals.GcsEvalSetResultsManager', + 'google.adk.evaluation.gcs_eval_set_results_manager.GcsEvalSetResultsManager', autospec=True, ) @mock.patch( - 'google.adk.cli.utils.evals.GcsEvalSetsManager', + 'google.adk.evaluation.gcs_eval_sets_manager.GcsEvalSetsManager', autospec=True, ) def test_create_gcs_eval_managers_from_uri_success( From 9fd0e07ff81bf0ddb2b11a2d3553e06f87411d27 Mon Sep 17 00:00:00 2001 From: Copybara Date: Fri, 22 May 2026 14:33:01 +0000 Subject: [PATCH 06/31] chore: Support test runs on v1 pull requests Change-Id: Ifcfa08f31513e6b25023dbe81d3de70645912303 --- .github/workflows/pre-commit.yml | 4 ++-- .github/workflows/python-unit-tests.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 23a032f87a..f18020a86b 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -16,13 +16,13 @@ name: Pre-commit Checks on: push: - branches: [main, v2] + branches: [main, v1, v2] paths: - '**.py' - '.pre-commit-config.yaml' - 'pyproject.toml' pull_request: - branches: [main, v2] + branches: [main, v1, v2] paths: - '**.py' - '.pre-commit-config.yaml' diff --git a/.github/workflows/python-unit-tests.yml b/.github/workflows/python-unit-tests.yml index 77d5781ead..fcedce9acc 100644 --- a/.github/workflows/python-unit-tests.yml +++ b/.github/workflows/python-unit-tests.yml @@ -2,9 +2,9 @@ name: Python Unit Tests on: push: - branches: [ main ] + branches: [main, v1, v2] pull_request: - branches: [ main, v2 ] + branches: [main, v1, v2] permissions: contents: read From 92cf19255e21a6edc08a5cf09a1cfbe936b5690e Mon Sep 17 00:00:00 2001 From: Kathy Wu Date: Thu, 21 May 2026 21:57:54 +0000 Subject: [PATCH 07/31] fix: Resolve circular import in base_tool Importing `ToolContext` at the top level in `base_tool.py` created a circular import cycle when `google.adk.tools.function_tool` was imported. The cycle was: `base_tool` -> `tool_context` -> `CallbackContext` -> `agents` -> `BaseAgent` -> `Event` -> `LlmResponse` -> `LlmRequest` -> `BaseTool`. Moving `ToolContext` to `TYPE_CHECKING` breaks this runtime cycle, using `from __future__ import annotations` to preserve type hints. Change-Id: I34731f46cf5aa4665ad3c6604495ad5fd84f8aa5 --- src/google/adk/tools/base_tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/google/adk/tools/base_tool.py b/src/google/adk/tools/base_tool.py index c131750de5..3525ca21c0 100644 --- a/src/google/adk/tools/base_tool.py +++ b/src/google/adk/tools/base_tool.py @@ -33,13 +33,13 @@ from ..utils.variant_utils import get_google_llm_variant from ..utils.variant_utils import GoogleLLMVariant -from .tool_context import ToolContext logger = logging.getLogger("google_adk." + __name__) if TYPE_CHECKING: from ..models.llm_request import LlmRequest from .tool_configs import ToolArgsConfig + from .tool_context import ToolContext SelfTool = TypeVar("SelfTool", bound="BaseTool") From a4f394e1396aaa53874c9aeb4f8ec17565bc769a Mon Sep 17 00:00:00 2001 From: George Weale Date: Fri, 22 May 2026 17:45:54 +0000 Subject: [PATCH 08/31] chore: edit files Change-Id: I4e3e3e9f1846520f3fda86cc83453b30bdd336ef --- contributing/samples/integrations/gepa/OWNERS | 3 -- .../fixture/context_update_test/OWNERS | 1 - tests/unittests/tools/BUILD | 46 ------------------- 3 files changed, 50 deletions(-) delete mode 100644 contributing/samples/integrations/gepa/OWNERS delete mode 100644 tests/integration/fixture/context_update_test/OWNERS delete mode 100644 tests/unittests/tools/BUILD diff --git a/contributing/samples/integrations/gepa/OWNERS b/contributing/samples/integrations/gepa/OWNERS deleted file mode 100644 index 66db12a593..0000000000 --- a/contributing/samples/integrations/gepa/OWNERS +++ /dev/null @@ -1,3 +0,0 @@ -aarg -jief -paulxz diff --git a/tests/integration/fixture/context_update_test/OWNERS b/tests/integration/fixture/context_update_test/OWNERS deleted file mode 100644 index 02f72c401c..0000000000 --- a/tests/integration/fixture/context_update_test/OWNERS +++ /dev/null @@ -1 +0,0 @@ -gkcng diff --git a/tests/unittests/tools/BUILD b/tests/unittests/tools/BUILD deleted file mode 100644 index 48c28cb449..0000000000 --- a/tests/unittests/tools/BUILD +++ /dev/null @@ -1,46 +0,0 @@ -load("//third_party/py/pytest:pytest_defs.bzl", "pytest_test") - -package( - default_applicable_licenses = ["//third_party/py/google/adk:package_license"], - default_visibility = ["//visibility:private"], -) - -pytest_test( - name = "test_local_environment", - srcs = ["test_local_environment.py"], - args = [ - "-p", - "pytest_asyncio.plugin", - ], - deps = [ - "//third_party/py/google/adk", - "//third_party/py/pytest_asyncio", - ], -) - -pytest_test( - name = "test_skill_toolset", - srcs = ["test_skill_toolset.py"], - args = [ - "-p", - "pytest_asyncio.plugin", - ], - deps = [ - "//third_party/py/google/adk", - "//third_party/py/google/genai", - "//third_party/py/pytest_asyncio", - ], -) - -pytest_test( - name = "test_environment_toolset", - srcs = ["test_environment_toolset.py"], - args = [ - "-p", - "pytest_asyncio.plugin", - ], - deps = [ - "//third_party/py/google/adk", - "//third_party/py/pytest_asyncio", - ], -) From eb379bea5b87579d5a649698c3fdd7473ac5e5a2 Mon Sep 17 00:00:00 2001 From: Google Team Member Date: Thu, 21 May 2026 16:40:57 -0700 Subject: [PATCH 09/31] feat: Add user.id to gen_ai.user.message log records for telemetry Merge: https://github.com/google/adk-python/pull/5796 ORIGINAL_AUTHOR=Achuth Narayan Rajagopal GitOrigin-RevId: ec8265ecab7ae4342f8a6b5605f112e3a3673775 Change-Id: Ib77c4e2ad3df29a7ca734f142b9dc42d0890f429 --- .../adk/telemetry/_experimental_semconv.py | 8 +- src/google/adk/telemetry/tracing.py | 60 ++++++-- .../telemetry/test_node_functional.py | 2 - tests/unittests/telemetry/test_spans.py | 130 +++++++++++++++++- 4 files changed, 182 insertions(+), 18 deletions(-) diff --git a/src/google/adk/telemetry/_experimental_semconv.py b/src/google/adk/telemetry/_experimental_semconv.py index a415d72de8..8d81339cc0 100644 --- a/src/google/adk/telemetry/_experimental_semconv.py +++ b/src/google/adk/telemetry/_experimental_semconv.py @@ -434,8 +434,14 @@ def _to_system_instructions( def set_operation_details_common_attributes( operation_details_common_attributes: MutableMapping[str, AttributeValue], attributes: Mapping[str, AttributeValue], -): + log_only_attributes: Mapping[str, AttributeValue] | None = None, +) -> None: operation_details_common_attributes.update(attributes) + if log_only_attributes and get_content_capturing_mode() in ( + 'EVENT_ONLY', + 'SPAN_AND_EVENT', + ): + operation_details_common_attributes.update(log_only_attributes) async def set_operation_details_attributes_from_request( diff --git a/src/google/adk/telemetry/tracing.py b/src/google/adk/telemetry/tracing.py index 7b813be16d..2b06135ad9 100644 --- a/src/google/adk/telemetry/tracing.py +++ b/src/google/adk/telemetry/tracing.py @@ -66,6 +66,7 @@ from .. import version from ..utils.model_name_utils import is_gemini_model +from ._experimental_semconv import get_content_capturing_mode from ._experimental_semconv import is_experimental_semconv from ._experimental_semconv import maybe_log_completion_details from ._experimental_semconv import set_operation_details_attributes_from_request @@ -556,20 +557,26 @@ def use_generate_content_span( common_attributes = { GEN_AI_AGENT_NAME: invocation_context.agent.name, GEN_AI_CONVERSATION_ID: invocation_context.session.id, - USER_ID: invocation_context.session.user_id, 'gcp.vertex.agent.event_id': model_response_event.id, 'gcp.vertex.agent.invocation_id': invocation_context.invocation_id, } + log_only_common_attributes = {} + if invocation_context.session.user_id is not None: + log_only_common_attributes[USER_ID] = invocation_context.session.user_id if ( _is_gemini_agent(invocation_context.agent) and _instrumented_with_opentelemetry_instrumentation_google_genai() ): - with _use_extra_generate_content_attributes(common_attributes): + with _use_extra_generate_content_attributes( + common_attributes, + log_only_extra_attributes=log_only_common_attributes, + ): yield else: with _use_native_generate_content_span_stable_semconv( llm_request=llm_request, common_attributes=common_attributes, + log_only_common_attributes=log_only_common_attributes, ) as span: yield span.span @@ -590,24 +597,32 @@ async def use_inference_span( common_attributes = { GEN_AI_AGENT_NAME: invocation_context.agent.name, GEN_AI_CONVERSATION_ID: invocation_context.session.id, - USER_ID: invocation_context.session.user_id, 'gcp.vertex.agent.event_id': model_response_event.id, 'gcp.vertex.agent.invocation_id': invocation_context.invocation_id, } + log_only_common_attributes = {} + if invocation_context.session.user_id is not None: + log_only_common_attributes[USER_ID] = invocation_context.session.user_id if ( _is_gemini_agent(invocation_context.agent) and _instrumented_with_opentelemetry_instrumentation_google_genai() ): - with _use_extra_generate_content_attributes(common_attributes): + with _use_extra_generate_content_attributes( + common_attributes, + log_only_extra_attributes=log_only_common_attributes, + ): yield else: async with _use_native_generate_content_span( llm_request=llm_request, common_attributes=common_attributes, + log_only_common_attributes=log_only_common_attributes, ) as gc_span: if is_experimental_semconv(): set_operation_details_common_attributes( - gc_span.operation_details_common_attributes, common_attributes + gc_span.operation_details_common_attributes, + common_attributes, + log_only_attributes=log_only_common_attributes, ) try: yield gc_span @@ -664,6 +679,7 @@ def _instrumented_with_opentelemetry_instrumentation_google_genai() -> bool: @contextmanager def _use_extra_generate_content_attributes( extra_attributes: Mapping[str, AttributeValue], + log_only_extra_attributes: Mapping[str, AttributeValue] | None = None, ): try: from opentelemetry.instrumentation.google_genai import GENERATE_CONTENT_EXTRA_ATTRIBUTES_CONTEXT_KEY @@ -675,13 +691,25 @@ def _use_extra_generate_content_attributes( + ' Please upgrade to version to 0.6b0 or above.' ) yield + return - tok = otel_context.attach( - otel_context.set_value( - GENERATE_CONTENT_EXTRA_ATTRIBUTES_CONTEXT_KEY, extra_attributes - ) + ctx = otel_context.set_value( + GENERATE_CONTENT_EXTRA_ATTRIBUTES_CONTEXT_KEY, extra_attributes ) + if log_only_extra_attributes: + try: + from opentelemetry.instrumentation.google_genai import GENERATE_CONTENT_EVENT_ONLY_EXTRA_ATTRIBUTES_CONTEXT_KEY + + ctx = otel_context.set_value( + GENERATE_CONTENT_EVENT_ONLY_EXTRA_ATTRIBUTES_CONTEXT_KEY, + log_only_extra_attributes, + context=ctx, + ) + except (ImportError, AttributeError): + pass + + tok = otel_context.attach(ctx) try: yield finally: @@ -713,6 +741,7 @@ def _set_common_generate_content_attributes( def _use_native_generate_content_span_stable_semconv( llm_request: LlmRequest, common_attributes: Mapping[str, AttributeValue], + log_only_common_attributes: Mapping[str, AttributeValue] | None = None, ) -> Iterator[GenerateContentSpan]: with tracer.start_as_current_span( f"generate_content {llm_request.model or ''}" @@ -734,12 +763,18 @@ def _use_native_generate_content_span_stable_semconv( attributes={GEN_AI_SYSTEM: _guess_gemini_system_name()}, ) ) + user_message_attributes = {GEN_AI_SYSTEM: _guess_gemini_system_name()} + if _should_log_prompt_response_content() and log_only_common_attributes: + user_id = log_only_common_attributes.get(USER_ID) + if user_id is not None: + user_message_attributes[USER_ID] = user_id + for content in llm_request.contents: otel_logger.emit( LogRecord( event_name='gen_ai.user.message', body={'content': _serialize_content_with_elision(content)}, - attributes={GEN_AI_SYSTEM: _guess_gemini_system_name()}, + attributes=user_message_attributes, ) ) @@ -750,10 +785,13 @@ def _use_native_generate_content_span_stable_semconv( async def _use_native_generate_content_span( llm_request: LlmRequest, common_attributes: Mapping[str, AttributeValue], + log_only_common_attributes: Mapping[str, AttributeValue] | None = None, ) -> AsyncIterator[GenerateContentSpan]: if not is_experimental_semconv(): with _use_native_generate_content_span_stable_semconv( - llm_request, common_attributes + llm_request, + common_attributes, + log_only_common_attributes=log_only_common_attributes, ) as gc_span: yield gc_span return diff --git a/tests/unittests/telemetry/test_node_functional.py b/tests/unittests/telemetry/test_node_functional.py index 63bb7d0688..661cbb3d45 100644 --- a/tests/unittests/telemetry/test_node_functional.py +++ b/tests/unittests/telemetry/test_node_functional.py @@ -255,7 +255,6 @@ async def some_node(ctx, node_input): ), 'gen_ai.request.model': 'mock', 'gen_ai.system': 'gemini', - 'user.id': 'some_user', }, children=[ SpanDigest( @@ -327,7 +326,6 @@ async def some_node(ctx, node_input): ), 'gen_ai.request.model': 'mock', 'gen_ai.system': 'gemini', - 'user.id': 'some_user', }, ), ], diff --git a/tests/unittests/telemetry/test_spans.py b/tests/unittests/telemetry/test_spans.py index c0e4cc20b9..c42c6d725c 100644 --- a/tests/unittests/telemetry/test_spans.py +++ b/tests/unittests/telemetry/test_spans.py @@ -27,6 +27,7 @@ from google.adk.sessions.in_memory_session_service import InMemorySessionService from google.adk.telemetry._experimental_semconv import _safe_json_serialize_no_whitespaces from google.adk.telemetry.tracing import _safe_json_serialize +from google.adk.telemetry.tracing import _use_extra_generate_content_attributes from google.adk.telemetry.tracing import ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS from google.adk.telemetry.tracing import GCP_MCP_SERVER_DESTINATION_ID from google.adk.telemetry.tracing import trace_agent_invocation @@ -810,12 +811,14 @@ async def test_trace_send_data_disabling_request_response_content( return_value='test_system', ) @pytest.mark.parametrize('capture_content', [True, False]) +@pytest.mark.parametrize('user_id', ['some-user-id', None]) async def test_generate_content_span( mock_guess_system_name, mock_tracer, mock_otel_logger, monkeypatch, capture_content, + user_id, ): """Test native generate_content span creation with attributes and logs.""" # Arrange @@ -830,7 +833,7 @@ async def test_generate_content_span( agent = LlmAgent(name='test_agent', model='not-a-gemini-model') invocation_context = await _create_invocation_context(agent) - + invocation_context.session.user_id = user_id system_instruction = types.Content( parts=[types.Part.from_text(text='You are a helpful assistant.')], ) @@ -889,11 +892,15 @@ async def test_generate_content_span( mock_span.set_attributes.assert_called_once_with({ GEN_AI_AGENT_NAME: invocation_context.agent.name, GEN_AI_CONVERSATION_ID: invocation_context.session.id, - USER_ID: invocation_context.session.user_id, 'gcp.vertex.agent.event_id': 'event-123', 'gcp.vertex.agent.invocation_id': invocation_context.invocation_id, }) + all_set_attribute_keys = [ + call.args[0] for call in mock_span.set_attribute.call_args_list + ] + assert USER_ID not in all_set_attribute_keys + # Assert Logs assert mock_otel_logger.emit.call_count == 4 @@ -932,8 +939,11 @@ async def test_generate_content_span( assert len(user_logs) == 2 assert expected_user1_body == user_logs[0].body assert expected_user2_body == user_logs[1].body + expected_user_log_attributes = {GEN_AI_SYSTEM: 'test_system'} + if capture_content and user_id is not None: + expected_user_log_attributes[USER_ID] = user_id for log in user_logs: - assert log.attributes == {GEN_AI_SYSTEM: 'test_system'} + assert log.attributes == expected_user_log_attributes choice_log = next( (lr for lr in log_records if lr.event_name == 'gen_ai.choice'), @@ -944,6 +954,52 @@ async def test_generate_content_span( assert choice_log.attributes == {GEN_AI_SYSTEM: 'test_system'} +@pytest.mark.asyncio +@mock.patch( + 'google.adk.telemetry.tracing._use_extra_generate_content_attributes' +) +async def test_generate_content_span_with_genai_instrumentation( + mock_use_extra, + monkeypatch, +): + """Test that genai-instrumentation delegation branch does not forward USER_ID in attributes.""" + monkeypatch.setattr( + 'google.adk.telemetry.tracing._instrumented_with_opentelemetry_instrumentation_google_genai', + lambda: True, + ) + # _is_gemini_agent returns true for gemini models. + agent = LlmAgent(name='test_agent', model='gemini-1.5-pro') + invocation_context = await _create_invocation_context(agent) + + llm_request = LlmRequest( + model='gemini-1.5-pro', + contents=[types.Content(role='user', parts=[types.Part(text='Hello')])], + ) + + model_response_event = mock.MagicMock() + model_response_event.id = 'event-123' + + mock_cm = mock.MagicMock() + mock_use_extra.return_value = mock_cm + + async with use_inference_span( + llm_request, invocation_context, model_response_event + ): + pass + + mock_use_extra.assert_called_once() + args, _ = mock_use_extra.call_args + common_attributes = args[0] + + assert GEN_AI_AGENT_NAME in common_attributes + assert GEN_AI_CONVERSATION_ID in common_attributes + assert 'gcp.vertex.agent.event_id' in common_attributes + assert 'gcp.vertex.agent.invocation_id' in common_attributes + + # USER_ID should NOT be in common_attributes passed to the genai instrumentor + assert USER_ID not in common_attributes + + def _mock_callable_tool(): """Description of some tool.""" return 'result' @@ -1001,12 +1057,14 @@ def _mock_tool_dict() -> types.ToolDict: 'capture_content', ['SPAN_AND_EVENT', 'EVENT_ONLY', 'SPAN_ONLY', 'NO_CONTENT'], ) +@pytest.mark.parametrize('user_id', ['some-user-id', None]) async def test_generate_content_span_with_experimental_semconv( mock_guess_system_name, mock_tracer, mock_otel_logger, monkeypatch, capture_content, + user_id, ): """Test native generate_content span creation with attributes and logs with experimental semconv enabled.""" # Arrange @@ -1025,6 +1083,7 @@ async def test_generate_content_span_with_experimental_semconv( agent = LlmAgent(name='test_agent', model='not-a-gemini-model') invocation_context = await _create_invocation_context(agent) + invocation_context.session.user_id = user_id system_instruction = types.Content( parts=[types.Part.from_text(text='You are a helpful assistant.')], @@ -1209,11 +1268,15 @@ async def test_generate_content_span_with_experimental_semconv( mock_span.set_attributes.assert_called_once_with({ GEN_AI_AGENT_NAME: invocation_context.agent.name, GEN_AI_CONVERSATION_ID: invocation_context.session.id, - USER_ID: invocation_context.session.user_id, 'gcp.vertex.agent.event_id': 'event-123', 'gcp.vertex.agent.invocation_id': invocation_context.invocation_id, }) + all_set_attribute_keys = [ + call.args[0] for call in mock_span.set_attribute.call_args_list + ] + assert USER_ID not in all_set_attribute_keys + if capture_content in ['SPAN_AND_EVENT', 'SPAN_ONLY']: mock_span.set_attribute.assert_any_call( GEN_AI_SYSTEM_INSTRUCTIONS, @@ -1260,6 +1323,15 @@ async def test_generate_content_span_with_experimental_semconv( attributes = operation_details_log.attributes + if ( + capture_content in ['EVENT_ONLY', 'SPAN_AND_EVENT'] + and user_id is not None + ): + assert USER_ID in attributes + assert attributes[USER_ID] == user_id + else: + assert USER_ID not in attributes + if capture_content in ['SPAN_AND_EVENT', 'EVENT_ONLY']: assert GEN_AI_SYSTEM_INSTRUCTIONS in attributes assert ( @@ -1397,3 +1469,53 @@ def test_safe_json_serialize_no_whitespaces_circular_dict_returns_not_serializab obj = {} obj['self'] = obj assert _safe_json_serialize_no_whitespaces(obj) == '' + + +def test_use_extra_generate_content_attributes_upgraded_version(monkeypatch): + # Arrange: Mock the presence of the new event-only context key in the contrib module + from opentelemetry.instrumentation import google_genai + + mock_event_only_key = 'MOCKED_EVENT_ONLY_EXTRA_ATTRIBUTES_CONTEXT_KEY' + monkeypatch.setattr( + google_genai, + 'GENERATE_CONTENT_EVENT_ONLY_EXTRA_ATTRIBUTES_CONTEXT_KEY', + mock_event_only_key, + raising=False, + ) + + # Act: Run the helper with mock.patch on the otel context + with mock.patch('opentelemetry.context.set_value') as mock_set_value: + with _use_extra_generate_content_attributes( + extra_attributes={'span.attr': 'value'}, + log_only_extra_attributes={USER_ID: 'user_123'}, + ): + pass + + # Assert: Verify set_value was called with the mocked event-only key + mock_set_value.assert_any_call( + mock_event_only_key, + {USER_ID: 'user_123'}, + context=mock.ANY, + ) + + +def test_use_extra_generate_content_attributes_older_version(monkeypatch): + # Arrange: Simulate an older version by deleting the key if present + from opentelemetry.instrumentation import google_genai + + if hasattr( + google_genai, 'GENERATE_CONTENT_EVENT_ONLY_EXTRA_ATTRIBUTES_CONTEXT_KEY' + ): + monkeypatch.delattr( + google_genai, 'GENERATE_CONTENT_EVENT_ONLY_EXTRA_ATTRIBUTES_CONTEXT_KEY' + ) + + # Act & Assert: Ensure execution does not throw any ImportError/AttributeError + try: + with _use_extra_generate_content_attributes( + extra_attributes={'span.attr': 'value'}, + log_only_extra_attributes={USER_ID: 'user_123'}, + ): + pass + except Exception as e: # pylint: disable=broad-exception-caught + pytest.fail(f'Graceful degradation failed: {e}') From 68785f53cf307bb0903ec7e4d34e6a30b2e7207b Mon Sep 17 00:00:00 2001 From: George Weale Date: Fri, 22 May 2026 18:23:35 +0000 Subject: [PATCH 10/31] chore: edit docs Change-Id: I47379cb495725595571ae5274ad34d8201ba1ca3 --- tests/integration/fixture/bigquery_agent/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/fixture/bigquery_agent/README.md b/tests/integration/fixture/bigquery_agent/README.md index 57b580a25e..34456fb68e 100644 --- a/tests/integration/fixture/bigquery_agent/README.md +++ b/tests/integration/fixture/bigquery_agent/README.md @@ -12,7 +12,7 @@ 1. Change to the current directory: ```shell - cd third_party/py/google/adk/tests/integration/fixture/bigquery_agent/ + cd tests/integration/fixture/bigquery_agent/ ``` 1. Customize the evaluation dataset to the environment `GOOGLE_CLOUD_PROJECT` by replacing the placeholder to the real project set in your environment: @@ -47,7 +47,7 @@ 1. Change to the directory containing agent folder: ```shell - cd third_party/py/google/adk/tests/integration/fixture/ + cd tests/integration/fixture/ ``` 1. Run the following command to start the ADK web app: From e56c021ef73193b24023494d853ac4fdab9115bb Mon Sep 17 00:00:00 2001 From: Google Team Member Date: Thu, 21 May 2026 16:41:00 -0700 Subject: [PATCH 11/31] feat: Fix error message telemetry for tool calls Merge: https://github.com/google/adk-python/pull/5797 ORIGINAL_AUTHOR=Achuth Narayan Rajagopal GitOrigin-RevId: 5c41f0352b2a2852eda1b1a80b746477ba4f61de Change-Id: I07edf4c4fff92e37228f876f16cf53310146a257 --- src/google/adk/flows/llm_flows/functions.py | 58 +++- src/google/adk/telemetry/_instrumentation.py | 2 + src/google/adk/telemetry/tracing.py | 7 + src/google/adk/tools/bash_tool.py | 6 + .../adk/tools/discovery_engine_search_tool.py | 6 + src/google/adk/tools/environment/_tools.py | 24 ++ src/google/adk/tools/function_tool.py | 6 + src/google/adk/tools/google_tool.py | 6 + src/google/adk/tools/mcp_tool/mcp_tool.py | 6 + .../openapi_spec_parser/rest_api_tool.py | 6 + src/google/adk/tools/skill_toolset.py | 22 ++ .../flows/llm_flows/test_functions_simple.py | 265 ++++++++++++++++++ tests/unittests/telemetry/test_spans.py | 242 ++++++++++++++++ 13 files changed, 654 insertions(+), 2 deletions(-) diff --git a/src/google/adk/flows/llm_flows/functions.py b/src/google/adk/flows/llm_flows/functions.py index 682a53fc94..259d40b6b6 100644 --- a/src/google/adk/flows/llm_flows/functions.py +++ b/src/google/adk/flows/llm_flows/functions.py @@ -68,6 +68,48 @@ _TOOL_THREAD_POOL_LOCK = threading.Lock() +def _detect_error_type_for_telemetry( + tool: BaseTool, + tool_context: ToolContext, + function_response: Any, +) -> Optional[str]: + """Detects an error type from a tool response for telemetry purposes. + + This does not modify the response. `_detect_error_in_response` is an optional + per-tool hook accessed via `getattr` to avoid adding a public API on + `BaseTool`. Any exception raised by the detector is logged and swallowed so + that telemetry logic never breaks tool execution. + + Args: + tool: The tool whose response is being inspected. + tool_context: The tool context for the current invocation. Detection is + skipped when the tool is requesting auth or confirmation. + function_response: The raw response returned by the tool. + + Returns: + The error type string reported by the tool's `_detect_error_in_response` + hook, or `None` if no error was detected, no hook is defined, or the hook + raised an exception. + """ + try: + if ( + tool_context.actions.requested_auth_configs + or tool_context.actions.requested_tool_confirmations + ): + return None + detector = getattr(tool, '_detect_error_in_response', None) + if detector is None: + return None + return detector(function_response) + except Exception: # pylint: disable=broad-exception-caught + # Never let telemetry logic break tool execution. + logger.exception( + 'Error while detecting error type for telemetry from tool %r.', + getattr(tool, 'name', tool), + ) + return None + + def _is_live_request_queue_annotation(param: inspect.Parameter) -> bool: """Check whether a parameter is annotated as LiveRequestQueue. @@ -482,6 +524,7 @@ async def _run_on_tool_error_callbacks( function_args = ( copy.deepcopy(function_call.args) if function_call.args else {} ) + detected_error_type: Optional[str] = None tool_context = _create_tool_context( invocation_context, function_call, tool_confirmation @@ -505,7 +548,7 @@ async def _run_on_tool_error_callbacks( raise tool_error async def _run_with_trace(): - nonlocal function_args + nonlocal function_args, detected_error_type # Step 1: Check if plugin before_tool_callback overrides the function # response. @@ -586,6 +629,10 @@ async def _run_with_trace(): # the tool returned nothing. return None + detected_error_type = _detect_error_type_for_telemetry( + tool, tool_context, function_response + ) + # Note: State deltas are not applied here - they are collected in # tool_context.actions.state_delta and applied later when the session # service processes the events @@ -600,6 +647,7 @@ async def _run_with_trace(): tool, agent, function_args ) as tel_ctx: tel_ctx.function_response_event = await _run_with_trace() + tel_ctx.error_type = detected_error_type return tel_ctx.function_response_event @@ -718,6 +766,7 @@ async def _run_on_tool_error_callbacks( function_args = ( copy.deepcopy(function_call.args) if function_call.args else {} ) + detected_error_type: Optional[str] = None tool_context = _create_tool_context(invocation_context, function_call) @@ -738,7 +787,7 @@ async def _run_on_tool_error_callbacks( raise tool_error async def _run_with_trace(): - nonlocal function_args + nonlocal function_args, detected_error_type # Do not use "args" as the variable name, because it is a reserved keyword # in python debugger. @@ -827,6 +876,10 @@ async def _run_with_trace(): # build when the tool returned nothing. return None + detected_error_type = _detect_error_type_for_telemetry( + tool, tool_context, function_response + ) + # Note: State deltas are not applied here - they are collected in # tool_context.actions.state_delta and applied later when the session # service processes the events @@ -841,6 +894,7 @@ async def _run_with_trace(): tool, agent, function_args ) as tel_ctx: tel_ctx.function_response_event = await _run_with_trace() + tel_ctx.error_type = detected_error_type return tel_ctx.function_response_event diff --git a/src/google/adk/telemetry/_instrumentation.py b/src/google/adk/telemetry/_instrumentation.py index 975b553c7a..1a3e6a8c38 100644 --- a/src/google/adk/telemetry/_instrumentation.py +++ b/src/google/adk/telemetry/_instrumentation.py @@ -68,6 +68,7 @@ class TelemetryContext: otel_context: context_api.Context function_response_event: event_lib.Event | None = None + error_type: str | None = None def _record_agent_metrics( @@ -148,6 +149,7 @@ async def record_tool_execution( args=function_args, function_response_event=response_event, error=caught_error, + error_type=tel_ctx.error_type, ) finally: try: diff --git a/src/google/adk/telemetry/tracing.py b/src/google/adk/telemetry/tracing.py index 2b06135ad9..a9cd57c1dd 100644 --- a/src/google/adk/telemetry/tracing.py +++ b/src/google/adk/telemetry/tracing.py @@ -172,6 +172,7 @@ def trace_tool_call( function_response_event: Event | None, error: Exception | None = None, span: Span | None = None, + error_type: str | None = None, ): """Traces tool call. @@ -181,6 +182,10 @@ def trace_tool_call( function_response_event: The event with the function response details. error: The exception raised during tool execution, if any. span: The span to record attributes on. If None, uses current span. + error_type: An error type string detected from the tool's response dict + (e.g., "HTTP_ERROR", "MCP_TOOL_ERROR"). Used when the tool returned an + error as a dict rather than raising an exception. Ignored if `error` is + also set (exception takes precedence). """ span = span or trace.get_current_span() @@ -197,6 +202,8 @@ def trace_tool_call( span.set_attribute(ERROR_TYPE, str(error.error_type)) else: span.set_attribute(ERROR_TYPE, type(error).__name__) + elif error_type is not None: + span.set_attribute(ERROR_TYPE, error_type) # Special case for client side association with a remote tool call if ( diff --git a/src/google/adk/tools/bash_tool.py b/src/google/adk/tools/bash_tool.py index 89de3bf34f..fa1925205d 100644 --- a/src/google/adk/tools/bash_tool.py +++ b/src/google/adk/tools/bash_tool.py @@ -247,3 +247,9 @@ async def run_async( "stdout": stdout_res, "stderr": stderr_res, } + + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get("error"): + return "TOOL_ERROR" + return None diff --git a/src/google/adk/tools/discovery_engine_search_tool.py b/src/google/adk/tools/discovery_engine_search_tool.py index 54603e6a83..eea843c35f 100644 --- a/src/google/adk/tools/discovery_engine_search_tool.py +++ b/src/google/adk/tools/discovery_engine_search_tool.py @@ -220,6 +220,12 @@ def discovery_engine_search( except GoogleAPICallError as e: return {'status': 'error', 'error_message': str(e)} + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None + def _do_search( self, query: str, diff --git a/src/google/adk/tools/environment/_tools.py b/src/google/adk/tools/environment/_tools.py index f8e365ae9a..ad339cf48f 100644 --- a/src/google/adk/tools/environment/_tools.py +++ b/src/google/adk/tools/environment/_tools.py @@ -137,6 +137,12 @@ async def run_async( result['error'] = f'Command timed out after {DEFAULT_TIMEOUT}s.' return result + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None + @experimental class ReadFileTool(BaseTool): @@ -261,6 +267,12 @@ async def run_async( except Exception as e: return {'status': 'error', 'error': str(e)} + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None + @experimental class WriteFileTool(BaseTool): @@ -312,6 +324,12 @@ async def run_async( return {'status': 'error', 'error': str(e)} return {'status': 'ok', 'message': f'Wrote {path}'} + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None + @experimental class EditFileTool(BaseTool): @@ -409,3 +427,9 @@ async def run_async( new_content = re.sub(pattern, lambda m: new_string, content, count=1) await self._environment.write_file(path, new_content) return {'status': 'ok', 'message': f'Edited {path}'} + + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None diff --git a/src/google/adk/tools/function_tool.py b/src/google/adk/tools/function_tool.py index c5db706d8b..6477fd6894 100644 --- a/src/google/adk/tools/function_tool.py +++ b/src/google/adk/tools/function_tool.py @@ -255,6 +255,12 @@ async def run_async( return await self._invoke_callable(self.func, args_to_call) + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('error'): + return 'TOOL_ERROR' + return None + async def _invoke_callable( self, target: Callable[..., Any], args_to_call: dict[str, Any] ) -> Any: diff --git a/src/google/adk/tools/google_tool.py b/src/google/adk/tools/google_tool.py index c6ab67bb9c..f4294b76cf 100644 --- a/src/google/adk/tools/google_tool.py +++ b/src/google/adk/tools/google_tool.py @@ -106,6 +106,12 @@ async def run_async( "error_details": str(ex), } + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get("status") == "ERROR": + return "TOOL_ERROR" + return None + async def _run_async_with_credential( self, credentials: Credentials, diff --git a/src/google/adk/tools/mcp_tool/mcp_tool.py b/src/google/adk/tools/mcp_tool/mcp_tool.py index 6a24651f92..4acc4ff847 100644 --- a/src/google/adk/tools/mcp_tool/mcp_tool.py +++ b/src/google/adk/tools/mcp_tool/mcp_tool.py @@ -461,6 +461,12 @@ async def _run_async_impl( ) return result + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get("isError"): + return "MCP_TOOL_ERROR" + return None + def _resolve_progress_callback( self, tool_context: ToolContext ) -> Optional[ProgressFnT]: diff --git a/src/google/adk/tools/openapi_tool/openapi_spec_parser/rest_api_tool.py b/src/google/adk/tools/openapi_tool/openapi_spec_parser/rest_api_tool.py index fa32ce932a..9516536a94 100644 --- a/src/google/adk/tools/openapi_tool/openapi_spec_parser/rest_api_tool.py +++ b/src/google/adk/tools/openapi_tool/openapi_spec_parser/rest_api_tool.py @@ -554,6 +554,12 @@ async def call( self._logger.debug("API Response (non-JSON): %s", response.text) return {"text": response.text} # Return text if not JSON + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get("error"): + return "HTTP_ERROR" + return None + def __str__(self): return ( f'RestApiTool(name="{self.name}", description="{self.description}",' diff --git a/src/google/adk/tools/skill_toolset.py b/src/google/adk/tools/skill_toolset.py index ef579d8256..7d8e06368e 100644 --- a/src/google/adk/tools/skill_toolset.py +++ b/src/google/adk/tools/skill_toolset.py @@ -24,6 +24,7 @@ import logging import mimetypes from typing import Any +from typing import Optional from typing import TYPE_CHECKING import warnings @@ -249,6 +250,13 @@ async def run_async( "frontmatter": skill.frontmatter.model_dump(), } + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get("error"): + error_code = response.get("error_code") + return error_code if error_code else "TOOL_ERROR" + return None + @experimental(FeatureName.SKILL_TOOLSET) class LoadSkillResourceTool(BaseTool): @@ -361,6 +369,13 @@ async def run_async( "content": content, } + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get("error"): + error_code = response.get("error_code") + return error_code if error_code else "TOOL_ERROR" + return None + @override async def process_llm_request( self, *, tool_context: ToolContext, llm_request: Any @@ -873,6 +888,13 @@ async def run_async( positional_args, # pylint: disable=protected-access ) + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get("error"): + error_code = response.get("error_code") + return error_code if error_code else "TOOL_ERROR" + return None + @experimental(FeatureName.SKILL_TOOLSET) class SkillToolset(BaseToolset): diff --git a/tests/unittests/flows/llm_flows/test_functions_simple.py b/tests/unittests/flows/llm_flows/test_functions_simple.py index be638de44c..71834d2c01 100644 --- a/tests/unittests/flows/llm_flows/test_functions_simple.py +++ b/tests/unittests/flows/llm_flows/test_functions_simple.py @@ -15,8 +15,11 @@ import asyncio from typing import Any from typing import Callable +from unittest import mock +from fastapi.openapi.models import HTTPBearer from google.adk.agents.llm_agent import Agent +from google.adk.auth.auth_tool import AuthConfig from google.adk.events.event import Event from google.adk.events.event_actions import EventActions from google.adk.events.ui_widget import UiWidget @@ -24,8 +27,10 @@ from google.adk.flows.llm_flows.functions import handle_function_calls_async from google.adk.flows.llm_flows.functions import handle_function_calls_live from google.adk.flows.llm_flows.functions import merge_parallel_function_response_events +from google.adk.tools.base_tool import BaseTool from google.adk.tools.computer_use.computer_use_tool import ComputerUseTool from google.adk.tools.function_tool import FunctionTool +from google.adk.tools.tool_confirmation import ToolConfirmation from google.adk.tools.tool_context import ToolContext from google.genai import types import pytest @@ -1319,3 +1324,263 @@ def simple_fn() -> dict[str, str]: assert result_parallel is not None assert result_parallel.live_session_id == 'test-live-session-id-parallel' + + +class _MockControlSignalTool(BaseTool): + """A tool that simulates requesting confirmation or OAuth authentication.""" + + def __init__(self, name: str, behavior: str): + super().__init__(name=name, description='Simulated control tool') + self.behavior = behavior + + async def run_async(self, *, args, tool_context): + if self.behavior == 'confirm': + tool_context.actions.requested_tool_confirmations = { + 'fc_test_confirm': ToolConfirmation(hint='Authorize execution?') + } + return {'error': 'This tool requires user approval.'} + elif self.behavior == 'auth': + tool_context.actions.requested_auth_configs = { + 'fc_test_auth': AuthConfig(auth_scheme=HTTPBearer()) + } + return {'error': 'Please complete OAuth setup.'} + + def _detect_error_in_response(self, response: Any) -> str | None: + if isinstance(response, dict) and 'error' in response: + return 'TOOL_ERROR' + return None + + +class _ErrorDetectingTool(BaseTool): + """A test tool whose _detect_error_in_response raises an exception.""" + + async def run_async(self, *, args, tool_context): + return {'result': 'result'} + + def _detect_error_in_response(self, response: Any) -> str | None: + raise RuntimeError('detection exploded') + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + 'handle_function_calls', + [ + (handle_function_calls_async), + (handle_function_calls_live), + ], +) +@pytest.mark.parametrize( + 'mock_response,expected_error_type', + [ + ({'error': 'Internal component timeout'}, 'TOOL_ERROR'), + ({'result': 'Execution succeeded'}, None), + ], + ids=['dict_error_recorded', 'success_dict_ignored'], +) +async def test_e2e_telemetry_error_classification( + monkeypatch, handle_function_calls, mock_response, expected_error_type +): + """E2E: asserts that tool outputs successfully translate to targeted OTel span error attributes.""" + recorded_calls = [] + + # Intercept trace_tool_call to capture final telemetry state + monkeypatch.setattr( + 'google.adk.telemetry._instrumentation.tracing.trace_tool_call', + lambda **kw: recorded_calls.append(kw), + ) + + tool = FunctionTool(func=lambda: mock_response) + model = testing_utils.MockModel.create(responses=[]) + agent = Agent(name='test_agent', model=model, tools=[tool]) + + invocation_context = await testing_utils.create_invocation_context( + agent=agent, user_content='' + ) + + function_call = types.FunctionCall(name=tool.name, args={}, id='fc_test') + event = Event( + invocation_id=invocation_context.invocation_id, + author=agent.name, + content=types.Content(parts=[types.Part(function_call=function_call)]), + ) + + await handle_function_calls(invocation_context, event, {tool.name: tool}) + + assert len(recorded_calls) == 1 + assert recorded_calls[0]['error_type'] == expected_error_type + assert recorded_calls[0]['error'] is None + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + 'handle_function_calls', + [ + (handle_function_calls_async), + (handle_function_calls_live), + ], +) +async def test_exception_takes_precedence_over_dict_error( + monkeypatch, handle_function_calls +): + """End-to-end integration: exception takes strict precedence over manual dict error_type.""" + recorded_calls = [] + monkeypatch.setattr( + 'google.adk.telemetry._instrumentation.tracing.trace_tool_call', + lambda **kw: recorded_calls.append(kw), + ) + + def mock_crashing_func(): + raise ValueError('Fatal arithmetic error') + + tool = FunctionTool(func=mock_crashing_func) + model = testing_utils.MockModel.create(responses=[]) + agent = Agent(name='test_agent', model=model, tools=[tool]) + invocation_context = await testing_utils.create_invocation_context( + agent=agent, user_content='' + ) + + function_call = types.FunctionCall( + name=tool.name, args={}, id='fc_test_exception' + ) + event = Event( + invocation_id=invocation_context.invocation_id, + author=agent.name, + content=types.Content(parts=[types.Part(function_call=function_call)]), + ) + + with pytest.raises(ValueError, match='Fatal arithmetic error'): + await handle_function_calls(invocation_context, event, {tool.name: tool}) + + assert len(recorded_calls) == 1 + assert isinstance(recorded_calls[0]['error'], ValueError) + assert recorded_calls[0]['error_type'] is None + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + 'handle_function_calls', + [ + (handle_function_calls_async), + (handle_function_calls_live), + ], +) +async def test_detection_skipped_when_confirmation_requested( + monkeypatch, handle_function_calls +): + """E2E confirmation verification: control prompt avoids polluting telemetry with TOOL_ERROR.""" + recorded_calls = [] + monkeypatch.setattr( + 'google.adk.telemetry._instrumentation.tracing.trace_tool_call', + lambda **kw: recorded_calls.append(kw), + ) + + tool = _MockControlSignalTool(name='confirm_tool', behavior='confirm') + model = testing_utils.MockModel.create(responses=[]) + agent = Agent(name='test_agent', model=model, tools=[tool]) + invocation_context = await testing_utils.create_invocation_context( + agent=agent, user_content='' + ) + + function_call = types.FunctionCall( + name=tool.name, args={}, id='fc_test_confirm' + ) + event = Event( + invocation_id=invocation_context.invocation_id, + author=agent.name, + content=types.Content(parts=[types.Part(function_call=function_call)]), + ) + + await handle_function_calls(invocation_context, event, {tool.name: tool}) + + assert len(recorded_calls) == 1 + assert recorded_calls[0]['error_type'] is None + assert recorded_calls[0]['error'] is None + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + 'handle_function_calls', + [ + (handle_function_calls_async), + (handle_function_calls_live), + ], +) +async def test_detection_skipped_when_auth_requested( + monkeypatch, handle_function_calls +): + """E2E OAuth verification: authenticate control prompt avoids polluting telemetry with TOOL_ERROR.""" + recorded_calls = [] + monkeypatch.setattr( + 'google.adk.telemetry._instrumentation.tracing.trace_tool_call', + lambda **kw: recorded_calls.append(kw), + ) + + tool = _MockControlSignalTool(name='auth_tool', behavior='auth') + model = testing_utils.MockModel.create(responses=[]) + agent = Agent(name='test_agent', model=model, tools=[tool]) + invocation_context = await testing_utils.create_invocation_context( + agent=agent, user_content='' + ) + + function_call = types.FunctionCall(name=tool.name, args={}, id='fc_test_auth') + event = Event( + invocation_id=invocation_context.invocation_id, + author=agent.name, + content=types.Content(parts=[types.Part(function_call=function_call)]), + ) + + await handle_function_calls(invocation_context, event, {tool.name: tool}) + + assert len(recorded_calls) == 1 + assert recorded_calls[0]['error_type'] is None + assert recorded_calls[0]['error'] is None + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + 'handle_function_calls', + [ + (handle_function_calls_async), + (handle_function_calls_live), + ], +) +async def test_detection_exception_does_not_break_tool_call( + monkeypatch, handle_function_calls +): + """Safety Verification: telemetry errors during error parsing are caught cleanly, not crashing tool calls.""" + recorded_calls = [] + monkeypatch.setattr( + 'google.adk.telemetry._instrumentation.tracing.trace_tool_call', + lambda **kw: recorded_calls.append(kw), + ) + + tool = _ErrorDetectingTool( + name='buggy_telemetry_tool', description='raises on tel' + ) + model = testing_utils.MockModel.create(responses=[]) + agent = Agent(name='test_agent', model=model, tools=[tool]) + invocation_context = await testing_utils.create_invocation_context( + agent=agent, user_content='' + ) + + function_call = types.FunctionCall( + name=tool.name, args={}, id='fc_test_buggy' + ) + event = Event( + invocation_id=invocation_context.invocation_id, + author=agent.name, + content=types.Content(parts=[types.Part(function_call=function_call)]), + ) + + result_event = await handle_function_calls( + invocation_context, event, {tool.name: tool} + ) + + assert result_event is not None + assert result_event.content.parts[0].function_response.response == { + 'result': 'result' + } + + assert len(recorded_calls) == 1 + assert recorded_calls[0]['error_type'] is None + assert recorded_calls[0]['error'] is None diff --git a/tests/unittests/telemetry/test_spans.py b/tests/unittests/telemetry/test_spans.py index c42c6d725c..109e77a369 100644 --- a/tests/unittests/telemetry/test_spans.py +++ b/tests/unittests/telemetry/test_spans.py @@ -1519,3 +1519,245 @@ def test_use_extra_generate_content_attributes_older_version(monkeypatch): pass except Exception as e: # pylint: disable=broad-exception-caught pytest.fail(f'Graceful degradation failed: {e}') + + +# --------------------------------------------------------------------------- +# Tests for _detect_error_in_response +# --------------------------------------------------------------------------- + + +class _ErrorDetectingTool(BaseTool): + """A test tool whose _detect_error_in_response raises.""" + + async def run_async(self, *, args, tool_context): + return 'result' + + def _detect_error_in_response(self, response: Any) -> Optional[str]: + raise RuntimeError('detection exploded') + + +def test_base_tool_does_not_define_detect_error_in_response(): + """BaseTool intentionally does not expose _detect_error_in_response as a public hook.""" + tool = SimpleTestTool(name='t', description='d') + # The hook is opt-in per subclass; BaseTool itself must not declare it so + # that telemetry callers can use getattr(...) to skip detection. + assert not hasattr(tool, '_detect_error_in_response') + + +def test_detect_error_function_tool_error(): + from google.adk.tools.function_tool import FunctionTool + + tool = FunctionTool(func=lambda: None) + assert ( + tool._detect_error_in_response({'error': 'missing arg'}) == 'TOOL_ERROR' + ) + + +def test_detect_error_function_tool_no_error(): + from google.adk.tools.function_tool import FunctionTool + + tool = FunctionTool(func=lambda: None) + assert tool._detect_error_in_response({'result': 'ok'}) is None + assert tool._detect_error_in_response('plain string') is None + assert tool._detect_error_in_response(None) is None + + +def test_detect_error_rest_api_tool(): + from google.adk.tools.openapi_tool.openapi_spec_parser.rest_api_tool import RestApiTool + + tool = RestApiTool.__new__(RestApiTool) + assert ( + tool._detect_error_in_response({'error': 'Status Code: 404'}) + == 'HTTP_ERROR' + ) + assert tool._detect_error_in_response({'result': 'ok'}) is None + assert tool._detect_error_in_response({'text': 'html response'}) is None + + +def test_detect_error_mcp_tool(): + from google.adk.tools.mcp_tool.mcp_tool import McpTool as AdkMcpTool + + tool = AdkMcpTool.__new__(AdkMcpTool) + assert ( + tool._detect_error_in_response({'isError': True, 'content': []}) + == 'MCP_TOOL_ERROR' + ) + assert ( + tool._detect_error_in_response({'isError': False, 'content': []}) is None + ) + assert tool._detect_error_in_response({'content': [{'text': 'ok'}]}) is None + + +def test_detect_error_google_tool(): + from google.adk.tools.google_tool import GoogleTool + + tool = GoogleTool.__new__(GoogleTool) + assert ( + tool._detect_error_in_response( + {'status': 'ERROR', 'error_details': 'fail'} + ) + == 'TOOL_ERROR' + ) + assert tool._detect_error_in_response({'status': 'OK', 'data': []}) is None + assert ( + tool._detect_error_in_response({'error': 'something'}) is None + ) # GoogleTool checks status, not error key + + +def test_detect_error_bash_tool(): + from google.adk.tools.bash_tool import ExecuteBashTool + + tool = ExecuteBashTool.__new__(ExecuteBashTool) + assert ( + tool._detect_error_in_response({'error': 'Execution failed'}) + == 'TOOL_ERROR' + ) + assert ( + tool._detect_error_in_response( + {'error': 'timeout', 'stdout': '', 'stderr': ''} + ) + == 'TOOL_ERROR' + ) + assert ( + tool._detect_error_in_response({'stdout': 'ok', 'returncode': 0}) is None + ) + + +def _environment_tool_classes(): + from google.adk.tools.environment._tools import EditFileTool + from google.adk.tools.environment._tools import ExecuteTool + from google.adk.tools.environment._tools import ReadFileTool + from google.adk.tools.environment._tools import WriteFileTool + + return [ExecuteTool, ReadFileTool, WriteFileTool, EditFileTool] + + +@pytest.mark.parametrize( + 'cls', + _environment_tool_classes(), + ids=lambda c: c.__name__, +) +@pytest.mark.parametrize( + 'response,expected', + [ + ({'status': 'error', 'error': 'fail'}, 'TOOL_ERROR'), + ({'status': 'ok', 'message': 'done'}, None), + # Environment tools check status, not the error key. + ({'error': 'something'}, None), + ], + ids=['status_error', 'status_ok', 'error_key_only'], +) +def test_detect_error_environment_tools(cls, response, expected): + tool = cls.__new__(cls) + assert tool._detect_error_in_response(response) == expected + + +@pytest.mark.parametrize( + 'cls_name', + ['LoadSkillTool', 'LoadSkillResourceTool', 'RunSkillScriptTool'], +) +@pytest.mark.parametrize( + 'response,expected', + [ + ( + {'error': 'missing', 'error_code': 'INVALID_ARGUMENTS'}, + 'INVALID_ARGUMENTS', + ), + ({'error': 'generic'}, 'TOOL_ERROR'), + ({'skill_name': 'x', 'instructions': 'y'}, None), + ], + ids=['with_error_code', 'error_no_code', 'no_error'], +) +def test_detect_error_skill_tools(cls_name, response, expected): + skill_toolset = pytest.importorskip('google.adk.tools.skill_toolset') + cls = getattr(skill_toolset, cls_name) + tool = cls.__new__(cls) + assert tool._detect_error_in_response(response) == expected + + +def test_detect_error_discovery_engine_search_tool(): + mod = pytest.importorskip('google.adk.tools.discovery_engine_search_tool') + DiscoveryEngineSearchTool = mod.DiscoveryEngineSearchTool + + tool = DiscoveryEngineSearchTool.__new__(DiscoveryEngineSearchTool) + assert ( + tool._detect_error_in_response( + {'status': 'error', 'error_message': 'fail'} + ) + == 'TOOL_ERROR' + ) + assert tool._detect_error_in_response({'status': 'ok', 'results': []}) is None + + +# --------------------------------------------------------------------------- +# Tests for trace_tool_call with error_type parameter +# --------------------------------------------------------------------------- + + +def test_trace_tool_call_with_error_type( + monkeypatch, mock_span_fixture, mock_tool_fixture +): + """error_type sets the span error.type attribute when no exception.""" + monkeypatch.setattr( + 'opentelemetry.trace.get_current_span', lambda: mock_span_fixture + ) + + trace_tool_call( + tool=mock_tool_fixture, + args={'x': 1}, + function_response_event=None, + error=None, + error_type='HTTP_ERROR', + ) + + mock_span_fixture.set_attribute.assert_any_call('error.type', 'HTTP_ERROR') + + +def test_trace_tool_call_error_takes_precedence_over_error_type( + monkeypatch, mock_span_fixture, mock_tool_fixture +): + """When both error and error_type are provided, error takes precedence.""" + monkeypatch.setattr( + 'opentelemetry.trace.get_current_span', lambda: mock_span_fixture + ) + + trace_tool_call( + tool=mock_tool_fixture, + args={'x': 1}, + function_response_event=None, + error=ValueError('boom'), + error_type='HTTP_ERROR', + ) + + # ValueError should be set, not HTTP_ERROR. + mock_span_fixture.set_attribute.assert_any_call('error.type', 'ValueError') + error_type_calls = [ + c + for c in mock_span_fixture.set_attribute.call_args_list + if c == mock.call('error.type', mock.ANY) + ] + assert len(error_type_calls) == 1 + + +def test_trace_tool_call_no_error_no_error_type( + monkeypatch, mock_span_fixture, mock_tool_fixture +): + """When neither error nor error_type is set, no error.type attribute.""" + monkeypatch.setattr( + 'opentelemetry.trace.get_current_span', lambda: mock_span_fixture + ) + + trace_tool_call( + tool=mock_tool_fixture, + args={'x': 1}, + function_response_event=None, + error=None, + error_type=None, + ) + + error_type_calls = [ + c + for c in mock_span_fixture.set_attribute.call_args_list + if c == mock.call('error.type', mock.ANY) + ] + assert len(error_type_calls) == 0 From ecb759cc16bc870aa190d01c4c3f0e1e3e97afab Mon Sep 17 00:00:00 2001 From: Xuan Yang Date: Fri, 22 May 2026 12:14:33 -0700 Subject: [PATCH 12/31] ci: Support bot-authored commits in PR handler Ensure both types of Copybara PR commits are correctly identified and parsed by checking author email, headers, and updated merge regex. Change-Id: Ic0cc19f67a3db4637fa597702393a11802b3f9aa --- .github/workflows/copybara-pr-handler.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/copybara-pr-handler.yml b/.github/workflows/copybara-pr-handler.yml index 4ca3c48803..c176d8746a 100644 --- a/.github/workflows/copybara-pr-handler.yml +++ b/.github/workflows/copybara-pr-handler.yml @@ -58,14 +58,19 @@ jobs: console.log(`Committer: ${committer}`); // Check if this is a Copybara commit - if (committer !== 'Copybara-Service') { + const isCopybara = committer === 'Copybara-Service' || + commit.author?.email === 'genai-sdk-bot@google.com' || + message.includes('GitOrigin-RevId:') || + message.includes('PiperOrigin-RevId:'); + + if (!isCopybara) { console.log('Not a Copybara commit, skipping'); continue; } // Extract PR number from commit message - // Pattern: "Merge https://github.com/google/adk-python/pull/3333" - const prMatch = message.match(/Merge https:\/\/github\.com\/google\/adk-python\/pull\/(\d+)/); + // Pattern matches both "Merge https://..." and "Merge: https://..." + const prMatch = message.match(/Merge:?\s+https:\/\/github\.com\/google\/adk-python\/pull\/(\d+)/); if (!prMatch) { console.log('No PR number found in Copybara commit message'); From db064160bf634b1c7e644012076f077cde6cfcef Mon Sep 17 00:00:00 2001 From: Han Cao Date: Fri, 22 May 2026 17:10:00 +0000 Subject: [PATCH 13/31] feat: Add chart generation and artifact loading to data agent Introduces a generate_chart tool to the Data Agent sample, leveraging Altair and vl-convert to render Vega-Lite specifications into charts. Co-authored-by: Han Cao Change-Id: I5765487406d511e650091f5dc884102c43568fd4 --- .../samples/integrations/data_agent/agent.py | 59 ++++++++++++++++++- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/contributing/samples/integrations/data_agent/agent.py b/contributing/samples/integrations/data_agent/agent.py index 01e83bed50..c72667eeff 100644 --- a/contributing/samples/integrations/data_agent/agent.py +++ b/contributing/samples/integrations/data_agent/agent.py @@ -12,15 +12,22 @@ # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import annotations + +import asyncio import os +from typing import Any from google.adk.agents import Agent from google.adk.auth.auth_credential import AuthCredentialTypes +from google.adk.tools import load_artifacts from google.adk.tools.data_agent.config import DataAgentToolConfig from google.adk.tools.data_agent.credentials import DataAgentCredentialsConfig from google.adk.tools.data_agent.data_agent_toolset import DataAgentToolset +from google.adk.tools.tool_context import ToolContext import google.auth import google.auth.transport.requests +from google.genai import types # Define the desired credential type. # By default use Application Default Credentials (ADC) from the local @@ -72,16 +79,62 @@ ], ) +# NOTE: The generate_chart tool requires 'altair' and 'vl-convert-python' to be +# installed in your environment. You can install them using: +# pip install altair vl-convert-python +async def generate_chart( + chart_spec: dict[str, Any], tool_context: ToolContext +) -> dict[str, str]: + """Generates a professional chart using Altair based on a Vega-Lite spec. + + Args: + chart_spec: A dictionary defining a Vega-Lite chart. + tool_context: The tool context. + + Returns: + A dictionary containing the status of the chart generation ("success" or + "error"), a detail message, and the filename if successful. + """ + import altair as alt + import vl_convert as vlc + + try: + # Altair can take a Vega-Lite dict directly and render it. + # We use vl-convert to transform the spec into a high-quality PNG. + png_data = await asyncio.to_thread(vlc.vegalite_to_png, chart_spec, scale=2) + + # Save as artifact + await tool_context.save_artifact( + "chart.png", + types.Part.from_bytes(data=png_data, mime_type="image/png"), + ) + title = chart_spec.get("title", "Chart") + return { + "status": "success", + "detail": ( + f"Professional chart '{title}' rendered using Altair/Vega-Lite." + ), + "filename": "chart.png", + } + except Exception as e: # pylint: disable=broad-exception-caught + return {"status": "error", "detail": f"Failed to render chart: {str(e)}"} + + root_agent = Agent( name="data_agent", - description="Agent to answer user questions using Data Agents.", + description=( + "Agent to answer user questions using Data Agents and generate charts." + ), instruction=( "## Persona\nYou are a helpful assistant that uses Data Agents" " to answer user questions about their data.\n\n## Tools\n- You can" " list available data agents using `list_accessible_data_agents`.\n-" " You can get information about a specific data agent using" " `get_data_agent_info`.\n- You can chat with a specific data" - " agent using `ask_data_agent`.\n" + " agent using `ask_data_agent`.\n- `generate_chart` renders" + " professional charts from a `chart_spec` (Vega-Lite JSON). Use this" + " whenever you need to visualize data; do not show raw JSON to the" + " user.\n- You can load artifacts using `load_artifacts`.\n" ), - tools=[da_toolset], + tools=[da_toolset, generate_chart, load_artifacts], ) From cbd14ebf99bbff22ed28273f095f1fc05793bed2 Mon Sep 17 00:00:00 2001 From: Emily Feng Date: Fri, 22 May 2026 17:09:49 +0000 Subject: [PATCH 14/31] feat: Add support for creating sandboxes from templates and snapshots This change allows AgentEngineSandboxComputer to create new sandboxes using either a specified sandbox template or a sandbox snapshot. The environment variables VMAAS_SANDBOX_TEMPLATE_NAME and VMAAS_SANDBOX_SNAPSHOT_NAME are introduced to configure this behavior. Co-authored-by: Emily Feng Change-Id: Iebdd980a16966ba765cacbce6d63d0d5b691650a --- .../sandbox_computer_use/agent.py | 7 +++ .../integrations/sandbox_computer_use/main.py | 3 + .../integrations/vmaas/sandbox_computer.py | 60 ++++++++++++++----- 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/contributing/samples/integrations/sandbox_computer_use/agent.py b/contributing/samples/integrations/sandbox_computer_use/agent.py index b95dbad89a..f559dc167f 100644 --- a/contributing/samples/integrations/sandbox_computer_use/agent.py +++ b/contributing/samples/integrations/sandbox_computer_use/agent.py @@ -24,6 +24,8 @@ - GOOGLE_CLOUD_PROJECT: Your GCP project ID - VMAAS_SERVICE_ACCOUNT: Your service account email - VMAAS_SANDBOX_NAME: (Optional) Existing sandbox resource name for BYOS mode + - VMAAS_SANDBOX_TEMPLATE_NAME: (Optional) Sandbox template name to create a new sandbox (mutually exclusive with VMAAS_SANDBOX_NAME) + - VMAAS_SANDBOX_SNAPSHOT_NAME: (Optional) Sandbox snapshot name to create a new sandbox (mutually exclusive with VMAAS_SANDBOX_NAME) Usage: # Run via ADK web UI @@ -53,12 +55,17 @@ SANDBOX_NAME = os.environ.get("SANDBOX_NAME") or os.environ.get( "VMAAS_SANDBOX_NAME" ) +SANDBOX_TEMPLATE_NAME = os.environ.get("VMAAS_SANDBOX_TEMPLATE_NAME") +SANDBOX_SNAPSHOT_NAME = os.environ.get("VMAAS_SANDBOX_SNAPSHOT_NAME") + # Create the sandbox computer sandbox_computer = AgentEngineSandboxComputer( project_id=PROJECT_ID, service_account_email=SERVICE_ACCOUNT, sandbox_name=SANDBOX_NAME, + sandbox_template_name=SANDBOX_TEMPLATE_NAME, + sandbox_snapshot_name=SANDBOX_SNAPSHOT_NAME, search_engine_url="https://www.google.com", ) diff --git a/contributing/samples/integrations/sandbox_computer_use/main.py b/contributing/samples/integrations/sandbox_computer_use/main.py index e6c60bc298..19fbd8a517 100644 --- a/contributing/samples/integrations/sandbox_computer_use/main.py +++ b/contributing/samples/integrations/sandbox_computer_use/main.py @@ -22,6 +22,9 @@ - GOOGLE_CLOUD_PROJECT: Your GCP project ID - VMAAS_SERVICE_ACCOUNT: Your service account email with roles/iam.serviceAccountTokenCreator permission + - VMAAS_SANDBOX_NAME: (Optional) Existing sandbox resource name for BYOS mode + - VMAAS_SANDBOX_TEMPLATE_NAME: (Optional) Sandbox template name to create a new sandbox (mutually exclusive with VMAAS_SANDBOX_NAME) + - VMAAS_SANDBOX_SNAPSHOT_NAME: (Optional) Sandbox snapshot name to create a new sandbox (mutually exclusive with VMAAS_SANDBOX_NAME) Usage: cd contributing/samples diff --git a/src/google/adk/integrations/vmaas/sandbox_computer.py b/src/google/adk/integrations/vmaas/sandbox_computer.py index 1dd38e0ac7..9c1c9d68d7 100644 --- a/src/google/adk/integrations/vmaas/sandbox_computer.py +++ b/src/google/adk/integrations/vmaas/sandbox_computer.py @@ -90,6 +90,8 @@ def __init__( location: str = "us-central1", service_account_email: str | None = None, sandbox_name: str | None = None, + sandbox_template_name: str | None = None, + sandbox_snapshot_name: str | None = None, sandbox_ttl_seconds: int = 3600, search_engine_url: str = "https://www.google.com", vertexai_client: "vertexai.Client | None" = None, @@ -97,25 +99,36 @@ def __init__( """Initialize the sandbox computer. Args: - project_id: GCP project ID. If None, uses Application Default - Credentials project. + project_id: GCP project ID. If None, uses Application Default Credentials + project. location: Vertex AI location (default: us-central1). - service_account_email: Service account email for token generation. - Must have roles/iam.serviceAccountTokenCreator permission. - If None, attempts to use ADC service account. - sandbox_name: Existing sandbox resource name (BYOS mode). If provided, - the agent engine name is extracted from it. If None, creates new - agent engine and sandbox on demand. - Format: projects/{project}/locations/{location}/reasoningEngines/{id}/sandboxEnvironments/{id} + service_account_email: Service account email for token generation. Must + have roles/iam.serviceAccountTokenCreator permission. If None, attempts + to use ADC service account. + sandbox_name: Existing sandbox resource name (BYOS mode). If provided, the + agent engine name is extracted from it. If None, creates new agent + engine and sandbox on demand. + Format: + projects/{project}/locations/{location}/reasoningEngines/{id}/sandboxEnvironments/{id} + sandbox_template_name: Sandbox template resource name to use for creating + new sandboxes. Templates allow faster creation and custom environments. + Format: + projects/{project}/locations/{location}/sandboxEnvironmentTemplates/{id} + sandbox_snapshot_name: Sandbox snapshot resource name to use for restoring + sandbox state, enabling faster startup. + Format: + projects/{project}/locations/{location}/reasoningEngines/{id}/sandboxEnvironmentSnapshots/{id} sandbox_ttl_seconds: TTL for auto-created sandboxes (default: 1 hour). search_engine_url: URL to navigate to for search() method. - vertexai_client: Optional Vertex AI client instance. If None, creates - one lazily using project_id and location. + vertexai_client: Optional Vertex AI client instance. If None, creates one + lazily using project_id and location. """ self._project_id = project_id self._location = location self._service_account_email = service_account_email self._sandbox_name = sandbox_name + self._sandbox_template_name = sandbox_template_name + self._sandbox_snapshot_name = sandbox_snapshot_name self._sandbox_ttl_seconds = sandbox_ttl_seconds self._search_engine_url = search_engine_url self._screen_size = (1280, 720) @@ -210,13 +223,29 @@ async def _get_sandbox(self) -> tuple[str, Any]: from vertexai import types + config = { + "display_name": "adk_computer_use_sandbox", + } + spec = None + if self._sandbox_template_name: + config["sandbox_environment_template"] = self._sandbox_template_name + logger.info( + "Creating sandbox from template: %s", self._sandbox_template_name + ) + elif self._sandbox_snapshot_name: + config["sandbox_environment_snapshot"] = self._sandbox_snapshot_name + logger.info( + "Creating sandbox from snapshot: %s", self._sandbox_snapshot_name + ) + else: + spec = {"computer_use_environment": {}} + logger.info("Creating sandbox with computer use environment spec") + operation = await asyncio.to_thread( client.agent_engines.sandboxes.create, - spec={"computer_use_environment": {}}, + spec=spec, name=agent_engine_name, - config=types.CreateAgentEngineSandboxConfig( - display_name="adk_computer_use_sandbox" - ), + config=config, ) sandbox_name = operation.response.name @@ -249,7 +278,6 @@ async def _get_access_token(self, sandbox_name: str) -> str: token = await asyncio.to_thread( client.agent_engines.sandboxes.generate_access_token, service_account_email=self._service_account_email, - sandbox_id=sandbox_name, timeout=_DEFAULT_TOKEN_TIMEOUT, ) From b3d0759e42d0def400160e4196a874171579a101 Mon Sep 17 00:00:00 2001 From: Wenhua Zhang Date: Fri, 22 May 2026 17:09:08 +0000 Subject: [PATCH 15/31] feat: Preserve transcription event order in conversation trajectory Refactor live inference to emit synthetic text events for output transcriptions as they arrive (streaming), rather than accumulating all transcriptions and appending a single synthetic event after the turn completes. This preserves event ordering and enables downstream consumers to process transcriptions incrementally. Co-authored-by: Wenhua Zhang Change-Id: Idca79968729f2145ebf1f1c993d79ab7f6a3bd78 --- .../adk/evaluation/evaluation_generator.py | 40 ++++++------- .../evaluation/test_evaluation_generator.py | 56 +++++++++++++++++++ 2 files changed, 74 insertions(+), 22 deletions(-) diff --git a/src/google/adk/evaluation/evaluation_generator.py b/src/google/adk/evaluation/evaluation_generator.py index d5a6629366..5b0100818c 100644 --- a/src/google/adk/evaluation/evaluation_generator.py +++ b/src/google/adk/evaluation/evaluation_generator.py @@ -383,6 +383,7 @@ async def _generate_inferences_for_single_user_invocation_live( current_invocation_id: str, turn_complete_event: asyncio.Event, live_timeout_seconds: int, + agent_name: str = _DEFAULT_AUTHOR, ) -> AsyncGenerator[Event, None]: """Generates inferences for a single user invocation in live mode.""" yield Event( @@ -408,6 +409,22 @@ async def _generate_inferences_for_single_user_invocation_live( event = await event_queue.get() if event.invocation_id == current_invocation_id: yield event + # Emit a synthetic text event for each transcription, preserving + # the order in which events are received. + if ( + event.author != _USER_AUTHOR + and event.output_transcription + and event.output_transcription.text + and event.partial + ): + yield Event( + content=Content( + role="model", + parts=[types.Part(text=event.output_transcription.text)], + ), + author=agent_name, + invocation_id=current_invocation_id, + ) @staticmethod async def _generate_inferences_from_root_agent_live( @@ -495,31 +512,10 @@ async def _generate_inferences_from_root_agent_live( current_invocation_id=live_session.current_invocation_id, turn_complete_event=live_session.turn_complete_event, live_timeout_seconds=live_timeout_seconds, + agent_name=runner.agent.name, ): events.append(event) - turn_transcription = "" - for evt in events: - if ( - evt.invocation_id == live_session.current_invocation_id - and evt.author != _USER_AUTHOR - and evt.output_transcription - ): - if not evt.partial and evt.output_transcription.text: - turn_transcription = evt.output_transcription.text - else: - turn_transcription += evt.output_transcription.text - if turn_transcription: - synthetic_event = Event( - content=Content( - role="model", - parts=[types.Part(text=turn_transcription)], - ), - author=runner.agent.name, - invocation_id=live_session.current_invocation_id, - ) - events.append(synthetic_event) - if live_session.live_finished.is_set(): logger.info("Live session finished signal detected.") break diff --git a/tests/unittests/evaluation/test_evaluation_generator.py b/tests/unittests/evaluation/test_evaluation_generator.py index 508b6f5c9c..05ab25cc72 100644 --- a/tests/unittests/evaluation/test_evaluation_generator.py +++ b/tests/unittests/evaluation/test_evaluation_generator.py @@ -453,6 +453,62 @@ async def test_generate_inferences_live(self, mocker): with pytest.raises(StopAsyncIteration): await gen.__anext__() + @pytest.mark.asyncio + async def test_generate_inferences_live_with_synthetic_events(self, mocker): + """Tests live inference generation with synthetic events.""" + mock_live_request_queue = mocker.MagicMock() + event_queue = asyncio.Queue() + turn_complete_event = asyncio.Event() + + user_content = types.Content(parts=[types.Part(text="User query")]) + invocation_id = "inv1" + + transcription = types.Transcription(text="Partial transcription") + partial_event = Event( + author="agent", + content=types.Content(parts=[]), + invocation_id=invocation_id, + output_transcription=transcription, + partial=True, + ) + + gen = EvaluationGenerator._generate_inferences_for_single_user_invocation_live( + live_request_queue=mock_live_request_queue, + event_queue=event_queue, + user_message=user_content, + current_invocation_id=invocation_id, + turn_complete_event=turn_complete_event, + live_timeout_seconds=300, + agent_name="custom_agent_name", + ) + + # First yield should be the user message + first_event = await gen.__anext__() + assert first_event.author == "user" + assert first_event.content == user_content + assert first_event.invocation_id == invocation_id + + # Mock turn_complete_event.wait to avoid blocking + turn_complete_event.wait = mocker.AsyncMock() + + # Put the partial event in the queue + await event_queue.put(partial_event) + + # Now advance + second_event = await gen.__anext__() + assert second_event == partial_event + + # Next should be the synthetic event + third_event = await gen.__anext__() + assert third_event.author == "custom_agent_name" + assert third_event.invocation_id == invocation_id + assert third_event.content.role == "model" + assert third_event.content.parts[0].text == "Partial transcription" + + # The generator should be exhausted now + with pytest.raises(StopAsyncIteration): + await gen.__anext__() + @pytest.fixture def mock_runner(mocker): From 104edc83170a5871075285b336d19ac9515c1a90 Mon Sep 17 00:00:00 2001 From: George Weale Date: Fri, 22 May 2026 22:58:34 +0000 Subject: [PATCH 16/31] fix: convert Union[Pydantic, Pydantic] tool args at runtime FunctionTool._preprocess_args only converted dict args to a Pydantic model for single-model and Optional[Model] annotations. A Union[ModelA, ModelB] parameter was left as a raw dict, so isinstance checks inside the tool failed with "Unexpected entity type: " Use pydantic.TypeAdapter to validate against the full Union so pydantic picks the matching member. None and instances of any declared union member pass through unchanged; instances of unrelated BaseModels fall back to the existing graceful-failure warning path. Close #5799 Change-Id: Ie69f8efc8395162eac375a0eaad0c77ed2097cec --- .../tests/test_create_company.json | 65 ++------ src/google/adk/tools/function_tool.py | 18 +++ .../tools/test_function_tool_pydantic.py | 151 ++++++++++++++++++ 3 files changed, 180 insertions(+), 54 deletions(-) diff --git a/contributing/samples/tools/pydantic_argument/tests/test_create_company.json b/contributing/samples/tools/pydantic_argument/tests/test_create_company.json index 5b9d227863..bf8d2ab0be 100644 --- a/contributing/samples/tools/pydantic_argument/tests/test_create_company.json +++ b/contributing/samples/tools/pydantic_argument/tests/test_create_company.json @@ -53,66 +53,23 @@ "id": "fc-1", "name": "create_entity_profile", "response": { - "message": "Unexpected entity type: ", - "status": "error" - } - } - } - ], - "role": "user" - }, - "id": "e-3", - "invocationId": "i-1", - "nodeInfo": { - "path": "profile_agent@1" - } - }, - { - "author": "profile_agent", - "content": { - "parts": [ - { - "functionCall": { - "args": { - "entity": { + "entity_type": "company", + "message": "Company profile created for Acme Corp!", + "profile": { "company_name": "Acme Corp", "employee_count": 50, - "industry": "tech" - } - }, - "id": "fc-2", - "name": "create_entity_profile" - } - } - ], - "role": "model" - }, - "finishReason": "STOP", - "id": "e-4", - "invocationId": "i-1", - "longRunningToolIds": [], - "nodeInfo": { - "path": "profile_agent@1" - } - }, - { - "author": "profile_agent", - "content": { - "parts": [ - { - "functionResponse": { - "id": "fc-2", - "name": "create_entity_profile", - "response": { - "message": "Unexpected entity type: ", - "status": "error" + "industry": "tech", + "model_type": "CompanyProfile", + "website": "Not provided" + }, + "status": "company_profile_created" } } } ], "role": "user" }, - "id": "e-5", + "id": "e-3", "invocationId": "i-1", "nodeInfo": { "path": "profile_agent@1" @@ -123,13 +80,13 @@ "content": { "parts": [ { - "text": "I apologize for the repeated errors. It seems I am having trouble with the tool's expected input format. I will try again to create the company profile for Acme Corp. I believe the issue was in how I was structuring the data for the `create_entity_profile` function. I will now use the correct dataclass constructor to create the profile. Please bear with me." + "text": "I have created a profile for Acme Corp in the tech industry with 50 employees." } ], "role": "model" }, "finishReason": "STOP", - "id": "e-6", + "id": "e-4", "invocationId": "i-1", "nodeInfo": { "path": "profile_agent@1" diff --git a/src/google/adk/tools/function_tool.py b/src/google/adk/tools/function_tool.py index 6477fd6894..7510cd6566 100644 --- a/src/google/adk/tools/function_tool.py +++ b/src/google/adk/tools/function_tool.py @@ -148,6 +148,24 @@ def _preprocess_args(self, args: dict[str, Any]) -> dict[str, Any]: ] if len(non_none_types) == 1: target_type = non_none_types[0] + elif len(non_none_types) > 1 and all( + inspect.isclass(t) and issubclass(t, pydantic.BaseModel) + for t in non_none_types + ): + if args[param_name] is None or isinstance( + args[param_name], tuple(non_none_types) + ): + continue + try: + converted_args[param_name] = pydantic.TypeAdapter( + param.annotation + ).validate_python(args[param_name]) + except Exception as e: + logger.warning( + f"Failed to convert argument '{param_name}' to" + f' {param.annotation}: {e}' + ) + continue # Check if the target type is a Pydantic model if inspect.isclass(target_type) and issubclass( diff --git a/tests/unittests/tools/test_function_tool_pydantic.py b/tests/unittests/tools/test_function_tool_pydantic.py index be04d3a033..02328e0452 100644 --- a/tests/unittests/tools/test_function_tool_pydantic.py +++ b/tests/unittests/tools/test_function_tool_pydantic.py @@ -15,6 +15,7 @@ # Pydantic model conversion tests from typing import Optional +from typing import Union from unittest.mock import MagicMock from google.adk.agents.invocation_context import InvocationContext @@ -40,6 +41,14 @@ class PreferencesModel(pydantic.BaseModel): notifications: bool = True +class CompanyModel(pydantic.BaseModel): + """Test Pydantic model for company data.""" + + company_name: str + industry: str + employee_count: int + + def sync_function_with_pydantic_model(user: UserModel) -> dict: """Sync function that takes a Pydantic model.""" return { @@ -370,3 +379,145 @@ def place_order(orders: list[UserModel]) -> int: result = await tool.run_async(args=args, tool_context=tool_context_mock) assert result == 50 + + +def _function_with_union_of_basemodels( + entity: Union[UserModel, CompanyModel], +) -> str: + return type(entity).__name__ + + +def test_preprocess_args_with_union_of_basemodels_picks_user(): + """Dict matching UserModel is converted to UserModel.""" + tool = FunctionTool(_function_with_union_of_basemodels) + + processed_args = tool._preprocess_args( + {"entity": {"name": "Diana", "age": 32, "email": "d@example.com"}} + ) + + assert isinstance(processed_args["entity"], UserModel) + assert processed_args["entity"].name == "Diana" + + +def test_preprocess_args_with_union_of_basemodels_picks_company(): + """Dict matching CompanyModel is converted to CompanyModel.""" + tool = FunctionTool(_function_with_union_of_basemodels) + + processed_args = tool._preprocess_args({ + "entity": { + "company_name": "Acme Corp", + "industry": "tech", + "employee_count": 50, + } + }) + + assert isinstance(processed_args["entity"], CompanyModel) + assert processed_args["entity"].company_name == "Acme Corp" + + +def test_preprocess_args_with_union_of_basemodels_existing_instance_unchanged(): + """Existing instance of any union member is left unchanged.""" + tool = FunctionTool(_function_with_union_of_basemodels) + + user = UserModel(name="Bob", age=25) + assert tool._preprocess_args({"entity": user})["entity"] is user + + company = CompanyModel( + company_name="Acme", industry="tech", employee_count=10 + ) + assert tool._preprocess_args({"entity": company})["entity"] is company + + +def test_preprocess_args_with_union_of_basemodels_unrelated_instance_passthrough(): + """A BaseModel instance not in the union is not silently accepted.""" + tool = FunctionTool(_function_with_union_of_basemodels) + + class UnrelatedModel(pydantic.BaseModel): + name: str + age: int + + unrelated = UnrelatedModel(name="Carol", age=20) + processed_args = tool._preprocess_args({"entity": unrelated}) + + # Conversion fails (UnrelatedModel is not in the union); value is left + # alone so the function receives it and raises a clear error itself. + assert processed_args["entity"] is unrelated + + +def test_preprocess_args_with_optional_union_of_basemodels_none(): + """Optional[Union[A, B]] passes None through unchanged.""" + + def fn(entity: Optional[Union[UserModel, CompanyModel]] = None) -> str: + return type(entity).__name__ + + tool = FunctionTool(fn) + + processed_args = tool._preprocess_args({"entity": None}) + + assert processed_args["entity"] is None + + +def test_preprocess_args_with_optional_union_of_basemodels_dict(): + """Optional[Union[A, B]] converts a dict to the matching model.""" + + def fn(entity: Optional[Union[UserModel, CompanyModel]] = None) -> str: + return type(entity).__name__ + + tool = FunctionTool(fn) + + processed_args = tool._preprocess_args({"entity": {"name": "Eve", "age": 40}}) + + assert isinstance(processed_args["entity"], UserModel) + assert processed_args["entity"].name == "Eve" + + +def test_preprocess_args_with_union_of_basemodels_invalid_data(): + """Invalid data for Union[BaseModel, BaseModel] is kept unchanged.""" + tool = FunctionTool(_function_with_union_of_basemodels) + + # Dict matches neither model. + processed_args = tool._preprocess_args( + {"entity": {"unrelated_field": "value"}} + ) + + assert processed_args["entity"] == {"unrelated_field": "value"} + + +@pytest.mark.asyncio +async def test_run_async_with_union_of_basemodels(): + """run_async end-to-end converts dict to the matching union member.""" + + def create_entity_profile( + entity: Union[UserModel, CompanyModel], + ) -> dict: + if isinstance(entity, UserModel): + return {"entity_type": "user", "name": entity.name} + if isinstance(entity, CompanyModel): + return {"entity_type": "company", "name": entity.company_name} + return {"entity_type": "unknown"} + + tool = FunctionTool(create_entity_profile) + + tool_context_mock = MagicMock(spec=ToolContext) + invocation_context_mock = MagicMock(spec=InvocationContext) + session_mock = MagicMock(spec=Session) + invocation_context_mock.session = session_mock + tool_context_mock.invocation_context = invocation_context_mock + + user_result = await tool.run_async( + args={"entity": {"name": "Diana", "age": 32}}, + tool_context=tool_context_mock, + ) + assert user_result == {"entity_type": "user", "name": "Diana"} + + company_result = await tool.run_async( + args={ + "entity": { + "company_name": "Acme Corp", + "industry": "tech", + "employee_count": 50, + } + }, + tool_context=tool_context_mock, + ) + assert company_result == {"entity_type": "company", "name": "Acme Corp"} From eaff9c021e29ec8986ac230591b05e2d6e5d826b Mon Sep 17 00:00:00 2001 From: Google Team Member Date: Fri, 22 May 2026 19:39:04 -0400 Subject: [PATCH 17/31] feat: Update check-file-contents.yml to check for non-mTLS hardcoded endpoints Merge 29fcd80cd084e24611cae24a6fa773577a76581e into 104edc83170a5871075285b336d19ac9515c1a90 ORIGINAL_AUTHOR=agrawalradhika-cell GitOrigin-RevId: 46a8802b04e4d4344763ae415ff5424b2bfde4bc Change-Id: I3c6c4e92c92c99567be2bb938e95ae950bbbc634 --- .github/workflows/check-file-contents.yml | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/.github/workflows/check-file-contents.yml b/.github/workflows/check-file-contents.yml index 985f6a0f77..a6c31788fa 100644 --- a/.github/workflows/check-file-contents.yml +++ b/.github/workflows/check-file-contents.yml @@ -100,3 +100,37 @@ jobs: else echo "✅ No relevant Python files found." fi + + - name: Check for hardcoded googleapis.com endpoints + run: | + git fetch origin ${GITHUB_BASE_REF} + CHANGED_FILES=$(git diff --diff-filter=ACMR --name-only origin/${GITHUB_BASE_REF}...HEAD | grep -E '\.py$' || true) + if [ -n "$CHANGED_FILES" ]; then + echo "Checking for hardcoded endpoints in: $CHANGED_FILES" + + # 1. Identify files containing any googleapis.com URL. + set +e + FILES_WITH_ENDPOINTS=$(grep -lE 'https?://[a-zA-Z0-9.-]+\.googleapis\.com' $CHANGED_FILES) + + # 2. From those, identify files that are MISSING the required mTLS version. + if [ -n "$FILES_WITH_ENDPOINTS" ]; then + FILES_MISSING_MTLS=$(grep -L '.mtls.googleapis.com' $FILES_WITH_ENDPOINTS) + fi + set -e + + if [ -n "$FILES_MISSING_MTLS" ]; then + echo "❌ Found hardcoded googleapis.com endpoints without mTLS support." + echo "The following files must define both standard and mTLS (.mtls.googleapis.com) endpoints" + echo "to support dynamic endpoint selection as required by security policy:" + echo "$FILES_MISSING_MTLS" + echo "" + echo "To fix this, please follow these steps:" + echo "1. Initialize an AuthorizedSession with your credentials." + echo "2. Use 'mtls.has_default_client_cert_source() from google-auth' to check for available client certificates." + echo "3. If certificates are present, use 'session.configure_mtls_channel()'." + echo "4. Dynamically select the '.mtls.' variant of the endpoint when mTLS is active." + exit 1 + else + echo "✅ All hardcoded endpoints have corresponding mTLS definitions or no endpoints found." + fi + fi From 7ad7994744de18f2394e4bcb961cd5c7a24afb4b Mon Sep 17 00:00:00 2001 From: asobran Date: Fri, 22 May 2026 23:44:41 +0000 Subject: [PATCH 18/31] chore: Set python unit test timeout to 10 minutes Change-Id: Ic0f6706714c31984783b0877446a5cf2f1a6af23 --- .github/workflows/python-unit-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/python-unit-tests.yml b/.github/workflows/python-unit-tests.yml index fcedce9acc..cff6d69923 100644 --- a/.github/workflows/python-unit-tests.yml +++ b/.github/workflows/python-unit-tests.yml @@ -12,6 +12,7 @@ permissions: jobs: test: runs-on: ubuntu-latest + timeout-minutes: 10 strategy: matrix: python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] From e623b3b4db9109d2f53915a7c3e8140db71ca336 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Sat, 23 May 2026 12:14:36 -0700 Subject: [PATCH 19/31] refactor: update tool and agent retrieval functions to support asynchronous execution Change-Id: Ie9328bcb15bb266b83fc6c73887422d59a29ace7 --- src/google/adk/cli/api_server.py | 2 +- src/google/adk/utils/agent_info.py | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/google/adk/cli/api_server.py b/src/google/adk/cli/api_server.py index 3392ecb64f..ea9dcdc3bf 100644 --- a/src/google/adk/cli/api_server.py +++ b/src/google/adk/cli/api_server.py @@ -1043,7 +1043,7 @@ async def get_adk_app_info(app_name: str) -> AppInfo: root_agent_name=root_agent.name, description=root_agent.description, language="python", - agents=get_agents_dict(root_agent), + agents=await get_agents_dict(root_agent), ) else: raise HTTPException( diff --git a/src/google/adk/utils/agent_info.py b/src/google/adk/utils/agent_info.py index 957b5808bf..f9997bdc32 100644 --- a/src/google/adk/utils/agent_info.py +++ b/src/google/adk/utils/agent_info.py @@ -34,14 +34,16 @@ class AgentInfo(pydantic.BaseModel): sub_agents: list[str] -def get_tools_info(tools: list[ToolUnion]) -> list[Any]: +async def get_tools_info(tools: list[ToolUnion]) -> list[Any]: """Returns the info for a given list of tools.""" final_tools = [] for tool in tools: if isinstance(tool, BaseTool): final_tools.append(tool) elif isinstance(tool, BaseToolset): - final_tools.extend(tool.get_tools()) + # Await the async coroutine call natively! + tools_res = await tool.get_tools() + final_tools.extend(tools_res) else: final_tools.append(FunctionTool(tool)) return [ @@ -51,27 +53,27 @@ def get_tools_info(tools: list[ToolUnion]) -> list[Any]: ] -def get_agents_dict(agent: LlmAgent) -> dict[str, AgentInfo]: +async def get_agents_dict(agent: LlmAgent) -> dict[str, AgentInfo]: """Returns a dict with info for the agent and its sub-agents.""" agents_dict = {} - def _traverse(current_agent: LlmAgent): + async def _traverse(current_agent: LlmAgent): if current_agent.name in agents_dict: return sub_agent_names = [] for sub_agent in current_agent.sub_agents: if isinstance(sub_agent, LlmAgent): - _traverse(sub_agent) + await _traverse(sub_agent) sub_agent_names.append(sub_agent.name) agents_dict[current_agent.name] = AgentInfo( name=current_agent.name, description=current_agent.description, instruction=current_agent.instruction, - tools=get_tools_info(current_agent.tools), + tools=await get_tools_info(current_agent.tools), sub_agents=sub_agent_names, ) - _traverse(agent) + await _traverse(agent) return agents_dict From 2144efcdc9e25476a3210efb75aac643042b209d Mon Sep 17 00:00:00 2001 From: Raman369AI Date: Sat, 9 May 2026 23:07:37 -0500 Subject: [PATCH 20/31] fix: terminate infinite retry loop in LoadSkillResourceTool on RESOURCE_NOT_FOUND When load_skill_resource returned RESOURCE_NOT_FOUND, the LLM treated it as a transient soft error and retried the same path indefinitely, producing runaway invocations and large API bills. Two complementary guards are added: 1. Code (LoadSkillResourceTool.run_async): an invocation-scoped retry guard tracks already-attempted (skill, path) pairs in tool_context.state under a temp:_adk_skill_resource_failed_paths_ key. The temp: prefix prevents persistence to durable session storage; the invocation_id suffix prevents leakage across invocations on in-memory session backends (where temp keys are not auto-cleared). A second call on the same path within the same invocation returns RESOURCE_NOT_FOUND_FATAL with an explicit stop instruction, giving the LLM an unambiguous terminal signal. 2. Prompt (DEFAULT_SKILL_SYSTEM_INSTRUCTION): adds a rule prohibiting retrying the same path after any error, and a scope boundary clarifying that load_skill_resource is for skill-bundled files only, not for runtime user input (a common source of hallucinated paths). Neither guard alone is sufficient: a code-only stop produces confusing downstream behavior when the LLM has no semantic understanding of why to stop; a prompt-only guard can be ignored under imperfect system prompts. Both layers are required for defense-in-depth. Tests cover: repeat-failure escalation, distinct-path soft errors not escalating, cross-invocation isolation with shared session state, and the temp: prefix on tracking keys. --- src/google/adk/tools/skill_toolset.py | 26 +++++- tests/unittests/tools/test_skill_toolset.py | 90 ++++++++++++++++++++- 2 files changed, 114 insertions(+), 2 deletions(-) diff --git a/src/google/adk/tools/skill_toolset.py b/src/google/adk/tools/skill_toolset.py index 7d8e06368e..9ce371a623 100644 --- a/src/google/adk/tools/skill_toolset.py +++ b/src/google/adk/tools/skill_toolset.py @@ -82,10 +82,14 @@ "of them in order.\n" "3. The `load_skill_resource` tool is for viewing files within a " "skill's directory (e.g., `references/*`, `assets/*`, `scripts/*`). " - "Do NOT use other tools to access these files.\n" + "It is ONLY for skill-bundled files — do NOT use it to access " + "documents or files provided by the user at runtime. Do NOT use " + "other tools to access skill files.\n" "4. Use `run_skill_script` to run scripts from a skill's `scripts/` " "directory. Use `load_skill_resource` to view script content first if " "needed.\n" + "5. If `load_skill_resource` returns any error, do not retry the " + "same path. Report the error to the user and stop.\n" ) @@ -351,6 +355,26 @@ async def run_async( } if content is None: + # Invocation-scoped retry guard. The `temp:` prefix prevents persistence + # to durable session storage; appending invocation_id ensures the guard + # does not leak across invocations on in-memory session backends (where + # temp keys are not auto-cleared). + failed_key = ( + f"temp:_adk_skill_resource_failed_paths_{tool_context.invocation_id}" + ) + failed_paths = list(tool_context.state.get(failed_key) or []) + resource_id = f"{skill_name}:{file_path}" + if resource_id in failed_paths: + return { + "error": ( + f"Resource '{file_path}' not found in skill '{skill_name}'." + " This path was already attempted and failed. Do not retry" + " — report the error to the user and stop." + ), + "error_code": "RESOURCE_NOT_FOUND_FATAL", + } + failed_paths.append(resource_id) + tool_context.state[failed_key] = failed_paths return { "error": f"Resource '{file_path}' not found in skill '{skill_name}'.", "error_code": "RESOURCE_NOT_FOUND", diff --git a/tests/unittests/tools/test_skill_toolset.py b/tests/unittests/tools/test_skill_toolset.py index f637377513..2305ba0262 100644 --- a/tests/unittests/tools/test_skill_toolset.py +++ b/tests/unittests/tools/test_skill_toolset.py @@ -497,10 +497,97 @@ async def test_scripts_resource_not_found(mock_skill1, tool_context_instance): assert result["error_code"] == "RESOURCE_NOT_FOUND" +@pytest.mark.asyncio +async def test_load_resource_repeated_failure_escalates_to_fatal(mock_skill1): + """Second call with same missing path within an invocation returns RESOURCE_NOT_FOUND_FATAL.""" + toolset = skill_toolset.SkillToolset([mock_skill1]) + tool = skill_toolset.LoadSkillResourceTool(toolset) + ctx = _make_tool_context_with_agent() + + args = {"skill_name": "skill1", "file_path": "references/nonexistent.md"} + + result1 = await tool.run_async(args=args, tool_context=ctx) + assert result1["error_code"] == "RESOURCE_NOT_FOUND" + + result2 = await tool.run_async(args=args, tool_context=ctx) + assert result2["error_code"] == "RESOURCE_NOT_FOUND_FATAL" + assert "Do not retry" in result2["error"] + assert "stop" in result2["error"].lower() + + +@pytest.mark.asyncio +async def test_load_resource_different_paths_each_soft_fail(mock_skill1): + """Different missing paths each get a soft error (no cross-path escalation).""" + toolset = skill_toolset.SkillToolset([mock_skill1]) + tool = skill_toolset.LoadSkillResourceTool(toolset) + ctx = _make_tool_context_with_agent() + + result1 = await tool.run_async( + args={"skill_name": "skill1", "file_path": "references/missing_a.md"}, + tool_context=ctx, + ) + assert result1["error_code"] == "RESOURCE_NOT_FOUND" + + result2 = await tool.run_async( + args={"skill_name": "skill1", "file_path": "references/missing_b.md"}, + tool_context=ctx, + ) + assert result2["error_code"] == "RESOURCE_NOT_FOUND" + + +@pytest.mark.asyncio +async def test_load_resource_failures_isolated_per_invocation(mock_skill1): + """Failed-path tracking does not leak across invocations. + + A path that hit a soft RESOURCE_NOT_FOUND in invocation A must still + return RESOURCE_NOT_FOUND (not _FATAL) on its first attempt in + invocation B, even when sharing session state. + """ + toolset = skill_toolset.SkillToolset([mock_skill1]) + tool = skill_toolset.LoadSkillResourceTool(toolset) + + shared_state = {} + ctx_a = _make_tool_context_with_agent(invocation_id="inv_a") + ctx_a.state = shared_state + ctx_b = _make_tool_context_with_agent(invocation_id="inv_b") + ctx_b.state = shared_state + + args = {"skill_name": "skill1", "file_path": "references/typo.md"} + + result_a = await tool.run_async(args=args, tool_context=ctx_a) + assert result_a["error_code"] == "RESOURCE_NOT_FOUND" + + result_b = await tool.run_async(args=args, tool_context=ctx_b) + assert result_b["error_code"] == "RESOURCE_NOT_FOUND" + + # And the fatal escalation still works within a single invocation. + result_b2 = await tool.run_async(args=args, tool_context=ctx_b) + assert result_b2["error_code"] == "RESOURCE_NOT_FOUND_FATAL" + + +@pytest.mark.asyncio +async def test_load_resource_failed_paths_use_temp_prefix(mock_skill1): + """Tracking key uses the `temp:` prefix so it is not persisted.""" + toolset = skill_toolset.SkillToolset([mock_skill1]) + tool = skill_toolset.LoadSkillResourceTool(toolset) + ctx = _make_tool_context_with_agent() + + await tool.run_async( + args={"skill_name": "skill1", "file_path": "references/missing.md"}, + tool_context=ctx, + ) + + # Every key written by the retry guard must start with `temp:` so it gets + # trimmed from the event delta and never reaches durable storage. + guard_keys = [k for k in ctx.state if "skill_resource_failed_paths" in k] + assert guard_keys, "Retry guard did not write a tracking key." + assert all(k.startswith("temp:") for k in guard_keys) + + # RunSkillScriptTool tests -def _make_tool_context_with_agent(agent=None): +def _make_tool_context_with_agent(agent=None, invocation_id="test_invocation"): """Creates a mock ToolContext with _invocation_context.agent.""" ctx = mock.MagicMock(spec=tool_context.ToolContext) ctx._invocation_context = mock.MagicMock() @@ -508,6 +595,7 @@ def _make_tool_context_with_agent(agent=None): ctx._invocation_context.agent.name = "test_agent" ctx._invocation_context.agent_states = {} ctx.agent_name = "test_agent" + ctx.invocation_id = invocation_id ctx.state = {} return ctx From d3232395f2893e1eee81e890b067a1d2c523e5c4 Mon Sep 17 00:00:00 2001 From: Raman369AI Date: Tue, 12 May 2026 21:54:57 -0500 Subject: [PATCH 21/31] fix: switch retry guard to invocation-wide counter --- src/google/adk/tools/skill_toolset.py | 25 +++--- tests/unittests/tools/test_skill_toolset.py | 92 +++++++++++++-------- 2 files changed, 71 insertions(+), 46 deletions(-) diff --git a/src/google/adk/tools/skill_toolset.py b/src/google/adk/tools/skill_toolset.py index 9ce371a623..a23285c4ca 100644 --- a/src/google/adk/tools/skill_toolset.py +++ b/src/google/adk/tools/skill_toolset.py @@ -355,26 +355,25 @@ async def run_async( } if content is None: - # Invocation-scoped retry guard. The `temp:` prefix prevents persistence - # to durable session storage; appending invocation_id ensures the guard - # does not leak across invocations on in-memory session backends (where - # temp keys are not auto-cleared). - failed_key = ( - f"temp:_adk_skill_resource_failed_paths_{tool_context.invocation_id}" + # Invocation-scoped failure counter. Counts RESOURCE_NOT_FOUND across ALL + # paths so the guard fires even when the LLM hallucinates a different path + # on each retry. The `temp:` prefix prevents persistence to durable + # session storage; invocation_id isolates in-memory backends. + counter_key = ( + f"temp:_adk_skill_resource_not_found_count_{tool_context.invocation_id}" ) - failed_paths = list(tool_context.state.get(failed_key) or []) - resource_id = f"{skill_name}:{file_path}" - if resource_id in failed_paths: + fail_count = int(tool_context.state.get(counter_key) or 0) + 1 + tool_context.state[counter_key] = fail_count + if fail_count > 1: return { "error": ( f"Resource '{file_path}' not found in skill '{skill_name}'." - " This path was already attempted and failed. Do not retry" - " — report the error to the user and stop." + f" This is resource lookup failure #{fail_count} this" + " invocation. Do not retry any path — report the error to" + " the user and stop." ), "error_code": "RESOURCE_NOT_FOUND_FATAL", } - failed_paths.append(resource_id) - tool_context.state[failed_key] = failed_paths return { "error": f"Resource '{file_path}' not found in skill '{skill_name}'.", "error_code": "RESOURCE_NOT_FOUND", diff --git a/tests/unittests/tools/test_skill_toolset.py b/tests/unittests/tools/test_skill_toolset.py index 2305ba0262..e75c6cfc73 100644 --- a/tests/unittests/tools/test_skill_toolset.py +++ b/tests/unittests/tools/test_skill_toolset.py @@ -332,16 +332,10 @@ async def test_load_skill_run_async_state_none( "error_code": "SKILL_NOT_FOUND", }, ), - ( - {"skill_name": "skill1", "file_path": "references/other.md"}, - { - "error": ( - "Resource 'references/other.md' not found in skill" - " 'skill1'." - ), - "error_code": "RESOURCE_NOT_FOUND", - }, - ), + # RESOURCE_NOT_FOUND is tested separately in + # test_load_resource_first_missing_returns_soft_error because the + # counter guard requires a real state dict (mock state.get() returns a + # truthy MagicMock that int() coerces to 1, skipping the soft path). ( {"skill_name": "skill1", "file_path": "invalid/path.txt"}, { @@ -487,19 +481,36 @@ def test_duplicate_skill_name_raises(mock_skill1): @pytest.mark.asyncio -async def test_scripts_resource_not_found(mock_skill1, tool_context_instance): +async def test_scripts_resource_not_found(mock_skill1): toolset = skill_toolset.SkillToolset([mock_skill1]) tool = skill_toolset.LoadSkillResourceTool(toolset) + ctx = _make_tool_context_with_agent() result = await tool.run_async( args={"skill_name": "skill1", "file_path": "scripts/nonexistent.sh"}, - tool_context=tool_context_instance, + tool_context=ctx, + ) + assert result["error_code"] == "RESOURCE_NOT_FOUND" + + +@pytest.mark.asyncio +async def test_load_resource_first_missing_returns_soft_error(mock_skill1): + """First RESOURCE_NOT_FOUND in an invocation returns the soft error code.""" + toolset = skill_toolset.SkillToolset([mock_skill1]) + tool = skill_toolset.LoadSkillResourceTool(toolset) + ctx = _make_tool_context_with_agent() + result = await tool.run_async( + args={"skill_name": "skill1", "file_path": "references/other.md"}, + tool_context=ctx, ) assert result["error_code"] == "RESOURCE_NOT_FOUND" + assert result["error"] == ( + "Resource 'references/other.md' not found in skill 'skill1'." + ) @pytest.mark.asyncio async def test_load_resource_repeated_failure_escalates_to_fatal(mock_skill1): - """Second call with same missing path within an invocation returns RESOURCE_NOT_FOUND_FATAL.""" + """Any second RESOURCE_NOT_FOUND within an invocation returns RESOURCE_NOT_FOUND_FATAL.""" toolset = skill_toolset.SkillToolset([mock_skill1]) tool = skill_toolset.LoadSkillResourceTool(toolset) ctx = _make_tool_context_with_agent() @@ -513,11 +524,16 @@ async def test_load_resource_repeated_failure_escalates_to_fatal(mock_skill1): assert result2["error_code"] == "RESOURCE_NOT_FOUND_FATAL" assert "Do not retry" in result2["error"] assert "stop" in result2["error"].lower() + assert "failure #2" in result2["error"] @pytest.mark.asyncio -async def test_load_resource_different_paths_each_soft_fail(mock_skill1): - """Different missing paths each get a soft error (no cross-path escalation).""" +async def test_load_resource_different_path_also_escalates_to_fatal(mock_skill1): + """A different missing path on the second call still escalates to RESOURCE_NOT_FOUND_FATAL. + + The counter is path-agnostic: any second not-found within the same invocation + is fatal, even when the LLM hallucinates a different path on each retry. + """ toolset = skill_toolset.SkillToolset([mock_skill1]) tool = skill_toolset.LoadSkillResourceTool(toolset) ctx = _make_tool_context_with_agent() @@ -532,16 +548,17 @@ async def test_load_resource_different_paths_each_soft_fail(mock_skill1): args={"skill_name": "skill1", "file_path": "references/missing_b.md"}, tool_context=ctx, ) - assert result2["error_code"] == "RESOURCE_NOT_FOUND" + assert result2["error_code"] == "RESOURCE_NOT_FOUND_FATAL" + assert "Do not retry" in result2["error"] @pytest.mark.asyncio async def test_load_resource_failures_isolated_per_invocation(mock_skill1): - """Failed-path tracking does not leak across invocations. + """Failure counter does not leak across invocations. - A path that hit a soft RESOURCE_NOT_FOUND in invocation A must still - return RESOURCE_NOT_FOUND (not _FATAL) on its first attempt in - invocation B, even when sharing session state. + A RESOURCE_NOT_FOUND in invocation A must not increment invocation B's + counter; invocation B's first missing-resource call must still return the + soft error, even when both invocations share the same session state dict. """ toolset = skill_toolset.SkillToolset([mock_skill1]) tool = skill_toolset.LoadSkillResourceTool(toolset) @@ -552,22 +569,31 @@ async def test_load_resource_failures_isolated_per_invocation(mock_skill1): ctx_b = _make_tool_context_with_agent(invocation_id="inv_b") ctx_b.state = shared_state - args = {"skill_name": "skill1", "file_path": "references/typo.md"} - - result_a = await tool.run_async(args=args, tool_context=ctx_a) + # invocation A: one failure — counter for inv_a reaches 1 (soft). + result_a = await tool.run_async( + args={"skill_name": "skill1", "file_path": "references/typo.md"}, + tool_context=ctx_a, + ) assert result_a["error_code"] == "RESOURCE_NOT_FOUND" - result_b = await tool.run_async(args=args, tool_context=ctx_b) - assert result_b["error_code"] == "RESOURCE_NOT_FOUND" + # invocation B, first attempt (different path) — counter for inv_b = 1 (soft). + result_b1 = await tool.run_async( + args={"skill_name": "skill1", "file_path": "references/typo.md"}, + tool_context=ctx_b, + ) + assert result_b1["error_code"] == "RESOURCE_NOT_FOUND" - # And the fatal escalation still works within a single invocation. - result_b2 = await tool.run_async(args=args, tool_context=ctx_b) + # invocation B, second attempt (different path) — counter for inv_b = 2 (fatal). + result_b2 = await tool.run_async( + args={"skill_name": "skill1", "file_path": "references/other.md"}, + tool_context=ctx_b, + ) assert result_b2["error_code"] == "RESOURCE_NOT_FOUND_FATAL" @pytest.mark.asyncio -async def test_load_resource_failed_paths_use_temp_prefix(mock_skill1): - """Tracking key uses the `temp:` prefix so it is not persisted.""" +async def test_load_resource_counter_uses_temp_prefix(mock_skill1): + """Failure-counter key uses the `temp:` prefix so it is not persisted.""" toolset = skill_toolset.SkillToolset([mock_skill1]) tool = skill_toolset.LoadSkillResourceTool(toolset) ctx = _make_tool_context_with_agent() @@ -577,10 +603,10 @@ async def test_load_resource_failed_paths_use_temp_prefix(mock_skill1): tool_context=ctx, ) - # Every key written by the retry guard must start with `temp:` so it gets - # trimmed from the event delta and never reaches durable storage. - guard_keys = [k for k in ctx.state if "skill_resource_failed_paths" in k] - assert guard_keys, "Retry guard did not write a tracking key." + # The counter key must start with `temp:` so it is trimmed from the event + # delta and never reaches durable storage. + guard_keys = [k for k in ctx.state if "skill_resource_not_found_count" in k] + assert guard_keys, "Failure counter did not write a tracking key." assert all(k.startswith("temp:") for k in guard_keys) From 187d581f86037f9e01fcd3383e32da4e01f21cd0 Mon Sep 17 00:00:00 2001 From: Raman369AI Date: Tue, 12 May 2026 22:06:25 -0500 Subject: [PATCH 22/31] style: apply pyink formatting --- src/google/adk/tools/skill_toolset.py | 4 +--- tests/unittests/tools/test_skill_toolset.py | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/google/adk/tools/skill_toolset.py b/src/google/adk/tools/skill_toolset.py index a23285c4ca..20e9a04c4d 100644 --- a/src/google/adk/tools/skill_toolset.py +++ b/src/google/adk/tools/skill_toolset.py @@ -359,9 +359,7 @@ async def run_async( # paths so the guard fires even when the LLM hallucinates a different path # on each retry. The `temp:` prefix prevents persistence to durable # session storage; invocation_id isolates in-memory backends. - counter_key = ( - f"temp:_adk_skill_resource_not_found_count_{tool_context.invocation_id}" - ) + counter_key = f"temp:_adk_skill_resource_not_found_count_{tool_context.invocation_id}" fail_count = int(tool_context.state.get(counter_key) or 0) + 1 tool_context.state[counter_key] = fail_count if fail_count > 1: diff --git a/tests/unittests/tools/test_skill_toolset.py b/tests/unittests/tools/test_skill_toolset.py index e75c6cfc73..ab4d41956d 100644 --- a/tests/unittests/tools/test_skill_toolset.py +++ b/tests/unittests/tools/test_skill_toolset.py @@ -528,7 +528,9 @@ async def test_load_resource_repeated_failure_escalates_to_fatal(mock_skill1): @pytest.mark.asyncio -async def test_load_resource_different_path_also_escalates_to_fatal(mock_skill1): +async def test_load_resource_different_path_also_escalates_to_fatal( + mock_skill1, +): """A different missing path on the second call still escalates to RESOURCE_NOT_FOUND_FATAL. The counter is path-agnostic: any second not-found within the same invocation From c6b231182fe2288b48aaaa95b3390ff5d3305d3a Mon Sep 17 00:00:00 2001 From: Raman369AI Date: Mon, 18 May 2026 18:57:43 -0500 Subject: [PATCH 23/31] docs(skill_toolset): align system instruction with invocation-wide retry guard Rule 5 in _DEFAULT_SKILL_SYSTEM_INSTRUCTION said "do not retry the same path" but the actual guard tracks failures invocation-wide and the RESOURCE_NOT_FOUND_FATAL message instructs "Do not retry any path". Update the prompt to match the implemented behavior. --- src/google/adk/tools/skill_toolset.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/google/adk/tools/skill_toolset.py b/src/google/adk/tools/skill_toolset.py index 20e9a04c4d..9e99676a96 100644 --- a/src/google/adk/tools/skill_toolset.py +++ b/src/google/adk/tools/skill_toolset.py @@ -88,8 +88,8 @@ "4. Use `run_skill_script` to run scripts from a skill's `scripts/` " "directory. Use `load_skill_resource` to view script content first if " "needed.\n" - "5. If `load_skill_resource` returns any error, do not retry the " - "same path. Report the error to the user and stop.\n" + "5. If `load_skill_resource` returns any error, do not retry any " + "path. Report the error to the user and stop.\n" ) From 2d465aa2e1569b27d2a348529f3cb5860a5a9783 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 22 May 2026 15:12:36 -0700 Subject: [PATCH 24/31] docs(skills): Use default model in sample agent templates Sample agents should not specify a model explicitly to ensure they default to the system-configured fallback model, unless a specific model is explicitly requested. Change-Id: I52817766b132b6a72ef6a40aa97012d218637cb1 --- .agents/skills/adk-sample-creator/SKILL.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.agents/skills/adk-sample-creator/SKILL.md b/.agents/skills/adk-sample-creator/SKILL.md index 4216665f23..c8f22f1f92 100644 --- a/.agents/skills/adk-sample-creator/SKILL.md +++ b/.agents/skills/adk-sample-creator/SKILL.md @@ -27,6 +27,9 @@ Use snake_case for the folder name (e.g., `dynamic_nodes`, `fan_out_fan_in`). The `agent.py` should focus on demonstrating a specific feature or agent pattern. Use absolute imports for testing convenience. +> [!IMPORTANT] +> **Model Selection**: Do not set the `model` parameter explicitly (e.g., `model="gemini-2.5-flash"`) on `Agent` instances in sample agents. Instead, let them default to the system-configured model, unless a specific model is explicitly requested by the user. + Choose one of the following patterns: #### Pattern A: Workflows (for complex graphs) @@ -74,7 +77,6 @@ from google.adk.tools import google_search # example ```python root_agent = Agent( name="standalone_assistant", - model="gemini-2.5-flash", instruction="You are a helpful assistant.", description="An assistant that can help with queries.", tools=[google_search], From c56bec8d6d6d9823744a6032e18a640d4c71cae0 Mon Sep 17 00:00:00 2001 From: Google Team Member Date: Tue, 26 May 2026 11:25:40 -0700 Subject: [PATCH 25/31] fix(runners): Preserve state_delta in NodeRunner path Merge a0d90de9c2b5c32c6a08c7e033d8ea906a2b75dd into e623b3b4db9109d2f53915a7c3e8140db71ca336 Merge https://github.com/google/adk-python/pull/5767 Resolves https://github.com/google/adk-python/issues/5763 ORIGINAL_AUTHOR=trongthanht3 GitOrigin-RevId: 3c3fb062a2583a676f770316eb0449f8906b628f Change-Id: I4281c6d6d68e5beb2998cf7698f3210854fd5199 --- src/google/adk/runners.py | 31 +++-- tests/unittests/runners/test_runner_node.py | 129 ++++++++++++++++++++ 2 files changed, 153 insertions(+), 7 deletions(-) diff --git a/src/google/adk/runners.py b/src/google/adk/runners.py index 053f2b3fec..0618d67b6a 100644 --- a/src/google/adk/runners.py +++ b/src/google/adk/runners.py @@ -455,6 +455,7 @@ async def _run_node_async( user_id: str, session_id: str, new_message: Optional[types.Content] = None, + state_delta: Optional[dict[str, Any]] = None, run_config: Optional[RunConfig] = None, yield_user_message: bool = False, node: Optional['BaseNode'] = None, @@ -512,7 +513,9 @@ async def _run_node_async( # Append user message to session for history if new_message: - user_event = await self._append_user_event(ic, new_message) + user_event = await self._append_user_event( + ic, new_message, state_delta=state_delta + ) if yield_user_message and user_event: yield user_event @@ -706,14 +709,26 @@ def _resolve_invocation_id_from_fr( return invocation_ids.pop() async def _append_user_event( - self, ic: InvocationContext, content: types.Content + self, + ic: InvocationContext, + content: types.Content, + *, + state_delta: Optional[dict[str, Any]] = None, ) -> Event: """Append a user message event to the session and return it.""" - event = Event( - invocation_id=ic.invocation_id, - author='user', - content=content, - ) + if state_delta: + event = Event( + invocation_id=ic.invocation_id, + author='user', + actions=EventActions(state_delta=state_delta), + content=content, + ) + else: + event = Event( + invocation_id=ic.invocation_id, + author='user', + content=content, + ) # when a paused task delegation is in flight, stamp # the new user message with that task's isolation_scope so the # task agent's content-build (scoped to ) sees it. @@ -989,6 +1004,7 @@ async def run_async( user_id=user_id, session_id=session_id, new_message=new_message, + state_delta=state_delta, run_config=run_config, yield_user_message=yield_user_message, node=agent_to_run, @@ -1008,6 +1024,7 @@ async def run_async( user_id=user_id, session_id=session_id, new_message=new_message, + state_delta=state_delta, run_config=run_config, yield_user_message=yield_user_message, ) diff --git a/tests/unittests/runners/test_runner_node.py b/tests/unittests/runners/test_runner_node.py index 7c01ce44a3..5abdfee529 100644 --- a/tests/unittests/runners/test_runner_node.py +++ b/tests/unittests/runners/test_runner_node.py @@ -24,7 +24,9 @@ from typing import Any from typing import AsyncGenerator +from google.adk.agents.callback_context import CallbackContext from google.adk.agents.context import Context +from google.adk.agents.llm_agent import LlmAgent from google.adk.events.event import Event from google.adk.runners import Runner from google.adk.sessions.in_memory_session_service import InMemorySessionService @@ -49,6 +51,10 @@ async def _run_impl( yield f'Echo: {text}' +def _user_message(text: str = 'hello') -> types.Content: + return types.Content(parts=[types.Part(text=text)], role='user') + + async def _run_node(node, message='hello'): """Run a BaseNode via Runner(node=...) and return (events, ss, session).""" ss = InMemorySessionService() @@ -288,6 +294,129 @@ async def test_yield_user_message_false_by_default(): assert user_events == [] +@pytest.mark.asyncio +async def test_node_runner_applies_state_delta_before_base_node_runs(): + """A BaseNode sees run_async state_delta as session state.""" + + class _StateReaderNode(BaseNode): + + async def _run_impl( + self, *, ctx: Context, node_input: Any + ) -> AsyncGenerator[Any, None]: + yield f'state:{ctx.state["test_state"]}' + + session_service = InMemorySessionService() + runner = Runner( + app_name='test', + node=_StateReaderNode(name='reader'), + session_service=session_service, + ) + session = await session_service.create_session(app_name='test', user_id='u') + + events: list[Event] = [] + async for event in runner.run_async( + user_id='u', + session_id=session.id, + new_message=_user_message(), + state_delta={'test_state': 'must_change'}, + ): + events.append(event) + + updated = await session_service.get_session( + app_name='test', user_id='u', session_id=session.id + ) + user_events = [event for event in updated.events if event.author == 'user'] + + assert [event.output for event in events if event.output is not None] == [ + 'state:must_change' + ] + assert updated.state['test_state'] == 'must_change' + assert user_events[0].actions.state_delta == {'test_state': 'must_change'} + + +@pytest.mark.asyncio +async def test_node_runner_yields_user_event_with_state_delta(): + """yield_user_message=True yields the user event with state_delta.""" + + class _NoopNode(BaseNode): + + async def _run_impl( + self, *, ctx: Context, node_input: Any + ) -> AsyncGenerator[Any, None]: + yield 'done' + + session_service = InMemorySessionService() + runner = Runner( + app_name='test', + node=_NoopNode(name='noop'), + session_service=session_service, + ) + session = await session_service.create_session(app_name='test', user_id='u') + + events: list[Event] = [] + async for event in runner.run_async( + user_id='u', + session_id=session.id, + new_message=_user_message(), + state_delta={'test_state': 'must_change'}, + yield_user_message=True, + ): + events.append(event) + + assert events[0].author == 'user' + assert events[0].actions.state_delta == {'test_state': 'must_change'} + + +@pytest.mark.asyncio +async def test_node_runner_applies_state_delta_before_llm_agent_runs(): + """An LlmAgent callback sees run_async state_delta before model execution.""" + + captured_state_value = None + + def _before_agent_callback( + callback_context: CallbackContext, + ) -> types.Content: + nonlocal captured_state_value + captured_state_value = callback_context.state['test_state'] + return types.Content( + role='model', + parts=[types.Part(text=f'state:{captured_state_value}')], + ) + + session_service = InMemorySessionService() + agent = LlmAgent( + name='state_agent', + before_agent_callback=_before_agent_callback, + ) + runner = Runner(app_name='test', agent=agent, session_service=session_service) + session = await session_service.create_session(app_name='test', user_id='u') + + events: list[Event] = [] + async for event in runner.run_async( + user_id='u', + session_id=session.id, + new_message=_user_message(), + state_delta={'test_state': 'must_change'}, + ): + events.append(event) + + updated = await session_service.get_session( + app_name='test', user_id='u', session_id=session.id + ) + user_events = [event for event in updated.events if event.author == 'user'] + response_texts = [ + part.text + for event in events + if event.content + for part in event.content.parts + if part.text + ] + + assert captured_state_value == 'must_change' + assert 'state:must_change' in response_texts + assert user_events[0].actions.state_delta == {'test_state': 'must_change'} + + # --------------------------------------------------------------------------- # Resume (HITL) # --------------------------------------------------------------------------- From 9d7195aaa6bde42cf4d8c9d7089a2dc234f68984 Mon Sep 17 00:00:00 2001 From: Google Team Member Date: Tue, 26 May 2026 10:56:29 -0700 Subject: [PATCH 26/31] chore: update bots ORIGINAL_AUTHOR=Rohit Yanamadala GitOrigin-RevId: 3e02a5cc7aa77522acfeaf34eb2a92b5f86a0b95 Change-Id: Ib76fb44ae514141eb39cfa23441a38e3fc477afd --- .github/workflows/issue-monitor.yml | 2 +- .github/workflows/stale-bot.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/issue-monitor.yml b/.github/workflows/issue-monitor.yml index 6fee4c9a7d..2f0ca04612 100644 --- a/.github/workflows/issue-monitor.yml +++ b/.github/workflows/issue-monitor.yml @@ -58,6 +58,6 @@ jobs: REPO: ${{ github.event.repository.name }} CONCURRENCY_LIMIT: 3 INITIAL_FULL_SCAN: ${{ github.event.inputs.full_scan == 'true' }} - LLM_MODEL_NAME: "gemini-2.5-flash" + LLM_MODEL_NAME: "gemini-3.5-flash" PYTHONPATH: contributing/samples/adk_team run: python -m adk_issue_monitoring_agent.main diff --git a/.github/workflows/stale-bot.yml b/.github/workflows/stale-bot.yml index e008549a6d..5653dd9eee 100644 --- a/.github/workflows/stale-bot.yml +++ b/.github/workflows/stale-bot.yml @@ -38,7 +38,7 @@ jobs: OWNER: ${{ github.repository_owner }} REPO: adk-python CONCURRENCY_LIMIT: 3 - LLM_MODEL_NAME: "gemini-2.5-flash" + LLM_MODEL_NAME: "gemini-3.5-flash" PYTHONPATH: contributing/samples/adk_team run: python -m adk_stale_agent.main From 91ded3a1dec8b6aafa92a59484b74de198d44cf3 Mon Sep 17 00:00:00 2001 From: Kathy Wu Date: Tue, 26 May 2026 20:18:39 +0000 Subject: [PATCH 27/31] chore: Remove experimental tag from SkillToolset Graduates SKILL_TOOLSET from EXPERIMENTAL to STABLE: removes the @experimental decorator from all SkillToolset tools, drops the __getattr__ deprecation warning on DEFAULT_SKILL_SYSTEM_INSTRUCTION, and renames _DEFAULT_SKILL_SYSTEM_INSTRUCTION to its public form. Change-Id: I43704a730ccb6ddb9b23ad530d1eb5565f080c6b --- src/google/adk/features/_feature_registry.py | 2 +- src/google/adk/tools/bash_tool.py | 2 -- src/google/adk/tools/skill_toolset.py | 26 ++------------------ tests/unittests/tools/test_skill_toolset.py | 8 ------ 4 files changed, 3 insertions(+), 35 deletions(-) diff --git a/src/google/adk/features/_feature_registry.py b/src/google/adk/features/_feature_registry.py index fbea1d2b2d..d155e3b7ac 100644 --- a/src/google/adk/features/_feature_registry.py +++ b/src/google/adk/features/_feature_registry.py @@ -150,7 +150,7 @@ class FeatureConfig: FeatureStage.EXPERIMENTAL, default_on=True ), FeatureName.SKILL_TOOLSET: FeatureConfig( - FeatureStage.EXPERIMENTAL, default_on=True + FeatureStage.STABLE, default_on=True ), FeatureName.SPANNER_ADMIN_TOOLSET: FeatureConfig( FeatureStage.EXPERIMENTAL, default_on=True diff --git a/src/google/adk/tools/bash_tool.py b/src/google/adk/tools/bash_tool.py index fa1925205d..b9aba1554e 100644 --- a/src/google/adk/tools/bash_tool.py +++ b/src/google/adk/tools/bash_tool.py @@ -29,7 +29,6 @@ from google.genai import types -from .. import features from .base_tool import BaseTool from .tool_context import ToolContext @@ -99,7 +98,6 @@ def _set_resource_limits(policy: BashToolPolicy) -> None: logger.warning("Failed to set resource limits: %s", e) -@features.experimental(features.FeatureName.SKILL_TOOLSET) class ExecuteBashTool(BaseTool): """Tool to execute a validated bash command within a workspace directory.""" diff --git a/src/google/adk/tools/skill_toolset.py b/src/google/adk/tools/skill_toolset.py index 7d8e06368e..f3c32162a8 100644 --- a/src/google/adk/tools/skill_toolset.py +++ b/src/google/adk/tools/skill_toolset.py @@ -26,7 +26,6 @@ from typing import Any from typing import Optional from typing import TYPE_CHECKING -import warnings from google.genai import types from typing_extensions import override @@ -34,8 +33,6 @@ from ..agents.readonly_context import ReadonlyContext from ..code_executors.base_code_executor import BaseCodeExecutor from ..code_executors.code_execution_utils import CodeExecutionInput -from ..features import experimental -from ..features import FeatureName from ..skills import models from ..skills import prompt from ..skills import SkillRegistry @@ -59,7 +56,7 @@ " conversation history for you to analyze." ) -_DEFAULT_SKILL_SYSTEM_INSTRUCTION = ( +DEFAULT_SKILL_SYSTEM_INSTRUCTION = ( "You can use specialized 'skills' to help you with complex tasks. " "You MUST use the skill tools to interact with these skills.\n\n" "Skills are folders of instructions and resources that extend your " @@ -89,7 +86,6 @@ ) -@experimental(FeatureName.SKILL_TOOLSET) class ListSkillsTool(BaseTool): """Tool to list all available skills.""" @@ -119,7 +115,6 @@ async def run_async( return prompt.format_skills_as_xml(skills) -@experimental(FeatureName.SKILL_TOOLSET) class SearchSkillsTool(BaseTool): """Tool to search for relevant skills in the registry.""" @@ -182,7 +177,6 @@ async def run_async( } -@experimental(FeatureName.SKILL_TOOLSET) class LoadSkillTool(BaseTool): """Tool to load a skill's instructions.""" @@ -258,7 +252,6 @@ def _detect_error_in_response(self, response: Any) -> Optional[str]: return None -@experimental(FeatureName.SKILL_TOOLSET) class LoadSkillResourceTool(BaseTool): """Tool to load resources (references, assets, or scripts) from a skill.""" @@ -725,7 +718,6 @@ def _build_wrapper_code( return "\n".join(code_lines) -@experimental(FeatureName.SKILL_TOOLSET) class RunSkillScriptTool(BaseTool): """Tool to execute scripts from a skill's scripts/ directory.""" @@ -896,7 +888,6 @@ def _detect_error_in_response(self, response: Any) -> Optional[str]: return None -@experimental(FeatureName.SKILL_TOOLSET) class SkillToolset(BaseToolset): """A toolset for managing and interacting with agent skills.""" @@ -1085,7 +1076,7 @@ async def process_llm_request( self, *, tool_context: ToolContext, llm_request: LlmRequest ) -> None: """Processes the outgoing LLM request to include available skills.""" - instructions = [_DEFAULT_SKILL_SYSTEM_INSTRUCTION] + instructions = [DEFAULT_SKILL_SYSTEM_INSTRUCTION] has_list_skills = any(isinstance(t, ListSkillsTool) for t in self._tools) @@ -1112,16 +1103,3 @@ async def close(self) -> None: cached.cancel() self._fetched_skill_cache.clear() await super().close() - - -def __getattr__(name: str) -> Any: - if name == "DEFAULT_SKILL_SYSTEM_INSTRUCTION": - warnings.warn( - "DEFAULT_SKILL_SYSTEM_INSTRUCTION is experimental. Its content " - "is internal implementation and will change in minor/patch releases " - "to tune agent performance.", - UserWarning, - stacklevel=2, - ) - return _DEFAULT_SKILL_SYSTEM_INSTRUCTION - raise AttributeError(f"module {__name__} has no attribute {name}") diff --git a/tests/unittests/tools/test_skill_toolset.py b/tests/unittests/tools/test_skill_toolset.py index f637377513..aca34c2523 100644 --- a/tests/unittests/tools/test_skill_toolset.py +++ b/tests/unittests/tools/test_skill_toolset.py @@ -471,14 +471,6 @@ async def test_process_llm_request_without_list_skills_tool( assert "skill2" in instructions[1] -def test_default_skill_system_instruction_warning(): - with pytest.warns( - UserWarning, match="DEFAULT_SKILL_SYSTEM_INSTRUCTION is experimental" - ): - instruction = skill_toolset.DEFAULT_SKILL_SYSTEM_INSTRUCTION - assert "specialized 'skills'" in instruction - - def test_duplicate_skill_name_raises(mock_skill1): skill_dup = mock.create_autospec(models.Skill, instance=True) skill_dup.name = "skill1" From 42c6e9072360f95da006a23a8d33bff1ac6fd80b Mon Sep 17 00:00:00 2001 From: "Wei (Jack) Sun" Date: Tue, 26 May 2026 13:27:00 -0700 Subject: [PATCH 28/31] chore(git): Ignore local Antigravity configuration directories Prevent accidental check-in of Antigravity agent CLI logs, configurations, and caches during development. Change-Id: Iaa8b8a48229614b10f2f727f40f1ffb32f45b45b --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index a4f3937c76..c3ddc7ea99 100644 --- a/.gitignore +++ b/.gitignore @@ -104,6 +104,7 @@ Thumbs.db .adk/ .claude/ .jetski* +.antigravity* CLAUDE.md .cursor/ .cursorrules From 1cc298edb801f9a4538aa532f3fa1c4d4187dc96 Mon Sep 17 00:00:00 2001 From: "Wei (Jack) Sun" Date: Tue, 26 May 2026 13:45:29 -0700 Subject: [PATCH 29/31] refactor(tools): Split environment tools into single-class _tool files Split the merged _tools.py into four private, single-class modules: _edit_file_tool.py, _execute_tool.py, _read_file_tool.py, and _write_file_tool.py. This aligns the environment tool implementations with ADK's file organization guidelines and improves maintainability. Change-Id: I351ac58e4cc30297db720aaa56c28760045c482d --- .../adk/tools/environment/_edit_file_tool.py | 140 ++++++ .../tools/environment/_environment_toolset.py | 8 +- .../adk/tools/environment/_execute_tool.py | 137 ++++++ .../adk/tools/environment/_read_file_tool.py | 168 +++++++ src/google/adk/tools/environment/_tools.py | 435 ------------------ src/google/adk/tools/environment/_utils.py | 26 ++ .../adk/tools/environment/_write_file_tool.py | 92 ++++ tests/unittests/telemetry/test_spans.py | 8 +- ...ls_edit_file.py => test_edit_file_tool.py} | 2 +- 9 files changed, 572 insertions(+), 444 deletions(-) create mode 100644 src/google/adk/tools/environment/_edit_file_tool.py create mode 100644 src/google/adk/tools/environment/_execute_tool.py create mode 100644 src/google/adk/tools/environment/_read_file_tool.py delete mode 100644 src/google/adk/tools/environment/_tools.py create mode 100644 src/google/adk/tools/environment/_utils.py create mode 100644 src/google/adk/tools/environment/_write_file_tool.py rename tests/unittests/tools/environment/{test_tools_edit_file.py => test_edit_file_tool.py} (98%) diff --git a/src/google/adk/tools/environment/_edit_file_tool.py b/src/google/adk/tools/environment/_edit_file_tool.py new file mode 100644 index 0000000000..fa9f3143f9 --- /dev/null +++ b/src/google/adk/tools/environment/_edit_file_tool.py @@ -0,0 +1,140 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""EditFileTool for performing surgical text replacements in existing files.""" + +from __future__ import annotations + +import logging +import re +from typing import Any +from typing import Optional +from typing import TYPE_CHECKING + +from google.genai import types +from typing_extensions import override + +from ...environment._base_environment import BaseEnvironment +from ...utils.feature_decorator import experimental +from ..base_tool import BaseTool + +if TYPE_CHECKING: + from ..tool_context import ToolContext + + +logger = logging.getLogger('google_adk.' + __name__) + + +@experimental +class EditFileTool(BaseTool): + """Perform a surgical text replacement in an existing file.""" + + def __init__(self, environment: BaseEnvironment): + super().__init__( + name='EditFile', + description=( + 'Replace an exact substring in an existing file ' + 'with new text. The old_string must appear exactly ' + 'once in the file. To create new files, use the WriteFile tool.' + ), + ) + self._environment = environment + + @override + def _get_declaration(self) -> Optional[types.FunctionDeclaration]: + return types.FunctionDeclaration( + name=self.name, + description=self.description, + parameters_json_schema={ + 'type': 'object', + 'properties': { + 'path': { + 'type': 'string', + 'description': ( + 'Path of the file to edit within the environment.' + ), + }, + 'old_string': { + 'type': 'string', + 'description': ( + 'The exact text to find and replace. Must not be empty.' + ), + }, + 'new_string': { + 'type': 'string', + 'description': 'The replacement text.', + }, + }, + 'required': ['path', 'old_string', 'new_string'], + }, + ) + + @override + async def run_async( + self, *, args: dict[str, Any], tool_context: ToolContext + ) -> Any: + path = args.get('path', '') + old_string = args.get('old_string', '') + new_string = args.get('new_string', '') + if not path: + return {'status': 'error', 'error': '`path` is required.'} + + if not old_string: + return { + 'status': 'error', + 'error': ( + '`old_string` cannot be empty. To create a new ' + 'file, use the WriteFile tool.' + ), + } + + try: + data_bytes = await self._environment.read_file(path) + content = data_bytes.decode('utf-8', errors='replace') + except FileNotFoundError: + return {'status': 'error', 'error': f'File not found: {path}'} + + # Normalize line breaks in old_string to \n and use regex for flexible matching + normalized_old = old_string.replace('\r\n', '\n') + pattern = re.escape(normalized_old).replace('\n', '\r?\n') + + matches = re.findall(pattern, content) + count = len(matches) + + if count == 0: + return { + 'status': 'error', + 'error': ( + '`old_string` not found in file. Read the file first ' + 'to verify contents.' + ), + } + if count > 1: + return { + 'status': 'error', + 'error': ( + f'`old_string` appears {count} times. Provide more ' + 'surrounding context to make it unique.' + ), + } + + new_content = re.sub(pattern, lambda m: new_string, content, count=1) + await self._environment.write_file(path, new_content) + return {'status': 'ok', 'message': f'Edited {path}'} + + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None diff --git a/src/google/adk/tools/environment/_environment_toolset.py b/src/google/adk/tools/environment/_environment_toolset.py index 2d894c3325..1dfaff7a39 100644 --- a/src/google/adk/tools/environment/_environment_toolset.py +++ b/src/google/adk/tools/environment/_environment_toolset.py @@ -26,10 +26,10 @@ from ...utils.feature_decorator import experimental from ..base_toolset import BaseToolset from ._constants import ENVIRONMENT_INSTRUCTION -from ._tools import EditFileTool -from ._tools import ExecuteTool -from ._tools import ReadFileTool -from ._tools import WriteFileTool +from ._edit_file_tool import EditFileTool +from ._execute_tool import ExecuteTool +from ._read_file_tool import ReadFileTool +from ._write_file_tool import WriteFileTool if TYPE_CHECKING: from ...agents.readonly_context import ReadonlyContext diff --git a/src/google/adk/tools/environment/_execute_tool.py b/src/google/adk/tools/environment/_execute_tool.py new file mode 100644 index 0000000000..73b245a089 --- /dev/null +++ b/src/google/adk/tools/environment/_execute_tool.py @@ -0,0 +1,137 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""ExecuteTool for running shell commands in the environment.""" + +from __future__ import annotations + +import logging +from typing import Any +from typing import Optional +from typing import TYPE_CHECKING + +from google.genai import types +from typing_extensions import override + +from ...environment._base_environment import BaseEnvironment +from ...environment._base_environment import ExecutionResult +from ...utils.feature_decorator import experimental +from ..base_tool import BaseTool +from ._constants import DEFAULT_TIMEOUT +from ._constants import MAX_OUTPUT_CHARS +from ._utils import truncate as _truncate + +if TYPE_CHECKING: + from ..tool_context import ToolContext + + +logger = logging.getLogger('google_adk.' + __name__) + + +_EXECUTE_TOOL_DESCRIPTION = """ +Run a shell command in the environment. For running programs, tests, and build +commands ONLY. WARNING: Do NOT use for file reading -- use the ReadFile tool +instead. Shell commands like 'cat, head, tail will produce inferior results. +Good: Execute("python3 script.py"), Execute("pytest"), Execute("find ..."). +Bad: Execute("head ..."), Execute("cat ..."). +""" + + +@experimental +class ExecuteTool(BaseTool): + """Run a shell command in the environment's working directory.""" + + def __init__( + self, + environment: BaseEnvironment, + *, + max_output_chars: Optional[int] = None, + ): + super().__init__( + name='Execute', + description=_EXECUTE_TOOL_DESCRIPTION, + ) + self._environment = environment + self._max_output_chars = ( + max_output_chars if max_output_chars is not None else MAX_OUTPUT_CHARS + ) + + @override + def _get_declaration(self) -> Optional[types.FunctionDeclaration]: + return types.FunctionDeclaration( + name=self.name, + description=self.description, + parameters_json_schema={ + 'type': 'object', + 'properties': { + 'command': { + 'type': 'string', + 'description': ( + 'The shell command to execute. Chain dependent commands' + ' with &&.' + ), + }, + }, + 'required': ['command'], + }, + ) + + @override + async def run_async( + self, *, args: dict[str, Any], tool_context: ToolContext + ) -> Any: + command = args.get('command', '') + if not command: + return {'status': 'error', 'error': '`command` is required.'} + + logger.debug('Execute command: %s', command) + try: + execution_result: ExecutionResult = await self._environment.execute( + command, timeout=DEFAULT_TIMEOUT + ) + logger.debug( + 'Execute result: exit_code=%d, stdout=%r, stderr=%r, timed_out=%r', + execution_result.exit_code, + execution_result.stdout[:200] if execution_result.stdout else '', + execution_result.stderr[:200] if execution_result.stderr else '', + execution_result.timed_out, + ) + except Exception as e: + logger.exception('Execute failed: %s', e) + return {'status': 'error', 'error': str(e)} + + result: dict[str, Any] = {'status': 'ok'} + if execution_result.stdout: + result['stdout'] = _truncate( + execution_result.stdout, + limit=self._max_output_chars, + ) + if execution_result.stderr: + result['stderr'] = _truncate( + execution_result.stderr, + limit=self._max_output_chars, + ) + if execution_result.exit_code != 0: + result['status'] = 'error' + result['exit_code'] = execution_result.exit_code + if execution_result.timed_out: + result['status'] = 'error' + result['error'] = f'Command timed out after {DEFAULT_TIMEOUT}s.' + return result + + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None diff --git a/src/google/adk/tools/environment/_read_file_tool.py b/src/google/adk/tools/environment/_read_file_tool.py new file mode 100644 index 0000000000..e3342d4d0a --- /dev/null +++ b/src/google/adk/tools/environment/_read_file_tool.py @@ -0,0 +1,168 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""ReadFileTool for reading file contents in the environment.""" + +from __future__ import annotations + +import logging +from typing import Any +from typing import Optional +from typing import TYPE_CHECKING + +from google.genai import types +from typing_extensions import override + +from ...environment._base_environment import BaseEnvironment +from ...utils.feature_decorator import experimental +from ..base_tool import BaseTool +from ._constants import MAX_OUTPUT_CHARS +from ._utils import truncate as _truncate + +if TYPE_CHECKING: + from ..tool_context import ToolContext + + +logger = logging.getLogger('google_adk.' + __name__) + + +@experimental +class ReadFileTool(BaseTool): + """Read a file from the environment.""" + + def __init__( + self, + environment: BaseEnvironment, + *, + max_output_chars: Optional[int] = None, + ): + super().__init__( + name='ReadFile', + description=( + 'Read the contents of a file in the environment. ' + 'Returns the file content with line numbers.' + ), + ) + self._environment = environment + self._max_output_chars = ( + max_output_chars if max_output_chars is not None else MAX_OUTPUT_CHARS + ) + + @override + def _get_declaration(self) -> Optional[types.FunctionDeclaration]: + return types.FunctionDeclaration( + name=self.name, + description=self.description, + parameters_json_schema={ + 'type': 'object', + 'properties': { + 'path': { + 'type': 'string', + 'description': ( + 'Path of the file to read within the environment.' + ), + }, + 'start_line': { + 'type': 'integer', + 'description': ( + 'First line to return (1-based, ' + 'inclusive). Defaults to 1.' + ), + }, + 'end_line': { + 'type': 'integer', + 'description': ( + 'Last line to return (1-based, ' + 'inclusive). Defaults to end of file.' + ), + }, + }, + 'required': ['path'], + }, + ) + + @override + async def run_async( + self, *, args: dict[str, Any], tool_context: ToolContext + ) -> Any: + path = args.get('path', '') + if not path: + return {'status': 'error', 'error': '`path` is required.'} + start_line = args.get('start_line') + end_line = args.get('end_line') + + # Use `sed` to read the file if start_line or end_line are specified. + if (start_line and start_line > 1) or end_line: + start = start_line or 1 + if end_line: + sed_range = f'{start},{end_line}' + else: + sed_range = f'{start},$' + # TODO: use shlex.quote to quote `path` and `sed_range`. + cmd = f"cat -n '{path}' | sed -n '{sed_range}p'" + res = await self._environment.execute(cmd) + if res.exit_code == 0: + return { + 'status': 'ok', + 'content': _truncate( + res.stdout, + limit=self._max_output_chars, + ), + } + + try: + data_bytes = await self._environment.read_file(path) + text = data_bytes.decode('utf-8', errors='replace') + lines = text.splitlines(True) + total = len(lines) + start = max(1, start_line or 1) + end = min(total, end_line or total) + if start > total: + return { + 'status': 'error', + 'error': ( + f'`start_line` {start} exceeds file length ({total} lines).' + ), + 'total_lines': total, + } + if start > end: + return { + 'status': 'error', + 'error': f'`start_line` ({start}) is after `end_line` ({end}).', + 'total_lines': total, + } + selected = lines[start - 1 : end] + numbered = ''.join( + f'{start + i:6d}\t{line}' for i, line in enumerate(selected) + ) + result = { + 'status': 'ok', + 'content': _truncate( + numbered, + limit=self._max_output_chars, + ), + } + if start > 1 or end < total: + result['total_lines'] = total + return result + except FileNotFoundError: + return {'status': 'error', 'error': f'File not found: {path}'} + except Exception as e: + return {'status': 'error', 'error': str(e)} + + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None diff --git a/src/google/adk/tools/environment/_tools.py b/src/google/adk/tools/environment/_tools.py deleted file mode 100644 index ad339cf48f..0000000000 --- a/src/google/adk/tools/environment/_tools.py +++ /dev/null @@ -1,435 +0,0 @@ -# Copyright 2026 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Tools available in the environment toolset.""" - -from __future__ import annotations - -import logging -import re -from typing import Any -from typing import Optional -from typing import TYPE_CHECKING - -from google.genai import types -from typing_extensions import override - -from ...environment._base_environment import BaseEnvironment -from ...environment._base_environment import ExecutionResult -from ...utils.feature_decorator import experimental -from ..base_tool import BaseTool -from ._constants import DEFAULT_TIMEOUT -from ._constants import MAX_OUTPUT_CHARS - -if TYPE_CHECKING: - from ..tool_context import ToolContext - - -logger = logging.getLogger('google_adk.' + __name__) - - -def _truncate(text: str, limit: int = MAX_OUTPUT_CHARS) -> str: - """Truncate text to *limit* characters with a notice.""" - if len(text) <= limit: - return text - return text[:limit] + f'\n... (truncated, {len(text)} total chars)' - - -_EXECUTE_TOOL_DESCRIPTION = """ -Run a shell command in the environment. For running programs, tests, and build -commands ONLY. WARNING: Do NOT use for file reading -- use the ReadFile tool -instead. Shell commands like 'cat, head, tail will produce inferior results. -Good: Execute("python3 script.py"), Execute("pytest"), Execute("find ..."). -Bad: Execute("head ..."), Execute("cat ..."). -""" - - -@experimental -class ExecuteTool(BaseTool): - """Run a shell command in the environment's working directory.""" - - def __init__( - self, - environment: BaseEnvironment, - *, - max_output_chars: Optional[int] = None, - ): - super().__init__( - name='Execute', - description=_EXECUTE_TOOL_DESCRIPTION, - ) - self._environment = environment - self._max_output_chars = ( - max_output_chars if max_output_chars is not None else MAX_OUTPUT_CHARS - ) - - @override - def _get_declaration(self) -> Optional[types.FunctionDeclaration]: - return types.FunctionDeclaration( - name=self.name, - description=self.description, - parameters_json_schema={ - 'type': 'object', - 'properties': { - 'command': { - 'type': 'string', - 'description': ( - 'The shell command to execute. Chain dependent commands' - ' with &&.' - ), - }, - }, - 'required': ['command'], - }, - ) - - @override - async def run_async( - self, *, args: dict[str, Any], tool_context: ToolContext - ) -> Any: - command = args.get('command', '') - if not command: - return {'status': 'error', 'error': '`command` is required.'} - - logger.debug('Execute command: %s', command) - try: - execution_result: ExecutionResult = await self._environment.execute( - command, timeout=DEFAULT_TIMEOUT - ) - logger.debug( - 'Execute result: exit_code=%d, stdout=%r, stderr=%r, timed_out=%r', - execution_result.exit_code, - execution_result.stdout[:200] if execution_result.stdout else '', - execution_result.stderr[:200] if execution_result.stderr else '', - execution_result.timed_out, - ) - except Exception as e: - logger.exception('Execute failed: %s', e) - return {'status': 'error', 'error': str(e)} - - result: dict[str, Any] = {'status': 'ok'} - if execution_result.stdout: - result['stdout'] = _truncate( - execution_result.stdout, - limit=self._max_output_chars, - ) - if execution_result.stderr: - result['stderr'] = _truncate( - execution_result.stderr, - limit=self._max_output_chars, - ) - if execution_result.exit_code != 0: - result['status'] = 'error' - result['exit_code'] = execution_result.exit_code - if execution_result.timed_out: - result['status'] = 'error' - result['error'] = f'Command timed out after {DEFAULT_TIMEOUT}s.' - return result - - def _detect_error_in_response(self, response: Any) -> Optional[str]: - """Telemetry hook: returns an error type if the response indicates an error.""" - if isinstance(response, dict) and response.get('status') == 'error': - return 'TOOL_ERROR' - return None - - -@experimental -class ReadFileTool(BaseTool): - """Read a file from the environment.""" - - def __init__( - self, - environment: BaseEnvironment, - *, - max_output_chars: Optional[int] = None, - ): - super().__init__( - name='ReadFile', - description=( - 'Read the contents of a file in the environment. ' - 'Returns the file content with line numbers.' - ), - ) - self._environment = environment - self._max_output_chars = ( - max_output_chars if max_output_chars is not None else MAX_OUTPUT_CHARS - ) - - @override - def _get_declaration(self) -> Optional[types.FunctionDeclaration]: - return types.FunctionDeclaration( - name=self.name, - description=self.description, - parameters_json_schema={ - 'type': 'object', - 'properties': { - 'path': { - 'type': 'string', - 'description': ( - 'Path of the file to read within the environment.' - ), - }, - 'start_line': { - 'type': 'integer', - 'description': ( - 'First line to return (1-based, ' - 'inclusive). Defaults to 1.' - ), - }, - 'end_line': { - 'type': 'integer', - 'description': ( - 'Last line to return (1-based, ' - 'inclusive). Defaults to end of file.' - ), - }, - }, - 'required': ['path'], - }, - ) - - @override - async def run_async( - self, *, args: dict[str, Any], tool_context: ToolContext - ) -> Any: - path = args.get('path', '') - if not path: - return {'status': 'error', 'error': '`path` is required.'} - start_line = args.get('start_line') - end_line = args.get('end_line') - - # Use `sed` to read the file if start_line or end_line are specified. - if (start_line and start_line > 1) or end_line: - start = start_line or 1 - if end_line: - sed_range = f'{start},{end_line}' - else: - sed_range = f'{start},$' - cmd = f"cat -n '{path}' | sed -n '{sed_range}p'" - res = await self._environment.execute(cmd) - if res.exit_code == 0: - return { - 'status': 'ok', - 'content': _truncate( - res.stdout, - limit=self._max_output_chars, - ), - } - - try: - data_bytes = await self._environment.read_file(path) - text = data_bytes.decode('utf-8', errors='replace') - lines = text.splitlines(True) - total = len(lines) - start = max(1, start_line or 1) - end = min(total, end_line or total) - if start > total: - return { - 'status': 'error', - 'error': ( - f'`start_line` {start} exceeds file length ({total} lines).' - ), - 'total_lines': total, - } - if start > end: - return { - 'status': 'error', - 'error': f'`start_line` ({start}) is after `end_line` ({end}).', - 'total_lines': total, - } - selected = lines[start - 1 : end] - numbered = ''.join( - f'{start + i:6d}\t{line}' for i, line in enumerate(selected) - ) - result = { - 'status': 'ok', - 'content': _truncate( - numbered, - limit=self._max_output_chars, - ), - } - if start > 1 or end < total: - result['total_lines'] = total - return result - except FileNotFoundError: - return {'status': 'error', 'error': f'File not found: {path}'} - except Exception as e: - return {'status': 'error', 'error': str(e)} - - def _detect_error_in_response(self, response: Any) -> Optional[str]: - """Telemetry hook: returns an error type if the response indicates an error.""" - if isinstance(response, dict) and response.get('status') == 'error': - return 'TOOL_ERROR' - return None - - -@experimental -class WriteFileTool(BaseTool): - """Create or overwrite a file in the environment.""" - - def __init__(self, environment: BaseEnvironment): - super().__init__( - name='WriteFile', - description=( - 'Create or overwrite a file in the environment. ' - 'Use for new files or full rewrites. For small ' - 'changes to existing files, prefer EditFile.' - ), - ) - self._environment = environment - - @override - def _get_declaration(self) -> Optional[types.FunctionDeclaration]: - return types.FunctionDeclaration( - name=self.name, - description=self.description, - parameters_json_schema={ - 'type': 'object', - 'properties': { - 'path': { - 'type': 'string', - 'description': 'Path to the file within the environment.', - }, - 'content': { - 'type': 'string', - 'description': 'The full file content to write.', - }, - }, - 'required': ['path', 'content'], - }, - ) - - @override - async def run_async( - self, *, args: dict[str, Any], tool_context: ToolContext - ) -> Any: - path = args.get('path', '') - content = args.get('content', '') - if not path: - return {'status': 'error', 'error': '`path` is required.'} - try: - await self._environment.write_file(path, content) - except Exception as e: - return {'status': 'error', 'error': str(e)} - return {'status': 'ok', 'message': f'Wrote {path}'} - - def _detect_error_in_response(self, response: Any) -> Optional[str]: - """Telemetry hook: returns an error type if the response indicates an error.""" - if isinstance(response, dict) and response.get('status') == 'error': - return 'TOOL_ERROR' - return None - - -@experimental -class EditFileTool(BaseTool): - """Perform a surgical text replacement in an existing file.""" - - def __init__(self, environment: BaseEnvironment): - super().__init__( - name='EditFile', - description=( - 'Replace an exact substring in an existing file ' - 'with new text. The old_string must appear exactly ' - 'once in the file. To create new files, use the WriteFile tool.' - ), - ) - self._environment = environment - - @override - def _get_declaration(self) -> Optional[types.FunctionDeclaration]: - return types.FunctionDeclaration( - name=self.name, - description=self.description, - parameters_json_schema={ - 'type': 'object', - 'properties': { - 'path': { - 'type': 'string', - 'description': ( - 'Path of the file to edit within the environment.' - ), - }, - 'old_string': { - 'type': 'string', - 'description': ( - 'The exact text to find and replace. Must not be empty.' - ), - }, - 'new_string': { - 'type': 'string', - 'description': 'The replacement text.', - }, - }, - 'required': ['path', 'old_string', 'new_string'], - }, - ) - - @override - async def run_async( - self, *, args: dict[str, Any], tool_context: ToolContext - ) -> Any: - path = args.get('path', '') - old_string = args.get('old_string', '') - new_string = args.get('new_string', '') - if not path: - return {'status': 'error', 'error': '`path` is required.'} - - if not old_string: - return { - 'status': 'error', - 'error': ( - '`old_string` cannot be empty. To create a new ' - 'file, use the WriteFile tool.' - ), - } - - try: - data_bytes = await self._environment.read_file(path) - content = data_bytes.decode('utf-8', errors='replace') - except FileNotFoundError: - return {'status': 'error', 'error': f'File not found: {path}'} - - # Normalize line breaks in old_string to \n and use regex for flexible matching - normalized_old = old_string.replace('\r\n', '\n') - pattern = re.escape(normalized_old).replace('\n', '\r?\n') - - matches = re.findall(pattern, content) - count = len(matches) - - if count == 0: - return { - 'status': 'error', - 'error': ( - '`old_string` not found in file. Read the file first ' - 'to verify contents.' - ), - } - if count > 1: - return { - 'status': 'error', - 'error': ( - f'`old_string` appears {count} times. Provide more ' - 'surrounding context to make it unique.' - ), - } - - new_content = re.sub(pattern, lambda m: new_string, content, count=1) - await self._environment.write_file(path, new_content) - return {'status': 'ok', 'message': f'Edited {path}'} - - def _detect_error_in_response(self, response: Any) -> Optional[str]: - """Telemetry hook: returns an error type if the response indicates an error.""" - if isinstance(response, dict) and response.get('status') == 'error': - return 'TOOL_ERROR' - return None diff --git a/src/google/adk/tools/environment/_utils.py b/src/google/adk/tools/environment/_utils.py new file mode 100644 index 0000000000..6e6cc85a16 --- /dev/null +++ b/src/google/adk/tools/environment/_utils.py @@ -0,0 +1,26 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Shared utilities for environment tools.""" + +from __future__ import annotations + +from ._constants import MAX_OUTPUT_CHARS + + +def truncate(text: str, limit: int = MAX_OUTPUT_CHARS) -> str: + """Truncate text to *limit* characters with a notice.""" + if len(text) <= limit: + return text + return text[:limit] + f'\n... (truncated, {len(text)} total chars)' diff --git a/src/google/adk/tools/environment/_write_file_tool.py b/src/google/adk/tools/environment/_write_file_tool.py new file mode 100644 index 0000000000..6bbb47648f --- /dev/null +++ b/src/google/adk/tools/environment/_write_file_tool.py @@ -0,0 +1,92 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""WriteFileTool for creating or overwriting files in the environment.""" + +from __future__ import annotations + +import logging +from typing import Any +from typing import Optional +from typing import TYPE_CHECKING + +from google.genai import types +from typing_extensions import override + +from ...environment._base_environment import BaseEnvironment +from ...utils.feature_decorator import experimental +from ..base_tool import BaseTool + +if TYPE_CHECKING: + from ..tool_context import ToolContext + + +logger = logging.getLogger('google_adk.' + __name__) + + +@experimental +class WriteFileTool(BaseTool): + """Create or overwrite a file in the environment.""" + + def __init__(self, environment: BaseEnvironment): + super().__init__( + name='WriteFile', + description=( + 'Create or overwrite a file in the environment. ' + 'Use for new files or full rewrites. For small ' + 'changes to existing files, prefer EditFile.' + ), + ) + self._environment = environment + + @override + def _get_declaration(self) -> Optional[types.FunctionDeclaration]: + return types.FunctionDeclaration( + name=self.name, + description=self.description, + parameters_json_schema={ + 'type': 'object', + 'properties': { + 'path': { + 'type': 'string', + 'description': 'Path to the file within the environment.', + }, + 'content': { + 'type': 'string', + 'description': 'The full file content to write.', + }, + }, + 'required': ['path', 'content'], + }, + ) + + @override + async def run_async( + self, *, args: dict[str, Any], tool_context: ToolContext + ) -> Any: + path = args.get('path', '') + content = args.get('content', '') + if not path: + return {'status': 'error', 'error': '`path` is required.'} + try: + await self._environment.write_file(path, content) + except Exception as e: + return {'status': 'error', 'error': str(e)} + return {'status': 'ok', 'message': f'Wrote {path}'} + + def _detect_error_in_response(self, response: Any) -> Optional[str]: + """Telemetry hook: returns an error type if the response indicates an error.""" + if isinstance(response, dict) and response.get('status') == 'error': + return 'TOOL_ERROR' + return None diff --git a/tests/unittests/telemetry/test_spans.py b/tests/unittests/telemetry/test_spans.py index 109e77a369..50fc192417 100644 --- a/tests/unittests/telemetry/test_spans.py +++ b/tests/unittests/telemetry/test_spans.py @@ -1624,10 +1624,10 @@ def test_detect_error_bash_tool(): def _environment_tool_classes(): - from google.adk.tools.environment._tools import EditFileTool - from google.adk.tools.environment._tools import ExecuteTool - from google.adk.tools.environment._tools import ReadFileTool - from google.adk.tools.environment._tools import WriteFileTool + from google.adk.tools.environment._edit_file_tool import EditFileTool + from google.adk.tools.environment._execute_tool import ExecuteTool + from google.adk.tools.environment._read_file_tool import ReadFileTool + from google.adk.tools.environment._write_file_tool import WriteFileTool return [ExecuteTool, ReadFileTool, WriteFileTool, EditFileTool] diff --git a/tests/unittests/tools/environment/test_tools_edit_file.py b/tests/unittests/tools/environment/test_edit_file_tool.py similarity index 98% rename from tests/unittests/tools/environment/test_tools_edit_file.py rename to tests/unittests/tools/environment/test_edit_file_tool.py index d96dcb873a..42204096eb 100644 --- a/tests/unittests/tools/environment/test_tools_edit_file.py +++ b/tests/unittests/tools/environment/test_edit_file_tool.py @@ -20,7 +20,7 @@ from pathlib import Path from google.adk.environment._local_environment import LocalEnvironment -from google.adk.tools.environment._tools import EditFileTool +from google.adk.tools.environment._edit_file_tool import EditFileTool import pytest import pytest_asyncio From ad8b6c769d65c293ca58a078ae447fe499209d15 Mon Sep 17 00:00:00 2001 From: "Wei (Jack) Sun" Date: Mon, 18 May 2026 16:37:51 -0700 Subject: [PATCH 30/31] refactor(agents): default model to gemini-3-flash-preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bump LlmAgent.DEFAULT_MODEL from gemini-2.5-flash to gemini-3-flash-preview ahead of the 2026-10-16 earliest-shutdown date for gemini-2.5-flash; see https://ai.google.dev/gemini-api/docs/deprecations. Also updates the docstring/example references in llm_agent_config.py and mcp_toolset.py so the documented default tracks the actual default. DEFAULT_LIVE_MODEL is unchanged — live mode has its own model family (gemini-live-*) and is not affected by the gemini-2.5-flash deprecation. Change-Id: Icfbf1d60e75a4015847b56b24c9da62e52512073 --- src/google/adk/agents/llm_agent.py | 4 ++-- src/google/adk/agents/llm_agent_config.py | 9 +++++---- src/google/adk/tools/mcp_tool/mcp_toolset.py | 1 - 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/google/adk/agents/llm_agent.py b/src/google/adk/agents/llm_agent.py index cd14cefb3b..76dd0789a9 100644 --- a/src/google/adk/agents/llm_agent.py +++ b/src/google/adk/agents/llm_agent.py @@ -193,7 +193,7 @@ async def _convert_tool_union_to_tools( class LlmAgent(BaseAgent): """LLM-based Agent.""" - DEFAULT_MODEL: ClassVar[str] = 'gemini-2.5-flash' + DEFAULT_MODEL: ClassVar[str] = 'gemini-3-flash-preview' """System default model used when no model is set on an agent.""" DEFAULT_LIVE_MODEL: ClassVar[str] = 'gemini-live-2.5-flash-native-audio' @@ -210,7 +210,7 @@ class LlmAgent(BaseAgent): When not set, the agent will inherit the model from its ancestor. If no ancestor provides a model, the agent uses the default model configured via - LlmAgent.set_default_model. The built-in default is gemini-2.5-flash. + LlmAgent.set_default_model. The built-in default is gemini-3-flash-preview. """ instruction: Union[str, InstructionProvider] = '' diff --git a/src/google/adk/agents/llm_agent_config.py b/src/google/adk/agents/llm_agent_config.py index 93ca718094..4a3c7c55a5 100644 --- a/src/google/adk/agents/llm_agent_config.py +++ b/src/google/adk/agents/llm_agent_config.py @@ -55,10 +55,11 @@ class LlmAgentConfig(BaseAgentConfig): default=None, description=( 'Optional. LlmAgent.model. Provide a model name string (e.g.' - ' "gemini-2.5-flash"). If not set, the model will be inherited from' - ' the ancestor or fall back to the system default (gemini-2.5-flash' - ' unless overridden via LlmAgent.set_default_model). To construct a' - ' model instance from code, use model_code.' + ' "gemini-3-flash-preview"). If not set, the model will be inherited' + ' from the ancestor or fall back to the system default' + ' (gemini-3-flash-preview unless overridden via' + ' LlmAgent.set_default_model). To construct a model instance from' + ' code, use model_code.' ), ) diff --git a/src/google/adk/tools/mcp_tool/mcp_toolset.py b/src/google/adk/tools/mcp_tool/mcp_toolset.py index b0be349bc5..6d3ccf7c65 100644 --- a/src/google/adk/tools/mcp_tool/mcp_toolset.py +++ b/src/google/adk/tools/mcp_tool/mcp_toolset.py @@ -83,7 +83,6 @@ class McpToolset(BaseToolset): # Use in an agent agent = LlmAgent( - model='gemini-2.5-flash', name='enterprise_assistant', instruction='Help user accessing their file systems', tools=[toolset], From e16629b38814ec31ed55dc6412dcc7ec33774749 Mon Sep 17 00:00:00 2001 From: "Wei (Jack) Sun" Date: Tue, 26 May 2026 16:26:41 -0700 Subject: [PATCH 31/31] fix(tools): Shell escape path and range in ReadFileTool command Use shlex.quote to escape the path and range arguments in the ReadFileTool shell command to prevent potential shell injection vulnerabilities. Change-Id: I5156b616296fc7fac3b98da2b500e4aeb1e3022c --- src/google/adk/tools/environment/_read_file_tool.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/google/adk/tools/environment/_read_file_tool.py b/src/google/adk/tools/environment/_read_file_tool.py index e3342d4d0a..1ff6febf53 100644 --- a/src/google/adk/tools/environment/_read_file_tool.py +++ b/src/google/adk/tools/environment/_read_file_tool.py @@ -17,6 +17,7 @@ from __future__ import annotations import logging +import shlex from typing import Any from typing import Optional from typing import TYPE_CHECKING @@ -109,8 +110,9 @@ async def run_async( sed_range = f'{start},{end_line}' else: sed_range = f'{start},$' - # TODO: use shlex.quote to quote `path` and `sed_range`. - cmd = f"cat -n '{path}' | sed -n '{sed_range}p'" + path_arg = shlex.quote(path) + sed_arg = shlex.quote(f'{sed_range}p') + cmd = f'cat -n {path_arg} | sed -n {sed_arg}' res = await self._environment.execute(cmd) if res.exit_code == 0: return {