mirror of
https://github.com/encounter/adk-python.git
synced 2026-07-09 18:19:28 -07:00
fix: relax runner app-name enforcement
- let _enforce_app_name_alignment warn instead of raising while caching the hint that now augments the existing “Session not found …” error - tighten _infer_agent_origin so it ignores hidden folders (like .venv) - make AgentTool reuse the parent runner’s app_name, stopping internal runners from conflicting in multi-agent setups PiperOrigin-RevId: 822205860
This commit is contained in:
committed by
Copybara-Service
parent
aeaec859bf
commit
dc4975dea9
@@ -15,6 +15,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import importlib.util
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
@@ -205,22 +206,30 @@ class AgentLoader(BaseAgentLoader):
|
||||
envs.load_dotenv_for_agent(actual_agent_name, str(agents_dir))
|
||||
|
||||
if root_agent := self._load_from_module_or_package(actual_agent_name):
|
||||
self._ensure_app_name_matches(
|
||||
maybe_app=root_agent,
|
||||
self._record_origin_metadata(
|
||||
loaded=root_agent,
|
||||
expected_app_name=actual_agent_name,
|
||||
module_name=actual_agent_name,
|
||||
agents_dir=agents_dir,
|
||||
)
|
||||
return root_agent
|
||||
|
||||
if root_agent := self._load_from_submodule(actual_agent_name):
|
||||
self._ensure_app_name_matches(
|
||||
maybe_app=root_agent,
|
||||
self._record_origin_metadata(
|
||||
loaded=root_agent,
|
||||
expected_app_name=actual_agent_name,
|
||||
module_name=f"{actual_agent_name}.agent",
|
||||
agents_dir=agents_dir,
|
||||
)
|
||||
return root_agent
|
||||
|
||||
if root_agent := self._load_from_yaml_config(actual_agent_name, agents_dir):
|
||||
self._record_origin_metadata(
|
||||
loaded=root_agent,
|
||||
expected_app_name=actual_agent_name,
|
||||
module_name=None,
|
||||
agents_dir=agents_dir,
|
||||
)
|
||||
return root_agent
|
||||
|
||||
# If no root_agent was found by any pattern
|
||||
@@ -250,32 +259,42 @@ class AgentLoader(BaseAgentLoader):
|
||||
f" root_agent is exposed.{hint}"
|
||||
)
|
||||
|
||||
def _ensure_app_name_matches(
|
||||
def _record_origin_metadata(
|
||||
self,
|
||||
*,
|
||||
maybe_app: Union[BaseAgent, App],
|
||||
loaded: Union[BaseAgent, App],
|
||||
expected_app_name: str,
|
||||
module_name: Optional[str],
|
||||
agents_dir: str,
|
||||
) -> None:
|
||||
"""Raises a detailed error when App.name does not match its directory."""
|
||||
"""Annotates loaded agent/App with its origin for later diagnostics."""
|
||||
|
||||
if not isinstance(maybe_app, App):
|
||||
return
|
||||
|
||||
# Built-in apps live under double-underscore directories.
|
||||
# Do not attach metadata for built-in agents (double underscore names).
|
||||
if expected_app_name.startswith("__"):
|
||||
return
|
||||
|
||||
if maybe_app.name == expected_app_name:
|
||||
return
|
||||
origin_path: Optional[Path] = None
|
||||
if module_name:
|
||||
spec = importlib.util.find_spec(module_name)
|
||||
if spec and spec.origin:
|
||||
module_origin = Path(spec.origin).resolve()
|
||||
origin_path = (
|
||||
module_origin.parent if module_origin.is_file() else module_origin
|
||||
)
|
||||
|
||||
raise ValueError(
|
||||
"App name mismatch detected. The App defined at "
|
||||
f"'{agents_dir}/{expected_app_name}' declares name "
|
||||
f"'{maybe_app.name}', but ADK expects it to match the directory "
|
||||
f"name '{expected_app_name}'. Rename the App or the folder so they "
|
||||
"match, then reload."
|
||||
)
|
||||
if origin_path is None:
|
||||
candidate = Path(agents_dir, expected_app_name)
|
||||
origin_path = candidate if candidate.exists() else Path(agents_dir)
|
||||
|
||||
def _attach_metadata(target: Union[BaseAgent, App]) -> None:
|
||||
setattr(target, "_adk_origin_app_name", expected_app_name)
|
||||
setattr(target, "_adk_origin_path", origin_path)
|
||||
|
||||
if isinstance(loaded, App):
|
||||
_attach_metadata(loaded)
|
||||
_attach_metadata(loaded.root_agent)
|
||||
else:
|
||||
_attach_metadata(loaded)
|
||||
|
||||
@override
|
||||
def load_agent(self, agent_name: str) -> Union[BaseAgent, App]:
|
||||
|
||||
+32
-18
@@ -155,6 +155,7 @@ class Runner:
|
||||
self._agent_origin_app_name,
|
||||
self._agent_origin_dir,
|
||||
) = self._infer_agent_origin(self.agent)
|
||||
self._app_name_alignment_hint: Optional[str] = None
|
||||
self._enforce_app_name_alignment()
|
||||
|
||||
def _validate_runner_params(
|
||||
@@ -230,35 +231,47 @@ class Runner:
|
||||
module_path = Path(module_file).resolve()
|
||||
project_root = Path.cwd()
|
||||
try:
|
||||
module_path.relative_to(project_root)
|
||||
relative_path = module_path.relative_to(project_root)
|
||||
except ValueError:
|
||||
return None, module_path.parent
|
||||
|
||||
current = module_path.parent
|
||||
while current != project_root and current.parent != current:
|
||||
parent = current.parent
|
||||
if parent.name == 'agents':
|
||||
return current.name, current
|
||||
current = parent
|
||||
|
||||
return None, module_path.parent
|
||||
origin_dir = module_path.parent
|
||||
if 'agents' not in relative_path.parts:
|
||||
return None, origin_dir
|
||||
origin_name = origin_dir.name
|
||||
if origin_name.startswith('.'):
|
||||
return None, origin_dir
|
||||
return origin_name, origin_dir
|
||||
|
||||
def _enforce_app_name_alignment(self) -> None:
|
||||
origin_name = self._agent_origin_app_name
|
||||
origin_dir = self._agent_origin_dir
|
||||
if not origin_name or origin_name.startswith('__'):
|
||||
self._app_name_alignment_hint = None
|
||||
return
|
||||
if origin_name == self.app_name:
|
||||
self._app_name_alignment_hint = None
|
||||
return
|
||||
origin_location = str(origin_dir) if origin_dir else origin_name
|
||||
message = (
|
||||
'App name mismatch detected. The runner is configured with '
|
||||
f'app name "{self.app_name}", but the root agent was loaded from '
|
||||
f'"{origin_location}", which implies app name "{origin_name}". '
|
||||
'Rename the App or its directory so the names match before running '
|
||||
'the agent.'
|
||||
mismatch_details = (
|
||||
'The runner is configured with app name '
|
||||
f'"{self.app_name}", but the root agent was loaded from '
|
||||
f'"{origin_location}", which implies app name "{origin_name}".'
|
||||
)
|
||||
resolution = (
|
||||
'Ensure the runner app_name matches that directory or pass app_name '
|
||||
'explicitly when constructing the runner.'
|
||||
)
|
||||
self._app_name_alignment_hint = f'{mismatch_details} {resolution}'
|
||||
logger.warning('App name mismatch detected. %s', mismatch_details)
|
||||
|
||||
def _format_session_not_found_message(self, session_id: str) -> str:
|
||||
message = f'Session not found: {session_id}'
|
||||
if not self._app_name_alignment_hint:
|
||||
return message
|
||||
return (
|
||||
f'{message}. {self._app_name_alignment_hint} '
|
||||
'The mismatch prevents the runner from locating the session.'
|
||||
)
|
||||
raise ValueError(message)
|
||||
|
||||
def run(
|
||||
self,
|
||||
@@ -362,7 +375,8 @@ class Runner:
|
||||
app_name=self.app_name, user_id=user_id, session_id=session_id
|
||||
)
|
||||
if not session:
|
||||
raise ValueError(f'Session not found: {session_id}')
|
||||
message = self._format_session_not_found_message(session_id)
|
||||
raise ValueError(message)
|
||||
if not invocation_id and not new_message:
|
||||
raise ValueError('Both invocation_id and new_message are None.')
|
||||
|
||||
|
||||
@@ -125,8 +125,13 @@ class AgentTool(BaseTool):
|
||||
role='user',
|
||||
parts=[types.Part.from_text(text=args['request'])],
|
||||
)
|
||||
invocation_context = tool_context._invocation_context
|
||||
parent_app_name = (
|
||||
invocation_context.app_name if invocation_context else None
|
||||
)
|
||||
child_app_name = parent_app_name or self.agent.name
|
||||
runner = Runner(
|
||||
app_name=self.agent.name,
|
||||
app_name=child_app_name,
|
||||
agent=self.agent,
|
||||
artifact_service=ForwardingArtifactService(tool_context),
|
||||
session_service=InMemorySessionService(),
|
||||
@@ -141,7 +146,7 @@ class AgentTool(BaseTool):
|
||||
if not k.startswith('_adk') # Filter out adk internal states
|
||||
}
|
||||
session = await runner.session_service.create_session(
|
||||
app_name=self.agent.name,
|
||||
app_name=child_app_name,
|
||||
user_id=tool_context._invocation_context.user_id,
|
||||
state=state_dict,
|
||||
)
|
||||
|
||||
@@ -12,6 +12,8 @@
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
from pathlib import Path
|
||||
import textwrap
|
||||
from typing import Optional
|
||||
|
||||
from google.adk.agents.base_agent import BaseAgent
|
||||
@@ -21,6 +23,7 @@ from google.adk.agents.llm_agent import LlmAgent
|
||||
from google.adk.apps.app import App
|
||||
from google.adk.apps.app import ResumabilityConfig
|
||||
from google.adk.artifacts.in_memory_artifact_service import InMemoryArtifactService
|
||||
from google.adk.cli.utils.agent_loader import AgentLoader
|
||||
from google.adk.events.event import Event
|
||||
from google.adk.plugins.base_plugin import BasePlugin
|
||||
from google.adk.runners import Runner
|
||||
@@ -158,6 +161,120 @@ class TestRunnerFindAgentToRun:
|
||||
artifact_service=self.artifact_service,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_not_found_message_includes_alignment_hint():
|
||||
|
||||
class RunnerWithMismatch(Runner):
|
||||
|
||||
def _infer_agent_origin(
|
||||
self, agent: BaseAgent
|
||||
) -> tuple[Optional[str], Optional[Path]]:
|
||||
del agent
|
||||
return "expected_app", Path("/workspace/agents/expected_app")
|
||||
|
||||
session_service = InMemorySessionService()
|
||||
runner = RunnerWithMismatch(
|
||||
app_name="configured_app",
|
||||
agent=MockLlmAgent("root_agent"),
|
||||
session_service=session_service,
|
||||
artifact_service=InMemoryArtifactService(),
|
||||
)
|
||||
|
||||
agen = runner.run_async(
|
||||
user_id="user",
|
||||
session_id="missing",
|
||||
new_message=types.Content(role="user", parts=[]),
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
await agen.__anext__()
|
||||
|
||||
await agen.aclose()
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "Session not found" in message
|
||||
assert "configured_app" in message
|
||||
assert "expected_app" in message
|
||||
assert "Ensure the runner app_name matches" in message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_allows_nested_agent_directories(tmp_path, monkeypatch):
|
||||
project_root = tmp_path / "workspace"
|
||||
agent_dir = project_root / "agents" / "examples" / "001_hello_world"
|
||||
agent_dir.mkdir(parents=True)
|
||||
# Make package structure importable.
|
||||
for pkg_dir in [
|
||||
project_root / "agents",
|
||||
project_root / "agents" / "examples",
|
||||
agent_dir,
|
||||
]:
|
||||
(pkg_dir / "__init__.py").write_text("", encoding="utf-8")
|
||||
# Extra directories that previously confused origin inference, e.g. virtualenv.
|
||||
(project_root / "agents" / ".venv").mkdir()
|
||||
|
||||
agent_source = textwrap.dedent("""\
|
||||
from google.adk.events.event import Event
|
||||
from google.adk.agents.base_agent import BaseAgent
|
||||
from google.genai import types
|
||||
|
||||
|
||||
class SimpleAgent(BaseAgent):
|
||||
|
||||
def __init__(self):
|
||||
super().__init__(name='simplest_agent', sub_agents=[])
|
||||
|
||||
async def _run_async_impl(self, invocation_context):
|
||||
yield Event(
|
||||
invocation_id=invocation_context.invocation_id,
|
||||
author=self.name,
|
||||
content=types.Content(
|
||||
role='model',
|
||||
parts=[types.Part(text='hello from nested')],
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
root_agent = SimpleAgent()
|
||||
""")
|
||||
(agent_dir / "agent.py").write_text(agent_source, encoding="utf-8")
|
||||
|
||||
monkeypatch.chdir(project_root)
|
||||
loader = AgentLoader(agents_dir="agents/examples")
|
||||
loaded_agent = loader.load_agent("001_hello_world")
|
||||
|
||||
assert isinstance(loaded_agent, BaseAgent)
|
||||
session_service = InMemorySessionService()
|
||||
artifact_service = InMemoryArtifactService()
|
||||
runner = Runner(
|
||||
app_name="001_hello_world",
|
||||
agent=loaded_agent,
|
||||
session_service=session_service,
|
||||
artifact_service=artifact_service,
|
||||
)
|
||||
assert runner._app_name_alignment_hint is None
|
||||
|
||||
session = await session_service.create_session(
|
||||
app_name="001_hello_world",
|
||||
user_id="user",
|
||||
)
|
||||
agen = runner.run_async(
|
||||
user_id=session.user_id,
|
||||
session_id=session.id,
|
||||
new_message=types.Content(
|
||||
role="user",
|
||||
parts=[types.Part(text="hi")],
|
||||
),
|
||||
)
|
||||
event = await agen.__anext__()
|
||||
await agen.aclose()
|
||||
|
||||
assert event.author == "simplest_agent"
|
||||
assert event.content
|
||||
assert event.content.parts
|
||||
assert event.content.parts[0].text == "hello from nested"
|
||||
|
||||
def test_find_agent_to_run_with_function_response_scenario(self):
|
||||
"""Test finding agent when last event is function response."""
|
||||
# Create a function call from sub_agent1
|
||||
|
||||
@@ -12,15 +12,23 @@
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
from typing import Any
|
||||
from typing import Optional
|
||||
|
||||
from google.adk.agents.callback_context import CallbackContext
|
||||
from google.adk.agents.invocation_context import InvocationContext
|
||||
from google.adk.agents.llm_agent import Agent
|
||||
from google.adk.agents.run_config import RunConfig
|
||||
from google.adk.agents.sequential_agent import SequentialAgent
|
||||
from google.adk.artifacts.in_memory_artifact_service import InMemoryArtifactService
|
||||
from google.adk.memory.in_memory_memory_service import InMemoryMemoryService
|
||||
from google.adk.models.llm_request import LlmRequest
|
||||
from google.adk.models.llm_response import LlmResponse
|
||||
from google.adk.plugins.base_plugin import BasePlugin
|
||||
from google.adk.plugins.plugin_manager import PluginManager
|
||||
from google.adk.sessions.in_memory_session_service import InMemorySessionService
|
||||
from google.adk.tools.agent_tool import AgentTool
|
||||
from google.adk.tools.tool_context import ToolContext
|
||||
from google.adk.utils.variant_utils import GoogleLLMVariant
|
||||
from google.genai import types
|
||||
from google.genai.types import Part
|
||||
@@ -51,6 +59,120 @@ def change_state_callback(callback_context: CallbackContext):
|
||||
print('change_state_callback: ', callback_context.state)
|
||||
|
||||
|
||||
@mark.asyncio
|
||||
async def test_agent_tool_inherits_parent_app_name(monkeypatch):
|
||||
parent_app_name = 'parent_app'
|
||||
captured: dict[str, str] = {}
|
||||
|
||||
class RecordingSessionService(InMemorySessionService):
|
||||
|
||||
async def create_session(
|
||||
self,
|
||||
*,
|
||||
app_name: str,
|
||||
user_id: str,
|
||||
state: Optional[dict[str, Any]] = None,
|
||||
session_id: Optional[str] = None,
|
||||
):
|
||||
captured['session_app_name'] = app_name
|
||||
return await super().create_session(
|
||||
app_name=app_name,
|
||||
user_id=user_id,
|
||||
state=state,
|
||||
session_id=session_id,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
'google.adk.sessions.in_memory_session_service.InMemorySessionService',
|
||||
RecordingSessionService,
|
||||
)
|
||||
|
||||
async def _empty_async_generator():
|
||||
if False:
|
||||
yield None
|
||||
|
||||
class StubRunner:
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
app_name: str,
|
||||
agent: Agent,
|
||||
artifact_service,
|
||||
session_service,
|
||||
memory_service,
|
||||
credential_service,
|
||||
plugins,
|
||||
):
|
||||
del artifact_service, memory_service, credential_service
|
||||
captured['runner_app_name'] = app_name
|
||||
self.agent = agent
|
||||
self.session_service = session_service
|
||||
self.plugin_manager = PluginManager(plugins=plugins)
|
||||
self.app_name = app_name
|
||||
|
||||
def run_async(
|
||||
self,
|
||||
*,
|
||||
user_id: str,
|
||||
session_id: str,
|
||||
invocation_id: Optional[str] = None,
|
||||
new_message: Optional[types.Content] = None,
|
||||
state_delta: Optional[dict[str, Any]] = None,
|
||||
run_config: Optional[RunConfig] = None,
|
||||
):
|
||||
del (
|
||||
user_id,
|
||||
session_id,
|
||||
invocation_id,
|
||||
new_message,
|
||||
state_delta,
|
||||
run_config,
|
||||
)
|
||||
return _empty_async_generator()
|
||||
|
||||
monkeypatch.setattr('google.adk.runners.Runner', StubRunner)
|
||||
|
||||
tool_agent = Agent(
|
||||
name='tool_agent',
|
||||
model='test-model',
|
||||
)
|
||||
agent_tool = AgentTool(agent=tool_agent)
|
||||
root_agent = Agent(
|
||||
name='root_agent',
|
||||
model='test-model',
|
||||
tools=[agent_tool],
|
||||
)
|
||||
|
||||
artifact_service = InMemoryArtifactService()
|
||||
parent_session_service = InMemorySessionService()
|
||||
parent_session = await parent_session_service.create_session(
|
||||
app_name=parent_app_name,
|
||||
user_id='user',
|
||||
)
|
||||
invocation_context = InvocationContext(
|
||||
artifact_service=artifact_service,
|
||||
session_service=parent_session_service,
|
||||
memory_service=InMemoryMemoryService(),
|
||||
plugin_manager=PluginManager(),
|
||||
invocation_id='invocation-id',
|
||||
agent=root_agent,
|
||||
session=parent_session,
|
||||
run_config=RunConfig(),
|
||||
)
|
||||
tool_context = ToolContext(invocation_context)
|
||||
|
||||
assert tool_context._invocation_context.app_name == parent_app_name
|
||||
|
||||
await agent_tool.run_async(
|
||||
args={'request': 'hello'},
|
||||
tool_context=tool_context,
|
||||
)
|
||||
|
||||
assert captured['runner_app_name'] == parent_app_name
|
||||
assert captured['session_app_name'] == parent_app_name
|
||||
|
||||
|
||||
def test_no_schema():
|
||||
mock_model = testing_utils.MockModel.create(
|
||||
responses=[
|
||||
|
||||
Reference in New Issue
Block a user