From 2df543202a539a1c4d6ffe0154b602ffa84bfe2a Mon Sep 17 00:00:00 2001 From: Zeel Date: Sat, 2 May 2026 20:52:56 -0400 Subject: [PATCH 1/3] refactor: tighten stage 1 ruff thresholds --- pyproject.toml | 10 +- src/apm_cli/commands/install.py | 79 +- src/apm_cli/install/mcp/command.py | 2 +- src/apm_cli/install/mcp/conflicts.py | 2 +- src/apm_cli/install/services.py | 2 +- src/apm_cli/install/validation.py | 2 +- src/apm_cli/integration/mcp_integrator.py | 1080 +++++++++++---------- src/apm_cli/marketplace/publisher.py | 461 ++++----- src/apm_cli/marketplace/semver.py | 21 +- src/apm_cli/policy/policy_checks.py | 40 +- 10 files changed, 923 insertions(+), 776 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 5c28cb945..2f04870f9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -92,13 +92,13 @@ ignore = [ # High initial thresholds set just above current codebase maximums. # Prevents new code from exceeding the worst existing violations. # Tighten these over time via dedicated refactoring PRs. -max-statements = 275 # current max: 269 (mcp_integrator.py::install) -max-args = 18 # current max: 16 (commands/install.py) -max-branches = 115 # current max: 108 (mcp_integrator.py::install) -max-returns = 18 # current max: 16 (marketplace/publisher.py) +max-statements = 200 # Stage 1 strangler threshold +max-args = 15 # Stage 1 strangler threshold +max-branches = 60 # Stage 1 strangler threshold +max-returns = 12 # Stage 1 strangler threshold [tool.ruff.lint.mccabe] -max-complexity = 100 # current max: 97 (mcp_integrator.py::install) +max-complexity = 50 # Stage 1 strangler threshold [tool.ruff.lint.per-file-ignores] # Subprocess calls are intentional in a CLI tool diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 30da0c163..6e7324837 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -678,7 +678,7 @@ def _validate_and_add_packages_to_apm_yml( # --------------------------------------------------------------------------- -def _handle_mcp_install( +def _handle_mcp_install( # noqa: PLR0913 *, mcp_name, transport, @@ -1753,32 +1753,46 @@ def _post_install_summary(*, logger, apm_count, mcp_count, apm_diagnostics, forc ) +@dataclasses.dataclass(frozen=True) +class _InstallApmDependenciesOptions: + force: bool = False + parallel_downloads: int = 4 + logger: "InstallLogger" = None + scope: Any = None + auth_resolver: "AuthResolver" = None + target: str = None + allow_insecure: bool = False + allow_insecure_hosts: tuple[str, ...] = () + marketplace_provenance: dict | None = None + protocol_pref: Any = None + allow_protocol_fallback: bool | None = None + no_policy: bool = False + skill_subset: tuple | None = None + skill_subset_from_cli: bool = False + legacy_skill_paths: bool = False + + @classmethod + def from_kwargs(cls, kwargs: dict[str, Any]) -> "_InstallApmDependenciesOptions": + known = {field.name for field in dataclasses.fields(cls)} + unknown = set(kwargs) - known + if unknown: + unknown_list = ", ".join(sorted(unknown)) + raise TypeError(f"unexpected install option(s): {unknown_list}") + return cls(**kwargs) + + # --------------------------------------------------------------------------- # Pipeline entry point -- thin re-export preserving the patch path # ``apm_cli.commands.install._install_apm_dependencies`` used by tests. # # The real implementation lives in ``apm_cli.install.pipeline`` (F2). # --------------------------------------------------------------------------- -def _install_apm_dependencies( # noqa: PLR0913 +def _install_apm_dependencies( apm_package: "APMPackage", update_refs: bool = False, verbose: bool = False, only_packages: "builtins.list" = None, # noqa: RUF013 - force: bool = False, - parallel_downloads: int = 4, - logger: "InstallLogger" = None, - scope=None, - auth_resolver: "AuthResolver" = None, - target: str = None, # noqa: RUF013 - allow_insecure: bool = False, - allow_insecure_hosts=(), - marketplace_provenance: dict = None, - protocol_pref=None, - allow_protocol_fallback: "bool | None" = None, - no_policy: bool = False, - skill_subset: "builtins.tuple | None" = None, - skill_subset_from_cli: bool = False, - legacy_skill_paths: bool = False, + **kwargs, ): """Thin wrapper -- builds an :class:`InstallRequest` and delegates to :class:`apm_cli.install.service.InstallService`. @@ -1794,25 +1808,26 @@ def _install_apm_dependencies( # noqa: PLR0913 from apm_cli.install.request import InstallRequest from apm_cli.install.service import InstallService + options = _InstallApmDependenciesOptions.from_kwargs(kwargs) request = InstallRequest( apm_package=apm_package, update_refs=update_refs, verbose=verbose, only_packages=only_packages, - force=force, - parallel_downloads=parallel_downloads, - logger=logger, - scope=scope, - auth_resolver=auth_resolver, - target=target, - allow_insecure=allow_insecure, - allow_insecure_hosts=allow_insecure_hosts, - marketplace_provenance=marketplace_provenance, - protocol_pref=protocol_pref, - allow_protocol_fallback=allow_protocol_fallback, - no_policy=no_policy, - skill_subset=skill_subset, - skill_subset_from_cli=skill_subset_from_cli, - legacy_skill_paths=legacy_skill_paths, + force=options.force, + parallel_downloads=options.parallel_downloads, + logger=options.logger, + scope=options.scope, + auth_resolver=options.auth_resolver, + target=options.target, + allow_insecure=options.allow_insecure, + allow_insecure_hosts=options.allow_insecure_hosts, + marketplace_provenance=options.marketplace_provenance, + protocol_pref=options.protocol_pref, + allow_protocol_fallback=options.allow_protocol_fallback, + no_policy=options.no_policy, + skill_subset=options.skill_subset, + skill_subset_from_cli=options.skill_subset_from_cli, + legacy_skill_paths=options.legacy_skill_paths, ) return InstallService().run(request) diff --git a/src/apm_cli/install/mcp/command.py b/src/apm_cli/install/mcp/command.py index 9d8d6c086..2452ead0d 100644 --- a/src/apm_cli/install/mcp/command.py +++ b/src/apm_cli/install/mcp/command.py @@ -36,7 +36,7 @@ pass -def run_mcp_install( +def run_mcp_install( # noqa: PLR0913 *, mcp_name: str, transport: str | None, diff --git a/src/apm_cli/install/mcp/conflicts.py b/src/apm_cli/install/mcp/conflicts.py index 6c8f726a2..dfb7e3cc8 100644 --- a/src/apm_cli/install/mcp/conflicts.py +++ b/src/apm_cli/install/mcp/conflicts.py @@ -24,7 +24,7 @@ ) -def validate_mcp_conflicts( +def validate_mcp_conflicts( # noqa: PLR0913 *, mcp_name: str | None, packages: Sequence[str], diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index 10edbf959..68c1f8315 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -74,7 +74,7 @@ def _deployed_path_entry( ) -def integrate_package_primitives( +def integrate_package_primitives( # noqa: PLR0913 package_info: Any, project_root: Path, *, diff --git a/src/apm_cli/install/validation.py b/src/apm_cli/install/validation.py index fae11abe5..e939e5912 100644 --- a/src/apm_cli/install/validation.py +++ b/src/apm_cli/install/validation.py @@ -133,7 +133,7 @@ def _local_path_no_markers_hint(local_dir, logger=None): _rich_echo(f" ... and {len(found) - 5} more", color="dim") -def _validate_package_exists(package, verbose=False, auth_resolver=None, logger=None): +def _validate_package_exists(package, verbose=False, auth_resolver=None, logger=None): # noqa: C901, PLR0911, PLR0915 """Validate that a package exists and is accessible on GitHub, Azure DevOps, or locally.""" import os import subprocess diff --git a/src/apm_cli/integration/mcp_integrator.py b/src/apm_cli/integration/mcp_integrator.py index 97fb40154..af0dfc807 100644 --- a/src/apm_cli/integration/mcp_integrator.py +++ b/src/apm_cli/integration/mcp_integrator.py @@ -431,7 +431,7 @@ def _check_self_defined_servers_needing_installation( # ------------------------------------------------------------------ @staticmethod - def remove_stale( + def remove_stale( # noqa: C901, PLR0912 stale_names: builtins.set, runtime: str = None, # noqa: RUF013 exclude: str = None, # noqa: RUF013 @@ -967,6 +967,552 @@ def _gate_project_scoped_runtimes( out = [r for r in out if r != rt] return out + @staticmethod + def _normalise_user_scope(scope, user_scope: bool) -> bool: + from apm_cli.core.scope import InstallScope + + if scope is InstallScope.USER: + return True + if scope is InstallScope.PROJECT: + return False + return user_scope + + @staticmethod + def _split_mcp_dependencies(mcp_deps: list) -> tuple[list, list, list, dict]: + registry_deps = [ + dep + for dep in mcp_deps + if isinstance(dep, str) + or (hasattr(dep, "is_registry_resolved") and dep.is_registry_resolved) + ] + self_defined_deps = [ + dep for dep in mcp_deps if hasattr(dep, "is_self_defined") and dep.is_self_defined + ] + registry_dep_names = [dep.name if hasattr(dep, "name") else dep for dep in registry_deps] + registry_dep_map = {dep.name: dep for dep in registry_deps if hasattr(dep, "name")} + return registry_deps, self_defined_deps, registry_dep_names, registry_dep_map + + @staticmethod + def _print_mcp_install_header(console, logger, dep_count: int) -> None: + if console: + try: + from rich.text import Text + + header = Text() + header.append("+- MCP Servers (", style="cyan") + header.append(str(dep_count), style="cyan bold") + header.append(")", style="cyan") + console.print(header) + return + except Exception: + pass + logger.progress(f"Installing MCP dependencies ({dep_count})...") + + @staticmethod + def _load_apm_config(project_root_path: Path) -> dict | None: + try: + apm_yml = project_root_path / "apm.yml" + if apm_yml.exists(): + from apm_cli.utils.yaml_io import load_yaml + + return load_yaml(apm_yml) + except Exception: + return None + return None + + @staticmethod + def _discover_installed_runtimes(project_root_path: Path) -> list[str]: + try: + from apm_cli.factory import ClientFactory + from apm_cli.runtime.manager import RuntimeManager + + manager = RuntimeManager() + installed_runtimes = [] + for runtime_name in [ + "copilot", + "codex", + "vscode", + "cursor", + "opencode", + "gemini", + "windsurf", + "claude", + ]: + try: + if MCPIntegrator._runtime_is_available( + runtime_name, project_root_path, manager + ): + ClientFactory.create_client(runtime_name) + installed_runtimes.append(runtime_name) + except (ValueError, ImportError): + continue + return installed_runtimes + except ImportError: + installed_runtimes = [ + rt for rt in ["copilot", "codex"] if shutil.which(rt) is not None + ] + if _is_vscode_available(project_root=project_root_path): + installed_runtimes.append("vscode") + for runtime_name, dirname in [ + ("cursor", ".cursor"), + ("opencode", ".opencode"), + ("gemini", ".gemini"), + ("windsurf", ".windsurf"), + ]: + root = Path.cwd() if runtime_name == "gemini" else project_root_path + if (root / dirname).is_dir(): + installed_runtimes.append(runtime_name) + if (project_root_path / ".claude").is_dir() or shutil.which("claude") is not None: + installed_runtimes.append("claude") + return installed_runtimes + + @staticmethod + def _runtime_is_available(runtime_name: str, project_root_path: Path, manager) -> bool: + if runtime_name == "vscode": + return _is_vscode_available(project_root=project_root_path) + if runtime_name in ("cursor", "opencode", "windsurf"): + return (project_root_path / f".{runtime_name}").is_dir() + if runtime_name == "gemini": + return (Path.cwd() / ".gemini").is_dir() + if runtime_name == "claude": + return (project_root_path / ".claude").is_dir() or shutil.which("claude") is not None + return manager.is_runtime_available(runtime_name) + + @staticmethod + def _select_target_runtimes( + *, + runtime: str | None, + exclude: str | None, + verbose: bool, + apm_config: dict | None, + project_root, + logger, + console, + ) -> tuple[list[str], dict | None]: + if runtime: + logger.progress(f"Targeting specific runtime: {runtime}") + return [runtime], apm_config + + project_root_path = Path(project_root) if project_root is not None else Path.cwd() + if apm_config is None: + apm_config = MCPIntegrator._load_apm_config(project_root_path) + + installed_runtimes = MCPIntegrator._discover_installed_runtimes(project_root_path) + script_runtimes = MCPIntegrator._detect_runtimes( + apm_config.get("scripts", {}) if apm_config else {} + ) + target_runtimes = MCPIntegrator._runtimes_from_scripts( + installed_runtimes, script_runtimes, verbose, logger, console + ) + + if exclude: + target_runtimes = [r for r in target_runtimes if r != exclude] + if not target_runtimes and installed_runtimes: + logger.warning( + f"All installed runtimes excluded (--exclude {exclude}), skipping MCP configuration" + ) + return [], apm_config + if not target_runtimes and not installed_runtimes: + logger.progress("No runtimes installed, using VS Code as fallback") + return ["vscode"], apm_config + return target_runtimes, apm_config + + @staticmethod + def _runtimes_from_scripts( + installed_runtimes: list[str], + script_runtimes: list[str], + verbose: bool, + logger, + console, + ) -> list[str]: + if script_runtimes: + target_runtimes = [rt for rt in installed_runtimes if rt in script_runtimes] + if verbose: + if console: + console.print("| [cyan][i] Runtime Detection[/cyan]") + console.print(f"| +- Installed: {', '.join(installed_runtimes)}") + console.print(f"| +- Used in scripts: {', '.join(script_runtimes)}") + if target_runtimes: + console.print( + f"| +- Target: {', '.join(target_runtimes)} " + f"(available + used in scripts)" + ) + console.print("|") + else: + logger.verbose_detail(f"Installed runtimes: {', '.join(installed_runtimes)}") + logger.verbose_detail(f"Script runtimes: {', '.join(script_runtimes)}") + if target_runtimes: + logger.verbose_detail(f"Target runtimes: {', '.join(target_runtimes)}") + if not target_runtimes: + logger.warning("Scripts reference runtimes that are not installed") + logger.progress("Install missing runtimes with: apm runtime setup ") + return target_runtimes + + if installed_runtimes: + if verbose: + logger.verbose_detail( + f"No scripts detected, using all installed runtimes: " + f"{', '.join(installed_runtimes)}" + ) + else: + logger.warning("No MCP-compatible runtimes installed") + logger.progress("Install a runtime with: apm runtime setup copilot") + return installed_runtimes + + @staticmethod + def _filter_user_scope_runtimes(target_runtimes: list[str], scope, logger) -> list[str]: + from apm_cli.core.scope import InstallScope + + if scope is not InstallScope.USER: + return target_runtimes + + from apm_cli.factory import ClientFactory as _CF + + pre_filter = list(target_runtimes) + filtered_runtimes = [] + for rt in target_runtimes: + try: + client = _CF.create_client(rt) + except ValueError: + continue + if client.supports_user_scope: + filtered_runtimes.append(rt) + skipped = set(pre_filter) - set(filtered_runtimes) + if skipped: + msg = ( + f"Skipped workspace-only runtimes at user scope: " + f"{', '.join(sorted(skipped))} -- omit --global to install these" + ) + logger.warning(msg) + if not filtered_runtimes: + logger.warning("No runtimes support user-scope MCP installation (supported: copilot, codex)") + return filtered_runtimes + + @staticmethod + def _install_registry_dependencies( + *, + registry_deps: list, + registry_dep_names: list, + registry_dep_map: dict, + target_runtimes: list[str], + stored_mcp_configs: dict, + servers_to_update: builtins.set, + successful_updates: builtins.set, + verbose: bool, + console, + logger, + project_root, + user_scope: bool, + ) -> int: + if not registry_dep_names: + return 0 + try: + from apm_cli.registry.operations import MCPServerOperations + except ImportError: + logger.warning("Registry operations not available") + logger.error("Cannot validate MCP servers without registry operations") + raise RuntimeError("Registry operations module required for MCP installation") # noqa: B904 + + operations = MCPServerOperations() + if verbose: + logger.verbose_detail(f"Validating {len(registry_deps)} registry servers...") + valid_servers, invalid_servers = operations.validate_servers_exist(registry_dep_names) + if invalid_servers: + logger.error(f"Server(s) not found in registry: {', '.join(invalid_servers)}") + logger.progress("Run 'apm mcp search ' to find available servers") + raise RuntimeError(f"Cannot install {len(invalid_servers)} missing server(s)") + if not valid_servers: + return 0 + + servers_to_install = operations.check_servers_needing_installation( + target_runtimes, + valid_servers, + project_root=project_root, + user_scope=user_scope, + ) + already_configured_servers = MCPIntegrator._prepare_registry_install_list( + valid_servers, + servers_to_install, + registry_dep_map, + stored_mcp_configs, + servers_to_update, + ) + if not servers_to_install: + MCPIntegrator._print_already_configured( + already_configured_servers, console, logger, "registry MCP servers" + ) + return 0 + MCPIntegrator._print_already_configured( + already_configured_servers, console, logger, "registry MCP servers" + ) + if verbose: + logger.verbose_detail(f"Installing {len(servers_to_install)} servers...") + server_info_cache = operations.batch_fetch_server_info(servers_to_install) + for server_name in servers_to_install: + dep = registry_dep_map.get(server_name) + if dep: + MCPIntegrator._apply_overlay(server_info_cache, dep) + shared_env_vars = operations.collect_environment_variables( + servers_to_install, server_info_cache + ) + for server_name in servers_to_install: + dep = registry_dep_map.get(server_name) + if dep and dep.env: + shared_env_vars.update(dep.env) + shared_runtime_vars = operations.collect_runtime_variables( + servers_to_install, server_info_cache + ) + return MCPIntegrator._configure_registry_servers( + servers_to_install, + target_runtimes, + server_info_cache, + shared_env_vars, + shared_runtime_vars, + servers_to_update, + successful_updates, + verbose, + console, + logger, + project_root, + user_scope, + ) + + @staticmethod + def _prepare_registry_install_list( + valid_servers: list, + servers_to_install: list, + registry_dep_map: dict, + stored_mcp_configs: dict, + servers_to_update: builtins.set, + ) -> list: + already_configured_candidates = [dep for dep in valid_servers if dep not in servers_to_install] + if stored_mcp_configs and already_configured_candidates: + drifted_reg_deps = [ + registry_dep_map[n] for n in already_configured_candidates if n in registry_dep_map + ] + drifted = MCPIntegrator._detect_mcp_config_drift( + drifted_reg_deps, stored_mcp_configs + ) + if drifted: + servers_to_update.update(drifted) + MCPIntegrator._append_drifted_to_install_list(servers_to_install, drifted) + return [dep for dep in already_configured_candidates if dep not in servers_to_update] + + @staticmethod + def _print_already_configured(items: list, console, logger, label: str) -> None: + if not items: + return + if console: + for dep in items: + console.print(f"| [green]+[/green] {dep} [dim](already configured)[/dim]") + else: + logger.verbose_detail(f"Already configured {label}: {', '.join(items)}") + + @staticmethod + def _configure_registry_servers( + servers_to_install: list, + target_runtimes: list[str], + server_info_cache: dict, + shared_env_vars: dict, + shared_runtime_vars: dict, + servers_to_update: builtins.set, + successful_updates: builtins.set, + verbose: bool, + console, + logger, + project_root, + user_scope: bool, + ) -> int: + configured_count = 0 + for dep in servers_to_install: + is_update = dep in servers_to_update + if console: + action_text = "Updating" if is_update else "Configuring" + console.print(f"| [cyan]>[/cyan] {dep}") + console.print( + f"| +- {action_text} for " + f"{', '.join([rt.title() for rt in target_runtimes])}..." + ) + any_ok = False + for rt in target_runtimes: + if verbose: + logger.verbose_detail(f"Configuring {rt}...") + if MCPIntegrator._install_for_runtime( + rt, + [dep], + shared_env_vars, + server_info_cache, + shared_runtime_vars, + project_root=project_root, + user_scope=user_scope, + logger=logger, + ): + any_ok = True + if any_ok: + if console: + label = "updated" if is_update else "configured" + console.print( + f"| [green]+[/green] {dep} -> " + f"{', '.join([rt.title() for rt in target_runtimes])}" + f" [dim]({label})[/dim]" + ) + configured_count += 1 + if is_update: + successful_updates.add(dep) + elif console: + console.print(f"| [red]x[/red] {dep} -- failed for all runtimes") + return configured_count + + @staticmethod + def _install_self_defined_dependencies( + *, + self_defined_deps: list, + target_runtimes: list[str], + stored_mcp_configs: dict, + servers_to_update: builtins.set, + successful_updates: builtins.set, + verbose: bool, + console, + logger, + project_root, + user_scope: bool, + ) -> int: + if not self_defined_deps: + return 0 + self_defined_names = [dep.name for dep in self_defined_deps] + self_defined_to_install = MCPIntegrator._check_self_defined_servers_needing_installation( + self_defined_names, + target_runtimes, + project_root=project_root, + user_scope=user_scope, + ) + already_configured = MCPIntegrator._prepare_self_defined_install_list( + self_defined_deps, + self_defined_to_install, + stored_mcp_configs, + servers_to_update, + ) + MCPIntegrator._print_already_configured( + already_configured, console, logger, "self-defined server(s)" + ) + configured_count = 0 + for dep in self_defined_deps: + if dep.name not in self_defined_to_install: + continue + if MCPIntegrator._configure_self_defined_server( + dep, + target_runtimes, + servers_to_update, + successful_updates, + verbose, + console, + logger, + project_root, + user_scope, + ): + configured_count += 1 + return configured_count + + @staticmethod + def _prepare_self_defined_install_list( + self_defined_deps: list, + self_defined_to_install: list, + stored_mcp_configs: dict, + servers_to_update: builtins.set, + ) -> list: + self_defined_names = [dep.name for dep in self_defined_deps] + already_configured_candidates = [ + name for name in self_defined_names if name not in self_defined_to_install + ] + if stored_mcp_configs and already_configured_candidates: + drifted_sd_deps = [ + dep for dep in self_defined_deps if dep.name in already_configured_candidates + ] + drifted_sd = MCPIntegrator._detect_mcp_config_drift( + drifted_sd_deps, stored_mcp_configs + ) + if drifted_sd: + servers_to_update.update(drifted_sd) + MCPIntegrator._append_drifted_to_install_list( + self_defined_to_install, drifted_sd + ) + return [name for name in already_configured_candidates if name not in servers_to_update] + + @staticmethod + def _configure_self_defined_server( + dep, + target_runtimes: list[str], + servers_to_update: builtins.set, + successful_updates: builtins.set, + verbose: bool, + console, + logger, + project_root, + user_scope: bool, + ) -> bool: + is_update = dep.name in servers_to_update + synthetic_info = MCPIntegrator._build_self_defined_info(dep) + self_defined_cache = {dep.name: synthetic_info} + self_defined_env = dep.env or {} + if console: + transport_label = dep.transport or "stdio" + action_text = "Updating" if is_update else "Configuring" + console.print( + f"| [cyan]>[/cyan] {dep.name} [dim](self-defined, {transport_label})[/dim]" + ) + console.print( + f"| +- {action_text} for " + f"{', '.join([rt.title() for rt in target_runtimes])}..." + ) + any_ok = False + for rt in target_runtimes: + if verbose: + logger.verbose_detail(f"Configuring {dep.name} for {rt}...") + if MCPIntegrator._install_for_runtime( + rt, + [dep.name], + self_defined_env, + self_defined_cache, + project_root=project_root, + user_scope=user_scope, + logger=logger, + ): + any_ok = True + if any_ok: + if console: + label = "updated" if is_update else "configured" + console.print( + f"| [green]+[/green] {dep.name} -> " + f"{', '.join([rt.title() for rt in target_runtimes])}" + f" [dim]({label})[/dim]" + ) + if is_update: + successful_updates.add(dep.name) + return True + if console: + console.print(f"| [red]x[/red] {dep.name} -- failed for all runtimes") + return False + + @staticmethod + def _print_mcp_install_summary( + console, + configured_count: int, + successful_updates: builtins.set, + ) -> None: + if not console: + return + if configured_count <= 0: + console.print("+- [green]All servers up to date[/green]") + return + update_count = builtins.len(successful_updates) + new_count = configured_count - update_count + parts = [] + if new_count > 0: + parts.append(f"configured {new_count} server{'s' if new_count != 1 else ''}") + if update_count > 0: + parts.append(f"updated {update_count} server{'s' if update_count != 1 else ''}") + console.print(f"+- [green]{', '.join(parts).capitalize()}[/green]") + @staticmethod def install( mcp_deps: list, @@ -1011,216 +1557,26 @@ def install( logger.warning("No MCP dependencies found in apm.yml") return 0 - from apm_cli.core.scope import InstallScope - - # The explicit scope enum takes precedence over the raw user_scope bool - # so callers cannot accidentally mix user-scope runtime filtering with - # project-scope config writes (or the inverse). - if scope is InstallScope.USER: - user_scope = True - elif scope is InstallScope.PROJECT: - user_scope = False - - # Split into registry-resolved and self-defined deps - # Backward compat: plain strings are treated as registry deps - registry_deps = [ - dep - for dep in mcp_deps - if isinstance(dep, str) - or (hasattr(dep, "is_registry_resolved") and dep.is_registry_resolved) - ] - self_defined_deps = [ - dep for dep in mcp_deps if hasattr(dep, "is_self_defined") and dep.is_self_defined - ] - registry_dep_names = [dep.name if hasattr(dep, "name") else dep for dep in registry_deps] - registry_dep_map = {dep.name: dep for dep in registry_deps if hasattr(dep, "name")} - + user_scope = MCPIntegrator._normalise_user_scope(scope, user_scope) + registry_deps, self_defined_deps, registry_dep_names, registry_dep_map = ( + MCPIntegrator._split_mcp_dependencies(mcp_deps) + ) console = _get_console() - # Track servers that were re-applied due to config drift servers_to_update: builtins.set = builtins.set() - # Track successful updates separately so the summary counts are accurate - # even when some drift-detected servers fail to install. successful_updates: builtins.set = builtins.set() if stored_mcp_configs is None: stored_mcp_configs = {} - # Start MCP section with clean header - if console: - try: - from rich.text import Text - - header = Text() - header.append("+- MCP Servers (", style="cyan") - header.append(str(len(mcp_deps)), style="cyan bold") - header.append(")", style="cyan") - console.print(header) - except Exception: - logger.progress(f"Installing MCP dependencies ({len(mcp_deps)})...") - else: - logger.progress(f"Installing MCP dependencies ({len(mcp_deps)})...") - - # Runtime detection and multi-runtime installation - if runtime: - # Single runtime mode - target_runtimes = [runtime] - logger.progress(f"Targeting specific runtime: {runtime}") - else: - project_root_path = Path(project_root) if project_root is not None else Path.cwd() - - if apm_config is None: - # Lazy load -- only when the caller doesn't provide it - try: - apm_yml = project_root_path / "apm.yml" - if apm_yml.exists(): - from apm_cli.utils.yaml_io import load_yaml - - apm_config = load_yaml(apm_yml) - except Exception: - apm_config = None - - # Step 1: Get all installed runtimes on the system - try: - from apm_cli.factory import ClientFactory - from apm_cli.runtime.manager import RuntimeManager - - manager = RuntimeManager() - installed_runtimes = [] - - for runtime_name in [ - "copilot", - "codex", - "vscode", - "cursor", - "opencode", - "gemini", - "windsurf", - "claude", - ]: - try: - if runtime_name == "vscode": - if _is_vscode_available(project_root=project_root_path): - ClientFactory.create_client(runtime_name) - installed_runtimes.append(runtime_name) - elif runtime_name == "cursor": - # Cursor is opt-in: only target when .cursor/ exists - if (project_root_path / ".cursor").is_dir(): - ClientFactory.create_client(runtime_name) - installed_runtimes.append(runtime_name) - elif runtime_name == "opencode": - # OpenCode is opt-in: only target when .opencode/ exists - if (project_root_path / ".opencode").is_dir(): - ClientFactory.create_client(runtime_name) - installed_runtimes.append(runtime_name) - elif runtime_name == "gemini": - # Gemini CLI is opt-in: only target when .gemini/ exists - if (Path.cwd() / ".gemini").is_dir(): - ClientFactory.create_client(runtime_name) - installed_runtimes.append(runtime_name) - elif runtime_name == "windsurf": - # Windsurf is opt-in: only target when .windsurf/ exists - if (project_root_path / ".windsurf").is_dir(): - ClientFactory.create_client(runtime_name) - installed_runtimes.append(runtime_name) - elif runtime_name == "claude": - # Claude Code is opt-in: target when .claude/ exists - # in the project (project-scope writes) OR when the - # `claude` binary is on PATH (user-scope writes). - # The PATH check is the gate that prevents the - # adapter from writing to ~/.claude.json on hosts - # where Claude Code was never installed. - if (project_root_path / ".claude").is_dir() or ( - shutil.which("claude") is not None - ): - ClientFactory.create_client(runtime_name) - installed_runtimes.append(runtime_name) - else: # noqa: PLR5501 - if manager.is_runtime_available(runtime_name): - ClientFactory.create_client(runtime_name) - installed_runtimes.append(runtime_name) - except (ValueError, ImportError): - continue - except ImportError: - installed_runtimes = [ - rt for rt in ["copilot", "codex"] if shutil.which(rt) is not None - ] - # VS Code: check binary on PATH or .vscode/ directory presence - if _is_vscode_available(project_root=project_root_path): - installed_runtimes.append("vscode") - # Cursor is directory-presence based, not binary-based - if (project_root_path / ".cursor").is_dir(): - installed_runtimes.append("cursor") - # OpenCode is directory-presence based - if (project_root_path / ".opencode").is_dir(): - installed_runtimes.append("opencode") - # Gemini CLI is directory-presence based - if (Path.cwd() / ".gemini").is_dir(): - installed_runtimes.append("gemini") - # Windsurf is directory-presence based - if (project_root_path / ".windsurf").is_dir(): - installed_runtimes.append("windsurf") - # Claude Code: directory-presence OR binary-on-PATH - if (project_root_path / ".claude").is_dir() or (shutil.which("claude") is not None): - installed_runtimes.append("claude") - - # Step 2: Get runtimes referenced in apm.yml scripts - script_runtimes = MCPIntegrator._detect_runtimes( - apm_config.get("scripts", {}) if apm_config else {} - ) - - # Step 3: Target runtimes BOTH installed AND referenced in scripts - if script_runtimes: - target_runtimes = [rt for rt in installed_runtimes if rt in script_runtimes] - - if verbose: - if console: - console.print("| [cyan][i] Runtime Detection[/cyan]") - console.print(f"| +- Installed: {', '.join(installed_runtimes)}") - console.print(f"| +- Used in scripts: {', '.join(script_runtimes)}") - if target_runtimes: - console.print( - f"| +- Target: {', '.join(target_runtimes)} " - f"(available + used in scripts)" - ) - console.print("|") - else: - logger.verbose_detail( - f"Installed runtimes: {', '.join(installed_runtimes)}" - ) - logger.verbose_detail(f"Script runtimes: {', '.join(script_runtimes)}") - if target_runtimes: - logger.verbose_detail(f"Target runtimes: {', '.join(target_runtimes)}") - - if not target_runtimes: - logger.warning("Scripts reference runtimes that are not installed") - logger.progress("Install missing runtimes with: apm runtime setup ") - else: - target_runtimes = installed_runtimes - if target_runtimes: - if verbose: - logger.verbose_detail( - f"No scripts detected, using all installed runtimes: " - f"{', '.join(target_runtimes)}" - ) - else: - logger.warning("No MCP-compatible runtimes installed") - logger.progress("Install a runtime with: apm runtime setup copilot") - - # Apply exclusions - if exclude: - target_runtimes = [r for r in target_runtimes if r != exclude] - - # All runtimes excluded -- nothing to configure - if not target_runtimes and installed_runtimes: - logger.warning( - f"All installed runtimes excluded (--exclude {exclude}), " - "skipping MCP configuration" - ) - return 0 - - # Fall back to VS Code only if no runtimes are installed at all - if not target_runtimes and not installed_runtimes: - target_runtimes = ["vscode"] - logger.progress("No runtimes installed, using VS Code as fallback") + MCPIntegrator._print_mcp_install_header(console, logger, len(mcp_deps)) + target_runtimes, apm_config = MCPIntegrator._select_target_runtimes( + runtime=runtime, + exclude=exclude, + verbose=verbose, + apm_config=apm_config, + project_root=project_root, + logger=logger, + console=console, + ) # Codex MCP is project-scoped: only configure it when Codex is an # active project target (silent skip, same as Cursor/OpenCode/Gemini). @@ -1238,288 +1594,38 @@ def install( if not target_runtimes: return 0 - # Scope filtering: at USER scope, keep only global-capable runtimes. - # Applied after both explicit --runtime and auto-discovery paths. - if scope is InstallScope.USER: - from apm_cli.factory import ClientFactory as _CF - - pre_filter = list(target_runtimes) - filtered_runtimes = [] - for rt in target_runtimes: - try: - client = _CF.create_client(rt) - except ValueError: - continue - if client.supports_user_scope: - filtered_runtimes.append(rt) - target_runtimes = filtered_runtimes - skipped = set(pre_filter) - set(target_runtimes) - if skipped: - msg = ( - f"Skipped workspace-only runtimes at user scope: " - f"{', '.join(sorted(skipped))}" - f" -- omit --global to install these" - ) - logger.warning(msg) - if not target_runtimes: - logger.warning( - "No runtimes support user-scope MCP installation (supported: copilot, codex)" - ) - return 0 - - # Use the new registry operations module for better server detection - configured_count = 0 - - # --- Registry-based deps --- - if registry_dep_names: - try: - from apm_cli.registry.operations import MCPServerOperations - - operations = MCPServerOperations() - - # Early validation: check all servers exist in registry (fail-fast) - if verbose: - logger.verbose_detail(f"Validating {len(registry_deps)} registry servers...") - valid_servers, invalid_servers = operations.validate_servers_exist( - registry_dep_names - ) - - if invalid_servers: - logger.error(f"Server(s) not found in registry: {', '.join(invalid_servers)}") - logger.progress("Run 'apm mcp search ' to find available servers") - raise RuntimeError(f"Cannot install {len(invalid_servers)} missing server(s)") - - if valid_servers: - servers_to_install = operations.check_servers_needing_installation( - target_runtimes, - valid_servers, - project_root=project_root, - user_scope=user_scope, - ) - already_configured_candidates = [ - dep for dep in valid_servers if dep not in servers_to_install - ] - - # Detect config drift for "already configured" servers - if stored_mcp_configs and already_configured_candidates: - drifted_reg_deps = [ - registry_dep_map[n] - for n in already_configured_candidates - if n in registry_dep_map - ] - drifted = MCPIntegrator._detect_mcp_config_drift( - drifted_reg_deps, - stored_mcp_configs, - ) - if drifted: - servers_to_update.update(drifted) - MCPIntegrator._append_drifted_to_install_list( - servers_to_install, drifted - ) - already_configured_servers = [ - dep for dep in already_configured_candidates if dep not in servers_to_update - ] - - if not servers_to_install: - if console: - for dep in already_configured_servers: - console.print( - f"| [green]+[/green] {dep} [dim](already configured)[/dim]" - ) - else: - logger.success("All registry MCP servers already configured") - else: - if already_configured_servers: - if console: - for dep in already_configured_servers: - console.print( - f"| [green]+[/green] {dep} [dim](already configured)[/dim]" - ) - else: - logger.verbose_detail( - "Already configured registry MCP servers: " - f"{', '.join(already_configured_servers)}" - ) - - # Batch fetch server info once - if verbose: - logger.verbose_detail( - f"Installing {len(servers_to_install)} servers..." - ) - server_info_cache = operations.batch_fetch_server_info(servers_to_install) - - # Apply overlays - for server_name in servers_to_install: - dep = registry_dep_map.get(server_name) - if dep: - MCPIntegrator._apply_overlay(server_info_cache, dep) - - # Collect env and runtime variables - shared_env_vars = operations.collect_environment_variables( - servers_to_install, server_info_cache - ) - for server_name in servers_to_install: - dep = registry_dep_map.get(server_name) - if dep and dep.env: - shared_env_vars.update(dep.env) - shared_runtime_vars = operations.collect_runtime_variables( - servers_to_install, server_info_cache - ) - - # Install for each target runtime - for dep in servers_to_install: - is_update = dep in servers_to_update - if console: - action_text = "Updating" if is_update else "Configuring" - console.print(f"| [cyan]>[/cyan] {dep}") - console.print( - f"| +- {action_text} for " - f"{', '.join([rt.title() for rt in target_runtimes])}..." - ) - - any_ok = False - for rt in target_runtimes: - if verbose: - logger.verbose_detail(f"Configuring {rt}...") - if MCPIntegrator._install_for_runtime( - rt, - [dep], - shared_env_vars, - server_info_cache, - shared_runtime_vars, - project_root=project_root, - user_scope=user_scope, - logger=logger, - ): - any_ok = True - - if any_ok: - if console: - label = "updated" if is_update else "configured" - console.print( - f"| [green]+[/green] {dep} -> " - f"{', '.join([rt.title() for rt in target_runtimes])}" - f" [dim]({label})[/dim]" - ) - configured_count += 1 - if is_update: - successful_updates.add(dep) - elif console: - console.print(f"| [red]x[/red] {dep} -- failed for all runtimes") - - except ImportError: - logger.warning("Registry operations not available") - logger.error("Cannot validate MCP servers without registry operations") - raise RuntimeError("Registry operations module required for MCP installation") # noqa: B904 - - # --- Self-defined deps (registry: false) --- - if self_defined_deps: - self_defined_names = [dep.name for dep in self_defined_deps] - self_defined_to_install = ( - MCPIntegrator._check_self_defined_servers_needing_installation( - self_defined_names, - target_runtimes, - project_root=project_root, - user_scope=user_scope, - ) - ) - already_configured_candidates_sd = [ - name for name in self_defined_names if name not in self_defined_to_install - ] - - # Detect config drift for "already configured" self-defined servers - if stored_mcp_configs and already_configured_candidates_sd: - drifted_sd_deps = [ - dep for dep in self_defined_deps if dep.name in already_configured_candidates_sd - ] - drifted_sd = MCPIntegrator._detect_mcp_config_drift( - drifted_sd_deps, - stored_mcp_configs, - ) - if drifted_sd: - servers_to_update.update(drifted_sd) - MCPIntegrator._append_drifted_to_install_list( - self_defined_to_install, drifted_sd - ) - already_configured_self_defined = [ - name for name in already_configured_candidates_sd if name not in servers_to_update - ] - - if already_configured_self_defined: - if console: - for name in already_configured_self_defined: - console.print(f"| [green]+[/green] {name} [dim](already configured)[/dim]") - else: - count = len(already_configured_self_defined) - logger.success(f"{count} self-defined server(s) already configured") - for name in already_configured_self_defined: - logger.verbose_detail(f"{name} already configured, skipping") - - for dep in self_defined_deps: - if dep.name not in self_defined_to_install: - continue - - is_update = dep.name in servers_to_update - synthetic_info = MCPIntegrator._build_self_defined_info(dep) - self_defined_cache = {dep.name: synthetic_info} - self_defined_env = dep.env or {} - - if console: - transport_label = dep.transport or "stdio" - action_text = "Updating" if is_update else "Configuring" - console.print( - f"| [cyan]>[/cyan] {dep.name} " - f"[dim](self-defined, {transport_label})[/dim]" - ) - console.print( - f"| +- {action_text} for " - f"{', '.join([rt.title() for rt in target_runtimes])}..." - ) - - any_ok = False - for rt in target_runtimes: - if verbose: - logger.verbose_detail(f"Configuring {dep.name} for {rt}...") - if MCPIntegrator._install_for_runtime( - rt, - [dep.name], - self_defined_env, - self_defined_cache, - project_root=project_root, - user_scope=user_scope, - logger=logger, - ): - any_ok = True - - if any_ok: - if console: - label = "updated" if is_update else "configured" - console.print( - f"| [green]+[/green] {dep.name} -> " - f"{', '.join([rt.title() for rt in target_runtimes])}" - f" [dim]({label})[/dim]" - ) - configured_count += 1 - if is_update: - successful_updates.add(dep.name) - elif console: - console.print(f"| [red]x[/red] {dep.name} -- failed for all runtimes") + target_runtimes = MCPIntegrator._filter_user_scope_runtimes( + target_runtimes, scope, logger + ) + if not target_runtimes: + return 0 - # Close the panel - if console: - if configured_count > 0: - # Use successful_updates (not servers_to_update) for accurate counts. - # servers_to_update = all drift-detected servers (some may have failed). - # successful_updates = servers that were re-applied AND succeeded. - update_count = builtins.len(successful_updates) - new_count = configured_count - update_count - parts = [] - if new_count > 0: - parts.append(f"configured {new_count} server{'s' if new_count != 1 else ''}") - if update_count > 0: - parts.append(f"updated {update_count} server{'s' if update_count != 1 else ''}") - console.print(f"+- [green]{', '.join(parts).capitalize()}[/green]") - else: - console.print("+- [green]All servers up to date[/green]") + configured_count = MCPIntegrator._install_registry_dependencies( + registry_deps=registry_deps, + registry_dep_names=registry_dep_names, + registry_dep_map=registry_dep_map, + target_runtimes=target_runtimes, + stored_mcp_configs=stored_mcp_configs, + servers_to_update=servers_to_update, + successful_updates=successful_updates, + verbose=verbose, + console=console, + logger=logger, + project_root=project_root, + user_scope=user_scope, + ) + configured_count += MCPIntegrator._install_self_defined_dependencies( + self_defined_deps=self_defined_deps, + target_runtimes=target_runtimes, + stored_mcp_configs=stored_mcp_configs, + servers_to_update=servers_to_update, + successful_updates=successful_updates, + verbose=verbose, + console=console, + logger=logger, + project_root=project_root, + user_scope=user_scope, + ) + MCPIntegrator._print_mcp_install_summary(console, configured_count, successful_updates) return configured_count diff --git a/src/apm_cli/marketplace/publisher.py b/src/apm_cli/marketplace/publisher.py index d4de9cb41..004c0b3d6 100644 --- a/src/apm_cli/marketplace/publisher.py +++ b/src/apm_cli/marketplace/publisher.py @@ -502,132 +502,261 @@ def _process(idx: int, target: ConsumerTarget) -> TargetResult: # -- per-target processing ---------------------------------------------- - def _process_single_target( + def _prepare_target_checkout( self, target: ConsumerTarget, plan: PublishPlan, - *, - dry_run: bool = False, - ) -> TargetResult: - """Clone, update, commit, and optionally push a single target.""" - with tempfile.TemporaryDirectory(prefix="apm-publish-") as tmpdir: - clone_dir = Path(tmpdir) / "repo" + clone_dir: Path, + tmpdir: str, + ) -> TargetResult | None: + url = f"https://github.com/{target.repo}.git" + try: + self._run_git( + [ + "git", + "clone", + "--depth=1", + "--branch", + target.branch, + url, + str(clone_dir), + ], + cwd=tmpdir, + ) + except subprocess.CalledProcessError as exc: + stderr = _redact_token(exc.stderr or "") + translated = translate_git_stderr( + stderr, + exit_code=exc.returncode, + operation="clone", + remote=target.repo, + ) + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=f"Clone failed: {translated.summary}", + ) - # 1. Shallow clone - url = f"https://github.com/{target.repo}.git" - try: - self._run_git( - [ - "git", - "clone", - "--depth=1", - "--branch", - target.branch, - url, - str(clone_dir), - ], - cwd=tmpdir, - ) - except subprocess.CalledProcessError as exc: - stderr = _redact_token(exc.stderr or "") - translated = translate_git_stderr( - stderr, - exit_code=exc.returncode, - operation="clone", - remote=target.repo, - ) - return TargetResult( - target=target, - outcome=PublishOutcome.FAILED, - message=f"Clone failed: {translated.summary}", - ) + try: + self._run_git(["git", "checkout", "-B", plan.branch_name], cwd=str(clone_dir)) + except subprocess.CalledProcessError as exc: + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=("Branch creation failed: " + _redact_token(str(exc))), + ) + return None - # 2. Create publish branch - try: - self._run_git( - ["git", "checkout", "-B", plan.branch_name], - cwd=str(clone_dir), - ) - except subprocess.CalledProcessError as exc: - return TargetResult( - target=target, - outcome=PublishOutcome.FAILED, - message=("Branch creation failed: " + _redact_token(str(exc))), - ) + def _load_target_apm_yml( + self, + target: ConsumerTarget, + clone_dir: Path, + ) -> tuple[Path, dict[str, Any]] | TargetResult: + apm_yml_path = clone_dir / target.path_in_repo + try: + ensure_path_within(apm_yml_path, clone_dir) + except PathTraversalError: + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=("Path traversal rejected: " + target.path_in_repo), + ) - # 3. Load consumer apm.yml - apm_yml_path = clone_dir / target.path_in_repo - try: - ensure_path_within(apm_yml_path, clone_dir) - except PathTraversalError: - return TargetResult( - target=target, - outcome=PublishOutcome.FAILED, - message=("Path traversal rejected: " + target.path_in_repo), - ) + if not apm_yml_path.exists(): + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=f"File not found: {target.path_in_repo}", + ) - if not apm_yml_path.exists(): - return TargetResult( - target=target, - outcome=PublishOutcome.FAILED, - message=f"File not found: {target.path_in_repo}", - ) + try: + raw_text = apm_yml_path.read_text(encoding="utf-8") + data = yaml.safe_load(raw_text) + except (yaml.YAMLError, OSError) as exc: + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=(f"Failed to parse {target.path_in_repo}: {exc}"), + ) + + if not isinstance(data, dict): + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message="Invalid apm.yml: expected a mapping", + ) + return apm_yml_path, data + + def _collect_marketplace_matches( + self, + target: ConsumerTarget, + plan: PublishPlan, + data: dict[str, Any], + ) -> tuple[list[tuple[int, str, str | None, str]], list[str]] | TargetResult: + deps = data.get("dependencies") + if not isinstance(deps, dict): + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=(f"Marketplace '{plan.marketplace_name}' not referenced in apm.yml"), + ) + apm_deps = deps.get("apm") + if not isinstance(apm_deps, list): + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=(f"Marketplace '{plan.marketplace_name}' not referenced in apm.yml"), + ) + + mkt_lower = plan.marketplace_name.lower() + matches: list[tuple[int, str, str | None, str]] = [] + warnings: list[str] = [] + for idx, entry_str in enumerate(apm_deps): + if not isinstance(entry_str, str): + continue try: - raw_text = apm_yml_path.read_text(encoding="utf-8") - data = yaml.safe_load(raw_text) - except (yaml.YAMLError, OSError) as exc: - return TargetResult( - target=target, - outcome=PublishOutcome.FAILED, - message=(f"Failed to parse {target.path_in_repo}: {exc}"), - ) + parsed = parse_marketplace_ref(entry_str) + except ValueError as exc: + warnings.append(str(exc)) + continue + if parsed is None: + continue + plugin_name, entry_mkt, old_ref = parsed + if entry_mkt.lower() == mkt_lower: + matches.append((idx, plugin_name, old_ref, entry_str)) + return matches, warnings + + def _check_publish_guards( + self, + target: ConsumerTarget, + plan: PublishPlan, + matches: list[tuple[int, str, str | None, str]], + new_ref: str, + ) -> TargetResult | None: + new_sv = parse_semver(new_ref.lstrip("vV")) + for _idx, _pname, old_ref, entry_str in matches: + if old_ref == new_ref: + continue + if old_ref is None: + if not plan.allow_ref_change: + return TargetResult( + target=target, + outcome=PublishOutcome.SKIPPED_REF_CHANGE, + message=(f"Entry '{entry_str}' uses implicit latest; pass allow_ref_change to pin"), + old_version=None, + new_version=new_ref, + ) + continue - if not isinstance(data, dict): + old_sv = parse_semver(old_ref.lstrip("vV")) + if old_sv is None and new_sv is not None and not plan.allow_ref_change: return TargetResult( target=target, - outcome=PublishOutcome.FAILED, - message="Invalid apm.yml: expected a mapping", + outcome=(PublishOutcome.SKIPPED_REF_CHANGE), + message=( + f"Entry '{entry_str}' uses non-semver ref '{old_ref}'; " + "pass allow_ref_change to switch" + ), + old_version=old_ref, + new_version=new_ref, ) - # 4. Find matching marketplace entries in dependencies.apm - deps = data.get("dependencies") - if not isinstance(deps, dict): + if old_sv and new_sv and new_sv < old_sv and not plan.allow_downgrade: return TargetResult( target=target, - outcome=PublishOutcome.FAILED, - message=(f"Marketplace '{plan.marketplace_name}' not referenced in apm.yml"), + outcome=(PublishOutcome.SKIPPED_DOWNGRADE), + message=( + f"Downgrade from {old_ref} to {new_ref}; " + "pass allow_downgrade to override" + ), + old_version=old_ref, + new_version=new_ref, ) + return None - apm_deps = deps.get("apm") - if not isinstance(apm_deps, list): - return TargetResult( - target=target, - outcome=PublishOutcome.FAILED, - message=(f"Marketplace '{plan.marketplace_name}' not referenced in apm.yml"), - ) + def _write_target_apm_yml(self, apm_yml_path: Path, data: dict[str, Any]) -> None: + new_text = yaml.safe_dump(data, default_flow_style=False, sort_keys=False) + tmp_yml = apm_yml_path.with_suffix(".yml.tmp") + try: + with open(tmp_yml, "w", encoding="utf-8") as fh: + fh.write(new_text) + fh.flush() + os.fsync(fh.fileno()) + os.replace(str(tmp_yml), str(apm_yml_path)) + except BaseException: + try: # noqa: SIM105 + tmp_yml.unlink(missing_ok=True) + except OSError: + pass + raise + + def _commit_target_update( + self, + target: ConsumerTarget, + plan: PublishPlan, + clone_dir: Path, + tmpdir: str, + ) -> TargetResult | None: + try: + self._run_git(["git", "add", target.path_in_repo], cwd=str(clone_dir)) + msg_file = Path(tmpdir) / "commit-msg.txt" + msg_file.write_text(plan.commit_message, encoding="utf-8") + self._run_git(["git", "commit", "-F", str(msg_file)], cwd=str(clone_dir)) + except subprocess.CalledProcessError as exc: + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=("Commit failed: " + _redact_token(str(exc))), + ) + return None - # Parse each entry with parse_marketplace_ref - new_ref = plan.new_ref - mkt_lower = plan.marketplace_name.lower() - matches: list[tuple[int, str, str | None, str]] = [] - warnings: list[str] = [] + def _push_target_update( + self, + target: ConsumerTarget, + plan: PublishPlan, + clone_dir: Path, + dry_run: bool, + ) -> TargetResult | None: + if dry_run: + return None + try: + self._run_git(["git", "push", "-u", "origin", plan.branch_name], cwd=str(clone_dir)) + except subprocess.CalledProcessError as exc: + stderr = _redact_token(exc.stderr or "") + return TargetResult( + target=target, + outcome=PublishOutcome.FAILED, + message=f"Push failed: {stderr}", + ) + return None - for idx, entry_str in enumerate(apm_deps): - if not isinstance(entry_str, str): - continue - try: - parsed = parse_marketplace_ref(entry_str) - except ValueError as exc: - warnings.append(str(exc)) - continue - if parsed is None: - continue # Direct repo ref -- not a marketplace entry - _plugin_name, entry_mkt, old_ref = parsed - if entry_mkt.lower() == mkt_lower: - matches.append((idx, _plugin_name, old_ref, entry_str)) + def _process_single_target( + self, + target: ConsumerTarget, + plan: PublishPlan, + *, + dry_run: bool = False, + ) -> TargetResult: + """Clone, update, commit, and optionally push a single target.""" + with tempfile.TemporaryDirectory(prefix="apm-publish-") as tmpdir: + clone_dir = Path(tmpdir) / "repo" + + checkout_error = self._prepare_target_checkout(target, plan, clone_dir, tmpdir) + if checkout_error: + return checkout_error + + loaded = self._load_target_apm_yml(target, clone_dir) + if isinstance(loaded, TargetResult): + return loaded + apm_yml_path, data = loaded + + collected = self._collect_marketplace_matches(target, plan, data) + if isinstance(collected, TargetResult): + return collected + matches, warnings = collected + new_ref = plan.new_ref - # 5. Zero matches -> FAILED if not matches: warn_suffix = "" if warnings: @@ -641,60 +770,10 @@ def _process_single_target( ), ) - # 6. Guards -- check every entry that would change - new_sv = parse_semver(new_ref.lstrip("vV")) + guard_result = self._check_publish_guards(target, plan, matches, new_ref) + if guard_result: + return guard_result - for _idx, _pname, old_ref, entry_str in matches: - if old_ref == new_ref: - continue # Already at target -- no guard needed - - # Ref-change guard - if old_ref is None: - # Implicit latest -> explicit pin - if not plan.allow_ref_change: - return TargetResult( - target=target, - outcome=PublishOutcome.SKIPPED_REF_CHANGE, - message=( - f"Entry '{entry_str}' uses implicit " - "latest; pass allow_ref_change to pin" - ), - old_version=None, - new_version=new_ref, - ) - else: - old_sv = parse_semver(old_ref.lstrip("vV")) - if old_sv is None and new_sv is not None: - # Non-semver ref -> semver tag - if not plan.allow_ref_change: - return TargetResult( - target=target, - outcome=(PublishOutcome.SKIPPED_REF_CHANGE), - message=( - f"Entry '{entry_str}' uses " - f"non-semver ref '{old_ref}'; " - "pass allow_ref_change to switch" - ), - old_version=old_ref, - new_version=new_ref, - ) - - # Downgrade guard - if old_sv and new_sv and new_sv < old_sv: - if not plan.allow_downgrade: - return TargetResult( - target=target, - outcome=(PublishOutcome.SKIPPED_DOWNGRADE), - message=( - f"Downgrade from {old_ref} to " - f"{new_ref}; pass allow_downgrade " - "to override" - ), - old_version=old_ref, - new_version=new_ref, - ) - - # 7. No-change check needs_update = any(old_ref != new_ref for _, _, old_ref, _ in matches) if not needs_update: return TargetResult( @@ -705,7 +784,7 @@ def _process_single_target( new_version=new_ref, ) - # 8. Apply updates to matching entries + apm_deps = data["dependencies"]["apm"] first_old_ref: str | None = None updated_count = 0 for idx, _pname, old_ref, entry_str in matches: @@ -720,63 +799,13 @@ def _process_single_target( apm_deps[idx] = f"{entry_str}#{new_ref}" updated_count += 1 - # 9. Write apm.yml atomically - new_text = yaml.safe_dump( - data, default_flow_style=False, sort_keys=False - ) # yaml-io-exempt - tmp_yml = apm_yml_path.with_suffix(".yml.tmp") - try: - with open(tmp_yml, "w", encoding="utf-8") as fh: - fh.write(new_text) - fh.flush() - os.fsync(fh.fileno()) - os.replace(str(tmp_yml), str(apm_yml_path)) - except BaseException: - try: # noqa: SIM105 - tmp_yml.unlink(missing_ok=True) - except OSError: - pass - raise - - # 10. Git add + commit - try: - self._run_git( - ["git", "add", target.path_in_repo], - cwd=str(clone_dir), - ) - msg_file = Path(tmpdir) / "commit-msg.txt" - msg_file.write_text(plan.commit_message, encoding="utf-8") - self._run_git( - ["git", "commit", "-F", str(msg_file)], - cwd=str(clone_dir), - ) - except subprocess.CalledProcessError as exc: - return TargetResult( - target=target, - outcome=PublishOutcome.FAILED, - message=("Commit failed: " + _redact_token(str(exc))), - ) - - # 11. Git push (unless dry_run) - if not dry_run: - try: - self._run_git( - [ - "git", - "push", - "-u", - "origin", - plan.branch_name, - ], - cwd=str(clone_dir), - ) - except subprocess.CalledProcessError as exc: - stderr = _redact_token(exc.stderr or "") - return TargetResult( - target=target, - outcome=PublishOutcome.FAILED, - message=f"Push failed: {stderr}", - ) + self._write_target_apm_yml(apm_yml_path, data) + commit_error = self._commit_target_update(target, plan, clone_dir, tmpdir) + if commit_error: + return commit_error + push_error = self._push_target_update(target, plan, clone_dir, dry_run) + if push_error: + return push_error old_label = first_old_ref or "unset" if updated_count == 1: diff --git a/src/apm_cli/marketplace/semver.py b/src/apm_cli/marketplace/semver.py index 612c9873f..f58665b88 100644 --- a/src/apm_cli/marketplace/semver.py +++ b/src/apm_cli/marketplace/semver.py @@ -202,18 +202,15 @@ def _satisfies_single(version: SemVer, spec: str) -> bool: return version >= base and version.major == base.major and version.minor == base.minor # Comparison operators - if spec.startswith(">="): - base = parse_semver(spec[2:]) - return base is not None and version >= base - if spec.startswith(">") and not spec.startswith(">="): - base = parse_semver(spec[1:]) - return base is not None and version > base - if spec.startswith("<="): - base = parse_semver(spec[2:]) - return base is not None and version <= base - if spec.startswith("<") and not spec.startswith("<="): - base = parse_semver(spec[1:]) - return base is not None and version < base + for operator, comparator in ( + (">=", lambda candidate, base: candidate >= base), + (">", lambda candidate, base: candidate > base), + ("<=", lambda candidate, base: candidate <= base), + ("<", lambda candidate, base: candidate < base), + ): + if spec.startswith(operator): + base = parse_semver(spec[len(operator) :]) + return base is not None and comparator(version, base) # Wildcard: 1.2.x or 1.2.* wildcard_match = re.match(r"^(\d+)\.(\d+)\.[xX*]$", spec) diff --git a/src/apm_cli/policy/policy_checks.py b/src/apm_cli/policy/policy_checks.py index 899b7d1c3..7deec343d 100644 --- a/src/apm_cli/policy/policy_checks.py +++ b/src/apm_cli/policy/policy_checks.py @@ -848,32 +848,32 @@ def _run(check: CheckResult) -> bool: return fail_fast and not check.passed # -- Dependency checks (1-6) ----------------------------------- - if _run(_check_dependency_allowlist(deps_list, policy.dependencies)): - return result - if _run(_check_dependency_denylist(deps_list, policy.dependencies)): - return result - if _run(_check_required_packages(deps_list, policy.dependencies)): - return result - if _run(_check_required_packages_deployed(deps_list, lockfile, policy.dependencies)): - return result - if _run(_check_required_package_version(deps_list, lockfile, policy.dependencies)): - return result - if _run(_check_transitive_depth(lockfile, policy.dependencies)): - return result + dependency_checks = ( + _check_dependency_allowlist(deps_list, policy.dependencies), + _check_dependency_denylist(deps_list, policy.dependencies), + _check_required_packages(deps_list, policy.dependencies), + _check_required_packages_deployed(deps_list, lockfile, policy.dependencies), + _check_required_package_version(deps_list, lockfile, policy.dependencies), + _check_transitive_depth(lockfile, policy.dependencies), + ) + for check in dependency_checks: + if _run(check): + return result # -- MCP checks (7-10) ---------------------------------------- # When mcp_deps is None (not provided), skip MCP checks entirely. # When mcp_deps is an empty list (provided but no MCP deps), still # run MCP checks so they report "no X configured" for completeness. if mcp_deps is not None: - if _run(_check_mcp_allowlist(mcp_list, policy.mcp)): - return result - if _run(_check_mcp_denylist(mcp_list, policy.mcp)): - return result - if _run(_check_mcp_transport(mcp_list, policy.mcp)): - return result - if _run(_check_mcp_self_defined(mcp_list, policy.mcp)): - return result + mcp_checks = ( + _check_mcp_allowlist(mcp_list, policy.mcp), + _check_mcp_denylist(mcp_list, policy.mcp), + _check_mcp_transport(mcp_list, policy.mcp), + _check_mcp_self_defined(mcp_list, policy.mcp), + ) + for check in mcp_checks: + if _run(check): + return result # -- Target / compilation checks (11-13) ----------------------- # Skipped when effective_target is None -- those run in a separate From 71784c5df002518b17ce8eba223631844b71c4b0 Mon Sep 17 00:00:00 2001 From: Zeel Date: Sat, 2 May 2026 21:34:16 -0400 Subject: [PATCH 2/3] fix: address stage 1 ruff review feedback --- src/apm_cli/commands/install.py | 10 +- src/apm_cli/integration/mcp_integrator.py | 172 ++++++++++++++-------- src/apm_cli/policy/policy_checks.py | 28 ++-- 3 files changed, 128 insertions(+), 82 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 6e7324837..7c857f768 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -1757,14 +1757,14 @@ def _post_install_summary(*, logger, apm_count, mcp_count, apm_diagnostics, forc class _InstallApmDependenciesOptions: force: bool = False parallel_downloads: int = 4 - logger: "InstallLogger" = None - scope: Any = None - auth_resolver: "AuthResolver" = None - target: str = None + logger: Optional["InstallLogger"] = None + scope: Any | None = None + auth_resolver: Optional["AuthResolver"] = None + target: str | None = None allow_insecure: bool = False allow_insecure_hosts: tuple[str, ...] = () marketplace_provenance: dict | None = None - protocol_pref: Any = None + protocol_pref: Any | None = None allow_protocol_fallback: bool | None = None no_policy: bool = False skill_subset: tuple | None = None diff --git a/src/apm_cli/integration/mcp_integrator.py b/src/apm_cli/integration/mcp_integrator.py index af0dfc807..8b057c6d5 100644 --- a/src/apm_cli/integration/mcp_integrator.py +++ b/src/apm_cli/integration/mcp_integrator.py @@ -9,13 +9,15 @@ (registry/operations.py) are *used* by this class, not modified. """ +from __future__ import annotations + import builtins import logging import re import shutil import warnings from pathlib import Path -from typing import List, Optional # noqa: F401, UP035 +from typing import TYPE_CHECKING, Any, List, Optional, Protocol # noqa: F401, UP035 import click # noqa: F401 @@ -31,6 +33,26 @@ _log = logging.getLogger(__name__) +if TYPE_CHECKING: + from apm_cli.core.scope import InstallScope + from apm_cli.models.dependency.mcp import MCPDependency + + +class _RuntimeManager(Protocol): + def is_runtime_available(self, runtime_name: str) -> bool: ... + + +class _MCPInstallLogger(Protocol): + def progress(self, message: str) -> None: ... + def verbose_detail(self, message: str) -> None: ... + def warning(self, message: str) -> None: ... + def error(self, message: str) -> None: ... + def success(self, message: str) -> None: ... + + +class _ConsolePrinter(Protocol): + def print(self, *objects: object, **kwargs: Any) -> None: ... + def _is_vscode_available(project_root: Path | str | None = None) -> bool: """Return True when VS Code can be targeted for MCP configuration. @@ -968,7 +990,7 @@ def _gate_project_scoped_runtimes( return out @staticmethod - def _normalise_user_scope(scope, user_scope: bool) -> bool: + def _normalise_user_scope(scope: InstallScope | None, user_scope: bool) -> bool: from apm_cli.core.scope import InstallScope if scope is InstallScope.USER: @@ -978,7 +1000,14 @@ def _normalise_user_scope(scope, user_scope: bool) -> bool: return user_scope @staticmethod - def _split_mcp_dependencies(mcp_deps: list) -> tuple[list, list, list, dict]: + def _split_mcp_dependencies( + mcp_deps: list[str | MCPDependency], + ) -> tuple[ + list[str | MCPDependency], + list[MCPDependency], + list[str], + dict[str, MCPDependency], + ]: registry_deps = [ dep for dep in mcp_deps @@ -993,7 +1022,11 @@ def _split_mcp_dependencies(mcp_deps: list) -> tuple[list, list, list, dict]: return registry_deps, self_defined_deps, registry_dep_names, registry_dep_map @staticmethod - def _print_mcp_install_header(console, logger, dep_count: int) -> None: + def _print_mcp_install_header( + console: _ConsolePrinter | None, + logger: _MCPInstallLogger, + dep_count: int, + ) -> None: if console: try: from rich.text import Text @@ -1067,7 +1100,11 @@ def _discover_installed_runtimes(project_root_path: Path) -> list[str]: return installed_runtimes @staticmethod - def _runtime_is_available(runtime_name: str, project_root_path: Path, manager) -> bool: + def _runtime_is_available( + runtime_name: str, + project_root_path: Path, + manager: _RuntimeManager, + ) -> bool: if runtime_name == "vscode": return _is_vscode_available(project_root=project_root_path) if runtime_name in ("cursor", "opencode", "windsurf"): @@ -1085,9 +1122,9 @@ def _select_target_runtimes( exclude: str | None, verbose: bool, apm_config: dict | None, - project_root, - logger, - console, + project_root: Path | str | None, + logger: _MCPInstallLogger, + console: _ConsolePrinter | None, ) -> tuple[list[str], dict | None]: if runtime: logger.progress(f"Targeting specific runtime: {runtime}") @@ -1122,8 +1159,8 @@ def _runtimes_from_scripts( installed_runtimes: list[str], script_runtimes: list[str], verbose: bool, - logger, - console, + logger: _MCPInstallLogger, + console: _ConsolePrinter | None, ) -> list[str]: if script_runtimes: target_runtimes = [rt for rt in installed_runtimes if rt in script_runtimes] @@ -1160,7 +1197,11 @@ def _runtimes_from_scripts( return installed_runtimes @staticmethod - def _filter_user_scope_runtimes(target_runtimes: list[str], scope, logger) -> list[str]: + def _filter_user_scope_runtimes( + target_runtimes: list[str], + scope: InstallScope | None, + logger: _MCPInstallLogger, + ) -> list[str]: from apm_cli.core.scope import InstallScope if scope is not InstallScope.USER: @@ -1191,17 +1232,17 @@ def _filter_user_scope_runtimes(target_runtimes: list[str], scope, logger) -> li @staticmethod def _install_registry_dependencies( *, - registry_deps: list, - registry_dep_names: list, - registry_dep_map: dict, + registry_deps: list[str | MCPDependency], + registry_dep_names: list[str], + registry_dep_map: dict[str, MCPDependency], target_runtimes: list[str], - stored_mcp_configs: dict, - servers_to_update: builtins.set, - successful_updates: builtins.set, + stored_mcp_configs: dict[str, Any], + servers_to_update: set[str], + successful_updates: set[str], verbose: bool, - console, - logger, - project_root, + console: _ConsolePrinter | None, + logger: _MCPInstallLogger, + project_root: Path | str | None, user_scope: bool, ) -> int: if not registry_dep_names: @@ -1279,12 +1320,12 @@ def _install_registry_dependencies( @staticmethod def _prepare_registry_install_list( - valid_servers: list, - servers_to_install: list, - registry_dep_map: dict, - stored_mcp_configs: dict, - servers_to_update: builtins.set, - ) -> list: + valid_servers: list[str], + servers_to_install: list[str], + registry_dep_map: dict[str, MCPDependency], + stored_mcp_configs: dict[str, Any], + servers_to_update: set[str], + ) -> list[str]: already_configured_candidates = [dep for dep in valid_servers if dep not in servers_to_install] if stored_mcp_configs and already_configured_candidates: drifted_reg_deps = [ @@ -1299,7 +1340,12 @@ def _prepare_registry_install_list( return [dep for dep in already_configured_candidates if dep not in servers_to_update] @staticmethod - def _print_already_configured(items: list, console, logger, label: str) -> None: + def _print_already_configured( + items: list[str], + console: _ConsolePrinter | None, + logger: _MCPInstallLogger, + label: str, + ) -> None: if not items: return if console: @@ -1310,17 +1356,17 @@ def _print_already_configured(items: list, console, logger, label: str) -> None: @staticmethod def _configure_registry_servers( - servers_to_install: list, + servers_to_install: list[str], target_runtimes: list[str], - server_info_cache: dict, - shared_env_vars: dict, - shared_runtime_vars: dict, - servers_to_update: builtins.set, - successful_updates: builtins.set, + server_info_cache: dict[str, Any], + shared_env_vars: dict[str, Any], + shared_runtime_vars: dict[str, Any], + servers_to_update: set[str], + successful_updates: set[str], verbose: bool, - console, - logger, - project_root, + console: _ConsolePrinter | None, + logger: _MCPInstallLogger, + project_root: Path | str | None, user_scope: bool, ) -> int: configured_count = 0 @@ -1366,15 +1412,15 @@ def _configure_registry_servers( @staticmethod def _install_self_defined_dependencies( *, - self_defined_deps: list, + self_defined_deps: list[MCPDependency], target_runtimes: list[str], - stored_mcp_configs: dict, - servers_to_update: builtins.set, - successful_updates: builtins.set, + stored_mcp_configs: dict[str, Any], + servers_to_update: set[str], + successful_updates: set[str], verbose: bool, - console, - logger, - project_root, + console: _ConsolePrinter | None, + logger: _MCPInstallLogger, + project_root: Path | str | None, user_scope: bool, ) -> int: if not self_defined_deps: @@ -1415,11 +1461,11 @@ def _install_self_defined_dependencies( @staticmethod def _prepare_self_defined_install_list( - self_defined_deps: list, - self_defined_to_install: list, - stored_mcp_configs: dict, - servers_to_update: builtins.set, - ) -> list: + self_defined_deps: list[MCPDependency], + self_defined_to_install: list[str], + stored_mcp_configs: dict[str, Any], + servers_to_update: set[str], + ) -> list[str]: self_defined_names = [dep.name for dep in self_defined_deps] already_configured_candidates = [ name for name in self_defined_names if name not in self_defined_to_install @@ -1440,14 +1486,14 @@ def _prepare_self_defined_install_list( @staticmethod def _configure_self_defined_server( - dep, + dep: MCPDependency, target_runtimes: list[str], - servers_to_update: builtins.set, - successful_updates: builtins.set, + servers_to_update: set[str], + successful_updates: set[str], verbose: bool, - console, - logger, - project_root, + console: _ConsolePrinter | None, + logger: _MCPInstallLogger, + project_root: Path | str | None, user_scope: bool, ) -> bool: is_update = dep.name in servers_to_update @@ -1495,9 +1541,9 @@ def _configure_self_defined_server( @staticmethod def _print_mcp_install_summary( - console, + console: _ConsolePrinter | None, configured_count: int, - successful_updates: builtins.set, + successful_updates: set[str], ) -> None: if not console: return @@ -1515,18 +1561,18 @@ def _print_mcp_install_summary( @staticmethod def install( - mcp_deps: list, + mcp_deps: list[str | MCPDependency], runtime: str = None, # noqa: RUF013 exclude: str = None, # noqa: RUF013 verbose: bool = False, - apm_config: dict = None, # noqa: RUF013 - stored_mcp_configs: dict = None, # noqa: RUF013 - project_root=None, + apm_config: dict[str, Any] | None = None, + stored_mcp_configs: dict[str, Any] | None = None, + project_root: Path | str | None = None, user_scope: bool = False, explicit_target: str | None = None, - logger=None, - diagnostics=None, - scope=None, + logger: _MCPInstallLogger | None = None, + diagnostics: Any = None, + scope: InstallScope | None = None, ) -> int: """Install MCP dependencies. diff --git a/src/apm_cli/policy/policy_checks.py b/src/apm_cli/policy/policy_checks.py index 7deec343d..51c5c1bec 100644 --- a/src/apm_cli/policy/policy_checks.py +++ b/src/apm_cli/policy/policy_checks.py @@ -849,15 +849,15 @@ def _run(check: CheckResult) -> bool: # -- Dependency checks (1-6) ----------------------------------- dependency_checks = ( - _check_dependency_allowlist(deps_list, policy.dependencies), - _check_dependency_denylist(deps_list, policy.dependencies), - _check_required_packages(deps_list, policy.dependencies), - _check_required_packages_deployed(deps_list, lockfile, policy.dependencies), - _check_required_package_version(deps_list, lockfile, policy.dependencies), - _check_transitive_depth(lockfile, policy.dependencies), + lambda: _check_dependency_allowlist(deps_list, policy.dependencies), + lambda: _check_dependency_denylist(deps_list, policy.dependencies), + lambda: _check_required_packages(deps_list, policy.dependencies), + lambda: _check_required_packages_deployed(deps_list, lockfile, policy.dependencies), + lambda: _check_required_package_version(deps_list, lockfile, policy.dependencies), + lambda: _check_transitive_depth(lockfile, policy.dependencies), ) - for check in dependency_checks: - if _run(check): + for build_check in dependency_checks: + if _run(build_check()): return result # -- MCP checks (7-10) ---------------------------------------- @@ -866,13 +866,13 @@ def _run(check: CheckResult) -> bool: # run MCP checks so they report "no X configured" for completeness. if mcp_deps is not None: mcp_checks = ( - _check_mcp_allowlist(mcp_list, policy.mcp), - _check_mcp_denylist(mcp_list, policy.mcp), - _check_mcp_transport(mcp_list, policy.mcp), - _check_mcp_self_defined(mcp_list, policy.mcp), + lambda: _check_mcp_allowlist(mcp_list, policy.mcp), + lambda: _check_mcp_denylist(mcp_list, policy.mcp), + lambda: _check_mcp_transport(mcp_list, policy.mcp), + lambda: _check_mcp_self_defined(mcp_list, policy.mcp), ) - for check in mcp_checks: - if _run(check): + for build_check in mcp_checks: + if _run(build_check()): return result # -- Target / compilation checks (11-13) ----------------------- From e27b737b889b096126424138dce242952c5ef1e6 Mon Sep 17 00:00:00 2001 From: Zeel Date: Mon, 4 May 2026 18:26:15 -0400 Subject: [PATCH 3/3] fix: resolve stage 1 CI blockers --- src/apm_cli/commands/install.py | 32 ++------------------- src/apm_cli/install/request.py | 32 ++++++++++++++++++++- src/apm_cli/integration/mcp_integrator.py | 35 +++++++++++------------ src/apm_cli/marketplace/publisher.py | 7 +++-- 4 files changed, 53 insertions(+), 53 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 548b9109b..2f0669ec5 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -1760,34 +1760,6 @@ def _post_install_summary( ) -@dataclasses.dataclass(frozen=True) -class _InstallApmDependenciesOptions: - force: bool = False - parallel_downloads: int = 4 - logger: Optional["InstallLogger"] = None - scope: Any | None = None - auth_resolver: Optional["AuthResolver"] = None - target: str | None = None - allow_insecure: bool = False - allow_insecure_hosts: tuple[str, ...] = () - marketplace_provenance: dict | None = None - protocol_pref: Any | None = None - allow_protocol_fallback: bool | None = None - no_policy: bool = False - skill_subset: tuple | None = None - skill_subset_from_cli: bool = False - legacy_skill_paths: bool = False - - @classmethod - def from_kwargs(cls, kwargs: dict[str, Any]) -> "_InstallApmDependenciesOptions": - known = {field.name for field in dataclasses.fields(cls)} - unknown = set(kwargs) - known - if unknown: - unknown_list = ", ".join(sorted(unknown)) - raise TypeError(f"unexpected install option(s): {unknown_list}") - return cls(**kwargs) - - # --------------------------------------------------------------------------- # Pipeline entry point -- thin re-export preserving the patch path # ``apm_cli.commands.install._install_apm_dependencies`` used by tests. @@ -1812,10 +1784,10 @@ def _install_apm_dependencies( if not APM_DEPS_AVAILABLE: raise RuntimeError("APM dependency system not available") - from apm_cli.install.request import InstallRequest + from apm_cli.install.request import InstallApmDependenciesOptions, InstallRequest from apm_cli.install.service import InstallService - options = _InstallApmDependenciesOptions.from_kwargs(kwargs) + options = InstallApmDependenciesOptions.from_kwargs(kwargs) request = InstallRequest( apm_package=apm_package, update_refs=update_refs, diff --git a/src/apm_cli/install/request.py b/src/apm_cli/install/request.py index 41b6f083a..3b5bd9781 100644 --- a/src/apm_cli/install/request.py +++ b/src/apm_cli/install/request.py @@ -8,7 +8,7 @@ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, fields from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple # noqa: F401, UP035 if TYPE_CHECKING: @@ -45,3 +45,33 @@ class InstallRequest: skill_subset: tuple[str, ...] | None = None # --skill filter for SKILL_BUNDLE packages skill_subset_from_cli: bool = False # True when user passed --skill (even --skill '*') legacy_skill_paths: bool = False # --legacy-skill-paths / APM_LEGACY_SKILL_PATHS + + +@dataclass(frozen=True) +class InstallApmDependenciesOptions: + """Compatibility options for the legacy commands.install wrapper.""" + + force: bool = False + parallel_downloads: int = 4 + logger: InstallLogger | None = None + scope: InstallScope | None = None + auth_resolver: AuthResolver | None = None + target: str | None = None + allow_insecure: bool = False + allow_insecure_hosts: tuple[str, ...] = () + marketplace_provenance: dict[str, Any] | None = None + protocol_pref: Any | None = None + allow_protocol_fallback: bool | None = None + no_policy: bool = False + skill_subset: tuple[str, ...] | None = None + skill_subset_from_cli: bool = False + legacy_skill_paths: bool = False + + @classmethod + def from_kwargs(cls, kwargs: dict[str, Any]) -> InstallApmDependenciesOptions: + known = {field.name for field in fields(cls)} + unknown = set(kwargs) - known + if unknown: + unknown_list = ", ".join(sorted(unknown)) + raise TypeError(f"unexpected install option(s): {unknown_list}") + return cls(**kwargs) diff --git a/src/apm_cli/integration/mcp_integrator.py b/src/apm_cli/integration/mcp_integrator.py index 8b057c6d5..105a10b59 100644 --- a/src/apm_cli/integration/mcp_integrator.py +++ b/src/apm_cli/integration/mcp_integrator.py @@ -1081,9 +1081,7 @@ def _discover_installed_runtimes(project_root_path: Path) -> list[str]: continue return installed_runtimes except ImportError: - installed_runtimes = [ - rt for rt in ["copilot", "codex"] if shutil.which(rt) is not None - ] + installed_runtimes = [rt for rt in ["copilot", "codex"] if shutil.which(rt) is not None] if _is_vscode_available(project_root=project_root_path): installed_runtimes.append("vscode") for runtime_name, dirname in [ @@ -1226,7 +1224,9 @@ def _filter_user_scope_runtimes( ) logger.warning(msg) if not filtered_runtimes: - logger.warning("No runtimes support user-scope MCP installation (supported: copilot, codex)") + logger.warning( + "No runtimes support user-scope MCP installation (supported: copilot, codex)" + ) return filtered_runtimes @staticmethod @@ -1326,14 +1326,14 @@ def _prepare_registry_install_list( stored_mcp_configs: dict[str, Any], servers_to_update: set[str], ) -> list[str]: - already_configured_candidates = [dep for dep in valid_servers if dep not in servers_to_install] + already_configured_candidates = [ + dep for dep in valid_servers if dep not in servers_to_install + ] if stored_mcp_configs and already_configured_candidates: drifted_reg_deps = [ registry_dep_map[n] for n in already_configured_candidates if n in registry_dep_map ] - drifted = MCPIntegrator._detect_mcp_config_drift( - drifted_reg_deps, stored_mcp_configs - ) + drifted = MCPIntegrator._detect_mcp_config_drift(drifted_reg_deps, stored_mcp_configs) if drifted: servers_to_update.update(drifted) MCPIntegrator._append_drifted_to_install_list(servers_to_install, drifted) @@ -1351,6 +1351,10 @@ def _print_already_configured( if console: for dep in items: console.print(f"| [green]+[/green] {dep} [dim](already configured)[/dim]") + elif label == "self-defined server(s)": + logger.success(f"{len(items)} {label} already configured") + for dep in items: + logger.verbose_detail(f"{dep} already configured, skipping") else: logger.verbose_detail(f"Already configured {label}: {', '.join(items)}") @@ -1474,14 +1478,10 @@ def _prepare_self_defined_install_list( drifted_sd_deps = [ dep for dep in self_defined_deps if dep.name in already_configured_candidates ] - drifted_sd = MCPIntegrator._detect_mcp_config_drift( - drifted_sd_deps, stored_mcp_configs - ) + drifted_sd = MCPIntegrator._detect_mcp_config_drift(drifted_sd_deps, stored_mcp_configs) if drifted_sd: servers_to_update.update(drifted_sd) - MCPIntegrator._append_drifted_to_install_list( - self_defined_to_install, drifted_sd - ) + MCPIntegrator._append_drifted_to_install_list(self_defined_to_install, drifted_sd) return [name for name in already_configured_candidates if name not in servers_to_update] @staticmethod @@ -1507,8 +1507,7 @@ def _configure_self_defined_server( f"| [cyan]>[/cyan] {dep.name} [dim](self-defined, {transport_label})[/dim]" ) console.print( - f"| +- {action_text} for " - f"{', '.join([rt.title() for rt in target_runtimes])}..." + f"| +- {action_text} for {', '.join([rt.title() for rt in target_runtimes])}..." ) any_ok = False for rt in target_runtimes: @@ -1640,9 +1639,7 @@ def install( if not target_runtimes: return 0 - target_runtimes = MCPIntegrator._filter_user_scope_runtimes( - target_runtimes, scope, logger - ) + target_runtimes = MCPIntegrator._filter_user_scope_runtimes(target_runtimes, scope, logger) if not target_runtimes: return 0 diff --git a/src/apm_cli/marketplace/publisher.py b/src/apm_cli/marketplace/publisher.py index 004c0b3d6..18b359140 100644 --- a/src/apm_cli/marketplace/publisher.py +++ b/src/apm_cli/marketplace/publisher.py @@ -643,7 +643,9 @@ def _check_publish_guards( return TargetResult( target=target, outcome=PublishOutcome.SKIPPED_REF_CHANGE, - message=(f"Entry '{entry_str}' uses implicit latest; pass allow_ref_change to pin"), + message=( + f"Entry '{entry_str}' uses implicit latest; pass allow_ref_change to pin" + ), old_version=None, new_version=new_ref, ) @@ -667,8 +669,7 @@ def _check_publish_guards( target=target, outcome=(PublishOutcome.SKIPPED_DOWNGRADE), message=( - f"Downgrade from {old_ref} to {new_ref}; " - "pass allow_downgrade to override" + f"Downgrade from {old_ref} to {new_ref}; pass allow_downgrade to override" ), old_version=old_ref, new_version=new_ref,