review comments to address

Signed-off-by: Prashant <prashant.rakheja@sap.com>

Author: prashantrakheja

src/sap_cloud_sdk/agentgateway/_customer.py

@@ -35,9 +35,6 @@
 # Default credential path for Kyma production deployments
 _CREDENTIALS_DEFAULT_PATH = "/etc/ums/credentials/credentials"
 
-# HTTP timeout for token requests and MCP server calls (seconds)
-_HTTP_TIMEOUT = 60.0
-
 # Resource URN for Agent Gateway token scope (hardcoded - production value)
 _AGW_RESOURCE_URN = "urn:sap:identity:application:provider:name:agent-gateway"
 
@@ -213,6 +210,7 @@ def _create_ssl_context(certificate: str, private_key: str) -> ssl.SSLContext:
 def _request_token_mtls(
     credentials: CustomerCredentials,
     grant_type: str,
+    timeout: float,
     app_tid: str | None = None,
     extra_data: dict | None = None,
 ) -> str:
@@ -255,7 +253,7 @@ def _request_token_mtls(
     try:
         with httpx.Client(
             verify=ssl_context,
-            timeout=_HTTP_TIMEOUT,
+            timeout=timeout,
         ) as client:
             response = client.post(
                 credentials.token_service_url,
@@ -293,6 +291,7 @@ def _request_token_mtls(
 
 def get_system_token_mtls(
     credentials: CustomerCredentials,
+    timeout: float,
     app_tid: str | None = None,
 ) -> str:
     """Get system-scoped token using mTLS client credentials flow.
@@ -301,6 +300,7 @@ def get_system_token_mtls(
 
     Args:
         credentials: Customer credentials.
+        timeout: HTTP timeout in seconds.
         app_tid: BTP Application Tenant ID of subscriber (optional).
 
     Returns:
@@ -310,6 +310,7 @@ def get_system_token_mtls(
     return _request_token_mtls(
         credentials,
         grant_type=_GRANT_TYPE_CLIENT_CREDENTIALS,
+        timeout=timeout,
         app_tid=app_tid,
         extra_data={"response_type": "token"},
     )
@@ -318,6 +319,7 @@ def get_system_token_mtls(
 def exchange_user_token(
     credentials: CustomerCredentials,
     user_token: str,
+    timeout: float,
     app_tid: str | None = None,
 ) -> str:
     """Exchange user token for AGW-scoped token using jwt-bearer grant.
@@ -328,6 +330,7 @@ def exchange_user_token(
     Args:
         credentials: Customer credentials.
         user_token: User's JWT token to exchange.
+        timeout: HTTP timeout in seconds.
         app_tid: BTP Application Tenant ID of subscriber (optional).
 
     Returns:
@@ -337,6 +340,7 @@ def exchange_user_token(
     return _request_token_mtls(
         credentials,
         grant_type=_GRANT_TYPE_JWT_BEARER,
+        timeout=timeout,
         app_tid=app_tid,
         extra_data={
             "assertion": user_token,
@@ -371,7 +375,7 @@ async def _list_server_tools(
     url: str,
     auth_token: str,
     dependency: IntegrationDependency,
-    timeout: float = _HTTP_TIMEOUT,
+    timeout: float,
 ) -> list[MCPTool]:
     """List tools from a single MCP server.
 
@@ -428,8 +432,8 @@ async def _list_server_tools(
 
 async def get_mcp_tools_customer(
     credentials: CustomerCredentials,
+    timeout: float,
     app_tid: str | None = None,
-    timeout: float = _HTTP_TIMEOUT,
 ) -> list[MCPTool]:
     """List all MCP tools from servers defined in credentials.
 
@@ -458,7 +462,7 @@ async def get_mcp_tools_customer(
     # Get system token for discovery
     loop = asyncio.get_running_loop()
     system_token = await loop.run_in_executor(
-        None, get_system_token_mtls, credentials, app_tid
+        None, get_system_token_mtls, credentials, timeout, app_tid
     )
 
     tools: list[MCPTool] = []
@@ -489,8 +493,8 @@ async def call_mcp_tool_customer(
     credentials: CustomerCredentials,
     tool: MCPTool,
     user_token: str | None,
+    timeout: float,
     app_tid: str | None = None,
-    timeout: float = _HTTP_TIMEOUT,
     **kwargs,
 ) -> str:
     """Invoke an MCP tool using customer flow.
@@ -516,7 +520,7 @@ async def call_mcp_tool_customer(
     if user_token:
         # Exchange user token for AGW-scoped token (with principal propagation)
         agw_token = await loop.run_in_executor(
-            None, exchange_user_token, credentials, user_token, app_tid
+            None, exchange_user_token, credentials, user_token, timeout, app_tid
         )
     else:
         # TODO: IBD workaround - use system token when user_token is not available.
@@ -527,7 +531,7 @@ async def call_mcp_tool_customer(
             "Principal propagation will NOT work."
         )
         agw_token = await loop.run_in_executor(
-            None, get_system_token_mtls, credentials, app_tid
+            None, get_system_token_mtls, credentials, timeout, app_tid
         )
 
     async with httpx.AsyncClient(

src/sap_cloud_sdk/agentgateway/_lob.py

@@ -36,9 +36,6 @@
 
 _DESTINATION_INSTANCE = "default"
 
-# HTTP timeout for MCP server requests (seconds)
-_HTTP_TIMEOUT = 60.0
-
 
 def _ias_dest_name() -> str:
     """Get IAS destination name based on landscape.
@@ -227,7 +224,7 @@ def _fetch_user_auth_sync():
 
 
 async def list_server_tools(
-    dest_url: str, system_auth: str, fragment_name: str, timeout: float = _HTTP_TIMEOUT
+    dest_url: str, system_auth: str, fragment_name: str, timeout: float
 ) -> list[MCPTool]:
     """List tools from a single MCP server.
 
@@ -273,7 +270,7 @@ async def list_server_tools(
 
 async def get_mcp_tools_lob(
     tenant_subdomain: str,
-    timeout: float = _HTTP_TIMEOUT,
+    timeout: float,
 ) -> list[MCPTool]:
     """List all MCP tools using LoB flow (destination-based).
 
@@ -333,7 +330,7 @@ async def call_mcp_tool_lob(
     tool: MCPTool,
     user_token: str,
     tenant_subdomain: str,
-    timeout: float = _HTTP_TIMEOUT,
+    timeout: float,
     **kwargs,
 ) -> str:
     """Invoke an MCP tool using LoB flow (destination-based).

src/sap_cloud_sdk/agentgateway/agw_client.py

@@ -158,7 +158,7 @@ async def list_mcp_tools(
                 )
                 credentials = load_customer_credentials(credentials_path)
                 return await get_mcp_tools_customer(
-                    credentials, app_tid, self._config.timeout
+                    credentials, self._config.timeout, app_tid
                 )
 
             # LoB flow - requires tenant_subdomain
@@ -250,8 +250,8 @@ async def call_mcp_tool(
                     credentials,
                     tool,
                     resolved_user_token,
-                    app_tid,
                     self._config.timeout,
+                    app_tid,
                     **kwargs,
                 )
 

tests/agentgateway/unit/test_customer.py

@@ -305,7 +305,7 @@ def test_requests_client_credentials_token(self, credentials):
             mock_client.post.return_value = mock_response
             mock_client_class.return_value = mock_client
 
-            result = get_system_token_mtls(credentials)
+            result = get_system_token_mtls(credentials, timeout=60.0)
 
             assert result == "system-token-123"
             mock_client.post.assert_called_once()
@@ -332,7 +332,7 @@ def test_raises_on_failed_request(self, credentials):
             mock_client_class.return_value = mock_client
 
             with pytest.raises(AgentGatewaySDKError, match="Token request failed"):
-                get_system_token_mtls(credentials)
+                get_system_token_mtls(credentials, timeout=60.0)
 
 
 # ============================================================
@@ -374,7 +374,7 @@ def test_exchanges_user_token_with_jwt_bearer(self, credentials):
             mock_client.post.return_value = mock_response
             mock_client_class.return_value = mock_client
 
-            result = exchange_user_token(credentials, "user-jwt-token")
+            result = exchange_user_token(credentials, "user-jwt-token", timeout=60.0)
 
             assert result == "exchanged-token-123"
             call_args = mock_client.post.call_args
@@ -402,7 +402,9 @@ def test_passes_app_tid_when_provided(self, credentials):
             mock_client.post.return_value = mock_response
             mock_client_class.return_value = mock_client
 
-            result = exchange_user_token(credentials, "user-jwt", app_tid="test-tid")
+            result = exchange_user_token(
+                credentials, "user-jwt", timeout=60.0, app_tid="test-tid"
+            )
 
             assert result == "token-with-tid"
             call_args = mock_client.post.call_args
@@ -449,7 +451,7 @@ async def test_raises_when_empty_dependencies(self):
         with pytest.raises(
             AgentGatewaySDKError, match="integrationDependencies is empty"
         ):
-            await get_mcp_tools_customer(credentials)
+            await get_mcp_tools_customer(credentials, timeout=60.0)
 
     @pytest.mark.asyncio
     async def test_discovers_tools_from_credentials(self, credentials):
@@ -475,7 +477,7 @@ async def test_discovers_tools_from_credentials(self, credentials):
                 return_value=mock_tools,
             ) as mock_list,
         ):
-            result = await get_mcp_tools_customer(credentials)
+            result = await get_mcp_tools_customer(credentials, timeout=60.0)
 
             assert len(result) == 1
             assert result[0].name == "list_cost_centers"
@@ -523,7 +525,7 @@ async def mock_list_tools(*args, **kwargs):
                 side_effect=mock_list_tools,
             ),
         ):
-            result = await get_mcp_tools_customer(credentials)
+            result = await get_mcp_tools_customer(credentials, timeout=60.0)
 
             # Should still return tools from server2
             assert len(result) == 1
@@ -613,11 +615,11 @@ async def test_exchanges_user_token_before_call(self, credentials, mock_tool):
             mock_session_class.return_value = mock_session_ctx
 
             result = await call_mcp_tool_customer(
-                credentials, mock_tool, "user-jwt", order_id="12345"
+                credentials, mock_tool, "user-jwt", 60.0, order_id="12345"
             )
 
             assert result == "Order created successfully"
-            mock_exchange.assert_called_once_with(credentials, "user-jwt", None)
+            mock_exchange.assert_called_once_with(credentials, "user-jwt", 60.0, None)
 
     @pytest.mark.asyncio
     async def test_uses_system_token_when_user_token_not_provided(
@@ -669,10 +671,10 @@ async def test_uses_system_token_when_user_token_not_provided(
 
             # Call without user_token (None)
             result = await call_mcp_tool_customer(
-                credentials, mock_tool, None, order_id="12345"
+                credentials, mock_tool, None, 60.0, order_id="12345"
             )
 
             assert result == "Result with system token"
             # Should use system token, not exchange
-            mock_system_token.assert_called_once_with(credentials, None)
+            mock_system_token.assert_called_once_with(credentials, 60.0, None)
             mock_exchange.assert_not_called()

tests/agentgateway/unit/test_lob.py

@@ -323,7 +323,7 @@ async def test_returns_empty_when_no_fragments(self):
         with patch("sap_cloud_sdk.agentgateway._lob.list_mcp_fragments") as mock_list:
             mock_list.return_value = []
 
-            result = await get_mcp_tools_lob("tenant-sub")
+            result = await get_mcp_tools_lob("tenant-sub", 60.0)
 
             assert result == []
 
@@ -337,7 +337,7 @@ async def test_skips_fragments_without_url(self):
         with patch("sap_cloud_sdk.agentgateway._lob.list_mcp_fragments") as mock_list:
             mock_list.return_value = [fragment]
 
-            result = await get_mcp_tools_lob("tenant-sub")
+            result = await get_mcp_tools_lob("tenant-sub", 60.0)
 
             assert result == []
 
@@ -372,7 +372,7 @@ async def test_uses_fragment_name_directly(self):
             mock_auth.return_value = "Bearer token"
             mock_tools.return_value = [mock_tool]
 
-            await get_mcp_tools_lob("tenant-sub")
+            await get_mcp_tools_lob("tenant-sub", 60.0)
 
             # Verify get_system_auth called with just tenant_subdomain
             mock_auth.assert_called_once_with("tenant-sub")
@@ -418,7 +418,7 @@ async def test_handles_exception_for_single_fragment(self):
             mock_auth.side_effect = [Exception("Auth failed"), "Bearer token"]
             mock_tools.return_value = [mock_tool]
 
-            result = await get_mcp_tools_lob("tenant-sub")
+            result = await get_mcp_tools_lob("tenant-sub", 60.0)
 
             # Should still get tools from second fragment
             assert len(result) == 1
@@ -477,7 +477,7 @@ async def test_calls_tool_with_user_auth(self):
             mock_session.return_value.__aenter__.return_value = mock_session_instance
 
             result = await call_mcp_tool_lob(
-                tool, "user-jwt", "tenant-sub", param1="value1"
+                tool, "user-jwt", "tenant-sub", 60.0, param1="value1"
             )
 
             assert result == "Tool result"
@@ -527,6 +527,6 @@ async def test_returns_empty_string_when_no_content(self):
             mock_session_instance.call_tool = AsyncMock(return_value=mock_result)
             mock_session.return_value.__aenter__.return_value = mock_session_instance
 
-            result = await call_mcp_tool_lob(tool, "user-jwt", "tenant-sub")
+            result = await call_mcp_tool_lob(tool, "user-jwt", "tenant-sub", 60.0)
 
             assert result == ""