Skip to content

fix: treat only deleted servers as reusable for remote URL validation#1204

Open
stationeros wants to merge 5 commits intomodelcontextprotocol:mainfrom
stationeros:main
Open

fix: treat only deleted servers as reusable for remote URL validation#1204
stationeros wants to merge 5 commits intomodelcontextprotocol:mainfrom
stationeros:main

Conversation

@stationeros
Copy link
Copy Markdown

@stationeros stationeros commented Apr 25, 2026

Treat only deleted servers as reusable for remote URL validation.

This updates remote URL validation to check for conflicts only against non-deleted servers. In practice, that means active and deprecated servers continue to reserve their remote URLs, while deleted servers do not.

The validation remains centralized in validateNoDuplicateRemoteURLs by querying ListServers with IncludeDeleted: false, rather than adding additional post-query status checks.

Motivation and Context

Remote URLs attached to deprecated servers are still valid for downstream clients, so they should continue to participate in conflict checks. Only deleted servers should release their remote URLs for reuse.

This change aligns the implementation with the intended status behavior:

  • active servers reserve their remote URLs
  • deprecated servers also reserve their remote URLs
  • deleted servers do not block reuse
  • publishing, updating a server, and restoring a deleted server to active all validate against the same non-deleted conflict set

This addresses the behavior discussed in #1193.

How Has This Been Tested?

Added targeted service-layer test coverage for the main scenarios:

  • creating a server with a remote URL that matches a deleted server is allowed
  • creating a server with a remote URL that matches a deprecated server fails
  • restoring a deleted server to active still fails if another non-deleted server now uses that remote URL
  • restoring all deleted versions to active fails under the same conflict condition

I have not validated this in a deployed registry instance; testing was focused on the service behavior and regression coverage around the affected code paths.

Breaking Changes

No breaking changes intended.

This change narrows remote URL reuse to deleted servers only and keeps deprecated servers in the conflict set.

Types of changes

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

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

The implementation is intentionally narrow:

  • validateNoDuplicateRemoteURLs uses ListServers with IncludeDeleted: false
  • deleted servers are excluded from remote URL conflict checks
  • deprecated servers remain part of the conflict set
  • UpdateServerStatus(..., active) continues to revalidate when restoring from deleted
  • UpdateAllVersionsStatus(..., active) does the same for deleted versions in the bulk restore flow

This keeps the behavior consistent across publish, update, and restore flows while preserving the intended lifecycle semantics for deprecated versus deleted servers.

@pree-dew
Copy link
Copy Markdown
Contributor

pree-dew commented Apr 25, 2026

@stationeros Thank you raising for the PR 👍 . Few points as per the design of status behaviour:

  • Remote url attached to a deprecated server is a valid URL. Deprecated server can still be an active server for downstream clients. It might not be maintained by the owner. If a user wants to publish a new package with this remote url, they have to mark this as deleted first.
  • Migrating from deleted to active and vice versa are the 2 status that require remote url validation.
  • If we are updating a server then also we have to check for remote url validation.

Comment thread internal/service/registry_service.go Outdated
// Check if any conflicting server has a different name
// Only active servers reserve remote URLs.
for _, conflictingServer := range conflictingServers {
if conflictingServer.Meta.Official != nil && conflictingServer.Meta.Official.Status != model.StatusActive {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if all conflictingServers are deleted then also we will allow this action to happen in case of publish server action.
In case of validating remote url from moving delete to active or updating a server or during publishing a server, we need to check if there is any active or deprecated server with that url, we can add another filter variable to the query of ListServers of include_deleted as false, we will not need another if condition in that case. Filter is designed to add filter condition for ListServer, what we want is to get servers list with same remote url which are not deleted so we can just pass include_deleted

Copy link
Copy Markdown
Author

@stationeros stationeros Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed clarification — agree to this . Have updated the commit and tests

@stationeros stationeros requested a review from pree-dew April 25, 2026 11:35
@stationeros stationeros changed the title fix: ignore deprecated servers in remote URL conflict checks fix: treat only deleted servers as reusable for remote URL validation Apr 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants