fix: propagate tool validation errors in gateway refresh response (#5290)

[MohanLaksh]

🐛 Bug-fix PR

📌 Summary

Tool validation errors collected during gateway refresh were silently discarded — the GatewayRefreshResponse.validation_errors field always returned an empty list, even when tools failed validation (e.g., name too long, schema too deeply nested).

🔁 Reproduction Steps

Fixes #5290

1. Register a gateway with a mix of valid and invalid tools
2. Call POST /gateways/{id}/tools/refresh
3. Observe response.validation_errors is always [] even when tools were skipped

🐞 Root Cause

In _refresh_gateway_tools_resources_prompts() at mcpgateway/services/gateway_service.py:5822, the 5th return value (validation_errors) from _initialize_gateway() was discarded with _:

_capabilities, tools, resources, prompts, _ = await self._initialize_gateway(...)

The result dict was initialized with "validation_errors": [] at line 5740, but was never populated with the actual errors from _validate_tools().

💡 Fix Description

Production fix (2 lines):

1. Line 5822: Changed _ → validation_errors to capture the return value
2. Line 5842: Added result["validation_errors"] = validation_errors after the except block to populate the result on the success path

📏 Reviewability

• This PR has one clear purpose
• The linked issue is not labeled triage
• Unrelated bugs or improvements are tracked in separate issues/PRs
• Tests are included with the code they validate
• If AI-assisted, I understand and can explain the generated changes

🧪 Verification

Check	Command	Status
Ruff linter	make ruff	✅ Pass
Black formatter	make black	✅ Pass
Isort	make isort	✅ Pass
Pre-commit (secrets, formatting, etc)	make pre-commit	✅ Pass
Unit tests (TestRefreshGatewayToolsResourcesPrompts)	pytest tests/unit/mcpgateway/services/test_gateway_service.py -k TestRefreshGatewayToolsResourcesPrompts -x	✅ 9/9
Unit tests (TestAutoRefreshGatewayToolsResourcesPrompts)	pytest tests/unit/mcpgateway/services/test_gateway_auto_refresh.py -k TestAutoRefreshGatewayToolsResourcesPrompts -x	✅ 9/9
Route handler tests (test_refresh_gateway_tools)	pytest tests/unit/mcpgateway/test_main_extended.py -k test_refresh_gateway_tools -x	✅ 2/2

Test Coverage

Three new tests validate the fix:

1. test_gateway_service.py::test_validation_errors_propagated — Mocks _initialize_gateway to return non-empty validation errors and asserts result["validation_errors"] contains them
2. test_gateway_auto_refresh.py::test_refresh_propagates_validation_errors — Same pattern via patch.object for the auto-refresh path
3. test_main_extended.py::test_refresh_gateway_tools_success_and_errors — Extended to assert both empty and non-empty validation_errors in the API response

📐 MCP Compliance (if relevant)

• Matches current MCP spec
• No breaking change to MCP clients

✅ Checklist

• Code formatted (make black isort pre-commit)
• No secrets/credentials committed

[ja8zyjits]

Hi @MohanLaksh

Few comments here

⚠️ Warnings

• API endpoint POST /gateways/{gateway_id}/tools/refresh not documented

  • File: docs/docs/manage/api-usage.md
  • Code: mcpgateway/main.py:7203
  • Impact: Users can't discover manual refresh endpoint
  • Fix: Add section with request format, response schema, curl example, validation_errors behavior

• validation_errors field lacks detailed documentation

  • File: mcpgateway/schemas.py:3963
  • Current: "List of validation errors encountered"
  • Fix: Specify error types (tool name >128 chars, schema depth, missing fields)

---

Code Changes

mcpgateway/services/gateway_service.py (2 production lines):

• Line 5805: _ → validation_errors (capture 5th return value)
• Line 5825: result["validation_errors"] = validation_errors (populate dict)

Formatting (non-functional): Lines 3752, 4928, 5568, 5577, 5585, 5593, 5596

---

Test Coverage

Three new tests validate propagation:

1. test_gateway_service.py:6474-6495 - Service layer unit test
2. test_gateway_auto_refresh.py:215-236 - Auto-refresh path
3. test_main_extended.py:4907-4939 - API response E2E

Gaps:

• Multiple validation errors not tested (only single error)
• No integration test with actual _validate_tools() call
• No test for validation errors + partial success

---

[Lang-Akshay]

Thanks for the PR @MohanLaksh .
The core fix is correct and surgically targeted.

[msureshkumar88]

The core fix is correct and well-scoped — capturing the 5th return value from _initialize_gateway() and propagating it to result["validation_errors"] is exactly the right two-line change. Good that three test paths are covered (service layer, auto-refresh, and route handler).

A few things worth tracking as follow-ups (none blocking):

• Missing warning log on validation errors: The exception path at L5816 already logs a logger.warning, but the success path with non-empty validation_errors is silent. Operators have no log signal that tools were silently skipped without polling the API. Worth adding if validation_errors: logger.warning(...) after the assignment.
• Test coverage: All three new tests use a single-item error list and mock _initialize_gateway entirely. A test with multiple errors and at least one that exercises _validate_tools() directly (e.g., tool name > 128 chars) would harden this against regressions in the validation logic itself.
• Schema docstring (schemas.py:3963): "List of validation errors encountered" doesn't document the string format or what triggers errors. Minor, but useful for API consumers.
• Unrelated formatting changes at lines 3752, 4928, 5568–5596 in gateway_service.py add diff noise — worth isolating in a separate chore commit next time.

Build s390x is stuck in QUEUED (infrastructure issue, not code) — may need a re-run to unblock merge. Otherwise this is ready.

[msureshkumar88]

E2E verification — real running gateway, real upstream MCP server

Ran this against two real, live mcp-context-forge instances (not mocks/unit tests) — one on this PR's commit (c669cff2b), one on its parent commit (8e9c049e9, i.e. pre-fix) — each talking to a real upstream MCP stdio server (bridged to SSE via mcpgateway.translate) that advertises one valid tool and one tool whose name is 129 characters (1 over the 128-char MCP-spec limit mcp-context-forge enforces).

Script: scripts/verify-pr-5317-e2e.sh (added for this verification, not committed)

Config used (exported, not the repo .env):

DATABASE_URL=sqlite:///<scratch>/e2e_pr5317.db
AUTH_REQUIRED=true, MCPGATEWAY_ADMIN_API_ENABLED=true, PLUGINS_ENABLED=false
SSRF_ALLOW_LOCALHOST=true, SSRF_ALLOW_PRIVATE_NETWORKS=true   # needed for gateway → 127.0.0.1 upstream
JWT_SECRET_KEY=<test-only secret>

Commands run:

python -m mcpgateway.translate --stdio "uv run python mock_upstream_server.py" --expose-sse --port 44552
uv run uvicorn mcpgateway.main:app --host 127.0.0.1 --port 44551
python -m mcpgateway.utils.create_jwt_token --username admin@example.com --exp 10 --secret ...
curl -X POST /gateways                       # register gateway against the mock upstream
curl -X POST /gateways/{id}/tools/refresh     # trigger the code path this PR changes
curl /tools?gateway_id={id}                   # sanity check the valid tool still landed

Result on pre-fix commit 8e9c049e9 (reproduces #5290):
Server logs show validation genuinely ran and failed the tool —

ERROR - Validation failed for tool '<129-char name>': Tool name exceeds MCP spec limit of 128 characters (got 129)
WARNING - Tool validation completed with 1 error(s). Successfully validated 1 tool(s).

but the API response silently drops it:

"validationErrors": []

Result on this PR's commit c669cff2b:
Same scenario, same server-side validation log lines, but now the API response correctly surfaces it:

"validationErrors": ["<129-char name>: Tool name exceeds MCP spec limit of 128 characters (got 129)"]

valid_echo_tool is still registered in both cases (partial-success behavior unaffected).

Unit tests referenced in the PR description (all green):

pytest tests/unit/mcpgateway/services/test_gateway_service.py -k TestRefreshGatewayToolsResourcesPrompts -x     → 9/9
pytest tests/unit/mcpgateway/services/test_gateway_auto_refresh.py -k TestAutoRefreshGatewayToolsResourcesPrompts -x → 9/9
pytest tests/unit/mcpgateway/test_main_extended.py -k test_refresh_gateway_tools -x                              → 2/2

This is a clean, minimal fix — confirmed end-to-end against a real gateway process and a real upstream MCP server, not just mocked unit tests. Approving.