fix: treat only deleted servers as reusable for remote URL validation#1204
Open
stationeros wants to merge 16 commits intomodelcontextprotocol:mainfrom
Open
fix: treat only deleted servers as reusable for remote URL validation#1204stationeros wants to merge 16 commits intomodelcontextprotocol:mainfrom
stationeros wants to merge 16 commits intomodelcontextprotocol:mainfrom
Conversation
Contributor
|
@stationeros Thank you raising for the PR 👍 . Few points as per the design of status behaviour:
|
pree-dew
reviewed
Apr 25, 2026
pree-dew
reviewed
Apr 27, 2026
pree-dew
reviewed
Apr 27, 2026
Contributor
|
@stationeros We might not need this change, except for test cases. As this behaviour is already part of code where the default value is to exclude deleted servers. If you will run the test cases without the filter change, you will notice that tests are passing. Reason for the issue is different where status of servers are not marked as |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
validateNoDuplicateRemoteURLsby queryingListServerswithIncludeDeleted: 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:
This addresses the behavior discussed in #1193.
How Has This Been Tested?
Added targeted service-layer test coverage for the main scenarios:
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
Checklist
Additional context
The implementation is intentionally narrow:
validateNoDuplicateRemoteURLsusesListServerswithIncludeDeleted: falseUpdateServerStatus(..., active)continues to revalidate when restoring fromdeletedUpdateAllVersionsStatus(..., active)does the same for deleted versions in the bulk restore flowThis keeps the behavior consistent across publish, update, and restore flows while preserving the intended lifecycle semantics for deprecated versus deleted servers.