diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index 1fe12428f..cd4ed1d5e 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -30,7 +30,8 @@ from .credentials import NotConfigured, get_credential -ChatMessage = dict[str, str] +# content is usually str; multimodal vision messages use OpenAI content-parts lists. +ChatMessage = dict[str, Any] @dataclass(frozen=True) @@ -745,6 +746,9 @@ def _coerce_input_text(value: Any) -> str: if isinstance(item, str): parts.append(item) elif isinstance(item, dict): + # OpenAI content-parts: {"type": "text", "text": "..."} + if isinstance(item.get("text"), str): + parts.append(item["text"]) content = item.get("content") if isinstance(content, str): parts.append(content) @@ -755,6 +759,15 @@ def _coerce_input_text(value: Any) -> str: return " ".join(parts) +def _coerce_message_content_text(content: Any) -> str: + """Best-effort plain text from chat message content (string or content-parts).""" + if isinstance(content, str): + return content + if isinstance(content, list): + return _coerce_input_text(content) + return "" + + def load_agents(path: str) -> list[ModelAgent]: # pragma: no cover """Load model agent definitions from an agents JSON file.""" with open(path, encoding="utf-8") as handle: @@ -1734,7 +1747,13 @@ def _needs_workflow(self, text: str) -> bool: return hits >= self.policy.conduct_hint_threshold or len(text) > 700 def _latest_user_text(self, messages: list[ChatMessage]) -> str: - return next((m.get("content", "") for m in reversed(messages) if m.get("role") == "user"), "") # pragma: no cover + for message in reversed(messages): + if message.get("role") != "user": + continue + text = _coerce_message_content_text(message.get("content", "")) + if text: + return text + return "" # pragma: no cover def _model_judge_verification(self, task: str, fallback: dict[str, Any]) -> dict[str, Any]: """Ask a model to judge the verifier report (fixes term-matching false negatives). diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 14bc89a83..6871365d0 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -1208,10 +1208,57 @@ def _require_pool_model(orchestrator: Any, model_name: str) -> None: ) -def _validate_messages(messages: Any) -> list[dict[str, str]]: +def _validate_message_content_parts(content: list[Any]) -> list[dict[str, Any]]: + """OpenAI multimodal content-parts array (text + image_url) for vision callers. + + Parts are shape-checked and returned for provider passthrough. Unsupported + part types fail closed with a named error so clients never believe audio or + other modalities were processed. + """ + if not content: + raise RequestError( + 400, + "invalid_message_content", + "multipart content arrays must be non-empty", + ) + parts: list[dict[str, Any]] = [] + for part in content: + if not isinstance(part, dict): + raise RequestError( + 400, + "invalid_message_content", + "message content part must be an object", + ) + part_type = part.get("type") + if part_type == "text": + if not isinstance(part.get("text"), str): + raise RequestError( + 400, + "invalid_message_content", + "text content part requires a string text field", + ) + elif part_type == "image_url": + image_url = part.get("image_url") + if not isinstance(image_url, dict) or not isinstance(image_url.get("url"), str): + raise RequestError( + 400, + "invalid_message_content", + "image_url content part requires image_url.url as a string", + ) + else: + raise RequestError( + 400, + "invalid_message_content", + "content part type must be text or image_url", + ) + parts.append(part) + return parts + + +def _validate_messages(messages: Any) -> list[dict[str, Any]]: if not isinstance(messages, list) or not messages: raise RequestError(400, "invalid_message", "messages must be a non-empty array") - validated: list[dict[str, str]] = [] + validated: list[dict[str, Any]] = [] for message in messages: if not isinstance(message, dict): raise RequestError(400, "invalid_message", "each message must be an object") @@ -1225,34 +1272,27 @@ def _validate_messages(messages: Any) -> list[dict[str, str]]: "invalid_message_role", "developer role is not supported on /v1/chat/completions; use system instead", ) - if isinstance(content, list): - # OpenAI multimodal content parts (text/image_url/input_audio/...) are not - # applied by this text-only gateway. Fail closed so SDKs cannot silently - # believe vision/audio parts were processed as plain text. - raise RequestError( - 400, - "invalid_message_content", - "multipart content arrays are not supported on /v1/chat/completions; " - "pass a string content", - ) if not isinstance(role, str) or role not in ALLOWED_MESSAGE_ROLES: raise RequestError(400, "invalid_message", "message role or content is invalid") # OpenAI assistant tool turns often send content:null with tool_calls; treat # explicit JSON null as empty string on assistant/tool (SDK optional default). if content is None and role in {"assistant", "tool"}: content = "" - if not isinstance(content, str): + if isinstance(content, list): + # Vision/omni callers send OpenAI content-parts arrays. Shape-check and + # passthrough text+image_url; other part types fail closed. + content = _validate_message_content_parts(content) + elif not isinstance(content, str): raise RequestError(400, "invalid_message", "message role or content is invalid") - # User/system turns drive the prompt — empty content is never applied and - # would only create silent no-op turns. Assistant/tool may still use empty - # content when tool_calls or tool results carry the payload. - if role in {"user", "system"} and not content.strip(): + # User/system turns drive the prompt — empty string content is never applied. + # Multimodal arrays are non-empty after parts validation. + if role in {"user", "system"} and isinstance(content, str) and not content.strip(): raise RequestError( 400, "invalid_message_content", "user and system message content must be a non-empty string", ) - entry: dict[str, str] = {"role": role, "content": content} + entry: dict[str, Any] = {"role": role, "content": content} if role == "tool": # OpenAI tool messages bind results to a prior tool_call via tool_call_id. tool_call_id = message.get("tool_call_id") diff --git a/tests/test_chat_developer_multimodal_content_http_honesty.py b/tests/test_chat_developer_multimodal_content_http_honesty.py index f2ba09740..319fb0793 100644 --- a/tests/test_chat_developer_multimodal_content_http_honesty.py +++ b/tests/test_chat_developer_multimodal_content_http_honesty.py @@ -88,7 +88,8 @@ def test_http_chat_rejects_developer_role() -> None: thread.join(timeout=5) -def test_http_chat_rejects_multipart_image_content() -> None: +def test_http_chat_accepts_multipart_image_content() -> None: + """Vision callers send text+image_url parts; shape-check and passthrough.""" server, thread, port = _server() try: status, body = _post( @@ -109,10 +110,8 @@ def test_http_chat_rejects_multipart_image_content() -> None: ], }, ) - assert status == 400, body - blob = json.dumps(body) - assert "invalid_message_content" in blob - assert "multipart" in blob or "not supported" in blob + assert status == 200, body + assert "choices" in body finally: server.shutdown() thread.join(timeout=5) @@ -165,7 +164,7 @@ def test_http_chat_rejects_non_string_non_array_content() -> None: if __name__ == "__main__": test_http_chat_accepts_string_content() test_http_chat_rejects_developer_role() - test_http_chat_rejects_multipart_image_content() + test_http_chat_accepts_multipart_image_content() test_http_chat_rejects_input_audio_content_part() test_http_chat_rejects_non_string_non_array_content() print("ok") diff --git a/tests/test_multimodal_message_content_http_honesty.py b/tests/test_multimodal_message_content_http_honesty.py new file mode 100644 index 000000000..6a038f70c --- /dev/null +++ b/tests/test_multimodal_message_content_http_honesty.py @@ -0,0 +1,146 @@ +"""OpenAI multimodal content-parts (text + image_url) honesty over HTTP.""" + +from __future__ import annotations + +import json +import threading +import urllib.error +import urllib.request +from pathlib import Path +import sys + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from contextual_orchestrator import ModelAgent, TaskOrchestrator # noqa: E402 +from contextual_orchestrator.server import SecurityConfig, build_server # noqa: E402 + +_TEST_AUTH_TOKEN = "multimodal_message_content_http_honesty_token" # noqa: S105 + + +def build() -> TaskOrchestrator: + return TaskOrchestrator( + [ModelAgent("general_agent", "mock-planner", tags=("reasoning", "writing"))] + ) + + +def _post(port: int, payload: dict) -> tuple[int, dict]: + request = urllib.request.Request( + f"http://127.0.0.1:{port}/v1/chat/completions", + data=json.dumps(payload).encode("utf-8"), + headers={ + "content-type": "application/json", + "authorization": f"Bearer {_TEST_AUTH_TOKEN}", + "connection": "close", + }, + method="POST", + ) + try: + with urllib.request.urlopen(request, timeout=15) as response: + return response.status, json.loads(response.read().decode("utf-8")) + except urllib.error.HTTPError as exc: + return exc.code, json.loads(exc.read().decode("utf-8")) + + +def _server(): + server = build_server( + build(), + port=0, + security=SecurityConfig(auth_token=_TEST_AUTH_TOKEN, rate_limit_requests=10_000), + ) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + return server, thread, server.server_address[1] + + +def test_http_chat_accepts_text_and_image_url_parts() -> None: + server, thread, port = _server() + try: + status, body = _post( + port, + { + "model": "mock-planner", + "messages": [ + { + "role": "user", + "content": [ + {"type": "text", "text": "describe this image"}, + { + "type": "image_url", + "image_url": { + "url": "data:image/png;base64,iVBORw0KGgo=" + }, + }, + ], + } + ], + }, + ) + assert status == 200, body + assert body.get("object") == "chat.completion" or "choices" in body + finally: + server.shutdown() + thread.join(timeout=5) + + +def test_http_chat_rejects_unknown_content_part_type() -> None: + server, thread, port = _server() + try: + status, body = _post( + port, + { + "model": "mock-planner", + "messages": [ + { + "role": "user", + "content": [{"type": "input_audio", "input_audio": {}}], + } + ], + }, + ) + assert status == 400, body + assert "invalid_message_content" in json.dumps(body) + finally: + server.shutdown() + thread.join(timeout=5) + + +def test_http_chat_rejects_empty_multipart_array() -> None: + server, thread, port = _server() + try: + status, body = _post( + port, + { + "model": "mock-planner", + "messages": [{"role": "user", "content": []}], + }, + ) + assert status == 400, body + blob = json.dumps(body) + assert "invalid_message" in blob or "invalid_message_content" in blob + finally: + server.shutdown() + thread.join(timeout=5) + + +def test_http_chat_still_accepts_string_content() -> None: + server, thread, port = _server() + try: + status, body = _post( + port, + { + "model": "mock-planner", + "messages": [{"role": "user", "content": "plain text"}], + }, + ) + assert status == 200, body + finally: + server.shutdown() + thread.join(timeout=5) + + +if __name__ == "__main__": + test_http_chat_accepts_text_and_image_url_parts() + test_http_chat_rejects_unknown_content_part_type() + test_http_chat_rejects_empty_multipart_array() + test_http_chat_still_accepts_string_content() + print("ok")