Skip to content

fix(agent_transfer): add hasattr guard for peer agents without mode attribute#5910

Open
AliMuhammadAslam wants to merge 3 commits into
google:mainfrom
AliMuhammadAslam:fix/loop-agent-mode-hasattr-peer-transfer-v2
Open

fix(agent_transfer): add hasattr guard for peer agents without mode attribute#5910
AliMuhammadAslam wants to merge 3 commits into
google:mainfrom
AliMuhammadAslam:fix/loop-agent-mode-hasattr-peer-transfer-v2

Conversation

@AliMuhammadAslam
Copy link
Copy Markdown

@AliMuhammadAslam AliMuhammadAslam commented May 30, 2026

Fixes #5863


_get_transfer_targets already uses a hasattr guard when building the sub-agent list:

if not hasattr(sub_agent, 'mode')
or sub_agent.mode not in ('single_turn', 'task')

The peer-agent list comprehension a few lines later was missing the same guard:

if peer_agent.name != agent.name
and peer_agent.mode not in ('single_turn', 'task')  # crashes on LoopAgent

LoopAgent (and any other BaseAgent subclass without a mode attribute) raises AttributeError: 'LoopAgent' object has no attribute 'mode' here, breaking any setup where a LoopAgent appears as a sibling of an LlmAgent under a shared parent.

The fix applies the same not hasattr(peer_agent, 'mode') guard to the peer list, matching the pattern already used for sub-agents.

Testing plan

Regression test added: tests/unittests/flows/llm_flows/test_get_transfer_targets.py

  • test_loop_agent_peer_does_not_raise_attribute_error — sets up a root agent with an LlmAgent and a LoopAgent as peer sub-agents, calls _get_transfer_targets on the LlmAgent, and asserts it returns the LoopAgent without raising.
  • test_loop_agent_sub_agent_does_not_raise_attribute_error — same check for the sub-agent list path (already worked before; included for symmetry).

Before the fix:

AttributeError: 'LoopAgent' object has no attribute 'mode'
  File ".../agent_transfer.py", line 196, in <listcomp>
    and peer_agent.mode not in ('single_turn', 'task')

After the fix:

# _get_transfer_targets returns normally, LoopAgent included as a peer target

All existing tests in tests/unittests/flows/llm_flows/ continue to pass.

…ttribute

_get_transfer_targets already guards sub-agent iteration with
`not hasattr(sub_agent, 'mode')` to handle agents like LoopAgent
that have no mode attribute. The peer-agent list comprehension on
the next branch was missing the same guard, causing an uncaught
AttributeError whenever a LoopAgent appeared as a sibling of an
LlmAgent under a shared parent.

Fixes google#5863
@adk-bot adk-bot added the core [Component] This issue is related to the core interface and implementation label May 30, 2026
@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented May 30, 2026

Response from ADK Triaging Agent

Hello @AliMuhammadAslam, thank you for creating this PR!

While we appreciate the regression tests included with your change, could you please add a testing plan section in your PR description to describe how you tested or intend to test this?

Additionally, since this is a bug fix, providing some command-line logs or output demonstrating the fix would be extremely helpful for the reviewers.

For more details, please refer to our contribution guidelines. This information will help reviewers review your PR more efficiently. Thanks!

@AliMuhammadAslam
Copy link
Copy Markdown
Author

Updated the PR description with a testing plan and before/after output as requested. The regression tests are in tests/unittests/flows/llm_flows/test_get_transfer_targets.py.

@rohityan rohityan self-assigned this Jun 5, 2026
@rohityan
Copy link
Copy Markdown
Collaborator

rohityan commented Jun 5, 2026

/adk-pr-analyze

@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented Jun 5, 2026

I have completed a thorough, read-only architectural and style analysis of PR #5910.

The results have been compiled into a premium PR Analysis Report in your artifacts workspace:
👉 analysis_results.md


🔑 Key Findings & Decisions

  1. Google CLA Compliance Gate:
    • SUCCESS: The cla/google status check has completed and verified that the contributor @AliMuhammadAslam has signed the Contributor License Agreement.
  2. Current Code Status & PR Scope:
    • The actual code fix resolving the AttributeError: 'LoopAgent' object has no attribute 'mode' in agent_transfer.py has already been integrated into main via an internal copybara sync.
    • Consequently, this PR is now exclusively presenting the regression test cases inside test_get_transfer_targets.py, which are highly valuable to prevent future regressions.
  3. Principle & Style Checklist Highlights:
    • Test Quality: Uses a clean Arrange-Act-Assert structure and utilizes core ADK modules with local mock models without external dependencies, conforming fully to testing guidelines.
    • Nit identified: The new Python module test_get_transfer_targets.py is missing the mandatory from __future__ import annotations declaration immediately following the license headers.

📋 Recommendation

  • Approve with Nits: The PR is extremely valuable for test coverage and should be merged once the author adds the from __future__ import annotations block as outlined in the report.

@rohityan
Copy link
Copy Markdown
Collaborator

rohityan commented Jun 5, 2026

/adk-pr-triage

@rohityan
Copy link
Copy Markdown
Collaborator

rohityan commented Jun 5, 2026

Hi @AliMuhammadAslam , Thank you for your contribution! We appreciate you taking the time to submit this pull request.

@rohityan rohityan added request clarification [Status] The maintainer need clarification or more information from the author and removed request clarification [Status] The maintainer need clarification or more information from the author labels Jun 5, 2026
- Add from __future__ import annotations to test_get_transfer_targets.py
- Strip trailing whitespace from browser JS chunks and workflow YAML
- Quote YAML name field to fix check-yaml error
- Mock google.auth.default in gemini enterprise fixture to fix
  DefaultCredentialsError in unit tests
@AliMuhammadAslam
Copy link
Copy Markdown
Author

Hi @rohityan, I've addressed all the failing checks in the latest commit:

  1. from __future__ import annotations — added to test_get_transfer_targets.py as flagged by the bot.

  2. Pre-commit: trailing whitespace — removed trailing whitespace from the 10 browser JS chunks and the workflow YAML that were flagged by the trim trailing whitespace hook.

  3. Pre-commit: check-yaml — quoted the name field in release-update-adk-web.yaml to fix the ambiguous colon error at line 1.

  4. Unit tests: DefaultCredentialsError — the test_app_with_gemini_enterprise fixture calls get_fast_api_app with gemini_enterprise_app_name, which internally calls google.auth.default(). Added a patch("google.auth.default", return_value=(MagicMock(), "test-project")) to the fixture's context so the 4 affected tests can run without real credentials.

All checks should pass now.

@rohityan
Copy link
Copy Markdown
Collaborator

rohityan commented Jun 5, 2026

/adk-pr-analyze

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

Labels

core [Component] This issue is related to the core interface and implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using LoopAgent causes Attribute Error in agent_transfer code

3 participants