Skip to content

fix(agentgateway): address sdk-review quality findings from PR #220 #230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tiagoek wants to merge 2 commits into
base: feat/transparent-tls-smahr
Choose a base branch
from
+27 −17
Open

fix(agentgateway): address sdk-review quality findings from PR #220 #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d28d766
fix(agentgateway): address sdk-review findings (CON-01, EL-01, EL-02,…
tiagoek Jul 15, 2026
87089cb
fix(agentgateway): replace remaining client_id literals with constant
tiagoek Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions src/sap_cloud_sdk/agentgateway/_customer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@
_GRANT_TYPE_CLIENT_CREDENTIALS = "client_credentials"
_GRANT_TYPE_JWT_BEARER = "urn:ietf:params:oauth:grant-type:jwt-bearer"

# Token response field keys
_TOKEN_FIELD_CLIENT_ID = "client_id"
_TOKEN_FIELD_ACCESS_TOKEN = "access_token"


def _cache_scope_key(credentials: CustomerCredentials) -> str:
"""Build a cache scope key for customer-flow tokens."""
Expand Down Expand Up @@ -147,7 +151,7 @@ def load_customer_credentials_from_env(

# Check required environment variables
required_vars = {
_INTEGRATION_CLIENT_ID_ENV: "client_id",
_INTEGRATION_CLIENT_ID_ENV: _TOKEN_FIELD_CLIENT_ID,
_INTEGRATION_AUTH_URL_ENV: "auth_url",
_INTEGRATION_GATEWAY_URL_ENV: "gateway_url",
}
Expand Down Expand Up @@ -200,7 +204,7 @@ def load_customer_credentials(path: str) -> CustomerCredentials:
# Credential file uses camelCase, we use snake_case
required_fields = {
_CredentialFields.TOKEN_SERVICE_URL: "token_service_url",
_CredentialFields.CLIENT_ID: "client_id",
_CredentialFields.CLIENT_ID: _TOKEN_FIELD_CLIENT_ID,
_CredentialFields.CERTIFICATE: "certificate",
_CredentialFields.PRIVATE_KEY: "private_key",
_CredentialFields.GATEWAY_URL: "gateway_url",
Expand Down Expand Up @@ -324,7 +328,7 @@ def _request_token_mtls(
ssl_context = _create_ssl_context(credentials.certificate, credentials.private_key)

data = {
"client_id": credentials.client_id,
_TOKEN_FIELD_CLIENT_ID: credentials.client_id,
"grant_type": grant_type,
"resource": _AGW_RESOURCE_URN,
}
Expand Down Expand Up @@ -363,7 +367,7 @@ def _request_token_mtls(
)

token_data = response.json()
access_token = token_data.get("access_token")
access_token = token_data.get(_TOKEN_FIELD_ACCESS_TOKEN)

if not access_token:
raise AgentGatewaySDKError(
Expand All @@ -374,7 +378,7 @@ def _request_token_mtls(
return token_data

except httpx.RequestError as e:
raise AgentGatewaySDKError(f"Token request failed: {e}")
raise AgentGatewaySDKError(f"Token request failed: {e}") from e


def _request_token_transparent(
Expand All @@ -401,7 +405,7 @@ def _request_token_transparent(
AgentGatewaySDKError: If token request fails.
"""
data = {
"client_id": credentials.client_id,
_TOKEN_FIELD_CLIENT_ID: credentials.client_id,
"grant_type": grant_type,
"resource": _AGW_RESOURCE_URN,
}
Expand Down Expand Up @@ -437,7 +441,7 @@ def _request_token_transparent(
)

token_data = response.json()
access_token = token_data.get("access_token")
access_token = token_data.get(_TOKEN_FIELD_ACCESS_TOKEN)

if not access_token:
raise AgentGatewaySDKError(
Expand All @@ -448,7 +452,7 @@ def _request_token_transparent(
return token_data

except httpx.RequestError as e:
raise AgentGatewaySDKError(f"Token request failed: {e}")
raise AgentGatewaySDKError(f"Token request failed: {e}") from e


def get_system_token_mtls(
Expand Down Expand Up @@ -499,7 +503,7 @@ def get_system_token_mtls(
extra_data={"response_type": "token"},
)

access_token = token_data["access_token"]
access_token = token_data[_TOKEN_FIELD_ACCESS_TOKEN]

if token_cache:
token_cache.set_system_token(
Expand Down Expand Up @@ -571,7 +575,7 @@ def exchange_user_token(
},
)

access_token = token_data["access_token"]
access_token = token_data[_TOKEN_FIELD_ACCESS_TOKEN]

if token_cache:
token_cache.set_user_token(
Expand Down
10 changes: 6 additions & 4 deletions src/sap_cloud_sdk/agentgateway/agw_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@

logger = logging.getLogger(__name__)

_LOG_TRANSPARENT_MODE = "Transparent mode credentials detected"


class AgentGatewayClient:
"""Client for discovering and invoking MCP tools via SAP Agent Gateway.
Expand Down Expand Up @@ -197,7 +199,7 @@ async def get_system_auth(self) -> AuthResult:

# Check for transparent mode
if detect_transparent_credentials():
logger.info("Transparent mode credentials detected")
logger.info(_LOG_TRANSPARENT_MODE)
credentials = load_customer_credentials_from_env()
loop = asyncio.get_running_loop()
token = await loop.run_in_executor(
Expand Down Expand Up @@ -283,7 +285,7 @@ async def get_user_auth(

# Check for transparent mode
if detect_transparent_credentials():
logger.info("Transparent mode credentials detected")
logger.info(_LOG_TRANSPARENT_MODE)
credentials = load_customer_credentials_from_env()
loop = asyncio.get_running_loop()
token = await loop.run_in_executor(
Expand Down Expand Up @@ -406,7 +408,7 @@ async def list_mcp_tools(

# Check for transparent mode
if detect_transparent_credentials():
logger.info("Transparent mode credentials detected")
logger.info(_LOG_TRANSPARENT_MODE)
credentials = load_customer_credentials_from_env()
return await get_mcp_tools_customer(
credentials, auth.access_token, self._config.timeout
Expand Down Expand Up @@ -560,7 +562,7 @@ async def call_mcp_tool(

# Check for transparent mode
if detect_transparent_credentials():
logger.debug("Transparent mode credentials detected")
logger.debug(_LOG_TRANSPARENT_MODE)

return await call_mcp_tool_customer(
tool, auth.access_token, self._config.timeout, **kwargs
Expand Down
10 changes: 7 additions & 3 deletions src/sap_cloud_sdk/core/_telemetry_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,22 @@
When telemetry packages are not installed, it provides no-op implementations.
"""

from typing import Any, Callable, TypeVar

_F = TypeVar("_F", bound=Callable[..., Any])

try:
from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics
TELEMETRY_AVAILABLE = True
except ImportError:
# Telemetry packages not installed — provide no-op implementations
TELEMETRY_AVAILABLE = False
# Provide no-op implementations when telemetry is not available
Module = None # type: ignore
Operation = None # type: ignore

def record_metrics(*args, **kwargs): # type: ignore
def record_metrics(*args: Any, **kwargs: Any) -> Any: # type: ignore
"""No-op decorator when telemetry is not available."""
def decorator(func):
def decorator(func: _F) -> _F:
return func
if args and callable(args[0]):
# Called without parentheses: @record_metrics
Expand Down