From e4ee9d7c46b57eed8493539d8f539c042bdfae60 Mon Sep 17 00:00:00 2001 From: Google Team Member Date: Thu, 18 Dec 2025 14:22:56 -0800 Subject: [PATCH 1/3] feat: Update event_converter used in A2ARemote agent to use a2a_task.status.message only if parts are non-empty PiperOrigin-RevId: 846413612 --- .../adk/a2a/converters/event_converter.py | 6 +- .../a2a/converters/test_event_converter.py | 62 +++++++------------ 2 files changed, 27 insertions(+), 41 deletions(-) diff --git a/src/google/adk/a2a/converters/event_converter.py b/src/google/adk/a2a/converters/event_converter.py index 47d5f077ab..c7b9b9e57f 100644 --- a/src/google/adk/a2a/converters/event_converter.py +++ b/src/google/adk/a2a/converters/event_converter.py @@ -229,7 +229,11 @@ def convert_a2a_task_to_event( message = Message( message_id="", role=Role.agent, parts=a2a_task.artifacts[-1].parts ) - elif a2a_task.status and a2a_task.status.message: + elif ( + a2a_task.status + and a2a_task.status.message + and a2a_task.status.message.parts + ): message = a2a_task.status.message elif a2a_task.history: message = a2a_task.history[-1] diff --git a/tests/unittests/a2a/converters/test_event_converter.py b/tests/unittests/a2a/converters/test_event_converter.py index 09fd65c3bf..3a06f50ba3 100644 --- a/tests/unittests/a2a/converters/test_event_converter.py +++ b/tests/unittests/a2a/converters/test_event_converter.py @@ -773,13 +773,9 @@ def test_convert_a2a_task_to_event_message_conversion_error(self): from google.adk.a2a.converters.event_converter import convert_a2a_task_to_event # Create mock message and task - mock_message = Mock(spec=Message) - mock_status = Mock() - mock_status.message = mock_message - mock_task = Mock(spec=Task) - mock_task.artifacts = None - mock_task.status = mock_status - mock_task.history = [] + mock_message = Mock(spec=Message, parts=[Mock()]) + mock_status = Mock(message=mock_message) + mock_task = Mock(spec=Task, artifacts=None, status=mock_status, history=[]) # Mock the convert_a2a_message_to_event function to raise an exception with patch( @@ -798,11 +794,9 @@ def test_convert_a2a_message_to_event_success(self): # Create mock parts and message with valid genai Part mock_a2a_part = Mock() mock_genai_part = genai_types.Part(text="test content") - mock_convert_part = Mock() - mock_convert_part.return_value = mock_genai_part + mock_convert_part = Mock(return_value=mock_genai_part) - mock_message = Mock(spec=Message) - mock_message.parts = [mock_a2a_part] + mock_message = Mock(spec=Message, parts=[mock_a2a_part]) result = convert_a2a_message_to_event( mock_message, @@ -829,11 +823,9 @@ def test_convert_a2a_message_to_event_with_multiple_parts_returned(self): mock_a2a_part = Mock() mock_genai_part1 = genai_types.Part(text="part 1") mock_genai_part2 = genai_types.Part(text="part 2") - mock_convert_part = Mock() - mock_convert_part.return_value = [mock_genai_part1, mock_genai_part2] + mock_convert_part = Mock(return_value=[mock_genai_part1, mock_genai_part2]) - mock_message = Mock(spec=Message) - mock_message.parts = [mock_a2a_part] + mock_message = Mock(spec=Message, parts=[mock_a2a_part]) # Act result = convert_a2a_message_to_event( @@ -855,13 +847,10 @@ def test_convert_a2a_message_to_event_with_long_running_tools(self): from google.adk.a2a.converters.event_converter import convert_a2a_message_to_event # Create mock parts and message - mock_a2a_part = Mock() - mock_message = Mock(spec=Message) - mock_message.parts = [mock_a2a_part] + mock_message = Mock(spec=Message, parts=[Mock()]) # Mock the part conversion to return None to simulate long-running tool detection logic - mock_convert_part = Mock() - mock_convert_part.return_value = None + mock_convert_part = Mock(return_value=None) # Patch the long-running tool detection since the main logic is in the actual conversion with patch( @@ -884,8 +873,7 @@ def test_convert_a2a_message_to_event_empty_parts(self): """Test conversion with empty parts list.""" from google.adk.a2a.converters.event_converter import convert_a2a_message_to_event - mock_message = Mock(spec=Message) - mock_message.parts = [] + mock_message = Mock(spec=Message, parts=[]) result = convert_a2a_message_to_event( mock_message, "test-author", self.mock_invocation_context @@ -910,11 +898,9 @@ def test_convert_a2a_message_to_event_part_conversion_fails(self): # Setup mock to return None (conversion failure) mock_a2a_part = Mock() - mock_convert_part = Mock() - mock_convert_part.return_value = None + mock_convert_part = Mock(return_value=None) - mock_message = Mock(spec=Message) - mock_message.parts = [mock_a2a_part] + mock_message = Mock(spec=Message, parts=[mock_a2a_part]) result = convert_a2a_message_to_event( mock_message, @@ -939,14 +925,14 @@ def test_convert_a2a_message_to_event_part_conversion_exception(self): mock_a2a_part2 = Mock() mock_genai_part = genai_types.Part(text="successful conversion") - mock_convert_part = Mock() - mock_convert_part.side_effect = [ - Exception("Conversion failed"), # First part fails - mock_genai_part, # Second part succeeds - ] + mock_convert_part = Mock( + side_effect=[ + Exception("Conversion failed"), # First part fails + mock_genai_part, # Second part succeeds + ] + ) - mock_message = Mock(spec=Message) - mock_message.parts = [mock_a2a_part1, mock_a2a_part2] + mock_message = Mock(spec=Message, parts=[mock_a2a_part1, mock_a2a_part2]) result = convert_a2a_message_to_event( mock_message, @@ -967,13 +953,10 @@ def test_convert_a2a_message_to_event_missing_tool_id(self): from google.adk.a2a.converters.event_converter import convert_a2a_message_to_event # Create mock parts and message - mock_a2a_part = Mock() - mock_message = Mock(spec=Message) - mock_message.parts = [mock_a2a_part] + mock_message = Mock(spec=Message, parts=[Mock()]) # Mock the part conversion to return None - mock_convert_part = Mock() - mock_convert_part.return_value = None + mock_convert_part = Mock(return_value=None) result = convert_a2a_message_to_event( mock_message, @@ -994,8 +977,7 @@ def test_convert_a2a_message_to_event_default_author(self, mock_uuid): """Test conversion with default author and no invocation context.""" from google.adk.a2a.converters.event_converter import convert_a2a_message_to_event - mock_message = Mock(spec=Message) - mock_message.parts = [] + mock_message = Mock(spec=Message, parts=[]) # Mock UUID generation mock_uuid.return_value = "generated-uuid" From f51b9b70b20369e7862c7c8a7527844bf613dea5 Mon Sep 17 00:00:00 2001 From: Rohit Yanamadala Date: Thu, 18 Dec 2025 16:03:49 -0800 Subject: [PATCH 2/3] fix: Prevent stale bot from flagging internal maintainer discussions Merge https://github.com/google/adk-python/pull/3938 Co-authored-by: Xuan Yang COPYBARA_INTEGRATE_REVIEW=https://github.com/google/adk-python/pull/3938 from ryanaiagent:fix/stale-bot-logic 605d3499784facffc0e0ca3c1fb0547202660f46 PiperOrigin-RevId: 846450195 --- .github/workflows/stale-bot.yml | 14 ++++++++++++++ .../adk_stale_agent/PROMPT_INSTRUCTION.txt | 19 ++++++++++++------- contributing/samples/adk_stale_agent/agent.py | 7 +++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/.github/workflows/stale-bot.yml b/.github/workflows/stale-bot.yml index 2e74e5e51f..b2a9cb82ad 100644 --- a/.github/workflows/stale-bot.yml +++ b/.github/workflows/stale-bot.yml @@ -1,3 +1,17 @@ +# Copyright 2025 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. + name: ADK Stale Issue Auditor on: diff --git a/contributing/samples/adk_stale_agent/PROMPT_INSTRUCTION.txt b/contributing/samples/adk_stale_agent/PROMPT_INSTRUCTION.txt index 8f5f585ff6..c56c76a8b6 100644 --- a/contributing/samples/adk_stale_agent/PROMPT_INSTRUCTION.txt +++ b/contributing/samples/adk_stale_agent/PROMPT_INSTRUCTION.txt @@ -45,7 +45,7 @@ Your job is to analyze a specific issue and report findings before taking action - **IF True**: The user edited the description silently, and we haven't alerted yet. -> **Action**: Call `alert_maintainer_of_edit`. -> **Report**: "Analysis for Issue #[number]: ACTIVE. Silent update detected (Description Edit). Alerted maintainer." - - **IF False**: + - **IF False**: -> **Report**: "Analysis for Issue #[number]: ACTIVE. Last action was by user. No action." - **Check Role**: If `last_action_role` is 'maintainer': @@ -53,16 +53,21 @@ Your job is to analyze a specific issue and report findings before taking action **STEP 3: ANALYZE MAINTAINER INTENT** - **Context**: The last person to act was a Maintainer. -- **Action**: Read the text in `last_comment_text`. - - **Question Check**: Does the text ask a question, request clarification, ask for logs, or suggest trying a fix? +- **Action**: Analyze `last_comment_text` using `maintainers` list and `last_actor_name`. + + - **Internal Discussion Check**: Does the comment mention or address any username found in the `maintainers` list (other than the speaker `last_actor_name`)? + - **Verdict**: **ACTIVE** (Internal Team Discussion). + - **Report**: "Analysis for Issue #[number]: ACTIVE. Maintainer is discussing with another maintainer. No action." + + - **Question Check**: Does the text ask a question, request clarification, ask for logs, or give suggestions? - **Time Check**: Is `days_since_activity` > {stale_threshold_days}? - **DECISION**: - - **IF (Question == YES) AND (Time == YES)**: + - **IF (Question == YES) AND (Time == YES) AND (Internal Discussion Check == FALSE):** - **Action**: Call `add_stale_label_and_comment`. - - **Check**: If '{REQUEST_CLARIFICATION_LABEL}' is not in `current_labels`, call `add_label_to_issue` for it. + - **Check**: If '{REQUEST_CLARIFICATION_LABEL}' is not in `current_labels`, call `add_label_to_issue` with '{REQUEST_CLARIFICATION_LABEL}'. - **Report**: "Analysis for Issue #[number]: STALE. Maintainer asked question [days_since_activity] days ago. Marking stale." - **IF (Question == YES) BUT (Time == NO)**: - **Report**: "Analysis for Issue #[number]: PENDING. Maintainer asked question, but threshold not met yet. No action." - - **IF (Question == NO)** (e.g., "I am working on this"): - - **Report**: "Analysis for Issue #[number]: ACTIVE. Maintainer gave status update (not a question). No action." \ No newline at end of file + - **IF (Question == NO) OR (Internal Discussion Check == TRUE):** + - **Report**: "Analysis for Issue #[number]: ACTIVE. Maintainer gave status update or internal discussion detected. No action." \ No newline at end of file diff --git a/contributing/samples/adk_stale_agent/agent.py b/contributing/samples/adk_stale_agent/agent.py index 8769adc193..2535f9cf45 100644 --- a/contributing/samples/adk_stale_agent/agent.py +++ b/contributing/samples/adk_stale_agent/agent.py @@ -318,11 +318,13 @@ def _replay_history_to_find_state( - last_activity_time (datetime): Timestamp of the last human action. - last_action_type (str): The type of the last action (e.g., 'commented'). - last_comment_text (Optional[str]): The text of the last comment. + - last_actor_name (str): The specific username of the last actor. """ last_action_role = "author" last_activity_time = history[0]["time"] last_action_type = "created" last_comment_text = None + last_actor_name = issue_author for event in history: actor = event["actor"] @@ -337,6 +339,7 @@ def _replay_history_to_find_state( last_action_role = role last_activity_time = event["time"] last_action_type = etype + last_actor_name = actor # Only store text if it was a comment (resets on other events like labels/edits) if etype == "commented": @@ -349,6 +352,7 @@ def _replay_history_to_find_state( "last_activity_time": last_activity_time, "last_action_type": last_action_type, "last_comment_text": last_comment_text, + "last_actor_name": last_actor_name, } @@ -428,6 +432,7 @@ def get_issue_state(item_number: int) -> Dict[str, Any]: "status": "success", "last_action_role": state["last_action_role"], "last_action_type": state["last_action_type"], + "last_actor_name": state["last_actor_name"], "maintainer_alert_needed": maintainer_alert_needed, "is_stale": is_stale, "days_since_activity": days_since_activity, @@ -436,6 +441,8 @@ def get_issue_state(item_number: int) -> Dict[str, Any]: "current_labels": labels_list, "stale_threshold_days": STALE_HOURS_THRESHOLD / 24, "close_threshold_days": CLOSE_HOURS_AFTER_STALE_THRESHOLD / 24, + "maintainers": maintainers, + "issue_author": issue_author, } except RequestException as e: From 71b32890f5ab279e2bed1fd28c0f4693cba3f45e Mon Sep 17 00:00:00 2001 From: Kathy Wu Date: Thu, 18 Dec 2025 16:06:13 -0800 Subject: [PATCH 3/3] fix: Only prepend "https://" to the MCP server url if it doesn't already have a scheme There isn't a consistent format for MCP server urls in the registry-- some customers add the https:// but others don't. To standardize, only prepend if it isn't there already. Co-authored-by: Kathy Wu PiperOrigin-RevId: 846451152 --- src/google/adk/tools/api_registry.py | 6 ++- tests/unittests/tools/test_api_registry.py | 52 +++++++++++++++++++++- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/google/adk/tools/api_registry.py b/src/google/adk/tools/api_registry.py index 01c6c7061b..313fd6c45b 100644 --- a/src/google/adk/tools/api_registry.py +++ b/src/google/adk/tools/api_registry.py @@ -102,9 +102,13 @@ def get_toolset( mcp_server_url = server["urls"][0] headers = self._get_auth_headers() + # Only prepend "https://" if the URL doesn't already have a scheme + if not mcp_server_url.startswith(("http://", "https://")): + mcp_server_url = "https://" + mcp_server_url + return McpToolset( connection_params=StreamableHTTPConnectionParams( - url="https://" + mcp_server_url, + url=mcp_server_url, headers=headers, ), tool_filter=tool_filter, diff --git a/tests/unittests/tools/test_api_registry.py b/tests/unittests/tools/test_api_registry.py index 6f1ebd4230..4a114c512b 100644 --- a/tests/unittests/tools/test_api_registry.py +++ b/tests/unittests/tools/test_api_registry.py @@ -18,6 +18,7 @@ from unittest.mock import MagicMock from unittest.mock import patch +from google.adk.tools import api_registry from google.adk.tools.api_registry import ApiRegistry from google.adk.tools.mcp_tool.mcp_session_manager import StreamableHTTPConnectionParams import httpx @@ -35,6 +36,14 @@ { "name": "test-mcp-server-no-url", }, + { + "name": "test-mcp-server-http", + "urls": ["http://mcp.server_http.com"], + }, + { + "name": "test-mcp-server-https", + "urls": ["https://mcp.server_https.com"], + }, ] } @@ -70,10 +79,12 @@ def test_init_success(self, MockHttpClient): api_registry_project_id=self.project_id, location=self.location ) - self.assertEqual(len(api_registry._mcp_servers), 3) + self.assertEqual(len(api_registry._mcp_servers), 5) self.assertIn("test-mcp-server-1", api_registry._mcp_servers) self.assertIn("test-mcp-server-2", api_registry._mcp_servers) self.assertIn("test-mcp-server-no-url", api_registry._mcp_servers) + self.assertIn("test-mcp-server-http", api_registry._mcp_servers) + self.assertIn("test-mcp-server-https", api_registry._mcp_servers) mock_client_instance.get.assert_called_once_with( f"https://cloudapiregistry.googleapis.com/v1beta/projects/{self.project_id}/locations/{self.location}/mcpServers", headers={ @@ -95,10 +106,12 @@ def test_init_with_quota_project_id_success(self, MockHttpClient): api_registry_project_id=self.project_id, location=self.location ) - self.assertEqual(len(api_registry._mcp_servers), 3) + self.assertEqual(len(api_registry._mcp_servers), 5) self.assertIn("test-mcp-server-1", api_registry._mcp_servers) self.assertIn("test-mcp-server-2", api_registry._mcp_servers) self.assertIn("test-mcp-server-no-url", api_registry._mcp_servers) + self.assertIn("test-mcp-server-http", api_registry._mcp_servers) + self.assertIn("test-mcp-server-https", api_registry._mcp_servers) mock_client_instance.get.assert_called_once_with( f"https://cloudapiregistry.googleapis.com/v1beta/projects/{self.project_id}/locations/{self.location}/mcpServers", headers={ @@ -232,6 +245,41 @@ async def test_get_toolset_with_filter_and_prefix( ) self.assertEqual(toolset, MockMcpToolset.return_value) + def test_get_toolset_url_scheme(self): + params = [ + ("test-mcp-server-http", "http://mcp.server_http.com"), + ("test-mcp-server-https", "https://mcp.server_https.com"), + ] + for mock_server_name, mock_url in params: + with self.subTest(server_name=mock_server_name): + with ( + patch.object(httpx, "Client", autospec=True) as MockHttpClient, + patch.object( + api_registry, "McpToolset", autospec=True + ) as MockMcpToolset, + ): + mock_response = create_autospec(httpx.Response, instance=True) + mock_response.json.return_value = MOCK_MCP_SERVERS_LIST + mock_client_instance = MockHttpClient.return_value + mock_client_instance.__enter__.return_value = mock_client_instance + mock_client_instance.get.return_value = mock_response + + api_registry_instance = ApiRegistry( + api_registry_project_id=self.project_id, location=self.location + ) + + api_registry_instance.get_toolset(mock_server_name) + + MockMcpToolset.assert_called_once_with( + connection_params=StreamableHTTPConnectionParams( + url=mock_url, + headers={"Authorization": "Bearer mock_token"}, + ), + tool_filter=None, + tool_name_prefix=None, + header_provider=None, + ) + @patch("httpx.Client", autospec=True) async def test_get_toolset_server_not_found(self, MockHttpClient): mock_response = MagicMock()