Conversation
addServer() appended unconditionally, so adding the same command twice spawned the server twice (pycodeloop serve received a duplicate --mcp flag) -- extra tool registrations, log noise, and resource usage. Also allowed blank/whitespace-only entries. Trims and checks against the existing list before appending; a duplicate or blank input is now a no-op. Fixes dotflow-io#10
Member
There was a problem hiding this comment.
🔍 Code Review
Code issues found: 2
| # | Severity | Comment |
|---|---|---|
| 1 | [Blocking] | Dedup check misses whitespace-padded existing entries |
| 2 | [Blocking] | No-op path returns original array reference |
Member
|
Thanks a lot for taking the time on this, @lukiod! Turns out this landed just after #12 merged an equivalent fix for addServer's trim/dedupe/blank-rejection (plus a matching fix on removeServer's side). Closing this one as covered, but really appreciate you spotting and fixing the duplicate-registration issue independently — good find! |
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 exact duplicates usingArray.includeson the trimmed value. Returns the existing list unchanged on no-op.test/mcpServerList.test.ts— Added 3 new unit tests covering: duplicate rejection (exact match and with extra whitespace), whitespace trimming before append, and blank/whitespace-only input rejection.Motivation and Context
addServer()appended entries unconditionally, allowing the same MCP server command to be registered multiple times. Each duplicate causedpycodeloop serveto receive an extra--mcpflag, spawning the server twice — doubling tool registrations, log noise, and resource usage. Blank inputs were also accepted without validation.Closes #10.
Types of changes
Checklist