-
Notifications
You must be signed in to change notification settings - Fork 7
Add direct HTTP type support and custom json key #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -60,6 +60,17 @@ | |||||
| "config_format": "json", | ||||||
| "mcp_key": "mcpServers" | ||||||
| }, | ||||||
| "vscode-mcp": { | ||||||
| "name": "VSCode Copilot", | ||||||
| "description": "Vscode Copilot MCP settings", | ||||||
|
||||||
| "description": "Vscode Copilot MCP settings", | |
| "description": "VS Code Copilot MCP settings", |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,21 +1,24 @@ | ||||||||||||||||||||||||||||||||
| """Pydantic models for configuration validation.""" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| from pydantic import BaseModel, Field, field_validator | ||||||||||||||||||||||||||||||||
| from pydantic import BaseModel, Field, field_validator, model_validator | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| class MCPServerConfig(BaseModel): | ||||||||||||||||||||||||||||||||
| """Configuration for an MCP server.""" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| command: str = Field(..., description="Command to run the server") | ||||||||||||||||||||||||||||||||
| args: list[str] = Field(default_factory=list, description="Additional arguments") | ||||||||||||||||||||||||||||||||
| env: dict[str, str] = Field(default_factory=dict, description="Environment variables") | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| @field_validator("command") | ||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||
| def validate_command(cls, v): | ||||||||||||||||||||||||||||||||
| if not v or not v.strip(): | ||||||||||||||||||||||||||||||||
| command: str | None = Field(default=None, description="Command to run the server") | ||||||||||||||||||||||||||||||||
| args: list[str] | None = Field(default=None, description="Additional arguments") | ||||||||||||||||||||||||||||||||
| env: dict[str, str] | None = Field(default=None, description="Environment variables") | ||||||||||||||||||||||||||||||||
| type: str | None = Field(default=None, description="Transport type (e.g., http)") | ||||||||||||||||||||||||||||||||
| url: str | None = Field(default=None, description="URL for HTTP/SSE transport") | ||||||||||||||||||||||||||||||||
| serverUrl: str | None = Field(default=None, description="Antigravity-specific URL field") | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| @model_validator(mode="after") | ||||||||||||||||||||||||||||||||
| def validate_command(self): | ||||||||||||||||||||||||||||||||
| # command is only required if transport type is not HTTP | ||||||||||||||||||||||||||||||||
| if (self.type or "").strip().lower() != "http" and (not self.command or not self.command.strip()): | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new vacuum path now attempts to import URL-based servers, but this validator still rejects any entry without Useful? React with 👍 / 👎. |
||||||||||||||||||||||||||||||||
| raise ValueError("Command cannot be empty") | ||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
20
|
||||||||||||||||||||||||||||||||
| # command is only required if transport type is not HTTP | |
| if (self.type or "").strip().lower() != "http" and (not self.command or not self.command.strip()): | |
| raise ValueError("Command cannot be empty") | |
| transport_type = (self.type or "").strip().lower() | |
| command = (self.command or "").strip() | |
| url = (self.url or "").strip() | |
| server_url = (self.serverUrl or "").strip() | |
| # command is only required if transport type is not HTTP | |
| if transport_type != "http" and not command: | |
| raise ValueError("Command cannot be empty") | |
| # HTTP transport requires at least one endpoint to be configured | |
| if transport_type == "http" and not url and not server_url: | |
| raise ValueError("HTTP transport requires either 'url' or 'serverUrl'") |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -76,7 +76,7 @@ def get_locations_config(self) -> LocationsConfig: | |||||||||||||||||
| def _save_locations_config(self, config: LocationsConfig) -> None: | ||||||||||||||||||
| """Save locations configuration.""" | ||||||||||||||||||
| with open(self.locations_file, "w") as f: | ||||||||||||||||||
| json.dump(config.model_dump(), f, indent=2) | ||||||||||||||||||
| json.dump(config.model_dump(exclude_none=True), f, indent=2) | ||||||||||||||||||
|
|
||||||||||||||||||
| def get_global_config(self) -> GlobalConfig: | ||||||||||||||||||
| """Get global configuration.""" | ||||||||||||||||||
|
|
@@ -107,7 +107,7 @@ def get_global_config(self) -> GlobalConfig: | |||||||||||||||||
| def _save_global_config(self, config: GlobalConfig) -> None: | ||||||||||||||||||
| """Save global configuration.""" | ||||||||||||||||||
| with open(self.global_config_file, "w") as f: | ||||||||||||||||||
| json.dump(config.model_dump(), f, indent=2) | ||||||||||||||||||
| json.dump(config.model_dump(exclude_none=True), f, indent=2) | ||||||||||||||||||
|
|
||||||||||||||||||
| def _migrate_server_config(self, config: dict) -> dict: | ||||||||||||||||||
| """Migrate old server config format to new format.""" | ||||||||||||||||||
|
|
@@ -168,9 +168,9 @@ def get_client_definitions(self) -> ClientDefinitions: | |||||||||||||||||
| def _save_user_client_definitions(self, definitions: ClientDefinitions) -> None: | ||||||||||||||||||
| """Save user client definitions.""" | ||||||||||||||||||
| with open(self.user_client_definitions_file, "w") as f: | ||||||||||||||||||
| json.dump(definitions.model_dump(), f, indent=2) | ||||||||||||||||||
| json.dump(definitions.model_dump(exclude_none=True), f, indent=2) | ||||||||||||||||||
|
|
||||||||||||||||||
| def add_location(self, path: str, name: str | None = None) -> bool: | ||||||||||||||||||
| def add_location(self, path: str, name: str | None = None, mcp_key: str | None = None) -> bool: | ||||||||||||||||||
| """Add a new location.""" | ||||||||||||||||||
| config = self.get_locations_config() | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -181,7 +181,12 @@ def add_location(self, path: str, name: str | None = None) -> bool: | |||||||||||||||||
|
|
||||||||||||||||||
| # Add new location | ||||||||||||||||||
| location_name = name or Path(path).stem | ||||||||||||||||||
| new_location = LocationConfig(path=path, name=location_name, type="manual") | ||||||||||||||||||
| new_location = LocationConfig( | ||||||||||||||||||
| path=path, | ||||||||||||||||||
| name=location_name, | ||||||||||||||||||
| type="manual", | ||||||||||||||||||
| mcp_key=mcp_key or "mcpServers" | ||||||||||||||||||
|
Comment on lines
+185
to
+188
|
||||||||||||||||||
| path=path, | |
| name=location_name, | |
| type="manual", | |
| mcp_key=mcp_key or "mcpServers" | |
| path=path, | |
| name=location_name, | |
| type="manual", | |
| mcp_key=mcp_key |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,7 +88,7 @@ def _build_master_server_list(self, global_only: bool, project_only: bool) -> di | |
| global_config = self.settings.get_global_config() | ||
| global_servers = global_config.mcpServers | ||
| for name, config in global_servers.items(): | ||
| master_servers[name] = {**config.model_dump(), "_source": "global"} | ||
| master_servers[name] = {**config.model_dump(exclude_none=True), "_source": "global"} | ||
|
|
||
| # Add project servers (override global) | ||
| if not global_only: | ||
|
|
@@ -151,7 +151,8 @@ def _sync_location( | |
| return | ||
|
|
||
| # Extract current MCP servers | ||
| current_servers = current_config.get("mcpServers", {}) | ||
| mcp_key = location.get("mcp_key", "mcpServers") | ||
| current_servers = current_config.get(mcp_key, {}) | ||
|
|
||
| # Build new server list | ||
| new_servers = {} | ||
|
|
@@ -184,10 +185,10 @@ def _sync_location( | |
|
|
||
| # Update config | ||
| new_config = current_config.copy() | ||
| new_config["mcpServers"] = new_servers | ||
| new_config[mcp_key] = new_servers | ||
|
|
||
| # Check if changes are needed | ||
| if current_config.get("mcpServers", {}) != new_servers: | ||
| if current_config.get(mcp_key, {}) != new_servers: | ||
| if not result.dry_run: | ||
| # Ensure directory exists | ||
| location_path.parent.mkdir(parents=True, exist_ok=True) | ||
|
|
@@ -327,15 +328,6 @@ def _sync_cli_location( | |
| for name, config in new_servers.items(): | ||
| if name not in current_servers or current_servers[name] != config: | ||
| # Check if this is a URL-based server (SSE/HTTP) | ||
| url = config.get("url") | ||
| if url: | ||
| # This is a URL-based server - skip for now | ||
| self.logger.info( | ||
| f"Skipping URL-based server {name} (URL: {url}) - " | ||
| "CLI client URL support not fully implemented" | ||
| ) | ||
| continue | ||
|
|
||
| command = config.get("command", []) | ||
| args = config.get("args", []) or [] | ||
| env_vars = config.get("env", {}) | ||
|
Comment on lines
330
to
333
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This add/update block now processes all changed servers as command-based entries, but newly supported Useful? React with 👍 / 👎. |
||
|
|
@@ -377,7 +369,7 @@ def get_server_status(self) -> dict[str, Any]: | |
| # Global servers | ||
| global_config = self.settings.get_global_config() | ||
| status["global_servers"] = { | ||
| name: config.model_dump() for name, config in global_config.mcpServers.items() | ||
| name: config.model_dump(exclude_none=True) for name, config in global_config.mcpServers.items() | ||
| } | ||
|
|
||
| # Project servers | ||
|
|
@@ -411,7 +403,8 @@ def get_server_status(self) -> dict[str, Any]: | |
| location_path = Path(location["path"]) | ||
| config = self._read_json_config(location_path) | ||
| if config is not None: | ||
| status["location_servers"][location["name"]] = config.get("mcpServers", {}) | ||
| mcp_key = location.get("mcp_key", "mcpServers") | ||
| status["location_servers"][location["name"]] = config.get(mcp_key, {}) | ||
| else: | ||
| status["location_servers"][location["name"]] = "error" | ||
|
|
||
|
|
@@ -611,15 +604,8 @@ def vacuum_configs( | |
| result.skipped_servers.append(server_name) | ||
| continue | ||
|
|
||
| # Skip URL-based servers as they're not supported by MCPServerConfig | ||
| # Support URL-based servers | ||
| config_data = server_info["config"].copy() | ||
| if "url" in config_data and "command" not in config_data: | ||
| self.logger.info( | ||
| f"Skipping URL-based server {server_name} - " | ||
| "not supported by current config model" | ||
| ) | ||
| result.skipped_servers.append(server_name) | ||
| continue | ||
|
|
||
| # Normalize command format for MCPServerConfig validation | ||
|
Comment on lines
+607
to
610
|
||
| # Command should be a string, args should be an array | ||
|
|
@@ -630,11 +616,11 @@ def vacuum_configs( | |
| config_data["command"] = command_list[0] | ||
| config_data["args"] = command_list[1:] + config_data.get("args", []) | ||
|
|
||
| # Ensure args and env are properly formatted | ||
| if "args" not in config_data: | ||
| config_data["args"] = [] | ||
| if "env" not in config_data: | ||
| config_data["env"] = {} | ||
| # Ensure args and env are only present if they have values | ||
| if not config_data.get("args"): | ||
| config_data.pop("args", None) | ||
| if not config_data.get("env"): | ||
| config_data.pop("env", None) | ||
|
|
||
| try: | ||
| server_config = MCPServerConfig(**config_data) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -64,8 +64,8 @@ def test_minimal_valid_configuration(self): | |||||
| """Test creating MCPServerConfig with minimal required fields.""" | ||||||
| config = MCPServerConfig(command="echo") | ||||||
| assert config.command == "echo" | ||||||
| assert config.args == [] | ||||||
| assert config.env == {} | ||||||
| assert config.args is None | ||||||
| assert config.env is None | ||||||
|
|
||||||
| def test_command_field_validation_non_empty(self): | ||||||
| """Test that command field cannot be empty.""" | ||||||
|
|
@@ -85,18 +85,39 @@ def test_command_field_validation_whitespace_only(self): | |||||
| assert error["type"] == "value_error" | ||||||
| assert "Command cannot be empty" in str(exc_info.value) | ||||||
|
|
||||||
| def test_http_type_allows_missing_command(self): | ||||||
| """Test that HTTP transport does not require a command.""" | ||||||
| config = MCPServerConfig(type="http", url="https://example.com/mcp") | ||||||
| assert config.type == "http" | ||||||
| assert config.command is None | ||||||
|
|
||||||
| def test_http_type_allows_whitespace_command(self): | ||||||
| """Test that HTTP transport ignores empty command values.""" | ||||||
| config = MCPServerConfig(type="http", command=" ", url="https://example.com/mcp") | ||||||
| assert config.type == "http" | ||||||
| assert config.command == " " | ||||||
|
||||||
| assert config.command == " " | |
| assert config.command is None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON example here is not valid: the
custom-serverobject is missing a comma after theenvblock, braces/indentation are mismatched, and thehttp-serverobject isn’t properly aligned/closed. This will confuse users copying the example; please fix the snippet to be valid JSON.