diff --git a/python/packages/devui/agent_framework_devui/_conversations.py b/python/packages/devui/agent_framework_devui/_conversations.py index 1c8b908b104..0977aaed7d7 100644 --- a/python/packages/devui/agent_framework_devui/_conversations.py +++ b/python/packages/devui/agent_framework_devui/_conversations.py @@ -12,7 +12,7 @@ import uuid from abc import ABC, abstractmethod from collections.abc import MutableSequence -from typing import Any, Literal, cast +from typing import Any, Literal, cast, get_args from agent_framework import AgentSession, Content, Message, get_namespaced_value from agent_framework._workflows._checkpoint import InMemoryCheckpointStorage, WorkflowCheckpoint @@ -35,6 +35,14 @@ _MODEL_OUTPUT_REFUSAL = "refusal" +class ConversationItemValidationError(ValueError): + """An item carried no content that maps to agent framework content. + + Raised instead of a plain ValueError so routes can answer with a client + error for a malformed item while a missing conversation stays 404. + """ + + def _refusal_additional_properties() -> dict[str, Any]: return { "core": {_MODEL_OUTPUT_KIND_KEY: _MODEL_OUTPUT_REFUSAL}, @@ -45,6 +53,9 @@ def _refusal_additional_properties() -> dict[str, Any]: # Type alias for OpenAI Message role literals MessageRole = Literal["unknown", "user", "assistant", "system", "critic", "discriminator", "developer", "tool"] +# Roles accepted for a conversation message, derived from MessageRole itself. +_MESSAGE_ROLES = frozenset(get_args(MessageRole)) + # Checkpoint item type constants CONVERSATION_ITEM_TYPE_CHECKPOINT = "checkpoint" CONVERSATION_TYPE_CHECKPOINT_CONTAINER = "checkpoint_container" @@ -319,6 +330,14 @@ async def add_items(self, conversation_id: str, items: list[dict[str, Any]]) -> chat_messages: list[Message] = [] for item in items: role = item.get("role", "user") + # Reject an unknown role before anything is stored: the AF Message + # accepts any string, so the bad item would otherwise be persisted + # and only fail later at OpenAI conversion, leaving reads of the + # conversation broken from then on. The accepted vocabulary is the + # module's own MessageRole (which includes developer), so this + # check can never drift from the OpenAI conversion it guards. + if role not in _MESSAGE_ROLES: + raise ConversationItemValidationError(f"Conversation message has unsupported role {role!r}") content = item.get("content", []) contents: list[Content] = [] @@ -332,7 +351,7 @@ async def add_items(self, conversation_id: str, items: list[dict[str, Any]]) -> contents.append(agent_content) if not contents: - raise ValueError("Conversation message did not contain any supported content") + raise ConversationItemValidationError("Conversation message did not contain any supported content") chat_msg = Message(role=role, contents=contents) chat_messages.append(chat_msg) diff --git a/python/packages/devui/agent_framework_devui/_server.py b/python/packages/devui/agent_framework_devui/_server.py index bc94dee9a21..30d8ab48318 100644 --- a/python/packages/devui/agent_framework_devui/_server.py +++ b/python/packages/devui/agent_framework_devui/_server.py @@ -21,6 +21,7 @@ from fastapi.responses import JSONResponse, StreamingResponse from fastapi.staticfiles import StaticFiles +from ._conversations import ConversationItemValidationError from ._deployment import DeploymentManager from ._discovery import EntityDiscovery from ._executor import AgentFrameworkExecutor @@ -1084,6 +1085,8 @@ async def create_conversation_items(conversation_id: str, request_data: dict[str items = request_data.get("items", []) conv_items = await executor.conversation_store.add_items(conversation_id, items=items) return {"object": "list", "data": [item.model_dump() for item in conv_items]} + except ConversationItemValidationError as e: + raise HTTPException(status_code=400, detail=str(e)) from e except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e except HTTPException: diff --git a/python/packages/devui/tests/devui/test_server.py b/python/packages/devui/tests/devui/test_server.py index 56055fd137a..3c85085001a 100644 --- a/python/packages/devui/tests/devui/test_server.py +++ b/python/packages/devui/tests/devui/test_server.py @@ -635,6 +635,64 @@ def test_streaming_response_does_not_hardcode_acao_header(): ) +def test_unsupported_conversation_item_content_returns_client_error(): + """Posting an item whose content parts are all unsupported is a client error on an + existing conversation; 404 stays reserved for a missing conversation.""" + server = _server_with_mock_agent(auth_enabled=False) + app = server.get_app() + + with TestClient(app, base_url="http://127.0.0.1") as client: + created = client.post("/v1/conversations", json={}) + assert created.status_code == 200 + conversation_id = created.json()["id"] + + unsupported = client.post( + f"/v1/conversations/{conversation_id}/items", + json={ + "items": [ + { + "type": "message", + "role": "user", + "content": [{"type": "input_video", "input_video": "data:video/mp4;base64,AAAA"}], + } + ] + }, + ) + assert unsupported.status_code == 400 + assert "supported content" in unsupported.json()["detail"] + + # An unknown role is rejected as a client error before anything is + # stored, so later reads of the conversation keep working. + bogus_role = client.post( + f"/v1/conversations/{conversation_id}/items", + json={"items": [{"type": "message", "role": "bogus", "content": "hi"}]}, + ) + assert bogus_role.status_code == 400 + assert "unsupported role" in bogus_role.json()["detail"] + items_after = client.get(f"/v1/conversations/{conversation_id}/items") + assert items_after.status_code == 200 + + # Every role in the module's MessageRole vocabulary is accepted, + # including developer instructions, and reads back intact. + for accepted_role in ("developer", "system", "user", "assistant"): + posted = client.post( + f"/v1/conversations/{conversation_id}/items", + json={"items": [{"type": "message", "role": accepted_role, "content": "msg"}]}, + ) + assert posted.status_code == 200, accepted_role + listed = client.get(f"/v1/conversations/{conversation_id}/items") + assert listed.status_code == 200 + roles_read_back = [item.get("role") for item in listed.json()["data"]] + for accepted_role in ("developer", "system", "user", "assistant"): + assert accepted_role in roles_read_back, accepted_role + + missing = client.post( + "/v1/conversations/does-not-exist/items", + json={"items": [{"type": "message", "role": "user", "content": "hello"}]}, + ) + assert missing.status_code == 404 + + def test_cors_default_does_not_allow_arbitrary_origin_even_on_localhost(): """Default CORS must not echo Access-Control-Allow-Origin to arbitrary origins.