From db50380a9a965667c897583725d26dde5e01bdb2 Mon Sep 17 00:00:00 2001 From: Abdullah Date: Tue, 12 Aug 2025 06:14:11 +0500 Subject: [PATCH 1/5] Fix: enforce strict instructions function signature in get_system_prompt --- src/agents/agent.py | 18 ++++++++++++--- tests/test_agent_instructions_signature.py | 26 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 tests/test_agent_instructions_signature.py diff --git a/src/agents/agent.py b/src/agents/agent.py index b17189e53..4304ada14 100644 --- a/src/agents/agent.py +++ b/src/agents/agent.py @@ -393,16 +393,28 @@ async def run_agent(context: RunContextWrapper, input: str) -> str: return run_agent async def get_system_prompt(self, run_context: RunContextWrapper[TContext]) -> str | None: - """Get the system prompt for the agent.""" if isinstance(self.instructions, str): return self.instructions elif callable(self.instructions): + # Inspect the signature of the instructions function + sig = inspect.signature(self.instructions) + params = list(sig.parameters.values()) + + # Enforce exactly 2 parameters + if len(params) != 2: + raise TypeError( + f"'instructions' callable must accept exactly 2 arguments (context, agent), " + f"but got {len(params)}: {[p.name for p in params]}" + ) + + # Call the instructions function properly if inspect.iscoroutinefunction(self.instructions): return await cast(Awaitable[str], self.instructions(run_context, self)) else: return cast(str, self.instructions(run_context, self)) + elif self.instructions is not None: - logger.error(f"Instructions must be a string or a function, got {self.instructions}") + logger.error(f"Instructions must be a string or a callable function, got {type(self.instructions).__name__}") return None @@ -410,4 +422,4 @@ async def get_prompt( self, run_context: RunContextWrapper[TContext] ) -> ResponsePromptParam | None: """Get the prompt for the agent.""" - return await PromptUtil.to_model_input(self.prompt, run_context, self) + return await PromptUtil.to_model_input(self.prompt, run_context, self) \ No newline at end of file diff --git a/tests/test_agent_instructions_signature.py b/tests/test_agent_instructions_signature.py new file mode 100644 index 000000000..dc0ffce19 --- /dev/null +++ b/tests/test_agent_instructions_signature.py @@ -0,0 +1,26 @@ +import pytest +from src.agents.agent import Agent # adjust if needed + +class DummyContext: + pass + +class DummyAgent(Agent): + def __init__(self, instructions): + super().__init__(instructions=instructions) + +@pytest.mark.asyncio +async def test_valid_signature(): + async def good_instructions(ctx, agent): + return "valid" + a = DummyAgent(good_instructions) + result = await a.get_system_prompt(DummyContext()) + assert result == "valid" + +@pytest.mark.asyncio +async def test_invalid_signature_raises(): + async def bad_instructions(ctx): + return "invalid" + a = DummyAgent(bad_instructions) + import pytest + with pytest.raises(TypeError): + await a.get_system_prompt(DummyContext()) From 69c406fe400565aea12d71ab8c19a7f823e52c24 Mon Sep 17 00:00:00 2001 From: Abdullah Date: Tue, 12 Aug 2025 06:22:05 +0500 Subject: [PATCH 2/5] fix: test_agent_instructions_signature --- tests/test_agent_instructions_signature.py | 131 +++++++++++++++++---- 1 file changed, 107 insertions(+), 24 deletions(-) diff --git a/tests/test_agent_instructions_signature.py b/tests/test_agent_instructions_signature.py index dc0ffce19..ae61d8ca2 100644 --- a/tests/test_agent_instructions_signature.py +++ b/tests/test_agent_instructions_signature.py @@ -1,26 +1,109 @@ import pytest -from src.agents.agent import Agent # adjust if needed +from unittest.mock import Mock +# Adjust import based on actual repo structure +from src.agents.agent import Agent, RunContextWrapper -class DummyContext: - pass - -class DummyAgent(Agent): - def __init__(self, instructions): - super().__init__(instructions=instructions) - -@pytest.mark.asyncio -async def test_valid_signature(): - async def good_instructions(ctx, agent): - return "valid" - a = DummyAgent(good_instructions) - result = await a.get_system_prompt(DummyContext()) - assert result == "valid" - -@pytest.mark.asyncio -async def test_invalid_signature_raises(): - async def bad_instructions(ctx): - return "invalid" - a = DummyAgent(bad_instructions) - import pytest - with pytest.raises(TypeError): - await a.get_system_prompt(DummyContext()) +class TestInstructionsSignatureValidation: + """Test suite for instructions function signature validation""" + + @pytest.fixture + def mock_run_context(self): + """Create a mock RunContextWrapper for testing""" + return Mock(spec=RunContextWrapper) + + @pytest.mark.asyncio + async def test_valid_async_signature_passes(self, mock_run_context): + """Test that async function with correct signature works""" + async def valid_instructions(context, agent): + return "Valid async instructions" + + agent = Agent(instructions=valid_instructions) + result = await agent.get_system_prompt(mock_run_context) + assert result == "Valid async instructions" + + @pytest.mark.asyncio + async def test_valid_sync_signature_passes(self, mock_run_context): + """Test that sync function with correct signature works""" + def valid_instructions(context, agent): + return "Valid sync instructions" + + agent = Agent(instructions=valid_instructions) + result = await agent.get_system_prompt(mock_run_context) + assert result == "Valid sync instructions" + + @pytest.mark.asyncio + async def test_one_parameter_raises_error(self, mock_run_context): + """Test that function with only one parameter raises TypeError""" + def invalid_instructions(context): + return "Should fail" + + agent = Agent(instructions=invalid_instructions) + + with pytest.raises(TypeError) as exc_info: + await agent.get_system_prompt(mock_run_context) + + assert "must accept exactly 2 arguments" in str(exc_info.value) + assert "but got 1" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_three_parameters_raises_error(self, mock_run_context): + """Test that function with three parameters raises TypeError""" + def invalid_instructions(context, agent, extra): + return "Should fail" + + agent = Agent(instructions=invalid_instructions) + + with pytest.raises(TypeError) as exc_info: + await agent.get_system_prompt(mock_run_context) + + assert "must accept exactly 2 arguments" in str(exc_info.value) + assert "but got 3" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_zero_parameters_raises_error(self, mock_run_context): + """Test that function with no parameters raises TypeError""" + def invalid_instructions(): + return "Should fail" + + agent = Agent(instructions=invalid_instructions) + + with pytest.raises(TypeError) as exc_info: + await agent.get_system_prompt(mock_run_context) + + assert "must accept exactly 2 arguments" in str(exc_info.value) + assert "but got 0" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_function_with_args_kwargs_passes(self, mock_run_context): + """Test that function with *args/**kwargs still works (edge case)""" + def flexible_instructions(context, agent, *args, **kwargs): + return "Flexible instructions" + + agent = Agent(instructions=flexible_instructions) + # This should potentially pass as it can accept the 2 required args + # Adjust this test based on your desired behavior + result = await agent.get_system_prompt(mock_run_context) + assert result == "Flexible instructions" + + @pytest.mark.asyncio + async def test_string_instructions_still_work(self, mock_run_context): + """Test that string instructions continue to work""" + agent = Agent(instructions="Static string instructions") + result = await agent.get_system_prompt(mock_run_context) + assert result == "Static string instructions" + + @pytest.mark.asyncio + async def test_none_instructions_return_none(self, mock_run_context): + """Test that None instructions return None""" + agent = Agent(instructions=None) + result = await agent.get_system_prompt(mock_run_context) + assert result is None + + @pytest.mark.asyncio + async def test_non_callable_instructions_log_error(self, mock_run_context, caplog): + """Test that non-callable instructions log an error""" + agent = Agent(instructions=123) # Invalid type + result = await agent.get_system_prompt(mock_run_context) + assert result is None + # Check that error was logged (adjust based on actual logging setup) + # assert "Instructions must be a string or a function" in caplog.text \ No newline at end of file From ff402cf9fdc8cef78b356743921de75d0feba542 Mon Sep 17 00:00:00 2001 From: Abdullah Date: Tue, 12 Aug 2025 04:22:46 +0000 Subject: [PATCH 3/5] fixed errors --- src/agents/agent.py | 7 ++- tests/test_agent_instructions_signature.py | 72 +++++++++++----------- 2 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/agents/agent.py b/src/agents/agent.py index 4304ada14..6fe803078 100644 --- a/src/agents/agent.py +++ b/src/agents/agent.py @@ -414,7 +414,10 @@ async def get_system_prompt(self, run_context: RunContextWrapper[TContext]) -> s return cast(str, self.instructions(run_context, self)) elif self.instructions is not None: - logger.error(f"Instructions must be a string or a callable function, got {type(self.instructions).__name__}") + logger.error( + f"Instructions must be a string or a callable function, " + f"got {type(self.instructions).__name__}" + ) return None @@ -422,4 +425,4 @@ async def get_prompt( self, run_context: RunContextWrapper[TContext] ) -> ResponsePromptParam | None: """Get the prompt for the agent.""" - return await PromptUtil.to_model_input(self.prompt, run_context, self) \ No newline at end of file + return await PromptUtil.to_model_input(self.prompt, run_context, self) diff --git a/tests/test_agent_instructions_signature.py b/tests/test_agent_instructions_signature.py index ae61d8ca2..4cf6ceba3 100644 --- a/tests/test_agent_instructions_signature.py +++ b/tests/test_agent_instructions_signature.py @@ -1,109 +1,111 @@ -import pytest from unittest.mock import Mock -# Adjust import based on actual repo structure -from src.agents.agent import Agent, RunContextWrapper + +import pytest + +from agents.agent import Agent, RunContextWrapper + class TestInstructionsSignatureValidation: """Test suite for instructions function signature validation""" - + @pytest.fixture def mock_run_context(self): """Create a mock RunContextWrapper for testing""" return Mock(spec=RunContextWrapper) - + @pytest.mark.asyncio async def test_valid_async_signature_passes(self, mock_run_context): """Test that async function with correct signature works""" async def valid_instructions(context, agent): return "Valid async instructions" - - agent = Agent(instructions=valid_instructions) + + agent = Agent(name="test_agent", instructions=valid_instructions) result = await agent.get_system_prompt(mock_run_context) assert result == "Valid async instructions" - + @pytest.mark.asyncio async def test_valid_sync_signature_passes(self, mock_run_context): """Test that sync function with correct signature works""" def valid_instructions(context, agent): return "Valid sync instructions" - - agent = Agent(instructions=valid_instructions) + + agent = Agent(name="test_agent", instructions=valid_instructions) result = await agent.get_system_prompt(mock_run_context) assert result == "Valid sync instructions" - + @pytest.mark.asyncio async def test_one_parameter_raises_error(self, mock_run_context): """Test that function with only one parameter raises TypeError""" def invalid_instructions(context): return "Should fail" - - agent = Agent(instructions=invalid_instructions) - + + agent = Agent(name="test_agent", instructions=invalid_instructions) + with pytest.raises(TypeError) as exc_info: await agent.get_system_prompt(mock_run_context) - + assert "must accept exactly 2 arguments" in str(exc_info.value) assert "but got 1" in str(exc_info.value) - + @pytest.mark.asyncio async def test_three_parameters_raises_error(self, mock_run_context): """Test that function with three parameters raises TypeError""" def invalid_instructions(context, agent, extra): return "Should fail" - - agent = Agent(instructions=invalid_instructions) - + + agent = Agent(name="test_agent", instructions=invalid_instructions) + with pytest.raises(TypeError) as exc_info: await agent.get_system_prompt(mock_run_context) - + assert "must accept exactly 2 arguments" in str(exc_info.value) assert "but got 3" in str(exc_info.value) - + @pytest.mark.asyncio async def test_zero_parameters_raises_error(self, mock_run_context): """Test that function with no parameters raises TypeError""" def invalid_instructions(): return "Should fail" - - agent = Agent(instructions=invalid_instructions) - + + agent = Agent(name="test_agent", instructions=invalid_instructions) + with pytest.raises(TypeError) as exc_info: await agent.get_system_prompt(mock_run_context) - + assert "must accept exactly 2 arguments" in str(exc_info.value) assert "but got 0" in str(exc_info.value) - + @pytest.mark.asyncio async def test_function_with_args_kwargs_passes(self, mock_run_context): """Test that function with *args/**kwargs still works (edge case)""" def flexible_instructions(context, agent, *args, **kwargs): return "Flexible instructions" - - agent = Agent(instructions=flexible_instructions) + + agent = Agent(name="test_agent", instructions=flexible_instructions) # This should potentially pass as it can accept the 2 required args # Adjust this test based on your desired behavior result = await agent.get_system_prompt(mock_run_context) assert result == "Flexible instructions" - + @pytest.mark.asyncio async def test_string_instructions_still_work(self, mock_run_context): """Test that string instructions continue to work""" - agent = Agent(instructions="Static string instructions") + agent = Agent(name="test_agent", instructions="Static string instructions") result = await agent.get_system_prompt(mock_run_context) assert result == "Static string instructions" - + @pytest.mark.asyncio async def test_none_instructions_return_none(self, mock_run_context): """Test that None instructions return None""" - agent = Agent(instructions=None) + agent = Agent(name="test_agent", instructions=None) result = await agent.get_system_prompt(mock_run_context) assert result is None - + @pytest.mark.asyncio async def test_non_callable_instructions_log_error(self, mock_run_context, caplog): """Test that non-callable instructions log an error""" - agent = Agent(instructions=123) # Invalid type + agent = Agent(name="test_agent", instructions=123) # Invalid type result = await agent.get_system_prompt(mock_run_context) assert result is None # Check that error was logged (adjust based on actual logging setup) - # assert "Instructions must be a string or a function" in caplog.text \ No newline at end of file + # assert "Instructions must be a string or a function" in caplog.text From 6720cb55e348df2a60ff8e640ff5bb985714df6d Mon Sep 17 00:00:00 2001 From: Abdullah Date: Tue, 12 Aug 2025 04:31:17 +0000 Subject: [PATCH 4/5] fixed test file import from agents --- tests/test_agent_instructions_signature.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_agent_instructions_signature.py b/tests/test_agent_instructions_signature.py index 4cf6ceba3..e4b4779db 100644 --- a/tests/test_agent_instructions_signature.py +++ b/tests/test_agent_instructions_signature.py @@ -2,7 +2,7 @@ import pytest -from agents.agent import Agent, RunContextWrapper +from agents import Agent, RunContextWrapper class TestInstructionsSignatureValidation: From 9e81d4847dda7dcf346172e0de239559151af99b Mon Sep 17 00:00:00 2001 From: Abdullah Date: Tue, 12 Aug 2025 13:29:26 +0000 Subject: [PATCH 5/5] fix: Checked all tests, typechecks,lint --- tests/test_agent_instructions_signature.py | 34 ++++++++++++---------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/tests/test_agent_instructions_signature.py b/tests/test_agent_instructions_signature.py index e4b4779db..bd16f9f57 100644 --- a/tests/test_agent_instructions_signature.py +++ b/tests/test_agent_instructions_signature.py @@ -39,7 +39,7 @@ async def test_one_parameter_raises_error(self, mock_run_context): def invalid_instructions(context): return "Should fail" - agent = Agent(name="test_agent", instructions=invalid_instructions) + agent = Agent(name="test_agent", instructions=invalid_instructions) # type: ignore[arg-type] with pytest.raises(TypeError) as exc_info: await agent.get_system_prompt(mock_run_context) @@ -53,7 +53,7 @@ async def test_three_parameters_raises_error(self, mock_run_context): def invalid_instructions(context, agent, extra): return "Should fail" - agent = Agent(name="test_agent", instructions=invalid_instructions) + agent = Agent(name="test_agent", instructions=invalid_instructions) # type: ignore[arg-type] with pytest.raises(TypeError) as exc_info: await agent.get_system_prompt(mock_run_context) @@ -67,7 +67,7 @@ async def test_zero_parameters_raises_error(self, mock_run_context): def invalid_instructions(): return "Should fail" - agent = Agent(name="test_agent", instructions=invalid_instructions) + agent = Agent(name="test_agent", instructions=invalid_instructions) # type: ignore[arg-type] with pytest.raises(TypeError) as exc_info: await agent.get_system_prompt(mock_run_context) @@ -76,16 +76,18 @@ def invalid_instructions(): assert "but got 0" in str(exc_info.value) @pytest.mark.asyncio - async def test_function_with_args_kwargs_passes(self, mock_run_context): - """Test that function with *args/**kwargs still works (edge case)""" + async def test_function_with_args_kwargs_fails(self, mock_run_context): + """Test that function with *args/**kwargs fails validation""" def flexible_instructions(context, agent, *args, **kwargs): return "Flexible instructions" agent = Agent(name="test_agent", instructions=flexible_instructions) - # This should potentially pass as it can accept the 2 required args - # Adjust this test based on your desired behavior - result = await agent.get_system_prompt(mock_run_context) - assert result == "Flexible instructions" + + with pytest.raises(TypeError) as exc_info: + await agent.get_system_prompt(mock_run_context) + + assert "must accept exactly 2 arguments" in str(exc_info.value) + assert "but got" in str(exc_info.value) @pytest.mark.asyncio async def test_string_instructions_still_work(self, mock_run_context): @@ -102,10 +104,10 @@ async def test_none_instructions_return_none(self, mock_run_context): assert result is None @pytest.mark.asyncio - async def test_non_callable_instructions_log_error(self, mock_run_context, caplog): - """Test that non-callable instructions log an error""" - agent = Agent(name="test_agent", instructions=123) # Invalid type - result = await agent.get_system_prompt(mock_run_context) - assert result is None - # Check that error was logged (adjust based on actual logging setup) - # assert "Instructions must be a string or a function" in caplog.text + 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: + 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) + assert "got int" in str(exc_info.value)