Skip to content

Commit c669cff

Browse files
committed
fix: propagate tool validation errors in gateway refresh response (#5290)
The _refresh_gateway_tools_resources_prompts method was discarding the 5th return value from _initialize_gateway() (validation_errors), causing GatewayRefreshResponse.validation_errors to always be empty. - Capture validation_errors instead of discarding with _ - Populate result['validation_errors'] after successful init - Add tests covering manual refresh, auto-refresh, and API response Closes #5290 Signed-off-by: Mohan Lakshmaiah <mohan.economist@gmail.com>
1 parent 8e9c049 commit c669cff

4 files changed

Lines changed: 69 additions & 1 deletion

File tree

mcpgateway/services/gateway_service.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5796,7 +5796,7 @@ async def _refresh_gateway_tools_resources_prompts(
57965796
_refresh_key = _enc.decrypt_secret_or_plaintext(_refresh_key)
57975797
except Exception:
57985798
logger.debug("client_key decryption skipped during gateway refresh")
5799-
_capabilities, tools, resources, prompts, _ = await self._initialize_gateway(
5799+
_capabilities, tools, resources, prompts, validation_errors = await self._initialize_gateway(
58005800
url=gateway_url,
58015801
authentication=gateway_auth_value,
58025802
transport=gateway_transport,
@@ -5816,6 +5816,8 @@ async def _refresh_gateway_tools_resources_prompts(
58165816
result["error"] = str(e)
58175817
return result
58185818

5819+
result["validation_errors"] = validation_errors
5820+
58195821
# For authorization_code OAuth gateways, empty responses may indicate incomplete auth flow
58205822
# Skip only if it's an auth_code gateway with no data (user may not have completed authorization)
58215823
is_auth_code_gateway = gateway_oauth_config and isinstance(gateway_oauth_config, dict) and gateway_oauth_config.get("grant_type") == "authorization_code"

tests/unit/mcpgateway/services/test_gateway_auto_refresh.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,29 @@ async def test_refresh_skips_auth_code_gateway_with_empty_response(self, gateway
212212
assert result["tools_added"] == 0
213213
assert result["tools_removed"] == 0
214214

215+
@pytest.mark.asyncio
216+
async def test_refresh_propagates_validation_errors(self, gateway_service):
217+
"""Validation errors from _initialize_gateway should propagate through auto-refresh."""
218+
mock_gateway = _make_mock_gateway()
219+
mock_session = MagicMock()
220+
mock_session.execute.return_value.scalar_one_or_none.return_value = mock_gateway
221+
222+
validation_errors = [
223+
"bad_tool: Tool name exceeds MCP spec limit of 128 characters (got 129)",
224+
]
225+
226+
with (
227+
patch("mcpgateway.services.gateway_service.fresh_db_session") as mock_fresh,
228+
patch.object(gateway_service, "_initialize_gateway", new_callable=AsyncMock) as mock_init,
229+
):
230+
mock_fresh.return_value.__enter__.return_value = mock_session
231+
mock_init.return_value = ({}, [], [], [], validation_errors)
232+
233+
result = await gateway_service._refresh_gateway_tools_resources_prompts("gw-123")
234+
235+
assert result["validation_errors"] == validation_errors
236+
assert result["success"] is True
237+
215238
@pytest.mark.asyncio
216239
async def test_refresh_processes_empty_non_auth_code_gateway(self, gateway_service):
217240
"""Test that refresh processes empty responses from non-auth_code gateways."""

tests/unit/mcpgateway/services/test_gateway_service.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6471,6 +6471,29 @@ async def test_init_failure_returns_error(self, gateway_service):
64716471
assert result["success"] is False
64726472
assert "connection refused" in result["error"]
64736473

6474+
@pytest.mark.asyncio
6475+
async def test_validation_errors_propagated(self, gateway_service):
6476+
"""Validation errors from _initialize_gateway populate result['validation_errors'] before early return."""
6477+
gw = SimpleNamespace(
6478+
enabled=True,
6479+
reachable=True,
6480+
name="val-gw",
6481+
url="http://example.com",
6482+
transport="sse",
6483+
auth_type="oauth",
6484+
auth_value=None,
6485+
oauth_config={"grant_type": "authorization_code"},
6486+
ca_certificate=None,
6487+
auth_query_params=None,
6488+
)
6489+
validation_errors = [
6490+
"bad_tool: Tool name exceeds MCP spec limit of 128 characters (got 129)",
6491+
]
6492+
gateway_service._initialize_gateway = AsyncMock(return_value=({}, [], [], [], validation_errors))
6493+
result = await gateway_service._refresh_gateway_tools_resources_prompts("gw-1", gateway=gw)
6494+
assert result["validation_errors"] == validation_errors
6495+
assert result["success"] is True
6496+
64746497
@pytest.mark.asyncio
64756498
async def test_auth_code_empty_response_returns_early(self, gateway_service):
64766499
gw = SimpleNamespace(

tests/unit/mcpgateway/test_main_extended.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4941,6 +4941,7 @@ async def test_refresh_gateway_tools_success_and_errors(self, monkeypatch):
49414941
"duration_ms": 1.0,
49424942
"refreshed_at": datetime.now(timezone.utc),
49434943
"tools_added": 1,
4944+
"validation_errors": [],
49444945
}
49454946
monkeypatch.setattr(main_mod.gateway_service, "get_gateway", AsyncMock(return_value=SimpleNamespace(id="gw-1")))
49464947
monkeypatch.setattr(main_mod, "_enforce_scoped_resource_access", lambda *_args, **_kwargs: None)
@@ -4955,6 +4956,25 @@ async def test_refresh_gateway_tools_success_and_errors(self, monkeypatch):
49554956
)
49564957
assert response.gateway_id == "gw-1"
49574958
assert response.tools_added == 1
4959+
assert response.validation_errors == []
4960+
4961+
# Test with non-empty validation errors
4962+
result_payload_with_errors = {
4963+
"duration_ms": 1.0,
4964+
"refreshed_at": datetime.now(timezone.utc),
4965+
"tools_added": 0,
4966+
"validation_errors": ["bad_tool: Tool name exceeds MCP spec limit of 128 characters (got 129)"],
4967+
}
4968+
monkeypatch.setattr(main_mod.gateway_service, "refresh_gateway_manually", AsyncMock(return_value=result_payload_with_errors))
4969+
response_with_errors = await main_mod.refresh_gateway_tools(
4970+
"gw-1",
4971+
request,
4972+
include_resources=True,
4973+
include_prompts=False,
4974+
db=db,
4975+
user={"email": "user@example.com"},
4976+
)
4977+
assert response_with_errors.validation_errors == result_payload_with_errors["validation_errors"]
49584978

49594979
monkeypatch.setattr(main_mod.gateway_service, "refresh_gateway_manually", AsyncMock(side_effect=GatewayNotFoundError("missing")))
49604980
with pytest.raises(HTTPException) as excinfo:

0 commit comments

Comments
 (0)