Skip to content

fix(samples): report failed issue counts and exit non-zero in adk_stale_agent - #6521

Open
chelsealong wants to merge 2 commits into
google:mainfrom
chelsealong:fix-adk-stale-agent-failed-issue-accounting
Open

fix(samples): report failed issue counts and exit non-zero in adk_stale_agent#6521
chelsealong wants to merge 2 commits into
google:mainfrom
chelsealong:fix-adk-stale-agent-failed-issue-accounting

Conversation

@chelsealong

Copy link
Copy Markdown

Description

In contributing/samples/adk_team/adk_stale_agent/main.py, process_single_issue() caught any exception from the runner and still returned its normal (duration, api_calls) result. main() then incremented processed_count by len(chunk) regardless of whether any of those issues actually succeeded, so a run where every issue raised could still log Successfully processed N issues. and exit 0.

This PR:

  • Changes process_single_issue() to return a success flag alongside its metrics, and sets it to False when it catches an exception.
  • Changes main() to only increment processed_count for successes, and to collect failed issue numbers separately, logging them.
  • Makes main() raise a RuntimeError if any issues failed, after all chunks have finished (so per-issue isolation within a batch is preserved).
  • Updates the __main__ block to track an explicit exit code and call sys.exit(exit_code), so the exception raised by main() actually results in a non-zero process exit status (previously it was caught and logged but the process still exited 0).

Fixes #6520

Testing plan

Added tests/unittests/test_adk_stale_agent_main.py, which imports the sample module (with a fake GITHUB_TOKEN) and:

  • Monkeypatches InMemoryRunner, get_old_open_issue_numbers, get_api_call_count, and reset_api_call_count with a fake runner whose run_async always raises (mirroring the issue's reproduction), then asserts main() raises RuntimeError when all issues fail.
  • Asserts process_single_issue() returns success=False when the runner raises.

Verified the test fails without the fix: reverted main.py to its pre-fix state (git stash/git checkout) and reran the test — it failed with DID NOT RAISE RuntimeError / ValueError: not enough values to unpack (expected 3, got 2), reproducing the exact bug described in the issue (batch logs "Successfully processed 3 issues" and exits 0 despite every issue failing). Restored the fix and reran; both tests pass.

tests/unittests/test_adk_stale_agent_main.py::test_main_raises_when_every_issue_fails PASSED
tests/unittests/test_adk_stale_agent_main.py::test_process_single_issue_reports_failure PASSED
2 passed

Also ran the broader sample test suite (tests/unittests/test_samples.py) to confirm no regressions: 85 passed.

Ran pre-commit run --files contributing/samples/adk_team/adk_stale_agent/main.py tests/unittests/test_adk_stale_agent_main.py (isort, pyink, addlicense, ADK compliance checks) — all passed.

AI-assistance disclosure

This change was prepared with the help of an AI coding assistant (Claude), with the diff and test evidence reviewed before submission.

…le_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 google#6520
@adk-bot adk-bot added the documentation [Component] This issue is related to documentation, it will be transferred to adk-docs label Jul 30, 2026
processed_count += len(chunk)
logger.info(
f"--- Finished chunk {current_chunk_num}. Progress:"
f" {processed_count}/{total_count} ---"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would introduce completed_count because the processed_count value is now controlled by a condition, so the progress log could be misleading

)

if failed_issue_numbers:
raise RuntimeError(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an unexpected runtime failure, some individual issues failing is a normal outcome

…r 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.
@chelsealong

Copy link
Copy Markdown
Author

Addressed both comments: introduced a separate completed_count (incremented for every attempted issue, success or failure) and use it in the per-chunk progress log instead of processed_count, which now only tracks successes. Also removed the RuntimeError for per-issue failures since that's a normal outcome, not an unexpected error — main() now returns a success bool, and the __main__ block sets the exit code from that directly.

f"{len(failed_issue_numbers)} of {total_count} issue(s) failed"
f" processing: {failed_issue_numbers}"
)
return not failed_issue_numbers

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the author confirm whether this PR, including its review-response commits, was created or updated by an automated AI pipeline? If so, please disclose the tool and confirm that a human has reviewed and tested the resulting changes.

@chelsealong

Copy link
Copy Markdown
Author

Yes — this PR, including the review-response commits (a4384a8, dad579e) addressing the completed_count and RuntimeError feedback, was prepared with the help of Claude Code (Anthropic's AI coding assistant), as disclosed in the PR description. I (chelsealong) reviewed the diff and test output before each push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation [Component] This issue is related to documentation, it will be transferred to adk-docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] adk_stale_agent counts failed issue runs as successfully processed and exits 0

4 participants