🪲 BUG: deduplicate and validate MCP server list input - #12
Merged
FernandoCelmer merged 3 commits intoAug 15, 2026
Merged
Conversation
FernandoCelmer
left a comment
FernandoCelmer
left a comment
Member
There was a problem hiding this comment.
🔍 Code Review
Code issues found: 1
| # | Severity | Comment |
|---|---|---|
| 1 | [Suggestion] | Missing add→remove round-trip test |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
…zation addServer trims whitespace before storing, but removeServer compared with strict equality — any legacy or existing whitespace-padded entry (including the " a " case this PR's own tests explicitly leave in storage) could never be removed through the UI. removeServer now trims both the stored entries and the incoming name before comparing, symmetric with addServer.
Member
|
Thanks for this one, @myukitty! Good catch on the dedupe/validation gap, and the tests made it really easy to spot the trim asymmetry between addServer and removeServer — fixed that and added a couple more tests to lock it in. Appreciate the contribution! |
7 tasks
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
2 participants
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.
Description
src/features/settings/mcpServerList.ts— UpdatedaddServerto trim whitespace from the input command, reject empty or whitespace-only strings, and detect duplicates by comparing trimmed forms. Now returns a trimmed copy of the new entry instead of the raw input.test/mcpServerList.test.ts— Added 4 new unit tests covering whitespace trimming, empty/whitespace-only rejection, exact duplicate detection, and duplicate detection against existing entries with whitespace.Motivation and Context
When managing custom MCP servers in the Settings webview, inputs could be appended without whitespace trimming or duplicate detection, allowing empty entries or whitespace-equivalent duplicates to accumulate in the stored list.
Closes #10.
Types of changes
Checklist