fix: remove invalid mode kwarg from trace_span + add integration tests
- Remove mode=mode.value from trace_span calls (trace_span only accepts name and logger parameters) - Add TestPermissionModeIntegration tests that verify mode string->enum conversion works correctly through the full request flow - Add TestAgentMethodSignatures tests that verify function signatures match expected interfaces (catches invalid kwargs at test time) These tests would have caught both the mode string/enum issue and the trace_span invalid kwarg issue before they hit production. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -142,7 +142,7 @@ class TaskAgentImpl(BaseAgent):
|
||||
# Get agent configured for this mode
|
||||
agent = self._get_agent_for_mode(mode)
|
||||
|
||||
async with trace_span("task_agent_run", mode=mode.value):
|
||||
async with trace_span("task_agent_run"):
|
||||
try:
|
||||
result = await agent.run(prompt, deps=ctx)
|
||||
return result.output
|
||||
@@ -180,7 +180,7 @@ class TaskAgentImpl(BaseAgent):
|
||||
# Get agent configured for this mode
|
||||
agent = self._get_agent_for_mode(mode)
|
||||
|
||||
async with trace_span("task_agent_stream", mode=mode.value):
|
||||
async with trace_span("task_agent_stream"):
|
||||
try:
|
||||
async with agent.run_stream(prompt, deps=ctx) as result:
|
||||
async for chunk in result.stream_text():
|
||||
|
||||
@@ -1,7 +1,13 @@
|
||||
"""
|
||||
Tests for agent REST API endpoints.
|
||||
|
||||
Includes integration tests that verify real code paths work correctly
|
||||
without over-mocking (only LLM calls are mocked).
|
||||
"""
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
|
||||
from src.domains.agents.schemas import PermissionMode
|
||||
|
||||
|
||||
class TestAgentListEndpoint:
|
||||
@@ -160,3 +166,184 @@ class TestAgentStreamEndpoint:
|
||||
)
|
||||
# Unknown agent returns 400, not streaming
|
||||
assert response.status_code == 400
|
||||
|
||||
|
||||
class TestPermissionModeIntegration:
|
||||
"""
|
||||
Integration tests for permission mode handling.
|
||||
|
||||
These tests verify that mode strings are correctly converted to enums
|
||||
and that the full request->router->agent flow works for each mode.
|
||||
"""
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_run_with_default_mode(self, auth_client):
|
||||
"""Test running agent with default mode passes through correctly."""
|
||||
# Mock the agent.run method to avoid LLM calls
|
||||
mock_result = MagicMock()
|
||||
mock_result.output = "Test response"
|
||||
|
||||
with patch("src.domains.agents.task.agent.TaskAgentImpl._get_agent_for_mode") as mock_get_agent:
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.run = AsyncMock(return_value=mock_result)
|
||||
mock_get_agent.return_value = mock_agent
|
||||
|
||||
response = await auth_client.post(
|
||||
"/agents/run",
|
||||
json={
|
||||
"prompt": "test prompt",
|
||||
"agent_type": "task",
|
||||
"working_dir": ".",
|
||||
"mode": "default"
|
||||
}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["success"] is True
|
||||
assert data["mode"] == "default"
|
||||
|
||||
# Verify mode was converted to enum and passed correctly
|
||||
mock_get_agent.assert_called_once_with(PermissionMode.default)
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_run_with_plan_mode(self, auth_client):
|
||||
"""Test running agent with plan mode passes through correctly."""
|
||||
mock_result = MagicMock()
|
||||
mock_result.output = "Plan response"
|
||||
|
||||
with patch("src.domains.agents.task.agent.TaskAgentImpl._get_agent_for_mode") as mock_get_agent:
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.run = AsyncMock(return_value=mock_result)
|
||||
mock_get_agent.return_value = mock_agent
|
||||
|
||||
response = await auth_client.post(
|
||||
"/agents/run",
|
||||
json={
|
||||
"prompt": "test prompt",
|
||||
"agent_type": "task",
|
||||
"working_dir": ".",
|
||||
"mode": "plan"
|
||||
}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["mode"] == "plan"
|
||||
mock_get_agent.assert_called_once_with(PermissionMode.plan)
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_run_with_auto_accept_mode(self, auth_client):
|
||||
"""Test running agent with auto_accept mode passes through correctly."""
|
||||
mock_result = MagicMock()
|
||||
mock_result.output = "Auto accept response"
|
||||
|
||||
with patch("src.domains.agents.task.agent.TaskAgentImpl._get_agent_for_mode") as mock_get_agent:
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.run = AsyncMock(return_value=mock_result)
|
||||
mock_get_agent.return_value = mock_agent
|
||||
|
||||
response = await auth_client.post(
|
||||
"/agents/run",
|
||||
json={
|
||||
"prompt": "test prompt",
|
||||
"agent_type": "task",
|
||||
"working_dir": ".",
|
||||
"mode": "auto_accept"
|
||||
}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["mode"] == "auto_accept"
|
||||
mock_get_agent.assert_called_once_with(PermissionMode.auto_accept)
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_run_with_invalid_mode(self, auth_client):
|
||||
"""Test running agent with invalid mode returns validation error."""
|
||||
response = await auth_client.post(
|
||||
"/agents/run",
|
||||
json={
|
||||
"prompt": "test prompt",
|
||||
"agent_type": "task",
|
||||
"working_dir": ".",
|
||||
"mode": "invalid_mode"
|
||||
}
|
||||
)
|
||||
|
||||
assert response.status_code == 422
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_stream_with_plan_mode(self, auth_client):
|
||||
"""Test streaming agent with plan mode passes through correctly."""
|
||||
async def mock_stream(*args, **kwargs):
|
||||
yield "chunk1"
|
||||
yield "chunk2"
|
||||
|
||||
with patch("src.domains.agents.task.agent.TaskAgentImpl._get_agent_for_mode") as mock_get_agent:
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.run_stream = MagicMock(return_value=MagicMock(
|
||||
__aenter__=AsyncMock(return_value=MagicMock(
|
||||
stream_text=lambda: mock_stream()
|
||||
)),
|
||||
__aexit__=AsyncMock(return_value=None)
|
||||
))
|
||||
mock_get_agent.return_value = mock_agent
|
||||
|
||||
response = await auth_client.post(
|
||||
"/agents/stream",
|
||||
json={
|
||||
"prompt": "test prompt",
|
||||
"agent_type": "task",
|
||||
"working_dir": ".",
|
||||
"mode": "plan"
|
||||
}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.headers["content-type"] == "text/event-stream; charset=utf-8"
|
||||
mock_get_agent.assert_called_once_with(PermissionMode.plan)
|
||||
|
||||
|
||||
class TestAgentMethodSignatures:
|
||||
"""
|
||||
Tests to verify agent method signatures match expected interfaces.
|
||||
|
||||
These catch issues like passing invalid kwargs to methods.
|
||||
"""
|
||||
|
||||
def test_task_agent_run_accepts_mode(self):
|
||||
"""Verify TaskAgentImpl.run() accepts mode parameter."""
|
||||
from src.domains.agents.task.agent import TaskAgentImpl
|
||||
import inspect
|
||||
|
||||
sig = inspect.signature(TaskAgentImpl.run)
|
||||
params = list(sig.parameters.keys())
|
||||
|
||||
assert "mode" in params
|
||||
# Verify mode has correct type annotation
|
||||
mode_param = sig.parameters["mode"]
|
||||
assert mode_param.default == PermissionMode.default
|
||||
|
||||
def test_task_agent_run_stream_accepts_mode(self):
|
||||
"""Verify TaskAgentImpl.run_stream() accepts mode parameter."""
|
||||
from src.domains.agents.task.agent import TaskAgentImpl
|
||||
import inspect
|
||||
|
||||
sig = inspect.signature(TaskAgentImpl.run_stream)
|
||||
params = list(sig.parameters.keys())
|
||||
|
||||
assert "mode" in params
|
||||
mode_param = sig.parameters["mode"]
|
||||
assert mode_param.default == PermissionMode.default
|
||||
|
||||
def test_trace_span_signature(self):
|
||||
"""Verify trace_span only accepts expected parameters."""
|
||||
from src.shared.logging import trace_span
|
||||
import inspect
|
||||
|
||||
sig = inspect.signature(trace_span.__init__)
|
||||
params = list(sig.parameters.keys())
|
||||
|
||||
# Should only have self, name, logger - not mode or other extras
|
||||
assert params == ["self", "name", "logger"]
|
||||
|
||||
Reference in New Issue
Block a user