feat(librarian): send explicit non-empty user on every library-desk request
Library-desk is removing its server-side default user, so a request without an explicit tenant will 422 after its next deploy: - New client-level _resolve_user() resolves the tenant (explicit arg or request context) and raises ValueError on an empty/whitespace value BEFORE any bytes hit the wire; all 15 tenant-scoped methods use it - extract_content / extract_content_batch now accept and send the user (query param), matching the rest of the API surface - search_web no longer falls back to a phantom "tatlock-librarian" tenant; it sends the resolved user - health_check stays user-less (public, not tenant-scoped) Tests: parametrized sweep pins the wire contract (user present in params or payload) for every tenant-scoped method, for both context and explicit users; empty-tenant calls are asserted to fail without any HTTP call; the recorded-fixture hybrid contract test now pins user as an explicit query param. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,22 +2,23 @@
|
||||
Tests for the Library-Desk HTTP client.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from src.agents.librarian.client import (
|
||||
LibraryDeskClient,
|
||||
Dossier,
|
||||
EntityLinking,
|
||||
GraphNode,
|
||||
HybridRAGResponse,
|
||||
HybridSearchResult,
|
||||
LibraryDeskClient,
|
||||
ResearchSummary,
|
||||
SmartCreateResponse,
|
||||
VectorSearchResult,
|
||||
WikiPage,
|
||||
WikiSearchResult,
|
||||
VectorSearchResult,
|
||||
GraphNode,
|
||||
Dossier,
|
||||
SmartCreateResponse,
|
||||
ResearchSummary,
|
||||
EntityLinking,
|
||||
)
|
||||
|
||||
|
||||
@@ -549,6 +550,128 @@ class TestSmartCreateWikiPage:
|
||||
assert result.research_summary.web_results == 0
|
||||
|
||||
|
||||
# All tenant-scoped client methods with minimal call kwargs. Used to sweep
|
||||
# the explicit-user contract: every library-desk request must carry a
|
||||
# non-empty user (the service is removing its server-side default).
|
||||
TENANT_SCOPED_METHODS = [
|
||||
("hybrid_search", {"query": "q"}),
|
||||
("search_wiki", {"query": "q"}),
|
||||
("get_wiki_page", {"page_id": 1}),
|
||||
("list_wiki_pages", {}),
|
||||
("create_wiki_page", {"title": "t", "path": "/p", "content": "c"}),
|
||||
("update_wiki_page", {"page_id": 1, "content": "c"}),
|
||||
("smart_create_wiki_page", {"topic": "t", "tags": ["x"]}),
|
||||
("list_dossiers", {}),
|
||||
("semantic_search", {"query": "q"}),
|
||||
("query_graph", {"cypher_query": "MATCH (n) RETURN n"}),
|
||||
("list_graph_nodes", {}),
|
||||
("get_graph_node", {"node_id": "n1"}),
|
||||
("search_web", {"query": "q"}),
|
||||
("extract_content", {"url": "http://example.com"}),
|
||||
("extract_content_batch", {"urls": ["http://example.com"]}),
|
||||
]
|
||||
|
||||
# One permissive response body that satisfies every method's parser
|
||||
# (extra keys are ignored by the pydantic models).
|
||||
UNIVERSAL_RESPONSE = {
|
||||
"id": 1,
|
||||
"path": "/p",
|
||||
"title": "T",
|
||||
"results": [],
|
||||
"pages": [],
|
||||
"dossiers": [],
|
||||
"records": [],
|
||||
"nodes": [],
|
||||
"keywords": [],
|
||||
"page": {"id": 1, "path": "/p", "title": "T"},
|
||||
"result": {"url": "http://example.com", "success": True},
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestExplicitUserContract:
|
||||
"""Every library-desk request sends a non-empty user explicitly."""
|
||||
|
||||
def _wire_client(self):
|
||||
mock_response = MagicMock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = UNIVERSAL_RESPONSE
|
||||
mock_response.raise_for_status = MagicMock()
|
||||
|
||||
mock_httpx = AsyncMock(spec=httpx.AsyncClient)
|
||||
mock_httpx.get.return_value = mock_response
|
||||
mock_httpx.post.return_value = mock_response
|
||||
mock_httpx.put.return_value = mock_response
|
||||
|
||||
client = LibraryDeskClient(base_url="http://test:8089", api_key="k")
|
||||
client._client = mock_httpx
|
||||
return client, mock_httpx
|
||||
|
||||
def _sent_user(self, mock_httpx) -> str:
|
||||
"""Extract the user sent on the single outgoing request."""
|
||||
calls = (
|
||||
mock_httpx.get.call_args_list
|
||||
+ mock_httpx.post.call_args_list
|
||||
+ mock_httpx.put.call_args_list
|
||||
)
|
||||
assert len(calls) == 1, "expected exactly one outgoing request"
|
||||
kwargs = calls[0].kwargs
|
||||
params = kwargs.get("params") or {}
|
||||
payload = kwargs.get("json") or {}
|
||||
return params.get("user") or payload.get("user") or ""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("method_name,kwargs", TENANT_SCOPED_METHODS)
|
||||
async def test_user_from_context_is_sent_on_the_wire(
|
||||
self, method_name, kwargs
|
||||
):
|
||||
"""With no explicit user, the context user is resolved and sent."""
|
||||
client, mock_httpx = self._wire_client()
|
||||
|
||||
with patch(
|
||||
"src.agents.librarian.client.get_user", return_value="llm_tester"
|
||||
):
|
||||
await getattr(client, method_name)(**kwargs)
|
||||
|
||||
assert self._sent_user(mock_httpx) == "llm_tester"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("method_name,kwargs", TENANT_SCOPED_METHODS)
|
||||
async def test_explicit_user_is_sent_on_the_wire(self, method_name, kwargs):
|
||||
"""An explicitly passed user is sent unchanged."""
|
||||
client, mock_httpx = self._wire_client()
|
||||
|
||||
await getattr(client, method_name)(user="test_phase_b", **kwargs)
|
||||
|
||||
assert self._sent_user(mock_httpx) == "test_phase_b"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("method_name,kwargs", TENANT_SCOPED_METHODS)
|
||||
async def test_empty_context_user_fails_before_any_request(
|
||||
self, method_name, kwargs
|
||||
):
|
||||
"""An empty resolved user raises before any bytes hit the wire."""
|
||||
client, mock_httpx = self._wire_client()
|
||||
|
||||
with patch("src.agents.librarian.client.get_user", return_value=""):
|
||||
with pytest.raises(ValueError, match="non-empty user"):
|
||||
await getattr(client, method_name)(**kwargs)
|
||||
|
||||
mock_httpx.get.assert_not_called()
|
||||
mock_httpx.post.assert_not_called()
|
||||
mock_httpx.put.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_explicit_whitespace_user_is_rejected(self):
|
||||
"""A whitespace-only explicit user is rejected."""
|
||||
client, mock_httpx = self._wire_client()
|
||||
|
||||
with pytest.raises(ValueError, match="non-empty user"):
|
||||
await client.hybrid_search("q", user=" ")
|
||||
|
||||
mock_httpx.post.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestNewResponseModels:
|
||||
"""Tests for new response models."""
|
||||
|
||||
@@ -150,6 +150,21 @@ class TestHybridRAGContract:
|
||||
assert config["enable_documents"] is False
|
||||
assert config["enable_volatile"] is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_user_always_sent_as_query_param(
|
||||
self, client_with_recorded_response
|
||||
):
|
||||
"""The tenant is always sent explicitly - library-desk is removing
|
||||
its server-side default, so a missing user would 422."""
|
||||
await client_with_recorded_response.hybrid_search(
|
||||
"home server infrastructure", user="testuser"
|
||||
)
|
||||
|
||||
params = client_with_recorded_response._client.post.call_args.kwargs[
|
||||
"params"
|
||||
]
|
||||
assert params["user"] == "testuser"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_source_counts_and_timing_parsed(
|
||||
self, client_with_recorded_response
|
||||
|
||||
Reference in New Issue
Block a user