fix(agents): make librarian failures structured and user-safe
- run_librarian / run_librarian_stream raise AgentError instead of
returning/yielding error text as normal output; detail stays in logs
- delegate_to_* wrappers now put a curated butler-toned sentence in
DelegationResult.output on failure and never expose str(e), so
streaming's error branch is reachable and honest
- _execute_single_delegation propagates success; direct delegation only
records delegate_to_* as called when the expert actually succeeded
- librarian tools return user-safe messages instead of
'Error searching: {e}' strings that leaked internal URLs into
synthesis; coordination stream errors are curated as well
- ruff cleanups (TYPE_CHECKING forward refs, B904, unused locals) in
the touched files to keep them lint-clean
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,60 @@
|
||||
"""
|
||||
Tests for structured failure behavior of The Librarian entry points.
|
||||
|
||||
run_librarian and run_librarian_stream must raise AgentError on failure
|
||||
instead of returning/yielding error text as if it were research output,
|
||||
and the raised error must not leak exception detail (internal URLs etc.).
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from src.agents.librarian.agent import run_librarian, run_librarian_stream
|
||||
from src.agents.protocol import AgentError
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestRunLibrarianFailures:
|
||||
"""run_librarian raises structured errors instead of returning text."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_librarian_raises_agent_error(self):
|
||||
"""Failures raise AgentError rather than returning error prose."""
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.run = AsyncMock(
|
||||
side_effect=RuntimeError("Connection refused to http://internal:8089")
|
||||
)
|
||||
|
||||
with patch(
|
||||
"src.agents.librarian.agent.get_librarian_agent",
|
||||
return_value=mock_agent,
|
||||
):
|
||||
with pytest.raises(AgentError) as exc_info:
|
||||
await run_librarian(task="Find Docker docs")
|
||||
|
||||
assert exc_info.value.agent_name == "librarian"
|
||||
# Exception detail stays in logs only
|
||||
assert "internal" not in str(exc_info.value)
|
||||
assert "Connection refused" not in str(exc_info.value)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_librarian_stream_raises_agent_error(self):
|
||||
"""Streaming failures raise instead of yielding error text."""
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.run_stream = MagicMock(
|
||||
side_effect=RuntimeError("Connection refused to http://internal:8089")
|
||||
)
|
||||
|
||||
with patch(
|
||||
"src.agents.librarian.agent.get_librarian_agent",
|
||||
return_value=mock_agent,
|
||||
):
|
||||
collected: list[str] = []
|
||||
with pytest.raises(AgentError) as exc_info:
|
||||
async for delta in run_librarian_stream(task="Find Docker docs"):
|
||||
collected.append(delta)
|
||||
|
||||
assert exc_info.value.agent_name == "librarian"
|
||||
assert collected == [], "no error text may be yielded as output"
|
||||
assert "internal" not in str(exc_info.value)
|
||||
@@ -5,21 +5,20 @@ Tests the tool functions that wrap the Library-Desk API,
|
||||
including the new web search and content extraction tools.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from src.agents.librarian.client import (
|
||||
BatchExtractionResponse,
|
||||
ContentExtractionResult,
|
||||
WebSearchResponse,
|
||||
WebSearchResult,
|
||||
)
|
||||
from src.agents.librarian.tools import (
|
||||
search_web,
|
||||
read_url,
|
||||
read_urls_batch,
|
||||
hybrid_search,
|
||||
search_wiki,
|
||||
)
|
||||
from src.agents.librarian.client import (
|
||||
WebSearchResult,
|
||||
WebSearchResponse,
|
||||
ContentExtractionResult,
|
||||
BatchExtractionResponse,
|
||||
search_web,
|
||||
)
|
||||
|
||||
|
||||
@@ -104,8 +103,10 @@ class TestSearchWeb:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_search_web_error_handling(self, mock_client):
|
||||
"""Test web search error handling."""
|
||||
mock_client.search_web.side_effect = Exception("Connection failed")
|
||||
"""Test web search errors return a user-safe message without internals."""
|
||||
mock_client.search_web.side_effect = Exception(
|
||||
"Connection failed to http://internal-host:8089"
|
||||
)
|
||||
|
||||
with patch(
|
||||
"src.agents.librarian.tools.LibraryDeskClient"
|
||||
@@ -115,8 +116,10 @@ class TestSearchWeb:
|
||||
|
||||
result = await search_web("test query")
|
||||
|
||||
assert "Error" in result
|
||||
assert "Connection failed" in result
|
||||
assert "unable to search the web" in result
|
||||
# Exception detail (internal URLs etc.) must not leak
|
||||
assert "Connection failed" not in result
|
||||
assert "internal-host" not in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_search_web_with_news_type(self, mock_client):
|
||||
@@ -227,7 +230,7 @@ class TestReadUrl:
|
||||
mock_client_class.return_value.__aenter__.return_value = mock_client
|
||||
mock_client_class.return_value.__aexit__.return_value = None
|
||||
|
||||
result = await read_url("https://example.com/long", max_length=2000)
|
||||
await read_url("https://example.com/long", max_length=2000)
|
||||
|
||||
mock_client.extract_content.assert_called_with(
|
||||
url="https://example.com/long",
|
||||
|
||||
@@ -4,18 +4,19 @@ Tests for delegation infrastructure.
|
||||
Tests the DelegationTask dataclass and delegation wrapper functions
|
||||
that implement the agent-as-tool pattern.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
|
||||
from src.agents.delegation import (
|
||||
ActionType,
|
||||
DelegationTask,
|
||||
DelegationResult,
|
||||
HOUSEHOLD_THINK_MESSAGES,
|
||||
STREAMING_DELEGATION_WRAPPERS,
|
||||
ActionType,
|
||||
DelegationResult,
|
||||
DelegationTask,
|
||||
_detect_action_type,
|
||||
delegate_to_librarian,
|
||||
get_think_message,
|
||||
_detect_action_type,
|
||||
)
|
||||
|
||||
|
||||
@@ -170,11 +171,11 @@ class TestDelegateToLibrarian:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delegate_to_librarian_handles_error(self):
|
||||
"""Test delegation handles Librarian errors gracefully."""
|
||||
"""Test delegation maps Librarian errors to a user-safe result."""
|
||||
with patch(
|
||||
"src.agents.librarian.agent.run_librarian",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=Exception("Connection refused"),
|
||||
side_effect=Exception("Connection refused to http://internal:8089"),
|
||||
):
|
||||
result = await delegate_to_librarian(
|
||||
task="Search for information",
|
||||
@@ -182,8 +183,15 @@ class TestDelegateToLibrarian:
|
||||
|
||||
assert isinstance(result, DelegationResult)
|
||||
assert result.success is False
|
||||
assert result.output == ""
|
||||
assert result.error == "Connection refused"
|
||||
# Output carries a curated butler-toned sentence
|
||||
assert result.output == get_think_message(
|
||||
"librarian", "Search for information", "error"
|
||||
)
|
||||
# Exception detail stays in logs only - never in the result
|
||||
assert "Connection refused" not in result.output
|
||||
assert result.error is not None
|
||||
assert "Connection refused" not in result.error
|
||||
assert "internal" not in result.error
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delegate_to_librarian_preserves_task(self):
|
||||
|
||||
Reference in New Issue
Block a user