Handle chat ProblemDetails SSE errors

Author: rodion-m

src/tests/test_chat_tool.py

@@ -58,6 +58,7 @@ async def mock_aiter_lines():
     ]
 
     assert result == "Hello world"
+    assert call_args.kwargs["headers"]["Accept"] == "text/event-stream, application/problem+json"
     assert call_args.kwargs["headers"]["X-CodeAlive-Tool"] == "chat"
 
 
@@ -139,20 +140,122 @@ async def mock_aiter_lines():
     result = await chat(
         ctx=ctx,
         question="Follow up",
-        conversation_id="conv_123"
+        conversation_id="69fceb3e7b2a6a7efdd18180"
     )
 
     call_args = mock_client.post.call_args
     request_data = call_args.kwargs["json"]
 
     # Should include conversation ID
-    assert request_data["conversationId"] == "conv_123"
+    assert request_data["conversationId"] == "69fceb3e7b2a6a7efdd18180"
     # Should not have explicit names when continuing conversation
     assert "names" not in request_data
-
     assert result == "Continued"
 
 
+@pytest.mark.asyncio
+@patch('tools.chat.get_api_key_from_context')
+async def test_chat_rejects_non_objectid_conversation_id(mock_get_api_key):
+    """Invalid continuation IDs fail locally with an actionable ToolError."""
+    mock_get_api_key.return_value = "test_key"
+
+    ctx = MagicMock(spec=Context)
+    ctx.info = AsyncMock()
+    ctx.warning = AsyncMock()
+    ctx.error = AsyncMock()
+
+    with pytest.raises(ToolError) as exc:
+        await chat(
+            ctx=ctx,
+            question="Follow up",
+            conversation_id="conv_123",
+        )
+
+    msg = str(exc.value)
+    assert "24-character hex Mongo ObjectId" in msg
+    assert "Retry: no" in msg
+
+
+@pytest.mark.asyncio
+@patch('tools.chat.get_api_key_from_context')
+async def test_chat_named_sse_error_raises_tool_error(mock_get_api_key):
+    """RFC 9457 `event: error` frames must not collapse to an empty answer."""
+    mock_get_api_key.return_value = "test_key"
+
+    ctx = MagicMock(spec=Context)
+    ctx.info = AsyncMock()
+    ctx.warning = AsyncMock()
+    ctx.error = AsyncMock()
+
+    mock_response = MagicMock()
+    mock_response.raise_for_status = MagicMock()
+
+    async def mock_aiter_lines():
+        yield 'event: error'
+        yield 'data: {"title":"Bad request","status":400,"detail":"Message content violates our content policy","requestId":"req-1"}'
+        yield ''
+
+    mock_response.aiter_lines = mock_aiter_lines
+
+    mock_client = AsyncMock()
+    mock_client.post.return_value = mock_response
+
+    mock_codealive_context = MagicMock()
+    mock_codealive_context.client = mock_client
+    mock_codealive_context.base_url = "https://app.codealive.ai"
+
+    ctx.request_context.lifespan_context = mock_codealive_context
+
+    with pytest.raises(ToolError) as exc:
+        await chat(ctx=ctx, question="Test question", data_sources=["repo123"])
+
+    msg = str(exc.value)
+    assert "Message content violates our content policy" in msg
+    assert "Code: 400" in msg
+    assert "Retry: no" in msg
+    assert "requestId=req-1" in msg
+
+
+@pytest.mark.asyncio
+@patch('tools.chat.get_api_key_from_context')
+async def test_chat_named_sse_rate_limit_error_is_retryable(mock_get_api_key):
+    """429 ProblemDetails frames should tell agents to back off, not fix input."""
+    mock_get_api_key.return_value = "test_key"
+
+    ctx = MagicMock(spec=Context)
+    ctx.info = AsyncMock()
+    ctx.warning = AsyncMock()
+    ctx.error = AsyncMock()
+
+    mock_response = MagicMock()
+    mock_response.raise_for_status = MagicMock()
+
+    async def mock_aiter_lines():
+        yield 'event: error'
+        yield 'data: {"title":"Plan limit","status":429,"detail":"Chat completion rate limit exceeded","requestId":"req-429"}'
+        yield ''
+
+    mock_response.aiter_lines = mock_aiter_lines
+
+    mock_client = AsyncMock()
+    mock_client.post.return_value = mock_response
+
+    mock_codealive_context = MagicMock()
+    mock_codealive_context.client = mock_client
+    mock_codealive_context.base_url = "https://app.codealive.ai"
+
+    ctx.request_context.lifespan_context = mock_codealive_context
+
+    with pytest.raises(ToolError) as exc:
+        await chat(ctx=ctx, question="Test question", data_sources=["repo123"])
+
+    msg = str(exc.value)
+    assert "Chat completion rate limit exceeded" in msg
+    assert "Retry: yes" in msg
+    assert "back off" in msg
+    assert "requestId=req-429" in msg
+
+
 @pytest.mark.asyncio
 @patch('tools.chat.get_api_key_from_context')
 async def test_chat_empty_question_validation(mock_get_api_key):

src/tests/test_e2e_tools.py

@@ -988,7 +988,11 @@ def handler(req):
 
 class TestChatE2E:
     @staticmethod
-    def _sse_body(chunks: list[str], conv_id: str = "conv-42", msg_id: str = "msg-1") -> str:
+    def _sse_body(
+        chunks: list[str],
+        conv_id: str = "69fceb3e7b2a6a7efdd18180",
+        msg_id: str = "69fceb3e7b2a6a7efdd18181",
+    ) -> str:
         """Build an SSE response body with metadata + content chunks + DONE."""
         lines = [
             "event: message",
@@ -1011,6 +1015,7 @@ def handler(req):
             data = json.loads(req.content)
             assert data["stream"] is True
             assert data["messages"][0]["content"] == "How does auth work?"
+            assert req.headers["accept"] == "text/event-stream, application/problem+json"
             return httpx.Response(200, text=body, headers={"content-type": "text/event-stream"})
 
         mcp = _server({"/api/chat/completions": handler})
@@ -1023,27 +1028,42 @@ def handler(req):
         text = _text(result)
         assert "Hello world!" in text
         # New conversation gets ID appended
-        assert "conv-42" in text
+        assert "69fceb3e7b2a6a7efdd18180" in text
 
     @pytest.mark.asyncio
     async def test_continuing_conversation(self):
-        body = self._sse_body(["Follow-up answer"], conv_id="conv-existing")
+        conversation_id = "69fceb3e7b2a6a7efdd18180"
+        body = self._sse_body(["Follow-up answer"], conv_id=conversation_id)
 
         def handler(req):
             data = json.loads(req.content)
-            assert data["conversationId"] == "conv-existing"
+            assert data["conversationId"] == conversation_id
             return httpx.Response(200, text=body, headers={"content-type": "text/event-stream"})
 
         mcp = _server({"/api/chat/completions": handler})
         async with Client(mcp) as client:
             result = await client.call_tool(
                 "chat",
-                {"question": "And the error handling?", "conversation_id": "conv-existing"},
+                {"question": "And the error handling?", "conversation_id": conversation_id},
             )
 
         text = _text(result)
         assert "Follow-up answer" in text
 
+    @pytest.mark.asyncio
+    async def test_invalid_conversation_id_returns_actionable_tool_error(self):
+        mcp = _server({})
+        async with Client(mcp) as client:
+            result = await client.call_tool(
+                "chat",
+                {"question": "And the error handling?", "conversation_id": "conv-existing"},
+                raise_on_error=False,
+            )
+
+        text = _text(result)
+        assert "24-character hex Mongo ObjectId" in text
+        assert "Retry: no" in text
+
     @pytest.mark.asyncio
     async def test_empty_question_returns_error(self):
         mcp = _server({})
@@ -1071,6 +1091,97 @@ async def test_backend_error_handled(self):
         text = _text(result)
         assert "401" in text or "auth" in text.lower()
 
+    @pytest.mark.asyncio
+    async def test_problem_details_backend_error_keeps_detail_and_request_id(self):
+        problem = {
+            "type": "https://app.codealive.ai/errors/bad-request",
+            "title": "Bad request",
+            "status": 400,
+            "detail": "Message content violates our content policy",
+            "requestId": "req-rest",
+        }
+
+        mcp = _server({
+            "/api/chat/completions": lambda r: httpx.Response(
+                400,
+                json=problem,
+                headers={"content-type": "application/problem+json"},
+            ),
+        })
+        async with Client(mcp) as client:
+            result = await client.call_tool(
+                "chat",
+                {"question": "hello"},
+                raise_on_error=False,
+            )
+
+        text = _text(result)
+        assert "Message content violates our content policy" in text
+        assert "requestId=req-rest" in text
+        assert "Retry: no" in text
+
+    @pytest.mark.asyncio
+    async def test_named_sse_problem_details_error_returns_tool_error(self):
+        problem = json.dumps({
+            "type": "https://app.codealive.ai/errors/bad-request",
+            "title": "Bad request",
+            "status": 400,
+            "detail": "Message content violates our content policy",
+            "requestId": "req-sse",
+        })
+        body = f"event: error\ndata: {problem}\n\n"
+
+        mcp = _server({
+            "/api/chat/completions": lambda r: httpx.Response(
+                200,
+                text=body,
+                headers={"content-type": "text/event-stream"},
+            ),
+        })
+        async with Client(mcp) as client:
+            result = await client.call_tool(
+                "chat",
+                {"question": "hello", "data_sources": ["backend"]},
+                raise_on_error=False,
+            )
+
+        text = _text(result)
+        assert "Message content violates our content policy" in text
+        assert "Code: 400" in text
+        assert "requestId=req-sse" in text
+        assert "Retry: no" in text
+
+    @pytest.mark.asyncio
+    async def test_named_sse_rate_limit_error_is_retryable(self):
+        problem = json.dumps({
+            "type": "https://app.codealive.ai/errors/plan-limit",
+            "title": "Plan limit",
+            "status": 429,
+            "detail": "Chat completion rate limit exceeded",
+            "requestId": "req-sse-429",
+        })
+        body = f"event: error\ndata: {problem}\n\n"
+
+        mcp = _server({
+            "/api/chat/completions": lambda r: httpx.Response(
+                200,
+                text=body,
+                headers={"content-type": "text/event-stream"},
+            ),
+        })
+        async with Client(mcp) as client:
+            result = await client.call_tool(
+                "chat",
+                {"question": "hello", "data_sources": ["backend"]},
+                raise_on_error=False,
+            )
+
+        text = _text(result)
+        assert "Chat completion rate limit exceeded" in text
+        assert "Retry: yes" in text
+        assert "back off" in text
+        assert "requestId=req-sse-429" in text
+
     @pytest.mark.asyncio
     async def test_unicode_preserved_in_streamed_response(self):
         """Cyrillic chunks streamed via SSE must survive as UTF-8 in the final text."""