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>

Author: MohanLaksh

mcpgateway/services/gateway_service.py

@@ -3749,14 +3749,7 @@ def _get_due_gateway_lifecycle_ids(self) -> List[str]:
             claim_filter = or_(DbGateway.lifecycle_claim_expires_at.is_(None), DbGateway.lifecycle_claim_expires_at <= now)
             deleting_ids = db.execute(select(DbGateway.id).where(DbGateway.status == "deleting").where(claim_filter)).scalars().all()
             pending_ids = (
-                db.execute(
-                    select(DbGateway.id)
-                    .where(DbGateway.status == "pending")
-                    .where(or_(DbGateway.next_retry_at.is_(None), DbGateway.next_retry_at <= now))
-                    .where(claim_filter)
-                )
-                .scalars()
-                .all()
+                db.execute(select(DbGateway.id).where(DbGateway.status == "pending").where(or_(DbGateway.next_retry_at.is_(None), DbGateway.next_retry_at <= now)).where(claim_filter)).scalars().all()
             )
         return [*deleting_ids, *(f"pending:{gateway_id}" for gateway_id in pending_ids)]
 
@@ -4933,6 +4926,7 @@ async def _run_health_checks(self, user_email: str) -> None:
         while True:
             try:
                 if self._redis_client and settings.cache_type == "redis":
+
                     async def require_redis_leader() -> bool:
                         """Check for redis leader"""
                         current_leader = await self._redis_client.get(self._leader_key)
@@ -5572,9 +5566,7 @@ def _created_via_allowed(created_via: Optional[str]) -> bool:
 
         stale_resource_ids = []
         if not skip_stale_cleanup and catalog_sync.new_resource_uris is not None:
-            stale_resource_ids = [
-                resource.id for resource in gateway.resources if resource.uri not in catalog_sync.new_resource_uris and _created_via_allowed(getattr(resource, "created_via", None))
-            ]
+            stale_resource_ids = [resource.id for resource in gateway.resources if resource.uri not in catalog_sync.new_resource_uris and _created_via_allowed(getattr(resource, "created_via", None))]
             if stale_resource_ids:
                 for i in range(0, len(stale_resource_ids), 500):
                     chunk = stale_resource_ids[i : i + 500]
@@ -5585,9 +5577,7 @@ def _created_via_allowed(created_via: Optional[str]) -> bool:
 
         stale_prompt_ids = []
         if not skip_stale_cleanup and catalog_sync.new_prompt_names is not None:
-            stale_prompt_ids = [
-                prompt.id for prompt in gateway.prompts if prompt.original_name not in catalog_sync.new_prompt_names and _created_via_allowed(getattr(prompt, "created_via", None))
-            ]
+            stale_prompt_ids = [prompt.id for prompt in gateway.prompts if prompt.original_name not in catalog_sync.new_prompt_names and _created_via_allowed(getattr(prompt, "created_via", None))]
             if stale_prompt_ids:
                 for i in range(0, len(stale_prompt_ids), 500):
                     chunk = stale_prompt_ids[i : i + 500]
@@ -5601,13 +5591,9 @@ def _created_via_allowed(created_via: Optional[str]) -> bool:
         if not skip_stale_cleanup:
             gateway.tools = [tool for tool in gateway.tools if tool.original_name in catalog_sync.new_tool_names or not _created_via_allowed(getattr(tool, "created_via", None))]
             if catalog_sync.new_resource_uris is not None:
-                gateway.resources = [
-                    resource for resource in gateway.resources if resource.uri in catalog_sync.new_resource_uris or not _created_via_allowed(getattr(resource, "created_via", None))
-                ]
+                gateway.resources = [resource for resource in gateway.resources if resource.uri in catalog_sync.new_resource_uris or not _created_via_allowed(getattr(resource, "created_via", None))]
             if catalog_sync.new_prompt_names is not None:
-                gateway.prompts = [
-                    prompt for prompt in gateway.prompts if prompt.original_name in catalog_sync.new_prompt_names or not _created_via_allowed(getattr(prompt, "created_via", None))
-                ]
+                gateway.prompts = [prompt for prompt in gateway.prompts if prompt.original_name in catalog_sync.new_prompt_names or not _created_via_allowed(getattr(prompt, "created_via", None))]
 
         tools_removed = len(stale_tool_ids)
         resources_removed = len(stale_resource_ids)
@@ -5819,7 +5805,7 @@ async def _refresh_gateway_tools_resources_prompts(
                     _refresh_key = _enc.decrypt_secret_or_plaintext(_refresh_key)
                 except Exception:
                     logger.debug("client_key decryption skipped during gateway refresh")
-            _capabilities, tools, resources, prompts, _ = await self._initialize_gateway(
+            _capabilities, tools, resources, prompts, validation_errors = await self._initialize_gateway(
                 url=gateway_url,
                 authentication=gateway_auth_value,
                 transport=gateway_transport,
@@ -5839,6 +5825,8 @@ async def _refresh_gateway_tools_resources_prompts(
             result["error"] = str(e)
             return result
 
+        result["validation_errors"] = validation_errors
+
         # For authorization_code OAuth gateways, empty responses may indicate incomplete auth flow
         # Skip only if it's an auth_code gateway with no data (user may not have completed authorization)
         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

@@ -212,6 +212,29 @@ async def test_refresh_skips_auth_code_gateway_with_empty_response(self, gateway
         assert result["tools_added"] == 0
         assert result["tools_removed"] == 0
 
+    @pytest.mark.asyncio
+    async def test_refresh_propagates_validation_errors(self, gateway_service):
+        """Validation errors from _initialize_gateway should propagate through auto-refresh."""
+        mock_gateway = _make_mock_gateway()
+        mock_session = MagicMock()
+        mock_session.execute.return_value.scalar_one_or_none.return_value = mock_gateway
+
+        validation_errors = [
+            "bad_tool: Tool name exceeds MCP spec limit of 128 characters (got 129)",
+        ]
+
+        with (
+            patch("mcpgateway.services.gateway_service.fresh_db_session") as mock_fresh,
+            patch.object(gateway_service, "_initialize_gateway", new_callable=AsyncMock) as mock_init,
+        ):
+            mock_fresh.return_value.__enter__.return_value = mock_session
+            mock_init.return_value = ({}, [], [], [], validation_errors)
+
+            result = await gateway_service._refresh_gateway_tools_resources_prompts("gw-123")
+
+        assert result["validation_errors"] == validation_errors
+        assert result["success"] is True
+
     @pytest.mark.asyncio
     async def test_refresh_processes_empty_non_auth_code_gateway(self, gateway_service):
         """Test that refresh processes empty responses from non-auth_code gateways."""

tests/unit/mcpgateway/services/test_gateway_service.py

@@ -6471,6 +6471,29 @@ async def test_init_failure_returns_error(self, gateway_service):
         assert result["success"] is False
         assert "connection refused" in result["error"]
 
+    @pytest.mark.asyncio
+    async def test_validation_errors_propagated(self, gateway_service):
+        """Validation errors from _initialize_gateway populate result['validation_errors'] before early return."""
+        gw = SimpleNamespace(
+            enabled=True,
+            reachable=True,
+            name="val-gw",
+            url="http://example.com",
+            transport="sse",
+            auth_type="oauth",
+            auth_value=None,
+            oauth_config={"grant_type": "authorization_code"},
+            ca_certificate=None,
+            auth_query_params=None,
+        )
+        validation_errors = [
+            "bad_tool: Tool name exceeds MCP spec limit of 128 characters (got 129)",
+        ]
+        gateway_service._initialize_gateway = AsyncMock(return_value=({}, [], [], [], validation_errors))
+        result = await gateway_service._refresh_gateway_tools_resources_prompts("gw-1", gateway=gw)
+        assert result["validation_errors"] == validation_errors
+        assert result["success"] is True
+
     @pytest.mark.asyncio
     async def test_auth_code_empty_response_returns_early(self, gateway_service):
         gw = SimpleNamespace(

tests/unit/mcpgateway/test_main_extended.py

@@ -4904,6 +4904,7 @@ async def test_refresh_gateway_tools_success_and_errors(self, monkeypatch):
             "duration_ms": 1.0,
             "refreshed_at": datetime.now(timezone.utc),
             "tools_added": 1,
+            "validation_errors": [],
         }
         monkeypatch.setattr(main_mod.gateway_service, "get_gateway", AsyncMock(return_value=SimpleNamespace(id="gw-1")))
         monkeypatch.setattr(main_mod, "_enforce_scoped_resource_access", lambda *_args, **_kwargs: None)
@@ -4918,6 +4919,25 @@ async def test_refresh_gateway_tools_success_and_errors(self, monkeypatch):
         )
         assert response.gateway_id == "gw-1"
         assert response.tools_added == 1
+        assert response.validation_errors == []
+
+        # Test with non-empty validation errors
+        result_payload_with_errors = {
+            "duration_ms": 1.0,
+            "refreshed_at": datetime.now(timezone.utc),
+            "tools_added": 0,
+            "validation_errors": ["bad_tool: Tool name exceeds MCP spec limit of 128 characters (got 129)"],
+        }
+        monkeypatch.setattr(main_mod.gateway_service, "refresh_gateway_manually", AsyncMock(return_value=result_payload_with_errors))
+        response_with_errors = await main_mod.refresh_gateway_tools(
+            "gw-1",
+            request,
+            include_resources=True,
+            include_prompts=False,
+            db=db,
+            user={"email": "user@example.com"},
+        )
+        assert response_with_errors.validation_errors == result_payload_with_errors["validation_errors"]
 
         monkeypatch.setattr(main_mod.gateway_service, "refresh_gateway_manually", AsyncMock(side_effect=GatewayNotFoundError("missing")))
         with pytest.raises(HTTPException) as excinfo: