From 6df65c9c7aaef92f56b662047ff154be1c6a894c Mon Sep 17 00:00:00 2001 From: Atirna <288419661+atirna@users.noreply.github.com> Date: Thu, 3 Sep 2026 10:29:47 +0530 Subject: [PATCH 1/2] fix(confluence): limit simple-search fallback --- src/mcp_atlassian/servers/confluence.py | 5 ++++- tests/unit/servers/test_confluence_server.py | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/mcp_atlassian/servers/confluence.py b/src/mcp_atlassian/servers/confluence.py index 5c5b3a18c..7b9eb48fb 100644 --- a/src/mcp_atlassian/servers/confluence.py +++ b/src/mcp_atlassian/servers/confluence.py @@ -13,6 +13,7 @@ from fastmcp import Context from mcp.types import BlobResourceContents, EmbeddedResource, ImageContent, TextContent from pydantic import BeforeValidator, Field +from requests.exceptions import HTTPError from mcp_atlassian.exceptions import MCPAtlassianAuthenticationError from mcp_atlassian.models.confluence import ConfluenceAttachment @@ -262,7 +263,9 @@ async def search( pages = confluence_fetcher.search( query, limit=limit, spaces_filter=spaces_filter ) - except Exception as e: + except HTTPError as e: + if e.response is None or e.response.status_code != 400: + raise logger.warning(f"siteSearch failed ('{e}'), falling back to text search.") query = f'text ~ "{original_query}"' logger.info(f"Falling back to text search with CQL: {query}") diff --git a/tests/unit/servers/test_confluence_server.py b/tests/unit/servers/test_confluence_server.py index a9cbd6f97..05c29c8bd 100644 --- a/tests/unit/servers/test_confluence_server.py +++ b/tests/unit/servers/test_confluence_server.py @@ -511,17 +511,26 @@ async def test_search(client, mock_confluence_fetcher): @pytest.mark.anyio -async def test_search_returns_error_details(client, mock_confluence_fetcher): +@pytest.mark.parametrize( + ("query", "expected_query"), + [("type=page", "type=page"), ("test search", 'siteSearch ~ "test search"')], +) +async def test_search_returns_error_details( + client, mock_confluence_fetcher, query, expected_query +): """Test that search tool failures preserve the original error message.""" - mock_confluence_fetcher.search.side_effect = RuntimeError( + mock_confluence_fetcher.search.side_effect = ValueError( "Confluence CQL rejected the query" ) with pytest.raises(ToolError) as excinfo: - await client.call_tool("confluence_search", {"query": "type=page"}) + await client.call_tool("confluence_search", {"query": query}) assert "Error calling tool 'search'" in str(excinfo.value) assert "Confluence CQL rejected the query" in str(excinfo.value) + mock_confluence_fetcher.search.assert_called_once_with( + expected_query, limit=10, spaces_filter=None + ) @pytest.mark.anyio From c238af11d663e45a14fdef6cb3c675081d860b85 Mon Sep 17 00:00:00 2001 From: Atirna <288419661+atirna@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:41:36 +0530 Subject: [PATCH 2/2] fix(confluence): catch the wrapped CQL parse error in the fallback cql() raises ApiValueError for a 400, which the fetcher's handle_atlassian_api_errors wraps in RuntimeError, so the previous HTTPError-only except never fired and the #270 fallback regressed. Unwrap the ApiError reason when the 400 arrives as RuntimeError and drive both carriers in the tests. --- src/mcp_atlassian/servers/confluence.py | 18 ++++++- tests/unit/servers/test_confluence_server.py | 52 ++++++++++++++++---- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/mcp_atlassian/servers/confluence.py b/src/mcp_atlassian/servers/confluence.py index 7b9eb48fb..945fa2565 100644 --- a/src/mcp_atlassian/servers/confluence.py +++ b/src/mcp_atlassian/servers/confluence.py @@ -10,6 +10,7 @@ from typing import Annotated from urllib.parse import parse_qs, urlsplit +from atlassian.errors import ApiError from fastmcp import Context from mcp.types import BlobResourceContents, EmbeddedResource, ImageContent, TextContent from pydantic import BeforeValidator, Field @@ -263,8 +264,21 @@ async def search( pages = confluence_fetcher.search( query, limit=limit, spaces_filter=spaces_filter ) - except HTTPError as e: - if e.response is None or e.response.status_code != 400: + except (HTTPError, RuntimeError) as e: + # Confluence.cql() converts an HTTP 400 (unparseable CQL) into + # ApiValueError, which handle_atlassian_api_errors then wraps as + # RuntimeError, so the 400 can arrive here as either type. + status_error = e + if isinstance(e, RuntimeError): + status_error = e.__cause__ + if not isinstance(status_error, ApiError): + raise + status_error = status_error.reason + if ( + not isinstance(status_error, HTTPError) + or status_error.response is None + or status_error.response.status_code != 400 + ): raise logger.warning(f"siteSearch failed ('{e}'), falling back to text search.") query = f'text ~ "{original_query}"' diff --git a/tests/unit/servers/test_confluence_server.py b/tests/unit/servers/test_confluence_server.py index 05c29c8bd..a22b20a4e 100644 --- a/tests/unit/servers/test_confluence_server.py +++ b/tests/unit/servers/test_confluence_server.py @@ -5,12 +5,14 @@ import logging from collections.abc import AsyncGenerator from contextlib import asynccontextmanager -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, call, patch import pytest +from atlassian.errors import ApiValueError from fastmcp import Client, FastMCP from fastmcp.client import FastMCPTransport from fastmcp.exceptions import ToolError +from requests.exceptions import HTTPError from starlette.requests import Request from src.mcp_atlassian.confluence import ConfluenceFetcher @@ -510,24 +512,56 @@ async def test_search(client, mock_confluence_fetcher): assert result_data[0]["title"] == "Test Page Mock Title" +def _search_runtime_error(status_code: int) -> RuntimeError: + """The chain handle_atlassian_api_errors produces for a cql() 400.""" + error = RuntimeError("Unexpected error during search") + error.__cause__ = ApiValueError( + "The query cannot be parsed", + reason=HTTPError(str(status_code), response=MagicMock(status_code=status_code)), + ) + return error + + +@pytest.mark.anyio +async def test_search_falls_back_on_cql_parse_error(client, mock_confluence_fetcher): + """A 400 CQL parse rejection falls back to text search exactly once.""" + mock_confluence_fetcher.search.side_effect = [_search_runtime_error(400), []] + + result = await client.call_tool("confluence_search", {"query": "test search"}) + + assert mock_confluence_fetcher.search.call_args_list == [ + call('siteSearch ~ "test search"', limit=10, spaces_filter=None), + call('text ~ "test search"', limit=10, spaces_filter=None), + ] + assert json.loads(result.content[0].text) == [] + + @pytest.mark.anyio @pytest.mark.parametrize( - ("query", "expected_query"), - [("type=page", "type=page"), ("test search", 'siteSearch ~ "test search"')], + ("query", "expected_query", "side_effect"), + [ + ("test search", 'siteSearch ~ "test search"', ValueError("read timeout")), + ("type=page", "type=page", ValueError("read timeout")), + ( + "test search", + 'siteSearch ~ "test search"', + HTTPError("500 Server Error", response=MagicMock(status_code=500)), + ), + ("test search", 'siteSearch ~ "test search"', RuntimeError("unexpected")), + ], + ids=["read-timeout", "cql-query", "http-500", "unexpected"], ) async def test_search_returns_error_details( - client, mock_confluence_fetcher, query, expected_query + client, mock_confluence_fetcher, query, expected_query, side_effect ): - """Test that search tool failures preserve the original error message.""" - mock_confluence_fetcher.search.side_effect = ValueError( - "Confluence CQL rejected the query" - ) + """Non-400 search failures surface their message once, without a fallback.""" + mock_confluence_fetcher.search.side_effect = side_effect with pytest.raises(ToolError) as excinfo: await client.call_tool("confluence_search", {"query": query}) assert "Error calling tool 'search'" in str(excinfo.value) - assert "Confluence CQL rejected the query" in str(excinfo.value) + assert str(side_effect) in str(excinfo.value) mock_confluence_fetcher.search.assert_called_once_with( expected_query, limit=10, spaces_filter=None )