fix(agentgateway): guard mcp list_tools null responses by tiagoek · Pull Request #226 · SAP/cloud-sdk-python

tiagoek · 2026-07-15T08:31:05Z

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Hardens Agent Gateway MCP tool discovery for LoB and customer flows when OpenTelemetry MCP instrumentation or auth mismatches produce empty or null tool lists.

Changes:

Defensive list_tools handling (LoB + customer) — In list_server_tools() / _list_server_tools(), if session.list_tools() returns None or tools is None (often when opentelemetry-instrumentation-mcp swallows errors via @dont_throw), the SDK logs a warning and returns an empty list for that fragment so discovery continues with other fragments. Avoids AttributeError: 'NoneType' object has no attribute 'tools'.
LoB HTTP 403 diagnostic hint — When system-token tool listing fails with HTTP 403 in the LoB flow, log guidance to use user-scoped discovery (user_token on list_mcp_tools()) where the landscape requires it.
Version bump — 0.35.2.
Documentation — Troubleshooting notes in the Agent Gateway and telemetry user guides (null tool lists, dual-path user_token anti-pattern, optional agent-side MCP OTel unwrap until upstream fixes; SDK does not strip instrumentation in auto_instrument()).

Out of scope / not fixed by this PR:

Supplier Invoice Insights empty tools from missing user_token on some call sites — agent-side ContextVar / dual-path auth fixes, not SDK behavior changes.
OpenTelemetry MCP unwrap in auto_instrument() — removed after review; agents may keep documented main.py unwrap workarounds without disabling telemetry globally.
Integration test failures related to agent_memory OAuth in CI — unrelated to this change set.

Jira: N/A

Related Issue

N/A (SDK hardening from internal community-support triage; no public GitHub issue in this repo)

Type of Change

Please check the relevant option:

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Code refactoring
Dependency update

How to Test

Run LoB unit tests: uv run pytest tests/agentgateway/unit/test_lob.py -v
Run telemetry unit tests: uv run pytest tests/core/unit/telemetry/test_auto_instrument.py -v
Expected: all tests pass; list_tools() returning None yields [] with a warning; HTTP 403 on system-token listing logs a user_token hint.

Checklist

Before submitting your PR, please review and check the following:

I have read the Contributing Guidelines
I have verified that my changes solve the issue
I have added/updated automated tests to cover my changes
All tests pass locally
I have verified that my code follows the Code Guidelines
I have updated documentation (if applicable)
I have added type hints for all public APIs
My code does not contain sensitive information (credentials, tokens, etc.)
I have followed Conventional Commits for commit messages

Breaking Changes

None.

Additional Notes

LoB auth semantics are unchanged: agents must still pass user_token on every list_mcp_tools() call that needs user-scoped tools (see user guide Troubleshooting).
Dunning-class OTel null list_tools symptoms are addressed by the None guard; optional agent-side unwrap remains documented for cases where instrumentation still returns null payloads.

When MCP instrumentation returns None from list_tools(), skip the fragment with a clear warning instead of raising AttributeError on result.tools.

Required for src/ change CI version check; ruff format on new tests.

…iew nits

Unwrap opentelemetry-instrumentation-mcp at the end of auto_instrument() so list_tools is not silently broken by @dont_throw wrappers. Log a user_token hint when LoB system-token tool listing returns HTTP 403. Bump version to 0.35.2.

…to_instrument unwrap

tiagoek · 2026-07-15T09:18:27Z

SDK Module Review

Check	Status	Findings
bdd	✅ PASS	0
binding-shape	✅ PASS	0
commits	✅ PASS	0
concurrency	✅ PASS	0
constants	✅ PASS	0
deletion-hygiene	✅ PASS	0
deps-supply	✅ PASS	0
disclosure	✅ PASS	0
docs	✅ PASS	0
errors-logging	⚠️ FLAG	1
hardcode	✅ PASS	0
http-hygiene	✅ PASS	0
license-spdx	✅ PASS	0
patterns	✅ PASS	0
pr-size	✅ PASS	0
quality-gate-parity	✅ PASS	0
secrets	✅ PASS	0
telemetry	✅ PASS	0
testing-depth	✅ PASS	0
versioning	✅ PASS	0

Findings (1)

[FLAG] PY-EL-02 — except block does not end with raise — swallowing? Add re-raise or comment justifying suppression (inline on src/sap_cloud_sdk/agentgateway/_lob.py:401)

1 finding(s): 1 posted as inline comment(s) on the affected lines, 0 not tied to a code line (listed above).

Generated by sdk-review-skill · v1

Remove reflection-based OpenTelemetry MCP unwrap from SDK init per review. Keep Agent Gateway None guards, HTTP 403 hints, and document agent-side unwrap as an optional workaround until upstream instrumentation fixes land.

tiagoek · 2026-07-15T11:33:58Z

            )
+        # intentional: fragment failure must not abort remaining fragments
+        # (HTTPStatusError: 403 → warning + continue; other status → exception log + skip)
+        except httpx.HTTPStatusError as exc:


[FLAG] PY-EL-02

except block does not end with raise — swallowing? Add re-raise or comment justifying suppression

Document intentional HTTPStatusError swallow when skipping a failed MCP fragment.

NicoleMGomes · 2026-07-15T12:15:58Z

                result = await session.list_tools()

+                if result is None or result.tools is None:
+                    logger.warning(


Why review OTel instrumentation?

NicoleMGomes · 2026-07-15T12:18:49Z

+            if exc.response.status_code == 403:
+                logger.warning(
+                    "HTTP 403 listing tools from fragment '%s' with system token — "
+                    "MCP list_tools may require a user-scoped token; use Phase 2 flow "


What is phase 2 flow?

NicoleMGomes · 2026-07-15T12:19:07Z

            )
+        # intentional: fragment failure must not abort remaining fragments
+        # (HTTPStatusError: 403 → warning + continue; other status → exception log + skip)
+        except httpx.HTTPStatusError as exc:  # sdk-review: ignore[el-02]


Comments of sdk-review in the code is not nice

NicoleMGomes · 2026-07-15T12:20:02Z

+            )
+        # intentional: fragment failure must not abort remaining fragments
+        # (unexpected errors → exception log + skip)
        except Exception:


This should still be executed if all fragments fail to load. And who requested this behaviour?

NicoleMGomes · 2026-07-15T12:21:42Z

 from litellm import completion
 # LLM calls are now automatically traced
 ```
+If OpenTelemetry MCP instrumentation (`opentelemetry-instrumentation-mcp`) swallows protocol errors and `list_tools()` returns `None`, the Agent Gateway SDK logs a warning and skips that fragment instead of crashing (SDK 0.35.2+). Removing instrumentation is not recommended. Until upstream fixes land, agents may keep an application-side unwrap after `auto_instrument()` (see Agent Gateway user guide — OpenTelemetry and MCP).


list_tools doesn't exist, it's list_mcp_tools

Replace internal Phase 2 wording with user_token/list_mcp_tools hints, remove sdk-review inline suppression, warn when all fragments fail, soften customer None-guard log, and align user guides with public API naming. Bump version to 0.35.3.

fix(agentgateway): guard list_tools None response in LoB flow

83c5dae

When MCP instrumentation returns None from list_tools(), skip the fragment with a clear warning instead of raising AttributeError on result.tools.

tiagoek force-pushed the fix/agentgateway-list-tools-none-guard branch 2 times, most recently from 8c36868 to 83c5dae Compare July 15, 2026 08:33

tiagoek added 2 commits July 15, 2026 10:35

chore: bump version to 0.35.1 and format test_lob.py

6d63e65

Required for src/ change CI version check; ruff format on new tests.

fix(agentgateway): extend None guard to customer flow and address rev…

9977ebf

…iew nits

tiagoek marked this pull request as ready for review July 15, 2026 08:58

tiagoek requested a review from a team as a code owner July 15, 2026 08:58

tiagoek changed the title ~~fix(agentgateway): guard list_tools None response in LoB flow~~ fix(agentgateway,telemetry): MCP list_tools hardening and OTel unwrap Jul 15, 2026

docs(agentgateway,telemetry): document MCP LoB troubleshooting and au…

0fc99d3

…to_instrument unwrap

tiagoek changed the title ~~fix(agentgateway,telemetry): MCP list_tools hardening and OTel unwrap~~ fix(agentgateway,telemetry): MCP list_tools guards, OTel unwrap, and docs Jul 15, 2026

tiagoek added the sdk-review: ⚠️ flagged label Jul 15, 2026

tiagoek changed the title ~~fix(agentgateway,telemetry): MCP list_tools guards, OTel unwrap, and docs~~ fix(agentgateway,telemetry): guard mcp list_tools and otel unwrap Jul 15, 2026

tiagoek added 5 commits July 15, 2026 11:20

fix(agentgateway,telemetry): fix user-guide eof and 403 log noise

6b398fc

fix(agentgateway): justify fragment skip exception handling (py-el-02)

aa08f9c

fix(agentgateway): document intentional fragment skip (py-el-02)

bd58a2c

fix(agentgateway): comment non-403 fragment skip path (py-el-02)

28c63d8

fix(agentgateway): document httpx fragment skip (py-el-02)

4eb8965

LucasAlvesSoares requested changes Jul 15, 2026

View reviewed changes

Comment thread src/sap_cloud_sdk/core/telemetry/auto_instrument.py Outdated

fix(telemetry): drop MCP unwrap from auto_instrument

a473774

Remove reflection-based OpenTelemetry MCP unwrap from SDK init per review. Keep Agent Gateway None guards, HTTP 403 hints, and document agent-side unwrap as an optional workaround until upstream instrumentation fixes land.

tiagoek changed the title ~~fix(agentgateway,telemetry): guard mcp list_tools and otel unwrap~~ fix(agentgateway): guard mcp list_tools null responses Jul 15, 2026

chore: ruff format auto_instrument after unwrap removal

9f837a9

tiagoek commented Jul 15, 2026

View reviewed changes

Add PY-EL-02 sdk-review suppression for fragment tool-list handler.

b71d7e2

Document intentional HTTPStatusError swallow when skipping a failed MCP fragment.

NicoleMGomes reviewed Jul 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(agentgateway): guard mcp list_tools null responses#226

fix(agentgateway): guard mcp list_tools null responses#226
tiagoek wants to merge 14 commits into
mainfrom
fix/agentgateway-list-tools-none-guard

tiagoek commented Jul 15, 2026 •

edited

Loading

Uh oh!

tiagoek commented Jul 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

tiagoek Jul 15, 2026

Uh oh!

NicoleMGomes Jul 15, 2026

Uh oh!

NicoleMGomes Jul 15, 2026

Uh oh!

NicoleMGomes Jul 15, 2026

Uh oh!

NicoleMGomes Jul 15, 2026

Uh oh!

NicoleMGomes Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tiagoek commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Type of Change

How to Test

Checklist

Breaking Changes

Additional Notes

Uh oh!

tiagoek commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SDK Module Review

Findings (1)

Uh oh!

Uh oh!

tiagoek Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

NicoleMGomes Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

NicoleMGomes Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

NicoleMGomes Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

NicoleMGomes Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

NicoleMGomes Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tiagoek commented Jul 15, 2026 •

edited

Loading

tiagoek commented Jul 15, 2026 •

edited

Loading