Skip to content
Closed
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: 17 additions & 16 deletions src/agents/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from typing_extensions import NotRequired, TypeAlias, TypedDict

from .agent_output import AgentOutputSchemaBase
from .exceptions import UserError
from .guardrail import InputGuardrail, OutputGuardrail
from .handoffs import Handoff
from .items import ItemHelpers
Expand Down Expand Up @@ -235,24 +236,24 @@ def __post_init__(self):
from typing import get_origin

if not isinstance(self.name, str):
raise TypeError(f"Agent name must be a string, got {type(self.name).__name__}")
raise UserError(f"Agent name must be a string, got {type(self.name).__name__}")

if self.handoff_description is not None and not isinstance(self.handoff_description, str):
raise TypeError(
raise UserError(
f"Agent handoff_description must be a string or None, "
f"got {type(self.handoff_description).__name__}"
)

if not isinstance(self.tools, list):
raise TypeError(f"Agent tools must be a list, got {type(self.tools).__name__}")
raise UserError(f"Agent tools must be a list, got {type(self.tools).__name__}")

if not isinstance(self.mcp_servers, list):
raise TypeError(
raise UserError(
f"Agent mcp_servers must be a list, got {type(self.mcp_servers).__name__}"
)

if not isinstance(self.mcp_config, dict):
raise TypeError(
raise UserError(
f"Agent mcp_config must be a dict, got {type(self.mcp_config).__name__}"
)

Expand All @@ -261,7 +262,7 @@ def __post_init__(self):
and not isinstance(self.instructions, str)
and not callable(self.instructions)
):
raise TypeError(
raise UserError(
f"Agent instructions must be a string, callable, or None, "
f"got {type(self.instructions).__name__}"
Comment on lines 262 to 267

Choose a reason for hiding this comment

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

P1 Badge Switching instructions type validation to UserError breaks existing expectations

Changing the instructions validation to raise UserError means Agent(...) now raises UserError when instructions is not a string or callable. Other tests still expect a TypeError in that scenario (tests/test_agent_instructions_signature.py::test_non_callable_instructions_raises_error), so the suite fails. If the intent is to migrate all validation errors to UserError, those tests and any callers relying on TypeError need to be updated or UserError must remain compatible (e.g. subclass or alias).

Useful? React with 👍 / 👎.

)
Expand All @@ -271,24 +272,24 @@ def __post_init__(self):
and not callable(self.prompt)
and not hasattr(self.prompt, "get")
):
raise TypeError(
raise UserError(
f"Agent prompt must be a Prompt, DynamicPromptFunction, or None, "
f"got {type(self.prompt).__name__}"
)

if not isinstance(self.handoffs, list):
raise TypeError(f"Agent handoffs must be a list, got {type(self.handoffs).__name__}")
raise UserError(f"Agent handoffs must be a list, got {type(self.handoffs).__name__}")

if self.model is not None and not isinstance(self.model, str):
from .models.interface import Model

if not isinstance(self.model, Model):
raise TypeError(
raise UserError(
f"Agent model must be a string, Model, or None, got {type(self.model).__name__}"
)

if not isinstance(self.model_settings, ModelSettings):
raise TypeError(
raise UserError(
f"Agent model_settings must be a ModelSettings instance, "
f"got {type(self.model_settings).__name__}"
)
Expand All @@ -314,12 +315,12 @@ def __post_init__(self):
self.model_settings = ModelSettings()

if not isinstance(self.input_guardrails, list):
raise TypeError(
raise UserError(
f"Agent input_guardrails must be a list, got {type(self.input_guardrails).__name__}"
)

if not isinstance(self.output_guardrails, list):
raise TypeError(
raise UserError(
f"Agent output_guardrails must be a list, "
f"got {type(self.output_guardrails).__name__}"
)
Expand All @@ -331,7 +332,7 @@ def __post_init__(self):
isinstance(self.output_type, (type, AgentOutputSchemaBase))
or get_origin(self.output_type) is not None
):
raise TypeError(
raise UserError(
f"Agent output_type must be a type, AgentOutputSchemaBase, or None, "
f"got {type(self.output_type).__name__}"
)
Expand All @@ -340,7 +341,7 @@ def __post_init__(self):
from .lifecycle import AgentHooksBase

if not isinstance(self.hooks, AgentHooksBase):
raise TypeError(
raise UserError(
f"Agent hooks must be an AgentHooks instance or None, "
f"got {type(self.hooks).__name__}"
)
Expand All @@ -353,13 +354,13 @@ def __post_init__(self):
and not isinstance(self.tool_use_behavior, dict)
and not callable(self.tool_use_behavior)
):
raise TypeError(
raise UserError(
f"Agent tool_use_behavior must be 'run_llm_again', 'stop_on_first_tool', "
f"StopAtTools dict, or callable, got {type(self.tool_use_behavior).__name__}"
)

if not isinstance(self.reset_tool_choice, bool):
raise TypeError(
raise UserError(
f"Agent reset_tool_choice must be a boolean, "
f"got {type(self.reset_tool_choice).__name__}"
)
Expand Down
25 changes: 18 additions & 7 deletions tests/test_agent_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from pydantic import BaseModel

from agents import Agent, AgentOutputSchema, Handoff, RunContextWrapper, handoff
from agents.exceptions import UserError
from agents.lifecycle import AgentHooksBase
from agents.model_settings import ModelSettings
from agents.run import AgentRunner
Expand Down Expand Up @@ -177,10 +178,10 @@ class TestAgentValidation:
def test_name_validation_critical_cases(self):
"""Test name validation - the original issue that started this PR"""
# This was the original failing case that caused JSON serialization errors
with pytest.raises(TypeError, match="Agent name must be a string, got int"):
with pytest.raises(UserError, match="Agent name must be a string, got int"):
Agent(name=1) # type: ignore

with pytest.raises(TypeError, match="Agent name must be a string, got NoneType"):
with pytest.raises(UserError, match="Agent name must be a string, got NoneType"):
Agent(name=None) # type: ignore

def test_tool_use_behavior_dict_validation(self):
Expand All @@ -189,7 +190,7 @@ def test_tool_use_behavior_dict_validation(self):
Agent(name="test", tool_use_behavior={"stop_at_tool_names": ["tool1"]})

# Invalid cases that should fail
with pytest.raises(TypeError, match="Agent tool_use_behavior must be"):
with pytest.raises(UserError, match="Agent tool_use_behavior must be"):
Agent(name="test", tool_use_behavior=123) # type: ignore

def test_hooks_validation_python39_compatibility(self):
Expand All @@ -202,25 +203,35 @@ class MockHooks(AgentHooksBase):
Agent(name="test", hooks=MockHooks()) # type: ignore

# Invalid case
with pytest.raises(TypeError, match="Agent hooks must be an AgentHooks instance"):
with pytest.raises(UserError, match="Agent hooks must be an AgentHooks instance"):
Agent(name="test", hooks="invalid") # type: ignore

def test_list_field_validation(self):
"""Test critical list fields that commonly get wrong types"""
# These are the most common mistakes users make
with pytest.raises(TypeError, match="Agent tools must be a list"):
with pytest.raises(UserError, match="Agent tools must be a list"):
Agent(name="test", tools="not_a_list") # type: ignore

with pytest.raises(TypeError, match="Agent handoffs must be a list"):
with pytest.raises(UserError, match="Agent handoffs must be a list"):
Agent(name="test", handoffs="not_a_list") # type: ignore

def test_tools_type_validation_issue_1443(self):
"""Test that UserError is raised when invalid tool type is passed (Issue #1443)"""
# Original bug: passing a string instead of a list should raise UserError, not TypeError
with pytest.raises(UserError, match="Agent tools must be a list, got str"):
Agent(
name="TestAgent",
instructions="Test agent",
tools="predict_weather", # type: ignore # should be a list
)

def test_model_settings_validation(self):
"""Test model_settings validation - prevents runtime errors"""
# Valid case
Agent(name="test", model_settings=ModelSettings())

# Invalid case that could cause runtime issues
with pytest.raises(
TypeError, match="Agent model_settings must be a ModelSettings instance"
UserError, match="Agent model_settings must be a ModelSettings instance"
):
Agent(name="test", model_settings={}) # type: ignore
5 changes: 3 additions & 2 deletions tests/test_agent_instructions_signature.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest

from agents import Agent, RunContextWrapper
from agents.exceptions import UserError


class TestInstructionsSignatureValidation:
Expand Down Expand Up @@ -111,8 +112,8 @@ async def test_none_instructions_return_none(self, mock_run_context):

@pytest.mark.asyncio
async def test_non_callable_instructions_raises_error(self, mock_run_context):
"""Test that non-callable instructions raise a TypeError during initialization"""
with pytest.raises(TypeError) as exc_info:
"""Test that non-callable instructions raise a UserError during initialization"""
with pytest.raises(UserError) as exc_info:
Agent(name="test_agent", instructions=123) # type: ignore[arg-type]

assert "Agent instructions must be a string, callable, or None" in str(exc_info.value)
Expand Down