Add server name validation to MCP config loader#145
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds validation to ensure server names in MCP configuration files contain only alphanumeric characters and hyphens. The validation prevents invalid configurations from being loaded by raising a ValueError early in the process.
Key changes:
- Added server name validation using regex pattern matching
- Modified configuration loading to validate each server name before creating McpServer instances
- Introduced early error handling for invalid server names
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
The _servers attribute is not initialized before being used. |
Karanraj-6
left a comment
There was a problem hiding this comment.
- Initialize self._servers = {} before populating
- Add validation for server names (letters, numbers, hyphens only)
|
Hi @Karanraj-6 thanks for your contribution. Can you please rebase and squash the commits? |
21d2c8e to
e4a3863
Compare
|
@cristipufu I’ve rebased and squashed the commits. Can you please review and merge when you get a chance? |
e4a3863 to
5f49306
Compare
5f49306 to
f14865b
Compare
Changes include:
Validate that server names contain only letters, numbers, and hyphens.
Raise a ValueError if an invalid server name is found, preventing the loading of invalid configurations early.
Update _load_config in McpConfig to incorporate the validation step.
Motivation:
Ensures that only properly formatted server names are loaded, reducing potential runtime errors and improving reliability of MCP configuration handling.
Testing:
Initialized McpConfig locally.
Printed all server names to confirm validation runs without errors.
Verified that invalid server names correctly raise ValueError.