diff --git a/pyproject.toml b/pyproject.toml index 1bb95676..ce51441e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "sap-cloud-sdk" -version = "0.48.2" +version = "0.48.3" description = "SAP Cloud SDK for Python" readme = "README.md" license = "Apache-2.0" diff --git a/src/sap_cloud_sdk/agentgateway/_customer.py b/src/sap_cloud_sdk/agentgateway/_customer.py index 676f2384..f0ec1020 100644 --- a/src/sap_cloud_sdk/agentgateway/_customer.py +++ b/src/sap_cloud_sdk/agentgateway/_customer.py @@ -668,6 +668,7 @@ async def _list_server_tools( ) result = await session.list_tools() + tools = result.tools or [] return [ MCPTool( @@ -677,7 +678,7 @@ async def _list_server_tools( input_schema=mcp_input_schema(t), url=url, ) - for t in result.tools + for t in tools ] diff --git a/src/sap_cloud_sdk/agentgateway/_lob.py b/src/sap_cloud_sdk/agentgateway/_lob.py index fd9d352a..8a495979 100644 --- a/src/sap_cloud_sdk/agentgateway/_lob.py +++ b/src/sap_cloud_sdk/agentgateway/_lob.py @@ -30,8 +30,8 @@ FragmentLabel, get_ias_fragment_name, get_ias_user_fragment_name, - list_mcp_fragments, list_a2a_fragments, + list_mcp_fragments, ) from sap_cloud_sdk.agentgateway._compat import ( mcp_input_schema, @@ -39,10 +39,10 @@ mcp_server_name, ) from sap_cloud_sdk.agentgateway._models import ( - JsonRpcError, Agent, AgentCard, AgentCardFilter, + JsonRpcError, MCPTool, MCPToolFilter, ) @@ -386,6 +386,11 @@ async def list_server_tools( init_result = await session.initialize() server_name = mcp_server_name(init_result) or fragment_name result = await session.list_tools() + tools = result.tools or [] + if not tools: + logger.info( + "No tools returned by AGW for fragment '%s'", fragment_name + ) return [ MCPTool( name=t.name, @@ -395,7 +400,7 @@ async def list_server_tools( url=dest_url, fragment_name=fragment_name, ) - for t in result.tools + for t in tools ] diff --git a/tests/agentgateway/unit/test_lob.py b/tests/agentgateway/unit/test_lob.py index 98c6ce02..d570b9cd 100644 --- a/tests/agentgateway/unit/test_lob.py +++ b/tests/agentgateway/unit/test_lob.py @@ -1,5 +1,6 @@ """Unit tests for LoB agent flow.""" +import logging import os from unittest.mock import patch, MagicMock, AsyncMock @@ -24,6 +25,7 @@ get_agent_cards_lob, _fetch_agent_card, call_mcp_tool_lob, + list_server_tools, ) from sap_cloud_sdk.agentgateway._models import ( Agent, @@ -35,7 +37,10 @@ from sap_cloud_sdk.agentgateway._token_cache import _GatewayUrlCache, _TokenCache from sap_cloud_sdk.agentgateway.config import ClientConfig from sap_cloud_sdk.destination import ConsumptionOptions, ConsumptionLevel -from sap_cloud_sdk.agentgateway.exceptions import AgentGatewaySDKError, MCPServerNotFoundError +from sap_cloud_sdk.agentgateway.exceptions import ( + AgentGatewaySDKError, + MCPServerNotFoundError, +) from sap_cloud_sdk.destination import ConsumptionLevel # Aliases for use in existing test assertions @@ -117,7 +122,9 @@ def test_strips_trailing_slashes_from_url(self): mock_dest.auth_tokens[0].http_header = {"value": header_value} mock_dest.url = "https://agw.example.com/v1/mcp///" - with patch("sap_cloud_sdk.agentgateway._lob.create_destination_client") as mock_client: + with patch( + "sap_cloud_sdk.agentgateway._lob.create_destination_client" + ) as mock_client: mock_client.return_value.get_destination.return_value = mock_dest result = _fetch_auth_token("dest-name", "tenant-sub") @@ -300,7 +307,9 @@ def test_returns_fragment_name(self): fragment = MagicMock() fragment.name = "sap-managed-runtime-agw-subscriber-ias-user-abc123" - with patch("sap_cloud_sdk.agentgateway._fragments.create_fragment_client") as mock_client: + with patch( + "sap_cloud_sdk.agentgateway._fragments.create_fragment_client" + ) as mock_client: mock_client.return_value.list_instance_fragments.return_value = [fragment] result = get_ias_user_fragment_name("tenant-sub") @@ -312,7 +321,9 @@ def test_uses_correct_filter_labels(self): fragment = MagicMock() fragment.name = "ias-user-fragment" - with patch("sap_cloud_sdk.agentgateway._fragments.create_fragment_client") as mock_client: + with patch( + "sap_cloud_sdk.agentgateway._fragments.create_fragment_client" + ) as mock_client: mock_client.return_value.list_instance_fragments.return_value = [fragment] get_ias_user_fragment_name("tenant-sub") @@ -326,10 +337,14 @@ def test_uses_correct_filter_labels(self): def test_raises_when_no_fragment_found(self): """Raise MCPServerNotFoundError when no IAS user fragment exists.""" - with patch("sap_cloud_sdk.agentgateway._fragments.create_fragment_client") as mock_client: + with patch( + "sap_cloud_sdk.agentgateway._fragments.create_fragment_client" + ) as mock_client: mock_client.return_value.list_instance_fragments.return_value = [] - with pytest.raises(MCPServerNotFoundError, match="No IAS user fragment found"): + with pytest.raises( + MCPServerNotFoundError, match="No IAS user fragment found" + ): get_ias_user_fragment_name("tenant-sub") @@ -409,7 +424,9 @@ async def test_reuses_cached_system_auth(self): async def test_raises_when_only_token_cache_provided(self): """Raise ValueError when token_cache given without gateway_url_cache.""" with pytest.raises(ValueError, match="both be provided or both be None"): - await fetch_system_auth("tenant-sub", token_cache=_TokenCache(ClientConfig())) + await fetch_system_auth( + "tenant-sub", token_cache=_TokenCache(ClientConfig()) + ) @pytest.mark.asyncio async def test_raises_when_only_gateway_url_cache_provided(self): @@ -434,10 +451,16 @@ async def test_fetches_user_auth_with_ias_user_fragment(self): with patch.dict(os.environ, {"APPFND_CONHOS_LANDSCAPE": "eu10"}): with ( - patch("sap_cloud_sdk.agentgateway._lob.get_ias_user_fragment_name") as mock_ias_user, - patch("sap_cloud_sdk.agentgateway._lob._fetch_auth_token") as mock_fetch, + patch( + "sap_cloud_sdk.agentgateway._lob.get_ias_user_fragment_name" + ) as mock_ias_user, + patch( + "sap_cloud_sdk.agentgateway._lob._fetch_auth_token" + ) as mock_fetch, ): - mock_ias_user.return_value = "sap-managed-runtime-agw-subscriber-ias-user-abc" + mock_ias_user.return_value = ( + "sap-managed-runtime-agw-subscriber-ias-user-abc" + ) mock_fetch.return_value = (raw_token, gateway_url) result = await fetch_user_auth("user-jwt", "tenant-sub") @@ -450,7 +473,10 @@ async def test_fetches_user_auth_with_ias_user_fragment(self): assert call_args[0][1] == "tenant-sub" options = call_args[0][2] assert options.user_token == "user-jwt" - assert options.fragment_name == "sap-managed-runtime-agw-subscriber-ias-user-abc" + assert ( + options.fragment_name + == "sap-managed-runtime-agw-subscriber-ias-user-abc" + ) assert options.fragment_level == ConsumptionLevel.INSTANCE @pytest.mark.asyncio @@ -491,13 +517,17 @@ async def test_reuses_cached_user_auth(self): async def test_raises_when_only_token_cache_provided(self): """Raise ValueError when token_cache given without gateway_url_cache.""" with pytest.raises(ValueError, match="both be provided or both be None"): - await fetch_user_auth("user-jwt", "tenant-sub", token_cache=_TokenCache(ClientConfig())) + await fetch_user_auth( + "user-jwt", "tenant-sub", token_cache=_TokenCache(ClientConfig()) + ) @pytest.mark.asyncio async def test_raises_when_only_gateway_url_cache_provided(self): """Raise ValueError when gateway_url_cache given without token_cache.""" with pytest.raises(ValueError, match="both be provided or both be None"): - await fetch_user_auth("user-jwt", "tenant-sub", gateway_url_cache=_GatewayUrlCache()) + await fetch_user_auth( + "user-jwt", "tenant-sub", gateway_url_cache=_GatewayUrlCache() + ) # ============================================================ @@ -791,6 +821,113 @@ async def test_empty_filter_lists_behave_like_none(self): assert [t.name for t in result] == ["get-sales-order"] +# ============================================================ +# Test: list_server_tools +# ============================================================ + + +class TestListServerTools: + """Tests for list_server_tools async function.""" + + def _setup_mocks( + self, mock_http, mock_stream, mock_session, init_server_name, tools + ): + mock_http.return_value.__aenter__.return_value = AsyncMock() + mock_stream.return_value.__aenter__.return_value = ( + AsyncMock(), + AsyncMock(), + None, + ) + + mock_init = MagicMock(spec=[]) + if init_server_name is not None: + mock_server_info = MagicMock(spec=["name"]) + mock_server_info.name = init_server_name + mock_init.server_info = mock_server_info + else: + mock_init.server_info = None + + mock_list = MagicMock() + mock_list.tools = tools + + mock_session_instance = AsyncMock() + mock_session_instance.initialize = AsyncMock(return_value=mock_init) + mock_session_instance.list_tools = AsyncMock(return_value=mock_list) + mock_session.return_value.__aenter__.return_value = mock_session_instance + + @pytest.mark.asyncio + async def test_returns_empty_list_and_logs_when_no_tools(self, caplog): + """Return [] and emit an info log when the server has no tools.""" + with ( + patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, + patch( + "sap_cloud_sdk.agentgateway._lob.streamable_http_client" + ) as mock_stream, + patch("sap_cloud_sdk.agentgateway._lob.ClientSession") as mock_session, + ): + self._setup_mocks(mock_http, mock_stream, mock_session, "my-server", []) + + with caplog.at_level( + logging.INFO, logger="sap_cloud_sdk.agentgateway._lob" + ): + result = await list_server_tools( + "https://example.com/mcp", "token", "my-fragment", 30.0 + ) + + assert result == [] + assert any("No tools returned" in r.message for r in caplog.records) + + @pytest.mark.asyncio + async def test_returns_tools_with_server_info_name(self): + """Use server_info.name from InitializeResult as server_name on returned tools.""" + tool_mock = MagicMock(spec=["name", "description", "input_schema"]) + tool_mock.name = "do-something" + tool_mock.description = "Does something" + tool_mock.input_schema = {"type": "object"} + + with ( + patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, + patch( + "sap_cloud_sdk.agentgateway._lob.streamable_http_client" + ) as mock_stream, + patch("sap_cloud_sdk.agentgateway._lob.ClientSession") as mock_session, + ): + self._setup_mocks( + mock_http, mock_stream, mock_session, "real-server-name", [tool_mock] + ) + + result = await list_server_tools( + "https://example.com/mcp", "token", "my-fragment", 30.0 + ) + + assert len(result) == 1 + assert result[0].name == "do-something" + assert result[0].server_name == "real-server-name" + + @pytest.mark.asyncio + async def test_falls_back_to_fragment_name_when_server_info_missing(self): + """Fall back to fragment_name when server_info or its name is absent.""" + tool_mock = MagicMock(spec=["name", "description", "input_schema"]) + tool_mock.name = "do-something" + tool_mock.description = "" + tool_mock.input_schema = {} + + with ( + patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, + patch( + "sap_cloud_sdk.agentgateway._lob.streamable_http_client" + ) as mock_stream, + patch("sap_cloud_sdk.agentgateway._lob.ClientSession") as mock_session, + ): + self._setup_mocks(mock_http, mock_stream, mock_session, None, [tool_mock]) + + result = await list_server_tools( + "https://example.com/mcp", "token", "my-fragment", 30.0 + ) + + assert result[0].server_name == "my-fragment" + + # ============================================================ # Test: call_mcp_tool_lob # ============================================================ @@ -907,21 +1044,25 @@ class TestOrdIdFromUrl: def test_extracts_ord_id_from_standard_url(self): """Return the second-to-last path segment as ord_id.""" - assert _ord_id_from_url( - "https://agw.example.com/v1/a2a/sap.s4:agent:v1/tenant-abc" - ) == "sap.s4:agent:v1" + assert ( + _ord_id_from_url( + "https://agw.example.com/v1/a2a/sap.s4:agent:v1/tenant-abc" + ) + == "sap.s4:agent:v1" + ) def test_extracts_ord_id_from_mcp_url(self): """Same extraction logic works for MCP fragment URLs.""" - assert _ord_id_from_url( - "https://agw.example.com/v1/mcp/sap.s4:apiAccess:salesOrder:v1/global-tenant-1" - ) == "sap.s4:apiAccess:salesOrder:v1" + assert ( + _ord_id_from_url( + "https://agw.example.com/v1/mcp/sap.s4:apiAccess:salesOrder:v1/global-tenant-1" + ) + == "sap.s4:apiAccess:salesOrder:v1" + ) def test_strips_trailing_slash(self): """Handle trailing slash on URL.""" - assert _ord_id_from_url( - "https://agw.example.com/v1/a2a/ord-1/gt-1/" - ) == "ord-1" + assert _ord_id_from_url("https://agw.example.com/v1/a2a/ord-1/gt-1/") == "ord-1" def test_returns_empty_for_single_segment(self): """Return empty string when URL has only one path segment.""" @@ -943,7 +1084,9 @@ def test_lists_fragments_with_a2a_label(self): with patch( "sap_cloud_sdk.agentgateway._fragments.create_fragment_client" ) as mock_client: - mock_client.return_value.list_instance_fragments.return_value = [mock_fragment] + mock_client.return_value.list_instance_fragments.return_value = [ + mock_fragment + ] result = list_a2a_fragments("tenant-sub") assert result == [mock_fragment] @@ -1027,7 +1170,9 @@ async def test_raises_on_non_200_status(self): mock_http.return_value.__aenter__.return_value = mock_http_instance with pytest.raises(AgentGatewaySDKError, match="404"): - await _fetch_agent_card("https://agw.example.com/base", "auth-token", 60.0) + await _fetch_agent_card( + "https://agw.example.com/base", "auth-token", 60.0 + ) @pytest.mark.asyncio async def test_raises_on_request_error(self): @@ -1042,7 +1187,9 @@ async def test_raises_on_request_error(self): mock_http.return_value.__aenter__.return_value = mock_http_instance with pytest.raises(AgentGatewaySDKError, match="Agent card request failed"): - await _fetch_agent_card("https://agw.example.com/base", "auth-token", 60.0) + await _fetch_agent_card( + "https://agw.example.com/base", "auth-token", 60.0 + ) # ============================================================ @@ -1078,9 +1225,7 @@ async def test_returns_agents_for_all_fragments(self): return_value=AgentCard(raw=card_payload), ), ): - result = await get_agent_cards_lob( - "tenant-sub", "system-token", 60.0 - ) + result = await get_agent_cards_lob("tenant-sub", "system-token", 60.0) assert len(result) == 1 assert isinstance(result[0], Agent) @@ -1101,8 +1246,12 @@ async def test_returns_empty_list_when_no_fragments(self): @pytest.mark.asyncio async def test_filters_by_agent_names(self): """Fetch all cards then keep only those whose agent card name matches.""" - frag_1 = self._make_fragment("frag-1", "https://agw.example.com/v1/a2a/ord-1/t1") - frag_2 = self._make_fragment("frag-2", "https://agw.example.com/v1/a2a/ord-2/t2") + frag_1 = self._make_fragment( + "frag-1", "https://agw.example.com/v1/a2a/ord-1/t1" + ) + frag_2 = self._make_fragment( + "frag-2", "https://agw.example.com/v1/a2a/ord-2/t2" + ) async def _cards_by_ord(fragment_url, token, timeout): if "ord-1" in fragment_url: @@ -1133,8 +1282,12 @@ async def _cards_by_ord(fragment_url, token, timeout): @pytest.mark.asyncio async def test_filters_by_ord_ids(self): """Only include fragments whose ordId (from URL) is in the ord_ids filter.""" - frag_1 = self._make_fragment("frag-1", "https://agw.example.com/v1/a2a/ord-1/t1") - frag_2 = self._make_fragment("frag-2", "https://agw.example.com/v1/a2a/ord-2/t2") + frag_1 = self._make_fragment( + "frag-1", "https://agw.example.com/v1/a2a/ord-1/t1" + ) + frag_2 = self._make_fragment( + "frag-2", "https://agw.example.com/v1/a2a/ord-2/t2" + ) with ( patch( @@ -1229,8 +1382,14 @@ def test_returns_client_id_from_destination_properties(self): mock_dest_client.get_destination.return_value = mock_dest with ( - patch("sap_cloud_sdk.agentgateway._lob._ias_dest_name", return_value="sap-managed-runtime-ias-eu10"), - patch("sap_cloud_sdk.agentgateway._lob.create_destination_client", return_value=mock_dest_client), + patch( + "sap_cloud_sdk.agentgateway._lob._ias_dest_name", + return_value="sap-managed-runtime-ias-eu10", + ), + patch( + "sap_cloud_sdk.agentgateway._lob.create_destination_client", + return_value=mock_dest_client, + ), ): result = get_ias_client_id_lob() @@ -1246,10 +1405,18 @@ def test_raises_when_destination_not_found(self): mock_dest_client.get_destination.return_value = None with ( - patch("sap_cloud_sdk.agentgateway._lob._ias_dest_name", return_value="sap-managed-runtime-ias-eu10"), - patch("sap_cloud_sdk.agentgateway._lob.create_destination_client", return_value=mock_dest_client), + patch( + "sap_cloud_sdk.agentgateway._lob._ias_dest_name", + return_value="sap-managed-runtime-ias-eu10", + ), + patch( + "sap_cloud_sdk.agentgateway._lob.create_destination_client", + return_value=mock_dest_client, + ), ): - with pytest.raises(AgentGatewaySDKError, match="sap-managed-runtime-ias-eu10"): + with pytest.raises( + AgentGatewaySDKError, match="sap-managed-runtime-ias-eu10" + ): get_ias_client_id_lob() def test_raises_when_client_id_property_absent(self): @@ -1259,13 +1426,22 @@ def test_raises_when_client_id_property_absent(self): mock_dest_client.get_destination.return_value = mock_dest with ( - patch("sap_cloud_sdk.agentgateway._lob._ias_dest_name", return_value="sap-managed-runtime-ias-eu10"), - patch("sap_cloud_sdk.agentgateway._lob.create_destination_client", return_value=mock_dest_client), + patch( + "sap_cloud_sdk.agentgateway._lob._ias_dest_name", + return_value="sap-managed-runtime-ias-eu10", + ), + patch( + "sap_cloud_sdk.agentgateway._lob.create_destination_client", + return_value=mock_dest_client, + ), ): with pytest.raises(AgentGatewaySDKError, match="clientId"): get_ias_client_id_lob() def test_raises_when_landscape_env_not_set(self): - with patch("sap_cloud_sdk.agentgateway._lob._ias_dest_name", side_effect=EnvironmentError("APPFND_CONHOS_LANDSCAPE not set")): + with patch( + "sap_cloud_sdk.agentgateway._lob._ias_dest_name", + side_effect=EnvironmentError("APPFND_CONHOS_LANDSCAPE not set"), + ): with pytest.raises(EnvironmentError, match="APPFND_CONHOS_LANDSCAPE"): get_ias_client_id_lob()