452Remove-Integration-Type

[Nayana-R-Gowda]

📌 Summary

What problem does this PR fix and why?
integrationType should only support REST, not MCP closes #452

💡 Fix Description

How did you solve it? Key design points.
The integrationType field in the admin UI (and API) currently allows "MCP", but this is not desired. The issue requests removing "MCP" from all input forms (modals)—keeping only "REST". There’s mention that support for "MCP" servers may eventually be added under a separate "Gateways" section.

🧪 Verification

Check	Command	Status
Lint suite	make lint	pass
Unit tests	make test	pass

📐 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

[crivetimihai]

PR Review: Remove MCP Integration Type (#678)

Summary

This PR removes "MCP" as an integration type option from the admin UI and API, keeping only "REST" as the supported integration type. This aligns with the requirement that MCP servers are supported under a separate "Gateways" section.

Changes Reviewed

1. Schema Changes (mcpgateway/schemas.py)

• Line 311: Changed integration_type field from Literal["MCP", "REST"] to Literal["REST"] with default value "REST"
• Lines 463-481: Updated validate_request_type to handle REST-only validation
• Lines 649-660: Updated ToolUpdate class to only allow REST request types

Issues Found:

• ⚠️ Inconsistent validation logic: The validate_request_type method still has MCP validation code (lines 475-478) even though MCP is no longer a valid option
• The error messages still reference MCP which could be confusing

2. Admin API Changes (mcpgateway/admin.py)

• Lines 1741-1750: Added logic to default request_type based on integration_type
• Line 1972: Changed default integration type to "REST" in edit tool
• Updated all doctest examples to use REST instead of MCP

Issues Found:

• ⚠️ Dead code: Lines 1745-1747 check for MCP integration type but MCP is no longer a valid option
• The defaulting logic for request_type when integration_type == "MCP" will never execute

3. Template Changes (mcpgateway/templates/admin.html)

• Line 894: Removed <option value="MCP">MCP</option> from add tool modal
• Line 2324: Removed <option value="MCP">MCP</option> from edit tool modal

Review: ✅ Clean removal of MCP options from UI

4. Test Updates

• tests/security/test_input_validation.py:
  • Commented out MCP validation tests (lines 498-503)
  • Updated default integration type assertion to "REST"
• tests/unit/mcpgateway/services/test_tool_service.py:
  • Updated all test cases to use REST methods instead of MCP
• tests/unit/mcpgateway/test_admin.py:
  • Updated test cases to use REST integration type
• docs/docs/testing/basic.md:
  • Updated example to use REST instead of MCP

Issues Found:

• ⚠️ Commented code: Instead of commenting out MCP tests, they should be removed entirely

Test Results

⚠️ Tests cannot run due to database migration issue:

• Error: alembic.util.exc.CommandError: Can't locate revision identified by 'cc7b95fec5d9'
• This appears to be an environment issue after rebasing from main, not related to the PR changes

Recommendations

Critical Issues to Fix:

1. Remove dead MCP code in mcpgateway/schemas.py:

   • Remove lines 475-478 that check for MCP integration type
   • Simplify the validation to only handle REST methods

2. Remove dead MCP code in mcpgateway/admin.py:

   • Remove lines 1745-1747 that handle MCP integration type
   • Simplify the defaulting logic

3. Clean up commented tests:

   • Remove commented MCP test code in test_input_validation.py instead of leaving it commented

Minor Improvements:

1. Update error messages: Remove references to MCP in error messages since it's no longer supported

2. Consider adding a database migration: If there are existing tools with integration_type="MCP" in the database, a migration might be needed to update them to REST

Overall Assessment

The PR successfully removes MCP as an integration type option from the UI and API. However, there's some dead code left behind that should be cleaned up. The changes are mostly correct but need some refinement to remove references to MCP completely.

[crivetimihai]

Will be reverting this through a separate PR.