From ce98f8f47510c04816f307efc04bca836a373287 Mon Sep 17 00:00:00 2001 From: User Date: Tue, 8 Jul 2025 18:24:22 -0700 Subject: [PATCH 1/8] feat: refactor commands to v2 structure and add HTTP server support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit refactors the command structure for MCPM v2 and adds comprehensive HTTP server support: ## Command Structure Changes - Move add/remove commands from target_operations to main commands directory - Rename commands to match v2 syntax: `add` → `install`, `remove` → `uninstall` - Remove v1-specific profile activation logic and complexity - Inline necessary helper functions into command files - Update CLI registration and imports ## HTTP Server Support - Add support for installing HTTP MCP servers via `mcpm install` - HTTP servers create RemoteServerConfig instead of STDIOServerConfig - Add dedicated headers field for HTTP server configuration - Headers support variable substitution (e.g., `"Authorization": "Bearer ${API_TOKEN}"`) - HTTP servers don't require command/args fields ## Smart Argument Filtering - Only prompt for arguments actually referenced in installation method - Scan for `${VARIABLE_NAME}` patterns in all installation fields - HTTP installations without variables prompt for no arguments - Traditional installations only prompt for used variables ## Bug Fixes - Fix AttributeError when accessing .command on RemoteServerConfig - Add proper type checking in run.py and display.py - Ensure HTTP servers are handled correctly throughout the system ## Tests - Update all tests to use new command structure - Add comprehensive tests for HTTP server installation - Add tests for argument filtering functionality - All tests passing with new architecture 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/mcpm/cli.py | 8 +- src/mcpm/commands/__init__.py | 7 +- .../{target_operations/add.py => install.py} | 161 ++++++++++--- src/mcpm/commands/list.py | 8 +- src/mcpm/commands/run.py | 12 +- src/mcpm/commands/target_operations/common.py | 162 ------------- .../remove.py => uninstall.py} | 52 +++-- src/mcpm/schemas/full_server_config.py | 21 +- src/mcpm/utils/display.py | 9 +- tests/test_add.py | 213 ++++++++++++++++-- tests/test_remove.py | 18 +- 11 files changed, 412 insertions(+), 259 deletions(-) rename src/mcpm/commands/{target_operations/add.py => install.py} (77%) delete mode 100644 src/mcpm/commands/target_operations/common.py rename src/mcpm/commands/{target_operations/remove.py => uninstall.py} (56%) diff --git a/src/mcpm/cli.py b/src/mcpm/cli.py index c495b2e..42a1499 100644 --- a/src/mcpm/cli.py +++ b/src/mcpm/cli.py @@ -10,19 +10,19 @@ from mcpm.clients.client_config import ClientConfigManager from mcpm.commands import ( - add, client, config, doctor, edit, info, inspect, + install, list, migrate, profile, - remove, run, search, + uninstall, usage, ) from mcpm.commands.share import share @@ -120,8 +120,8 @@ def main(ctx, version, help_flag): main.add_command(search.search) main.add_command(info.info) main.add_command(list.list, name="ls") -main.add_command(add.add, name="install") -main.add_command(remove.remove, name="uninstall") +main.add_command(install.install) +main.add_command(uninstall.uninstall) main.add_command(edit.edit) main.add_command(run.run) main.add_command(inspect.inspect) diff --git a/src/mcpm/commands/__init__.py b/src/mcpm/commands/__init__.py index 8daf3f6..7ac7152 100644 --- a/src/mcpm/commands/__init__.py +++ b/src/mcpm/commands/__init__.py @@ -3,18 +3,18 @@ """ __all__ = [ - "add", "client", "config", "doctor", "info", "inspect", + "install", "list", "migrate", "profile", - "remove", "run", "search", + "uninstall", "usage", ] @@ -27,11 +27,12 @@ doctor, info, inspect, + install, list, migrate, profile, run, search, + uninstall, usage, ) -from .target_operations import add, remove diff --git a/src/mcpm/commands/target_operations/add.py b/src/mcpm/commands/install.py similarity index 77% rename from src/mcpm/commands/target_operations/add.py rename to src/mcpm/commands/install.py index a67e1e9..23f7374 100644 --- a/src/mcpm/commands/target_operations/add.py +++ b/src/mcpm/commands/install.py @@ -15,9 +15,9 @@ from rich.progress import Progress, SpinnerColumn, TextColumn from rich.prompt import Confirm -from mcpm.commands.target_operations.common import ( - global_add_server, -) +from mcpm.global_config import GlobalConfigManager +from mcpm.core.schema import ServerConfig, STDIOServerConfig +from mcpm.utils.config import NODE_EXECUTABLES, ConfigManager from mcpm.profile.profile_config import ProfileConfigManager from mcpm.schemas.full_server_config import FullServerConfig from mcpm.utils.repository import RepositoryManager @@ -26,6 +26,7 @@ console = Console() repo_manager = RepositoryManager() profile_config_manager = ProfileConfigManager() +global_config_manager = GlobalConfigManager() # Create a prompt session with custom styling prompt_session = PromptSession() @@ -43,6 +44,34 @@ kb = KeyBindings() +def _replace_node_executable(server_config: ServerConfig) -> ServerConfig: + """Replace node executable with configured alternative if applicable.""" + if not isinstance(server_config, STDIOServerConfig): + return server_config + command = server_config.command.strip() + if command not in NODE_EXECUTABLES: + return server_config + config = ConfigManager().get_config() + config_node_executable = config.get("node_executable") + if not config_node_executable: + return server_config + if config_node_executable != command: + console.print(f"[bold cyan]Replace node executable {command} with {config_node_executable}[/]") + server_config.command = config_node_executable + return server_config + + +def global_add_server(server_config: ServerConfig, force: bool = False) -> bool: + """Add a server to the global MCPM configuration.""" + if global_config_manager.server_exists(server_config.name) and not force: + console.print(f"[bold red]Error:[/] Server '{server_config.name}' already exists in global configuration.") + console.print("Use --force to override.") + return False + + server_config = _replace_node_executable(server_config) + return global_config_manager.add_server(server_config, force) + + def prompt_with_default(prompt_text, default="", hide_input=False, required=False): """Prompt the user with a default value that can be edited directly. @@ -91,7 +120,7 @@ def prompt_with_default(prompt_text, default="", hide_input=False, required=Fals @click.option("--force", is_flag=True, help="Force reinstall if server is already installed") @click.option("--alias", help="Alias for the server", required=False) @click.help_option("-h", "--help") -def add(server_name, force=False, alias=None): +def install(server_name, force=False, alias=None): """Install an MCP server to the global configuration. Installs servers to the global MCPM configuration where they can be @@ -100,22 +129,14 @@ def add(server_name, force=False, alias=None): Examples: \b - mcpm add time - mcpm add everything --force - mcpm add youtube --alias yt + mcpm install time + mcpm install everything --force + mcpm install youtube --alias yt """ - # v2.0: use global config - - # Check if this is a profile (starts with %) - if server_name.startswith("%"): - profile_name = server_name[1:] # Remove % prefix - add_profile_to_client(profile_name, "global", alias, force) - return - config_name = alias or server_name - # v2.0: All servers are installed to global configuration + # All servers are installed to global configuration console.print("[yellow]Installing server to global configuration...[/]") # Get server metadata from repository @@ -240,12 +261,30 @@ def add(server_name, force=False, alias=None): # Process variables to store in config processed_variables = {} - # First, prompt for all defined arguments even if they're not in env_vars - progress.stop() + # Extract which arguments are actually referenced in the selected installation method + referenced_vars = _extract_referenced_variables(selected_method) if selected_method else set() + + # Filter arguments to only those that are referenced + relevant_arguments = {} if all_arguments: + if referenced_vars: + # Only include arguments that are referenced + for arg_name, arg_info in all_arguments.items(): + if arg_name in referenced_vars: + relevant_arguments[arg_name] = arg_info + elif selected_method: + # If we have a selected method but no referenced vars, don't prompt for any + relevant_arguments = {} + else: + # No selected method - use all arguments (backward compatibility) + relevant_arguments = all_arguments + + # First, prompt for relevant arguments + progress.stop() + if relevant_arguments: console.print("\n[bold]Configure server arguments:[/]") - for arg_name, arg_info in all_arguments.items(): + for arg_name, arg_info in relevant_arguments.items(): description = arg_info.get("description", "") is_required = arg_info.get("required", False) example = arg_info.get("example", "") @@ -328,6 +367,21 @@ def add(server_name, force=False, alias=None): if key in processed_variables and replacement_status == ReplacementStatus.NON_STANDARD_REPLACE: has_non_standard_argument_define = True + # For HTTP servers, headers should be extracted from the installation method + # not from processed variables + processed_headers = {} + if installation_method == "http" and selected_method: + # Extract headers from the installation method if defined + headers_template = selected_method.get("headers", {}) + for key, value in headers_template.items(): + # Replace variables in header values + header_replaced, _ = _replace_variables(value, processed_variables) + if header_replaced: + processed_headers[key] = header_replaced + else: + # If no replacement, use the original value + processed_headers[key] = value + if has_non_standard_argument_define: # no matter in argument / env console.print( @@ -338,11 +392,22 @@ def add(server_name, force=False, alias=None): # Get actual MCP execution command, args, and env from the selected installation method # This ensures we use the actual server command information instead of placeholders + mcp_url = None + mcp_command = None + mcp_args = [] + if selected_method: - mcp_command = selected_method.get("command", install_command) - mcp_args = processed_args + # For HTTP servers, extract the URL and don't set command/args + if installation_method == "http": + mcp_url = selected_method.get("url") + # HTTP servers don't have command/args + else: + # For non-HTTP servers, get command and args + mcp_command = selected_method.get("command", install_command) + mcp_args = processed_args # Env vars are already processed above else: + # Fallback for when no selected method mcp_command = install_command mcp_args = processed_args @@ -356,9 +421,11 @@ def add(server_name, force=False, alias=None): env=processed_env, # Use the simplified installation method installation=installation_method, + url=mcp_url, # Include URL for HTTP servers + headers=processed_headers, # Include headers for HTTP servers ) - # v2.0: Add server to global configuration + # Add server to global configuration success = global_add_server(full_server_config.to_server_config(), force) if success: @@ -393,6 +460,41 @@ def _should_hide_input(arg_name: str) -> bool: return "token" in arg_name.lower() or "key" in arg_name.lower() or "secret" in arg_name.lower() +def _extract_referenced_variables(installation_method: dict) -> set: + """Extract all variable names referenced in an installation method. + + Scans through all fields in the installation method (command, args, env, url, etc.) + looking for ${VARIABLE_NAME} patterns. + + Args: + installation_method: The installation method configuration dict + + Returns: + Set of variable names that are referenced + """ + referenced = set() + + def extract_from_value(value): + """Recursively extract variables from a value.""" + if isinstance(value, str): + # Find all ${VAR_NAME} patterns + matches = re.findall(r"\$\{([^}]+)\}", value) + referenced.update(matches) + elif isinstance(value, list): + for item in value: + extract_from_value(item) + elif isinstance(value, dict): + for v in value.values(): + extract_from_value(v) + + # Check all fields in the installation method + for key, value in installation_method.items(): + if key not in ["type", "description", "recommended"]: # Skip metadata fields + extract_from_value(value) + + return referenced + + class ReplacementStatus(str, Enum): NOT_REPLACED = "not_replaced" STANDARD_REPLACE = "standard_replace" @@ -473,18 +575,3 @@ def _replace_argument_variables(value: str, prev_value: str, variables: dict) -> # nothing to replace return value, ReplacementStatus.NOT_REPLACED - - -def add_profile_to_client(profile_name: str, client: str, alias: str | None = None, force: bool = False): - if not force and not Confirm.ask(f"Add this profile {profile_name} to {client}{' as ' + alias if alias else ''}?"): - console.print("[yellow]Operation cancelled.[/]") - raise click.ClickException("Operation cancelled") - - console.print("[bold red]Error:[/] Profile activation has been removed in MCPM v2.0.") - console.print("[yellow]Use 'mcpm profile share' to share profiles instead.[/]") - success = False - if success: - console.print(f"[bold green]Successfully added profile {profile_name} to {client}![/]") - else: - console.print(f"[bold red]Failed to add profile {profile_name} to {client}.[/]") - raise click.ClickException(f"Failed to add profile {profile_name} to {client}") diff --git a/src/mcpm/commands/list.py b/src/mcpm/commands/list.py index 8690723..45b9a47 100644 --- a/src/mcpm/commands/list.py +++ b/src/mcpm/commands/list.py @@ -4,13 +4,19 @@ from rich.console import Console -from mcpm.commands.target_operations.common import global_list_servers +from mcpm.global_config import GlobalConfigManager from mcpm.profile.profile_config import ProfileConfigManager from mcpm.utils.display import print_server_config from mcpm.utils.rich_click_config import click console = Console() profile_manager = ProfileConfigManager() +global_config_manager = GlobalConfigManager() + + +def global_list_servers(): + """List all servers in the global MCPM configuration.""" + return global_config_manager.list_servers() @click.command() diff --git a/src/mcpm/commands/run.py b/src/mcpm/commands/run.py index b15dd8a..3f398b7 100644 --- a/src/mcpm/commands/run.py +++ b/src/mcpm/commands/run.py @@ -151,7 +151,17 @@ def run(server_name, http, port): # Debug logging is now handled by the Rich logging setup in CLI # Just log debug info - the level is controlled centrally logger.debug(f"Running server '{server_name}' from {location} configuration") - logger.debug(f"Command: {server_config.command} {' '.join(server_config.args or [])}") + + # Log command details based on server type + from mcpm.core.schema import RemoteServerConfig, STDIOServerConfig + + if isinstance(server_config, STDIOServerConfig): + logger.debug(f"Command: {server_config.command} {' '.join(server_config.args or [])}") + elif isinstance(server_config, RemoteServerConfig): + logger.debug(f"URL: {server_config.url}") + if server_config.headers: + logger.debug(f"Headers: {list(server_config.headers.keys())}") + logger.debug(f"Mode: {'HTTP' if http else 'stdio'}") if http: logger.debug(f"Port: {port}") diff --git a/src/mcpm/commands/target_operations/common.py b/src/mcpm/commands/target_operations/common.py deleted file mode 100644 index 55e3d9e..0000000 --- a/src/mcpm/commands/target_operations/common.py +++ /dev/null @@ -1,162 +0,0 @@ -from rich.console import Console - -from mcpm.clients.client_registry import ClientRegistry -from mcpm.core.schema import ServerConfig, STDIOServerConfig -from mcpm.global_config import GlobalConfigManager -from mcpm.profile.profile_config import ProfileConfigManager -from mcpm.utils.config import NODE_EXECUTABLES, ConfigManager -from mcpm.utils.scope import ScopeType, parse_server - -console = Console() -global_config_manager = GlobalConfigManager() - - -def determine_scope(scope: str | None) -> tuple[ScopeType | None, str | None]: - """v2.0: This function is deprecated. All operations use global configuration. - - This is kept for backwards compatibility but always returns global scope. - """ - # v2.0: Everything uses global configuration - no scope needed - # Return a special marker to indicate global scope - return ScopeType.GLOBAL, "global" - - -def determine_target(target: str) -> tuple[ScopeType | None, str | None, str | None]: - """v2.0: Parse target but always use global scope for servers.""" - scope_type, scope, server_name = parse_server(target) - - # In v2.0, if no scope is specified, default to global - if not scope and server_name: - return ScopeType.GLOBAL, "global", server_name - - # If scope is specified but we're looking for a server, it might be a profile operation - if scope and server_name: - return scope_type, scope, server_name - - # If no server name, this might be a profile-only operation - if scope and not server_name: - return scope_type, scope, "" - - return None, None, None - - -# v2.0 Global server management functions - - -def global_add_server(server_config: ServerConfig, force: bool = False) -> bool: - """Add a server to the global MCPM configuration.""" - if global_config_manager.server_exists(server_config.name) and not force: - console.print(f"[bold red]Error:[/] Server '{server_config.name}' already exists in global configuration.") - console.print("Use --force to override.") - return False - - server_config = _replace_node_executable(server_config) - return global_config_manager.add_server(server_config, force) - - -def global_remove_server(server_name: str) -> bool: - """Remove a server from the global MCPM configuration and clean up profile tags.""" - if not global_config_manager.server_exists(server_name): - console.print(f"[bold red]Error:[/] Server '{server_name}' not found in global configuration.") - return False - - # Remove from global config (this automatically removes all profile tags) - success = global_config_manager.remove_server(server_name) - - # No need for additional profile cleanup since virtual profiles - # are managed automatically through profile tags on servers - - return success - - -def global_get_server(server_name: str) -> ServerConfig | None: - """Get a server from the global MCPM configuration.""" - server = global_config_manager.get_server(server_name) - if not server: - console.print(f"[bold red]Error:[/] Server '{server_name}' not found in global configuration.") - return server - - -def global_list_servers() -> dict[str, ServerConfig]: - """List all servers in the global MCPM configuration.""" - return global_config_manager.list_servers() - - -def _replace_node_executable(server_config: ServerConfig) -> ServerConfig: - if not isinstance(server_config, STDIOServerConfig): - return server_config - command = server_config.command.strip() - if command not in NODE_EXECUTABLES: - return server_config - config = ConfigManager().get_config() - config_node_executable = config.get("node_executable") - if not config_node_executable: - return server_config - if config_node_executable != command: - console.print(f"[bold cyan]Replace node executable {command} with {config_node_executable}[/]") - server_config.command = config_node_executable - return server_config - - -def client_add_server(client: str, server_config: ServerConfig, force: bool = False) -> bool: - client_manager = ClientRegistry.get_client_manager(client) - if not client_manager: - console.print(f"[bold red]Error:[/] Client '{client}' not found.") - return False - if client_manager.get_server(server_config.name) and not force: - console.print(f"[bold red]Error:[/] Server '{server_config.name}' already exists in {client}.") - console.print("Use --force to override.") - return False - server_config = _replace_node_executable(server_config) - success = client_manager.add_server(server_config) - - return success - - -def client_remove_server(client: str, server: str) -> bool: - client_manager = ClientRegistry.get_client_manager(client) - if not client_manager: - console.print(f"[bold red]Error:[/] Client '{client}' not found.") - return False - success = client_manager.remove_server(server) - return success - - -def client_get_server(client: str, server: str) -> ServerConfig | None: - client_manager = ClientRegistry.get_client_manager(client) - if not client_manager: - console.print(f"[bold red]Error:[/] Client '{client}' not found.") - return None - return client_manager.get_server(server) - - -def profile_add_server(profile: str, server_config: ServerConfig, force: bool = False) -> bool: - profile_manager = ProfileConfigManager() - if profile_manager.get_profile(profile) is None: - console.print(f"[bold red]Error:[/] Profile '{profile}' not found.") - return False - - if profile_manager.get_profile_server(profile, server_config.name) and not force: - console.print(f"[bold red]Error:[/] Server '{server_config.name}' already exists in {profile}.") - console.print("Use --force to override.") - return False - server_config = _replace_node_executable(server_config) - success = profile_manager.set_profile(profile, server_config) - return success - - -def profile_remove_server(profile: str, server: str) -> bool: - profile_manager = ProfileConfigManager() - if profile_manager.get_profile(profile) is None: - console.print(f"[bold red]Error:[/] Profile '{profile}' not found.") - return False - success = profile_manager.remove_server(profile, server) - return success - - -def profile_get_server(profile: str, server: str) -> ServerConfig | None: - profile_manager = ProfileConfigManager() - if profile_manager.get_profile(profile) is None: - console.print(f"[bold red]Error:[/] Profile '{profile}' not found.") - return None - return profile_manager.get_profile_server(profile, server) diff --git a/src/mcpm/commands/target_operations/remove.py b/src/mcpm/commands/uninstall.py similarity index 56% rename from src/mcpm/commands/target_operations/remove.py rename to src/mcpm/commands/uninstall.py index 46831be..4e058f4 100644 --- a/src/mcpm/commands/target_operations/remove.py +++ b/src/mcpm/commands/uninstall.py @@ -1,26 +1,42 @@ """ -Remove command for MCPM +Uninstall command for removing MCP servers from global configuration """ from rich.console import Console from rich.prompt import Confirm -from mcpm.commands.target_operations.common import ( - determine_target, - global_get_server, - global_remove_server, -) +from mcpm.global_config import GlobalConfigManager from mcpm.utils.display import print_server_config from mcpm.utils.rich_click_config import click console = Console() +global_config_manager = GlobalConfigManager() + + +def global_get_server(server_name: str): + """Get a server from the global MCPM configuration.""" + server = global_config_manager.get_server(server_name) + if not server: + console.print(f"[bold red]Error:[/] Server '{server_name}' not found in global configuration.") + return server + + +def global_remove_server(server_name: str) -> bool: + """Remove a server from the global MCPM configuration and clean up profile tags.""" + if not global_config_manager.server_exists(server_name): + console.print(f"[bold red]Error:[/] Server '{server_name}' not found in global configuration.") + return False + + # Remove from global config (this automatically removes all profile tags) + success = global_config_manager.remove_server(server_name) + return success @click.command() @click.argument("server_name") @click.option("--force", "-f", is_flag=True, help="Force removal without confirmation") @click.help_option("-h", "--help") -def remove(server_name, force): +def uninstall(server_name, force): """Remove an installed MCP server from global configuration. Removes servers from the global MCPM configuration and clears @@ -29,17 +45,11 @@ def remove(server_name, force): Examples: \b - mcpm rm filesystem - mcpm rm filesystem --force + mcpm uninstall filesystem + mcpm uninstall filesystem --force """ - # v2.0: Extract server name and use global configuration - scope_type, scope, extracted_server_name = determine_target(server_name) - - # In v2.0, we use the extracted server name, or the original if no extraction occurred - actual_server_name = extracted_server_name if extracted_server_name else server_name - # Get server from global configuration - server_info = global_get_server(actual_server_name) + server_info = global_get_server(server_name) if not server_info: return # Error message already printed by global_get_server @@ -59,13 +69,13 @@ def remove(server_name, force): return # Log the removal action - console.print(f"[bold red]Removing MCP server from global configuration:[/] {actual_server_name}") + console.print(f"[bold red]Removing MCP server from global configuration:[/] {server_name}") - # v2.0: Remove from global configuration - success = global_remove_server(actual_server_name) + # Remove from global configuration + success = global_remove_server(server_name) if success: - console.print(f"[green]Successfully removed server:[/] {actual_server_name}") + console.print(f"[green]Successfully removed server:[/] {server_name}") console.print("[dim]Note: Server has been removed from global config. Profile tags are also cleared.[/]") else: - console.print(f"[bold red]Error:[/] Failed to remove server '{actual_server_name}'.") + console.print(f"[bold red]Error:[/] Failed to remove server '{server_name}'.") diff --git a/src/mcpm/schemas/full_server_config.py b/src/mcpm/schemas/full_server_config.py index 391447f..7a527f9 100644 --- a/src/mcpm/schemas/full_server_config.py +++ b/src/mcpm/schemas/full_server_config.py @@ -4,7 +4,7 @@ from pydantic import BaseModel, Field, model_validator -from mcpm.core.schema import ServerConfig, STDIOServerConfig +from mcpm.core.schema import RemoteServerConfig, ServerConfig, STDIOServerConfig class FullServerConfig(BaseModel): @@ -17,14 +17,18 @@ class FullServerConfig(BaseModel): # Required fields name: str - command: str - args: List[str] + + # Fields that are required for STDIO servers but optional for HTTP servers + command: Optional[str] = None + args: List[str] = Field(default_factory=list) # Optional fields env: Dict[str, str] = Field(default_factory=dict) display_name: Optional[str] = None description: str = "" installation: Optional[str] = None + url: Optional[str] = None # For HTTP servers + headers: Dict[str, str] = Field(default_factory=dict) # For HTTP servers model_config = { "populate_by_name": True, @@ -67,4 +71,13 @@ def to_dict(self) -> Dict: def to_server_config(self) -> ServerConfig: """Convert FullServerConfig to a common server configuration format""" - return STDIOServerConfig(name=self.name, command=self.command, args=self.args, env=self.env) + # Check if this is an HTTP server + if self.installation == "http" and self.url: + # For HTTP servers, use the headers field directly + return RemoteServerConfig(name=self.name, url=self.url, headers=self.headers) + else: + # Default to STDIO for all other cases + # Command is required for STDIO servers + if not self.command: + raise ValueError(f"Command is required for non-HTTP server '{self.name}'") + return STDIOServerConfig(name=self.name, command=self.command, args=self.args, env=self.env) diff --git a/src/mcpm/utils/display.py b/src/mcpm/utils/display.py index e29b632..8f38170 100644 --- a/src/mcpm/utils/display.py +++ b/src/mcpm/utils/display.py @@ -8,7 +8,7 @@ from rich.markup import escape from rich.table import Table -from mcpm.core.schema import CustomServerConfig, RemoteServerConfig, ServerConfig +from mcpm.core.schema import CustomServerConfig, RemoteServerConfig, ServerConfig, STDIOServerConfig from mcpm.utils.scope import CLIENT_PREFIX, PROFILE_PREFIX console = Console() @@ -45,6 +45,13 @@ def print_server_config(server_config: ServerConfig, is_stashed=False, show_name console.print(json.dumps(server_config.config, indent=2)) console.print(" " + "-" * 50) return + + # Handle STDIOServerConfig - all remaining configs should be STDIO + if not isinstance(server_config, STDIOServerConfig): + console.print(" Type: [red]Unknown server type[/]") + console.print(" " + "-" * 50) + return + command = server_config.command console.print(f" Command: [green]{command}[/]") diff --git a/tests/test_add.py b/tests/test_add.py index d2d04d4..aa30974 100644 --- a/tests/test_add.py +++ b/tests/test_add.py @@ -2,8 +2,8 @@ from click.testing import CliRunner -from mcpm.commands.target_operations.add import add -from mcpm.core.schema import RemoteServerConfig +from mcpm.commands.install import install +from mcpm.core.schema import RemoteServerConfig, STDIOServerConfig from mcpm.global_config import GlobalConfigManager from mcpm.utils.config import ConfigManager from mcpm.utils.repository import RepositoryManager @@ -15,7 +15,7 @@ def test_add_server(windsurf_manager, monkeypatch, tmp_path): global_config_path = tmp_path / "servers.json" global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) - monkeypatch.setattr("mcpm.commands.target_operations.common.global_config_manager", global_config_manager) + monkeypatch.setattr("mcpm.commands.install.global_config_manager", global_config_manager) monkeypatch.setattr( RepositoryManager, @@ -43,7 +43,7 @@ def test_add_server(windsurf_manager, monkeypatch, tmp_path): # Mock prompt_toolkit's prompt to return our test values with patch("prompt_toolkit.PromptSession.prompt", side_effect=["json", "test-api-key"]): runner = CliRunner() - result = runner.invoke(add, ["server-test", "--force", "--alias", "test"]) + result = runner.invoke(install, ["server-test", "--force", "--alias", "test"]) assert result.exit_code == 0 # Check that the server was added to global configuration with alias @@ -60,7 +60,7 @@ def test_add_server_with_missing_arg(windsurf_manager, monkeypatch, tmp_path): global_config_path = tmp_path / "servers.json" global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) - monkeypatch.setattr("mcpm.commands.target_operations.common.global_config_manager", global_config_manager) + monkeypatch.setattr("mcpm.commands.install.global_config_manager", global_config_manager) monkeypatch.setattr( RepositoryManager, @@ -103,7 +103,7 @@ def test_add_server_with_missing_arg(windsurf_manager, monkeypatch, tmp_path): ): # Use CliRunner which provides its own isolated environment runner = CliRunner() - result = runner.invoke(add, ["server-test", "--force", "--alias", "test-missing-arg"]) + result = runner.invoke(install, ["server-test", "--force", "--alias", "test-missing-arg"]) if result.exit_code != 0: print(f"Exit code: {result.exit_code}") @@ -127,7 +127,7 @@ def test_add_server_with_empty_args(windsurf_manager, monkeypatch, tmp_path): global_config_path = tmp_path / "servers.json" global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) - monkeypatch.setattr("mcpm.commands.target_operations.common.global_config_manager", global_config_manager) + monkeypatch.setattr("mcpm.commands.install.global_config_manager", global_config_manager) monkeypatch.setattr( RepositoryManager, @@ -173,7 +173,7 @@ def test_add_server_with_empty_args(windsurf_manager, monkeypatch, tmp_path): patch("rich.progress.Progress.add_task"), ): runner = CliRunner() - result = runner.invoke(add, ["server-test", "--force", "--alias", "test-empty-args"]) + result = runner.invoke(install, ["server-test", "--force", "--alias", "test-empty-args"]) assert result.exit_code == 0 @@ -224,17 +224,17 @@ def test_add_profile_to_client(windsurf_manager, monkeypatch, tmp_path): global_config_path = tmp_path / "servers.json" global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) - monkeypatch.setattr("mcpm.commands.target_operations.common.global_config_manager", global_config_manager) + monkeypatch.setattr("mcpm.commands.install.global_config_manager", global_config_manager) profile_name = "work" - # test cli - in v2.0, profile with % prefix should fail gracefully + # test cli - in v2.0, % prefix is treated as a regular server name since profile activation is removed runner = CliRunner() - result = runner.invoke(add, ["%" + profile_name, "--force", "--alias", "work"]) + result = runner.invoke(install, ["%" + profile_name, "--force", "--alias", "work"]) - # In v2.0, this should fail because % profiles and profile activation are not supported - assert result.exit_code == 1 # Command fails - assert "Profile activation has been removed" in result.output + # Should fail because "%work" is not a valid server in the registry + assert result.exit_code == 0 # Command doesn't crash but will show error about server not found + assert "not found in registry" in result.output def test_add_server_with_configured_npx(windsurf_manager, monkeypatch, tmp_path): @@ -242,7 +242,7 @@ def test_add_server_with_configured_npx(windsurf_manager, monkeypatch, tmp_path) global_config_path = tmp_path / "servers.json" global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) - monkeypatch.setattr("mcpm.commands.target_operations.common.global_config_manager", global_config_manager) + monkeypatch.setattr("mcpm.commands.install.global_config_manager", global_config_manager) monkeypatch.setattr(ConfigManager, "get_config", Mock(return_value={"node_executable": "bunx"})) monkeypatch.setattr( @@ -275,7 +275,7 @@ def test_add_server_with_configured_npx(windsurf_manager, monkeypatch, tmp_path) patch("prompt_toolkit.PromptSession.prompt", side_effect=["json", "test-api-key"]), ): runner = CliRunner() - result = runner.invoke(add, ["server-test", "--force", "--alias", "test"]) + result = runner.invoke(install, ["server-test", "--force", "--alias", "test"]) assert result.exit_code == 0 # Check that the server was added with alias @@ -285,3 +285,184 @@ def test_add_server_with_configured_npx(windsurf_manager, monkeypatch, tmp_path) assert server.command == "bunx" assert server.args == ["-y", "@modelcontextprotocol/server-test", "--fmt", "json"] assert server.env["API_KEY"] == "test-api-key" + + +def test_add_http_server(windsurf_manager, monkeypatch, tmp_path): + """Test add HTTP server to global configuration""" + # Setup temporary global config + global_config_path = tmp_path / "servers.json" + global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) + + monkeypatch.setattr("mcpm.commands.install.global_config_manager", global_config_manager) + + monkeypatch.setattr( + RepositoryManager, + "_fetch_servers", + Mock( + return_value={ + "github": { + "display_name": "GitHub MCP Server", + "description": "Official GitHub MCP server", + "installations": { + "http": { + "type": "http", + "url": "https://github.com/github/github-mcp-server/releases/latest/download/github-mcp-server", + "description": "Run as HTTP server", + "recommended": True, + } + }, + "arguments": { + "GITHUB_API_TOKEN": {"type": "string", "description": "GitHub API token", "required": True}, + }, + } + } + ), + ) + + # Mock prompt_toolkit's prompt to return our test values + # Since HTTP installation doesn't reference any variables, no prompts should occur + with ( + patch("prompt_toolkit.PromptSession.prompt") as mock_prompt, + patch("rich.progress.Progress.start"), + patch("rich.progress.Progress.stop"), + patch("rich.progress.Progress.add_task"), + ): + runner = CliRunner() + result = runner.invoke(install, ["github", "--force"]) + assert result.exit_code == 0 + # No prompts should have been shown since HTTP method doesn't reference any variables + mock_prompt.assert_not_called() + + # Check that the server was added to global configuration as RemoteServerConfig + server = global_config_manager.get_server("github") + assert server is not None + assert isinstance(server, RemoteServerConfig) + assert server.url == "https://github.com/github/github-mcp-server/releases/latest/download/github-mcp-server" + # No headers should be set since the test installation doesn't define any + assert server.headers == {} + + +def test_add_server_with_filtered_arguments(windsurf_manager, monkeypatch, tmp_path): + """Test that only referenced arguments are prompted for""" + # Setup temporary global config + global_config_path = tmp_path / "servers.json" + global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) + + monkeypatch.setattr("mcpm.commands.install.global_config_manager", global_config_manager) + + monkeypatch.setattr( + RepositoryManager, + "_fetch_servers", + Mock( + return_value={ + "test-server": { + "display_name": "Test Server", + "description": "Test server with multiple arguments", + "installations": { + "docker": { + "type": "docker", + "command": "docker", + "args": ["run", "-e", "API_KEY=${API_KEY}", "test-server"], + "env": {"API_KEY": "${API_KEY}"}, + "description": "Run with Docker - only uses API_KEY", + "recommended": True, + } + }, + "arguments": { + "API_KEY": {"type": "string", "description": "API key", "required": True}, + "DATABASE_URL": {"type": "string", "description": "Database URL", "required": True}, + "UNUSED_VAR": { + "type": "string", + "description": "This var is not used in docker", + "required": True, + }, + }, + } + } + ), + ) + + # Mock prompt_toolkit's prompt to return our test values + # Should only be called once for API_KEY since that's the only referenced variable + prompt_calls = [] + + def mock_prompt_func(*args, **kwargs): + prompt_calls.append(kwargs.get("message", "")) + return "test-api-key" + + with ( + patch("prompt_toolkit.PromptSession.prompt", side_effect=mock_prompt_func), + patch("rich.progress.Progress.start"), + patch("rich.progress.Progress.stop"), + patch("rich.progress.Progress.add_task"), + ): + runner = CliRunner() + result = runner.invoke(install, ["test-server", "--force"]) + assert result.exit_code == 0 + + # Check that only API_KEY was prompted for + assert len(prompt_calls) == 1 + assert "API_KEY" in str(prompt_calls[0]) + assert "DATABASE_URL" not in str(prompt_calls[0]) + assert "UNUSED_VAR" not in str(prompt_calls[0]) + + # Check that the server was added correctly + server = global_config_manager.get_server("test-server") + assert server is not None + assert isinstance(server, STDIOServerConfig) + assert server.command == "docker" + assert server.args == ["run", "-e", "API_KEY=test-api-key", "test-server"] + assert server.env["API_KEY"] == "test-api-key" + + +def test_add_http_server_with_headers(windsurf_manager, monkeypatch, tmp_path): + """Test add HTTP server with headers to global configuration""" + # Setup temporary global config + global_config_path = tmp_path / "servers.json" + global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) + + monkeypatch.setattr("mcpm.commands.install.global_config_manager", global_config_manager) + + monkeypatch.setattr( + RepositoryManager, + "_fetch_servers", + Mock( + return_value={ + "api-server": { + "display_name": "API Server", + "description": "HTTP API server with auth headers", + "installations": { + "http": { + "type": "http", + "url": "https://api.example.com/mcp", + "headers": {"Authorization": "Bearer ${API_TOKEN}", "X-API-Version": "1.0"}, + "description": "Connect via HTTP with authentication", + "recommended": True, + } + }, + "arguments": { + "API_TOKEN": {"type": "string", "description": "API authentication token", "required": True}, + }, + } + } + ), + ) + + # Mock prompt_toolkit's prompt to return our test values + with ( + patch("prompt_toolkit.PromptSession.prompt", side_effect=["test-token-123"]), + patch("rich.progress.Progress.start"), + patch("rich.progress.Progress.stop"), + patch("rich.progress.Progress.add_task"), + ): + runner = CliRunner() + result = runner.invoke(install, ["api-server", "--force"]) + assert result.exit_code == 0 + + # Check that the server was added to global configuration as RemoteServerConfig + server = global_config_manager.get_server("api-server") + assert server is not None + assert isinstance(server, RemoteServerConfig) + assert server.url == "https://api.example.com/mcp" + # Check headers were properly set with variable replacement + assert server.headers == {"Authorization": "Bearer test-token-123", "X-API-Version": "1.0"} diff --git a/tests/test_remove.py b/tests/test_remove.py index cc6dd30..83dd80f 100644 --- a/tests/test_remove.py +++ b/tests/test_remove.py @@ -8,7 +8,7 @@ from click.testing import CliRunner -from mcpm.commands.target_operations.remove import remove +from mcpm.commands.uninstall import uninstall from mcpm.core.schema import STDIOServerConfig from mcpm.global_config import GlobalConfigManager @@ -30,9 +30,9 @@ def test_remove_server_success(): global_config_manager.add_server(test_server) # Mock the global config manager in the remove command - with patch("mcpm.commands.target_operations.common.global_config_manager", global_config_manager): + with patch("mcpm.commands.uninstall.global_config_manager", global_config_manager): runner = CliRunner() - result = runner.invoke(remove, ["test-server", "--force"]) + result = runner.invoke(uninstall, ["test-server", "--force"]) assert result.exit_code == 0 assert "Successfully removed server: test-server" in result.output @@ -47,9 +47,9 @@ def test_remove_server_not_found(): global_config_manager = GlobalConfigManager(config_path=str(global_config_path)) # Mock the global config manager - with patch("mcpm.commands.target_operations.common.global_config_manager", global_config_manager): + with patch("mcpm.commands.uninstall.global_config_manager", global_config_manager): runner = CliRunner() - result = runner.invoke(remove, ["non-existent-server", "--force"]) + result = runner.invoke(uninstall, ["non-existent-server", "--force"]) assert result.exit_code == 0 assert "Server 'non-existent-server' not found in global configuration" in result.output @@ -73,11 +73,11 @@ def test_remove_server_cancelled(): # Mock the global config manager and user input with ( - patch("mcpm.commands.target_operations.common.global_config_manager", global_config_manager), + patch("mcpm.commands.uninstall.global_config_manager", global_config_manager), patch("rich.prompt.Confirm.ask", return_value=False), ): runner = CliRunner() - result = runner.invoke(remove, ["test-server"]) + result = runner.invoke(uninstall, ["test-server"]) assert result.exit_code == 0 assert "Removal cancelled" in result.output @@ -103,11 +103,11 @@ def test_remove_server_with_confirmation(): # Mock the global config manager and user input with ( - patch("mcpm.commands.target_operations.common.global_config_manager", global_config_manager), + patch("mcpm.commands.uninstall.global_config_manager", global_config_manager), patch("rich.prompt.Confirm.ask", return_value=True), ): runner = CliRunner() - result = runner.invoke(remove, ["test-server"]) + result = runner.invoke(uninstall, ["test-server"]) assert result.exit_code == 0 assert "Successfully removed server: test-server" in result.output From 172a4e886e9525d16e088670a9442a5b687cfa0e Mon Sep 17 00:00:00 2001 From: User Date: Tue, 8 Jul 2025 18:26:56 -0700 Subject: [PATCH 2/8] chore: update dependencies and GitHub server config for HTTP support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Update fastmcp to v2.10.2 for improved HTTP server support - Update GitHub server configuration to use OAuth HTTP endpoint - Simplify GitHub server config to use GITHUB_PERSONAL_ACCESS_TOKEN - Remove custom Go installation method for cleaner config 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- mcp-registry/servers/github.json | 41 +++++++------------------------- pyproject.toml | 2 +- uv.lock | 8 +++---- 3 files changed, 14 insertions(+), 37 deletions(-) diff --git a/mcp-registry/servers/github.json b/mcp-registry/servers/github.json index 6dad09c..ca7a4af 100644 --- a/mcp-registry/servers/github.json +++ b/mcp-registry/servers/github.json @@ -11,9 +11,7 @@ "name": "GitHub" }, "license": "MIT", - "categories": [ - "Dev Tools" - ], + "categories": ["Dev Tools"], "tags": [ "github", "code", @@ -24,37 +22,19 @@ "api" ], "arguments": { - "GITHUB_API_TOKEN": { - "description": "GitHub Personal Access Token for authentication", + "GITHUB_PERSONAL_ACCESS_TOKEN": { + "description": "GitHub Personal Access Token for authentication. Not required for http installation.", "required": true, "example": "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" - }, - "GITHUB_BASE_URL": { - "description": "GitHub API base URL (for GitHub Enterprise Server)", - "required": false, - "example": "https://github.example.com/api/v3" } }, "installations": { "http": { "type": "http", - "url": "https://github.com/github/github-mcp-server/releases/latest/download/github-mcp-server", - "description": "Download and run binary from GitHub releases", + "url": "https://api.githubcopilot.com/mcp/", + "description": "Run github mcp directly with OAuth", "recommended": true }, - "custom": { - "type": "custom", - "command": "go", - "args": [ - "run", - "github.com/github/github-mcp-server@latest" - ], - "env": { - "GITHUB_API_TOKEN": "${GITHUB_API_TOKEN}", - "GITHUB_BASE_URL": "${GITHUB_BASE_URL}" - }, - "description": "Run directly with Go" - }, "docker": { "type": "docker", "command": "docker", @@ -63,14 +43,11 @@ "-i", "--rm", "-e", - "GITHUB_API_TOKEN", - "-e", - "GITHUB_BASE_URL", - "ghcr.io/github/github-mcp-server:latest" + "${GITHUB_PERSONAL_ACCESS_TOKEN}", + "ghcr.io/github/github-mcp-server" ], "env": { - "GITHUB_API_TOKEN": "${GITHUB_API_TOKEN}", - "GITHUB_BASE_URL": "${GITHUB_BASE_URL}" + "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_PERSONAL_ACCESS_TOKEN}" }, "description": "Run with Docker" } @@ -893,4 +870,4 @@ ], "is_official": true, "is_archived": false -} \ No newline at end of file +} diff --git a/pyproject.toml b/pyproject.toml index f2569be..8477b7f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ dependencies = [ "requests>=2.28.0", "pydantic>=2.5.1", "mcp>=1.8.0", - "fastmcp>=2.0.0", + "fastmcp>=2.10.2", "ruamel-yaml>=0.18.10", "watchfiles>=1.0.4", "duckdb>=1.2.2", diff --git a/uv.lock b/uv.lock index 6ed7f33..9c4a220 100644 --- a/uv.lock +++ b/uv.lock @@ -323,7 +323,7 @@ wheels = [ [[package]] name = "fastmcp" -version = "2.10.1" +version = "2.10.2" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "authlib" }, @@ -336,9 +336,9 @@ dependencies = [ { name = "rich" }, { name = "typer" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/33/1f/0031ea07bcad9f9b38d3500772d2749ca2b16335b92bd012f1d2f86a853e/fastmcp-2.10.1.tar.gz", hash = "sha256:450c72e523926a2203c7eecdb4a8b0507506667bc8736b8b7bb44f6312424649", size = 2730387 } +sdist = { url = "https://files.pythonhosted.org/packages/ad/ac/4906336669a643b634b7ecde539c79126dff9f47b1e9e4b312dd170c1715/fastmcp-2.10.2.tar.gz", hash = "sha256:1ad519fda8bfde1de7f54a7ed4be0b6353eb66230cc1e35eef29b47666a0027b", size = 2738203 } wheels = [ - { url = "https://files.pythonhosted.org/packages/29/a2/52ef74287ec5fe0e5a0ffedde7d0809da5ec3ac85f4e3f2ed5587b39471a/fastmcp-2.10.1-py3-none-any.whl", hash = "sha256:17d0acea04eeb3464c9eca42b6774fb06b38b72cface9af6a7482b3aa561db13", size = 182108 }, + { url = "https://files.pythonhosted.org/packages/44/f6/df91b178740108bcee4c6df732369dfa9250578a979caac4cbd1e5d8b42c/fastmcp-2.10.2-py3-none-any.whl", hash = "sha256:3e5929772d5d22bad03581c2c4db40e008926309180168d48f3a7eac678ae645", size = 185331 }, ] [[package]] @@ -654,7 +654,7 @@ requires-dist = [ { name = "click", specifier = ">=8.1.3" }, { name = "deprecated", specifier = ">=1.2.18" }, { name = "duckdb", specifier = ">=1.2.2" }, - { name = "fastmcp", specifier = ">=2.0.0" }, + { name = "fastmcp", specifier = ">=2.10.2" }, { name = "inquirerpy", specifier = ">=0.3.4" }, { name = "mcp", specifier = ">=1.8.0" }, { name = "prompt-toolkit", specifier = ">=3.0.0" }, From 278334cffed3bffc14b2aca0d91d8760af4ba9b7 Mon Sep 17 00:00:00 2001 From: User Date: Tue, 8 Jul 2025 19:07:33 -0700 Subject: [PATCH 3/8] fix: suppress FastMCP banner in run command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hide the FastMCP startup banner by setting show_banner=False for both stdio and HTTP modes. This provides a cleaner user experience without the verbose FastMCP branding output. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/mcpm/commands/run.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mcpm/commands/run.py b/src/mcpm/commands/run.py index 3f398b7..e6cd307 100644 --- a/src/mcpm/commands/run.py +++ b/src/mcpm/commands/run.py @@ -73,12 +73,12 @@ async def run_server_with_fastmcp(server_config, server_name, http_mode=False, p # Run FastMCP proxy in HTTP mode with uvicorn logging control await proxy.run_http_async( - host="127.0.0.1", port=actual_port, uvicorn_config={"log_level": get_uvicorn_log_level()} + host="127.0.0.1", port=actual_port, show_banner=False, uvicorn_config={"log_level": get_uvicorn_log_level()} ) else: # Run FastMCP proxy in stdio mode (default) logger.info(f"Starting server '{server_name}' over stdio") - await proxy.run_stdio_async() + await proxy.run_stdio_async(show_banner=False) return 0 From e39adafb1fb46b2fa6e7ad01a396744d0f934b93 Mon Sep 17 00:00:00 2001 From: cnie Date: Wed, 9 Jul 2025 13:37:54 +0800 Subject: [PATCH 4/8] Update src/mcpm/schemas/full_server_config.py Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com> --- src/mcpm/schemas/full_server_config.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mcpm/schemas/full_server_config.py b/src/mcpm/schemas/full_server_config.py index 7a527f9..7c2697c 100644 --- a/src/mcpm/schemas/full_server_config.py +++ b/src/mcpm/schemas/full_server_config.py @@ -72,8 +72,9 @@ def to_dict(self) -> Dict: def to_server_config(self) -> ServerConfig: """Convert FullServerConfig to a common server configuration format""" # Check if this is an HTTP server - if self.installation == "http" and self.url: - # For HTTP servers, use the headers field directly + if self.installation == "http": + if not self.url: + raise ValueError(f"URL is required for HTTP server '{self.name}'") return RemoteServerConfig(name=self.name, url=self.url, headers=self.headers) else: # Default to STDIO for all other cases From 0d5a8de9cff2ff8ecd50e5eef8d628ee929b5728 Mon Sep 17 00:00:00 2001 From: User Date: Tue, 8 Jul 2025 23:17:26 -0700 Subject: [PATCH 5/8] feat: remove debug option from CLI commands and keep middleware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove --debug option from mcpm run and mcpm profile run commands - Remove debug parameter from proxy factory and convenience functions - Keep MCPMDebugMiddleware class for future use - Add --host option to both run commands for network binding flexibility - Fix type annotations in middleware and proxy for Optional parameters - Remove unused imports and variables 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- ...mcp_progress_notification_investigation.md | 325 ++++++++++++++++++ pyproject.toml | 2 +- src/mcpm/commands/profile/run.py | 24 +- src/mcpm/commands/run.py | 28 +- src/mcpm/fastmcp_integration/middleware.py | 131 ++++++- src/mcpm/fastmcp_integration/proxy.py | 22 +- uv.lock | 2 +- 7 files changed, 499 insertions(+), 35 deletions(-) create mode 100644 docs/fastmcp_progress_notification_investigation.md diff --git a/docs/fastmcp_progress_notification_investigation.md b/docs/fastmcp_progress_notification_investigation.md new file mode 100644 index 0000000..46fc239 --- /dev/null +++ b/docs/fastmcp_progress_notification_investigation.md @@ -0,0 +1,325 @@ +# FastMCP Progress Notification Investigation + +**Date**: July 8, 2025 +**Issue**: Progress notifications not working when FastMCP proxy uses HTTP-to-Stdio transport +**Status**: Root cause identified, architectural limitation in FastMCP + +## Executive Summary + +Progress notifications work correctly when FastMCP proxy communicates with servers via HTTP-to-HTTP transport, but fail when using HTTP-to-Stdio transport. The issue is a context isolation problem in FastMCP's proxy implementation where progress tokens cannot be properly tracked across process boundaries. + +## Issue Description + +### Observed Behavior + +When running a server through MCPM's FastMCP proxy: + +**Working Case (HTTP → Proxy → HTTP):** +- Progress notifications flow correctly from server to client +- Progress tokens are preserved throughout the request lifecycle + +**Failing Case (HTTP → Proxy → Stdio):** +- Progress notifications are received by proxy but fail to forward to client +- Error: `[CONTEXT] No progress token available, skipping notification` + +### Error Logs + +``` +2025-07-08 20:22:36,003 - timer.main - DEBUG - Sending progress notification: 10000/180000 ms +[07/08/25 20:22:36] INFO FastMCP.fastmcp.server.proxy: received progress notification: progress=10000.0, total=180000.0, message=None + DEBUG fastmcp.server.context: [CONTEXT] Reporting progress: progress=10000.0, total=180000.0, message=None + DEBUG fastmcp.server.context: [CONTEXT] No progress token available, skipping notification +``` + +## Technical Root Cause Analysis + +### MCP Protocol Support + +The MCP protocol **correctly supports** progress notifications across all transports: + +1. **Progress Token Propagation**: MCP passes progress tokens in `_meta.progressToken` field +2. **Request Metadata**: Found in `/mcp/types.py:43-50`: + ```python + class RequestParams(BaseModel): + class Meta(BaseModel): + progressToken: ProgressToken | None = None + """ + If specified, the caller requests out-of-band progress notifications for + this request (as represented by notifications/progress). The value of this + parameter is an opaque token that will be attached to any subsequent + notifications. + """ + ``` + +3. **Session Implementation**: In `/mcp/shared/session.py:443+`, `send_request` properly sets progress tokens: + ```python + # Set up progress token if progress callback is provided + if progress_callback is not None: + # Use request_id as progress token + if "_meta" not in request_data["params"]: + request_data["params"]["_meta"] = {} + request_data["params"]["_meta"]["progressToken"] = request_id + # Store the callback for this request + self._progress_callbacks[request_id] = progress_callback + ``` + +### FastMCP Implementation Details + +**Progress Handler Location**: `/fastmcp/server/proxy.py:514-539` +```python +@classmethod +async def default_progress_handler( + cls, + progress: float, + total: float | None, + message: str | None, +) -> None: + """ + A handler that forwards the progress notification from the remote server to the proxy's connected clients. + """ + ctx = get_context() + logger.info("received progress notification: progress=%s, total=%s, message=%s", progress, total, message) + await ctx.report_progress(progress, total, message) +``` + +**Context Report Progress**: `/fastmcp/server/context.py:125-155` +```python +async def report_progress( + self, progress: float, total: float | None = None, message: str | None = None +) -> None: + progress_token = ( + self.request_context.meta.progressToken + if self.request_context.meta + else None + ) + + if progress_token is None: + logger.debug("[CONTEXT] No progress token available, skipping notification") + return + + await self.session.send_progress_notification( + progress_token=progress_token, + progress=progress, + total=total, + message=message, + related_request_id=self.request_id, + ) +``` + +### Context Isolation Problem + +**HTTP-to-HTTP Flow (✅ Working):** +``` +Client Request (HTTP) → Proxy → HTTP Server +├─ Request context with progressToken flows through HTTP session +├─ Progress handler executes in same HTTP context +└─ Progress notifications reference original progressToken +``` + +**HTTP-to-Stdio Flow (❌ Broken):** +``` +Client Request (HTTP) → Proxy → Stdio Subprocess +├─ Request context with progressToken sent to subprocess +├─ Subprocess runs in isolated process with separate context +├─ Progress handler (`default_progress_handler`) runs in subprocess context +├─ `get_context()` returns subprocess context (no original progressToken) +└─ Progress notification dropped due to missing token +``` + +### Key Technical Components + +1. **Context Management**: `/fastmcp/server/dependencies.py:27-33` + ```python + def get_context() -> Context: + from fastmcp.server.context import _current_context + context = _current_context.get() + if context is None: + raise RuntimeError("No active context found.") + return context + ``` + +2. **Context Variables**: Uses Python `ContextVar` which is isolated per async task/process + +3. **Stdio Transport**: `/fastmcp/client/transports.py:300+` - Creates subprocess with separate memory space + +## Debug Implementation Added + +### Debug Middleware + +Created `MCPMDebugMiddleware` in `/src/mcpm/fastmcp_integration/middleware.py:19-198`: + +- **Limitation Documented**: Middleware only intercepts messages flowing FROM clients TO servers +- **Progress notifications flow FROM servers TO clients**, bypassing middleware pipeline +- **Added debug flags** to `mcpm run --debug` and `mcpm profile run --debug` + +### Enhanced Logging + +Added debug logging in multiple layers: + +1. **Proxy Progress Handler**: `/fastmcp/server/proxy.py:524+` +2. **Context Report Progress**: `/fastmcp/server/context.py:134-147` +3. **Debug context inspection** in progress handler + +## Architectural Limitation + +### Why This Is Hard to Fix + +FastMCP's proxy architecture has a fundamental limitation: + +1. **Process Isolation**: Stdio servers run in separate processes with isolated memory +2. **Context Variables**: `_current_context` is per-process, not shared +3. **Missing Mapping**: No mechanism to associate stdio server progress with original client requests + +### Required Fix + +To properly support HTTP-to-Stdio progress notifications, FastMCP would need: + +1. **Request Tracking**: Maintain mapping between client requests and stdio subprocess instances +2. **Token Preservation**: Store original progress tokens when spawning stdio requests +3. **Context Injection**: Inject correct progress token when handling progress from stdio servers + +Example fix architecture: +```python +class ProxyRequestTracker: + def __init__(self): + self._request_mapping: Dict[subprocess_id, ProgressToken] = {} + + def track_request(self, subprocess_id: str, progress_token: ProgressToken): + self._request_mapping[subprocess_id] = progress_token + + def get_progress_token(self, subprocess_id: str) -> ProgressToken | None: + return self._request_mapping.get(subprocess_id) +``` + +## Current Workarounds + +### For Users + +1. **Use HTTP servers** when progress notifications are needed: + ```bash + mcpm run server-name --http --port 8001 + ``` + +2. **Debug with HTTP-to-HTTP** setup: + ```bash + mcpm run timer --http --debug + ``` + +### For Development + +1. **Enable comprehensive debug logging**: + ```bash + export PYTHONUNBUFFERED=1 + export LOG_LEVEL=DEBUG + mcpm run server-name --debug + ``` + +2. **Use enhanced logging** in FastMCP components (already added) + +## Files Modified During Investigation + +### MCPM Code Changes + +1. **Debug Middleware**: `/src/mcpm/fastmcp_integration/middleware.py` + - Added `MCPMDebugMiddleware` class with comprehensive logging + - Handles all MCP operations: tools, resources, prompts, notifications + - Safe serialization for complex objects + +2. **Proxy Factory**: `/src/mcpm/fastmcp_integration/proxy.py` + - Added `debug: bool` parameter to `MCPMProxyFactory` + - Debug middleware added first in middleware chain when enabled + - Updated convenience function `create_mcpm_proxy()` + +3. **CLI Commands**: + - `/src/mcpm/commands/run.py`: Added `--debug` flag + - `/src/mcpm/commands/profile/run.py`: Added `--debug` flag + - Debug parameter propagated through all execution paths + +### FastMCP Library Changes (for debugging) + +1. **Proxy Progress Handler**: `/fastmcp/server/proxy.py:524` + - Fixed logging syntax error (string formatting) + - Added comprehensive debug context inspection + +2. **Context Report Progress**: `/fastmcp/server/context.py:134-147` + - Added detailed debug logging for progress reporting + - Logs progress token availability and notification sending + +## Testing and Validation + +### Confirmed Working Cases + +- **HTTP-to-HTTP**: Progress notifications work correctly +- **Local servers**: Progress works when not proxied +- **Debug logging**: All layers now provide detailed debugging information + +### Confirmed Failing Cases + +- **HTTP-to-Stdio**: Progress notifications fail due to context isolation +- **Any proxied stdio server**: Same issue across all stdio transport types + +### Debug Output Examples + +**Working HTTP-to-HTTP**: +``` +[PROXY DEBUG] TOOL CALL: timer_tool +[CONTEXT] Reporting progress: progress=10000.0, total=180000.0, message=None +[CONTEXT] Sending progress notification with token=12345 +``` + +**Failing HTTP-to-Stdio**: +``` +[PROXY DEBUG] TOOL CALL: timer_tool +INFO: received progress notification: progress=10000.0, total=180000.0, message=None +[CONTEXT] Reporting progress: progress=10000.0, total=180000.0, message=None +[CONTEXT] No progress token available, skipping notification +``` + +## Future Work Recommendations + +### FastMCP Core Fix + +The proper solution requires changes to FastMCP core: + +1. **Enhance ProxyClient**: Maintain request-to-subprocess mapping +2. **Modify default_progress_handler**: Inject correct progress token from mapping +3. **Add request lifecycle tracking**: Clean up mappings when requests complete + +### Alternative Approaches + +1. **HTTP Wrapper Pattern**: Create HTTP servers that wrap stdio servers +2. **Progress Relay Service**: Separate service to bridge context gaps +3. **Enhanced Middleware**: Middleware that can intercept outgoing notifications + +### Testing Strategy + +1. **Unit tests** for request tracking components +2. **Integration tests** for HTTP-to-Stdio progress scenarios +3. **Performance testing** for overhead of request tracking + +## References + +### Key Files Investigated + +- `/mcp/types.py`: MCP protocol definitions including progress tokens +- `/mcp/shared/session.py`: Base session with progress token handling +- `/mcp/client/session.py`: Client session implementation +- `/fastmcp/server/proxy.py`: Proxy server and progress handlers +- `/fastmcp/server/context.py`: Request context and progress reporting +- `/fastmcp/server/dependencies.py`: Context management and retrieval +- `/fastmcp/client/transports.py`: Transport implementations including stdio + +### FastMCP Version + +- **Version**: 2.10.2 (from `pyproject.toml`) +- **Library location**: `.venv/lib/python3.13/site-packages/fastmcp/` + +### Related Issues + +This issue demonstrates a broader challenge in MCP proxy implementations: maintaining request context across different transport types. Similar issues may exist in other proxy scenarios where context isolation occurs. + +--- + +**Investigation completed by**: Claude Code +**Next steps**: Resume work on implementing FastMCP core fix or alternative workarounds +**Priority**: Medium (affects progress notification functionality, workarounds available) \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 8477b7f..99e5f6a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ dependencies = [ "requests>=2.28.0", "pydantic>=2.5.1", "mcp>=1.8.0", - "fastmcp>=2.10.2", + "fastmcp==2.10.2", "ruamel-yaml>=0.18.10", "watchfiles>=1.0.4", "duckdb>=1.2.2", diff --git a/src/mcpm/commands/profile/run.py b/src/mcpm/commands/profile/run.py index 1fb242f..1a2ffc9 100644 --- a/src/mcpm/commands/profile/run.py +++ b/src/mcpm/commands/profile/run.py @@ -42,7 +42,9 @@ async def find_available_port(preferred_port, max_attempts=10): return preferred_port -async def run_profile_fastmcp(profile_servers, profile_name, http_mode=False, port=DEFAULT_PORT): +async def run_profile_fastmcp( + profile_servers, profile_name, http_mode=False, port=DEFAULT_PORT, host="127.0.0.1" +): """Run profile servers using FastMCP proxy for proper aggregation.""" server_count = len(profile_servers) logger.debug(f"Using FastMCP proxy to aggregate {server_count} server(s)") @@ -75,7 +77,7 @@ async def run_profile_fastmcp(profile_servers, profile_name, http_mode=False, po logger.debug(f"Port {port} is busy, using port {actual_port} instead") # Display profile information in a nice panel - http_url = f"http://127.0.0.1:{actual_port}/mcp/" + http_url = f"http://{host}:{actual_port}/mcp/" # Build server list server_list = "\n".join([f" • [cyan]{server.name}[/]" for server in profile_servers]) @@ -91,11 +93,11 @@ async def run_profile_fastmcp(profile_servers, profile_name, http_mode=False, po ) console.print(panel) - logger.debug(f"Starting FastMCP proxy for profile '{profile_name}' on port {actual_port}") + logger.debug(f"Starting FastMCP proxy for profile '{profile_name}' on {host}:{actual_port}") # Run the aggregated proxy over HTTP with uvicorn logging control await proxy.run_http_async( - host="127.0.0.1", port=actual_port, uvicorn_config={"log_level": get_uvicorn_log_level()} + host=host, port=actual_port, uvicorn_config={"log_level": get_uvicorn_log_level()} ) else: # Run the aggregated proxy over stdio (default) @@ -116,8 +118,9 @@ async def run_profile_fastmcp(profile_servers, profile_name, http_mode=False, po @click.argument("profile_name") @click.option("--http", is_flag=True, help="Run profile over HTTP instead of stdio") @click.option("--port", type=int, default=DEFAULT_PORT, help=f"Port for HTTP mode (default: {DEFAULT_PORT})") +@click.option("--host", type=str, default="127.0.0.1", help="Host address for HTTP mode (default: 127.0.0.1)") @click.help_option("-h", "--help") -def run(profile_name, http, port): +def run(profile_name, http, port, host): """Execute all servers in a profile over stdio or HTTP. Uses FastMCP proxy to aggregate servers into a unified MCP interface @@ -126,9 +129,10 @@ def run(profile_name, http, port): Examples: \b - mcpm profile run web-dev # Run over stdio (default) - mcpm profile run --http web-dev # Run over HTTP on port 6276 - mcpm profile run --http --port 9000 ai # Run over HTTP on port 9000 + mcpm profile run web-dev # Run over stdio (default) + mcpm profile run --http web-dev # Run over HTTP on 127.0.0.1:6276 + mcpm profile run --http --port 9000 ai # Run over HTTP on 127.0.0.1:9000 + mcpm profile run --http --host 0.0.0.0 web-dev # Run over HTTP on 0.0.0.0:6276 Debug logging: Set MCPM_DEBUG=1 for verbose output """ @@ -171,4 +175,6 @@ def run(profile_name, http, port): logger.debug(f"HTTP mode on port {port}") # Run the async function - return asyncio.run(run_profile_fastmcp(profile_servers, profile_name, http_mode=http, port=port)) + return asyncio.run( + run_profile_fastmcp(profile_servers, profile_name, http_mode=http, port=port, host=host) + ) diff --git a/src/mcpm/commands/run.py b/src/mcpm/commands/run.py index e6cd307..5f3191f 100644 --- a/src/mcpm/commands/run.py +++ b/src/mcpm/commands/run.py @@ -32,7 +32,9 @@ def find_installed_server(server_name): return None, None -async def run_server_with_fastmcp(server_config, server_name, http_mode=False, port=None): +async def run_server_with_fastmcp( + server_config, server_name, http_mode=False, port=None, host="127.0.0.1" +): """Run server using FastMCP proxy (stdio or HTTP).""" try: # Use default port if none specified @@ -62,18 +64,18 @@ async def run_server_with_fastmcp(server_config, server_name, http_mode=False, p logger.debug(f"Port {port} is busy, using port {actual_port} instead") # Display server information in a nice panel - http_url = f"http://127.0.0.1:{actual_port}/mcp/" + http_url = f"http://{host}:{actual_port}/mcp/" panel_content = f"[bold]Server:[/] {server_name}\n[bold]URL:[/] [cyan]{http_url}[/cyan]\n\n[dim]Press Ctrl+C to stop the server[/]" panel = Panel( panel_content, title="🌐 Local Server Running", title_align="left", border_style="green", padding=(1, 2) ) console.print(panel) - logger.debug(f"Starting FastMCP proxy for server '{server_name}' on port {actual_port}") + logger.debug(f"Starting FastMCP proxy for server '{server_name}' on {host}:{actual_port}") # Run FastMCP proxy in HTTP mode with uvicorn logging control await proxy.run_http_async( - host="127.0.0.1", port=actual_port, show_banner=False, uvicorn_config={"log_level": get_uvicorn_log_level()} + host=host, port=actual_port, show_banner=False, uvicorn_config={"log_level": get_uvicorn_log_level()} ) else: # Run FastMCP proxy in stdio mode (default) @@ -115,17 +117,19 @@ async def find_available_port(preferred_port, max_attempts=10): @click.argument("server_name") @click.option("--http", is_flag=True, help="Run server over HTTP instead of stdio") @click.option("--port", type=int, default=DEFAULT_PORT, help=f"Port for HTTP mode (default: {DEFAULT_PORT})") +@click.option("--host", type=str, default="127.0.0.1", help="Host address for HTTP mode (default: 127.0.0.1)") @click.help_option("-h", "--help") -def run(server_name, http, port): +def run(server_name, http, port, host): """Execute a server from global configuration over stdio or HTTP. Runs an installed MCP server from the global configuration. By default runs over stdio for client communication, but can run over HTTP with --http. Examples: - mcpm run mcp-server-browse # Run over stdio (default) - mcpm run --http mcp-server-browse # Run over HTTP on port 6276 - mcpm run --http --port 9000 filesystem # Run over HTTP on port 9000 + mcpm run mcp-server-browse # Run over stdio (default) + mcpm run --http mcp-server-browse # Run over HTTP on 127.0.0.1:6276 + mcpm run --http --port 9000 filesystem # Run over HTTP on 127.0.0.1:9000 + mcpm run --http --host 0.0.0.0 filesystem # Run over HTTP on 0.0.0.0:6276 Note: stdio mode is typically used in MCP client configurations: {"command": ["mcpm", "run", "mcp-server-browse"]} @@ -169,9 +173,13 @@ def run(server_name, http, port): # Choose execution method if http: # Use FastMCP proxy for HTTP mode - exit_code = asyncio.run(run_server_with_fastmcp(server_config, server_name, http_mode=True, port=port)) + exit_code = asyncio.run( + run_server_with_fastmcp(server_config, server_name, http_mode=True, port=port, host=host) + ) else: # Use FastMCP proxy for stdio mode (enables middleware and usage tracking) - exit_code = asyncio.run(run_server_with_fastmcp(server_config, server_name, http_mode=False, port=port)) + exit_code = asyncio.run( + run_server_with_fastmcp(server_config, server_name, http_mode=False, port=port, host=host) + ) sys.exit(exit_code) diff --git a/src/mcpm/fastmcp_integration/middleware.py b/src/mcpm/fastmcp_integration/middleware.py index 376558b..2c58756 100644 --- a/src/mcpm/fastmcp_integration/middleware.py +++ b/src/mcpm/fastmcp_integration/middleware.py @@ -2,16 +2,141 @@ FastMCP middleware adapters for MCPM monitoring and authentication. """ +import json +import logging import time import uuid +from typing import Any -from fastmcp.server.middleware import Middleware +import mcp.types as mt +from fastmcp.server.middleware import CallNext, Middleware, MiddlewareContext from mcpm.monitor.base import AccessEventType, AccessMonitor, SessionSource, SessionTransport # MCPMMonitoringMiddleware removed - functionality moved to MCPMUnifiedTrackingMiddleware +class MCPMDebugMiddleware(Middleware): + """Debug middleware that logs all proxy activity including notifications when debug is enabled. + + Note: This middleware intercepts messages flowing FROM clients TO servers through the proxy. + Progress notifications that flow FROM servers TO clients are handled differently and will + not appear in the middleware logs. To debug progress notifications, check the FastMCP + proxy logs and the Context.report_progress debug messages. + """ + + def __init__(self): + self.logger = logging.getLogger(__name__) + + async def on_message( + self, + context: MiddlewareContext[Any], + call_next: CallNext[Any, Any], + ) -> Any: + """Log all messages (requests and notifications) with basic information.""" + message_type = context.type + method = context.method or "unknown" + source = context.source + + self.logger.debug(f"[PROXY DEBUG] {message_type.upper()} - {method} from {source}") + + try: + result = await call_next(context) + return result + + except Exception as e: + self.logger.debug(f"[PROXY DEBUG] Error in {method}: {type(e).__name__}: {e}") + raise + + async def on_notification( + self, + context: MiddlewareContext[mt.Notification], + call_next: CallNext[mt.Notification, Any], + ) -> Any: + """Log notification details including progress notifications.""" + notification = context.message + method = notification.method if hasattr(notification, "method") else "unknown" + + # Only log progress notifications with details, others just basic info + if method == "notifications/progress": + params = getattr(notification, "params", None) + if params: + progress = getattr(params, "progress", "unknown") + total = getattr(params, "total", "unknown") + self.logger.debug(f"[PROXY DEBUG] Progress notification: {progress}/{total}") + + return await call_next(context) + + async def on_call_tool( + self, + context: MiddlewareContext[mt.CallToolRequestParams], + call_next: CallNext[mt.CallToolRequestParams, mt.CallToolResult], + ) -> mt.CallToolResult: + """Log tool invocations with timing information.""" + params = context.message + tool_name = params.name if hasattr(params, "name") else "unknown" + + start_time = time.time() + self.logger.debug(f"[PROXY DEBUG] TOOL CALL: {tool_name}") + + try: + result = await call_next(context) + duration = (time.time() - start_time) * 1000 + self.logger.debug(f"[PROXY DEBUG] Tool {tool_name} completed in {duration:.2f}ms") + return result + + except Exception as e: + duration = (time.time() - start_time) * 1000 + self.logger.debug(f"[PROXY DEBUG] Tool {tool_name} failed after {duration:.2f}ms: {e}") + raise + + async def on_read_resource( + self, + context: MiddlewareContext[mt.ReadResourceRequestParams], + call_next: CallNext[mt.ReadResourceRequestParams, mt.ReadResourceResult], + ) -> mt.ReadResourceResult: + """Log resource access with timing information.""" + params = context.message + uri = params.uri if hasattr(params, "uri") else "unknown" + + start_time = time.time() + self.logger.debug(f"[PROXY DEBUG] RESOURCE READ: {uri}") + + try: + result = await call_next(context) + duration = (time.time() - start_time) * 1000 + self.logger.debug(f"[PROXY DEBUG] Resource {uri} read in {duration:.2f}ms") + return result + + except Exception as e: + duration = (time.time() - start_time) * 1000 + self.logger.debug(f"[PROXY DEBUG] Resource {uri} failed after {duration:.2f}ms: {e}") + raise + + async def on_get_prompt( + self, + context: MiddlewareContext[mt.GetPromptRequestParams], + call_next: CallNext[mt.GetPromptRequestParams, mt.GetPromptResult], + ) -> mt.GetPromptResult: + """Log prompt execution with timing information.""" + params = context.message + prompt_name = params.name if hasattr(params, "name") else "unknown" + + start_time = time.time() + self.logger.debug(f"[PROXY DEBUG] PROMPT GET: {prompt_name}") + + try: + result = await call_next(context) + duration = (time.time() - start_time) * 1000 + self.logger.debug(f"[PROXY DEBUG] Prompt {prompt_name} executed in {duration:.2f}ms") + return result + + except Exception as e: + duration = (time.time() - start_time) * 1000 + self.logger.debug(f"[PROXY DEBUG] Prompt {prompt_name} failed after {duration:.2f}ms: {e}") + raise + + class MCPMAuthMiddleware(Middleware): """FastMCP middleware that integrates with MCPM's authentication system.""" @@ -86,9 +211,9 @@ class MCPMUnifiedTrackingMiddleware(Middleware): def __init__( self, access_monitor: AccessMonitor, - server_name: str = None, + server_name: str | None = None, action: str = "proxy", - profile_name: str = None, + profile_name: str | None = None, transport: SessionTransport = SessionTransport.HTTP, ): self.monitor = access_monitor diff --git a/src/mcpm/fastmcp_integration/proxy.py b/src/mcpm/fastmcp_integration/proxy.py index 7e8203f..8c4b7be 100644 --- a/src/mcpm/fastmcp_integration/proxy.py +++ b/src/mcpm/fastmcp_integration/proxy.py @@ -2,6 +2,7 @@ FastMCP proxy factory for MCPM server aggregation. """ +import logging from typing import Any, Dict, List, Optional from fastmcp import FastMCP @@ -19,6 +20,8 @@ # from .config import create_mcp_config, create_stdio_server_config, create_remote_server_config from .middleware import MCPMAuthMiddleware, MCPMUnifiedTrackingMiddleware +logger = logging.getLogger(__name__) + class MCPMProxyFactory: """Factory for creating FastMCP proxies with MCPM integration.""" @@ -85,7 +88,6 @@ async def create_proxy_for_servers( # Ensure all environment values are strings (server.env is Dict[str, str]) env_config.update(server.env) - config: Dict[str, Any] = {"command": command_parts, "env": env_config} server_configs[server.name] = StdioMCPServer( command=server.command, args=server.args or [], @@ -93,14 +95,11 @@ async def create_proxy_for_servers( ) elif isinstance(server, RemoteServerConfig): # RemoteServerConfig - HTTP/SSE transport - config: Dict[str, Any] = {"url": server.url} - if server.headers: - # Convert all header values to strings (FastMCP expects Dict[str, str]) - string_headers = {k: str(v) for k, v in server.headers.items()} - config["headers"] = string_headers + # Convert all header values to strings (FastMCP expects Dict[str, str]) + string_headers = {k: str(v) for k, v in server.headers.items()} if server.headers else {} server_configs[server.name] = RemoteMCPServer( url=server.url, - headers=server.headers or {}, + headers=string_headers, ) elif isinstance(server, CustomServerConfig): # CustomServerConfig is for non-standard client configs - skip it @@ -150,11 +149,12 @@ def _add_mcpm_middleware( self, proxy: FastMCP, stdio_mode: bool = True, - server_name: str = None, + server_name: Optional[str] = None, action: str = "proxy", - profile_name: str = None, + profile_name: Optional[str] = None, ) -> None: """Add MCPM-specific middleware to the proxy.""" + # Add unified tracking middleware (replaces both monitoring and usage tracking) if self.access_monitor: transport = SessionTransport.STDIO if stdio_mode else SessionTransport.HTTP @@ -168,10 +168,10 @@ def _add_mcpm_middleware( proxy.add_middleware(unified_middleware) # Store reference for cleanup - proxy._mcpm_unified_middleware = unified_middleware + setattr(proxy, "_mcpm_unified_middleware", unified_middleware) # Add authentication middleware (only for HTTP/network operations, not stdio) - if self.auth_enabled and not stdio_mode: + if self.auth_enabled and not stdio_mode and self.api_key: proxy.add_middleware(MCPMAuthMiddleware(self.api_key)) diff --git a/uv.lock b/uv.lock index 9c4a220..55cad57 100644 --- a/uv.lock +++ b/uv.lock @@ -654,7 +654,7 @@ requires-dist = [ { name = "click", specifier = ">=8.1.3" }, { name = "deprecated", specifier = ">=1.2.18" }, { name = "duckdb", specifier = ">=1.2.2" }, - { name = "fastmcp", specifier = ">=2.10.2" }, + { name = "fastmcp", specifier = "==2.10.2" }, { name = "inquirerpy", specifier = ">=0.3.4" }, { name = "mcp", specifier = ">=1.8.0" }, { name = "prompt-toolkit", specifier = ">=3.0.0" }, From 65ac79192b57753df3dc495310e45608f44fa0d7 Mon Sep 17 00:00:00 2001 From: User Date: Wed, 9 Jul 2025 00:06:15 -0700 Subject: [PATCH 6/8] fix: reduce log level for missing client config files from warning to debug Missing client config files are normal when clients aren't installed. This reduces noise in the default log output while still providing debug information when needed. --- src/mcpm/clients/base.py | 4 ++-- src/mcpm/commands/client.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/mcpm/clients/base.py b/src/mcpm/clients/base.py index 48d7685..6caf118 100644 --- a/src/mcpm/clients/base.py +++ b/src/mcpm/clients/base.py @@ -186,7 +186,7 @@ def _load_config(self) -> Dict[str, Any]: empty_config = {self.configure_key_name: {}} if not os.path.exists(self.config_path): - logger.warning(f"Client config file not found at: {self.config_path}") + logger.debug(f"Client config file not found at: {self.config_path}") return empty_config try: @@ -403,7 +403,7 @@ def _load_config(self) -> Dict[str, Any]: empty_config = self._get_empty_config() if not os.path.exists(self.config_path): - logger.warning(f"Client config file not found at: {self.config_path}") + logger.debug(f"Client config file not found at: {self.config_path}") return empty_config try: diff --git a/src/mcpm/commands/client.py b/src/mcpm/commands/client.py index 82d7e0a..c6409ce 100644 --- a/src/mcpm/commands/client.py +++ b/src/mcpm/commands/client.py @@ -194,13 +194,14 @@ def list_clients(verbose): console.print(table) console.print() - # Show uninstalled clients in compact format + # Show uninstalled clients in compact format with client codes if uninstalled_client_names: uninstalled_display_names = [] for client_name in sorted(uninstalled_client_names): client_info = ClientRegistry.get_client_info(client_name) display_name = client_info.get("name", client_name) - uninstalled_display_names.append(display_name) + # Include client code for undetected clients + uninstalled_display_names.append(f"{display_name} ({client_name})") console.print(f"[dim]Additional supported clients (not detected): {', '.join(uninstalled_display_names)}[/]") console.print() @@ -242,9 +243,11 @@ def edit_client(client_name, external, config_path_override): display_name = client_info.get("name", client_name) # Check if the client is installed - if not client_manager.is_client_installed(): - print_error(f"{display_name} installation not detected.") - return + client_is_installed = client_manager.is_client_installed() + if not client_is_installed: + console.print(f"[yellow]⚠️ {display_name} installation not detected.[/]") + console.print(f"[yellow]Config file will be created at: {client_manager.config_path}[/]") + console.print(f"[dim]You can still configure servers, but make sure to install {display_name} later.[/]\n") # Get the client config file path config_path = client_manager.config_path From 0b15cd45cdf5dd72e369b2eb1bdfb85f95353f47 Mon Sep 17 00:00:00 2001 From: User Date: Wed, 9 Jul 2025 00:35:20 -0700 Subject: [PATCH 7/8] test: fix client edit test for undetected clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update test_client_edit_command_client_not_installed to match current behavior where undetected clients show warning messages instead of calling print_error. The command now continues with interactive selection after showing appropriate warnings about config file creation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- tests/test_client.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index 4832ebd..8b279a5 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -294,15 +294,15 @@ def test_client_edit_command_client_not_installed(monkeypatch): monkeypatch.setattr(ClientRegistry, "get_client_manager", Mock(return_value=mock_client_manager)) monkeypatch.setattr(ClientRegistry, "get_client_info", Mock(return_value={"name": "Windsurf"})) - # Mock print_error - with patch("mcpm.commands.client.print_error") as mock_print_error: - # Run the command - runner = CliRunner() - result = runner.invoke(edit_client, ["windsurf"]) + # Run the command + runner = CliRunner() + result = runner.invoke(edit_client, ["windsurf"]) - # Check the result - assert result.exit_code == 0 - mock_print_error.assert_called_once_with("Windsurf installation not detected.") + # Check the result - should show warning but continue with interactive selection + assert result.exit_code == 0 + assert "⚠️ Windsurf installation not detected." in result.output + assert "Config file will be created at: /path/to/config.json" in result.output + assert "You can still configure servers, but make sure to install Windsurf later." in result.output def test_client_edit_command_config_exists(monkeypatch, tmp_path): From 87dbb5b7a78231d1e449d818933b100fe4b984d6 Mon Sep 17 00:00:00 2001 From: User Date: Wed, 9 Jul 2025 00:38:13 -0700 Subject: [PATCH 8/8] style: apply code formatting fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply ruff formatting fixes to multiple files: - Fix import ordering in install.py - Format long function signatures in run.py and profile/run.py - Remove unused imports in middleware.py and proxy.py - Ensure consistent code style across the codebase 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/mcpm/commands/install.py | 4 ++-- src/mcpm/commands/profile/run.py | 8 ++------ src/mcpm/commands/run.py | 4 +--- src/mcpm/fastmcp_integration/middleware.py | 1 - src/mcpm/fastmcp_integration/proxy.py | 2 +- 5 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/mcpm/commands/install.py b/src/mcpm/commands/install.py index 23f7374..f526a41 100644 --- a/src/mcpm/commands/install.py +++ b/src/mcpm/commands/install.py @@ -15,11 +15,11 @@ from rich.progress import Progress, SpinnerColumn, TextColumn from rich.prompt import Confirm -from mcpm.global_config import GlobalConfigManager from mcpm.core.schema import ServerConfig, STDIOServerConfig -from mcpm.utils.config import NODE_EXECUTABLES, ConfigManager +from mcpm.global_config import GlobalConfigManager from mcpm.profile.profile_config import ProfileConfigManager from mcpm.schemas.full_server_config import FullServerConfig +from mcpm.utils.config import NODE_EXECUTABLES, ConfigManager from mcpm.utils.repository import RepositoryManager from mcpm.utils.rich_click_config import click diff --git a/src/mcpm/commands/profile/run.py b/src/mcpm/commands/profile/run.py index 1a2ffc9..002f715 100644 --- a/src/mcpm/commands/profile/run.py +++ b/src/mcpm/commands/profile/run.py @@ -42,9 +42,7 @@ async def find_available_port(preferred_port, max_attempts=10): return preferred_port -async def run_profile_fastmcp( - profile_servers, profile_name, http_mode=False, port=DEFAULT_PORT, host="127.0.0.1" -): +async def run_profile_fastmcp(profile_servers, profile_name, http_mode=False, port=DEFAULT_PORT, host="127.0.0.1"): """Run profile servers using FastMCP proxy for proper aggregation.""" server_count = len(profile_servers) logger.debug(f"Using FastMCP proxy to aggregate {server_count} server(s)") @@ -175,6 +173,4 @@ def run(profile_name, http, port, host): logger.debug(f"HTTP mode on port {port}") # Run the async function - return asyncio.run( - run_profile_fastmcp(profile_servers, profile_name, http_mode=http, port=port, host=host) - ) + return asyncio.run(run_profile_fastmcp(profile_servers, profile_name, http_mode=http, port=port, host=host)) diff --git a/src/mcpm/commands/run.py b/src/mcpm/commands/run.py index 5f3191f..c60bfa4 100644 --- a/src/mcpm/commands/run.py +++ b/src/mcpm/commands/run.py @@ -32,9 +32,7 @@ def find_installed_server(server_name): return None, None -async def run_server_with_fastmcp( - server_config, server_name, http_mode=False, port=None, host="127.0.0.1" -): +async def run_server_with_fastmcp(server_config, server_name, http_mode=False, port=None, host="127.0.0.1"): """Run server using FastMCP proxy (stdio or HTTP).""" try: # Use default port if none specified diff --git a/src/mcpm/fastmcp_integration/middleware.py b/src/mcpm/fastmcp_integration/middleware.py index 2c58756..5795cdb 100644 --- a/src/mcpm/fastmcp_integration/middleware.py +++ b/src/mcpm/fastmcp_integration/middleware.py @@ -2,7 +2,6 @@ FastMCP middleware adapters for MCPM monitoring and authentication. """ -import json import logging import time import uuid diff --git a/src/mcpm/fastmcp_integration/proxy.py b/src/mcpm/fastmcp_integration/proxy.py index 8c4b7be..76a34d0 100644 --- a/src/mcpm/fastmcp_integration/proxy.py +++ b/src/mcpm/fastmcp_integration/proxy.py @@ -3,7 +3,7 @@ """ import logging -from typing import Any, Dict, List, Optional +from typing import Dict, List, Optional from fastmcp import FastMCP from fastmcp.utilities.mcp_config import (