diff --git a/CHANGELOG.md b/CHANGELOG.md index a8a1901..e630010 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## [Unreleased] +### Fixed + +- **Hosted invocation compatibility.** Include the required `api-version=v1` + query parameter and accept the current Responses `conversation.id` envelope + alongside legacy `conversation_id`, rejecting malformed or conflicting IDs. +- **Backend-specific history registration.** Hosted chat no longer registers + the classic Dapr-backed data layer; OAuth, feedback, session context and + classic authenticated history remain unchanged. + ### Changed - Technically validate candidate exception registry updates without mandatory diff --git a/src/gpt_rag_ui/api/callbacks.py b/src/gpt_rag_ui/api/callbacks.py index ce962b1..124c861 100644 --- a/src/gpt_rag_ui/api/callbacks.py +++ b/src/gpt_rag_ui/api/callbacks.py @@ -1,9 +1,12 @@ """Single registration owner for Chainlit callbacks and data-layer wiring.""" +import logging + import chainlit as cl from gpt_rag_ui.services import chat +logger = logging.getLogger(__name__) _registered = False @@ -16,9 +19,15 @@ def register_callbacks() -> None: register_oauth_callback() if chat.OAUTH_CONFIGURED or chat.COPILOT_ENABLED: - from gpt_rag_ui.api.history import register_data_layer - - register_data_layer() + if chat.CHAT_BACKEND == "orchestrator": + from gpt_rag_ui.api.history import register_data_layer + + register_data_layer() + else: + logger.info( + "Hosted chat does not register the classic orchestrator history " + "data layer; conversation context remains session-scoped" + ) if chat.ENABLE_FEEDBACK: from gpt_rag_ui.api.feedback import register_feedback_handlers diff --git a/src/gpt_rag_ui/clients/hosted_agent_client.py b/src/gpt_rag_ui/clients/hosted_agent_client.py index b958e1b..89cfbfa 100644 --- a/src/gpt_rag_ui/clients/hosted_agent_client.py +++ b/src/gpt_rag_ui/clients/hosted_agent_client.py @@ -549,6 +549,7 @@ async def stream( async with self._http_client.stream( "POST", self.settings.invocations_url, + params={"api-version": "v1"}, json=payload, headers=headers, ) as response: @@ -574,9 +575,21 @@ async def stream( "response.created is missing its response object." ) new_conversation_id = response_data.get("conversation_id") - if not isinstance(new_conversation_id, str) or not new_conversation_id: + conversation = response_data.get("conversation") + if conversation is not None: + if not isinstance(conversation, dict): + raise HostedAgentProtocolError( + "response.created has an invalid conversation object." + ) + nested_id = conversation.get("id") + if new_conversation_id is not None and new_conversation_id != nested_id: + raise HostedAgentProtocolError( + "response.created has conflicting conversation identifiers." + ) + new_conversation_id = nested_id + if not isinstance(new_conversation_id, str) or not new_conversation_id.strip(): raise HostedAgentProtocolError( - "response.created is missing managed conversation_id." + "response.created is missing a conversation identifier." ) managed_conversation_id = new_conversation_id yield "", {"conversation_id": new_conversation_id} diff --git a/tests/test_callback_registration.py b/tests/test_callback_registration.py new file mode 100644 index 0000000..36a001d --- /dev/null +++ b/tests/test_callback_registration.py @@ -0,0 +1,90 @@ +"""Backend-specific callback registration without network or Chainlit sessions.""" + +import importlib.util +import sys +import unittest +from types import ModuleType, SimpleNamespace +from unittest.mock import Mock, patch + + +class TestCallbackRegistration(unittest.TestCase): + def register(self, backend, *, oauth=True, copilot=False, feedback=False): + spec = importlib.util.find_spec("gpt_rag_ui.api.callbacks") + self.assertIsNotNone(spec) + self.assertIsNotNone(spec.loader) + chainlit = ModuleType("chainlit") + for name in ("on_chat_start", "on_chat_resume", "on_message"): + setattr(chainlit, name, Mock()) + chat = SimpleNamespace( + CHAT_BACKEND=backend, + OAUTH_CONFIGURED=oauth, + COPILOT_ENABLED=copilot, + ENABLE_FEEDBACK=feedback, + ALLOW_ANONYMOUS=not oauth, + get_auth_info=Mock(), + on_chat_start=Mock(), + on_chat_resume=Mock(), + handle_message=Mock(), + ) + services = ModuleType("gpt_rag_ui.services") + services.chat = chat + history = ModuleType("gpt_rag_ui.api.history") + history.register_data_layer = Mock() + auth = ModuleType("gpt_rag_ui.api.oauth") + auth.register_oauth_callback = Mock() + feedback_module = ModuleType("gpt_rag_ui.api.feedback") + feedback_module.register_feedback_handlers = Mock() + modules = { + "chainlit": chainlit, + "gpt_rag_ui.services": services, + "gpt_rag_ui.api.history": history, + "gpt_rag_ui.api.oauth": auth, + "gpt_rag_ui.api.feedback": feedback_module, + } + callbacks = importlib.util.module_from_spec(spec) + with patch.dict(sys.modules, modules): + spec.loader.exec_module(callbacks) + callbacks.register_callbacks() + callbacks.register_callbacks() + for name in ("on_chat_start", "on_chat_resume", "on_message"): + getattr(chainlit, name).assert_called_once_with(getattr(chat, { + "on_message": "handle_message", + }.get(name, name))) + return history, auth, feedback_module, chat + + def test_hosted_retains_oauth_without_classic_history(self): + history, auth, _, _ = self.register("hosted_agent") + history.register_data_layer.assert_not_called() + auth.register_oauth_callback.assert_called_once() + + def test_classic_retains_authenticated_history(self): + history, auth, _, _ = self.register("orchestrator") + history.register_data_layer.assert_called_once() + auth.register_oauth_callback.assert_called_once() + + def test_hosted_copilot_does_not_register_classic_history(self): + history, auth, _, _ = self.register("hosted_agent", oauth=False, copilot=True) + history.register_data_layer.assert_not_called() + auth.register_oauth_callback.assert_not_called() + + def test_classic_copilot_preserves_history(self): + history, auth, _, _ = self.register("orchestrator", oauth=False, copilot=True) + history.register_data_layer.assert_called_once() + auth.register_oauth_callback.assert_not_called() + + def test_anonymous_classic_does_not_enable_history(self): + history, auth, _, _ = self.register("orchestrator", oauth=False) + history.register_data_layer.assert_not_called() + auth.register_oauth_callback.assert_not_called() + + def test_hosted_preserves_authenticated_feedback(self): + _, _, feedback, chat = self.register("hosted_agent", feedback=True) + feedback.register_feedback_handlers.assert_called_once_with( + chat.get_auth_info, allow_standalone_anonymous=False + ) + + def test_hosted_preserves_explicit_anonymous_feedback(self): + _, _, feedback, chat = self.register("hosted_agent", oauth=False, feedback=True) + feedback.register_feedback_handlers.assert_called_once_with( + chat.get_auth_info, allow_standalone_anonymous=True + ) diff --git a/tests/test_hosted_agent_client.py b/tests/test_hosted_agent_client.py index 0bb74da..624003b 100644 --- a/tests/test_hosted_agent_client.py +++ b/tests/test_hosted_agent_client.py @@ -546,7 +546,7 @@ async def test_real_frames_surface_text_citation_tool_and_completion(self): self.assertEqual(capture[0]["method"], "POST") self.assertEqual( capture[0]["url"], - "https://agent.example.com/protocol/invocations", + "https://agent.example.com/protocol/invocations?api-version=v1", ) transmitted_authorization = capture[0]["headers"]["authorization"] self.assertEqual( @@ -676,6 +676,65 @@ async def handler(request: httpx.Request) -> httpx.Response: ): self.assertNotIn(forbidden, captures[1]) + async def test_accepts_canonical_conversation_object(self): + body = _frame( + "response.created", + {"response": {"conversation": {"id": "conv-managed"}}}, + ) + _completed() + client = await self._client_for_body(body) + results = await self._collect(client, [InvocationMessage("user", "hello")]) + self.assertIn(("", {"conversation_id": "conv-managed"}), results) + self.assertTrue(any(meta.get("completed") for _, meta in results)) + + async def test_accepts_matching_canonical_and_legacy_identifiers(self): + body = _frame( + "response.created", + { + "response": { + "conversation": {"id": "conv-managed"}, + "conversation_id": "conv-managed", + } + }, + ) + _completed() + client = await self._client_for_body(body) + results = await self._collect(client, [InvocationMessage("user", "hello")]) + self.assertIn(("", {"conversation_id": "conv-managed"}), results) + + async def test_rejects_malformed_conversation_identifiers(self): + invalid_responses = [ + {}, + {"conversation_id": ""}, + {"conversation_id": " "}, + {"conversation_id": 42}, + {"conversation": "conv-managed"}, + {"conversation": {}}, + {"conversation": {"id": ""}}, + {"conversation": {"id": " "}}, + {"conversation": {"id": 42}}, + {"conversation": {"id": None}}, + ] + for response in invalid_responses: + with self.subTest(response=response): + client = await self._client_for_body( + _frame("response.created", {"response": response}) + _completed() + ) + with self.assertRaises(HostedAgentProtocolError): + await self._collect(client, [InvocationMessage("user", "hello")]) + + async def test_rejects_conflicting_conversation_identifiers(self): + body = _frame( + "response.created", + { + "response": { + "conversation": {"id": "conv-managed"}, + "conversation_id": "another-conversation", + } + }, + ) + _completed() + client = await self._client_for_body(body) + with self.assertRaisesRegex(HostedAgentProtocolError, "conflicting"): + await self._collect(client, [InvocationMessage("user", "hello")]) + async def test_error_frame_raises_typed_error(self): body = _created("conv-1") + _frame( "error", diff --git a/tests/test_installed_package.py b/tests/test_installed_package.py index 4d4e06e..5723520 100644 --- a/tests/test_installed_package.py +++ b/tests/test_installed_package.py @@ -138,7 +138,9 @@ def test_both_import_orders_have_one_owner_and_one_registration(self): register_callbacks() register_callbacks() assert start.call_count == resume.call_count == message.call_count == 1 - assert data.call_count == oauth.call_count == 1 + from gpt_rag_ui.services import chat + assert data.call_count == int(chat.CHAT_BACKEND == "orchestrator") + assert oauth.call_count == 1 import dependencies from gpt_rag_ui.config import dependencies as owner assert dependencies.get_config is owner.get_config