Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions astrbot/core/astr_agent_tool_exec.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
MessageEventResult,
)
from astrbot.core.platform.message_session import MessageSession
from astrbot.core.provider import Provider
from astrbot.core.provider.entites import ProviderRequest
from astrbot.core.provider.register import llm_tools
from astrbot.core.tools.computer_tools import (
Expand Down Expand Up @@ -354,6 +355,36 @@ async def _execute_handoff(
continue

prov_settings: dict = ctx.get_config(umo=umo).get("provider_settings", {})
fallback_ids = prov_settings.get("fallback_chat_models", [])
fallback_providers: list[Provider] = []
if not isinstance(fallback_ids, list):
logger.warning(
"fallback_chat_models setting is not a list, skip handoff fallback providers."
)
else:
seen_provider_ids = {prov_id} if prov_id else set()
for fallback_id in fallback_ids:
if not isinstance(fallback_id, str) or not fallback_id:
continue
if fallback_id in seen_provider_ids:
continue
fallback_provider = ctx.get_provider_by_id(fallback_id)
if fallback_provider is None:
logger.warning(
"Handoff fallback chat provider `%s` not found, skip.",
fallback_id,
)
continue
if not isinstance(fallback_provider, Provider):
logger.warning(
"Handoff fallback chat provider `%s` is invalid type: %s, skip.",
fallback_id,
type(fallback_provider),
)
continue
fallback_providers.append(fallback_provider)
seen_provider_ids.add(fallback_id)

Comment on lines +358 to +387

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This block of code is almost identical to the _get_fallback_chat_providers helper function defined in astrbot/core/astr_main_agent.py. To avoid code duplication and adhere to the general rules, we should reuse that helper function.

We can import _get_fallback_chat_providers inside _execute_handoff to avoid circular dependencies, and pass the resolved Provider object directly.

        provider = ctx.get_provider_by_id(prov_id) if prov_id else None
        if provider:
            from astrbot.core.astr_main_agent import _get_fallback_chat_providers
            fallback_providers = _get_fallback_chat_providers(provider, ctx, prov_settings)
        else:
            fallback_providers = []
References
  1. When implementing similar functionality for different cases, refactor the logic into a shared helper function to avoid code duplication.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I am keeping the resolution inline: importing a private helper from astr_main_agent would couple the tool executor to the main-agent module, which already imports FunctionToolExecutor, creating a circular import risk. This is currently the second use site, so extracting a shared provider utility would add broader scope without improving the handoff fix.

agent_max_step = int(prov_settings.get("max_agent_step", 30))
stream = prov_settings.get("streaming_response", False)
llm_resp = await ctx.tool_loop_agent(
Expand All @@ -367,6 +398,8 @@ async def _execute_handoff(
max_steps=agent_max_step,
tool_call_timeout=run_context.tool_call_timeout,
stream=stream,
fallback_providers=fallback_providers,
request_max_retries=prov_settings.get("request_max_retries", 5),
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
)
yield mcp.types.CallToolResult(
content=[mcp.types.TextContent(type="text", text=llm_resp.completion_text)]
Expand Down
70 changes: 70 additions & 0 deletions tests/unit/test_astr_agent_tool_exec.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,76 @@ async def _fake_tool_loop_agent(**kwargs):
assert captured["tool_call_timeout"] == 120


@pytest.mark.asyncio
async def test_execute_handoff_passes_valid_fallbacks_and_retries_to_tool_loop_agent(
monkeypatch: pytest.MonkeyPatch,
):
captured: dict = {}

class _ChatProvider:
pass

primary = _ChatProvider()
fallback = _ChatProvider()
non_chat_provider = object()
providers = {
"primary": primary,
"fallback": fallback,
"non-chat": non_chat_provider,
}

async def _fake_get_current_chat_provider_id(_umo):
return "primary"

async def _fake_tool_loop_agent(**kwargs):
captured.update(kwargs)
return SimpleNamespace(completion_text="ok")

context = SimpleNamespace(
get_current_chat_provider_id=_fake_get_current_chat_provider_id,
get_provider_by_id=lambda provider_id: providers.get(provider_id),
tool_loop_agent=_fake_tool_loop_agent,
get_config=lambda **_kwargs: {
"provider_settings": {
"fallback_chat_models": [
"primary",
"fallback",
"fallback",
"missing",
"non-chat",
],
"request_max_retries": 7,
}
},
)
event = _DummyEvent([])
run_context = ContextWrapper(context=SimpleNamespace(event=event, context=context))
tool = SimpleNamespace(
name="transfer_to_subagent",
provider_id="primary",
agent=SimpleNamespace(
name="subagent",
tools=[],
instructions="subagent-instructions",
begin_dialogs=[],
run_hooks=None,
),
)
monkeypatch.setattr("astrbot.core.astr_agent_tool_exec.Provider", _ChatProvider)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since we are refactoring the fallback provider resolution to use _get_fallback_chat_providers from astrbot.core.astr_main_agent, we need to monkeypatch Provider in astrbot.core.astr_main_agent instead of astrbot.core.astr_agent_tool_exec so that the isinstance check inside the helper function works correctly with our mock provider class.

Suggested change
monkeypatch.setattr("astrbot.core.astr_agent_tool_exec.Provider", _ChatProvider)
monkeypatch.setattr('astrbot.core.astr_main_agent.Provider', _ChatProvider)


async for _result in FunctionToolExecutor._execute_handoff(
tool,
run_context,
image_urls_prepared=True,
input="hello",
image_urls=[],
):
pass

assert captured["fallback_providers"] == [fallback]
assert captured["request_max_retries"] == 7


@pytest.mark.asyncio
async def test_background_wakeup_passes_provider_settings_to_main_agent(
monkeypatch: pytest.MonkeyPatch,
Expand Down