From a4384a8461f0087840b05cfe9f4451a3d9d4da23 Mon Sep 17 00:00:00 2001 From: chelsealong Date: Thu, 30 Jul 2026 15:19:07 +0000 Subject: [PATCH 1/2] fix(samples): report failed issue counts and exit non-zero in adk_stale_agent process_single_issue() swallowed per-issue exceptions and still returned a normal result, so main() counted every issue as processed and the batch always exited 0 even when every issue audit failed. Now failures are tracked separately, main() raises if any issue failed, and the __main__ handler propagates a non-zero exit code while still isolating per-issue failures within a batch. Fixes #6520 --- .../samples/adk_team/adk_stale_agent/main.py | 38 ++++++-- tests/unittests/test_adk_stale_agent_main.py | 96 +++++++++++++++++++ 2 files changed, 126 insertions(+), 8 deletions(-) create mode 100644 tests/unittests/test_adk_stale_agent_main.py diff --git a/contributing/samples/adk_team/adk_stale_agent/main.py b/contributing/samples/adk_team/adk_stale_agent/main.py index f61f87f3330..5085b466882 100644 --- a/contributing/samples/adk_team/adk_stale_agent/main.py +++ b/contributing/samples/adk_team/adk_stale_agent/main.py @@ -14,6 +14,7 @@ import asyncio import logging +import sys import time from typing import Tuple @@ -37,7 +38,7 @@ USER_ID = "stale_bot_user" -async def process_single_issue(issue_number: int) -> Tuple[float, int]: +async def process_single_issue(issue_number: int) -> Tuple[bool, float, int]: """ Processes a single GitHub issue using the AI agent and logs execution metrics. @@ -45,12 +46,10 @@ async def process_single_issue(issue_number: int) -> Tuple[float, int]: issue_number (int): The GitHub issue number to audit. Returns: - Tuple[float, int]: A tuple containing: + Tuple[bool, float, int]: A tuple containing: + - success (bool): Whether the issue was processed without error. - duration (float): Time taken to process the issue in seconds. - api_calls (int): The number of API calls made during this specific execution. - - Raises: - Exception: catches generic exceptions to prevent one failure from stopping the batch. """ start_time = time.perf_counter() @@ -59,6 +58,8 @@ async def process_single_issue(issue_number: int) -> Tuple[float, int]: logger.info(f"Processing Issue #{issue_number}...") logger.debug(f"#{issue_number}: Initializing runner and session.") + success = True + try: runner = InMemoryRunner(agent=root_agent, app_name=APP_NAME) session = await runner.session_service.create_session( @@ -87,6 +88,7 @@ async def process_single_issue(issue_number: int) -> Tuple[float, int]: except Exception as e: logger.error(f"Error processing issue #{issue_number}: {e}", exc_info=True) + success = False duration = time.perf_counter() - start_time @@ -98,7 +100,7 @@ async def process_single_issue(issue_number: int) -> Tuple[float, int]: f"with ~{issue_api_calls} API calls." ) - return duration, issue_api_calls + return success, duration, issue_api_calls async def main(): @@ -138,6 +140,7 @@ async def main(): total_processing_time = 0.0 total_issue_api_calls = 0 processed_count = 0 + failed_issue_numbers = [] # Process the list in chunks of size CONCURRENCY_LIMIT for i in range(0, total_count, CONCURRENCY_LIMIT): @@ -152,11 +155,14 @@ async def main(): results = await asyncio.gather(*tasks) - for duration, api_calls in results: + for issue_num, (success, duration, api_calls) in zip(chunk, results): total_processing_time += duration total_issue_api_calls += api_calls + if success: + processed_count += 1 + else: + failed_issue_numbers.append(issue_num) - processed_count += len(chunk) logger.info( f"--- Finished chunk {current_chunk_num}. Progress:" f" {processed_count}/{total_count} ---" @@ -175,21 +181,37 @@ async def main(): logger.info("--- Stale Agent Run Finished ---") logger.info(f"Successfully processed {processed_count} issues.") + if failed_issue_numbers: + logger.error( + f"Failed to process {len(failed_issue_numbers)} issues:" + f" {failed_issue_numbers}" + ) logger.info(f"Total API calls made this run: {total_api_calls_for_run}") logger.info( f"Average processing time per issue: {avg_time_per_issue:.2f} seconds." ) + if failed_issue_numbers: + raise RuntimeError( + f"{len(failed_issue_numbers)} of {total_count} issue(s) failed" + f" processing: {failed_issue_numbers}" + ) + if __name__ == "__main__": start_time = time.perf_counter() + exit_code = 0 try: asyncio.run(main()) except KeyboardInterrupt: logger.warning("Bot execution interrupted manually.") + exit_code = 1 except Exception as e: logger.critical(f"Unexpected fatal error: {e}", exc_info=True) + exit_code = 1 duration = time.perf_counter() - start_time logger.info(f"Full audit finished in {duration/60:.2f} minutes.") + + sys.exit(exit_code) diff --git a/tests/unittests/test_adk_stale_agent_main.py b/tests/unittests/test_adk_stale_agent_main.py new file mode 100644 index 00000000000..83a26e7323d --- /dev/null +++ b/tests/unittests/test_adk_stale_agent_main.py @@ -0,0 +1,96 @@ +# 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 the adk_stale_agent sample's batch outcome accounting. + +Regression test for a bug where `process_single_issue` swallowed exceptions +and still reported success, letting `main()` log "Successfully processed N +issues" and exit 0 even when every issue in the batch failed. +""" + +from __future__ import annotations + +import asyncio +import importlib +from pathlib import Path +import sys +from types import SimpleNamespace + +import pytest + +ADK_TEAM_DIR = ( + Path(__file__).parent.parent.parent + / "contributing" + / "samples" + / "adk_team" +) + + +class _FailingRunner: + """A fake InMemoryRunner whose run_async always raises.""" + + def __init__(self, **kwargs): + self.session_service = self + + async def create_session(self, **kwargs): + return SimpleNamespace(id="synthetic-session") + + async def run_async(self, **kwargs): + if False: # pragma: no cover - makes this an async generator. + yield None + raise RuntimeError("synthetic runner failure") + + +def _unload_adk_stale_agent(): + for mod_name in list(sys.modules): + if mod_name == "adk_stale_agent" or mod_name.startswith("adk_stale_agent."): + del sys.modules[mod_name] + + +@pytest.fixture +def stale_agent_main(monkeypatch): + """Imports adk_stale_agent.main with a fake GitHub token in place.""" + monkeypatch.setenv("GITHUB_TOKEN", "test-token") + monkeypatch.syspath_prepend(str(ADK_TEAM_DIR)) + _unload_adk_stale_agent() + + main = importlib.import_module("adk_stale_agent.main") + yield main + + _unload_adk_stale_agent() + + +def test_main_raises_when_every_issue_fails(stale_agent_main, monkeypatch): + main = stale_agent_main + monkeypatch.setattr(main, "InMemoryRunner", _FailingRunner) + monkeypatch.setattr( + main, + "get_old_open_issue_numbers", + lambda *args, **kwargs: [101, 202, 303], + ) + monkeypatch.setattr(main, "get_api_call_count", lambda: 0) + monkeypatch.setattr(main, "reset_api_call_count", lambda: None) + + with pytest.raises(RuntimeError, match="3 of 3 issue"): + asyncio.run(main.main()) + + +def test_process_single_issue_reports_failure(stale_agent_main, monkeypatch): + main = stale_agent_main + monkeypatch.setattr(main, "InMemoryRunner", _FailingRunner) + monkeypatch.setattr(main, "get_api_call_count", lambda: 0) + + success, _, _ = asyncio.run(main.process_single_issue(101)) + + assert success is False From dad579e93f9009d805d24e865b63e70c015514b2 Mon Sep 17 00:00:00 2001 From: chelsealong Date: Thu, 30 Jul 2026 19:57:49 +0000 Subject: [PATCH 2/2] fix(samples): use completed_count for progress log, avoid RuntimeError for expected per-issue failures Addresses review feedback on adk_stale_agent: track attempted vs. successful issues separately so the per-chunk progress log isn't misleading, and stop treating per-issue failures (a normal outcome) as an unexpected runtime error. main() now returns a success bool that sets the process exit code directly. --- .../samples/adk_team/adk_stale_agent/main.py | 24 +++++++++++-------- tests/unittests/test_adk_stale_agent_main.py | 7 +++--- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/contributing/samples/adk_team/adk_stale_agent/main.py b/contributing/samples/adk_team/adk_stale_agent/main.py index 5085b466882..9a9bf15bd4b 100644 --- a/contributing/samples/adk_team/adk_stale_agent/main.py +++ b/contributing/samples/adk_team/adk_stale_agent/main.py @@ -103,12 +103,17 @@ async def process_single_issue(issue_number: int) -> Tuple[bool, float, int]: return success, duration, issue_api_calls -async def main(): +async def main() -> bool: """ Main entry point to run the stale issue bot concurrently. Fetches old issues and processes them in batches to respect API rate limits and concurrency constraints. + + Returns: + bool: True if every fetched issue was processed successfully, False if + one or more individual issues failed. Some issues failing is a + normal, expected outcome and is not treated as a fatal error here. """ logger.info(f"--- Starting Stale Bot for {OWNER}/{REPO} ---") logger.info(f"Concurrency level set to {CONCURRENCY_LIMIT}") @@ -122,7 +127,7 @@ async def main(): all_issues = get_old_open_issue_numbers(OWNER, REPO, days_old=filter_days) except Exception as e: logger.critical(f"Failed to fetch issue list: {e}", exc_info=True) - return + return True total_count = len(all_issues) @@ -130,7 +135,7 @@ async def main(): if total_count == 0: logger.info("No issues matched the criteria. Run finished.") - return + return True logger.info( f"Found {total_count} issues to process. " @@ -140,6 +145,7 @@ async def main(): total_processing_time = 0.0 total_issue_api_calls = 0 processed_count = 0 + completed_count = 0 failed_issue_numbers = [] # Process the list in chunks of size CONCURRENCY_LIMIT @@ -158,6 +164,7 @@ async def main(): for issue_num, (success, duration, api_calls) in zip(chunk, results): total_processing_time += duration total_issue_api_calls += api_calls + completed_count += 1 if success: processed_count += 1 else: @@ -165,7 +172,7 @@ async def main(): logger.info( f"--- Finished chunk {current_chunk_num}. Progress:" - f" {processed_count}/{total_count} ---" + f" {completed_count}/{total_count} ---" ) if (i + CONCURRENCY_LIMIT) < total_count: @@ -191,11 +198,7 @@ async def main(): f"Average processing time per issue: {avg_time_per_issue:.2f} seconds." ) - if failed_issue_numbers: - raise RuntimeError( - f"{len(failed_issue_numbers)} of {total_count} issue(s) failed" - f" processing: {failed_issue_numbers}" - ) + return not failed_issue_numbers if __name__ == "__main__": @@ -203,7 +206,8 @@ async def main(): exit_code = 0 try: - asyncio.run(main()) + if not asyncio.run(main()): + exit_code = 1 except KeyboardInterrupt: logger.warning("Bot execution interrupted manually.") exit_code = 1 diff --git a/tests/unittests/test_adk_stale_agent_main.py b/tests/unittests/test_adk_stale_agent_main.py index 83a26e7323d..55cada62f12 100644 --- a/tests/unittests/test_adk_stale_agent_main.py +++ b/tests/unittests/test_adk_stale_agent_main.py @@ -71,7 +71,9 @@ def stale_agent_main(monkeypatch): _unload_adk_stale_agent() -def test_main_raises_when_every_issue_fails(stale_agent_main, monkeypatch): +def test_main_returns_false_when_every_issue_fails( + stale_agent_main, monkeypatch +): main = stale_agent_main monkeypatch.setattr(main, "InMemoryRunner", _FailingRunner) monkeypatch.setattr( @@ -82,8 +84,7 @@ def test_main_raises_when_every_issue_fails(stale_agent_main, monkeypatch): monkeypatch.setattr(main, "get_api_call_count", lambda: 0) monkeypatch.setattr(main, "reset_api_call_count", lambda: None) - with pytest.raises(RuntimeError, match="3 of 3 issue"): - asyncio.run(main.main()) + assert asyncio.run(main.main()) is False def test_process_single_issue_reports_failure(stale_agent_main, monkeypatch):